From d1617b1e539b4bfda475b531e34ef1fa6996a802 Mon Sep 17 00:00:00 2001 From: Tim Bradgate Date: Thu, 22 Oct 2026 18:32:25 +0100 Subject: [PATCH 1/4] Add auth audit logging and digiscript_auth_events_total metric Log login, logout, password change/reset, token refresh and API token generate/revoke on DigiScript.Audit with sanitised usernames, never including secrets, and count them in a bounded Prometheus counter. Co-Authored-By: Claude Sonnet 5.5 --- docs/pages/user_config.md | 9 + server/controllers/api/v1/auth/token.py | 23 +++ server/controllers/api/v1/auth/user.py | 77 ++++++++ .../test/controllers/api/v1/auth/test_auth.py | 179 ++++++++++++++++++ .../controllers/api/v1/auth/test_token.py | 48 +++++ server/test/utils/web/test_auth_audit.py | 53 ++++++ server/utils/web/auth_audit.py | 115 +++++++++++ 7 files changed, 504 insertions(+) create mode 100644 server/test/utils/web/test_auth_audit.py create mode 100644 server/utils/web/auth_audit.py diff --git a/docs/pages/user_config.md b/docs/pages/user_config.md index 612d6018..cd0fd3df 100644 --- a/docs/pages/user_config.md +++ b/docs/pages/user_config.md @@ -100,6 +100,15 @@ All log entries are attributed to the logged-in user where one is present: - **Client log limits**: Submitting client logs requires being logged in, so entries generated before login are held in the browser and sent after login. A single batch may contain at most 100 entries, messages are truncated at 4 KB, oversized extra data is dropped, and each IP address is limited to 600 entries per minute (further entries are rejected until the window resets). - **Server file paths**: The database, log, compiled script and draft script locations are only shown to administrators (on the Settings tab); other users and logged-out visitors never receive them. +#### Authentication Audit Log + +Security-relevant authentication actions are recorded as `AUDIT` lines (logger `DigiScript.Audit`) at the default `INFO` log level, independent of the **Enable Log Redaction** setting. Successes are logged at `INFO` and failures at `WARNING`. The covered events are login, logout, password change, admin password reset, token refresh, and API token generation and revocation. For example: + +- `AUDIT login failure username="alice" ip=203.0.113.5 reason=bad_password` +- `AUDIT password_reset success username="admin" user_id=1 ip=203.0.113.5 target_user_id=4` (the username is the administrator who performed the reset) + +A failed login records whether the username was unknown or the password was wrong, but the response sent to the client is identical in both cases. Passwords, tokens and API keys are never written to the log. Usernames typed by a client are stripped of control characters and truncated to 64 characters. The same events are counted in the `digiscript_auth_events_total{event,result}` Prometheus metric, which is useful for alerting on bursts of failed logins. + #### Filtering Use the **Username** filter field to show only entries from a specific user. This filter applies to both Server and Client sources. Combined with the **Level** and **Search** filters, you can quickly isolate activity from a particular user across the full log stream. diff --git a/server/controllers/api/v1/auth/token.py b/server/controllers/api/v1/auth/token.py index e0ec1ca4..05dc8b05 100644 --- a/server/controllers/api/v1/auth/token.py +++ b/server/controllers/api/v1/auth/token.py @@ -1,5 +1,6 @@ from models.user import User from utils.web.api_keys import ApiKeyService +from utils.web.auth_audit import audit from utils.web.base_controller import BaseAPIController from utils.web.route import ApiRoute, ApiVersion from utils.web.web_decorators import ( @@ -24,6 +25,13 @@ async def post(self): session.commit() self.application.jwt_service.api_keys.invalidate() + audit( + "api_token_generate", + "success", + username=self.current_user.get("username"), + user_id=user.id, + remote_ip=self.request.remote_ip, + ) self.set_status(200) await self.finish( { @@ -46,6 +54,14 @@ async def post(self): return if not user.api_token: + audit( + "api_token_revoke", + "failure", + username=self.current_user.get("username"), + user_id=user.id, + remote_ip=self.request.remote_ip, + detail="reason=no_token", + ) self.set_status(400) await self.finish({"message": "No API token to revoke"}) return @@ -54,6 +70,13 @@ async def post(self): session.commit() self.application.jwt_service.api_keys.invalidate() + audit( + "api_token_revoke", + "success", + username=self.current_user.get("username"), + user_id=user.id, + remote_ip=self.request.remote_ip, + ) self.set_status(200) await self.finish({"message": "API token revoked successfully"}) diff --git a/server/controllers/api/v1/auth/user.py b/server/controllers/api/v1/auth/user.py index bf7d6451..cdcbc3ed 100644 --- a/server/controllers/api/v1/auth/user.py +++ b/server/controllers/api/v1/auth/user.py @@ -8,6 +8,7 @@ from registry.named_locks import NamedLockRegistry from schemas.schemas import UserSchema from services.password_service import PasswordService +from utils.web.auth_audit import audit from utils.web.base_controller import BaseAPIController from utils.web.json_body import get_json_object from utils.web.route import ApiRoute, ApiVersion @@ -166,12 +167,20 @@ async def post(self): await self.finish({"message": "Password missing"}) return + ip = self.request.remote_ip with self.make_session() as session: async with NamedLockRegistry.acquire(f"UserLock::{username}"): user = session.scalars( select(User).where(User.username == username) ).first() if not user: + audit( + "login", + "failure", + username=username, + remote_ip=ip, + detail="reason=unknown_user", + ) self.set_status(401) await self.finish({"message": "Invalid username/password"}) return @@ -202,6 +211,13 @@ async def post(self): }, ) + audit( + "login", + "success", + username=user.username, + user_id=user.id, + remote_ip=ip, + ) self.set_status(200) await self.finish( { @@ -211,6 +227,14 @@ async def post(self): } ) else: + audit( + "login", + "failure", + username=username, + user_id=user.id, + remote_ip=ip, + detail="reason=bad_password", + ) self.set_status(401) await self.finish({"message": "Invalid username/password"}) @@ -277,6 +301,13 @@ async def post(self): # Logs its own failures, each saying how far it got. self._log_out_client(session_id) + audit( + "logout", + "success", + username=self.current_user.get("username"), + user_id=self.current_user["id"], + remote_ip=self.request.remote_ip, + ) self.set_status(200) await self.finish({"message": "Successfully logged out"}) else: @@ -295,6 +326,13 @@ async def post(self): }, ) + audit( + "token_refresh", + "success", + username=self.current_user.get("username"), + user_id=self.current_user["id"], + remote_ip=self.request.remote_ip, + ) self.set_status(200) await self.finish({"access_token": access_token, "token_type": "bearer"}) @@ -361,6 +399,14 @@ async def patch(self): old_password, user.password ) if not password_matches: + audit( + "password_change", + "failure", + username=user.username, + user_id=user.id, + remote_ip=self.request.remote_ip, + detail="reason=bad_old_password", + ) self.set_status(401) await self.finish({"message": "Current password is incorrect"}) return @@ -376,6 +422,14 @@ async def patch(self): force_logout_sessions=False, ) except ValueError as e: + audit( + "password_change", + "failure", + username=user.username, + user_id=user.id, + remote_ip=self.request.remote_ip, + detail="reason=invalid_new_password", + ) self.set_status(400) await self.finish({"message": str(e)}) return @@ -390,6 +444,13 @@ async def patch(self): user, new_token ) + audit( + "password_change", + "success", + username=user.username, + user_id=user.id, + remote_ip=self.request.remote_ip, + ) self.set_status(200) await self.finish( { @@ -452,10 +513,26 @@ async def post(self): requires_password_change=True, ) except ValueError as e: + audit( + "password_reset", + "failure", + username=self.current_user.get("username"), + user_id=self.current_user["id"], + remote_ip=self.request.remote_ip, + detail=f"target_user_id={target_user.id} reason=invalid_password", + ) self.set_status(400) await self.finish({"message": str(e)}) return + audit( + "password_reset", + "success", + username=self.current_user.get("username"), + user_id=self.current_user["id"], + remote_ip=self.request.remote_ip, + detail=f"target_user_id={target_user.id}", + ) self.set_status(200) await self.finish( { diff --git a/server/test/controllers/api/v1/auth/test_auth.py b/server/test/controllers/api/v1/auth/test_auth.py index 212c0c0b..bcc6b6e4 100644 --- a/server/test/controllers/api/v1/auth/test_auth.py +++ b/server/test/controllers/api/v1/auth/test_auth.py @@ -1,3 +1,4 @@ +from prometheus_client import REGISTRY from sqlalchemy import select from tornado import escape @@ -1299,3 +1300,181 @@ def test_change_password_invalid_json_returns_400(self): token = self._create_and_login_admin() resp = self._post("/api/v1/auth/change-password", "nope", token, "PATCH") self.assertEqual(400, resp.code) + + +class TestAuthAuditLogging(DigiScriptTestCase): + """Auth endpoints emit ``DigiScript.Audit`` lines and bump the counter.""" + + PASSWORD = "Sup3r-Secret-Pw-1" + NEW_PASSWORD = "N3w-Secret-Pw-2" + + def _count(self, event: str, result: str) -> float: + return ( + REGISTRY.get_sample_value( + "digiscript_auth_events_total", {"event": event, "result": result} + ) + or 0.0 + ) + + def _post(self, path, body=None, token=None, method="POST"): + headers = {"Authorization": f"Bearer {token}"} if token else {} + return self.fetch( + path, + method=method, + body=escape.json_encode(body or {}), + headers=headers, + ) + + def _create(self, username="alice", is_admin=True): + response = self._post( + "/api/v1/auth/create", + {"username": username, "password": self.PASSWORD, "is_admin": is_admin}, + ) + self.assertEqual(200, response.code) + + def _login(self, username="alice", password=None): + return self._post( + "/api/v1/auth/login", + {"username": username, "password": password or self.PASSWORD}, + ) + + def _token(self, username="alice"): + return escape.json_decode(self._login(username).body)["access_token"] + + def _assert_no_secrets(self, logs, *secrets): + output = "\n".join(logs.output) + for secret in secrets: + self.assertNotIn(secret, output) + + def test_login_success(self): + self._create() + before = self._count("login", "success") + with self.assertLogs("DigiScript", "INFO") as logs: + response = self._login() + self.assertEqual(200, response.code) + token = escape.json_decode(response.body)["access_token"] + audit = [r for r in logs.records if r.name == "DigiScript.Audit"] + self.assertEqual(1, len(audit)) + self.assertEqual("INFO", audit[0].levelname) + self.assertIn("AUDIT login success", audit[0].getMessage()) + self.assertIn('username="alice"', audit[0].getMessage()) + self.assertEqual("alice", audit[0].username) + self.assertIsNotNone(audit[0].user_id) + self.assertTrue(audit[0].remote_ip) + self._assert_no_secrets(logs, self.PASSWORD, token) + self.assertEqual(before + 1, self._count("login", "success")) + + def test_login_failure_reasons_differ_in_log_but_not_response(self): + self._create() + before = self._count("login", "failure") + with self.assertLogs("DigiScript.Audit", "WARNING") as bad_pw: + bad_password = self._login(password="wrong-Secret-Pw-9") + with self.assertLogs("DigiScript.Audit", "WARNING") as unknown: + unknown_user = self._login(username="mallory") + self.assertEqual(401, bad_password.code) + self.assertEqual(bad_password.code, unknown_user.code) + self.assertEqual(bad_password.body, unknown_user.body) + self.assertIn("reason=bad_password", bad_pw.output[0]) + self.assertIn("reason=unknown_user", unknown.output[0]) + self.assertEqual("WARNING", bad_pw.records[0].levelname) + self._assert_no_secrets(bad_pw, "wrong-Secret-Pw-9", self.PASSWORD) + self.assertEqual(before + 2, self._count("login", "failure")) + + def test_login_failure_sanitises_username(self): + hostile = 'bob" ip=6.6.6.6\r\nAUDIT login success\x1b[31m' + "x" * 200 + with self.assertLogs("DigiScript.Audit", "WARNING") as logs: + response = self._login(username=hostile) + self.assertEqual(401, response.code) + self.assertEqual(1, len(logs.output)) + message = logs.records[0].getMessage() + self.assertNotIn("\n", message) + self.assertNotIn("\r", message) + self.assertNotIn("\x1b", message) + self.assertLessEqual(len(logs.records[0].username), 64) + self.assertTrue(message.startswith("AUDIT login failure username=")) + + def test_logout(self): + self._create() + token = self._token() + before = self._count("logout", "success") + with self.assertLogs("DigiScript", "INFO") as logs: + response = self._post("/api/v1/auth/logout", token=token) + self.assertEqual(200, response.code) + self.assertTrue(any("AUDIT logout success" in o for o in logs.output)) + self._assert_no_secrets(logs, token) + self.assertEqual(before + 1, self._count("logout", "success")) + + def test_refresh_token(self): + self._create() + token = self._token() + before = self._count("token_refresh", "success") + with self.assertLogs("DigiScript", "INFO") as logs: + response = self._post("/api/v1/auth/refresh-token", token=token) + self.assertEqual(200, response.code) + new_token = escape.json_decode(response.body)["access_token"] + self.assertTrue(any("AUDIT token_refresh success" in o for o in logs.output)) + self._assert_no_secrets(logs, token, new_token) + self.assertEqual(before + 1, self._count("token_refresh", "success")) + + def test_change_password_success_and_failure(self): + self._create() + token = self._token() + ok_before = self._count("password_change", "success") + fail_before = self._count("password_change", "failure") + + with self.assertLogs("DigiScript", "INFO") as logs: + bad = self._post( + "/api/v1/auth/change-password", + {"old_password": "wrong-Secret-Pw-9", "new_password": "x-Pw-3"}, + token=token, + method="PATCH", + ) + self.assertEqual(401, bad.code) + self.assertTrue(any("reason=bad_old_password" in o for o in logs.output)) + self._assert_no_secrets(logs, "wrong-Secret-Pw-9", self.PASSWORD, "x-Pw-3") + + with self.assertLogs("DigiScript", "INFO") as logs: + good = self._post( + "/api/v1/auth/change-password", + {"old_password": self.PASSWORD, "new_password": self.NEW_PASSWORD}, + token=token, + method="PATCH", + ) + self.assertEqual(200, good.code) + new_token = escape.json_decode(good.body)["access_token"] + self.assertTrue(any("AUDIT password_change success" in o for o in logs.output)) + self._assert_no_secrets(logs, self.PASSWORD, self.NEW_PASSWORD, new_token) + self.assertEqual(ok_before + 1, self._count("password_change", "success")) + self.assertEqual(fail_before + 1, self._count("password_change", "failure")) + + def test_change_password_weak_new_password_is_audited(self): + self._create() + token = self._token() + with self.assertLogs("DigiScript.Audit", "WARNING") as logs: + response = self._post( + "/api/v1/auth/change-password", + {"old_password": self.PASSWORD, "new_password": "a"}, + token=token, + method="PATCH", + ) + self.assertEqual(400, response.code) + self.assertIn("reason=invalid_new_password", logs.output[0]) + + def test_admin_reset_password(self): + self._create() + token = self._token() + bob_id = make_user(self._app, username="bob", is_admin=False) + before = self._count("password_reset", "success") + with self.assertLogs("DigiScript", "INFO") as logs: + response = self._post( + "/api/v1/auth/reset-password", {"user_id": bob_id}, token=token + ) + self.assertEqual(200, response.code) + temp = escape.json_decode(response.body)["temporary_password"] + audit = [r for r in logs.records if r.name == "DigiScript.Audit"] + self.assertEqual(1, len(audit)) + self.assertIn("AUDIT password_reset success", audit[0].getMessage()) + self.assertIn(f"target_user_id={bob_id}", audit[0].getMessage()) + self.assertEqual("alice", audit[0].username) + self._assert_no_secrets(logs, temp, self.PASSWORD) + self.assertEqual(before + 1, self._count("password_reset", "success")) diff --git a/server/test/controllers/api/v1/auth/test_token.py b/server/test/controllers/api/v1/auth/test_token.py index a1d6dbb7..150111dd 100644 --- a/server/test/controllers/api/v1/auth/test_token.py +++ b/server/test/controllers/api/v1/auth/test_token.py @@ -1,6 +1,7 @@ import hashlib import bcrypt +from prometheus_client import REGISTRY from tornado import escape from models.user import User @@ -52,3 +53,50 @@ def test_regenerating_replaces_a_legacy_key(self): key = escape.json_decode(response.body)["api_token"] self.assertEqual(200, self._whoami(key).code) self.assertEqual(401, self._whoami("old-legacy-key").code) + + +class TestApiTokenAudit(DigiScriptTestCase): + def _count(self, event: str, result: str) -> float: + return ( + REGISTRY.get_sample_value( + "digiscript_auth_events_total", {"event": event, "result": result} + ) + or 0.0 + ) + + def _post(self, path: str, user_id: int): + return self.fetch( + path, + method="POST", + body=escape.json_encode({}), + headers={"Authorization": f"Bearer {craft_token(self._app, user_id)}"}, + ) + + def test_generate_and_revoke_are_audited_without_the_key(self): + user_id = make_user(self._app, username="carol") + gen_before = self._count("api_token_generate", "success") + rev_before = self._count("api_token_revoke", "success") + + with self.assertLogs("DigiScript", "INFO") as logs: + response = self._post("/api/v1/auth/api-token/generate", user_id) + key = escape.json_decode(response.body)["api_token"] + revoked = self._post("/api/v1/auth/api-token/revoke", user_id) + self.assertEqual(200, revoked.code) + + output = "\n".join(logs.output) + self.assertIn("AUDIT api_token_generate success", output) + self.assertIn("AUDIT api_token_revoke success", output) + self.assertIn('username="carol"', output) + self.assertNotIn(key, output) + self.assertEqual(gen_before + 1, self._count("api_token_generate", "success")) + self.assertEqual(rev_before + 1, self._count("api_token_revoke", "success")) + + def test_revoke_without_token_is_audited_as_failure(self): + user_id = make_user(self._app) + before = self._count("api_token_revoke", "failure") + with self.assertLogs("DigiScript.Audit", "WARNING") as logs: + response = self._post("/api/v1/auth/api-token/revoke", user_id) + self.assertEqual(400, response.code) + self.assertIn("AUDIT api_token_revoke failure", logs.output[0]) + self.assertIn("reason=no_token", logs.output[0]) + self.assertEqual(before + 1, self._count("api_token_revoke", "failure")) diff --git a/server/test/utils/web/test_auth_audit.py b/server/test/utils/web/test_auth_audit.py new file mode 100644 index 00000000..0d21f67c --- /dev/null +++ b/server/test/utils/web/test_auth_audit.py @@ -0,0 +1,53 @@ +import unittest + +from prometheus_client import REGISTRY + +from utils.web.auth_audit import audit, sanitise + + +class TestSanitise(unittest.TestCase): + def test_strips_control_characters(self): + self.assertEqual("ab c", sanitise("a\r\nb\x00 \x1bc")) + + def test_truncates(self): + self.assertEqual(64, len(sanitise("x" * 500))) + + def test_non_string(self): + self.assertEqual("12", sanitise(12)) + + +class TestAudit(unittest.TestCase): + def _count(self, event, result): + return ( + REGISTRY.get_sample_value( + "digiscript_auth_events_total", {"event": event, "result": result} + ) + or 0.0 + ) + + def test_success_is_info_with_extra(self): + before = self._count("login", "success") + with self.assertLogs("DigiScript.Audit", "INFO") as logs: + audit("login", "success", username="alice", user_id=3, remote_ip="1.2.3.4") + record = logs.records[0] + self.assertEqual("INFO", record.levelname) + self.assertEqual( + 'AUDIT login success username="alice" user_id=3 ip=1.2.3.4', + record.getMessage(), + ) + self.assertEqual("alice", record.username) + self.assertEqual(3, record.user_id) + self.assertEqual("1.2.3.4", record.remote_ip) + self.assertEqual(before + 1, self._count("login", "success")) + + def test_failure_is_warning_with_detail(self): + with self.assertLogs("DigiScript.Audit", "WARNING") as logs: + audit("login", "failure", username="bob", detail="reason=bad_password") + self.assertEqual("WARNING", logs.records[0].levelname) + self.assertTrue(logs.output[0].endswith("reason=bad_password")) + + def test_unknown_event_or_result_rejected(self): + with self.assertRaises(ValueError): + audit("bogus", "success") + with self.assertRaises(ValueError): + audit("login", "maybe") diff --git a/server/utils/web/auth_audit.py b/server/utils/web/auth_audit.py new file mode 100644 index 00000000..9ea8dbc8 --- /dev/null +++ b/server/utils/web/auth_audit.py @@ -0,0 +1,115 @@ +"""Audit logging and metrics for authentication events. + +Every security-relevant authentication action (login, logout, password change +and reset, token refresh, API token generation and revocation) is recorded +through :func:`audit`. Each call emits a single line on the ``DigiScript.Audit`` +logger and increments the ``digiscript_auth_events_total`` Prometheus counter. + +Secrets (passwords, tokens, API keys) must never be passed to this module. +Attacker-controlled strings (notably the username supplied to the login +endpoint) are sanitised before they are logged, and the metric labels are drawn +from fixed sets, so a client cannot inject log lines or create unbounded label +cardinality. + +The counter is module-level so it is registered exactly once per process, +however many application instances are created (tests create many). +""" + +from __future__ import annotations + +import json +import logging +import unicodedata +from typing import Optional + +from prometheus_client import Counter + +from digi_server.logger import get_logger + + +EVENTS = frozenset( + { + "login", + "logout", + "password_change", + "password_reset", + "token_refresh", + "api_token_generate", + "api_token_revoke", + } +) +RESULTS = frozenset({"success", "failure"}) + +MAX_FIELD_LENGTH = 64 + +auth_events_total = Counter( + name="digiscript_auth_events_total", + documentation="Authentication events by type and outcome", + labelnames=("event", "result"), +) + + +def sanitise(value: object, max_length: int = MAX_FIELD_LENGTH) -> str: + """Make an untrusted value safe to embed in a single log line. + + Control and other non-printable characters (including newlines) are removed + and the result is truncated to *max_length* characters. + + :param value: The value to sanitise; converted with ``str``. + :param max_length: Maximum number of characters kept. + :returns: The sanitised string. + """ + text = "".join(ch for ch in str(value) if unicodedata.category(ch)[0] != "C") + return text[:max_length] + + +def audit( + event: str, + result: str, + *, + username: Optional[str] = None, + user_id: Optional[int] = None, + remote_ip: Optional[str] = None, + detail: Optional[str] = None, +) -> None: + """Record an authentication event in the log and the metrics. + + Successes are logged at INFO and failures at WARNING on + ``DigiScript.Audit``. The ``detail`` is a short reason or context string + (for example ``reason=bad_password``); it must not contain secrets. + + :param event: One of :data:`EVENTS`. + :param result: ``"success"`` or ``"failure"``. + :param username: Username the event concerns (sanitised before logging). + :param user_id: Database id of that user, if known. + :param remote_ip: Client IP address. + :param detail: Optional short context, sanitised before logging. + :raises ValueError: If ``event`` or ``result`` is not a known value. + """ + if event not in EVENTS or result not in RESULTS: + raise ValueError(f"Unknown audit event/result: {event!r}/{result!r}") + + auth_events_total.labels(event=event, result=result).inc() + + safe_username = sanitise(username) if username is not None else None + parts = [f"AUDIT {event} {result}"] + if safe_username is not None: + # JSON-quoted so spaces in a hostile name cannot spoof other fields. + parts.append(f"username={json.dumps(safe_username)}") + if user_id is not None: + parts.append(f"user_id={user_id}") + if remote_ip: + parts.append(f"ip={sanitise(remote_ip)}") + if detail: + parts.append(sanitise(detail, 128)) + + extra: dict = {} + if safe_username is not None: + extra["username"] = safe_username + if user_id is not None: + extra["user_id"] = user_id + if remote_ip: + extra["remote_ip"] = remote_ip + + level = logging.INFO if result == "success" else logging.WARNING + get_logger("Audit").log(level, " ".join(parts), extra=extra) From 68ff3fb6a87b0ad318404a60ee3fa3600775c98d Mon Sep 17 00:00:00 2001 From: Tim Bradgate Date: Thu, 22 Oct 2026 18:41:17 +0100 Subject: [PATCH 2/4] Audit v2 token endpoints and user create/update/delete Adds user_create, user_update and user_delete audit events and covers the v2 users and token controllers plus v1 user create/delete. Co-Authored-By: Claude Sonnet 5.5 --- docs/pages/user_config.md | 4 +- server/controllers/api/v1/auth/user.py | 46 +++++-- server/controllers/api/v2/users/token.py | 9 ++ server/controllers/api/v2/users/users.py | 59 +++++++-- .../test/controllers/api/v1/auth/test_auth.py | 71 +++++++++++ server/test/controllers/api/v2/test_token.py | 38 ++++++ server/test/controllers/api/v2/test_users.py | 117 ++++++++++++++++++ server/utils/web/auth_audit.py | 18 +++ 8 files changed, 346 insertions(+), 16 deletions(-) diff --git a/docs/pages/user_config.md b/docs/pages/user_config.md index cd0fd3df..d71d6b11 100644 --- a/docs/pages/user_config.md +++ b/docs/pages/user_config.md @@ -102,7 +102,9 @@ All log entries are attributed to the logged-in user where one is present: #### Authentication Audit Log -Security-relevant authentication actions are recorded as `AUDIT` lines (logger `DigiScript.Audit`) at the default `INFO` log level, independent of the **Enable Log Redaction** setting. Successes are logged at `INFO` and failures at `WARNING`. The covered events are login, logout, password change, admin password reset, token refresh, and API token generation and revocation. For example: +Security-relevant authentication actions are recorded as `AUDIT` lines (logger `DigiScript.Audit`) at the default `INFO` log level, independent of the **Enable Log Redaction** setting. Successes are logged at `INFO` and failures at `WARNING`. The covered events are login, logout, password change, admin password reset, token refresh, API token generation and revocation, and user creation, update (such as changing the admin flag) and deletion (on both the v1 and v2 APIs). For user management events the username is the acting administrator and `target_user_id` identifies the affected account; the creation of the very first user is logged without an actor. For example: + +- `AUDIT user_delete success username="admin" user_id=1 ip=203.0.113.5 target_user_id=7` - `AUDIT login failure username="alice" ip=203.0.113.5 reason=bad_password` - `AUDIT password_reset success username="admin" user_id=1 ip=203.0.113.5 target_user_id=4` (the username is the administrator who performed the reset) diff --git a/server/controllers/api/v1/auth/user.py b/server/controllers/api/v1/auth/user.py index cdcbc3ed..ce768030 100644 --- a/server/controllers/api/v1/auth/user.py +++ b/server/controllers/api/v1/auth/user.py @@ -8,7 +8,7 @@ from registry.named_locks import NamedLockRegistry from schemas.schemas import UserSchema from services.password_service import PasswordService -from utils.web.auth_audit import audit +from utils.web.auth_audit import actor, audit from utils.web.base_controller import BaseAPIController from utils.web.json_body import get_json_object from utils.web.route import ApiRoute, ApiVersion @@ -61,6 +61,12 @@ async def post(self): # Validate password strength is_valid, error_msg = PasswordService.validate_password_strength(password) if not is_valid: + audit( + "user_create", + "failure", + detail="reason=weak_password", + **actor(self), + ) self.set_status(400) await self.finish({"message": error_msg}) return @@ -70,21 +76,32 @@ async def post(self): select(User).where(User.username == username) ).first() if conflict_user: + audit( + "user_create", + "failure", + detail="reason=username_taken", + **actor(self), + ) self.set_status(400) await self.finish({"message": "Username already taken"}) return hashed_password = await PasswordService.hash_password(password) - session.add( - User( - username=username, - password=hashed_password, - is_admin=is_admin, - created_on=datetime.now(tz=timezone.utc), - ) + new_user = User( + username=username, + password=hashed_password, + is_admin=is_admin, + created_on=datetime.now(tz=timezone.utc), ) + session.add(new_user) session.commit() + audit( + "user_create", + "success", + detail=f"target_user_id={new_user.id} is_admin={bool(is_admin)}", + **actor(self), + ) if is_admin: await self.application.digi_settings.set("has_admin_user", True) @@ -125,6 +142,12 @@ async def post(self): return if user_to_delete.is_admin and len(all_admins) <= 1: + audit( + "user_delete", + "failure", + detail=f"target_user_id={user_to_delete.id} reason=only_admin", + **actor(self), + ) self.set_status(400) await self.finish({"message": "Cannot delete the only admin user"}) return @@ -141,8 +164,15 @@ async def post(self): self.application.rbac.delete_actor(user_to_delete) # Then we can delete the user + target_id = user_to_delete.id session.delete(user_to_delete) session.commit() + audit( + "user_delete", + "success", + detail=f"target_user_id={target_id}", + **actor(self), + ) self.set_status(200) await self.application.ws_send_to_all("NOOP", "GET_USERS", {}) diff --git a/server/controllers/api/v2/users/token.py b/server/controllers/api/v2/users/token.py index b12ca653..df21c0b7 100644 --- a/server/controllers/api/v2/users/token.py +++ b/server/controllers/api/v2/users/token.py @@ -1,5 +1,6 @@ from models.user import User from utils.web.api_keys import ApiKeyService +from utils.web.auth_audit import actor, audit from utils.web.base_controller import BaseAPIController from utils.web.route import ApiRoute, ApiVersion from utils.web.web_decorators import api_authenticated @@ -32,6 +33,7 @@ async def post(self): session.commit() self.application.jwt_service.api_keys.invalidate() + audit("api_token_generate", "success", **actor(self)) self.set_status(200) await self.finish( { @@ -50,6 +52,12 @@ async def delete(self): return if not user.api_token: + audit( + "api_token_revoke", + "failure", + detail="reason=no_token", + **actor(self), + ) self.set_status(400) await self.finish({"message": "No API token to revoke"}) return @@ -58,5 +66,6 @@ async def delete(self): session.commit() self.application.jwt_service.api_keys.invalidate() + audit("api_token_revoke", "success", **actor(self)) self.set_status(200) await self.finish({"message": "API token revoked successfully"}) diff --git a/server/controllers/api/v2/users/users.py b/server/controllers/api/v2/users/users.py index a583aeb2..e034929f 100644 --- a/server/controllers/api/v2/users/users.py +++ b/server/controllers/api/v2/users/users.py @@ -6,6 +6,7 @@ from registry.named_locks import NamedLockRegistry from schemas.schemas import UserSchema from services.password_service import PasswordService +from utils.web.auth_audit import actor, audit from utils.web.base_controller import BaseAPIController from utils.web.json_body import get_json_object from utils.web.route import ApiRoute, ApiVersion @@ -59,6 +60,12 @@ async def post(self): is_valid, error_msg = PasswordService.validate_password_strength(password) if not is_valid: + audit( + "user_create", + "failure", + detail="reason=weak_password", + **actor(self), + ) self.set_status(400) await self.finish({"message": error_msg}) return @@ -68,21 +75,32 @@ async def post(self): select(User).where(User.username == username) ).first() if conflict_user: + audit( + "user_create", + "failure", + detail="reason=username_taken", + **actor(self), + ) self.set_status(400) await self.finish({"message": "Username already taken"}) return hashed_password = await PasswordService.hash_password(password) - session.add( - User( - username=username, - password=hashed_password, - is_admin=is_admin, - created_on=datetime.now(tz=timezone.utc), - ) + new_user = User( + username=username, + password=hashed_password, + is_admin=is_admin, + created_on=datetime.now(tz=timezone.utc), ) + session.add(new_user) session.commit() + audit( + "user_create", + "success", + detail=f"target_user_id={new_user.id} is_admin={bool(is_admin)}", + **actor(self), + ) if is_admin: await self.application.digi_settings.set("has_admin_user", True) @@ -119,17 +137,31 @@ async def patch(self): select(User).where(User.is_admin.is_(True)) ).all() if len(all_admins) <= 1: + audit( + "user_update", + "failure", + detail=f"target_user_id={user.id} reason=only_admin", + **actor(self), + ) self.set_status(400) await self.finish( {"message": "Cannot remove admin from the only admin user"} ) return + changed = [] for field in self.EDITABLE_FIELDS: if field in data: setattr(user, field, data[field]) + changed.append(f"{field}={bool(data[field])}") session.commit() + audit( + "user_update", + "success", + detail=f"target_user_id={user.id} changed={','.join(changed) or 'none'}", + **actor(self), + ) self.set_status(200) await self.application.ws_send_to_all("NOOP", "GET_USERS", {}) @@ -163,6 +195,12 @@ async def delete(self): select(User).where(User.is_admin.is_(True)) ).all() if user_to_delete.is_admin and len(all_admins) <= 1: + audit( + "user_delete", + "failure", + detail=f"target_user_id={user_to_delete.id} reason=only_admin", + **actor(self), + ) self.set_status(400) await self.finish({"message": "Cannot delete the only admin user"}) return @@ -174,8 +212,15 @@ async def delete(self): session, user_to_delete ) self.application.rbac.delete_actor(user_to_delete) + target_id = user_to_delete.id session.delete(user_to_delete) session.commit() + audit( + "user_delete", + "success", + detail=f"target_user_id={target_id}", + **actor(self), + ) self.set_status(200) await self.application.ws_send_to_all("NOOP", "GET_USERS", {}) diff --git a/server/test/controllers/api/v1/auth/test_auth.py b/server/test/controllers/api/v1/auth/test_auth.py index bcc6b6e4..e23f4f39 100644 --- a/server/test/controllers/api/v1/auth/test_auth.py +++ b/server/test/controllers/api/v1/auth/test_auth.py @@ -1478,3 +1478,74 @@ def test_admin_reset_password(self): self.assertEqual("alice", audit[0].username) self._assert_no_secrets(logs, temp, self.PASSWORD) self.assertEqual(before + 1, self._count("password_reset", "success")) + + +class TestAuthUserAdminAudit(DigiScriptTestCase): + PASSWORD = "Sup3r-Secret-Pw-1" + + def _count(self, event, result): + return ( + REGISTRY.get_sample_value( + "digiscript_auth_events_total", {"event": event, "result": result} + ) + or 0.0 + ) + + def _post(self, path, body, token=None): + headers = {"Authorization": f"Bearer {token}"} if token else {} + return self.fetch( + path, method="POST", body=escape.json_encode(body), headers=headers + ) + + def test_user_create_and_delete_are_audited(self): + with self.assertLogs("DigiScript.Audit", "INFO") as boot: + self._post( + "/api/v1/auth/create", + {"username": "root", "password": self.PASSWORD, "is_admin": True}, + ) + self.assertIn("AUDIT user_create success", boot.output[0]) + token = escape.json_decode( + self._post( + "/api/v1/auth/login", {"username": "root", "password": self.PASSWORD} + ).body + )["access_token"] + + create_before = self._count("user_create", "success") + fail_before = self._count("user_create", "failure") + delete_before = self._count("user_delete", "success") + with self.assertLogs("DigiScript", "INFO") as logs: + self.assertEqual( + 200, + self._post( + "/api/v1/auth/create", + {"username": "dave", "password": self.PASSWORD}, + token, + ).code, + ) + self.assertEqual( + 400, + self._post( + "/api/v1/auth/create", + {"username": "dave", "password": self.PASSWORD}, + token, + ).code, + ) + with self._app.get_db().sessionmaker() as session: + dave_id = ( + session.scalars(select(User).where(User.username == "dave")) + .one() + .id + ) + self.assertEqual( + 200, self._post("/api/v1/auth/delete", {"id": dave_id}, token).code + ) + output = "\n".join(logs.output) + self.assertIn("AUDIT user_create success", output) + self.assertIn("reason=username_taken", output) + self.assertIn("AUDIT user_delete success", output) + self.assertIn(f"target_user_id={dave_id}", output) + self.assertNotIn(self.PASSWORD, output) + self.assertNotIn(token, output) + self.assertEqual(create_before + 1, self._count("user_create", "success")) + self.assertEqual(fail_before + 1, self._count("user_create", "failure")) + self.assertEqual(delete_before + 1, self._count("user_delete", "success")) diff --git a/server/test/controllers/api/v2/test_token.py b/server/test/controllers/api/v2/test_token.py index 89294ad0..6233db20 100644 --- a/server/test/controllers/api/v2/test_token.py +++ b/server/test/controllers/api/v2/test_token.py @@ -1,5 +1,6 @@ import hashlib +from prometheus_client import REGISTRY from tornado import escape from models.user import User @@ -49,3 +50,40 @@ def test_has_token_reflects_state_and_delete_revokes_key(self): self.assertEqual(200, response.code) self.assertFalse(has()) self.assertEqual(401, self._whoami(key).code) + + +class TestUsersTokenV2Audit(DigiScriptTestCase): + def _count(self, event, result): + return ( + REGISTRY.get_sample_value( + "digiscript_auth_events_total", {"event": event, "result": result} + ) + or 0.0 + ) + + def _req(self, method, user_id): + return self.fetch( + "/api/v2/users/token", + method=method, + body=None if method == "DELETE" else escape.json_encode({}), + headers={"Authorization": f"Bearer {craft_token(self._app, user_id)}"}, + ) + + def test_generate_and_revoke_audited_without_key(self): + user_id = make_user(self._app, username="erin") + gen = self._count("api_token_generate", "success") + rev = self._count("api_token_revoke", "success") + fail = self._count("api_token_revoke", "failure") + with self.assertLogs("DigiScript", "INFO") as logs: + key = escape.json_decode(self._req("POST", user_id).body)["api_token"] + self.assertEqual(200, self._req("DELETE", user_id).code) + self.assertEqual(400, self._req("DELETE", user_id).code) + output = "\n".join(logs.output) + self.assertIn("AUDIT api_token_generate success", output) + self.assertIn("AUDIT api_token_revoke success", output) + self.assertIn("AUDIT api_token_revoke failure", output) + self.assertIn('username="erin"', output) + self.assertNotIn(key, output) + self.assertEqual(gen + 1, self._count("api_token_generate", "success")) + self.assertEqual(rev + 1, self._count("api_token_revoke", "success")) + self.assertEqual(fail + 1, self._count("api_token_revoke", "failure")) diff --git a/server/test/controllers/api/v2/test_users.py b/server/test/controllers/api/v2/test_users.py index 1d22e0f0..59ec90c4 100644 --- a/server/test/controllers/api/v2/test_users.py +++ b/server/test/controllers/api/v2/test_users.py @@ -1,3 +1,4 @@ +from prometheus_client import REGISTRY from sqlalchemy import select from tornado import escape @@ -318,3 +319,119 @@ def test_create_user_invalid_json_returns_400(self): "/api/v2/users", method="POST", body="nope", raise_error=False ) self.assertEqual(400, resp.code) + + +class TestUsersV2Audit(DigiScriptTestCase): + PASSWORD = "Sup3r-Secret-Pw-1" + + def _count(self, event, result): + return ( + REGISTRY.get_sample_value( + "digiscript_auth_events_total", {"event": event, "result": result} + ) + or 0.0 + ) + + def _req(self, path, method, body=None, token=None): + headers = {"Authorization": f"Bearer {token}"} if token else {} + return self.fetch( + path, + method=method, + body=None if method == "DELETE" else escape.json_encode(body or {}), + headers=headers, + ) + + def _admin_token(self): + resp = self._req( + "/api/v2/users", + "POST", + {"username": "root", "password": self.PASSWORD, "is_admin": True}, + ) + self.assertEqual(200, resp.code) + resp = self._req( + "/api/v1/auth/login", + "POST", + {"username": "root", "password": self.PASSWORD}, + ) + return escape.json_decode(resp.body)["access_token"] + + def _no_secrets(self, logs, *secrets): + output = "\n".join(logs.output) + for secret in secrets: + self.assertNotIn(secret, output) + + def test_create_update_delete_are_audited(self): + # First-user bootstrap is unauthenticated: audited without an actor. + with self.assertLogs("DigiScript", "INFO") as boot: + token = self._admin_token() + self.assertTrue(any("AUDIT user_create success" in o for o in boot.output)) + + create_before = self._count("user_create", "success") + update_before = self._count("user_update", "success") + delete_before = self._count("user_delete", "success") + with self.assertLogs("DigiScript", "INFO") as logs: + self.assertEqual( + 200, + self._req( + "/api/v2/users", + "POST", + {"username": "dave", "password": self.PASSWORD}, + token, + ).code, + ) + with self._app.get_db().sessionmaker() as session: + dave_id = ( + session.scalars(select(User).where(User.username == "dave")) + .one() + .id + ) + self.assertEqual( + 200, + self._req( + f"/api/v2/users?id={dave_id}", "PATCH", {"is_admin": True}, token + ).code, + ) + self.assertEqual( + 200, + self._req(f"/api/v2/users?id={dave_id}", "DELETE", None, token).code, + ) + audit = [r.getMessage() for r in logs.records if r.name == "DigiScript.Audit"] + self.assertTrue(any("AUDIT user_create success" in m for m in audit)) + self.assertTrue( + any( + "AUDIT user_update success" in m + and f"target_user_id={dave_id}" in m + and "is_admin=True" in m + and 'username="root"' in m + for m in audit + ) + ) + self.assertTrue( + any( + "AUDIT user_delete success" in m and f"target_user_id={dave_id}" in m + for m in audit + ) + ) + self._no_secrets(logs, self.PASSWORD, token) + self.assertEqual(create_before + 1, self._count("user_create", "success")) + self.assertEqual(update_before + 1, self._count("user_update", "success")) + self.assertEqual(delete_before + 1, self._count("user_delete", "success")) + + def test_create_failures_are_audited(self): + token = self._admin_token() + before = self._count("user_create", "failure") + with self.assertLogs("DigiScript.Audit", "WARNING") as logs: + taken = self._req( + "/api/v2/users", + "POST", + {"username": "root", "password": self.PASSWORD}, + token, + ) + weak = self._req( + "/api/v2/users", "POST", {"username": "eve", "password": "a"}, token + ) + self.assertEqual(400, taken.code) + self.assertEqual(400, weak.code) + self.assertIn("reason=username_taken", logs.output[0]) + self.assertIn("reason=weak_password", logs.output[1]) + self.assertEqual(before + 2, self._count("user_create", "failure")) diff --git a/server/utils/web/auth_audit.py b/server/utils/web/auth_audit.py index 9ea8dbc8..13a65329 100644 --- a/server/utils/web/auth_audit.py +++ b/server/utils/web/auth_audit.py @@ -36,6 +36,9 @@ "token_refresh", "api_token_generate", "api_token_revoke", + "user_create", + "user_update", + "user_delete", } ) RESULTS = frozenset({"success", "failure"}) @@ -63,6 +66,21 @@ def sanitise(value: object, max_length: int = MAX_FIELD_LENGTH) -> str: return text[:max_length] +def actor(controller) -> dict: + """Return the ``audit`` keyword arguments identifying a request's actor. + + :param controller: A request handler with ``current_user`` and ``request``. + :returns: ``username``, ``user_id`` and ``remote_ip`` for the caller (the + first two are ``None`` when the request is unauthenticated). + """ + user = controller.current_user or {} + return { + "username": user.get("username"), + "user_id": user.get("id"), + "remote_ip": controller.request.remote_ip, + } + + def audit( event: str, result: str, From df45bdaac5bb53895aee936cf5517fb71d5316f8 Mon Sep 17 00:00:00 2001 From: Tim Bradgate Date: Thu, 22 Oct 2026 23:20:34 +0100 Subject: [PATCH 3/4] Address audit review: non-raising audit(), preinit labels, more tests audit() now logs and swallows failures instead of raising, every (event, result) series is pre-initialised, ip is quoted unless a valid address, reset-password refusals are audited, v1 token controllers use actor(), and docs/tests are tightened. Co-Authored-By: Claude Sonnet 5.5 --- docs/pages/user_config.md | 6 +- server/controllers/api/v1/auth/token.py | 14 ++--- server/controllers/api/v1/auth/user.py | 12 ++++ server/controllers/api/v2/users/users.py | 2 +- .../test/controllers/api/v1/auth/test_auth.py | 59 +++++++++++++++++- server/test/controllers/api/v2/test_users.py | 9 ++- server/test/utils/web/test_auth_audit.py | 58 ++++++++++++++++-- server/utils/web/auth_audit.py | 61 ++++++++++++++----- 8 files changed, 187 insertions(+), 34 deletions(-) diff --git a/docs/pages/user_config.md b/docs/pages/user_config.md index d71d6b11..d36d3156 100644 --- a/docs/pages/user_config.md +++ b/docs/pages/user_config.md @@ -105,11 +105,11 @@ All log entries are attributed to the logged-in user where one is present: Security-relevant authentication actions are recorded as `AUDIT` lines (logger `DigiScript.Audit`) at the default `INFO` log level, independent of the **Enable Log Redaction** setting. Successes are logged at `INFO` and failures at `WARNING`. The covered events are login, logout, password change, admin password reset, token refresh, API token generation and revocation, and user creation, update (such as changing the admin flag) and deletion (on both the v1 and v2 APIs). For user management events the username is the acting administrator and `target_user_id` identifies the affected account; the creation of the very first user is logged without an actor. For example: - `AUDIT user_delete success username="admin" user_id=1 ip=203.0.113.5 target_user_id=7` - -- `AUDIT login failure username="alice" ip=203.0.113.5 reason=bad_password` +- `AUDIT login failure username="alice" user_id=3 ip=203.0.113.5 reason=bad_password` +- `AUDIT login failure username="alcie" ip=203.0.113.5 reason=unknown_user` (no `user_id`, because no such account exists) - `AUDIT password_reset success username="admin" user_id=1 ip=203.0.113.5 target_user_id=4` (the username is the administrator who performed the reset) -A failed login records whether the username was unknown or the password was wrong, but the response sent to the client is identical in both cases. Passwords, tokens and API keys are never written to the log. Usernames typed by a client are stripped of control characters and truncated to 64 characters. The same events are counted in the `digiscript_auth_events_total{event,result}` Prometheus metric, which is useful for alerting on bursts of failed logins. +A failed login records whether the username was unknown or the password was wrong, but the response sent to the client is identical in both cases. Passwords, tokens and API keys are never deliberately logged. The username of a failed login is recorded as typed, so text mistakenly entered into the username field (for example a password) will appear in the log; treat the log files as sensitive. Usernames typed by a client are stripped of control characters, truncated to 64 characters and quoted, and the client address is quoted unless it is a valid IP address. The events are also counted in the `digiscript_auth_events_total{event,result}` Prometheus metric (per event type and result, never per user; every combination is reported as `0` until it first occurs), which is useful for alerting on bursts of failed logins. #### Filtering diff --git a/server/controllers/api/v1/auth/token.py b/server/controllers/api/v1/auth/token.py index 05dc8b05..55bb4912 100644 --- a/server/controllers/api/v1/auth/token.py +++ b/server/controllers/api/v1/auth/token.py @@ -1,6 +1,6 @@ from models.user import User from utils.web.api_keys import ApiKeyService -from utils.web.auth_audit import audit +from utils.web.auth_audit import actor, audit from utils.web.base_controller import BaseAPIController from utils.web.route import ApiRoute, ApiVersion from utils.web.web_decorators import ( @@ -28,9 +28,7 @@ async def post(self): audit( "api_token_generate", "success", - username=self.current_user.get("username"), - user_id=user.id, - remote_ip=self.request.remote_ip, + **actor(self), ) self.set_status(200) await self.finish( @@ -57,10 +55,8 @@ async def post(self): audit( "api_token_revoke", "failure", - username=self.current_user.get("username"), - user_id=user.id, - remote_ip=self.request.remote_ip, detail="reason=no_token", + **actor(self), ) self.set_status(400) await self.finish({"message": "No API token to revoke"}) @@ -73,9 +69,7 @@ async def post(self): audit( "api_token_revoke", "success", - username=self.current_user.get("username"), - user_id=user.id, - remote_ip=self.request.remote_ip, + **actor(self), ) self.set_status(200) await self.finish({"message": "API token revoked successfully"}) diff --git a/server/controllers/api/v1/auth/user.py b/server/controllers/api/v1/auth/user.py index ce768030..6f08bdd8 100644 --- a/server/controllers/api/v1/auth/user.py +++ b/server/controllers/api/v1/auth/user.py @@ -519,11 +519,23 @@ async def post(self): with self.make_session() as session: target_user = session.get(User, int(user_id)) if not target_user: + audit( + "password_reset", + "failure", + detail="reason=not_found", + **actor(self), + ) self.set_status(404) await self.finish({"message": "User not found"}) return if target_user.id == self.current_user["id"]: + audit( + "password_reset", + "failure", + detail="reason=self_reset", + **actor(self), + ) self.set_status(400) await self.finish( {"message": "Cannot reset your own password via admin endpoint"} diff --git a/server/controllers/api/v2/users/users.py b/server/controllers/api/v2/users/users.py index e034929f..31ee2389 100644 --- a/server/controllers/api/v2/users/users.py +++ b/server/controllers/api/v2/users/users.py @@ -153,7 +153,7 @@ async def patch(self): for field in self.EDITABLE_FIELDS: if field in data: setattr(user, field, data[field]) - changed.append(f"{field}={bool(data[field])}") + changed.append(f"{field}={data[field]!r}") session.commit() audit( diff --git a/server/test/controllers/api/v1/auth/test_auth.py b/server/test/controllers/api/v1/auth/test_auth.py index e23f4f39..68806ab2 100644 --- a/server/test/controllers/api/v1/auth/test_auth.py +++ b/server/test/controllers/api/v1/auth/test_auth.py @@ -1,3 +1,6 @@ +import json +from unittest.mock import AsyncMock, patch + from prometheus_client import REGISTRY from sqlalchemy import select from tornado import escape @@ -1391,7 +1394,56 @@ def test_login_failure_sanitises_username(self): self.assertNotIn("\r", message) self.assertNotIn("\x1b", message) self.assertLessEqual(len(logs.records[0].username), 64) - self.assertTrue(message.startswith("AUDIT login failure username=")) + # Control chars removed, capped at 64, quote escaped; the lookalike + # ``ip=`` stays inside the quoted username and the real one appears once. + sanitised = 'bob" ip=6.6.6.6AUDIT login success[31m' + "x" * 26 + self.assertEqual( + f"AUDIT login failure username={json.dumps(sanitised)} " + "ip=127.0.0.1 reason=unknown_user", + message, + ) + self.assertEqual(1, message.count(" ip=127.0.0.1")) + self.assertNotIn('username="bob" ip=6.6.6.6', message) + + def test_admin_reset_password_refusals_are_audited(self): + self._create() + token = self._token() + with self._app.get_db().sessionmaker() as session: + admin_id = ( + session.scalars(select(User).where(User.username == "alice")).one().id + ) + before = self._count("password_reset", "failure") + with self.assertLogs("DigiScript.Audit", "WARNING") as logs: + missing = self._post( + "/api/v1/auth/reset-password", {"user_id": 9999}, token=token + ) + own = self._post( + "/api/v1/auth/reset-password", {"user_id": admin_id}, token=token + ) + self.assertEqual(404, missing.code) + self.assertEqual(400, own.code) + self.assertIn("reason=not_found", logs.output[0]) + self.assertIn("reason=self_reset", logs.output[1]) + self.assertEqual(before + 2, self._count("password_reset", "failure")) + + def test_admin_reset_password_invalid_password_is_audited(self): + self._create() + token = self._token() + bob_id = make_user(self._app, username="bob", is_admin=False) + before = self._count("password_reset", "failure") + with patch.object( + type(self._app.user_service), + "change_password", + AsyncMock(side_effect=ValueError("too weak")), + ): + with self.assertLogs("DigiScript.Audit", "WARNING") as logs: + response = self._post( + "/api/v1/auth/reset-password", {"user_id": bob_id}, token=token + ) + self.assertEqual(400, response.code) + self.assertIn("reason=invalid_password", logs.output[0]) + self.assertIn(f"target_user_id={bob_id}", logs.output[0]) + self.assertEqual(before + 1, self._count("password_reset", "failure")) def test_logout(self): self._create() @@ -1504,6 +1556,11 @@ def test_user_create_and_delete_are_audited(self): {"username": "root", "password": self.PASSWORD, "is_admin": True}, ) self.assertIn("AUDIT user_create success", boot.output[0]) + # No actor: the bootstrap request is unauthenticated. + self.assertNotIn(" username=", boot.output[0]) + self.assertNotIn(" user_id=", boot.output[0]) + self.assertFalse(hasattr(boot.records[0], "username")) + self.assertFalse(hasattr(boot.records[0], "user_id")) token = escape.json_decode( self._post( "/api/v1/auth/login", {"username": "root", "password": self.PASSWORD} diff --git a/server/test/controllers/api/v2/test_users.py b/server/test/controllers/api/v2/test_users.py index 59ec90c4..ff390dc4 100644 --- a/server/test/controllers/api/v2/test_users.py +++ b/server/test/controllers/api/v2/test_users.py @@ -364,7 +364,14 @@ def test_create_update_delete_are_audited(self): # First-user bootstrap is unauthenticated: audited without an actor. with self.assertLogs("DigiScript", "INFO") as boot: token = self._admin_token() - self.assertTrue(any("AUDIT user_create success" in o for o in boot.output)) + created = [ + r for r in boot.records if "AUDIT user_create success" in r.getMessage() + ] + self.assertEqual(1, len(created)) + self.assertNotIn(" username=", created[0].getMessage()) + self.assertNotIn(" user_id=", created[0].getMessage()) + self.assertFalse(hasattr(created[0], "username")) + self.assertFalse(hasattr(created[0], "user_id")) create_before = self._count("user_create", "success") update_before = self._count("user_update", "success") diff --git a/server/test/utils/web/test_auth_audit.py b/server/test/utils/web/test_auth_audit.py index 0d21f67c..56f9fb18 100644 --- a/server/test/utils/web/test_auth_audit.py +++ b/server/test/utils/web/test_auth_audit.py @@ -1,8 +1,15 @@ import unittest +from unittest.mock import patch from prometheus_client import REGISTRY -from utils.web.auth_audit import audit, sanitise +from utils.web.auth_audit import ( + EVENTS, + RESULTS, + audit, + auth_events_total, + sanitise, +) class TestSanitise(unittest.TestCase): @@ -46,8 +53,51 @@ def test_failure_is_warning_with_detail(self): self.assertEqual("WARNING", logs.records[0].levelname) self.assertTrue(logs.output[0].endswith("reason=bad_password")) - def test_unknown_event_or_result_rejected(self): - with self.assertRaises(ValueError): + def test_unknown_event_or_result_is_logged_not_raised_or_counted(self): + series_before = len(list(auth_events_total.collect()[0].samples)) + with self.assertLogs("DigiScript.Audit", "ERROR") as logs: audit("bogus", "success") - with self.assertRaises(ValueError): audit("login", "maybe") + self.assertEqual(2, len(logs.output)) + self.assertEqual( + series_before, len(list(auth_events_total.collect()[0].samples)) + ) + + def test_internal_failure_is_swallowed_and_logged(self): + with patch("utils.web.auth_audit._emit", side_effect=RuntimeError("boom")): + with self.assertLogs("DigiScript.Audit", "ERROR") as logs: + audit("login", "success", username="alice") + self.assertIsNotNone(logs.records[0].exc_info) + + def test_non_ip_address_is_quoted(self): + with self.assertLogs("DigiScript.Audit", "INFO") as logs: + audit("login", "success", remote_ip="1.2.3.4 user_id=0") + self.assertIn('ip="1.2.3.4 user_id=0"', logs.output[0]) + + def test_ipv6_address_is_bare(self): + with self.assertLogs("DigiScript.Audit", "INFO") as logs: + audit("login", "success", remote_ip="::1") + self.assertTrue(logs.records[0].getMessage().endswith(" ip=::1")) + + +class TestAuthEventsMetric(unittest.TestCase): + def test_label_names_are_bounded(self): + self.assertEqual(("event", "result"), auth_events_total._labelnames) + + def test_every_event_result_pair_is_preinitialised(self): + for event in EVENTS: + for result in RESULTS: + self.assertIsNotNone( + REGISTRY.get_sample_value( + "digiscript_auth_events_total", + {"event": event, "result": result}, + ), + f"{event}/{result} missing", + ) + + def test_usernames_do_not_create_series(self): + before = len(list(auth_events_total.collect()[0].samples)) + with self.assertLogs("DigiScript.Audit", "WARNING"): + audit("login", "failure", username="user-one") + audit("login", "failure", username="user-two") + self.assertEqual(before, len(list(auth_events_total.collect()[0].samples))) diff --git a/server/utils/web/auth_audit.py b/server/utils/web/auth_audit.py index 13a65329..98561fca 100644 --- a/server/utils/web/auth_audit.py +++ b/server/utils/web/auth_audit.py @@ -1,15 +1,21 @@ """Audit logging and metrics for authentication events. -Every security-relevant authentication action (login, logout, password change -and reset, token refresh, API token generation and revocation) is recorded -through :func:`audit`. Each call emits a single line on the ``DigiScript.Audit`` -logger and increments the ``digiscript_auth_events_total`` Prometheus counter. - -Secrets (passwords, tokens, API keys) must never be passed to this module. -Attacker-controlled strings (notably the username supplied to the login -endpoint) are sanitised before they are logged, and the metric labels are drawn -from fixed sets, so a client cannot inject log lines or create unbounded label -cardinality. +Every security-relevant authentication or user-management action (see +:data:`EVENTS` for the full set) is recorded through :func:`audit`. Each call +emits a single line on the ``DigiScript.Audit`` logger and increments the +``digiscript_auth_events_total`` Prometheus counter. + +Secrets (passwords, tokens, API keys) are never deliberately passed to this +module. Attacker-controlled strings (notably the username supplied to the login +endpoint) are stripped of control characters, length-capped and JSON-quoted, and +the client address is emitted bare only when it parses as an IP address +(otherwise it is quoted), so a client cannot inject extra lines or fake +fields. The metric labels are drawn from fixed sets and every combination is +pre-initialised, so cardinality is bounded and alerts see ``0`` rather than no +data. Text typed into the username field is recorded as typed. + +:func:`audit` never raises: it runs after the real action has succeeded, so a +failure here is logged and swallowed rather than failing the request. The counter is module-level so it is registered exactly once per process, however many application instances are created (tests create many). @@ -17,6 +23,7 @@ from __future__ import annotations +import ipaddress import json import logging import unicodedata @@ -50,6 +57,10 @@ documentation="Authentication events by type and outcome", labelnames=("event", "result"), ) +# Create every series up front so alerts on failures see 0, not "no data". +for _event in EVENTS: + for _result in RESULTS: + auth_events_total.labels(event=_event, result=_result) def sanitise(value: object, max_length: int = MAX_FIELD_LENGTH) -> str: @@ -102,11 +113,33 @@ def audit( :param user_id: Database id of that user, if known. :param remote_ip: Client IP address. :param detail: Optional short context, sanitised before logging. - :raises ValueError: If ``event`` or ``result`` is not a known value. + + This function never raises. An unknown ``event``/``result`` (a programming + error) is logged at ERROR and not counted, so labels stay bounded; any other + failure is logged with its traceback. """ if event not in EVENTS or result not in RESULTS: - raise ValueError(f"Unknown audit event/result: {event!r}/{result!r}") - + get_logger("Audit").error( + f"Unknown audit event/result: {sanitise(event)!r}/{sanitise(result)!r}" + ) + return + try: + _emit(event, result, username, user_id, remote_ip, detail) + except Exception: + get_logger("Audit").exception(f"Failed to record audit event {event} {result}") + + +def _format_ip(remote_ip: str) -> str: + """Render a client address, quoting it unless it is a valid IP address.""" + try: + ipaddress.ip_address(remote_ip) + except ValueError: + return json.dumps(sanitise(remote_ip)) + return remote_ip + + +def _emit(event, result, username, user_id, remote_ip, detail) -> None: + """Count and log one already-validated audit event.""" auth_events_total.labels(event=event, result=result).inc() safe_username = sanitise(username) if username is not None else None @@ -117,7 +150,7 @@ def audit( if user_id is not None: parts.append(f"user_id={user_id}") if remote_ip: - parts.append(f"ip={sanitise(remote_ip)}") + parts.append(f"ip={_format_ip(remote_ip)}") if detail: parts.append(sanitise(detail, 128)) From bd30551527a54b6e33068693fabff12f83fbb49f Mon Sep 17 00:00:00 2001 From: Tim Bradgate Date: Thu, 22 Oct 2026 23:53:16 +0100 Subject: [PATCH 4/4] Share v1/v2 user and API-token handler bodies to remove duplication Move user create/delete and API token generate/revoke (with their audit calls) into utils/web/user_admin.py, called by both API versions. Pure refactor: responses, status codes, audit lines and broadcasts unchanged. Co-Authored-By: Claude Sonnet 5.5 --- server/controllers/api/v1/auth/token.py | 58 +----- server/controllers/api/v1/auth/user.py | 129 +------------- server/controllers/api/v2/users/token.py | 49 +----- server/controllers/api/v2/users/users.py | 125 +------------ server/utils/web/user_admin.py | 213 +++++++++++++++++++++++ 5 files changed, 225 insertions(+), 349 deletions(-) create mode 100644 server/utils/web/user_admin.py diff --git a/server/controllers/api/v1/auth/token.py b/server/controllers/api/v1/auth/token.py index 55bb4912..f142f545 100644 --- a/server/controllers/api/v1/auth/token.py +++ b/server/controllers/api/v1/auth/token.py @@ -1,8 +1,7 @@ from models.user import User -from utils.web.api_keys import ApiKeyService -from utils.web.auth_audit import actor, audit from utils.web.base_controller import BaseAPIController from utils.web.route import ApiRoute, ApiVersion +from utils.web.user_admin import generate_api_token, revoke_api_token from utils.web.web_decorators import ( api_authenticated, ) @@ -13,30 +12,7 @@ class ApiTokenGenerateController(BaseAPIController): @api_authenticated async def post(self): """Generate a new API token for the authenticated user""" - with self.make_session() as session: - user = session.get(User, self.current_user["id"]) - if not user: - self.set_status(404) - await self.finish({"message": "User not found"}) - return - - # Key is a random secret; only its SHA-256 digest is stored - new_token, user.api_token = ApiKeyService.generate() - session.commit() - self.application.jwt_service.api_keys.invalidate() - - audit( - "api_token_generate", - "success", - **actor(self), - ) - self.set_status(200) - await self.finish( - { - "message": "API token generated successfully", - "api_token": new_token, - } - ) + await generate_api_token(self) @ApiRoute("auth/api-token/revoke", ApiVersion.V1) @@ -44,35 +20,7 @@ class ApiTokenRevokeController(BaseAPIController): @api_authenticated async def post(self): """Revoke the API token for the authenticated user""" - with self.make_session() as session: - user = session.get(User, self.current_user["id"]) - if not user: - self.set_status(404) - await self.finish({"message": "User not found"}) - return - - if not user.api_token: - audit( - "api_token_revoke", - "failure", - detail="reason=no_token", - **actor(self), - ) - self.set_status(400) - await self.finish({"message": "No API token to revoke"}) - return - - user.api_token = None - session.commit() - self.application.jwt_service.api_keys.invalidate() - - audit( - "api_token_revoke", - "success", - **actor(self), - ) - self.set_status(200) - await self.finish({"message": "API token revoked successfully"}) + await revoke_api_token(self) @ApiRoute("auth/api-token", ApiVersion.V1) diff --git a/server/controllers/api/v1/auth/user.py b/server/controllers/api/v1/auth/user.py index 745821bf..90bffb0a 100644 --- a/server/controllers/api/v1/auth/user.py +++ b/server/controllers/api/v1/auth/user.py @@ -12,6 +12,7 @@ from utils.web.base_controller import BaseAPIController from utils.web.json_body import get_json_object from utils.web.route import ApiRoute, ApiVersion +from utils.web.user_admin import create_user, delete_user from utils.web.web_decorators import ( allow_when_password_required, api_authenticated, @@ -35,86 +36,7 @@ async def post(self): # The has-any-users check and the insert form one critical section; # otherwise concurrent first-run requests could each create an admin. async with NamedLockRegistry.acquire(USER_CREATE_LOCK): - await self._create_user() - - async def _create_user(self): - with self.make_session() as session: - # If there are no users, allow creation without authentication, otherwise require admin. - has_any_users = session.scalars(select(User)).first() is not None - if has_any_users: - self.requires_admin() - - data = get_json_object(self) - - username = data.get("username", "") - if not username: - self.set_status(400) - await self.finish({"message": "Username missing"}) - return - - password = data.get("password", "") - if not password: - self.set_status(400) - await self.finish({"message": "Password missing"}) - return - - is_admin = data.get("is_admin", False) - if not has_any_users and not is_admin: - self.set_status(400) - await self.finish({"message": "First user must be an admin"}) - return - - # Validate password strength - is_valid, error_msg = PasswordService.validate_password_strength(password) - if not is_valid: - audit( - "user_create", - "failure", - detail="reason=weak_password", - **actor(self), - ) - self.set_status(400) - await self.finish({"message": error_msg}) - return - - async with NamedLockRegistry.acquire(f"UserLock::{username}"): - conflict_user = session.scalars( - select(User).where(User.username == username) - ).first() - if conflict_user: - audit( - "user_create", - "failure", - detail="reason=username_taken", - **actor(self), - ) - self.set_status(400) - await self.finish({"message": "Username already taken"}) - return - - hashed_password = await PasswordService.hash_password(password) - - new_user = User( - username=username, - password=hashed_password, - is_admin=is_admin, - created_on=datetime.now(tz=timezone.utc), - ) - session.add(new_user) - session.commit() - audit( - "user_create", - "success", - detail=f"target_user_id={new_user.id} is_admin={bool(is_admin)}", - **actor(self), - ) - - if is_admin: - await self.application.digi_settings.set("has_admin_user", True) - - self.set_status(200) - await self.application.ws_send_to_all("NOOP", "GET_USERS", {}) - await self.finish({"message": "Successfully created user"}) + await create_user(self) @ApiRoute("auth/delete", ApiVersion.V1) @@ -132,57 +54,12 @@ async def post(self): with self.make_session() as session: user_to_delete: User = session.get(User, int(user_to_delete)) - all_admins = session.scalars( - select(User).where(User.is_admin.is_(True)) - ).all() if not user_to_delete: self.set_status(400) await self.finish({"message": "Could not find user to delete"}) return - if user_to_delete.id == self.current_user["id"]: - self.set_status(400) - await self.finish( - {"message": "Cannot delete currently authenticated user"} - ) - return - - if user_to_delete.is_admin and len(all_admins) <= 1: - audit( - "user_delete", - "failure", - detail=f"target_user_id={user_to_delete.id} reason=only_admin", - **actor(self), - ) - self.set_status(400) - await self.finish({"message": "Cannot delete the only admin user"}) - return - - async with NamedLockRegistry.acquire( - f"UserLock::{user_to_delete.username}" - ): - # Force logout all sessions using UserService - await self.application.user_service.force_logout_all_sessions( - session, user_to_delete - ) - - # Delete all RBAC associations for this user - self.application.rbac.delete_actor(user_to_delete) - - # Then we can delete the user - target_id = user_to_delete.id - session.delete(user_to_delete) - session.commit() - audit( - "user_delete", - "success", - detail=f"target_user_id={target_id}", - **actor(self), - ) - - self.set_status(200) - await self.application.ws_send_to_all("NOOP", "GET_USERS", {}) - await self.finish({"message": "Successfully deleted user"}) + await delete_user(self, session, user_to_delete) @ApiRoute("auth/login", ApiVersion.V1) diff --git a/server/controllers/api/v2/users/token.py b/server/controllers/api/v2/users/token.py index df21c0b7..ce19095f 100644 --- a/server/controllers/api/v2/users/token.py +++ b/server/controllers/api/v2/users/token.py @@ -1,8 +1,7 @@ from models.user import User -from utils.web.api_keys import ApiKeyService -from utils.web.auth_audit import actor, audit from utils.web.base_controller import BaseAPIController from utils.web.route import ApiRoute, ApiVersion +from utils.web.user_admin import generate_api_token, revoke_api_token from utils.web.web_decorators import api_authenticated @@ -22,50 +21,8 @@ def get(self): @api_authenticated async def post(self): - with self.make_session() as session: - user = session.get(User, self.current_user["id"]) - if not user: - self.set_status(404) - await self.finish({"message": "User not found"}) - return - - new_token, user.api_token = ApiKeyService.generate() - session.commit() - self.application.jwt_service.api_keys.invalidate() - - audit("api_token_generate", "success", **actor(self)) - self.set_status(200) - await self.finish( - { - "message": "API token generated successfully", - "api_token": new_token, - } - ) + await generate_api_token(self) @api_authenticated async def delete(self): - with self.make_session() as session: - user = session.get(User, self.current_user["id"]) - if not user: - self.set_status(404) - await self.finish({"message": "User not found"}) - return - - if not user.api_token: - audit( - "api_token_revoke", - "failure", - detail="reason=no_token", - **actor(self), - ) - self.set_status(400) - await self.finish({"message": "No API token to revoke"}) - return - - user.api_token = None - session.commit() - self.application.jwt_service.api_keys.invalidate() - - audit("api_token_revoke", "success", **actor(self)) - self.set_status(200) - await self.finish({"message": "API token revoked successfully"}) + await revoke_api_token(self) diff --git a/server/controllers/api/v2/users/users.py b/server/controllers/api/v2/users/users.py index 23f81404..d134a389 100644 --- a/server/controllers/api/v2/users/users.py +++ b/server/controllers/api/v2/users/users.py @@ -1,15 +1,13 @@ -from datetime import datetime, timezone - from sqlalchemy import select from models.user import User from registry.named_locks import USER_CREATE_LOCK, NamedLockRegistry from schemas.schemas import UserSchema -from services.password_service import PasswordService from utils.web.auth_audit import actor, audit from utils.web.base_controller import BaseAPIController from utils.web.json_body import get_json_object from utils.web.route import ApiRoute, ApiVersion +from utils.web.user_admin import create_user, delete_user from utils.web.web_decorators import ( api_authenticated, no_live_session, @@ -36,84 +34,7 @@ async def post(self): # The has-any-users check and the insert form one critical section; # otherwise concurrent first-run requests could each create an admin. async with NamedLockRegistry.acquire(USER_CREATE_LOCK): - await self._create_user() - - async def _create_user(self): - with self.make_session() as session: - has_any_users = session.scalars(select(User)).first() is not None - if has_any_users: - self.requires_admin() - - data = get_json_object(self) - - username = data.get("username", "") - if not username: - self.set_status(400) - await self.finish({"message": "Username missing"}) - return - - password = data.get("password", "") - if not password: - self.set_status(400) - await self.finish({"message": "Password missing"}) - return - - is_admin = data.get("is_admin", False) - if not has_any_users and not is_admin: - self.set_status(400) - await self.finish({"message": "First user must be an admin"}) - return - - is_valid, error_msg = PasswordService.validate_password_strength(password) - if not is_valid: - audit( - "user_create", - "failure", - detail="reason=weak_password", - **actor(self), - ) - self.set_status(400) - await self.finish({"message": error_msg}) - return - - async with NamedLockRegistry.acquire(f"UserLock::{username}"): - conflict_user = session.scalars( - select(User).where(User.username == username) - ).first() - if conflict_user: - audit( - "user_create", - "failure", - detail="reason=username_taken", - **actor(self), - ) - self.set_status(400) - await self.finish({"message": "Username already taken"}) - return - - hashed_password = await PasswordService.hash_password(password) - - new_user = User( - username=username, - password=hashed_password, - is_admin=is_admin, - created_on=datetime.now(tz=timezone.utc), - ) - session.add(new_user) - session.commit() - audit( - "user_create", - "success", - detail=f"target_user_id={new_user.id} is_admin={bool(is_admin)}", - **actor(self), - ) - - if is_admin: - await self.application.digi_settings.set("has_admin_user", True) - - self.set_status(200) - await self.application.ws_send_to_all("NOOP", "GET_USERS", {}) - await self.finish({"message": "Successfully created user"}) + await create_user(self) @api_authenticated @require_admin @@ -190,44 +111,4 @@ async def delete(self): await self.finish({"message": "User not found"}) return - if user_to_delete.id == self.current_user["id"]: - self.set_status(400) - await self.finish( - {"message": "Cannot delete currently authenticated user"} - ) - return - - all_admins = session.scalars( - select(User).where(User.is_admin.is_(True)) - ).all() - if user_to_delete.is_admin and len(all_admins) <= 1: - audit( - "user_delete", - "failure", - detail=f"target_user_id={user_to_delete.id} reason=only_admin", - **actor(self), - ) - self.set_status(400) - await self.finish({"message": "Cannot delete the only admin user"}) - return - - async with NamedLockRegistry.acquire( - f"UserLock::{user_to_delete.username}" - ): - await self.application.user_service.force_logout_all_sessions( - session, user_to_delete - ) - self.application.rbac.delete_actor(user_to_delete) - target_id = user_to_delete.id - session.delete(user_to_delete) - session.commit() - audit( - "user_delete", - "success", - detail=f"target_user_id={target_id}", - **actor(self), - ) - - self.set_status(200) - await self.application.ws_send_to_all("NOOP", "GET_USERS", {}) - await self.finish({"message": "Successfully deleted user"}) + await delete_user(self, session, user_to_delete) diff --git a/server/utils/web/user_admin.py b/server/utils/web/user_admin.py new file mode 100644 index 00000000..dcbc990b --- /dev/null +++ b/server/utils/web/user_admin.py @@ -0,0 +1,213 @@ +"""Request-handler bodies shared by the v1 and v2 user and API-token endpoints. + +The v1 and v2 controllers differ only in routing and in a few response details, +so the common work (and its audit calls) lives here, once. Each function takes +the calling controller and writes the response itself, exactly as the inline +handler bodies did. +""" + +from __future__ import annotations + +from datetime import datetime, timezone + +from sqlalchemy import select + +from models.user import User +from registry.named_locks import NamedLockRegistry +from services.password_service import PasswordService +from utils.web.api_keys import ApiKeyService +from utils.web.auth_audit import actor, audit +from utils.web.json_body import get_json_object + + +async def create_user(controller) -> None: + """Create a user from the request body and write the response. + + The caller must hold the ``USER_CREATE_LOCK`` so the has-any-users check and + the insert form one critical section. Anyone may create the first user (who + must be an admin); after that only an admin may. + + :param controller: The request handler. + """ + with controller.make_session() as session: + # If there are no users, allow creation without authentication, otherwise + # require admin. + has_any_users = session.scalars(select(User)).first() is not None + if has_any_users: + controller.requires_admin() + + data = get_json_object(controller) + + username = data.get("username", "") + if not username: + controller.set_status(400) + await controller.finish({"message": "Username missing"}) + return + + password = data.get("password", "") + if not password: + controller.set_status(400) + await controller.finish({"message": "Password missing"}) + return + + is_admin = data.get("is_admin", False) + if not has_any_users and not is_admin: + controller.set_status(400) + await controller.finish({"message": "First user must be an admin"}) + return + + is_valid, error_msg = PasswordService.validate_password_strength(password) + if not is_valid: + audit( + "user_create", + "failure", + detail="reason=weak_password", + **actor(controller), + ) + controller.set_status(400) + await controller.finish({"message": error_msg}) + return + + async with NamedLockRegistry.acquire(f"UserLock::{username}"): + conflict_user = session.scalars( + select(User).where(User.username == username) + ).first() + if conflict_user: + audit( + "user_create", + "failure", + detail="reason=username_taken", + **actor(controller), + ) + controller.set_status(400) + await controller.finish({"message": "Username already taken"}) + return + + hashed_password = await PasswordService.hash_password(password) + + new_user = User( + username=username, + password=hashed_password, + is_admin=is_admin, + created_on=datetime.now(tz=timezone.utc), + ) + session.add(new_user) + session.commit() + audit( + "user_create", + "success", + detail=f"target_user_id={new_user.id} is_admin={bool(is_admin)}", + **actor(controller), + ) + + if is_admin: + await controller.application.digi_settings.set("has_admin_user", True) + + controller.set_status(200) + await controller.application.ws_send_to_all("NOOP", "GET_USERS", {}) + await controller.finish({"message": "Successfully created user"}) + + +async def delete_user(controller, session, user_to_delete: User) -> None: + """Delete an existing user, refusing self-deletion and the only admin. + + :param controller: The request handler (its current user is the actor). + :param session: Open database session that loaded ``user_to_delete``. + :param user_to_delete: The user to remove. + """ + if user_to_delete.id == controller.current_user["id"]: + controller.set_status(400) + await controller.finish( + {"message": "Cannot delete currently authenticated user"} + ) + return + + all_admins = session.scalars(select(User).where(User.is_admin.is_(True))).all() + if user_to_delete.is_admin and len(all_admins) <= 1: + audit( + "user_delete", + "failure", + detail=f"target_user_id={user_to_delete.id} reason=only_admin", + **actor(controller), + ) + controller.set_status(400) + await controller.finish({"message": "Cannot delete the only admin user"}) + return + + async with NamedLockRegistry.acquire(f"UserLock::{user_to_delete.username}"): + await controller.application.user_service.force_logout_all_sessions( + session, user_to_delete + ) + controller.application.rbac.delete_actor(user_to_delete) + target_id = user_to_delete.id + session.delete(user_to_delete) + session.commit() + audit( + "user_delete", + "success", + detail=f"target_user_id={target_id}", + **actor(controller), + ) + + controller.set_status(200) + await controller.application.ws_send_to_all("NOOP", "GET_USERS", {}) + await controller.finish({"message": "Successfully deleted user"}) + + +async def generate_api_token(controller) -> None: + """Generate an API token for the current user and write the response. + + :param controller: The request handler. + """ + with controller.make_session() as session: + user = session.get(User, controller.current_user["id"]) + if not user: + controller.set_status(404) + await controller.finish({"message": "User not found"}) + return + + # Key is a random secret; only its SHA-256 digest is stored + new_token, user.api_token = ApiKeyService.generate() + session.commit() + controller.application.jwt_service.api_keys.invalidate() + + audit("api_token_generate", "success", **actor(controller)) + controller.set_status(200) + await controller.finish( + { + "message": "API token generated successfully", + "api_token": new_token, + } + ) + + +async def revoke_api_token(controller) -> None: + """Revoke the current user's API token and write the response. + + :param controller: The request handler. + """ + with controller.make_session() as session: + user = session.get(User, controller.current_user["id"]) + if not user: + controller.set_status(404) + await controller.finish({"message": "User not found"}) + return + + if not user.api_token: + audit( + "api_token_revoke", + "failure", + detail="reason=no_token", + **actor(controller), + ) + controller.set_status(400) + await controller.finish({"message": "No API token to revoke"}) + return + + user.api_token = None + session.commit() + controller.application.jwt_service.api_keys.invalidate() + + audit("api_token_revoke", "success", **actor(controller)) + controller.set_status(200) + await controller.finish({"message": "API token revoked successfully"})