From 6319b87ea2227b46c20e9eac2b39947e9213c66d Mon Sep 17 00:00:00 2001 From: Chris Wagner Date: Mon, 3 Jun 2024 16:03:01 -0700 Subject: [PATCH] Update /login and /us-signin to have the same behavior when called by an already authenticated user. They were similar - now identical. GET w/ JSON now returns the same attributes (possible identity attributes etc) as a GET when not authenticated. close #973 --- CHANGES.rst | 10 +++++-- docs/openapi.yaml | 47 +++++++++++++++++++------------- flask_security/unified_signin.py | 46 ++++++++----------------------- flask_security/utils.py | 29 ++++++++++++++++++++ flask_security/views.py | 31 ++++----------------- tests/test_basic.py | 10 +++++++ tests/test_unified_signin.py | 31 ++++++++++----------- 7 files changed, 106 insertions(+), 98 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index a0a5d5d6..0e515d9c 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -11,12 +11,18 @@ Released TBD Features & Improvements +++++++++++++++++++++++ - (:issue:`956`) Add support for changing registered user's email (:py:data:`SECURITY_CHANGE_EMAIL`). -- (:pr:`xxx`) Change default password hash to argon2 (was bcrypt). See below for details. +- (:issue:`944`) Change default password hash to argon2 (was bcrypt). See below for details. Fixes +++++ - (:pr:`972`) Set :py:data:`SECURITY_CSRF_COOKIE` at beginning (GET /login) of authentication ritual - just as we return the CSRF token. (thanks @e-goto) +- (:issue:`973`) login and unified sign in should handle GET for authenticated user consistently + +Docs and Chores ++++++++++++++++ +- (:pr:`979`) Update Russian translations (ademaro) +- (:pr:`981` and :pr:`956`) Improve docs Backwards Compatibility Concerns +++++++++++++++++++++++++++++++++ @@ -151,7 +157,7 @@ Backwards Compatibility Concerns Flask-Security now changes any `None` key to `""`. - The default unauthorized handler behavior has changed slightly and is now documented. The default (:data:`SECURITY_UNAUTHORIZED_VIEW` == ``None``) has not changed (a default HTTP 403 response). - The precise behavior when :data:`SECURITY_UNAUTHORIZED_VIEW` was set was never documented. + The precise behavior when :data:`SECURITY_UNAUTHORIZED_VIEW` is set was never documented. The important change is that Flask-Security no longer ever looks at the request.referrer header and will never redirect to it. If an application needs that, it can provide a callable that can return that or any other header. diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 7b524496..49b2bca6 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -31,8 +31,9 @@ paths: Login form or user information. The JSON response will always carry the csrf_token information. If SECURITY_CSRF_COOKIE_NAME is set then a cookie with the csrf token will be set. - If the caller is logged in, then - additional information is returned. This can be very useful for single-page applications where during a force refresh, all state is lost. + If the caller is already authenticated, then + additional information is returned for JSON requests. + This can be very useful for single-page applications where during a force refresh, all state is lost. By performing this GET, the session cookie will authenticate the user and the response will contain user information. content: text/html: @@ -43,7 +44,7 @@ paths: application/json: schema: allOf: - - $ref: '#/components/schemas/BaseJsonResponse' + - $ref: '#/components/schemas/DefaultJsonResponse' - type: object properties: response: @@ -102,8 +103,8 @@ paths: example: render_template(SECURITY_LOGIN_USER_TEMPLATE) with error values 302: description: > - If the caller already authenticated, the form contents is ignored and a - redirect is done: redirect(next) or redirect(SECURITY_POST_LOGIN_VIEW). + If the caller is already authenticated, the form contents is ignored and a + redirect is done: redirect(SECURITY_POST_LOGIN_VIEW) (note that 'next' is ignored). If the caller is NOT already authenticated, and the form contents are validated the caller will be redirected to: @@ -734,7 +735,13 @@ paths: summary: GET Unified Sign In form responses: 200: - description: Sign in form + description: > + Sign in form + If SECURITY_CSRF_COOKIE_NAME is set then a cookie with the csrf token will be set. + If the caller is already authenticated, then + additional information is returned for JSON requests. + This can be very useful for single-page applications where during a force refresh, all state is lost. + By performing this GET, the session cookie will authenticate the user and the response will contain user information. content: text/html: schema: @@ -743,17 +750,19 @@ paths: example: render_template(SECURITY_US_SIGNIN_TEMPLATE) application/json: schema: - type: object - properties: - available_methods: - type: string - description: Config setting SECURITY_US_ENABLED_METHODS - code_methods: - type: string - description: All SECURITY_US_ENABLED_METHODS that require a code to be generated and sent. - identity_attributes: - type: string - description: Configuration setting SECURITY_USER_IDENTITY_ATTRIBUTES + allOf: + - $ref: '#/components/schemas/DefaultJsonResponse' + - type: object + properties: + available_methods: + type: string + description: Config setting SECURITY_US_ENABLED_METHODS + code_methods: + type: string + description: All SECURITY_US_ENABLED_METHODS that require a code to be generated and sent. + identity_attributes: + type: string + description: Configuration setting SECURITY_USER_IDENTITY_ATTRIBUTES post: summary: Unified Sign In parameters: @@ -786,8 +795,8 @@ paths: example: render_template(SECURITY_US_SIGNIN_TEMPLATE) with error values 302: description: > - If the caller already authenticated, the form contents is ignored and a - redirect is done: redirect(next) or redirect(SECURITY_POST_LOGIN_VIEW). + If the caller already authenticated, the form contents are ignored and a + redirect is done: redirect(SECURITY_POST_LOGIN_VIEW) (note that 'next' is ignored). If the caller is NOT already authenticated, and the form contents are validated the caller will be redirected to: diff --git a/flask_security/unified_signin.py b/flask_security/unified_signin.py index abe953e4..2951728b 100644 --- a/flask_security/unified_signin.py +++ b/flask_security/unified_signin.py @@ -79,9 +79,9 @@ get_message, get_url, get_within_delta, + handle_already_auth, is_user_authenticated, localize_callback, - json_error_response, login_user, lookup_identity, propagate_next, @@ -539,33 +539,20 @@ def us_signin() -> ResponseValue: Unified sign in view. This takes an identity (as configured in USER_IDENTITY_ATTRIBUTES) and a passcode (password or OTP). - - Allow already authenticated users. For GET this is useful for - single-page-applications on refresh - session still active but need to - access user info and csrf-token. - For POST - redirects to POST_LOGIN_VIEW (forms) or returns 400 (json). """ - - if is_user_authenticated(current_user) and request.method == "POST": - # Just redirect current_user to POST_LOGIN_VIEW (or next). - # While its tempting to try to logout the current user and login the - # new requested user - that simply doesn't work with CSRF. - - # While this is close to anonymous_user_required - it differs in that - # it uses get_post_login_redirect which correctly handles 'next'. - # TODO: consider changing anonymous_user_required to also call - # get_post_login_redirect - not sure why it never has? - if _security._want_json(request): - payload = json_error_response( - errors=get_message("ANONYMOUS_USER_REQUIRED")[0] - ) - return _security._render_json(payload, 400, None, None) - else: - return redirect(get_post_login_redirect()) - form = t.cast(UnifiedSigninForm, build_form_from_request("us_signin_form")) form.submit.data = True form.submit_send_code.data = False + code_methods = _compute_code_methods() + payload = { + "available_methods": cv("US_ENABLED_METHODS"), + "code_methods": code_methods, + "identity_attributes": get_identity_attributes(), + } + + if is_user_authenticated(current_user): + return handle_already_auth(form, payload=payload) + # Clean out any potential old session info - in case of previous # aborted 2FA attempt. tf_clean_session() @@ -604,20 +591,9 @@ def us_signin() -> ResponseValue: # base_render_json always sending the csrf_token session["fs_cc"] = "set" - code_methods = _compute_code_methods() if _security._want_json(request): - payload = { - "available_methods": cv("US_ENABLED_METHODS"), - "code_methods": code_methods, - "identity_attributes": get_identity_attributes(), - } return base_render_json(form, include_user=False, additional=payload) - if is_user_authenticated(current_user): - # Basically a no-op if authenticated - just perform the same - # post-login redirect as if user just logged in. - return redirect(get_post_login_redirect()) - # On error - wipe code form.passcode.data = None diff --git a/flask_security/utils.py b/flask_security/utils.py index 2082ad58..9d466425 100644 --- a/flask_security/utils.py +++ b/flask_security/utils.py @@ -30,6 +30,7 @@ current_app, flash, g, + redirect, request, render_template, session, @@ -1345,3 +1346,31 @@ def convert_password_tuple(value): entries = dict(map(convert_password_tuple, raw.upper().split("\r\n"))) return entries.get(sha1[5:].upper(), 0) + + +def handle_already_auth(form, payload=None): + """ + Allow already authenticated users. For GET this is useful for + single-page-applications on refresh - session still active but need to + access user info and csrf-token. + For GET with forms - redirect to POST_LOGIN_VIEW. + For POST - redirects to POST_LOGIN_VIEW (forms) or returns 400 (json). + + This does NOT use get_post_login_redirect() so that it doesn't look at + 'next' - which can cause infinite redirect loops + (see test_basic::test_authenticated_loop) + + While it's tempting to try to logout the current user and login the + new requested user - that simply doesn't work with CSRF. + """ + if _security._want_json(request): + if request.method == "POST": + payload = json_error_response( + errors=get_message("ANONYMOUS_USER_REQUIRED")[0] + ) + return _security._render_json(payload, 400, None, None) + else: + form.user = current_user + return base_render_json(form, additional=payload) + else: + return redirect(get_url(config_value("POST_LOGIN_VIEW"))) diff --git a/flask_security/views.py b/flask_security/views.py index b2873579..2e0516c7 100644 --- a/flask_security/views.py +++ b/flask_security/views.py @@ -107,9 +107,9 @@ get_post_verify_redirect, get_request_attr, get_url, + handle_already_auth, hash_password, is_user_authenticated, - json_error_response, localize_callback, login_user, logout_user, @@ -155,34 +155,13 @@ def _ctx(endpoint): @unauth_csrf() def login() -> "ResponseValue": - """View function for login view - - Allow already authenticated users. For GET this is useful for - single-page-applications on refresh - session still active but need to - access user info and csrf-token. - For POST - redirects to POST_LOGIN_VIEW (forms) or returns 400 (json). - """ + """View function for login view""" form = t.cast(LoginForm, build_form_from_request("login_form")) if is_user_authenticated(current_user): - # Just redirect current_user to POST_LOGIN_VIEW. - # While its tempting to try to logout the current user and login the - # new requested user - that simply doesn't work with CSRF. - - # This does NOT use get_post_login_redirect() so that it doesn't look at - # 'next' - which can cause infinite redirect loops - # (see test_common::test_authenticated_loop) - if _security._want_json(request): - if request.method == "POST": - payload = json_error_response( - errors=get_message("ANONYMOUS_USER_REQUIRED")[0] - ) - return _security._render_json(payload, 400, None, None) - else: - form.user = current_user - return base_render_json(form) - else: - return redirect(get_url(cv("POST_LOGIN_VIEW"))) + return handle_already_auth( + form, payload={"identity_attributes": get_identity_attributes()} + ) # Clean out any potential old session info - in case of previous # aborted 2FA attempt. diff --git a/tests/test_basic.py b/tests/test_basic.py index a6d6a8c0..b587a6b3 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -199,6 +199,16 @@ def test_get_already_authenticated(client): response = client.get("/login", follow_redirects=True) assert b"Post Login" in response.data + # should still get extra goodies + headers = {"Accept": "application/json", "Content-Type": "application/json"} + response = client.get("/login", headers=headers) + assert response.status_code == 200 + + jresponse = response.json["response"] + assert all(a in jresponse for a in ["identity_attributes"]) + assert "authentication_token" not in jresponse["user"] + assert all(a in jresponse["user"] for a in ["email", "last_update"]) + @pytest.mark.settings(post_login_view="/post_login") def test_get_already_authenticated_next(client): diff --git a/tests/test_unified_signin.py b/tests/test_unified_signin.py index ce283876..9bc40829 100644 --- a/tests/test_unified_signin.py +++ b/tests/test_unified_signin.py @@ -4,7 +4,7 @@ Unified signin tests - :copyright: (c) 2019-2023 by J. Christopher Wagner (jwag). + :copyright: (c) 2019-2024 by J. Christopher Wagner (jwag). :license: MIT, see LICENSE for more details. """ @@ -558,20 +558,25 @@ def test_admin_setup_reset(app, client_nc, get_message): @pytest.mark.settings(post_login_view="/post_login") -def test_get_already_authenticated(client): +def test_get_already_authenticated(app, client): response = authenticate(client, follow_redirects=True) assert b"Welcome matt@lp.com" in response.data - response = client.get("/us-signin", follow_redirects=True) + # This should ignore next + response = client.get("/us-signin?next=/page1", follow_redirects=True) assert b"Post Login" in response.data + # should still get extra goodies + headers = {"Accept": "application/json", "Content-Type": "application/json"} + response = client.get("/us-signin", headers=headers) + assert response.status_code == 200 -@pytest.mark.settings(post_login_view="/post_login") -def test_get_already_authenticated_next(client): - response = authenticate(client, follow_redirects=True) - assert b"Welcome matt@lp.com" in response.data - # This should override post_login_view - response = client.get("/us-signin?next=/page1", follow_redirects=True) - assert b"Page 1" in response.data + jresponse = response.json["response"] + assert all( + a in jresponse + for a in ["code_methods", "identity_attributes", "available_methods"] + ) + assert "authentication_token" not in jresponse["user"] + assert all(a in jresponse["user"] for a in ["email", "last_update"]) @pytest.mark.settings(post_login_view="/post_login") @@ -581,12 +586,6 @@ def test_post_already_authenticated(client, get_message): data = dict(email="matt@lp.com", password="password") response = client.post("/us-signin", data=data, follow_redirects=True) assert b"Post Login" in response.data - response = client.post("/us-signin?next=/page1", data=data, follow_redirects=True) - assert b"Page 1" in response.data - # should work in form as well - ndata = dict(email="matt@lp.com", password="password", next="/page1") - response = client.post("/us-signin", data=ndata, follow_redirects=True) - assert b"Page 1" in response.data headers = {"Accept": "application/json", "Content-Type": "application/json"} response = client.post("/us-signin", json=data, headers=headers)