Fix auth responses

server/mergin/auth/api.yaml

@@ -360,8 +360,6 @@ paths:
           $ref: "#/components/responses/BadStatusResp"
         "401":
           $ref: "#/components/responses/UnauthorizedError"
-        "423":
-          $ref: "#/components/responses/LockedResp"
   /app/auth/logout:
     get:
       summary: Logout
@@ -430,10 +428,6 @@ paths:
           description: OK
         "400":
           $ref: "#/components/responses/BadStatusResp"
-        "403":
-          $ref: "#/components/responses/Forbidden"
-        "404":
-          $ref: "#/components/responses/NotFoundResp"
   /app/auth/reset-password/{token}:
     post:
       summary: Confirm reset password
@@ -639,8 +633,6 @@ paths:
           $ref: "#/components/responses/NotFoundResp"
         "415":
           $ref: "#/components/responses/UnsupportedMediaType"
-        "423":
-          $ref: "#/components/responses/LockedResp"
       x-openapi-router-controller: mergin.auth.controller
   /app/admin/login:
     post:
@@ -670,8 +662,6 @@ paths:
           $ref: "#/components/responses/UnauthorizedError"
         "403":
           $ref: "#/components/responses/Forbidden"
-        "423":
-          $ref: "#/components/responses/LockedResp"
   /v2/users:
     post:
       tags:
@@ -744,8 +734,6 @@ components:
       description: Request could not be processed becuase of conflict in resources
     UnprocessableEntity:
       description: Request was correct and yet server could not process it
-    LockedResp:
-      description: Account is temporarily locked due to too many failed login attempts.
     NoContent:
       description: Success. No content returned.
   schemas:

server/mergin/auth/app.py

@@ -3,6 +3,7 @@
 # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial
 
 import functools
+import logging
 from typing import Optional
 from blinker import signal
 from flask import current_app, render_template, Flask
@@ -12,8 +13,7 @@
 
 from .commands import add_commands
 from .config import Configuration
-from .models import User
-from .errors import AccountLockedError
+from .models import User, _check_dummy_password
 
 # signal for other versions to listen to
 user_account_closed = signal("user_account_closed")
@@ -101,9 +101,12 @@ def authenticate(login, password):
         query = func.lower(User.username) == func.lower(login)
     user = User.query.filter(query).one_or_none()
     if user is None:
+        _check_dummy_password(password)
         return None
     if user.is_locked_out():
-        raise AccountLockedError()
+        logging.info(f"Rejected login attempt for locked-out user {user.id}")
+        _check_dummy_password(password)
+        return None
     needs_commit = False
     # reset non-null locked_until as it has already expired
     if user.locked_until is not None:

server/mergin/auth/controller.py

@@ -26,7 +26,6 @@
 )
 from .bearer import encode_token
 from .models import User, LoginHistory
-from .errors import AccountLockedError
 from .schemas import UserSchema, UserSearchSchema, UserProfileSchema, UserInfoSchema
 from .forms import (
     LoginForm,
@@ -140,10 +139,7 @@ def login_public():  # noqa: E501
     """
     form = ApiLoginForm()
     if form.validate():
-        try:
-            user = authenticate(form.login.data, form.password.data)
-        except AccountLockedError as e:
-            return e.response(423)
+        user = authenticate(form.login.data, form.password.data)
         if user and user.active:
             expire = datetime.now(pytz.utc) + timedelta(
                 seconds=current_app.config["BEARER_TOKEN_EXPIRATION"]
@@ -227,10 +223,7 @@ def search_users():  # pylint: disable=W0613,W0612
 def login():  # pylint: disable=W0613,W0612
     form = LoginForm()
     if form.validate():
-        try:
-            user = authenticate(form.login.data, form.password.data)
-        except AccountLockedError as e:
-            return e.response(423)
+        user = authenticate(form.login.data, form.password.data)
         if user and user.active:
             login_user(user)
             if not os.path.isfile(current_app.config["MAINTENANCE_FILE"]):
@@ -247,10 +240,7 @@ def admin_login():  # pylint: disable=W0613,W0612
     if not form.validate():
         return jsonify(form.errors), 400
 
-    try:
-        user = authenticate(form.login.data, form.password.data)
-    except AccountLockedError as e:
-        return e.response(423)
+    user = authenticate(form.login.data, form.password.data)
     if user:
         if user.active and user.is_admin:
             login_user(user)
@@ -300,25 +290,18 @@ def password_reset():  # pylint: disable=W0613,W0612
     if not form.validate():
         return jsonify(form.errors), 400
 
+    # respond the same regardless of account existence/state (enumeration)
     user = User.query.filter(
         func.lower(User.email) == func.lower(form.email.data.strip())
     ).one_or_none()
-    if not user:
-        return jsonify({"email": ["Account with given email does not exist"]}), 404
-    if not user.active:
-        # user should confirm email first
-        return jsonify({"email": ["Account is not active"]}), 400
-    if not user.can_edit_profile:
-        # using SSO
-        abort(403, CANNOT_EDIT_PROFILE_MSG)
-
-    send_confirmation_email(
-        current_app,
-        user,
-        "change-password",
-        "email/password_reset.html",
-        "Password reset",
-    )
+    if user and user.active and user.can_edit_profile:
+        send_confirmation_email(
+            current_app,
+            user,
+            "change-password",
+            "email/password_reset.html",
+            "Password reset",
+        )
     return "", 200
 
 

server/mergin/auth/models.py

@@ -17,6 +17,12 @@
 MAX_USERNAME_LENGTH = 50
 
 
+def _check_dummy_password(password: str) -> None:
+    """Burn the same bcrypt cost as a real check, without an actual user."""
+    rounds = current_app.config.get("BCRYPT_LOG_ROUNDS", 12)
+    bcrypt.hashpw(password.encode("utf-8"), bcrypt.gensalt(rounds))
+
+
 def _parse_lockout_policy(policy_str: str) -> list:
     """Parse "5:300,10:3600" into [(5, 300), (10, 3600)] sorted ascending by threshold."""
     result = []
@@ -66,7 +72,8 @@ def __repr__(self):
     def check_password(self, password):
         # users created through SSO
         if self.passwd is None:
-            return
+            _check_dummy_password(password)
+            return False
         if isinstance(password, str):
             password = password.encode("utf-8")
         return bcrypt.checkpw(password, self.passwd.encode("utf-8"))

server/mergin/tests/test_auth.py

@@ -111,14 +111,16 @@ def test_login_lockout(send_email_mock, client):
     """
     user = add_user("lockoutuser", "correctpassword")
     since = datetime.utcnow() - timedelta(hours=1)
+    baseline = None
 
     def assert_locked():
+        # must be byte-identical to an ordinary wrong-password response
         resp = client.post(
             url_for("/.mergin_auth_controller_login"),
             json={"login": "lockoutuser", "password": "wrong"},
         )
-        assert resp.status_code == 423
-        assert resp.json["code"] == "AccountLocked"
+        assert resp.status_code == baseline.status_code
+        assert resp.json == baseline.json
 
     with patch.dict(client.application.config, {"LOCKOUT_POLICY": "3:60,4:3600"}):
         # tier 1: 3 failures → 60s lock
@@ -128,6 +130,9 @@ def assert_locked():
                 json={"login": "lockoutuser", "password": "wrong"},
             )
             assert resp.status_code == 401
+            if baseline is None:
+                # baseline shape before any lock kicks in
+                baseline = resp
 
         # lockout email dispatched exactly once, at the moment the lock triggers
         assert send_email_mock.call_count == 1
@@ -139,13 +144,14 @@ def assert_locked():
             url_for("/.mergin_auth_controller_login"),
             json={"login": "lockoutuser", "password": "correctpassword"},
         )
-        assert resp.status_code == 423
+        assert resp.status_code == baseline.status_code
+        assert resp.json == baseline.json
 
         # no new failures recorded while already locked out
         assert LoginHistory.count_recent_failures(user.id, since) == 3
         assert user.locked_until is not None
 
-        # no further emails while already locked out (attempts above were all 423s)
+        # no further emails while already locked out (attempts above were all masked 401s)
         assert send_email_mock.call_count == 1
 
         # tier 2 escalation: one more failure after tier-1 expiry
@@ -340,6 +346,43 @@ def test_login_history_records_failures(client):
     assert user.last_signed_in == last_signed_in
 
 
+@patch("mergin.celery.send_email_async.apply_async")
+def test_invalid_login_timing(send_email_mock, client):
+    """A bcrypt operation must run for every login outcome - nonexistent
+    user, locked-out user, SSO account, and real wrong password.
+    """
+    import bcrypt
+
+    def login_attempt(login, password="dummy"):
+        client.post(
+            url_for("/.mergin_auth_controller_login"),
+            json={"login": login, "password": password},
+        )
+
+    locked_user = add_user("timinguser", "correctpassword")
+    with patch(
+        "mergin.auth.models.bcrypt.hashpw", wraps=bcrypt.hashpw
+    ) as mock_hashpw, patch(
+        "mergin.auth.models.bcrypt.checkpw", wraps=bcrypt.checkpw
+    ) as mock_checkpw:
+        login_attempt("no-such-user")
+        assert mock_hashpw.call_count + mock_checkpw.call_count == 1
+
+        with patch.dict(client.application.config, {"LOCKOUT_POLICY": "1:3600"}):
+            login_attempt("timinguser", "wrong")  # real check, also triggers the lock
+            assert mock_hashpw.call_count + mock_checkpw.call_count == 2
+            assert locked_user.is_locked_out()
+
+            login_attempt("timinguser", "wrong")  # now locked - dummy path
+            assert mock_hashpw.call_count + mock_checkpw.call_count == 3
+
+        sso_user = User("ssouser", "sso@test.com")
+        db.session.add(sso_user)
+        db.session.commit()
+        login_attempt("ssouser")  # SSO - dummy path
+        assert mock_hashpw.call_count + mock_checkpw.call_count == 4
+
+
 def test_bcrypt_lazy_rehash(app):
     """Password is transparently rehashed on login when the cost factor changes."""
     import bcrypt
@@ -540,12 +583,12 @@ def test_confirm_password(app, client):
     assert resp.status_code == 400
 
 
-# reset password tests: success, no email, not-existing user
+# reset password tests: success, no email, not-existing user (200 - masked)
 test_reset_data = [
     ({"email": "mergin@mergin.com"}, 200),
     ({"email": "Mergin@mergin.com"}, 200),  # case insensitive
     ({}, 400),
-    ({"email": "tests@mergin.com"}, 404),
+    ({"email": "tests@mergin.com"}, 200),
 ]
 
 
@@ -559,6 +602,26 @@ def test_reset_password(client, data, expected):
     assert resp.status_code == expected
 
 
+@patch("mergin.celery.send_email_async.apply_async")
+def test_reset_password_masks_account_existence(send_email_mock, client):
+    """Response must be identical whether or not the account exists."""
+    resp_existing = client.post(
+        url_for("/.mergin_auth_controller_password_reset"),
+        json={"email": "mergin@mergin.com"},
+    )
+    assert resp_existing.status_code == 200
+    assert send_email_mock.call_count == 1
+
+    resp_missing = client.post(
+        url_for("/.mergin_auth_controller_password_reset"),
+        json={"email": "no-such-user@mergin.com"},
+    )
+    assert resp_missing.status_code == resp_existing.status_code
+    assert resp_missing.json == resp_existing.json
+    # no email dispatched for a nonexistent account
+    assert send_email_mock.call_count == 1
+
+
 def test_change_password(client):
     username = "user_test"
     old_password = "user_password"

web-app/packages/lib/src/modules/user/store.ts

@@ -2,7 +2,6 @@
 //
 // SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial
 
-import axios from 'axios'
 import { defineStore, getActivePinia } from 'pinia'
 import { isNavigationFailure } from 'vue-router'
 
@@ -19,7 +18,6 @@
   ResetPasswordPayload,
   ChangePasswordWithTokenPayload,
   ChangePasswordPayload,
-  ErrorCodes,
   IsWorkspaceAdminPayload,
   LoginPayload,
   UserDetailResponse,
@@ -240,21 +238,11 @@
     async userLogin(payload: LoginPayload) {
       const instanceStore = useInstanceStore()
       const formStore = useFormStore()
-      const notificationStore = useNotificationStore()
 
       try {
         await UserApi.login(payload.data)
         await instanceStore.initApp()
       } catch (err) {
-        if (axios.isAxiosError(err)) {
-          const code = err.response?.data?.code as ErrorCodes
-          if (code === 'AccountLocked') {
-            await notificationStore.error({
-              text: 'Your account is temporarily locked due to too many failed login attempts. Please check your email for a link to unlock it.'
-            })
-            return
-          }
-        }
         await formStore.handleError({
           componentId: payload.componentId,
           error: err,
@@ -311,7 +299,7 @@
         await notificationStore.show({
           text: `Email was sent to address: ${payload.email}`
         })
       } catch (err) {
         await notificationStore.error({
           text: 'Failed to send confirmation email, please check your address in user profile settings'
         })

web-app/packages/lib/src/modules/user/types.ts

@@ -152,6 +152,4 @@ export interface UserRouteParams {
   reset?: string
 }
 
-export type ErrorCodes = 'AccountLocked'
-
 /* eslint-enable camelcase */