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

TWO_FACTOR_VALIDITY changes. #823

Merged
merged 1 commit into from
Jul 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
16 changes: 16 additions & 0 deletions tests/test_confirmable.py
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,22 @@ def test_spa_get_bad_token(app, client, get_message):
assert len(flashes) == 1


@pytest.mark.filterwarnings("ignore")
@pytest.mark.registerable()
@pytest.mark.settings(auto_login_after_confirm=True, post_login_view="/postlogin")
def test_auto_login(app, client, get_message):
with capture_registrations() as registrations:
data = dict(email="mary@lp.com", password="password", next="")
client.post("/register", data=data, follow_redirects=True)

assert not is_authenticated(client, get_message)

token = registrations[0]["confirm_token"]
response = client.get("/confirm/" + token, follow_redirects=False)
assert "/postlogin" == response.location
assert is_authenticated(client, get_message)


@pytest.mark.filterwarnings("ignore")
@pytest.mark.two_factor()
@pytest.mark.registerable()
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