Make password reset tokens single-use

deployment/community/.env.template

@@ -57,6 +57,8 @@ SECRET_KEY=fixme
 
 #BEARER_TOKEN_EXPIRATION=3600 * 12  # in seconds
 
+#PASSWORD_RESET_TOKEN_EXPIRATION=900  # in seconds
+
 #SECURITY_BEARER_SALT=NODEFAULT
 SECURITY_BEARER_SALT=fixme
 

deployment/enterprise/.env.template

@@ -62,6 +62,8 @@ SECRET_KEY=fixme
 
 #BEARER_TOKEN_EXPIRATION=3600 * 12  # in seconds
 
+#PASSWORD_RESET_TOKEN_EXPIRATION=900  # in seconds
+
 #SECURITY_BEARER_SALT=NODEFAULT
 SECURITY_BEARER_SALT=fixme
 

server/mergin/auth/app.py

@@ -140,33 +140,50 @@ def authenticate(login, password):
         return None
 
 
-def generate_confirmation_token(app, email, salt):
+def generate_confirmation_token(app, data, salt):
     serializer = URLSafeTimedSerializer(app.config["SECRET_KEY"])
-    return serializer.dumps(email, salt=salt)
+    return serializer.dumps(data, salt=salt)
 
 
 def confirm_token(token, salt, expiration=3600):
     serializer = URLSafeTimedSerializer(current_app.config["SECRET_KEY"])
     try:
-        email = serializer.loads(token, salt=salt, max_age=expiration)
+        data = serializer.loads(token, salt=salt, max_age=expiration)
     except:
         return
-    return email
+    return data
 
 
 def send_confirmation_email(app, user, url, template, header, **kwargs):
     """
     Send confirmation email from selected template with customizable email subject and confirmation URL.
     Optional kwargs are passed to render_template method if needed for particular template.
     """
-    from ..celery import send_email_async
+    token = generate_confirmation_token(
+        app, user.email, app.config["SECURITY_EMAIL_SALT"]
+    )
+    _send_token_email(app, user, url, token, template, header, **kwargs)
+
 
-    salt = (
-        app.config["SECURITY_EMAIL_SALT"]
-        if url == "confirm-email"
-        else app.config["SECURITY_PASSWORD_SALT"]
+def send_password_reset_email(app: Flask, user: User) -> None:
+    """Issue a new single-use password reset token (revoking any previous one) and email it."""
+    from ..app import db
+
+    token = generate_password_reset_token(app, user)
+    db.session.commit()
+    _send_token_email(
+        app,
+        user,
+        "change-password",
+        token,
+        "email/password_reset.html",
+        "Password reset",
     )
-    token = generate_confirmation_token(app, user.email, salt)
+
+
+def _send_token_email(app, user, url, token, template, header, **kwargs):
+    from ..celery import send_email_async
+
     confirm_url = f"{url}/{token}"
     html = render_template(
         template, subject=header, confirm_url=confirm_url, user=user, **kwargs
@@ -180,6 +197,14 @@ def send_confirmation_email(app, user, url, template, header, **kwargs):
     send_email_async.delay(**email_data)
 
 
+def generate_password_reset_token(app: Flask, user: User) -> str:
+    """Sign a reset token bound to a fresh nonce, revoking previously issued ones."""
+    payload = {"email": user.email, "nonce": user.rotate_password_reset_nonce()}
+    return generate_confirmation_token(
+        app, payload, app.config["SECURITY_PASSWORD_SALT"]
+    )
+
+
 def generate_unlock_token(app: Flask, user: User) -> str:
     """Sign a token binding the current lock episode (email + locked_until) to the user."""
     serializer = URLSafeTimedSerializer(app.config["SECRET_KEY"])

server/mergin/auth/config.py

@@ -13,6 +13,9 @@ class Configuration(object):
     BEARER_TOKEN_EXPIRATION = config(
         "BEARER_TOKEN_EXPIRATION", default=3600 * 12, cast=int
     )  # in seconds
+    PASSWORD_RESET_TOKEN_EXPIRATION = config(
+        "PASSWORD_RESET_TOKEN_EXPIRATION", default=900, cast=int
+    )  # in seconds
     ACCOUNT_EXPIRATION = config("ACCOUNT_EXPIRATION", default=5, cast=int)  # in days
     BCRYPT_LOG_ROUNDS = config("BCRYPT_LOG_ROUNDS", default=12, cast=int)
     # Comma-separated "attempts:seconds" pairs, e.g. "5:300,10:3600"

server/mergin/auth/controller.py

@@ -16,6 +16,7 @@
     auth_required,
     authenticate,
     send_confirmation_email,
+    send_password_reset_email,
     confirm_token,
     generate_confirmation_token,
     confirm_unlock_token,
@@ -380,19 +381,18 @@ def password_reset():  # pylint: disable=W0613,W0612
         target_email=form.email.data.strip(),
     )
     if user and user.active and user.can_edit_profile:
-        send_confirmation_email(
-            current_app,
-            user,
-            "change-password",
-            "email/password_reset.html",
-            "Password reset",
-        )
+        send_password_reset_email(current_app, user)
     return "", 200
 
 
 def confirm_new_password(token):  # pylint: disable=W0613,W0612
-    email = confirm_token(token, salt=current_app.config["SECURITY_PASSWORD_SALT"])
-    if not email:
+    payload = confirm_token(
+        token,
+        salt=current_app.config["SECURITY_PASSWORD_SALT"],
+        expiration=current_app.config["PASSWORD_RESET_TOKEN_EXPIRATION"],
+    )
+    # tokens issued before nonces were introduced carried only the email
+    if not isinstance(payload, dict):
         emit(
             AuthEventType.USER_PASSWORD_RESET_FAILED,
             **request_context(),
@@ -402,6 +402,7 @@ def confirm_new_password(token):  # pylint: disable=W0613,W0612
         )
         abort(400, "Invalid token")
 
+    email = payload["email"]
     user = User.query.filter_by(email=email).first()
     if not user:
         emit(
@@ -412,6 +413,16 @@ def confirm_new_password(token):  # pylint: disable=W0613,W0612
             reason="user_not_found",
         )
         abort(404)
+    # token was already used, superseded by a newer one or revoked by a password change
+    if not user.password_reset_nonce or user.password_reset_nonce != payload["nonce"]:
+        emit(
+            AuthEventType.USER_PASSWORD_RESET_FAILED,
+            **request_context(),
+            target_user_id=user.id,
+            target_email=user.email,
+            reason="token_revoked",
+        )
+        abort(400, "Invalid token")
     if not user.active:
         emit(
             AuthEventType.USER_PASSWORD_RESET_FAILED,

server/mergin/auth/listeners.py

@@ -13,7 +13,7 @@
 from .models import User
 
 # Fields excluded from user.updated audit events:
-#   - sensitive values that must never appear in logs (passwd)
+#   - sensitive values that must never appear in logs (passwd, password_reset_nonce)
 #   - high-frequency operational fields (last_signed_in, registration_date)
 #   - lifecycle state fields covered by dedicated events (active, inactive_since)
 #   - is_admin covered by the dedicated user.admin_panel_access.changed event
@@ -22,6 +22,7 @@
 _EXCLUDED_FROM_USER_UPDATED = frozenset(
     {
         "passwd",
+        "password_reset_nonce",
         "last_signed_in",
         "registration_date",
         "active",

server/mergin/auth/models.py

@@ -4,6 +4,7 @@
 
 from __future__ import annotations
 import datetime
+import secrets
 from typing import List, Optional
 import bcrypt
 import re
@@ -51,6 +52,8 @@ class User(db.Model):
     )
     last_signed_in = db.Column(db.DateTime(), nullable=True)
     locked_until = db.Column(db.DateTime(), nullable=True)
+    # nonce of the only valid password reset token, cleared on any password change
+    password_reset_nonce = db.Column(db.String(64), nullable=True)
     receive_notifications = db.Column(
         db.Boolean, default=True, nullable=False, index=True
     )
@@ -89,6 +92,7 @@ def assign_password(self, password):
             if password
             else None
         )
+        self.password_reset_nonce = None
 
     def needs_rehash(self):
         """Return True if the stored hash was generated with a different cost factor than configured."""
@@ -143,6 +147,11 @@ def reset_lockout(self) -> None:
         """Clear lockout state after a successful login."""
         self.locked_until = None
 
+    def rotate_password_reset_nonce(self) -> str:
+        """Set a new password reset nonce, invalidating any previously issued reset token."""
+        self.password_reset_nonce = secrets.token_urlsafe(32)
+        return self.password_reset_nonce
+
     @property
     def is_authenticated(self):
         """For Flask-Login"""
@@ -267,6 +276,7 @@ def anonymize(self):
             self.username = del_str
             self.email = None
             self.passwd = None
+            self.password_reset_nonce = None
             self.first_name = None
             self.last_name = None
             db.session.commit()

server/mergin/tests/test_audit_events.py

@@ -12,7 +12,10 @@
 from flask import current_app
 
 from ..app import db
-from ..auth.app import generate_confirmation_token, generate_unlock_token
+from ..auth.app import (
+    generate_password_reset_token,
+    generate_unlock_token,
+)
 from ..auth.events import AuthEventType
 from ..auth.models import User
 from ..sync.events import SyncEventType
@@ -82,9 +85,10 @@ def test_user_password_changed(client, audit_capture):
 
 def test_user_password_reset(app, client, audit_capture):
     user = User.query.filter_by(username=DEFAULT_USER[0]).first()
-    token = generate_confirmation_token(
-        app, user.email, app.config["SECURITY_PASSWORD_SALT"]
-    )
+    token = generate_password_reset_token(app, user)
+    db.session.commit()
+    # the nonce is a secret - storing it must not emit user.updated
+    assert len(audit_capture.of_type(AuthEventType.USER_UPDATED)) == 0
 
     client.post(
         f"/app/auth/reset-password/{token}",
@@ -94,6 +98,16 @@ def test_user_password_reset(app, client, audit_capture):
     e = audit_capture.one(AuthEventType.USER_PASSWORD_RESET_COMPLETED)
     assert e.target_user_id == user.id
     assert e.metadata["target_email"] == user.email
+    assert len(audit_capture.of_type(AuthEventType.USER_UPDATED)) == 0
+
+    # reusing the token is rejected
+    client.post(
+        f"/app/auth/reset-password/{token}",
+        json={"password": "NewPass#456", "confirm": "NewPass#456"},
+    )
+    e = audit_capture.one(AuthEventType.USER_PASSWORD_RESET_FAILED)
+    assert e.target_user_id == user.id
+    assert e.metadata["reason"] == "token_revoked"
 
 
 def test_user_created(audit_capture):