Skip to content
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

SessionAuthentication can trigger an erroneous CSRF token error response #2376

Closed
avoidwork opened this issue Jan 5, 2015 · 10 comments
Closed

Comments

@avoidwork
Copy link

Hi,

I'm working on an API which was added to an existing django app, which utilizes a few auth strategies, including 'session', which is last in the list. If I authenticate against the web app, I receive a 'sessionid' cookie, and then if I use that cookie to make an API request, I'll end up in DRF authentication.py at line ~129 for 3.0, which is reason = CSRFCheck().process_view(request, None, (), {}).

The None is the bug. Without a resolver, the token check will not know if the view is @csrf_exempt, and my valid request results in a 403, with a body of CSRF Failed: CSRF cookie not set.. Passing an invalid token results in the expected 'invalid token' error, if the view wasn't exempt ;)

Changing the None to utilize a resolver should be sufficient to pass the check.

@avoidwork
Copy link
Author

The "fix" breaks existing tests (#2377).

avoidwork pushed a commit to fluidware/django-rest-framework that referenced this issue Jan 5, 2015
@tomchristie
Copy link
Member

if I use that cookie to make an API request

If you use a session cookie then you do need to include the CSRF token. The behavior seems entirely correct as described. Unclear to me that there's any issue here, tho happy to reconsider with more details.

@avoidwork
Copy link
Author

Well, it is a bit of an edge case, but this is the general steps.

  1. Log into django app, get the session cookie
  2. Using PostMan, or similar method, such that the same browser & cookie are utilized, interact with the DRF API, & send a POST to a view.
  3. CSRF error happens; which is programmed to occur.

The CSRF error is erroneous in this work flow, because there's no chance to receive a token; authentication & authorization have been mingled in a way that this scenario cannot be resolved, and telling someone "clear your cookies" doesn't seem like a good solution.

@thedrow
Copy link
Contributor

thedrow commented Jan 6, 2015

I think we saw a similar issue way back and we decided to disable CSRF because of it.

@tomchristie
Copy link
Member

The CSRF error is erroneous in this work flow, because there's no chance to receive a token;

It's a nonsensical workflow tho. Why are you including a session cookie in this context?

@tomchristie
Copy link
Member

Unclear what you'd expect the resolution to be - POST/PUT/DELETE requests with session authenticated views must be CSRF protected.

@avoidwork
Copy link
Author

It's not nonsensical; I've had a few people run into it over the last 2 weeks as they start utilizing the API more, and the django web app less. I believe said people flip between the web app & something like postman for their workflow purposes, which we are not in a position to say is right or wrong, they're simply doing it.

I don't have a good resolution, because it's a collision. I think the only solution is (for me) to tell the users to make a GET to retrieve a token first.

@tomchristie
Copy link
Member

users to make a GET to retrieve a token first.

If they're using SessionAuthentication then yes that has and will always be what they need to do - however session auth isn't really an appropriate scheme to use if you're not using browser based clients. It's appropriate in an AJAX environment where the client does have a CSRF token available.

The docs on session auth do already note all this tho I'd be happy to consider suggestions on differing emphasis or wording.

@avoidwork
Copy link
Author

Yeah, the two apps should be separated so this issue wouldn't be possible, but it is what it is.

@tomchristie
Copy link
Member

the two apps should be separated so this issue wouldn't be possible

Just document/recommend that API users use your token or whatever scheme.

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

No branches or pull requests

3 participants