diff --git a/deployment/community/.env.template b/deployment/community/.env.template index 95497b68..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 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/auth/api.yaml b/server/mergin/auth/api.yaml index a4c7b637..fdcba129 100644 --- a/server/mergin/auth/api.yaml +++ b/server/mergin/auth/api.yaml @@ -472,6 +472,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 00575a5f..34c7b099 100644 --- a/server/mergin/auth/app.py +++ b/server/mergin/auth/app.py @@ -3,10 +3,11 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial import functools +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 @@ -120,8 +121,10 @@ def authenticate(login, password): db.session.commit() return user else: - user.record_failed_login() + duration = user.record_failed_login() db.session.commit() + if duration is not None: + send_account_locked_email(current_app, user, duration) return None @@ -163,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 3d2215ee..0e9c81ca 100644 --- a/server/mergin/auth/config.py +++ b/server/mergin/auth/config.py @@ -9,6 +9,7 @@ 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 diff --git a/server/mergin/auth/controller.py b/server/mergin/auth/controller.py index 474696cc..56e99cd8 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, @@ -43,6 +44,7 @@ EMAIL_CONFIRMATION_EXPIRATION = 12 * 3600 +ACCOUNT_UNLOCK_TOKEN_EXPIRATION = 24 * 3600 # public endpoints @@ -363,6 +365,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 e8cfbd90..e3f2ae90 100644 --- a/server/mergin/auth/models.py +++ b/server/mergin/auth/models.py @@ -102,8 +102,11 @@ def is_locked_out(self) -> bool: return False return self.locked_until > datetime.datetime.utcnow() - def record_failed_login(self) -> None: - """Increment the failed-login counter and apply a lockout if a threshold is crossed.""" + def record_failed_login(self) -> Optional[int]: + """Increment the failed-login counter and apply a lockout if a threshold is crossed. + + Returns the lockout duration in seconds if a new lock was just applied, else None. + """ 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") @@ -117,6 +120,7 @@ def record_failed_login(self) -> 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.""" 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 3554152b..a4220eea 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,14 +99,14 @@ def test_logout(client): assert resp.status_code == 200 -def test_login_lockout(client): +@patch("mergin.celery.send_email_async.apply_async") +def test_login_lockout(send_email_mock, 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(): @@ -112,54 +117,140 @@ def assert_locked(): assert resp.status_code == 423 assert resp.json["code"] == "AccountLocked" - # tier 1: 3 failures → 60s lock - for _ in range(3): + 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 + + # 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 == 423 + + # counter stays frozen during lockout + assert user.failed_login_attempts == 3 + assert user.locked_until is not None + + # no further emails while already locked out (attempts above were all 423s) + assert send_email_mock.call_count == 1 + + # 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.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 user.failed_login_attempts == 4 - assert_locked() + # second lockout email dispatched for the tier-2 re-lock + assert send_email_mock.call_count == 2 - # 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 + # successful login after expiry resets everything + 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.failed_login_attempts == 0 + assert user.locked_until is None - # counter stays frozen during lockout - assert user.failed_login_attempts == 3 - assert user.locked_until is not None + # no email on successful login + assert send_email_mock.call_count == 2 - # 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.utcnow() - timedelta(seconds=1) - db.session.commit() +@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.""" + user = add_user("unlockuser", "correctpassword") + + def unlock(token): + return client.post( + url_for("/.mergin_auth_controller_unlock_account", token=token) + ) - 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 user.failed_login_attempts == 4 + def lock_out(): + for _ in range(3): + client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "unlockuser", "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 + lock_out() + 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.failed_login_attempts == 0 + 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 + lock_out() + assert user.is_locked_out() + stale_token = generate_unlock_token(app, user) + user.locked_until = datetime.utcnow() - timedelta(seconds=1) + db.session.commit() + resp = unlock(stale_token) + assert resp.status_code == 400 - # successful login after expiry resets everything - 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.failed_login_attempts == 0 - assert user.locked_until is None + # cross-tier reuse: a token minted for one lock episode must not unlock + # a later, different lock episode for the same user + user.locked_until = None + user.failed_login_attempts = 0 + db.session.commit() + lock_out() + tier1_token_2 = generate_unlock_token(app, user) + # escalate to tier 2 with a new locked_until + user.locked_until = datetime.utcnow() - timedelta(seconds=1) + db.session.commit() + client.post( + url_for("/.mergin_auth_controller_login"), + json={"login": "unlockuser", "password": "wrong"}, + ) + assert user.failed_login_attempts == 4 + assert user.locked_until > datetime.utcnow() + timedelta(seconds=60) + resp = unlock(tier1_token_2) + assert resp.status_code == 400 def test_bcrypt_lazy_rehash(app): 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/modules/user/routes.ts b/web-app/packages/lib/src/modules/user/routes.ts index d2a52edd..d31a7062 100644 --- a/web-app/packages/lib/src/modules/user/routes.ts +++ b/web-app/packages/lib/src/modules/user/routes.ts @@ -16,6 +16,7 @@ export enum UserRouteName { Login = 'login', ConfirmEmail = 'confirm_email', ChangePassword = 'change_password', + UnlockAccount = 'unlock_account', UserProfile = 'user_profile' } @@ -29,6 +30,7 @@ export const getUserTitle = (route: RouteLocationNormalizedLoaded) => { ], [UserRouteName.ConfirmEmail]: ['Confirm email address', DEFAULT_PAGE_TITLE], [UserRouteName.ChangePassword]: ['Change password', DEFAULT_PAGE_TITLE], + [UserRouteName.UnlockAccount]: ['Unlock your account', DEFAULT_PAGE_TITLE], [UserRouteName.UserProfile]: ['Your profile'] } return titles[name] diff --git a/web-app/packages/lib/src/modules/user/userApi.ts b/web-app/packages/lib/src/modules/user/userApi.ts index 1ecab091..bd9cb169 100644 --- a/web-app/packages/lib/src/modules/user/userApi.ts +++ b/web-app/packages/lib/src/modules/user/userApi.ts @@ -72,6 +72,10 @@ export const UserApi = { return UserModule.httpService.get('/app/auth/resend-confirm-email') }, + unlockAccount: (token: string): Promise> => { + return UserModule.httpService.post(`/app/auth/unlock-account/${token}`) + }, + login: (data: LoginData): Promise> => UserModule.httpService.post('/app/auth/login', data), diff --git a/web-app/packages/lib/src/modules/user/views/AccountUnlockView.vue b/web-app/packages/lib/src/modules/user/views/AccountUnlockView.vue new file mode 100644 index 00000000..05bec944 --- /dev/null +++ b/web-app/packages/lib/src/modules/user/views/AccountUnlockView.vue @@ -0,0 +1,57 @@ + + + + + + + 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'