-
Notifications
You must be signed in to change notification settings - Fork 597
[Exploratory Testing] No way of distinguishing 401 vs 403 for apis. #134
Comments
Back in november we discussed this, that this time we want to get it right - so please do. |
Related issue (in MVC): aspnet/Mvc#634 👯 |
Please file a bug stating for both the cookie and bearer authn middleware: If the middleware is processing an outgoing 401, and the middleware has produced an identity for the current request, then it should change the status code to 403 and should not redirect. |
I can take a look at making this change as part of the security auth changes, gives me an excuse to dig deeper into guts of the cookie/bearer code |
@lodejard @HaoK hum, this change seems really weird. Why do you think the cookies or bearer middleware should be responsible of setting the response status code? IMHO, the component responsible of doing the authorization stuff should set the 403 status code when needed. In MVC, you should change that line to use 403 when the user has been correctly authenticated: https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNet.Mvc.Core/Filters/AuthorizationFilterAttribute.cs#L34 |
This is an odd place to implement this. It is at the very least incomplete since something like Facebook or OIDC could convert the 401 to a redirect before getting to the cookie auth middleware, so you'd still be stuck in a loop. Doing this centrally in the authorization layer would be less error prone than trying to handle it in the individual authentication components. |
To be honest I agree with you as well. We know exactly in the authorize filter when to return a 403 (only after the authorize policy fails) and it's much simpler than sprinkling this 401 to 403 logic all over the various auth middlewares. But @loudej feels strongly that this is how it should work. Perhaps we can get some insight into why this approach is better? |
And to underscore one thing; The 401 (Unauthorized) status code indicates that the request has not been applied because it lacks valid authentication credentials for the target resource. The server generating a 401 response MUST send a WWW-Authenticate header field (Section 4.1) containing at least one challenge applicable to the target resource. |
Having Cookie Auth generate a 403 is even stranger than I originally thought. To be consistent with the 401 scenario that redirects to LoginPath, it should redirect to a similar AccessDeniedPath if set. |
Yes, a new [HandleForbidden] might be in order: |
Summary of current thinking:
|
@HaoK the more I think about it, the more I realize that it's semantically incorrect: // Simulate Authorization failure
var result = await context.AuthenticateAsync(CookieAuthenticationDefaults.AuthenticationType);
res.Challenge(CookieAuthenticationDefaults.AuthenticationType); It actually corresponds to a different scenario, where the user has been correctly authenticated but you want him to re-authenticate anyway. In this case, you can't alter the 401 response to return a 403 one, because the client is not supposed to try to re-authenticate.
|
Merged 775eb5e |
Re-opening and linking to #246 |
I'm creating more work now, because it's fun for me :D |
Proposed changes are here: #273 Closing as dupe |
Since we do not have a separate authentication phase, we do authentication and authorization while doing authorization. Since the current overloads only return a bool, there is currently no way of distinguishing between an authentication failure ( 401 ) vs an authorization failure ( 403 ).
Also read #133.
The text was updated successfully, but these errors were encountered: