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

Update /login and /us-signin to have the same behavior when called by… #986

Merged
merged 1 commit into from
Jun 13, 2024
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
10 changes: 8 additions & 2 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
+++++++++++++++++++++++++++++++++
Expand Down Expand Up @@ -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.
Expand Down
47 changes: 28 additions & 19 deletions docs/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -43,7 +44,7 @@ paths:
application/json:
schema:
allOf:
- $ref: '#/components/schemas/BaseJsonResponse'
- $ref: '#/components/schemas/DefaultJsonResponse'
- type: object
properties:
response:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
46 changes: 11 additions & 35 deletions flask_security/unified_signin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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

Expand Down
29 changes: 29 additions & 0 deletions flask_security/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
current_app,
flash,
g,
redirect,
request,
render_template,
session,
Expand Down Expand Up @@ -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")))
31 changes: 5 additions & 26 deletions flask_security/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 10 additions & 0 deletions tests/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
31 changes: 15 additions & 16 deletions tests/test_unified_signin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

"""
Expand Down Expand Up @@ -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")
Expand All @@ -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)
Expand Down
Loading