From c0d57d29ea9d2906d0b3aef29d09333d669be077 Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Mon, 15 Jun 2026 14:26:16 +0200 Subject: [PATCH 1/5] Fixed session cookie gap for deactivated users - `load_user` now returns None for inactive users, so their session cookies are rejected by Flask-Login - @auth_required adds `is_active` check - anonymize() now explicitly sets active=False for defence-in-depth --- server/mergin/app.py | 4 +++- server/mergin/auth/app.py | 6 +++++- server/mergin/auth/models.py | 1 + server/mergin/tests/test_auth.py | 20 +++++++++++++++++++- 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/server/mergin/app.py b/server/mergin/app.py index e5eb42d5..77d5a5ac 100644 --- a/server/mergin/app.py +++ b/server/mergin/app.py @@ -188,7 +188,9 @@ def create_app(public_keys: List[str] = None) -> Flask: # adjust login manager @login_manager.user_loader def load_user(user_id): # pylint: disable=W0613,W0612 - return User.query.get(user_id) + user = User.query.get(user_id) + if user and user.active: + return user @login_manager.header_loader def load_user_from_header(header_val): # pylint: disable=W0613,W0612 diff --git a/server/mergin/auth/app.py b/server/mergin/auth/app.py index acfccf43..2f57bc73 100644 --- a/server/mergin/auth/app.py +++ b/server/mergin/auth/app.py @@ -61,7 +61,11 @@ def auth_required(f=None, permissions=None): @functools.wraps(f) def wrapped_func(*args, **kwargs): - if not current_user or not current_user.is_authenticated: + if ( + not current_user + or not current_user.is_authenticated + or not current_user.is_active + ): return "Authentication information is missing or invalid.", 401 if permissions: for check_permission in permissions: diff --git a/server/mergin/auth/models.py b/server/mergin/auth/models.py index 760ab740..390fbbe0 100644 --- a/server/mergin/auth/models.py +++ b/server/mergin/auth/models.py @@ -185,6 +185,7 @@ def anonymize(self): """Anonymize user object in database - remove personal information""" ts = round(datetime.datetime.utcnow().timestamp() * 1000) del_str = f"deleted_{ts}" + self.active = False self.username = del_str self.email = None self.passwd = None diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index ba7730c3..dc08cd57 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -94,6 +94,23 @@ def test_logout(client): assert resp.status_code == 200 +def test_deactivated_user_session_rejected(client): + """Session cookie for a deactivated account must be rejected.""" + user = add_user("testdeactivate", "testpassword") + login(client, "testdeactivate", "testpassword") + + # session works before deactivation + resp = client.get(f"/v1/user/{user.username}") + assert resp.status_code == 200 + + user.active = False + db.session.commit() + + # same session must now be rejected + resp = client.get(f"/v1/user/{user.username}") + assert resp.status_code == 401 + + # user registration tests test_user_reg_data = [ ("test@test.com", "#pwd1234", 201), # success @@ -469,7 +486,8 @@ def test_update_user(client): data=json.dumps(data), headers=json_headers, ) - assert resp.status_code == 403 + # user is deactivated, so session is rejected before permission check + assert resp.status_code == 401 def test_update_user_profile(client): From 26a0698e25aa0dd95e4719d6dc20bfd5bd15d566 Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Mon, 15 Jun 2026 14:37:49 +0200 Subject: [PATCH 2/5] Add configurable bcrypt cost factor Existing passwords will be rehashed organically if needed. --- server/mergin/auth/app.py | 5 +++++ server/mergin/auth/config.py | 1 + server/mergin/auth/models.py | 15 ++++++++++++++- server/mergin/tests/test_auth.py | 22 ++++++++++++++++++++++ 4 files changed, 42 insertions(+), 1 deletion(-) diff --git a/server/mergin/auth/app.py b/server/mergin/auth/app.py index 2f57bc73..d62462da 100644 --- a/server/mergin/auth/app.py +++ b/server/mergin/auth/app.py @@ -91,12 +91,17 @@ def wrapped_func(*args, **kwargs): def authenticate(login, password): + from ..app import db + if "@" in login: query = func.lower(User.email) == func.lower(login) else: query = func.lower(User.username) == func.lower(login) user = User.query.filter(query).one_or_none() if user and user.check_password(password): + if user.needs_rehash(): + user.assign_password(password) + db.session.commit() return user diff --git a/server/mergin/auth/config.py b/server/mergin/auth/config.py index 07b5a05d..a04a4e82 100644 --- a/server/mergin/auth/config.py +++ b/server/mergin/auth/config.py @@ -13,3 +13,4 @@ class Configuration(object): "BEARER_TOKEN_EXPIRATION", default=3600 * 12, 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) diff --git a/server/mergin/auth/models.py b/server/mergin/auth/models.py index 390fbbe0..d20325e2 100644 --- a/server/mergin/auth/models.py +++ b/server/mergin/auth/models.py @@ -64,12 +64,25 @@ def check_password(self, password): def assign_password(self, password): if isinstance(password, str): password = password.encode("utf-8") + rounds = current_app.config.get("BCRYPT_LOG_ROUNDS", 12) self.passwd = ( - bcrypt.hashpw(password, bcrypt.gensalt()).decode("utf-8") + bcrypt.hashpw(password, bcrypt.gensalt(rounds)).decode("utf-8") if password else None ) + def needs_rehash(self): + """Return True if the stored hash was generated with a different cost factor than configured.""" + if self.passwd is None: + return False + rounds = current_app.config.get("BCRYPT_LOG_ROUNDS", 12) + try: + # bcrypt hash format: $2b$$ + hash_rounds = int(self.passwd.split("$")[2]) + return hash_rounds != rounds + except (IndexError, ValueError): + return False + @property def is_authenticated(self): """For Flask-Login""" diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index dc08cd57..ea91b3ca 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -94,6 +94,28 @@ def test_logout(client): assert resp.status_code == 200 +def test_bcrypt_lazy_rehash(app): + """Password is transparently rehashed on login when the cost factor changes.""" + import bcrypt + from ..auth.app import authenticate + + user = add_user("rehashuser", "rehashpassword") + # Store a hash with a low cost factor (4 is the minimum bcrypt allows) + low_rounds_hash = bcrypt.hashpw(b"rehashpassword", bcrypt.gensalt(4)).decode( + "utf-8" + ) + user.passwd = low_rounds_hash + db.session.commit() + + app.config["BCRYPT_LOG_ROUNDS"] = 5 + result = authenticate("rehashuser", "rehashpassword") + assert result is not None + + db.session.refresh(user) + hash_rounds = int(user.passwd.split("$")[2]) + assert hash_rounds == 5 + + def test_deactivated_user_session_rejected(client): """Session cookie for a deactivated account must be rejected.""" user = add_user("testdeactivate", "testpassword") From ce00ed3e1d3edd2d75bf1b2dbb5539c501bef955 Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Wed, 17 Jun 2026 15:08:21 +0200 Subject: [PATCH 3/5] Add temporary account lockout --- server/mergin/auth/api.yaml | 8 +++ server/mergin/auth/app.py | 17 ++++- server/mergin/auth/config.py | 2 + server/mergin/auth/controller.py | 16 ++++- server/mergin/auth/errors.py | 21 ++++++ server/mergin/auth/models.py | 46 +++++++++++++ server/mergin/tests/test_auth.py | 69 +++++++++++++++++++ .../a3c8f2e1d947_add_login_lockout_fields.py | 42 +++++++++++ 8 files changed, 217 insertions(+), 4 deletions(-) create mode 100644 server/mergin/auth/errors.py create mode 100644 server/migrations/community/a3c8f2e1d947_add_login_lockout_fields.py diff --git a/server/mergin/auth/api.yaml b/server/mergin/auth/api.yaml index fa482a36..a4c7b637 100644 --- a/server/mergin/auth/api.yaml +++ b/server/mergin/auth/api.yaml @@ -360,6 +360,8 @@ paths: $ref: "#/components/responses/BadStatusResp" "401": $ref: "#/components/responses/UnauthorizedError" + "423": + $ref: "#/components/responses/LockedResp" /app/auth/logout: get: summary: Logout @@ -617,6 +619,8 @@ 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: @@ -646,6 +650,8 @@ paths: $ref: "#/components/responses/UnauthorizedError" "403": $ref: "#/components/responses/Forbidden" + "423": + $ref: "#/components/responses/LockedResp" /v2/users: post: tags: @@ -718,6 +724,8 @@ 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: diff --git a/server/mergin/auth/app.py b/server/mergin/auth/app.py index d62462da..f9d1a038 100644 --- a/server/mergin/auth/app.py +++ b/server/mergin/auth/app.py @@ -12,6 +12,7 @@ from .commands import add_commands from .config import Configuration from .models import User +from .errors import AccountLockedError # signal for other versions to listen to user_account_closed = signal("user_account_closed") @@ -98,11 +99,25 @@ def authenticate(login, password): else: query = func.lower(User.username) == func.lower(login) user = User.query.filter(query).one_or_none() - if user and user.check_password(password): + if user is None: + return None + needs_commit = False + if user.is_locked_out(): + raise AccountLockedError(user.locked_until) + if user.check_password(password): + if user.failed_login_attempts or user.locked_until: + user.reset_lockout() + needs_commit = True if user.needs_rehash(): user.assign_password(password) + needs_commit = True + if needs_commit: db.session.commit() return user + else: + user.record_failed_login() + db.session.commit() + return None def generate_confirmation_token(app, email, salt): diff --git a/server/mergin/auth/config.py b/server/mergin/auth/config.py index a04a4e82..3d2215ee 100644 --- a/server/mergin/auth/config.py +++ b/server/mergin/auth/config.py @@ -14,3 +14,5 @@ class Configuration(object): ) # 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" + LOCKOUT_POLICY = config("LOCKOUT_POLICY", default="5:300,10:3600") diff --git a/server/mergin/auth/controller.py b/server/mergin/auth/controller.py index 06859255..ed104641 100644 --- a/server/mergin/auth/controller.py +++ b/server/mergin/auth/controller.py @@ -25,6 +25,7 @@ ) 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, @@ -137,7 +138,10 @@ def login_public(): # noqa: E501 """ form = ApiLoginForm() if form.validate(): - user = authenticate(form.login.data, form.password.data) + try: + user = authenticate(form.login.data, form.password.data) + except AccountLockedError as e: + return e.response(423) if user and user.active: expire = datetime.now(pytz.utc) + timedelta( seconds=current_app.config["BEARER_TOKEN_EXPIRATION"] @@ -221,7 +225,10 @@ def search_users(): # pylint: disable=W0613,W0612 def login(): # pylint: disable=W0613,W0612 form = LoginForm() if form.validate(): - user = authenticate(form.login.data, form.password.data) + try: + user = authenticate(form.login.data, form.password.data) + except AccountLockedError as e: + return e.response(423) if user and user.active: login_user(user) if not os.path.isfile(current_app.config["MAINTENANCE_FILE"]): @@ -238,7 +245,10 @@ def admin_login(): # pylint: disable=W0613,W0612 if not form.validate(): return jsonify(form.errors), 400 - user = authenticate(form.login.data, form.password.data) + try: + user = authenticate(form.login.data, form.password.data) + except AccountLockedError as e: + abort(423, f"Account temporarily locked until {e.locked_until.isoformat()}") if user: if user.active and user.is_admin: login_user(user) diff --git a/server/mergin/auth/errors.py b/server/mergin/auth/errors.py new file mode 100644 index 00000000..a337bb77 --- /dev/null +++ b/server/mergin/auth/errors.py @@ -0,0 +1,21 @@ +# Copyright (C) Lutra Consulting Limited +# +# SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial + +import datetime +from typing import Dict + +from ..app import ResponseError + + +class AccountLockedError(Exception, ResponseError): + code = "AccountLocked" + detail = "Account temporarily locked due to too many failed login attempts" + + def __init__(self, locked_until: datetime.datetime): + self.locked_until = locked_until + + def to_dict(self) -> Dict: + data = super().to_dict() + data["locked_until"] = self.locked_until.isoformat() + return data diff --git a/server/mergin/auth/models.py b/server/mergin/auth/models.py index d20325e2..234fee44 100644 --- a/server/mergin/auth/models.py +++ b/server/mergin/auth/models.py @@ -13,10 +13,20 @@ from ..app import db from ..sync.models import ProjectUser from ..sync.utils import get_user_agent, get_ip, get_device_id, is_reserved_word +from .errors import AccountLockedError MAX_USERNAME_LENGTH = 50 +def _parse_lockout_policy(policy_str: str) -> list: + """Parse "5:300,10:3600" into [(5, 300), (10, 3600)] sorted ascending by threshold.""" + result = [] + for part in policy_str.split(","): + threshold, seconds = part.strip().split(":") + result.append((int(threshold), int(seconds))) + return sorted(result, key=lambda x: x[0]) + + class User(db.Model): id = db.Column(db.Integer, primary_key=True) username = db.Column(db.String(80), info={"label": "Username"}) @@ -33,6 +43,10 @@ class User(db.Model): default=datetime.datetime.utcnow, ) last_signed_in = db.Column(db.DateTime(), nullable=True) + failed_login_attempts = db.Column( + db.Integer, default=0, nullable=False, server_default="0" + ) + locked_until = db.Column(db.DateTime(), nullable=True) receive_notifications = db.Column( db.Boolean, default=True, nullable=False, index=True ) @@ -83,6 +97,38 @@ def needs_rehash(self): except (IndexError, ValueError): return False + def is_locked_out(self) -> bool: + """Return True if the account is currently under a temporary lockout.""" + if self.locked_until is None: + return False + now = datetime.datetime.utcnow() + if self.locked_until <= now: + # lockout has expired — clear it so subsequent queries see a clean state + self.locked_until = None + return False + return True + + def record_failed_login(self) -> None: + """Increment the failed-login counter and apply a lockout if a threshold is crossed.""" + self.failed_login_attempts = (self.failed_login_attempts or 0) + 1 + policy = _parse_lockout_policy( + current_app.config.get("LOCKOUT_POLICY", "5:300,10:3600") + ) + # find the highest applicable tier + duration = None + for threshold, seconds in policy: + if self.failed_login_attempts >= threshold: + duration = seconds + if duration is not None: + self.locked_until = datetime.datetime.utcnow() + datetime.timedelta( + seconds=duration + ) + + def reset_lockout(self) -> None: + """Clear lockout state after a successful login.""" + self.failed_login_attempts = 0 + self.locked_until = None + @property def is_authenticated(self): """For Flask-Login""" diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index ea91b3ca..dd52ebba 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -94,6 +94,75 @@ def test_logout(client): assert resp.status_code == 200 +def test_login_lockout(client): + """Test account lockout: progressive tiers, freeze during lock, reset on success. + + policy: 3 failures → 60s lock, 4 failures → 3600s lock + counter is never reset between lockouts, so tier-2 is reached after one + extra failure following the first expired tier-1 lock + """ + client.application.config["LOCKOUT_POLICY"] = "3:60,4:3600" + user = add_user("lockoutuser", "correctpassword") + + def assert_locked(): + 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 "locked_until" in resp.json + + # tier 1: 3 failures → 60s lock + for _ in range(3): + resp = client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "lockoutuser", "password": "wrong"}, + ) + assert resp.status_code == 401 + + assert_locked() + + # correct password is also blocked while locked + resp = client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "lockoutuser", "password": "correctpassword"}, + ) + assert resp.status_code == 423 + + # counter stays frozen during lockout + assert user.failed_login_attempts == 3 + assert user.locked_until is not None + + # tier 2 escalation: one more failure after tier-1 expiry + # counter was at 3; one new failure pushes it to 4, crossing tier-2 threshold + + # expire_lock + user.locked_until = datetime.now(tz=timezone.utc) - timedelta(seconds=1) + db.session.commit() + + resp = client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "lockoutuser", "password": "wrong"}, + ) + # returns 401 (wrong password), but now locked for 3600s + assert resp.status_code == 401 + assert_locked() + assert user.locked_until > datetime.now(tz=timezone.utc) + timedelta(seconds=60) + assert user.failed_login_attempts == 4 + + # successful login after expiry resets everything + user.locked_until = datetime.now(tz=timezone.utc) - timedelta(seconds=1) + db.session.commit() + resp = client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "lockoutuser", "password": "correctpassword"}, + ) + assert resp.status_code == 200 + assert user.failed_login_attempts == 0 + assert user.locked_until is None + + def test_bcrypt_lazy_rehash(app): """Password is transparently rehashed on login when the cost factor changes.""" import bcrypt diff --git a/server/migrations/community/a3c8f2e1d947_add_login_lockout_fields.py b/server/migrations/community/a3c8f2e1d947_add_login_lockout_fields.py new file mode 100644 index 00000000..a4a453e1 --- /dev/null +++ b/server/migrations/community/a3c8f2e1d947_add_login_lockout_fields.py @@ -0,0 +1,42 @@ +"""Add failed_login_attempts and locked_until to user table + +Revision ID: a3c8f2e1d947 +Revises: f1d9e4a7b823 +Create Date: 2026-06-15 00:00:00.000000 + +""" + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "a3c8f2e1d947" +down_revision = "a1b2c3d4e5f6" +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column( + "user", + sa.Column( + "failed_login_attempts", + sa.Integer(), + nullable=False, + server_default="0", + ), + ) + op.add_column( + "user", + sa.Column( + "locked_until", + sa.DateTime(), + nullable=True, + ), + ) + + +def downgrade(): + op.drop_column("user", "locked_until") + op.drop_column("user", "failed_login_attempts") From 33732207e74bfd3d238ef33879a09d6b60c542c7 Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Fri, 19 Jun 2026 13:26:38 +0200 Subject: [PATCH 4/5] Fix tests --- server/mergin/tests/test_auth.py | 8 ++++---- .../community/a3c8f2e1d947_add_login_lockout_fields.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index dd52ebba..a3ee168f 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -2,7 +2,7 @@ # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial -from datetime import datetime, timedelta, timezone +from datetime import datetime, timedelta import time import itsdangerous import pytest @@ -138,7 +138,7 @@ def assert_locked(): # counter was at 3; one new failure pushes it to 4, crossing tier-2 threshold # expire_lock - user.locked_until = datetime.now(tz=timezone.utc) - timedelta(seconds=1) + user.locked_until = datetime.utcnow() - timedelta(seconds=1) db.session.commit() resp = client.post( @@ -148,11 +148,11 @@ def assert_locked(): # returns 401 (wrong password), but now locked for 3600s assert resp.status_code == 401 assert_locked() - assert user.locked_until > datetime.now(tz=timezone.utc) + timedelta(seconds=60) + assert user.locked_until > datetime.utcnow() + timedelta(seconds=60) assert user.failed_login_attempts == 4 # successful login after expiry resets everything - user.locked_until = datetime.now(tz=timezone.utc) - timedelta(seconds=1) + user.locked_until = datetime.utcnow() - timedelta(seconds=1) db.session.commit() resp = client.post( url_for("/.mergin_auth_controller_login"), diff --git a/server/migrations/community/a3c8f2e1d947_add_login_lockout_fields.py b/server/migrations/community/a3c8f2e1d947_add_login_lockout_fields.py index a4a453e1..bcd7f76a 100644 --- a/server/migrations/community/a3c8f2e1d947_add_login_lockout_fields.py +++ b/server/migrations/community/a3c8f2e1d947_add_login_lockout_fields.py @@ -12,7 +12,7 @@ # revision identifiers, used by Alembic. revision = "a3c8f2e1d947" -down_revision = "a1b2c3d4e5f6" +down_revision = "f1d9e4a7b823" branch_labels = None depends_on = None From f2708ce1cdd082d964cd6b0ea5ff85e516edfb02 Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Fri, 19 Jun 2026 13:46:06 +0200 Subject: [PATCH 5/5] Add missing import --- server/mergin/tests/test_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index a3ee168f..2294de49 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -2,7 +2,7 @@ # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone import time import itsdangerous import pytest