diff --git a/deployment/community/.env.template b/deployment/community/.env.template index ea6b8ccc..cfb526ac 100644 --- a/deployment/community/.env.template +++ b/deployment/community/.env.template @@ -66,6 +66,9 @@ SECURITY_EMAIL_SALT=fixme #SECURITY_PASSWORD_SALT=NODEFAULT SECURITY_PASSWORD_SALT=fixme +#SECURITY_UNLOCK_SALT=NODEFAULT +SECURITY_UNLOCK_SALT=fixme + #WTF_CSRF_ENABLED=True #WTF_CSRF_TIME_LIMIT=3600 * 24 # in seconds @@ -109,6 +112,9 @@ LOCAL_PROJECTS=/data #BLACKLIST='.mergin/, .DS_Store, .directory' # cast=Csv() +# extra file extensions to permit beyond the default block-list, e.g. '.py, .sh' +#UPLOAD_EXTENSIONS_WHITELIST= + #FILE_EXPIRATION=48 * 3600 # for clean up of old files where diffs were applied, in seconds #LOCKFILE_EXPIRATION=300 # in seconds diff --git a/deployment/enterprise/.env.template b/deployment/enterprise/.env.template index 49a235cc..ebdb8716 100644 --- a/deployment/enterprise/.env.template +++ b/deployment/enterprise/.env.template @@ -71,6 +71,9 @@ SECURITY_EMAIL_SALT=fixme #SECURITY_PASSWORD_SALT=NODEFAULT SECURITY_PASSWORD_SALT=fixme +#SECURITY_UNLOCK_SALT=NODEFAULT +SECURITY_UNLOCK_SALT=fixme + #WTF_CSRF_ENABLED=True #WTF_CSRF_TIME_LIMIT=3600 * 24 # in seconds diff --git a/server/.test.env b/server/.test.env index 7545a7ce..0ab2ce8a 100644 --- a/server/.test.env +++ b/server/.test.env @@ -23,6 +23,7 @@ GEODIFF_WORKING_DIR=/tmp/geodiff SECURITY_BEARER_SALT='bearer' SECURITY_EMAIL_SALT='email' SECURITY_PASSWORD_SALT='password' +SECURITY_UNLOCK_SALT='unlock' DIAGNOSTIC_LOGS_DIR=/tmp/diagnostic_logs GEVENT_WORKER=0 OTEL_ENABLED=0 \ No newline at end of file diff --git a/server/mergin/.env b/server/mergin/.env index 0ff3dc42..ec45d5a1 100644 --- a/server/mergin/.env +++ b/server/mergin/.env @@ -4,5 +4,6 @@ SECRET_KEY='top-secret' SECURITY_BEARER_SALT='top-secret' SECURITY_EMAIL_SALT='top-secret' SECURITY_PASSWORD_SALT='top-secret' +SECURITY_UNLOCK_SALT='top-secret' MAIL_DEFAULT_SENDER='' FLASK_DEBUG=0 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/api.yaml b/server/mergin/auth/api.yaml index fa482a36..6fba1355 100644 --- a/server/mergin/auth/api.yaml +++ b/server/mergin/auth/api.yaml @@ -428,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 @@ -470,6 +466,26 @@ paths: $ref: "#/components/responses/Forbidden" "404": $ref: "#/components/responses/NotFoundResp" + /app/auth/unlock-account/{token}: + post: + summary: Unlock account + description: Clear an active lockout for the user encoded in the token + operationId: mergin.auth.controller.unlock_account + parameters: + - name: token + in: path + description: User token for account unlock verification + required: true + schema: + type: string + example: InRlc3RAbHV0cmFjb25zdWx0aW5nLmNvLnVrIg.YN2KRg.Vj1LSzSvQx9DcNnQFgZ0baS7LPU + responses: + "200": + description: OK + "400": + $ref: "#/components/responses/BadStatusResp" + "404": + $ref: "#/components/responses/NotFoundResp" /app/auth/confirm-email/{token}: post: summary: Email verified diff --git a/server/mergin/auth/app.py b/server/mergin/auth/app.py index acfccf43..09bab97e 100644 --- a/server/mergin/auth/app.py +++ b/server/mergin/auth/app.py @@ -3,15 +3,17 @@ # 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 +from flask import current_app, render_template, Flask from flask_login import current_user -from itsdangerous import URLSafeTimedSerializer +from itsdangerous import URLSafeTimedSerializer, BadData from sqlalchemy import func from .commands import add_commands from .config import Configuration -from .models import User +from .models import User, _check_dummy_password # signal for other versions to listen to user_account_closed = signal("user_account_closed") @@ -61,7 +63,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: @@ -87,13 +93,39 @@ 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 is None: + _check_dummy_password(password) + return None + if user.is_locked_out(): + 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: + user.locked_until = None + needs_commit = True + + if user.check_password(password): + if user.needs_rehash(): + user.assign_password(password) + needs_commit = True + if needs_commit: + db.session.commit() return user + else: + duration = user.record_failed_login() + db.session.commit() + if duration is not None: + send_account_locked_email(current_app, user, duration) + return None def generate_confirmation_token(app, email, salt): @@ -134,3 +166,61 @@ def send_confirmation_email(app, user, url, template, header, **kwargs): "sender": app.config["MAIL_DEFAULT_SENDER"], } send_email_async.delay(**email_data) + + +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"]) + payload = { + "email": user.email, + "locked_until": user.locked_until.replace(microsecond=0).isoformat(), + } + return serializer.dumps(payload, salt=app.config["SECURITY_UNLOCK_SALT"]) + + +def confirm_unlock_token(token: str, expiration: int = 24 * 3600) -> Optional[dict]: + serializer = URLSafeTimedSerializer(current_app.config["SECRET_KEY"]) + try: + payload = serializer.loads( + token, salt=current_app.config["SECURITY_UNLOCK_SALT"], max_age=expiration + ) + except BadData: + return None + return payload + + +def _format_lockout_duration(seconds: int) -> str: + """Humanize a lockout duration, e.g. 300 -> "5 minutes", 3600 -> "1 hour".""" + minutes, secs = divmod(int(seconds), 60) + hours, minutes = divmod(minutes, 60) + parts = [] + if hours: + parts.append(f"{hours} hour{'s' if hours != 1 else ''}") + if minutes: + parts.append(f"{minutes} minute{'s' if minutes != 1 else ''}") + if not parts: + parts.append(f"{secs} second{'s' if secs != 1 else ''}") + return " ".join(parts) + + +def send_account_locked_email(app: Flask, user: User, duration_seconds: int) -> None: + """Notify user their account was locked out and give them a link to unlock it.""" + from ..celery import send_email_async + + token = generate_unlock_token(app, user) + confirm_url = f"unlock-account/{token}" + html = render_template( + "email/account_locked.html", + subject="Account locked", + confirm_url=confirm_url, + user=user, + lockout_duration=_format_lockout_duration(duration_seconds), + locked_until=user.locked_until, + ) + email_data = { + "subject": "Account locked", + "html": html, + "recipients": [user.email], + "sender": app.config["MAIL_DEFAULT_SENDER"], + } + send_email_async.delay(**email_data) diff --git a/server/mergin/auth/config.py b/server/mergin/auth/config.py index 07b5a05d..24cbc50f 100644 --- a/server/mergin/auth/config.py +++ b/server/mergin/auth/config.py @@ -9,7 +9,13 @@ class Configuration(object): SECURITY_BEARER_SALT = config("SECURITY_BEARER_SALT") SECURITY_EMAIL_SALT = config("SECURITY_EMAIL_SALT") SECURITY_PASSWORD_SALT = config("SECURITY_PASSWORD_SALT") + SECURITY_UNLOCK_SALT = config("SECURITY_UNLOCK_SALT") BEARER_TOKEN_EXPIRATION = config( "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) + # Comma-separated "attempts:seconds" pairs, e.g. "5:300,10:3600" + LOCKOUT_POLICY = config("LOCKOUT_POLICY", default="5:300,10:3600") + # trailing window in seconds over which failed login attempts are counted + LOCKOUT_WINDOW = config("LOCKOUT_WINDOW", default=3600, cast=int) diff --git a/server/mergin/auth/controller.py b/server/mergin/auth/controller.py index 06859255..66c9b258 100644 --- a/server/mergin/auth/controller.py +++ b/server/mergin/auth/controller.py @@ -18,6 +18,7 @@ send_confirmation_email, confirm_token, generate_confirmation_token, + confirm_unlock_token, user_created, user_account_closed, edit_profile_enabled, @@ -42,6 +43,7 @@ EMAIL_CONFIRMATION_EXPIRATION = 12 * 3600 +ACCOUNT_UNLOCK_TOKEN_EXPIRATION = 24 * 3600 # public endpoints @@ -242,6 +244,7 @@ def admin_login(): # pylint: disable=W0613,W0612 if user: if user.active and user.is_admin: login_user(user) + LoginHistory.add_record(user.id, request) return "", 200 else: abort(403, "You do not have permissions") @@ -288,25 +291,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 @@ -324,6 +320,7 @@ def confirm_new_password(token): # pylint: disable=W0613,W0612 form = UserPasswordForm.from_json(request.json) if form.validate(): user.assign_password(form.password.data) + user.reset_lockout() db.session.add(user) db.session.commit() return "", 200 @@ -353,6 +350,25 @@ def confirm_email(token): # pylint: disable=W0613,W0612 return "", 200 +def unlock_account(token: str): # pylint: disable=W0613,W0612 + payload = confirm_unlock_token(token, expiration=ACCOUNT_UNLOCK_TOKEN_EXPIRATION) + if not payload: + abort(400, "Invalid or expired link") + + user = User.query.filter_by(email=payload["email"]).first_or_404() + stale = ( + not user.is_locked_out() + or user.locked_until.replace(microsecond=0).isoformat() + != payload["locked_until"] + ) + if stale: + abort(400, "This unlock link is no longer valid") + + user.reset_lockout() + db.session.commit() + return "", 200 + + @auth_required @edit_profile_enabled def update_user_profile(): # pylint: disable=W0613,W0612 diff --git a/server/mergin/auth/models.py b/server/mergin/auth/models.py index 760ab740..f80ccbe5 100644 --- a/server/mergin/auth/models.py +++ b/server/mergin/auth/models.py @@ -17,6 +17,21 @@ 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 = [] + 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 +48,7 @@ class User(db.Model): default=datetime.datetime.utcnow, ) last_signed_in = db.Column(db.DateTime(), nullable=True) + locked_until = db.Column(db.DateTime(), nullable=True) receive_notifications = db.Column( db.Boolean, default=True, nullable=False, index=True ) @@ -56,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")) @@ -64,12 +81,63 @@ 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 + + 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 + return self.locked_until > datetime.datetime.utcnow() + + def record_failed_login(self) -> Optional[int]: + """Record a failed login attempt and apply a lockout if a threshold is crossed, + counting only failed attempts within the trailing LOCKOUT_WINDOW and + since the last successful login (whichever bound is more recent). + + Returns the lockout duration in seconds if a new lock was just applied, else None. + """ + LoginHistory.add_record(self.id, request, successful=False) + window = current_app.config.get("LOCKOUT_WINDOW", 3600) + since = datetime.datetime.utcnow() - datetime.timedelta(seconds=window) + if self.last_signed_in and self.last_signed_in > since: + since = self.last_signed_in + recent_failures = LoginHistory.count_recent_failures(self.id, since) + + policy = _parse_lockout_policy( + current_app.config.get("LOCKOUT_POLICY", "5:300,10:3600") + ) + # only trigger on landing exactly on a tier's threshold + duration = None + for threshold, seconds in policy: + if recent_failures == threshold: + duration = seconds + if duration is not None: + self.locked_until = datetime.datetime.utcnow() + datetime.timedelta( + seconds=duration + ) + return duration + + def reset_lockout(self) -> None: + """Clear lockout state after a successful login.""" + self.locked_until = None + @property def is_authenticated(self): """For Flask-Login""" @@ -185,6 +253,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 @@ -274,29 +343,54 @@ class LoginHistory(db.Model): ip_address = db.Column(db.String, index=True) ip_geolocation_country = db.Column(db.String, index=True) device_id = db.Column(db.String, index=True, nullable=True) + successful = db.Column(db.Boolean, nullable=False, server_default="true") + + __table_args__ = ( + db.Index( + "ix_login_history_user_id_successful_timestamp", + "user_id", + "successful", + "timestamp", + ), + ) - def __init__(self, user_id: int, ua: str, ip: str, device_id: Optional[str] = None): + def __init__( + self, + user_id: int, + ua: str, + ip: str, + device_id: Optional[str] = None, + successful: bool = True, + ): self.user_id = user_id self.user_agent = ua self.ip_address = ip self.device_id = device_id + self.successful = successful self.timestamp = datetime.datetime.now(tz=datetime.timezone.utc) @staticmethod - def add_record(user_id: int, req: request) -> None: + def add_record(user_id: int, req: request, successful: bool = True) -> None: ua = get_user_agent(req) ip = get_ip(req) device_id = get_device_id(req) - # ignore login attempts coming from urllib - related to db sync tool - if "DB-sync" in ua: - return - lh = LoginHistory(user_id, ua, ip, device_id) + lh = LoginHistory(user_id, ua, ip, device_id, successful=successful) db.session.add(lh) - # cache user last login - User.query.filter_by(id=user_id).update({"last_signed_in": lh.timestamp}) + if successful: + # cache user last login + User.query.filter_by(id=user_id).update({"last_signed_in": lh.timestamp}) db.session.commit() + @staticmethod + def count_recent_failures(user_id: int, since: datetime.datetime) -> int: + """Count failed login attempts for a user since the given timestamp.""" + return LoginHistory.query.filter( + LoginHistory.user_id == user_id, + LoginHistory.successful.is_(False), + LoginHistory.timestamp >= since, + ).count() + @staticmethod def get_users_last_signed_in(user_ids: list) -> dict: """Get users last signed in dates. @@ -307,7 +401,10 @@ def get_users_last_signed_in(user_ids: list) -> dict: LoginHistory.user_id, func.max(LoginHistory.timestamp).label("last_signed_in"), ) - .filter(LoginHistory.user_id.in_(user_ids)) + .filter( + LoginHistory.user_id.in_(user_ids), + LoginHistory.successful.is_(True), + ) .group_by(LoginHistory.user_id) .all() ) diff --git a/server/mergin/sync/config.py b/server/mergin/sync/config.py index 8a5081ec..a5c8167a 100644 --- a/server/mergin/sync/config.py +++ b/server/mergin/sync/config.py @@ -82,5 +82,9 @@ class Configuration(object): ) # files that should be ignored during extension and MIME type checks UPLOAD_FILES_WHITELIST = config("UPLOAD_FILES_WHITELIST", default="", cast=Csv()) + # extra extensions to permit beyond the default block-list + UPLOAD_EXTENSIONS_WHITELIST = config( + "UPLOAD_EXTENSIONS_WHITELIST", default="", cast=Csv() + ) # max batch size for fetch projects in batch endpoint MAX_BATCH_SIZE = config("MAX_BATCH_SIZE", default=100, cast=int) diff --git a/server/mergin/sync/utils.py b/server/mergin/sync/utils.py index 48966457..6dd7abe1 100644 --- a/server/mergin/sync/utils.py +++ b/server/mergin/sync/utils.py @@ -463,7 +463,10 @@ def check_skip_validation(file_path: str) -> bool: Some files are allowed even if they have forbidden extension or mime type. """ file_name = os.path.basename(file_path) - return file_name in Configuration.UPLOAD_FILES_WHITELIST + if file_name in Configuration.UPLOAD_FILES_WHITELIST: + return True + ext = os.path.splitext(file_path)[1].lower() + return ext in {e.lower() for e in Configuration.UPLOAD_EXTENSIONS_WHITELIST} FORBIDDEN_MIME_TYPES = { diff --git a/server/mergin/templates/email/account_locked.html b/server/mergin/templates/email/account_locked.html new file mode 100644 index 00000000..e94efc87 --- /dev/null +++ b/server/mergin/templates/email/account_locked.html @@ -0,0 +1,15 @@ + + +{% set base_url = config['MERGIN_BASE_URL'] %} +{% extends "email/components/content.html" %} +{% block html %} +

Dear {{ user.username }},


+

Your account has been temporarily locked for {{ lockout_duration }} after several failed login attempts. If this wasn't you, someone may be trying to access your account - consider changing your password once you're back in.

+

You will be able to log in again at {{ locked_until.strftime('%Y-%m-%d %H:%M') }} UTC, or you can unlock your account right now by following this link:

+

{{ base_url }}/{{ confirm_url }}

+{% endblock %} +{% block notifications_footer %}{% endblock %} diff --git a/server/mergin/templates/email/components/base.html b/server/mergin/templates/email/components/base.html index ec1d066c..4fc8d222 100644 --- a/server/mergin/templates/email/components/base.html +++ b/server/mergin/templates/email/components/base.html @@ -221,6 +221,7 @@

+ {% block notifications_footer %}
@@ -230,6 +231,7 @@

+ {% endblock %} diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index ba7730c3..69a2e4fd 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -3,6 +3,7 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial from datetime import datetime, timedelta, timezone +from types import SimpleNamespace import time import itsdangerous import pytest @@ -13,7 +14,11 @@ from ..auth.bearer import decode_token, encode_token from ..auth.forms import ResetPasswordForm -from ..auth.app import generate_confirmation_token, confirm_token +from ..auth.app import ( + generate_confirmation_token, + confirm_token, + generate_unlock_token, +) from ..auth.models import User, LoginHistory from ..auth.tasks import anonymize_removed_users from ..app import db @@ -94,6 +99,380 @@ def test_logout(client): assert resp.status_code == 200 +@patch("mergin.celery.send_email_async.apply_async") +def test_login_lockout(send_email_mock, client): + """Test account lockout: progressive tiers, freeze during lock. + + policy: 3 failures → 60s lock, 4 failures → 3600s lock, counted over a + trailing window (LOCKOUT_WINDOW, default 1h) via LoginHistory, bounded + by the last successful login. + """ + 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 == 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 + for _ in range(3): + resp = client.post( + url_for("/.mergin_auth_controller_login"), + 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 + + 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 == 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 masked 401s) + assert send_email_mock.call_count == 1 + + # tier 2 escalation: one more failure after tier-1 expiry + # window count was at 3; one new failure pushes it to 4, crossing tier-2 threshold + + # expire_lock + user.locked_until = datetime.utcnow() - 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.utcnow() + timedelta(seconds=60) + assert LoginHistory.count_recent_failures(user.id, since) == 4 + + # second lockout email dispatched for the tier-2 re-lock + assert send_email_mock.call_count == 2 + + # successful login after expiry unlocks the account and gives a + # clean slate - the 4 prior failures stay in the audit trail... + user.locked_until = datetime.utcnow() - 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.locked_until is None + assert LoginHistory.count_recent_failures(user.id, since) == 4 + + # no email on successful login + assert send_email_mock.call_count == 2 + + # ...but no longer count toward a new lock: one wrong attempt right + # after a successful login must not immediately relock the account + resp = client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "lockoutuser", "password": "wrong"}, + ) + assert resp.status_code == 401 + assert not user.is_locked_out() + assert send_email_mock.call_count == 2 + + +@patch("mergin.celery.send_email_async.apply_async") +def test_unlock_account(send_email_mock, client, app): + """Test the self-service unlock-account link: valid use, reuse, natural + expiry, and cross-tier reuse, per the token-binding design. + + Each scenario below uses its own user so failure counts (sourced from + LoginHistory over a trailing window) don't bleed between scenarios + within the same test. + """ + + def unlock(token): + return client.post( + url_for("/.mergin_auth_controller_unlock_account", token=token) + ) + + def lock_out(username): + for _ in range(3): + client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": username, "password": "wrong"}, + ) + + with patch.dict(client.application.config, {"LOCKOUT_POLICY": "3:60,4:3600"}): + # unknown user -> 404 + fake_user = SimpleNamespace(email="nope@x.com", locked_until=datetime.utcnow()) + resp = unlock(generate_unlock_token(app, fake_user)) + assert resp.status_code == 404 + + # tamper with a valid-looking token -> 400 + resp = unlock("not-a-real-token") + assert resp.status_code == 400 + + # trigger tier-1 lock and capture its token + user = add_user("unlockuser", "correctpassword") + lock_out("unlockuser") + assert user.is_locked_out() + tier1_token = generate_unlock_token(app, user) + + # valid token unlocks successfully + resp = unlock(tier1_token) + assert resp.status_code == 200 + assert user.locked_until is None + + # reuse of the same (now-consumed) token fails + resp = unlock(tier1_token) + assert resp.status_code == 400 + + # naturally-expired lock: token itself still cryptographically valid, + # but the lock episode it points to is no longer active + stale_user = add_user("staleuser", "correctpassword") + lock_out("staleuser") + assert stale_user.is_locked_out() + stale_token = generate_unlock_token(app, stale_user) + stale_user.locked_until = datetime.utcnow() - timedelta(seconds=1) + db.session.commit() + resp = unlock(stale_token) + assert resp.status_code == 400 + + # cross-tier reuse: a token minted for one lock episode must not unlock + # a later, different lock episode for the same user + cross_user = add_user("crossuser", "correctpassword") + lock_out("crossuser") + tier1_token_2 = generate_unlock_token(app, cross_user) + # escalate to tier 2 with a new locked_until + cross_user.locked_until = datetime.utcnow() - timedelta(seconds=1) + db.session.commit() + client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "crossuser", "password": "wrong"}, + ) + assert cross_user.locked_until > datetime.utcnow() + timedelta(seconds=60) + resp = unlock(tier1_token_2) + assert resp.status_code == 400 + + +def test_login_lockout_window_expiry(client): + """Failures older than LOCKOUT_WINDOW no longer count toward the threshold.""" + user = add_user("windowuser", "correctpassword") + + with patch.dict( + client.application.config, + {"LOCKOUT_POLICY": "3:60,4:3600", "LOCKOUT_WINDOW": 3600}, + ): + for _ in range(3): + resp = client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "windowuser", "password": "wrong"}, + ) + assert resp.status_code == 401 + assert user.is_locked_out() + + # push all recorded failures outside the 1h window, and let the lock expire + LoginHistory.query.filter_by(user_id=user.id, successful=False).update( + {"timestamp": datetime.utcnow() - timedelta(hours=2)} + ) + user.locked_until = datetime.utcnow() - timedelta(seconds=1) + db.session.commit() + + # old failures no longer count - a single new failure should not relock + resp = client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "windowuser", "password": "wrong"}, + ) + assert resp.status_code == 401 + assert not user.is_locked_out() + assert user.locked_until is None + assert ( + LoginHistory.count_recent_failures( + user.id, datetime.utcnow() - timedelta(hours=1) + ) + == 1 + ) + + +def test_login_lockout_only_at_threshold(client): + """With a gap between tiers, counts strictly between two thresholds + must not trigger (or re-trigger) a lock - only landing exactly on a + threshold does.""" + user = add_user("borderuser", "correctpassword") + + def attempt(): + return client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "borderuser", "password": "wrong"}, + ) + + with patch.dict(client.application.config, {"LOCKOUT_POLICY": "5:300,10:3600"}): + # 1-4: nothing happens + for _ in range(4): + attempt() + assert not user.is_locked_out() + + # 5: tier-1 lock + attempt() + assert user.is_locked_out() + assert user.locked_until < datetime.utcnow() + timedelta(seconds=301) + + # simulate the tier-1 lock having expired, then 6-9: nothing happens, + # even though the count is still above the tier-1 threshold + for expected_count in range(6, 10): + user.locked_until = datetime.utcnow() - timedelta(seconds=1) + db.session.commit() + attempt() + assert not user.is_locked_out() + assert ( + LoginHistory.count_recent_failures( + user.id, datetime.utcnow() - timedelta(hours=1) + ) + == expected_count + ) + + # 10: tier-2 lock + user.locked_until = datetime.utcnow() - timedelta(seconds=1) + db.session.commit() + attempt() + assert user.is_locked_out() + assert user.locked_until > datetime.utcnow() + timedelta(seconds=3000) + + +def test_login_history_records_failures(client): + """Failed attempts are recorded (successful=False) without touching + last_signed_in; successful logins are recorded as successful=True and + do update last_signed_in.""" + user = add_user("historyuser", "correctpassword") + assert user.last_signed_in is None + + resp = client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "historyuser", "password": "wrong"}, + ) + assert resp.status_code == 401 + failed = LoginHistory.query.filter_by(user_id=user.id, successful=False).all() + assert len(failed) == 1 + assert user.last_signed_in is None + + resp = client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "historyuser", "password": "correctpassword"}, + ) + assert resp.status_code == 200 + successful = LoginHistory.query.filter_by(user_id=user.id, successful=True).all() + assert len(successful) == 1 + assert user.last_signed_in is not None + last_signed_in = user.last_signed_in + + # a later failed attempt must not be reported/cached as the last signed-in + # time, even though it's the most recent row for this user overall + resp = client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "historyuser", "password": "wrong"}, + ) + assert resp.status_code == 401 + user.last_signed_in = None + db.session.commit() + users_last_signed_in = LoginHistory.get_users_last_signed_in([user.id]) + assert users_last_signed_in[user.id] == last_signed_in + 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 + 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") + 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 @@ -255,12 +634,56 @@ def test_confirm_password(app, client): assert resp.status_code == 400 -# reset password tests: success, no email, not-existing user +@patch("mergin.celery.send_email_async.apply_async") +def test_confirm_password_clears_lockout(send_email_mock, app, client): + """Resetting a password via the emailed link clears an active lock, + same as the unlock-account link.""" + user = add_user("resetlockuser", "correctpassword") + + with patch.dict(client.application.config, {"LOCKOUT_POLICY": "3:60,4:3600"}): + for _ in range(3): + client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "resetlockuser", "password": "wrong"}, + ) + assert user.is_locked_out() + + token = generate_confirmation_token( + app, user.email, app.config["SECURITY_PASSWORD_SALT"] + ) + resp = client.post( + url_for("/.mergin_auth_controller_confirm_new_password", token=token), + data=json.dumps({"password": "newpass#1", "confirm": "newpass#1"}), + headers=json_headers, + ) + assert resp.status_code == 200 + assert user.locked_until is None + + # failure history is untouched - one more wrong attempt re-locks + resp = client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "resetlockuser", "password": "wrong"}, + ) + assert resp.status_code == 401 + assert user.is_locked_out() + + # but logging in with the new password succeeds and gives a clean slate + user.locked_until = None + db.session.commit() + resp = client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "resetlockuser", "password": "newpass#1"}, + ) + assert resp.status_code == 200 + assert not user.is_locked_out() + + +# 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), ] @@ -274,6 +697,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" @@ -393,6 +836,8 @@ def test_api_login(client, data, headers, expected): def test_api_login_from_urllib(client): + """DB-sync logins are recorded in LoginHistory just like any other client, + to keep a full picture of login activity (including for lockout purposes).""" with patch("mergin.auth.models.get_user_agent") as mock: mock.return_value = "DB-sync/0.1" resp = client.post( @@ -407,9 +852,9 @@ def test_api_login_from_urllib(client): .order_by(desc(LoginHistory.timestamp)) .first() ) - assert not login_history - # we do not have recored last login yet - assert user.last_signed_in is None + assert login_history + assert login_history.successful + assert user.last_signed_in == login_history.timestamp def test_api_user_profile(client): @@ -469,7 +914,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): diff --git a/server/mergin/tests/test_utils.py b/server/mergin/tests/test_utils.py index 1f447875..288577b0 100644 --- a/server/mergin/tests/test_utils.py +++ b/server/mergin/tests/test_utils.py @@ -402,3 +402,31 @@ def test_mime_type_validation_skip(): # Should be forbidden assert not is_supported_type("other.js") + + +def test_allowed_extensions_override(): + """Extensions in UPLOAD_EXTENSIONS_WHITELIST are accepted even though they are in FORBIDDEN_EXTENSIONS.""" + with patch( + "mergin.sync.utils.Configuration.UPLOAD_EXTENSIONS_WHITELIST", [".py", ".sh"] + ): + # forbidden by default, now explicitly allowed + assert is_supported_extension("model.py") + assert is_supported_extension("scripts/deploy.sh") + # match is case-insensitive + assert is_supported_extension("MODEL.PY") + # extensions not in the override stay blocked + assert not is_supported_extension("malware.exe") + assert not is_supported_extension("app.js") + + +def test_extension_whitelist_skips_mime_check(): + """A whitelisted extension also bypasses the MIME check via check_skip_validation.""" + with patch("mergin.sync.utils.get_mimetype", return_value="text/x-shellscript"): + # blocked when the extension is not whitelisted + with patch("mergin.sync.utils.Configuration.UPLOAD_EXTENSIONS_WHITELIST", []): + assert not is_supported_type("deploy.sh") + # allowed once the extension is whitelisted + with patch( + "mergin.sync.utils.Configuration.UPLOAD_EXTENSIONS_WHITELIST", [".sh"] + ): + assert is_supported_type("deploy.sh") diff --git a/server/migrations/community/7a095270b252_login_history_index_concurrently.py b/server/migrations/community/7a095270b252_login_history_index_concurrently.py new file mode 100644 index 00000000..38b6e35f --- /dev/null +++ b/server/migrations/community/7a095270b252_login_history_index_concurrently.py @@ -0,0 +1,41 @@ +"""Create login_history user/successful/timestamp index concurrently + +CONCURRENTLY avoids blocking writes to login_history during the build +(every login attempt writes here), at the cost of running outside the +migration's transaction (autocommit_block) and leaving an INVALID index +behind on failure - upgrade()/downgrade() self-heal by dropping that first. + +Revision ID: 7a095270b252 +Revises: a3c8f2e1d947 +Create Date: 2026-07-24 00:00:00.000000 + +""" + +from alembic import op + + +# revision identifiers, used by Alembic. +revision = "7a095270b252" +down_revision = "a3c8f2e1d947" +branch_labels = None +depends_on = None + + +def upgrade(): + with op.get_context().autocommit_block(): + op.execute( + "DROP INDEX CONCURRENTLY IF EXISTS ix_login_history_user_id_successful_timestamp" + ) + op.create_index( + "ix_login_history_user_id_successful_timestamp", + "login_history", + ["user_id", "successful", "timestamp"], + postgresql_concurrently=True, + ) + + +def downgrade(): + with op.get_context().autocommit_block(): + op.execute( + "DROP INDEX CONCURRENTLY IF EXISTS ix_login_history_user_id_successful_timestamp" + ) 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..c754085d --- /dev/null +++ b/server/migrations/community/a3c8f2e1d947_add_login_lockout_fields.py @@ -0,0 +1,42 @@ +"""Add locked_until to user table and successful flag to login_history + +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 = "f1d9e4a7b823" +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column( + "user", + sa.Column( + "locked_until", + sa.DateTime(), + nullable=True, + ), + ) + op.add_column( + "login_history", + sa.Column( + "successful", + sa.Boolean(), + nullable=False, + server_default="true", + ), + ) + + +def downgrade(): + op.drop_column("login_history", "successful") + op.drop_column("user", "locked_until") diff --git a/web-app/packages/app/src/router.ts b/web-app/packages/app/src/router.ts index 4fb00555..04888ba0 100644 --- a/web-app/packages/app/src/router.ts +++ b/web-app/packages/app/src/router.ts @@ -3,6 +3,7 @@ // SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial import { + AccountUnlockView, ChangePasswordView, FileBrowserView, FileVersionDetailView, @@ -81,6 +82,13 @@ export const createRouter = (pinia: Pinia) => { props: true, meta: { public: true } }, + { + path: '/unlock-account/:token', + name: UserRouteName.UnlockAccount, + component: AccountUnlockView, + props: true, + meta: { public: true } + }, { path: '/dashboard', name: DashboardRouteName.Dashboard, diff --git a/web-app/packages/lib/src/assets/sass/themes/mm-theme-light/_extensions.scss b/web-app/packages/lib/src/assets/sass/themes/mm-theme-light/_extensions.scss index 5b88226b..b873a944 100644 --- a/web-app/packages/lib/src/assets/sass/themes/mm-theme-light/_extensions.scss +++ b/web-app/packages/lib/src/assets/sass/themes/mm-theme-light/_extensions.scss @@ -147,6 +147,38 @@ img { } } +.p-button.p-button-outlined { + &.p-button-warning { + border: 2px solid map-get($colors, 'earth'); + color: map-get($colors, 'earth'); + + &:hover { + background-color: map-get($colors, 'warning'); + border: 2px solid map-get($colors, 'warning'); + } + } + + &.p-button-danger { + border: 2px solid map-get($colors, 'grape'); + color: map-get($colors, 'grape'); + + &:hover { + background-color: map-get($colors, 'negative'); + border: 2px solid map-get($colors, 'negative'); + } + } + + &.p-button-primary { + border: 2px solid $primaryDarkColor; + color: $primaryDarkColor; + + &:hover { + background-color: $primaryColor; + border: 2px solid $primaryColor; + } + } +} + // Color of error messages in inputs ... .p-error { color: map-get($map: $colors, $key: grape); diff --git a/web-app/packages/lib/src/modules/dialog/components/ConfirmDialog.vue b/web-app/packages/lib/src/modules/dialog/components/ConfirmDialog.vue index 087af8d8..3b26b612 100644 --- a/web-app/packages/lib/src/modules/dialog/components/ConfirmDialog.vue +++ b/web-app/packages/lib/src/modules/dialog/components/ConfirmDialog.vue @@ -9,7 +9,9 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial Cover for confirm dialog {{ text }} {{ description }} - {{ hint }} + {{ + hint + }}
{{ cancelText }} + :label="cancelText" + /> {{ confirmText }} @@ -59,7 +63,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial + + diff --git a/web-app/packages/lib/src/modules/user/views/index.ts b/web-app/packages/lib/src/modules/user/views/index.ts index 44df67bf..2498d990 100644 --- a/web-app/packages/lib/src/modules/user/views/index.ts +++ b/web-app/packages/lib/src/modules/user/views/index.ts @@ -2,6 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial +export { default as AccountUnlockView } from './AccountUnlockView.vue' export { default as ChangePasswordView } from './ChangePasswordView.vue' export { default as LoginViewTemplate } from './LoginViewTemplate.vue' export { default as ProfileViewTemplate } from './ProfileViewTemplate.vue'