plone / guillotina

Python AsyncIO data API to manage billions of resources
https://guillotina.readthedocs.io/en/latest/
Other
187 stars 51 forks source link

Status codes when authorised but without permission should be 403 #1091

Open JeffersonBledsoe opened 3 years ago

JeffersonBledsoe commented 3 years ago

Currently, if a user is signed-in and attempts to access content which they do not have permission to access, Guillotina respondes with a status code of 401. My understanding is that this status code is used to indicate that the authentication method is either missing or invalid. I propose that in these situations, the status code should instead be a 403. As noted in the linked MDN docs, this status code indicates that while the authentication method may be valid, the application logic does not allow access to this resource, such as if the user has insufficient rights to the resource.

masipcat commented 3 years ago

I like this! I think we could change the behavior on G7. What do you think @bloodbare @vangheem ?

vangheem commented 3 years ago

I like it. In fact, I can't believe we weren't already doing it!

I suppose the only potential negative here is exposure(tells you authentication was success but not authorization). I know some APIs these days that only ever hand out 404 and won't give you 401 or 403 to prevent exposing some of that knowledge(private github orgs/repos for example)

bloodbare commented 3 years ago

I agree also!

JeffersonBledsoe commented 3 years ago

I suppose the only potential negative here is exposure(tells you authentication was success but not authorization). I know some APIs these days that only ever hand out 404 and won't give you 401 or 403 to prevent exposing some of that knowledge(private github orgs/repos for example)

@vangheem Fairly good point. Would an app_settings option be suitable to configure the behaviour between always returning a 404 or returning something more descriptive? I suppose default behaviour of providing more information and allowing the user to opt-in to privacy features is the most developer-friendly?

vangheem commented 3 years ago

That could work.

Something as simple as:

{
  "status_codes": {
    "not_authenticated": 401,
    "not_authorized": 403
  }
}

But I'm open to anything that would be proposed.