wp-graphql / wp-graphql-jwt-authentication

Authentication for WPGraphQL using JWT (JSON Web Tokens)
GNU General Public License v3.0
342 stars 74 forks source link

Set RefreshToken as HttpOnly cookie on server response (login/register) #73

Open henrikwirth opened 4 years ago

henrikwirth commented 4 years ago

As mentioned in this post: https://blog.hasura.io/best-practices-of-using-jwt-with-graphql

Persisting JWT token in localstorage (prone to XSS) < Persisting JWT token in an HttpOnly cookie (prone to CSRF, a little bit better for XSS) < Persisting refresh token in an HttpOnly cookie (safe from CSRF, a little bit better for XSS).

Note that while this method is not resilient to serious XSS attacks, coupled with the usual XSS mitigation techniques, an HttpOnly cookie is a recommended way persisting session related information. But by persisting our session indirectly via a refresh token, we prevent a direct CSRF vulnerability we would have had with a JWT token.

Note that the new SameSite cookie spec which is getting increased support in most browsers will make Cookie based approaches safe from CSRF attacks. It might not be a solution if your Auth and API servers are hosted on different domains, but it should work really well otherwise!

So therefore I believe, this would make sense.

jasonbahl commented 4 years ago

So, In looking into the Hasura article, it looks like they removed the HTTP Only cookie from their demo repo

https://github.com/chewtoys/hasura-backend-plus/pull/7/files#diff-0d0de5d9ddbfa61dbbb85bd82bc412bbL213-L218

we may want to follow-up with them to learn more about why

ricksteruk commented 4 years ago

Yes it would be really helpful if the login and refresh responses set an Http cookie with the refresh token.

I followed the guidelines on the Hasura article and have managed to set it up as they suggest, but I had to create custom wp rest login and refresh routes in order to send and receive the refresh token in the http cookie.

If you do implement a http cookie it's worth adding options for SameSite and Secure as well, because those caused me headaches when I tried testing out the system that had been working perfectly on the dev machine from different pcs.

It would also be helpful if the "refreshJwtAuthToken" mutation could supply the new jwtAuthExpiration. We need this in order to know when to do the next refresh.

As it's not supplied I had to use a second graphql() within my rest refresh function to grab it via the user query { user(id: "") { jwtAuthExpiration } }

Which meant I had to also send the user's id with the refresh request as well the the AuthToken.

( By the way Jason thanks for all your amazing work on WP-GraphQL :) )

henrikwirth commented 4 years ago

Hi @ricksteruk, so for the expiry time you can just use the token itself. It always is encoded in the JWT. I use jwt-decode for it.

Then if you want the cookie to be set, I created a branch here: https://github.com/wp-graphql/wp-graphql-jwt-authentication/tree/feature/add-set-cookies-option

ricksteruk commented 4 years ago

Hi @henrikwirth, I wish I'd spotted your branch before I went through setting it up my own way!

Are the Http cookies that your branch creates set as secure and do they have a SameSite setting? I had troubles with these and I've had to set mine as "SameSite: None" and Secure as well as being Http only.

I'm a beginner at this stuff so I learned quite a lot through the process anyway.

Thanks for the advice on the AuthExpiry - jwt-decode will be invaluable.

Rick

ricksteruk commented 4 years ago

Hi again @henrikwirth, I've had a proper look at the branch now!

So just to confirm I am understanding it correctly - you've changed the behaviour of the login mutation so that it uses wp_sign_on to create the standard wordpress cookies. In your use case for WooCommerce.

I'll test it out and see if any cookies get sent to me by wordpress when I log in (one it's been turned on by setting GRAPHQL_JWT_AUTH_SET_COOKIES).

Maybe if I feel brave I will see if I can modify the login mutation to send to refresh token as a cookie and the refresh mutation to receive the refresh token only as cookie (only when the GRAPHQL_JWT_AUTH_SET_COOKIES option is defined) - it's basically what I've already done in my own rest routes.

henrikwirth commented 4 years ago

@ricksteruk you might also need to use the CORS plugin (https://github.com/funkhaus/wp-graphql-cors) and enter your frontend domain. Don't activate the login mutation though.

ricksteruk commented 4 years ago

Hi @henrikwirth - thanks for your thoughts! Yes I did have CORS problems and tried out that plug-in, but in the end I was getting other errors so I had to add some other headers to my php, which mean I could do away with the CORS plug in. I'm currently using this in my php:

header('Access-Control-Allow-Origin: http://localhost:3000'); header('Access-Control-Allow-Credentials: true' ); header('Access-Control-Allow-Headers: Content-Type'); header('Access-Control-Allow-Methods: GET, POST, OPTIONS');

At the moment my WordPress install is actually live on the web on a secure address, so that is a good testing ground - but my React app is still on localhost. Hopefully when I move the React app online I will just change the: header('Access-Control-Allow-Origin: http:// to the new web address and all will be well!

I did test out the branch and it does not appear to be automatically sending cookies out on the /graphql endpoint (as expected I guess!) - if I fancy a break from coding my own app I might try modifying this login mutation to send the http cookies - but I reckon there are more experienced coders out there who would do a better job ;)

ricksteruk commented 4 years ago

Also on a related point - I just updated one of my machines to Mac Os Catalina 10.15.4 and Safari does not appear to accept the HTTP cookies unless you go into Safari preferences and disable the " Website Tracking: Prevent cross-site tracking " option which was ON by default.

I'd noticed similar behaviour with my iPad after I updated that this week as well.

What was happening was that the auth was working properly for the first 5 minutes until the original auth token ran out and then it would fail - so I could tell the refresh cookies were not making it through.

valstu commented 4 years ago

What's the status of this? Definitely something that would be useful. This would make it easier to get the user data on server side eg. with Next.js