diff --git a/CHANGES.rst b/CHANGES.rst index 16ed7a35..f124978e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -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 ++++++ @@ -18,13 +18,15 @@ 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 +++++++++++++++++++++++++++++++++ @@ -32,7 +34,10 @@ 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. diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 2f1405fc..427f3c84 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -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: > @@ -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' @@ -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 ] diff --git a/flask_security/tf_plugin.py b/flask_security/tf_plugin.py index e05c7c6a..65555665 100644 --- a/flask_security/tf_plugin.py +++ b/flask_security/tf_plugin.py @@ -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 @@ -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 diff --git a/flask_security/views.py b/flask_security/views.py index eef481d9..e1db74d7 100644 --- a/flask_security/views.py +++ b/flask_security/views.py @@ -972,10 +972,10 @@ 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"))) @@ -983,10 +983,7 @@ def two_factor_token_validation(): 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 diff --git a/flask_security/webauthn.py b/flask_security/webauthn.py index b9dd8b51..03b2ccdd 100644 --- a/flask_security/webauthn.py +++ b/flask_security/webauthn.py @@ -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) ) diff --git a/tests/test_confirmable.py b/tests/test_confirmable.py index 82dfb525..7d2f2a8f 100644 --- a/tests/test_confirmable.py +++ b/tests/test_confirmable.py @@ -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() diff --git a/tests/test_two_factor.py b/tests/test_two_factor.py index b43264b2..4dc89ef4 100644 --- a/tests/test_two_factor.py +++ b/tests/test_two_factor.py @@ -28,6 +28,7 @@ capture_send_code_requests, get_form_action, get_session, + is_authenticated, logout, ) @@ -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 @@ -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") @@ -104,21 +102,19 @@ 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" @@ -126,7 +122,6 @@ def test_always_validate(app, client): "/login", json=data, follow_redirects=True, - headers={"Content-Type": "application/json"}, ) assert response.status_code == 200 assert response.json["response"]["tf_required"] @@ -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, @@ -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, @@ -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, @@ -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, diff --git a/tests/test_unified_signin.py b/tests/test_unified_signin.py index f1cdfda9..183912b4 100644 --- a/tests/test_unified_signin.py +++ b/tests/test_unified_signin.py @@ -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 @@ -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, diff --git a/tests/test_webauthn.py b/tests/test_webauthn.py index 16110d8c..3d1a31fd 100644 --- a/tests/test_webauthn.py +++ b/tests/test_webauthn.py @@ -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) @@ -914,12 +915,8 @@ 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", @@ -927,7 +924,6 @@ def test_tf_validity_window_json(app, client, get_message): email="matt@lp.com", password="password", remember=True, - tf_validity_token=tf_token, ), ) assert response.status_code == 200