djangorestframework-simplejwt
djangorestframework-simplejwt copied to clipboard
Add JWT httpOnly cookie storage.
refs #71
Very nice! I've tested this locally and it seems to work for me.
It does however not deal with the possible CSRF attacks as discussed in #71.
I would suggest going the same route as jpadilla/django-rest-framework-jwt#434 and injecting the csrf cookie in TokenViewBase responses. I would suggest making this configurable just as as in the linked PR and adding similar documentation.
In the documentation we should:
- Give an example how to protect a view against CSRF using
@method_decorator(csrf_protect), as DRF exempts allAPIViews from CSRF protection - Mention that the user can define a custom
CSRF_FAILURE_VIEWthat provides a nice DRF-like response, as opposed to the standard HTML output django generates
@davesque If we get this out of the way, could you look into merging this or is there still anything to discuss in the original issue?
What's the status of completing this PR? We would like to use this :)
@NiyazNz - thanks for your hard work on this. Are you aware of any issues with the current implementation? I’d like to integrate this ASAP.
@NiyazNz Thanks for your work. My project also customized in a similar way. I looking forward to integrating with this work as soon as possible.
Same as the above, would be awesome to implement this into the library.
Any chance this PR gets merged soon ?
Is there any reason I'm missing why it is not ?
Also waiting for this one.
Also waiting
I think, it would be very useful if you update TokenVerifyView with verification token from cookie. Now we have to pass token in request body
Also waiting for this one. I do not fully understand what is delaying PR. Is it something that lefts yet?
It's because I wrote this library for free and have a day job.
It's because I wrote this library for free and have a day job.
Dave I know that. My question is technically speaking what lefts to make the PR. Thanks for your job.
My question is technically speaking what lefts to make the PR.
Fair enough. I guess the honest answer is that I don't know because I haven't really had time to review it. On the other hand, it's a largish PR so the risk of something bad creeping in is higher. Because of that, I don't want to just merge it in if the tests pass without giving it a good look.
My suggestion to people is that they should pip install their own fork with a git url (see here) if they urgently need this functionality. Otherwise, they'll have to wait until I have time to review it and merge it in.
On the other hand, it's a largish PR so the risk of something bad creeping in is higher. Because of that, I don't want to just merge it in if the tests pass without giving it a good look.
Aah oks! I understand. Thanks! I'll do the fork.
You can use the original fork from NiyazNz, It's working perfectly
https://github.com/sieira/pycont/blob/master/api/build/requirements.base.txt#L4
Happy new year everyone, by the way
Could the TokenVerifyView be modified to verify the tokens in cookies as well?
I am very... confused as to the purpose of this? Why would you use JWT authentication for web when you have session cookies already? So assuming this is for the web application / the website, here's my opinion:
If anything, you're going to be implementing a lot more JS to grab the cookied tokens which means a new point of attack (as previously mentioned in the XSS and CSRF token misplacements or nonexistence). Seems more like a headache to me; providing this would be a disservice.
DRF should really only be used for mobile application and occasionally be used with AJAX on an insecure endpoint. Sure, you can tack on some IP rate limit, but, in reality, you should be using a user rate limit and sending 403 forbidden back to unauthenticated users. But with a user rate limit, you're sending refresh tokens via http which can easily be eavesdropped on... so again:
What's the point of this? Feels more like a security vulnerability to users than a practical feature.
Also a reminder: I could also be interpreting this issue incorrectly :) lemme know if I did.
@Andrew-Chen-Wang because of this response https://github.com/davesque/django-rest-framework-simplejwt/pull/157#discussion_r361431083
@Andrew-Chen-Wang I empathize with your concerns. However, for better or for worse, single page apps that use JWTs as proof of auth are extremely common these days. For that use-case, it's generally preferred that auth credentials are not kept in local storage just in case an attacker succeeds in performing XSS. The better option is to keep them in an http-only cookie. Then you can still get the benefits of stateless auth (avoid using session keys for DB-backed sessions) but use the more traditional delivery method for tokens (in cookies).
any update to merge this PR?
Hello!
I have implemented a version of this pull request with DRF and Angular running on different servers. If this has a chance to be merged, I could try to provide a minimal example of how it works and how it could be implemented in a SPA.
I think there is also some more work to be done in the doc, we'd have to move what in the README to the official doc, you may want to add a few tests etc... So there is still quite a bit of work to be done and if people don't want it, I'll just stop here and I'll just maintain the fork.
This is what I have at the moment: https://github.com/SimpleJWT/django-rest-framework-simplejwt/compare/master...AtuzSolution:jwt_cookie Which cleaned up and rebased version of https://github.com/SimpleJWT/django-rest-framework-simplejwt/pull/157 which also includes https://github.com/SimpleJWT/django-rest-framework-simplejwt/pull/175
If you want to test it just add this line git+git://github.com/AtuzSolution/django-rest-framework-simplejwt@jwt_3#egg=rest_framework_simplejwt
Hi all, I've found an article regarding React + Django by using session authentication: https://www.valentinog.com/blog/drf/ Please let me know if this is a suitable option and easier for everyone. I am still not convinced about this usage of SimpleJWT in a browser, but who knows until someone hacks into it properly. Again, let me know if that blog about using a single template will work for people using React, Angular, and Vue.js! @loicgasser @derek-adair
Also, any provided example would be great to see.
Serving my SPA via Django is a No Go for me. For the scope of my current project this is the right solution. I might need to serve some mobile applications with the same back-end in the future. Using JWT from the start might spare me a few headaches later on. If someone can hack into this setup, please show me the code. As I said I am happy maintaining my fork aside, I just wanted to share.
I have similar situation with current project, so work on this is greatly appreciated. Using Django + React as decoupled API + SPA (option 2 from cited article (https://www.valentinog.com/blog/drf/), mobile apps will be required in the future. So far i have been using @NiyazNz fork with no issues.
Switching to @loicgasser breaks using refresh token in cookie, i believe the problem is in views.py line 69 , where self.token_refresh_data_key was probably meant to be used.
Also, the link to install the fork does not work, use this instead: git+git://github.com/AtuzSolution/django-rest-framework-simplejwt@jwt_cookie#egg=rest_framework_simplejwt
Thanks @TychoVrahe yes you're correct the last commit was untested, I just fixed it (which means there is a test missing there.).
That's why I gave a link to a release which I then deleted :smile: to create jwt_3. (updated the original link)
This is what I currently use in dev.
git+git://github.com/AtuzSolution/django-rest-framework-simplejwt@jwt_3#egg=rest_framework_simplejwt
cool, seems to be working fine now
Friendly reminder for those asking for a merge/push. This project is looking for maintainers
I think this can be merged so long as the communication of the token isn't too frequent. An example repository SHOULD be setup as a de facto setup with this package in ALL frontend frameworks. Hearing about a DRF SimpleJWT hack in thousands of websites is not what anyone wants to hear.
I worry about XSS and DOS. If XSS happens, your site's kinda screwed altogether and no one would really care too much about getting someone's token (which could still happen if someone like a president or official gets hacked). I originally worried about XSS mostly because indirectly it affects your website's performance (gravely due to authentication.py. Edit: also forgot to mention, for the most part, people can't do firewalls and network security groups correctly... so basically DNS and SSH stuff happens...).
TL;DR There needs to be a default setup for SimpleJWT. That means using middle wear like Axios or something like that. @davesque can create repositories for each framework as template repositories (I guess for now, they'll be under my name, but Dave can transfer them to SimpleJWT organization).
Once there are at least two frameworks setup, then this could be merged.
I didn't know I could create repositories as a Triage member, but I can. Please take a look at the organization's repositories and contribute. That way, red+blue teamers can try it out asap and we can see a merger soon. Thank you!
Edit: The repositories are setup. Please go and fork them, and please contribute! You may update the requirements.txt file. When updating the requirements.txt file and pointing the requirement for simplejwt to a git repository, you MUST specify a commit SHA. Otherwise, we cannot faithfully track down which commit works and which doesn't. It must be consistent with updates.
React repository: https://github.com/SimpleJWT/drf-SimpleJWT-React
VueJS repository: https://github.com/SimpleJWT/drf-SimpleJWT-Vue
iOS and Android is already listed in the example repositories. You can find that here: https://github.com/Andrew-Chen-Wang/mobile-auth-example
If I am missing a framework, please generate a template from here: https://github.com/SimpleJWT/drf-SimpleJWT-server-template
Good luck and thanks for contributing! Remember, if you can set up a template repository, then this PR can be merged quicker. At least two JS frameworks will be needed with unit tests to be considered for merge.
Also, please use Travis with a .travis.yml file. That yellow circle, red X, and green check makes taking a look at everything much easier to the eye and to the brain. It also allows passer-bys to think that the repository is actually good-to-go.
thanks @Andrew-Chen-Wang for setting up the repositories. I use Angular but I have some knowledge of React. I'll see what I can do to help.
Something to consider for the next django versions. If you want to explicitly set the SameSite cookie to None (for instance to avoid Google Warnings) we will need to use a string. 'None' or 'none'.
https://github.com/django/django/commit/b33bfc383935cd26e19a2cf71d066ac6edd1425f