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

Do not check JWT_TOKEN_LOCATION when testing if cookie_csrf_protect is enabled #538

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

vimalloc
Copy link
Owner

Previously, we would only include the csrf double submit value in a
jwt if JWT_COOKIE_CSRF_PROTECT was true (the default) AND
JWT_TOKEN_LOCATION was configured to use cookies.

However, since we allow overwriting locations on a per-route basis
instead of only globally for he whole application, we could create a
situation where a single route was configured to use cookies when the
rest of the app was not, and csrf checks were not happening against
that endpoint.

This change makes it so that any jwts will be encoded with a csrf value
when JWT_COOKIE_CSRF_PROTECT is true, regardless of if the app is
globally configured to use cookies. It will also verify the csrf double
submit token on any route that uses cookies when JWT_COOKIE_CSRF_PROTECT
is true, regardless of if that is set globally in the application or on an
individual route.

As a result of this change, you might notice that using jwts without
cookies now include a csrf value. This will not change the behavior
of non-jwt based endpoints at all, your jwts will just be a little
bigger. You can remove that key from the jwt by explicitly setting
JWT_COOKIE_CSRF_PROTECT to False, if you are not using cookies.

Previously, we would only include the csrf double submit value in a
jwt if `JWT_COOKIE_CSRF_PROTECT` was true (the default) AND
`JWT_TOKEN_LOCATION` was configured to use cookies.

However, since we allow overwriting `locations` on a per-route basis
instead of only globally for he whole application, we could create a
situation where a single route was configured to use cookies when the
rest of the app was not, and csrf checks were not happening against
that endpoint.

This change makes it so that any jwts will be encoded with a csrf value
when `JWT_COOKIE_CSRF_PROTECT` is true, regardless of if the app is
globally configured to use cookies. It will also verify the csrf double
submit token on any route that uses cookies when `JWT_COOKIE_CSRF_PROTECT`
is true, regardless of if that is set globally in the application or on an
individual route.

As a result of this change, you might notice that using jwts without
cookies now include a csrf value. This will not change the behavior
of non-jwt based endpoints at all, your jwts will just be a little
bigger. You can remove that key from the jwt by explicitly setting
`JWT_COOKIE_CSRF_PROTECT` to False, if you are not using cookies.
@vimalloc vimalloc self-assigned this Dec 13, 2023
@vimalloc vimalloc marked this pull request as ready for review December 13, 2023 05:23
@vimalloc vimalloc merged commit 84c1946 into master Dec 13, 2023
28 checks passed
@vimalloc vimalloc deleted the fix_csrf_edge_case branch December 13, 2023 05:24
@vimalloc vimalloc changed the title Do not check JWT_TOKEN_LOCATION when setting csrf value in a jwt Do not check JWT_TOKEN_LOCATION when testing if cookie_csrf_protect is enabled Dec 13, 2023
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

Successfully merging this pull request may close these issues.

None yet

1 participant