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

Security clarification: "wherever" is not a good place to store JWTs in the browser #188

Open
jkantr opened this issue Dec 30, 2019 · 1 comment
Assignees

Comments

@jkantr
Copy link
Contributor

jkantr commented Dec 30, 2019

This note is more for myself - but feel free to engage in discussion if there's any necessary.

Problem

The security page (https://www.graphile.org/postgraphile/security/) has some general practices regarding JWT use. One in particular can be a huge security pitfall the way it's notated.

Within the code snippet under "Sending JWTs to the server":

// get the authentication token from wherever you store it

Although it's not necessarily in a browser context, i think it's pretty safe to say that is the general majority use of apollo-client. While it's well known that JWTs should never be stored in arbitrarily accessible locations (such as localStorage and sessionStorage, see: owsap reasonings), unfortunately there are a lot of careless "tutorials" around from a number of years ago when JWT first became a hot topic that claim cookies are dead and shouldn't be used. It's gotten better, but even today a google search for "where to store authentication token" reveals a 3rd position result to SO with a selected "answer" that touts html5 storage (localStorage) as the superior option.

Solution

While I think the discussion in graphile/crystal#1049 is an important one, and there should be probably be a section just as large discussing cookie/session auth + maybe a note that the creator of postgraphile doesn't use JWT themselves.... for now I think it's important this specific comment gets replaced with either a note / link to a security-related source or a small subsection explaining the attack vectors - which is the scope of this issue.

I'll make a note when I actually take this on - but please just leave a comment if someone else wants to make this fix in the meantime so we don't duplicate work.

@benjie
Copy link
Member

benjie commented Jan 7, 2020

I'd like to make it so that we do not take responsibility or make recommendations for where people store JWTs, or even for the sources of truth about what these places should be/should not be (except large well trusted sources like OWASP, W3, MDN, etc). So I'm definitely for changing this to something like:

/*
 * Get the authentication token from where it's stored. (WARNING: ensure you
 * do sufficient research before choosing where to store your JWT; there are
 * serious security ramifications if you get this wrong.)
 */

Possibly with a link to a few places (e.g. OWASP) that outline some of the security issues.

Please feel free to raise a PR with a change along these lines 👍

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

No branches or pull requests

2 participants