Skip to content

Commit

Permalink
Do not check JWT_TOKEN_LOCATION when setting csrf value in a jwt (#538)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
vimalloc authored Dec 13, 2023
1 parent f65202f commit 84c1946
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 18 deletions.
4 changes: 2 additions & 2 deletions flask_jwt_extended/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ def refresh_json_key(self) -> str:
return current_app.config["JWT_REFRESH_JSON_KEY"]

@property
def csrf_protect(self) -> bool:
return self.jwt_in_cookies and current_app.config["JWT_COOKIE_CSRF_PROTECT"]
def cookie_csrf_protect(self) -> bool:
return current_app.config["JWT_COOKIE_CSRF_PROTECT"]

@property
def csrf_request_methods(self) -> Iterable[str]:
Expand Down
2 changes: 1 addition & 1 deletion flask_jwt_extended/jwt_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ def _encode_jwt_from_config(
algorithm=config.algorithm,
audience=config.encode_audience,
claim_overrides=claim_overrides,
csrf=config.csrf_protect,
csrf=config.cookie_csrf_protect,
expires_delta=expires_delta,
fresh=fresh,
header_overrides=header_overrides,
Expand Down
8 changes: 4 additions & 4 deletions flask_jwt_extended/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ def set_access_cookies(
samesite=config.cookie_samesite,
)

if config.csrf_protect and config.csrf_in_cookies:
if config.cookie_csrf_protect and config.csrf_in_cookies:
response.set_cookie(
config.access_csrf_cookie_name,
value=get_csrf_token(encoded_access_token),
Expand Down Expand Up @@ -358,7 +358,7 @@ def set_refresh_cookies(
samesite=config.cookie_samesite,
)

if config.csrf_protect and config.csrf_in_cookies:
if config.cookie_csrf_protect and config.csrf_in_cookies:
response.set_cookie(
config.refresh_csrf_cookie_name,
value=get_csrf_token(encoded_refresh_token),
Expand Down Expand Up @@ -408,7 +408,7 @@ def unset_access_cookies(response: Response, domain: Optional[str] = None) -> No
samesite=config.cookie_samesite,
)

if config.csrf_protect and config.csrf_in_cookies:
if config.cookie_csrf_protect and config.csrf_in_cookies:
response.set_cookie(
config.access_csrf_cookie_name,
value="",
Expand Down Expand Up @@ -446,7 +446,7 @@ def unset_refresh_cookies(response: Response, domain: Optional[str] = None) -> N
samesite=config.cookie_samesite,
)

if config.csrf_protect and config.csrf_in_cookies:
if config.cookie_csrf_protect and config.csrf_in_cookies:
response.set_cookie(
config.refresh_csrf_cookie_name,
value="",
Expand Down
2 changes: 1 addition & 1 deletion flask_jwt_extended/view_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ def _decode_jwt_from_cookies(refresh: bool) -> Tuple[str, Optional[str]]:
if not encoded_token:
raise NoAuthorizationError('Missing cookie "{}"'.format(cookie_key))

if config.csrf_protect and request.method in config.csrf_request_methods:
if config.cookie_csrf_protect and request.method in config.csrf_request_methods:
csrf_value = request.headers.get(csrf_header_key, None)
if not csrf_value and config.csrf_check_form:
csrf_value = request.form.get(csrf_field_key, None)
Expand Down
14 changes: 4 additions & 10 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def test_default_configs(app):
assert config.json_key == "access_token"
assert config.refresh_json_key == "refresh_token"

assert config.csrf_protect is False
assert config.cookie_csrf_protect is True
assert config.csrf_request_methods == ["POST", "PUT", "PATCH", "DELETE"]
assert config.csrf_in_cookies is True
assert config.access_csrf_cookie_name == "csrf_access_token"
Expand Down Expand Up @@ -142,7 +142,7 @@ def test_override_configs(app, delta_func):
assert config.session_cookie is False
assert config.cookie_samesite == "Strict"

assert config.csrf_protect is True
assert config.cookie_csrf_protect is True
assert config.csrf_request_methods == ["GET"]
assert config.csrf_in_cookies is False
assert config.access_csrf_cookie_name == "access_csrf_cookie"
Expand Down Expand Up @@ -333,17 +333,11 @@ def test_jwt_token_locations_config(app):

def test_csrf_protect_config(app):
with app.test_request_context():
app.config["JWT_TOKEN_LOCATION"] = ["headers"]
app.config["JWT_COOKIE_CSRF_PROTECT"] = True
assert config.csrf_protect is False
assert config.cookie_csrf_protect is True

app.config["JWT_TOKEN_LOCATION"] = ["cookies"]
app.config["JWT_COOKIE_CSRF_PROTECT"] = True
assert config.csrf_protect is True

app.config["JWT_TOKEN_LOCATION"] = ["cookies"]
app.config["JWT_COOKIE_CSRF_PROTECT"] = False
assert config.csrf_protect is False
assert config.cookie_csrf_protect is False


def test_missing_algorithm_in_decode_algorithms(app):
Expand Down

0 comments on commit 84c1946

Please sign in to comment.