djangorestframework-simplejwt icon indicating copy to clipboard operation
djangorestframework-simplejwt copied to clipboard

Allow httpOnly cookie storage

Open jaycle opened this issue 6 years ago • 65 comments

Similar to #23 but with a different motivation.

To protect against XSS, I would like the option to store the JWT in an HttpOnly cookie. django-rest-framework-jwt has this feature as an optional setting but that project I believe is abandoned and also has a vulnerability due to preventing the usage of django's CSRF token (see: https://github.com/GetBlimp/django-rest-framework-jwt/pull/434). Combining an HttpOnly cookie with CSRF token would be a pretty rock solid solution.

References: https://stormpath.com/blog/where-to-store-your-jwts-cookies-vs-html5-web-storage https://stackoverflow.com/questions/44133536/is-it-safe-to-store-a-jwt-in-localstorage-with-reactjs

jaycle avatar Jan 26 '19 20:01 jaycle

This is super important for me going forward... and it should be for everyone who uses this project. It SEEMS to be rather trivial to set the cookie.

Unsure what other side effects this may have.

derek-adair avatar Feb 09 '19 18:02 derek-adair

@derek-adair I pick the cookie out in nginx and put it in the authorization header. This is a workaround, but it works for me.

fergyfresh avatar Apr 03 '19 19:04 fergyfresh

I'll hopefully be submitting a PR for this in the near future!

derek-adair avatar Apr 05 '19 15:04 derek-adair

Cool, @derek-adair . Looking forward to seeing that!

davesque avatar Apr 09 '19 01:04 davesque

I could give it a try too, but I'm not sure how the refresh tokens come into play with cookies.. I think they don't make sense with cookies,

  • If both are cookied, then both would be included in each request.. which takes away little secrecy the refresh token had.
  • If only one of the tokens is cookied, the other will be kept in the client, which makes it vulnerable to XSS attacks. In this case, it would make more sense to leave the access token in the client, since its lifespan is shorter

So.. I guess that in order to touch the less amount of code possible, the best solution would be to cookie both tokens, but yeah, the refresh tokens doesn't add any security here

mrodal avatar May 27 '19 03:05 mrodal

@mrodal - yes it does, when its an httpOnly cookie. This may only add an extra step, but exposing your tokens to native, insecure browser api's is not good.

https://blog.codinghorror.com/protecting-your-cookies-httponly/

derek-adair avatar May 29 '19 14:05 derek-adair

I'm pretty sure the scenario described is the way to go. If anyone else has a more thorough understanding of how to secure these tokens i'd love to learn about it.

While modern browsers support and prevent reading/writing via httpOnly. there are a fair amount of legacy browsers that allow for reading/writing of these headers. So this will really only slow down script kiddies, but its a pretty low effort feature that will add security.

It's my understanding that JWT is inherently insecure and requires certain steps to harden that are likely beyond the scope of this project. I'm certainly no expert on this subject so I welcome any corrections or direction on how to implement this project, or JWT in general... securely.

derek-adair avatar May 29 '19 14:05 derek-adair

what do you mean by "it does"? What I meant is that as far as i know, refresh tokens provide an extra security layer because they are less exposed (they are only used when the access token expires)

But if they are stored in cookies, both travel in each request, so I don't see how refresh tokens make it more secure

mrodal avatar May 29 '19 14:05 mrodal

@mrodal makes a good point. Refresh tokens are supposed to be sent over the wire less frequently (only when obtaining a new access token or access/refresh pair). It's a bit awkward in the context of web browsers, but JWTs can potentially be used by non-browser clients as well. The access/refresh dichotomy makes more sense in that broader scope.

davesque avatar May 29 '19 19:05 davesque

IIRC, another option would be that the refresh token cookie is only set for the refresh view's path. Then, it would only be sent over the wire for refresh requests in particular.

davesque avatar May 29 '19 19:05 davesque

@mrodal it's my understanding that httpOnly + same site cookie is more secure than storing it in memory or localStorage (XSS vulnerability).

If the refresh token is not persisted in a cookie where would you suggest storing it?

Honestly this shit is soooo obtuse... this is the healthiest dialog i've participated around this subject. I'm almost convinced to NOT implement JWT for web browsers. However, i'm not sure the alternative for a single page app.

IIRC, another option would be that the refresh token cookie is only set for the refresh view's path. Then, it would only be sent over the wire for refresh requests in particular.

Could you elaborate? Didn't know it was possible to do this.

derek-adair avatar May 31 '19 13:05 derek-adair

Does django mitigate XSS?

As someone who possesses JUST enough knowledge about security to fuck it up... i'm quite confused as to how to implement ANY jwt setup securely (in a web browser).

derek-adair avatar May 31 '19 13:05 derek-adair

If the refresh token is not persisted in a cookie where would you suggest storing it?

I would actually not use it at all when using cookies.. Since it requires extra logic in the front-end and, as I said before, I don't see the benefit from using it. This is what Im doing in a mobile app that's also going to be a spa in the future.

Take into account that storing it in the cookie makes you possibly vulnerable to CSRF attacks if you don't use CSRF tokens, since the auth token is automatically added to each request...

Does django mitigate XSS?

If you are using django templates, you are protected by default when printing data from the server side. But you could still be vulnerable if you generate content on the front-end, although last time I checked, browsers also help quite a lot to prevent these attacks.

mrodal avatar May 31 '19 13:05 mrodal

I would actually not use it at all when using cookies

So just make them re-auth when it expires and/or store the actual token for longer?

Since it requires extra logic in the front-end

Now that you mention it... man i wish i would have came to that conclusion before I implemented a replay request thing in my redux app.... ha!

This is what Im doing in a mobile app that's also going to be a spa in the future.

Right, it's clear this is where the miscommunication.

Take into account that storing it in the cookie makes you possibly vulnerable to CSRF attacks if you don't use CSRF tokens, since the auth token is automatically added to each request...

What about if you are using CSRF tokens? Should be good is my understanding.

derek-adair avatar May 31 '19 14:05 derek-adair

Jeeeeez. So the refresh token seems to be useless in a browser... however....

httpOnly cookie still seems essential for the access-token.

derek-adair avatar May 31 '19 14:05 derek-adair

So just make them re-auth when it expires and/or store the actual token for longer?

Make the expiration time longer.. assuming https is being used, the risk of it being captured in the middle is negligible, and since its an httpOnly cookie, from js it cant be accessed either.. the only possibility I see is an attacker gaining physical access to the device and copying the cookies, but that I believe is always present anyways

What about if you are using CSRF tokens? Should be good is my understanding

Yeah, if you are using both CSRF and httpOnly cookies for the access token you are pretty much covered

Please someone correct me if I'm missing something

mrodal avatar May 31 '19 15:05 mrodal

@derek-adair This is what I was talking about with the cookie path. If the refresh token is set for the refresh view path, the browser will only include a Cookie header with the refresh token when making requests to the refresh view. So the refresh token is still used more securely and isn't sent over the wire as often as an access token.

What @mrodal is saying about CSRF also applies. Since cookies are automatically sent with any request that matches them, they don't care about what initiated that request or from where it originated. So you can't really tell if the user triggered the request intentionally. You need to include the CSRF token with the request so that Django can verify the request came from an "authorized" source e.g. a redux app hosted on a trusted server. All this stuff is bothersome but necessary. Since JWTs are still relatively new tech, they require that you think more about the nuts and bolts of auth system and why they work the way they do.

As XSS goes, Django does provide out-of-the-box protection against it, but it's not perfect. There are still certain cases in which it can be disabled whether intentionally or unintentionally. You still need to think about how it could happen and verify that the possibility doesn't apply to you.

davesque avatar May 31 '19 18:05 davesque

An HttpOnly Cookie access token combined with a Refresh on a certain url (refresh view) looks nice to me.

As for SPA-clients, you can protect against CSRF by setting the x-requested-with header and checking that one on the server (see https://markitzeroday.com/x-requested-with/cors/2017/06/29/csrf-mitigation-for-ajax-requests.html).

@davesque How would you cover both web and mobile scenario's? I have the same django rest backend for web and mobile. As for mobile, it's ok to send both tokens and store them on the device (secure storage it is). When handling a web-request I want to use the cookie tokens?

yoerivdm avatar Dec 31 '19 13:12 yoerivdm

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 which means a new point of attack (as previously mentioned in the XSS and CSRF token misplacements). 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 avatar Feb 22 '20 00:02 Andrew-Chen-Wang

@Andrew-Chen-Wang I just think we need both :-)

I have one API for both web and mobile, would like to use cookies for the websession part and jwt for API auth. If we can't differentiate the method (cookie vs jwt) then the jwt-token should be stored in a httponly cookie, not readable by js. In this case a few more measures need to be taken to further secure the token.

https://blog.logrocket.com/jwt-authentication-best-practices/

yoerivdm avatar Feb 22 '20 09:02 yoerivdm

Shouldn't we store the refresh token rather than the access token inside the http only cookie. JWT access tokens are best stored in memory with a countdown to refresh using the refresh endpoint. If you store the access token in the http cookie, like this patch/plugin suggests, where do I store the refresh token.

belhassen07 avatar Feb 29 '20 17:02 belhassen07

I liked @davesque's idea of storing both in HttpOnly cookies but setting the refresh token cookie for the refresh path only. This way there's no need to store tokens in localStorage but the refresh token isn't sent with every request.

juhanakristian avatar Mar 02 '20 12:03 juhanakristian

What's the progress on this issue?

belhassen07 avatar May 04 '20 22:05 belhassen07

I've made a fork using a pull request allowing httponly and merged it with the current master. You can find it here: https://github.com/schlunsen/django-rest-framework-simplejwt

schlunsen avatar May 05 '20 06:05 schlunsen

DRF should really only be used for mobile application

Definitely false.

derek-adair avatar May 05 '20 12:05 derek-adair

@Andrew-Chen-Wang This is a really heated and opinionated subject to the point where some have sworn JWT is insecure by design.

You can read up on why you would want to store your tokens here. Honsetly not sure of the quality of that article I just really am sick to death of this subject.

EDIT:

If anything, you're going to be implementing a lot more JS

You set and read cookies. It's really not any different than any other storage method. The ideal solution would be transparent to the client and have no effect if you do not want to use secure cookies.

derek-adair avatar May 05 '20 12:05 derek-adair

@schlunsen Sticking with the original package looks safer to me especially when there's still discussions about the PR. I'm no django expert, so I can't tolerate risk, thank you!

belhassen07 avatar May 05 '20 15:05 belhassen07

@derek-adair If you use that fork (you can pip install via a link) and deploy and publish on GitHub a project using React, Django, and SimpleJWT, then I'd love to see it! I've also added my two cents to the PR that you should probably change. If anything, you may need to just download the fork, import it into your project, and manually change a couple things.

Otherwise, regarding your comment, I agree that this is a very heated topic -- to the point that Calude Peroz, a core Django Developer, made a PR for JWT implementation only to get fired down by other core developers (mainly James Bennett) for security reasons. The link to the discussion. However, I will need to point out that the links James sent regarding security issues is not up to date with the updated RFCs AFTER they have been fixed -- luckily, PyJWT was one of the libraries to be fixed after I'd say the 30 links (including child links) out of 45 (if I can count correctly and if I even read all of them) sent.

The reason I said DRF is for mobile apps is because when I look at React + DRF websites, they all use this package and they all have the same level of implementation issues. If you can create a package that utilizes this fork, then I'd be happy to look over it! I have a Swift and Kotlin repo for using SimpleJWT here; you can make a PR there, and, in the requirements.txt, change the drf-simplejwt to the forked repo. If it's a worthy example, then the merging process can quicken.

Anyhow, tests are not enough for these kinds of packages. A production ready test is necessary. If you'd like, you can also use https://github.com/pydanny/cookiecutter-django to setup your project, deploy on Heroku or an EC2 instance or python anywhere, and testing and analyzing can be done much faster from me and Dave. Otherwise, I promise, this PR will never get merged. It's a hyped fork with no real implementation, yet.

Andrew-Chen-Wang avatar May 10 '20 16:05 Andrew-Chen-Wang

@Andrew-Chen-Wang THANK you for this django thread. This should be a great read and maybe there is some clarity here. Like I said, I gave up on JWT do to my lack of comprehension of how one might exploit JWT AND the ridiculous/tedious arguments in said threads.

The reason I said DRF is for mobile apps is because when I look at React + DRF websites, they all use this package

I can agree that this project is only suited for mobile apps and not websites. One of the (presumably) many ways you can use DRF w/o this project is by having django serve the UI.

derek-adair avatar May 11 '20 12:05 derek-adair

Regarding a production ready test... I had started implementing JWT in a seed project I use. It currently shamefully uses localstorage, and I broke the refresh middleware at somepoint... but it could serve as starting point. I also built this under the assumption that you could set httpOnly cookies form JS (you cant).

Which has lead me to believe that what @fergyfresh is doing... (setting cookie in nginx) seems to be the way its supposed to be used.

derek-adair avatar May 11 '20 13:05 derek-adair