Skip to content

Commit

Permalink
TWO_FACTOR_VALIDITY changes.
Browse files Browse the repository at this point in the history
Now only a cookie is set - for both forms and JSON, and the tf_validity token is never returned as part of a JSON response.
Probably overkill, but in the response containing the tf_validity cookie - we add the Vary: Cookie header.
  • Loading branch information
jwag956 committed Jul 25, 2023
1 parent 58f93ef commit 15e3ed8
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 81 deletions.
15 changes: 10 additions & 5 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ Version 5.3.0

Released TBD

This is a minor version bump due to the (possible) incompatibility w.r.t.
WebAuthn, and changes to /reset.
This is a minor version bump due to some small backwards incompatible changes to
WebAuthn, recoverability (/reset), confirmation (/confirm) and the two factor validity feature.

Fixes
++++++
Expand All @@ -18,21 +18,26 @@ Fixes
- (:pr:`809`) Fix MongoDB support by eliminating dependency on flask-mongoengine.
Improve MongoDB quickstart.
- (:issue:`801`) Fix Quickstart for SQLAlchemy with scoped session.
- (:issue:`806`) Login no longer, by default, check for email deliverability.
- (:issue:`791`) Token authentication is accepted on endpoints which only allow
- (:issue:`806`) Login no longer, by default, checks for email deliverability.
- (:issue:`791`) Token authentication is no longer accepted on endpoints which only allow
'session' as authentication-method. (N247S)
- (:issue:`814`) /reset and /confirm and GENERIC_RESPONSES and additional form args don't mix.
- (:issue:`281`) Reset password can be exploited and other OWASP improvements.
- (:pr:`817`) Confirmation can be exploited and other OWASP improvements.
- (:pr:`819`) Convert to pyproject.toml, build, remove setup.
- (:pr:`xxx`) the tf_validity feature now ONLY sets a cookie - and the token is no longer
returned as part of a JSON response.

Backwards Compatibility Concerns
+++++++++++++++++++++++++++++++++

- To align with the W3C WebAuthn Level2 and 3 spec - transports are now part of the registration response.
This has been changed BOTH in the server code (using py_webauth data structures) as well as the sample
javascript code. If an application has their own javascript front end code - it might need to be changed.
- Reset password was changed to improve adhere to OWASP recommendations and reduce possible exploitation:
- The tf_validity feature :py:data`SECURITY_TWO_FACTOR_ALWAYS_VALIDATE` used to set a cookie if the request was
form based, and return the token as part of a JSON response. Now, this feature is ONLY cookie based and the token
is no longer returned as part of any response.
- Reset password was changed to adhere to OWASP recommendations and reduce possible exploitation:

- A new email (with new token) is no longer sent upon expired token. Users must restart
the reset password process.
Expand Down
10 changes: 0 additions & 10 deletions docs/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1818,9 +1818,6 @@ components:
type: boolean
description: >
If true, will remember userid as part of cookie. There is a configuration variable DEFAULT_REMEMBER_ME that can be set. This field will override that.
tf_validity_token:
type: string
description: Code verifying the user has successfully verified 2FA in the past. If verified, the user is able to skip validation of the second factor. Only used when SECURITY_TWO_FACTOR_ALWAYS_VALIDATE is False.
LoginJsonResponse:
type: object
description: >
Expand Down Expand Up @@ -2037,9 +2034,6 @@ components:
description: password or code
remember:
type: boolean
tf_validity_token:
type: string
description: Code verifying the user has successfully verified 2FA in the past. If verified, the user is able to skip validation of the second factor. Only used when SECURITY_TWO_FACTOR_ALWAYS_VALIDATE is False.
UsSigninJsonResponse:
allOf:
- $ref: '#/components/schemas/BaseJsonResponse'
Expand Down Expand Up @@ -2199,10 +2193,6 @@ components:
csrf_token:
type: string
description: Session CSRF token
tf_validity_token:
type: string
description: A timed token that verifies that the user has successfully completed 2FA. Only sent if SECURITY_TWO_FACTOR_ALWAYS_VALIDATE is False and remember_me (from /login POST) is True

WanRegister:
type: object
required: [ name, usage ]
Expand Down
10 changes: 4 additions & 6 deletions flask_security/tf_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,11 +282,7 @@ def tf_verify_validity_token(fs_uniquifier: str) -> bool:
:param fs_uniquifier: The ``fs_uniquifier`` of the submitting user.
"""
if request.is_json and request.content_length:
token = request.get_json().get("tf_validity_token", None) # type: ignore
else:
token = request.cookies.get("tf_validity", default=None)

token = request.cookies.get("tf_validity", default=None)
if token is None:
return False

Expand All @@ -307,7 +303,9 @@ def tf_set_validity_token_cookie(response: "Response", token: str) -> "Response"
cookie_kwargs = cv("TWO_FACTOR_VALIDITY_COOKIE")
max_age = int(get_within_delta("TWO_FACTOR_LOGIN_VALIDITY").total_seconds())
response.set_cookie("tf_validity", value=token, max_age=max_age, **cookie_kwargs)

# This is likely overkill since so far we only return this on a POST which is
# unlikely to be cached.
response.vary.add("Cookie")
return response


Expand Down
9 changes: 3 additions & 6 deletions flask_security/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -972,21 +972,18 @@ def two_factor_token_validation():
)

after_this_request(view_commit)
if token:
after_this_request(partial(tf_set_validity_token_cookie, token=token))

if not _security._want_json(request):
if token:
after_this_request(partial(tf_set_validity_token_cookie, token=token))
do_flash(*get_message(completion_message))
if changing:
return redirect(get_url(cv("TWO_FACTOR_POST_SETUP_VIEW")))
else:
return redirect(get_post_login_redirect())

else:
json_payload = {}
if token:
json_payload["tf_validity_token"] = token
return base_render_json(form, additional=json_payload)
return base_render_json(form)

# GET or not successful POST

Expand Down
1 change: 0 additions & 1 deletion flask_security/webauthn.py
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,6 @@ def webauthn_signin_response(token: str) -> "ResponseValue":
if is_secondary:
tf_token = _security.two_factor_plugins.tf_complete(form.user, True)
if tf_token:
json_payload["tf_validity_token"] = tf_token
after_this_request(
partial(tf_set_validity_token_cookie, token=tf_token)
)
Expand Down
39 changes: 17 additions & 22 deletions tests/test_two_factor.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
capture_send_code_requests,
get_form_action,
get_session,
is_authenticated,
logout,
)

Expand Down Expand Up @@ -59,15 +60,12 @@ def tf_authenticate(app, client, json=False, validate=True, remember=False):
response = client.post(
"/tf-validate",
json=dict(code=code),
headers={"Content-Type": "application/json"},
)
assert b'"code": 200' in response.data
return response.json["response"].get("tf_validity_token", None)
assert response.status_code == 200
else:
response = client.post(
"/tf-validate", data=dict(code=code), follow_redirects=True
)

assert response.status_code == 200


Expand All @@ -86,7 +84,7 @@ def tf_in_session(session):


@pytest.mark.settings(two_factor_always_validate=False)
def test_always_validate(app, client):
def test_always_validate(app, client, get_message):
tf_authenticate(app, client, remember=True)
assert client.get_cookie("tf_validity")

Expand All @@ -104,29 +102,26 @@ def test_always_validate(app, client):

# make sure the cookie doesn't affect the JSON request
client.delete_cookie("tf_validity")
# Test JSON
token = tf_authenticate(app, client, json=True, remember=True)
# Test JSON (this authenticates gal@lp.com)
tf_authenticate(app, client, json=True, remember=True)
logout(client)
data = dict(email="gal@lp.com", password="password", tf_validity_token=token)

data = dict(email="gal@lp.com", password="password")
response = client.post(
"/login",
json=data,
follow_redirects=True,
headers={"Content-Type": "application/json"},
)
assert response.status_code == 200
# verify logged in
response = client.get("/profile", follow_redirects=False)
assert response.status_code == 200

is_authenticated(client, get_message)
logout(client)

data["email"] = "gal2@lp.com"
response = client.post(
"/login",
json=data,
follow_redirects=True,
headers={"Content-Type": "application/json"},
)
assert response.status_code == 200
assert response.json["response"]["tf_required"]
Expand All @@ -144,11 +139,11 @@ def test_do_not_remember_tf_validity(app, client):
assert b"Please enter your authentication code" in response.data

# Test JSON
token = tf_authenticate(app, client, json=True)
tf_authenticate(app, client, json=True)
logout(client)
assert token is None
assert not client.get_cookie("tf_validity")

data = dict(email="gal@lp.com", password="password", tf_validity_token=token)
data = dict(email="gal@lp.com", password="password")
response = client.post(
"/login",
json=data,
Expand All @@ -175,9 +170,9 @@ def test_tf_expired_cookie(app, client):
assert b"Please enter your authentication code" in response.data

# Test JSON
token = tf_authenticate(app, client, json=True, remember=True)
tf_authenticate(app, client, json=True, remember=True)
logout(client)
data = dict(email="gal@lp.com", password="password", tf_validity_token=token)
data = dict(email="gal@lp.com", password="password")
response = client.post(
"/login",
json=data,
Expand Down Expand Up @@ -206,13 +201,13 @@ def test_change_uniquifier_invalidates_cookie(app, client):

client.delete_cookie("tf_validity")
# Test JSON
token = tf_authenticate(app, client, json=True, remember=True)
tf_authenticate(app, client, json=True, remember=True)
logout(client)
with app.app_context():
user = app.security.datastore.find_user(email="gal@lp.com")
app.security.datastore.set_uniquifier(user)
app.security.datastore.commit()
data = dict(email="gal@lp.com", password="password", tf_validity_token=token)
data = dict(email="gal@lp.com", password="password")
response = client.post(
"/login",
json=data,
Expand Down Expand Up @@ -241,13 +236,13 @@ def test_tf_reset_invalidates_cookie(app, client):

client.delete_cookie("tf_validity")
# Test JSON
token = tf_authenticate(app, client, json=True, remember=True, validate=False)
tf_authenticate(app, client, json=True, remember=True, validate=False)
logout(client)
with app.app_context():
user = app.security.datastore.find_user(email="gal@lp.com")
app.security.datastore.reset_user_access(user)
app.security.datastore.commit()
data = dict(email="gal@lp.com", password="password", tf_validity_token=token)
data = dict(email="gal@lp.com", password="password")
response = client.post(
"/login",
json=data,
Expand Down
33 changes: 8 additions & 25 deletions tests/test_unified_signin.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,10 @@ def us_tf_authenticate(app, client, json=False, validate=True, remember=False):
headers={"Content-Type": "application/json"},
)
assert b'"code": 200' in response.data
return response.json["response"].get("tf_validity_token", None)
else:
response = client.post(
"/tf-validate", data=dict(code=code), follow_redirects=True
)

assert response.status_code == 200


Expand Down Expand Up @@ -1803,37 +1801,22 @@ def test_totp_generation(app, client, get_message):
)
def test_us_tf_validity(app, client, get_message):
us_tf_authenticate(app, client, remember=True)
assert client.get_cookie("tf_validity")
logout(client)
data = dict(identity="gal@lp.com", passcode="password")
response = client.post(
"/us-signin", json=data, headers={"Content-Type": "application/json"}
)
assert b'"code": 200' in response.data
# logout does NOT remove this cookie
assert client.get_cookie("tf_validity")

# This time shouldn't require code
data = dict(identity="gal@lp.com", passcode="password")
response = client.post("/us-signin", json=data)
assert response.json["meta"]["code"] == 200
assert is_authenticated(client, get_message)
logout(client)

data = dict(identity="gal2@lp.com", passcode="password")
response = client.post("/us-signin", data=data, follow_redirects=True)
assert b"Please enter your authentication code" in response.data

# clear the cookie to make sure it's not picking it up with json.
client.delete("tf_validity")

token = us_tf_authenticate(app, client, remember=True, json=True)
logout(client)
data = dict(identity="gal@lp.com", passcode="password", tf_validity_token=token)
response = client.post(
"/us-signin",
json=data,
follow_redirects=True,
headers={"Content-Type": "application/json"},
)
assert response.status_code == 200
# verify logged in
assert is_authenticated(client, get_message)
logout(client)

data["identity"] = "gal2@lp.com"
response = client.post(
"/us-signin",
json=data,
Expand Down
8 changes: 2 additions & 6 deletions tests/test_webauthn.py
Original file line number Diff line number Diff line change
Expand Up @@ -900,7 +900,8 @@ def test_tf_validity_window(app, client, get_message):
)
def test_tf_validity_window_json(app, client, get_message):
# Test with a two-factor validity setting - we don't get re-prompted.
response = json_authenticate(client)
# This also relies on the tf_validity_cookie
json_authenticate(client)
register_options, response_url = _register_start_json(client)
client.post(response_url, json=dict(credential=json.dumps(REG_DATA1)))
logout(client)
Expand All @@ -914,20 +915,15 @@ def test_tf_validity_window_json(app, client, get_message):
signin_options, response_url = _signin_start(client, "matt@lp.com")
response = client.post(response_url, json=dict(credential=json.dumps(SIGNIN_DATA1)))
assert response.status_code == 200
tf_token = response.json["response"]["tf_validity_token"]
logout(client)

# make sure the cookie doesn't affect the JSON request
client.delete_cookie("tf_validity")

# Sign in again - shouldn't require 2FA
response = client.post(
"/login",
json=dict(
email="matt@lp.com",
password="password",
remember=True,
tf_validity_token=tf_token,
),
)
assert response.status_code == 200
Expand Down

0 comments on commit 15e3ed8

Please sign in to comment.