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

Missing CSRF token when cookie is set #385

Closed
dodumosu opened this issue Feb 1, 2021 · 5 comments
Closed

Missing CSRF token when cookie is set #385

dodumosu opened this issue Feb 1, 2021 · 5 comments

Comments

@dodumosu
Copy link

dodumosu commented Feb 1, 2021

thanks for all the work on the extension. works great!

i'm not sure what i'm doing wrong, but when i send the CSRF token in a cookie (with defaults for JWT_CSRF_IN_COOKIES, or it explicitly set to True), i get a 401 with the message that the CSRF token is missing. when i extract the token from the cookie and send it in the X-CSRF-TOKEN header, i am able to access the protected endpoint.

i seem to be having issues uploading screenshots, but i'm using httpie as my client

@vimalloc
Copy link
Owner

vimalloc commented Feb 1, 2021

The cookie is not enough, you need to include an X-CSRF-TOKEN header in each request to actually prevent the CSRF attack.

I'm re-working the documentation for the 4.0.0 branch right now. I think it is a lot more clear on all this stuff. It's not live yet, but let me post the new version here to see if it clears things up for you. I would love any feedback or questions you have after reading it!

Cookies
~~~~~~~
Cookies are a fantastic way of handling JWTs if you are using a web browser.
They offer some nice benefits compared to the headers approach:

* They can be configured to send only over HTTPS. This prevents a JWT from
  accidentally being sent, and possibly compromised, over an unsecure connection.
* They are stored in an http-only cookie, which prevents XSS attacks from being
  able to steal the underlying JWT.
* You Flask application can implicitly refresh JWTs that are close to expiring,
  which simplifies the logic of keeping active users logged in. More on this in
  the next section!

Of course, when using cookies you also need to do some additional work to prevent
Cross Site Request Forgery (CSRF) attacks. In this extension we handle this via
something called double submit verification.

The basic idea behind double submit verification is that a JWT coming from a
cookie will only be considered valid if a special double submit token is also
present in the request, and that double submit token must not be something that
is automatically sent by a web browser (ie it cannot be another cookie).

By default, we accomplish this by setting two cookies when someone logging in.
The first cookie contains the JWT, and encoded in that JWT is the double submit
token. This cookie is set as http-only, so that it cannot be access via javascript
(this is what prevents XSS attacks from being able to steal the JWT). The second
cookie we set contains only the same double submit token, but this time in a
cookie that is readable by javascript. Whenever a request is made, it needs to
include an `X-CSRF-TOKEN` header, with the value of the double submit token.
If the value in this header does not match the value stored in the JWT, the
request is kicked out as invalid.

Because the double submit token needs to be present as a header (which wont be
automatically sent on a request), and some malicious javascript running on a
different domain will not be able to read the cookie containing the double submit
token on your website, we have successfully thwarted any CSRF attacks.

This does mean that whenever you are making a request, you need to manually
include the `X-CSRF-TOKEN` header, otherwise your requests will be kicked
out as invalid too. Lets look at how to do that in javascript:

.. code-block :: javascript

  async function login() {
    await fetch('/login', {method: 'post'});
  }

  async function logout() {
    await fetch('/logout', {method: 'post'});
  }

  function getCookie(name) {
    const value = `; ${document.cookie}`;
    const parts = value.split(`; ${name}=`);
    if (parts.length === 2) return parts.pop().split(';').shift();
  }

  async function makeRequestWithJWT() {
    const options = {
      method: 'post',
      headers: {
        'X-CSRF-TOKEN': getCookie('csrf_access_token'),
      },
    };
    const response = await fetch('/protected', options);
    const result = await response.json();
    return result;
  }

Note that there are additional CSRF options, such as looking for the double
submit token in a form, changing cookie paths, etc, that can be used to
tailor things to the needs of your application. See
:ref:`Cross Site Request Forgery Options` for details.

@dodumosu
Copy link
Author

dodumosu commented Feb 1, 2021

@vimalloc that reads great in my opinion. well done on the new documentation! about the only recommendation i would add is the credentials: 'same-origin' to support sending of cookies in older browsers using fetch() (since we're talking about using cookies, after all).

this isn't directly related, but i'm used to CSRF tokens from Django and Flask-WTF, and in my experience with them, it's possible for a token to expire. does this apply to tokens here?

@dodumosu dodumosu closed this as completed Feb 1, 2021
@dodumosu dodumosu reopened this Feb 1, 2021
@dodumosu dodumosu closed this as completed Feb 1, 2021
@vimalloc-mavenlink
Copy link
Contributor

Thanks for the feedback about 'same-origin', I will incorporate that into the docs! 👍

As for the CSRF tokens, they are directly tied to the current JWT, so if the current JWT expires so does the CSRF token. Does that answer your question?

@dodumosu
Copy link
Author

dodumosu commented Feb 1, 2021

thank you, it does. looking forward to the new docs. if there's anything i can do to help even though i don't have a lot of confidence, please let me know.

@vimalloc-mavenlink
Copy link
Contributor

If you want to go over some of the new documentation that would actually be really helpful! Documentation is always one of the hardest parts for me, and having some more eyes on it to help with everything from spelling/grammar errors to reworking parts that just don't make a lot of sense would be hugely appreciated!

If you wanted to start reviewing the new documentation now you could do so by cloning this repository, checking out the 4.0.0-dev branch, and runningmake clean && make html && open _build/html/index.html in the docs/ directory (after you have installed the dev-requirements.txt). Alternatively you could wait until the docs were published and go over them at that point.

There is also a few bugs and feature work available, if you wanted to get your hands dirty with the code. #347, #333, and #299 could all be good candidates for that!

If you have any questions or want to discuss things further feel free to hop into our discord (linked in the readme), and we can coordinate in there!

Thanks! 👍

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