Neoteroi / BlackSheep

Fast ASGI web framework for Python
https://www.neoteroi.dev/blacksheep/
MIT License
1.86k stars 78 forks source link

AuthorizationStrategy returns the wrong HTTP status code for certain auth failure cases #371

Closed waweber closed 1 year ago

waweber commented 1 year ago

This is a bit of a nitpick, but some clients might take different actions depending on what 40X code was returned due to an authorization failure.

Try the example included in the docs:


@auth("admin")
@get("/admin")
async def only_for_administrators():
    # This method requires "admin" role in user's claims
    return ok("example")

If you attempt to access this endpoint as a logged in but non-admin user, the request fails with 401 Unauthorized when it perhaps should have returned a 403 Forbidden.

Documentation for 401 Unauthorized says:

The HyperText Transfer Protocol (HTTP) 401 Unauthorized response status code indicates that the client request has not been completed because it lacks valid authentication credentials for the requested resource.

This status code is sent with an HTTP WWW-Authenticate response header that contains information on how the client can request for the resource again after prompting the user for authentication credentials.

In other words, 401 Unauthorized is returned when a client hasn't provided valid credentials (no or invalid auth token, not logged in, etc.). If a client receives this error code, it might show a login prompt (as with HTTP Basic auth) or redirect the user to a login page.

Documentation for 403 Forbidden says:

The HTTP 403 Forbidden response status code indicates that the server understands the request but refuses to authorize it.

This status is similar to 401, but for the 403 Forbidden status code, re-authenticating makes no difference. The access is tied to the application logic, such as insufficient rights to a resource.

Meaning the user has provided valid credentials, but they aren't permitted to access the resource. A client might show an error message, but wouldn't prompt the user to log in.

The AuthorizationStrategy always seems to return a 401 Unauthorized error when it fails. It could be improved by providing a way to specify the manner in which the authorization failed, whether it be from missing credentials vs insufficient permissions.

This would allow a client to know the difference between having no permissions (403) vs the login being invalid or the session being expired (401) so it can take some action to re-authenticate.

Some users might want to even override the status code returned for either case: for some endpoints, if a user has insufficient permissions, they might want to return a 404 Not Found to deny the existence of the resource instead of revealing that a resource exists, but a user is not allowed to access it.

I am currently working around this by raising Forbidden in my Requirement class, but this short-circuits the processing of the rest of the policy.

RobertoPrevato commented 1 year ago

@waweber again, very good feedback. You are right (I was considering this in the past, too, when I came across a conversation on this subject on StackOverflow). I will wrap my mind around this and prioritize to improve the feature.

RobertoPrevato commented 1 year ago

Fixed at 1.2.17.