Fix #97 and #106

[nuxwin]

PR which allows to fix/close the following issues: #97 and #106

First of all, it should be noted that explainations below assume that #92 was never merged. That fix was a mistake and it is reverted by this new PR. See #106 for more details about #92 mistakes.

Expected behavior

• If a resource is private, client must be authenticated to access it. If it is not, it must be challenged with a 401 response
• If a resource is public, client must be allowed to access it, even if not authenticated (case of a guest identity)
• If client is authenticated but not authorized to access a resource (case of an authenticated identity with extended permissions system), a 403 response must be returned.

Current behavior

Authentication is needed for public and private resources, even if the developer makes them public. This is due to the fact that at the authentication level (authenticationPost), we are returning the response, leading to short-circuit of the route event on which authorization listeners are also attached (they have a lower priority than authentication listeners).

Thus, returning a response from the authentication layer, in case of an authentication failure or if the client get challenged, means that authorization rules for a guest identity are never evaluated and therefore, public resources are not because authentication is always required.

New behavior

Authentication layer

• In case of an authentication failure, the response is no longer returned. We ensure that the response is set on the MvcEvent instead. This allows to not short-circuit the route event, and thus, evaluate authorization rules in the context of a guest identity prior returning the response.

Authorization layer

• If the client is authorized to access the resource, the MVC response which can have been modified by authentication layer is reseted. This avoid challenging the client in case of a guest identity that is allowed to access the resource after all (public resources).
• The response, if already an ApiResponseProblem instance, is not overriden by the ZF\Apigility\MvcAuth\UnauthorizedListener listener. This allows sending correct response (401) in case of an authentication failure. (A separated PR for the zf-apigility module will be made according this change).

About 401 status code vs 403 status code

Generally speaking, the 403 status code involves an authenticated user. User is authenticated but is not authorized to access the requested resource. This is generally the case when an authenticated user try to access resource owned by another user.

Currently, Apigility provides a very basic permissions system, based on HTTP methods and static identities. Access to a resource requires either an authenticated identity or a guest identity. The extended permissions system implementation, if it is required, is left to the developer. Of course, a 403 response is still send by the authorization layer in case of an unauthorized client, but only if no authentication is required (case of an already authenticated user). This will be the case if the developer implements extended permissions system (e.g, conditional ACL with assertions).

Regarding the above, it should be noted that even if authentication and authorization layers have differents concerns, this doesn't prevent one layer (e.g. the authorization layer) to be aware of the other. Furthermore, it shouldn't be forgotten that these layers are intimately linked. In other words, we can see the zf-mvc-auth module as a component that provides sub-components (authentication, authorization) which are coworkers. As such, they can share information.

Final note: Sorry for the verbosity here. I wanted to be as clear as possible.

[nuxwin]

Note: This PR involves merging of another PR related to the zf-apigility module. I'll reference it here soon.

[nuxwin]

@weierophinney

Works done. Please review and see also the related PR made for zf-apigility module, which is available at zfcampus/zf-apigility#146

Thank you for your time.

[jackdpeterson]

Thanks for the work on this! I look forward to testing on my end. Very much appreciated!

[nuxwin]

@jackdpeterson

Thanks. ;)

I'll now check behavior of Oauth authentication adapter. For me, current behavior is wrong (403 instead of 401) when authentication is expected... Behavior should be consistent over all adpaters.

[nuxwin]

After more thinking, I think that the solution provided here is bad because this involve too much kaking in existents listeners (eg, resetting response and so on). Right now, close this PR. I'll provide a new one soon.