Skip to content

Commit

Permalink
Fix 2 CSRF issues. (#928)
Browse files Browse the repository at this point in the history
1) If CSRF_COOKIE_NAME wasn't set, then our CSRF_HEADER wasn't added to the list in WTforms and therefore didn't work.
2) If you set CSRF_PROTECT_MECHANISMS to an empty list (to hopefully disable CSRF) - it in fact didn't stop CSRF...

closes #870
  • Loading branch information
jwag956 authored Feb 20, 2024
1 parent 263b2c2 commit cfc66b4
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 7 deletions.
8 changes: 4 additions & 4 deletions flask_security/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1667,18 +1667,18 @@ def _csrf_init(app):
if cv("CSRF_COOKIE_NAME", app=app) and not csrf:
# Common use case is for cookie value to be used as contents for header
# which is only looked at when CsrfProtect is initialized.
# Yes, this is opinionated - they can always get CSRF token via:
# 'get /login'
raise ValueError(
"CSRF_COOKIE defined however CsrfProtect not part of application"
)

if csrf:
csrf.exempt("flask_security.views.logout")
# Add configured header to WTF_CSRF_HEADERS
if ch := cv("CSRF_HEADER", app=app):
if ch not in app.config["WTF_CSRF_HEADERS"]:
app.config["WTF_CSRF_HEADERS"].append(ch)
if cv("CSRF_COOKIE_NAME", app=app):
app.after_request(csrf_cookie_handler)
# Add configured header to WTF_CSRF_HEADERS
app.config["WTF_CSRF_HEADERS"].append(cv("CSRF_HEADER", app=app))

def set_form_info(self, name: str, form_info: FormInfo) -> None:
"""Set form instantiation info.
Expand Down
4 changes: 2 additions & 2 deletions flask_security/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,8 @@ def handle_csrf(method: str, json_response: bool = False) -> ResponseValue | Non
payload = json_error_response(errors=e.description)
return _security._render_json(payload, 400, None, None)
raise
else:
set_request_attr("fs_ignore_csrf", True)
return None
set_request_attr("fs_ignore_csrf", True)
return None


Expand Down
31 changes: 30 additions & 1 deletion tests/test_csrf.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,10 +259,13 @@ def test_cp_reset(app, client):

@pytest.mark.changeable()
@pytest.mark.csrf(csrfprotect=True)
@pytest.mark.settings(csrf_header="X-XSRF-Token")
def test_cp_with_token(app, client):
# Make sure can use returned CSRF-Token in Header.
# Since the csrf token isn't in the form - must enable app-wide CSRF
# using CSRFProtect() - as the above mark does.
# Using X-XSRF-Token as header tests that we properly
# add that as a known header to WTFforms.
auth_token, csrf_token = json_login(client, use_header=True)

# make sure returned csrf_token works in header.
Expand All @@ -277,7 +280,7 @@ def test_cp_with_token(app, client):
"/change",
content_type="application/json",
json=data,
headers={"X-CSRF-Token": csrf_token},
headers={"X-XSRF-Token": csrf_token},
)
assert response.status_code == 200
assert mp.success == 1 and mp.failure == 0
Expand Down Expand Up @@ -417,6 +420,32 @@ def test_different_mechanisms_nc(app, client_nc):
assert mp.success == 0 and mp.failure == 0


@pytest.mark.changeable()
@pytest.mark.csrf(csrfprotect=True)
@pytest.mark.settings(csrf_protect_mechanisms=[])
def test_cp_with_token_empty_mechanisms(app, client):
# If no mechanisms - shouldn't do any CSRF
auth_token, csrf_token = json_login(client, use_header=True)

# make sure returned csrf_token works in header.
data = dict(
password="password",
new_password="battery staple",
new_password_confirm="battery staple",
)

response = client.post(
"/change",
content_type="application/json",
json=data,
headers={
"Content-Type": "application/json",
"Authentication-Token": auth_token,
},
)
assert response.status_code == 200


@pytest.mark.settings(csrf_ignore_unauth_endpoints=True, CSRF_COOKIE_NAME="XSRF-Token")
def test_csrf_cookie(app, sqlalchemy_datastore):
app.config["WTF_CSRF_ENABLED"] = True
Expand Down

0 comments on commit cfc66b4

Please sign in to comment.