search

zfcampus / zf-mvc-auth

BSD 3-Clause "New" or "Revised" License
42 stars 46 forks source link

Add handling for missing Authorization header (send a 401 on authorization failure if no identity is asserted) #97

Open jackdpeterson opened 9 years ago

jackdpeterson commented 9 years ago

Apigility-specific issue: When submitting a request where authorization is required (e.g., admin menu -> authorization -> GET) is checked and no Authorization header is provided a 403 is returned rather than a 401.

Because no identity is asserted by the original request the appropriate response would (from an "API as a black box" perspective) be a 401.

Due to the current authorization flow in Apigility a 403 is returned because a Guest Identity (default identity should authentication not happen) is not allowed to perform the action that an Authenticated Identity can perform.

This request is to add in some kind of listener to intercept the request and return a 401 when the guest identity is not allowed to perform an action and authorization is checked in the admin UI.

dorongutman commented 9 years ago

This is fixed in #83

jackdpeterson commented 9 years ago

This is not quite fixed in #83. Per discussions on IRC with @weierophinney and @nuxwin some additional configuration options are under consideration to send a 401 instead of a 403 when the authorization header is not provided for a resources that has the appropriate authorization header checked as an optional configuration parameter or as an external listener add-on (the purpose of this issue). The problem is that a guest identity is assumed to be asserted as a default in Apigility despite the HTTP request not making that kind of assertion as to its identity (via authorization header). This leads to an unintuitive 403 forbidden rather than a 401 unauthorized.

Example 1 - No headers provided GET /protected-resource HTTP/1.1 Host: www.example.local Cache-Control: no-cache

Response: 403 Forbidden

################

Example 2 - Bad headers provided: GET /protected-resource HTTP/1.1 Host: www.example.local Authorization: Bearer badHeader Cache-Control: no-cache

Response: 401

Example 1, which would be default request which may trigger an authentication event on a client-side application would not occur. Instead the client-side application would believe that its locally stored credentials (if applicable) were in fact not authorized to access that resource. In reality though, no header was sent on behalf of the client-side application.

Example 2, which is the poor-man's workaround client-side is to first append protected-resource requests with a bogus header when not-yet authenticated.

Yes, there are client-side workarounds;however, the current situation leads to an unintuitive 403 response that is unique to Apigility in that it has to do exclusively with how Apigility is processing its default identity assignment and then performing an authorization request against the default identity (rather than immediately returning a 401).

PowerKiKi commented 9 years ago

Same issue when trying to integrate angular-http-auth which expects 401 when there is no token for an API that requires one. But in practice we get a 403 which won't work as expected on the frontend side.

PowerKiKi commented 9 years ago

There are several issues and PR around this topic over the last few months. That seems to indicate that the situation is not clear and/or not tested properly:

I think those are the HTTP status that we should expect:

No credentials Invalid credentials Correct credentials
protected API 401 401 200
public API 200 200 200

Is there a discussion somewhere that we could follow to track the progress on this issue ?

jackdpeterson commented 9 years ago

Well, I may be a bit biased ... but I think #97 https://github.com/zfcampus/zf-mvc-auth/issues/97 is an issue that clearly that captures what the unintuitive 403 Forbidden problem. The comment below with the expected table makes sense to me; however, it is not how Apigiliy is currently designed to handle authentication / authorization without a backwards compatibility break.

I agree that this needs a little more love in terms of an optional switch to return a 401 when Guest Identity is provided (but not asserted by the requestor) against a protected resource. My workaround at the moment on client-side JS involves doing some extra detection steps to determine if I have credentials stored and so forth when intercepting 40[1,3] responses from the API before re-authenticating and re-submitting requests.

On Tue, Sep 22, 2015 at 1:34 AM, Adrien Crivelli notifications@github.com wrote:

There are several issues and PR around this topic over the last few months. That seems to indicate that the situation is not not clear and/or not tested properly:

I think those are the HTTP status that we should expect: No credentials Invalid credentials Correct credentials protected API 401 401 200 public API 200 200 200

Is there a discussion somewhere that we could follow to track the progress on this issue ?

— Reply to this email directly or view it on GitHub https://github.com/zfcampus/zf-mvc-auth/issues/97#issuecomment-142212649 .

Jack Peterson Jack.Peterson@gmail.com (425) 372 - 8514 (Cell)

nuxwin commented 8 years ago

@PowerKiKi

The problem with your approach is that you have either a public API or private API while in most cases, an API will provide both: public and private resources.

PowerKiKi commented 8 years ago

@nuxwin I may have expressed myself poorly, when I wrote "API", what I really meant was "one specific route in a single API", or in Apigility parlance "one specific RPC or REST service". But all of that doesn't change much the point I was trying to make: Apigilty behavior is wrong and unclear and it should be fixed IMHO.

nuxwin commented 8 years ago

@PowerKiKi

You exactly meant: https://github.com/zfcampus/zf-mvc-auth/issues/106 right?

PowerKiKi commented 8 years ago

Yes, I had the same issue as described in #106. And it is still not solved AFAIK.

But please try to refrain from copy/pasting very similar text on several issues. That makes it very hard to follow the discussion. IMHO #106 is a duplicate of this issue and should not have been opened.

nuxwin commented 8 years ago

@PowerKiKi

Copy/pasting? #106 is more concerned about reverting #92 which break initial design. Of course, I tried to explain the #92 purpose which is wrong after all, and profited to give my thinking about a solution for supporting public/private resources over same API. But right, there is many issues related to same domain. The problem is that ATM, there is nothing proposed (from implementation point of view).

nuxwin commented 8 years ago

@PowerKiKi

Fixes available here:

weierophinney commented 4 years ago

This repository has been closed and moved to laminas-api-tools/api-tools-mvc-auth; a new issue has been opened at https://github.com/laminas-api-tools/api-tools-mvc-auth/issues/15.