Skip to content

Use 401 with Authenticate header, when appropriate. #187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Use 401 with Authenticate header, when appropriate. #187

wants to merge 2 commits into from

Conversation

sebastibe
Copy link
Contributor

A 403 error response indicates that the client's request is formed correctly, but the REST API refuses to honor it. A 403 response is not a case of insufficient client credentials; that would be 401 ("Unauthorized").

403 should only be used to enforce application-level permissions.

Edit [@tomchristie] Replaced title from "Replace 403 by 401 error in permissions.IsAuthenticated"

@tomaszzielinski
Copy link

Just to clarify - 401 is connected to HTTP Authentication, not to the authentication in general. Therefore if e.g. UserLoggedInAuthentication is used and the test fails, 403 is the proper response.

@sebastibe
Copy link
Contributor Author

Where does the 401 Error should be used then?

I found it convenient to differentiate permissions from authorization for my client side. I think that what is also done in django-tastypie.

@tomaszzielinski
Copy link

@sebastibe
Copy link
Contributor Author

"The request requires user authentication." Yes I read this when I proposed my patch.

To precise my question: Where in django-rest-framework the 401 should be raised? Because for the moment it is raised nowhere.

@tomaszzielinski
Copy link

@sebastibe: I don't know where 401 belongs in django-rest-framework. I only wanted to promote good practices regarding HTTP status codes :)

@mjtamlyn
Copy link
Contributor

Whilst I acknowledge that the 401 spec does mention Basic Auth, the 403 spec says "Authorization will not help", which in this case is clearly not true. So neither fits exactly, and I think 401 is a closer fit.

@tomaszzielinski
Copy link

@lovelydinosaur lovelydinosaur mentioned this pull request Sep 27, 2012
@lovelydinosaur
Copy link
Member

If anyone is interested in tackling this I've sketched out what I think need to happen to do this properly.
Would really appreciate any help, along the lines of:

  • I write the docs.
  • You implement the code and tests.

Anyone fancy helping?....

@lovelydinosaur
Copy link
Member

Updated title to better reflect the work that needs to be done.

@lovelydinosaur
Copy link
Member

Written the docs for this in unauthenticated-response branch

@lovelydinosaur
Copy link
Member

What needs to happen now...

  • Request._authenticate should set a boolean flag on the request depending on if it was authenticated or not.
  • view.permission_denied should raise either Unauthenticated or PermissionDenied depending on that flag.
  • View.handle_exception should catch Unauthenticated responses, and call the .authenticate_header() method on the first authentication scheme to determine if a 401 or 403 should be returned.

@lovelydinosaur
Copy link
Member

Note also that I've split out AuthenticationFailed and NotAuthenticated exceptions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants