Change confirmation to default to NOT auto-login. (#820)

This makes it consistent with /reset and with OWASP best practices.

CHANGES.rst

@@ -24,7 +24,7 @@ Fixes
 - (: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:`xxx`) Convert to pyproject.toml, build, remove setup.
+- (:pr:`819`) Convert to pyproject.toml, build, remove setup.
 
 Backwards Compatibility Concerns
 +++++++++++++++++++++++++++++++++
@@ -51,6 +51,9 @@ Backwards Compatibility Concerns
       the confirmation process.
     - Identity information (identity, email) is no longer sent as part of the URL redirect
       query params.
+    - The :py:data:`SECURITY_AUTO_LOGIN_AFTER_CONFIRM` configuration variable now defaults to ``False`` - meaning
+      after a successful email confirmation, the user must still sign in using the usual mechanisms. This is to
+      align better with OWASP best practices. Setting it to ``True`` will restore prior behavior.
     - The SECURITY_MSG_CONFIRMATION_EXPIRED message no longer contains the user's identity/email.
     - The response to GET /reset/<token> sets the HTTP header `Referrer-Policy` to `no-referrer` as suggested
       by OWASP.

docs/configuration.rst

@@ -901,12 +901,13 @@ Confirmable
     Default: ``None``.
 .. py:data:: SECURITY_AUTO_LOGIN_AFTER_CONFIRM
 
-    If ``False`` then on confirmation  the user will be required to login again.
+    If ``True``, then the user corresponding to the confirmation token will be automatically signed in.
+    If ``False`` (the default) then the user will be requires to authenticate using the usual mechanism(s).
     Note that the confirmation token is not valid after being used once.
-    If ``True``, then the user corresponding to the
-    confirmation token will be automatically logged in.
 
-    Default: ``True``.
+    Default: ``False``.
+
+    .. deprecated:: 5.3.0
 .. py:data:: SECURITY_LOGIN_WITHOUT_CONFIRMATION
 
     Specifies if a user may login before confirming their email when
@@ -916,8 +917,7 @@ Confirmable
 .. py:data:: SECURITY_REQUIRES_CONFIRMATION_ERROR_VIEW
 
     Specifies a redirect page if the users tries to login, reset password or us-signin with an unconfirmed account.
-    If an URL endpoint is specified, flashes an error messages and passes user email as an argument.
-    For us-signin, no argument is specified: it simply flashes the error message and redirects.
+    If an URL endpoint is specified, flashes an error messages and redirects.
     Default behavior is to reload the form with an error message without redirecting to an other page.
 
     Default: ``None``.

docs/openapi.yaml

@@ -584,8 +584,8 @@ paths:
         This is the result of getting a confirmation token and is usually
         the result of clicking the link from a confirmation email.
         This ALWAYS results in a 302 redirect.
-        By default (unless __SECURITY_AUTO_LOGIN_AFTER_CONFIRM__ == False), the user
-        denoted by the token is logged in as a side-effect.
+        By default (unless __SECURITY_AUTO_LOGIN_AFTER_CONFIRM__ == True), the user
+        denoted by the token must authenticate using normal mechanisms.
       responses:
         302:
           description: >

docs/spa.rst

@@ -42,7 +42,6 @@ An example configuration::
     SECURITY_CONFIRMABLE = True
     SECURITY_REGISTERABLE = True
     SECURITY_UNIFIED_SIGNIN = True
-    SECURITY_WEBAUTHN = True
 
     # These need to be defined to handle redirects
     # As defined in the API documentation - they will receive the relevant context

flask_security/core.py

@@ -248,7 +248,7 @@
     "CONFIRM_EMAIL_WITHIN": "5 days",
     "RESET_PASSWORD_WITHIN": "1 days",
     "LOGIN_WITHOUT_CONFIRMATION": False,
-    "AUTO_LOGIN_AFTER_CONFIRM": True,
+    "AUTO_LOGIN_AFTER_CONFIRM": False,
     "AUTO_LOGIN_AFTER_RESET": False,
     "EMAIL_SENDER": LocalProxy(
         lambda: current_app.config.get("MAIL_DEFAULT_SENDER", "no-reply@localhost")
@@ -1457,6 +1457,13 @@ def init_app(
                 DeprecationWarning,
                 stacklevel=2,
             )
+        if cv("AUTO_LOGIN_AFTER_CONFIRM", app=app):
+            warnings.warn(
+                "The auto-login after successful confirmation functionality"
+                "has been deprecated and will be removed in a future release.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
         if cv("USERNAME_ENABLE", app):
             if hasattr(self.datastore, "user_model") and not hasattr(
                 self.datastore.user_model, "username"

flask_security/views.py

@@ -433,7 +433,10 @@ def send_confirmation():
 
 
 def confirm_email(token):
-    """View function which handles an email confirmation request."""
+    """
+    View function which handles an email confirmation request.
+    This is always a GET from an email - so for 'spa' must always redirect.
+    """
 
     expired, invalid, user = confirm_email_token_status(token)
 
@@ -454,7 +457,6 @@ def confirm_email(token):
         )
 
     already_confirmed = user.confirmed_at is not None
-
     if already_confirmed:
         m, c = get_message("ALREADY_CONFIRMED")
 
@@ -483,13 +485,16 @@ def confirm_email(token):
     after_this_request(view_commit)
     m, c = get_message("EMAIL_CONFIRMED")
 
+    # ? The only case where user is logged in already would be if
+    # LOGIN_WITHOUT_CONFIRMATION
     if user != current_user:
         logout_user()
         if cv("AUTO_LOGIN_AFTER_CONFIRM"):
             # N.B. this is a (small) security risk if email went to wrong place.
             # and you have the LOGIN_WITHOUT_CONFIRMATION flag since in that case
             # you can be logged in and doing stuff - but another person could
             # get the email.
+            # Note also this goes against OWASP recommendations.
             response = _security.two_factor_plugins.tf_enter(
                 user, False, "confirm", next_loc=propagate_next(request.url)
             )

tests/test_confirmable.py

@@ -22,6 +22,7 @@
     authenticate,
     capture_flashes,
     capture_registrations,
+    is_authenticated,
     logout,
     populate_data,
 )
@@ -91,15 +92,15 @@ def on_instructions_sent(app, **kwargs):
     assert response.headers.get("Referrer-Policy", None) == "no-referrer"
     response = clients.get(response.location)
     assert get_message("EMAIL_CONFIRMED") in response.data
+    # make sure not logged in
+    assert not is_authenticated(clients, get_message)
 
     # Test already confirmed
     response = clients.get("/confirm/" + token, follow_redirects=True)
     assert get_message("ALREADY_CONFIRMED") in response.data
     assert len(recorded_instructions_sent) == 2
 
     # Test already confirmed when asking for confirmation instructions
-    logout(clients)
-
     response = clients.get("/confirm")
     assert response.status_code == 200
 
@@ -290,25 +291,34 @@ def test_confirmation_different_user_when_logged_in_no_auto(client, get_message)
 @pytest.mark.registerable()
 @pytest.mark.settings(login_without_confirmation=True)
 def test_confirmation_different_user_when_logged_in(client, get_message):
+    # with default no-auto-login - first user should get logged out and second
+    # should be properly confirmed (but not logged in)
     e1 = "dude@lp.com"
     e2 = "lady@lp.com"
 
     with capture_registrations() as registrations:
         for e in e1, e2:
             data = dict(email=e, password="awesome sunset", next="")
-            client.post("/register", data=data)
+            response = client.post("/register", data=data)
+            assert is_authenticated(client, get_message)
             logout(client)
 
     token1 = registrations[0]["confirm_token"]
     token2 = registrations[1]["confirm_token"]
 
-    client.get("/confirm/" + token1, follow_redirects=True)
-    logout(client)
-    authenticate(client, email=e1)
+    response = client.get("/confirm/" + token1, follow_redirects=False)
+    assert "/login" in response.location
+    authenticate(client, email=e1, password="awesome sunset")
+    assert is_authenticated(client, get_message)
 
     response = client.get("/confirm/" + token2, follow_redirects=True)
     assert get_message("EMAIL_CONFIRMED") in response.data
-    assert b"Welcome lady@lp.com" in response.data
+
+    # first user should have been logged out
+    assert not is_authenticated(client, get_message)
+
+    authenticate(client, email=e2, password="awesome sunset")
+    assert is_authenticated(client, get_message)
 
 
 @pytest.mark.registerable()
@@ -451,30 +461,29 @@ def test_spa_get_bad_token(app, client, get_message):
     assert len(flashes) == 1
 
 
+@pytest.mark.filterwarnings("ignore")
 @pytest.mark.two_factor()
 @pytest.mark.registerable()
-@pytest.mark.settings(two_factor_required=True)
-def test_two_factor(app, client):
+@pytest.mark.settings(two_factor_required=True, auto_login_after_confirm=True)
+def test_two_factor(app, client, get_message):
     """If two-factor is enabled, the confirm shouldn't login, but start the
     2-factor setup.
     """
     with capture_registrations() as registrations:
         data = dict(email="mary@lp.com", password="password", next="")
         client.post("/register", data=data, follow_redirects=True)
 
-    # make sure not logged in
-    response = client.get("/profile")
-    assert response.status_code == 302
-    assert "/login?next=%2Fprofile" in response.location
+    assert not is_authenticated(client, get_message)
 
     token = registrations[0]["confirm_token"]
     response = client.get("/confirm/" + token, follow_redirects=False)
     assert "tf-setup" in response.location
 
 
+@pytest.mark.filterwarnings("ignore")
 @pytest.mark.two_factor()
 @pytest.mark.registerable()
-@pytest.mark.settings(two_factor_required=True)
+@pytest.mark.settings(two_factor_required=True, auto_login_after_confirm=True)
 def test_two_factor_json(app, client, get_message):
     with capture_registrations() as registrations:
         data = dict(email="dude@lp.com", password="password")
@@ -484,12 +493,7 @@ def test_two_factor_json(app, client, get_message):
         assert len(response.json["response"]) == 2
         assert all(k in response.json["response"] for k in ["csrf_token", "user"])
 
-    # make sure not logged in
-    response = client.get("/profile", headers={"accept": "application/json"})
-    assert response.status_code == 401
-    assert response.json["response"]["errors"][0].encode("utf-8") == get_message(
-        "UNAUTHENTICATED"
-    )
+    assert not is_authenticated(client, get_message)
 
     token = registrations[0]["confirm_token"]
     response = client.get("/confirm/" + token, headers={"Accept": "application/json"})
@@ -514,9 +518,7 @@ def test_email_not_identity(app, client, get_message):
     token = registrations[0]["confirm_token"]
     response = client.get("/confirm/" + token, headers={"Accept": "application/json"})
     assert response.status_code == 302
-    assert "/" in response.location
-
-    logout(client)
+    assert not is_authenticated(client, get_message)
 
     # check that username must be unique
     data = dict(email="mary4@lp.com", username="mary", password="awesome sunset")

tests/test_unified_signin.py

@@ -26,6 +26,7 @@
     capture_flashes,
     capture_reset_password_requests,
     get_form_action,
+    is_authenticated,
     logout,
     reset_fresh,
     setup_tf_sms,
@@ -203,12 +204,10 @@ def authned(myapp, user, **extra_args):
     assert not clients.get_cookie("remember_token")
     assert "email" in auths[0][1]
 
-    response = clients.get("/profile", follow_redirects=False)
-    assert response.status_code == 200
+    assert is_authenticated(clients, get_message)
 
     logout(clients)
-    response = clients.get("/profile", follow_redirects=False)
-    assert "/login?next=%2Fprofile" in response.location
+    assert not is_authenticated(clients, get_message)
 
     # login via SMS
     sms_sender = SmsSenderFactory.createSender("test")
@@ -231,8 +230,7 @@ def authned(myapp, user, **extra_args):
     assert clients.get_cookie("remember_token")
     assert "sms" in auths[1][1]
 
-    response = clients.get("/profile", follow_redirects=False)
-    assert response.status_code == 200
+    assert is_authenticated(clients, get_message)
 
     logout(clients)
     assert not clients.get_cookie("remember_token")
@@ -294,8 +292,7 @@ def authned(myapp, user, **extra_args):
         assert "email" in auths[0][1]
 
         logout(client_nc)
-        response = client_nc.get("/profile", headers=headers, follow_redirects=False)
-        assert response.status_code == 401
+        assert not is_authenticated(client_nc, get_message)
 
         # login via SMS
         sms_sender = SmsSenderFactory.createSender("test")
@@ -459,9 +456,8 @@ def test_us_passwordless_confirm_json(app, client, get_message):
     outbox = app.mail.outbox
     matcher = re.findall(r"\w+:.*", outbox[0].body, re.IGNORECASE)
     link = matcher[0].split(":", 1)[1]
-    response = client.get(link, headers=headers, follow_redirects=True)
-    assert get_message("EMAIL_CONFIRMED") in response.data
-    logout(client)
+    response = client.get(link, headers=headers, follow_redirects=False)
+    assert response.location == "/login"
 
     # should be able to authenticate now.
     response = client.post(
@@ -644,8 +640,7 @@ def authned(myapp, user, **extra_args):
     assert "email" in auths[0][1]
 
     # verify logged in
-    response = client.get("/profile", follow_redirects=False)
-    assert response.status_code == 200
+    assert is_authenticated(client, get_message)
 
 
 @pytest.mark.settings(
@@ -712,8 +707,7 @@ def test_verify_link_spa(app, client, get_message):
     qparams = dict(parse_qsl(split.query))
     assert qparams["email"] == "matt@lp.com"
 
-    response = client.get("/profile", headers=headers)
-    assert response.status_code == 200
+    assert is_authenticated(client, get_message)
 
 
 def test_setup(app, client, get_message):
@@ -1308,8 +1302,7 @@ def test_confirmable(app, client, get_message):
     assert get_message("CONFIRMATION_REQUIRED") in response.data
 
     # Verify not authenticated
-    response = client.get("/profile", follow_redirects=False)
-    assert "/login?next=%2Fprofile" in response.location
+    assert not is_authenticated(client, get_message)
 
 
 @pytest.mark.registerable()
@@ -1443,9 +1436,7 @@ def authned(myapp, user, **extra_args):
     assert response.status_code == 200
     assert client.get_cookie("remember_token")
     assert "password" in auths[0][1]
-
-    response = client.get("/profile", follow_redirects=False)
-    assert response.status_code == 200
+    assert is_authenticated(client, get_message)
 
 
 @pytest.mark.settings(
@@ -1591,8 +1582,7 @@ def test_tf_not(app, client, get_message):
     # assert "sms" in auths[1][1]
 
     # Verify authenticated
-    response = client.get("/profile", follow_redirects=False)
-    assert response.status_code == 200
+    assert is_authenticated(client, get_message)
 
 
 @pytest.mark.settings(sms_service="bad")
@@ -1840,11 +1830,7 @@ def test_us_tf_validity(app, client, get_message):
     )
     assert response.status_code == 200
     # verify logged in
-    response = client.get("/profile", follow_redirects=False)
-
-    assert response.status_code == 200
-    assert b"Welcome gal@lp.com" in response.data
-
+    assert is_authenticated(client, get_message)
     logout(client)
 
     data["identity"] = "gal2@lp.com"

tests/test_utils.py

@@ -49,6 +49,20 @@ def json_authenticate(client, email="matt@lp.com", password="password", endpoint
     return client.post(ep, content_type="application/json", data=data)
 
 
+def is_authenticated(client, get_message):
+    # Return True is 'client' is authenticated.
+    # Return False is not
+    # Raise ValueError not certain...
+    response = client.get("/profile", headers={"accept": "application/json"})
+    if response.status_code == 200:
+        return True
+    if response.status_code == 401 and response.json["response"]["errors"][0].encode(
+        "utf-8"
+    ) == get_message("UNAUTHENTICATED"):
+        return False
+    raise ValueError("Failed to figure out if authenticated")
+
+
 def verify_token(client_nc, token, status=None):
     # Use passed auth token in API that requires auth and verify status.
     # Pass in a client_nc to get valid results.