diff --git a/.env.example b/.env.example index 15791c9d..84effb9d 100644 --- a/.env.example +++ b/.env.example @@ -61,7 +61,7 @@ OIDC_CLIENT_ID= OIDC_CLIENT_SECRET= OIDC_SCOPE=openid profile email groups OIDC_GROUPS_CLAIM=groups -OIDC_ADMIN_GROUPS=Admin,Owner,Steering Committee +OIDC_ADMIN_GROUPS=authentik Admins OIDC_CALLBACK_PATH=/auth/callback # Optional external base URL used to build redirect_uri (defaults to request base URL) OIDC_REDIRECT_BASE_URL= @@ -78,9 +78,12 @@ DASHBOARD_PUBLIC_BASE_URL= # Discord admin link checks (DB-first, Discord API fallback) DISCORD_ADMIN_GUILD_ID= -DISCORD_ADMIN_ROLES=Admin,Owner,Steering Committee +DISCORD_ADMIN_ROLES=Admin,Owner DISCORD_API_TIMEOUT_SECONDS=8.0 DISCORD_LINK_TTL_SECONDS=600 +# Temporary bootstrap switch: when false, Discord deep-link logins do not require +# an OIDC roundtrip and skip OIDC-based admin-group and email-to-Discord-link checks. +DISCORD_LINK_REQUIRE_OIDC_IDENTITY_CHECKS=true # Worker / consumer (optional defaults) WORKER_NAME=worker diff --git a/apps/api/README.md b/apps/api/README.md index bff1a139..f2f38e7c 100644 --- a/apps/api/README.md +++ b/apps/api/README.md @@ -41,6 +41,13 @@ curl -X GET "http://localhost:8090/jobs/" \ - `GET /auth/discord/link/{token}`: Resolve Discord deep link into authenticated dashboard redirect. - Auth flows emit best-effort human audit events (`auth.login`, `auth.logout`) under source `admin_dashboard`. +Discord deep-link identity policy: + +- `DISCORD_ADMIN_ROLES` controls who can mint/use Discord deep links (`Admin,Owner` recommended). +- `OIDC_ADMIN_GROUPS` controls normal OIDC dashboard admin membership (`authentik Admins` recommended). +- `DISCORD_LINK_REQUIRE_OIDC_IDENTITY_CHECKS=true` (default): Discord deep links also require OIDC admin group + OIDC email linked to Discord admin identity. +- `DISCORD_LINK_REQUIRE_OIDC_IDENTITY_CHECKS=false`: Discord deep links create a Discord-backed admin session directly after re-validating active CRM membership + Discord admin role, without forcing an OIDC roundtrip. + ### Known handler wiring expectation - `/jobs/{job_id}/rerun` replays the source job’s stored call arguments; rerunnable job types must only include callables that are also registered for worker execution. diff --git a/apps/api/src/five08/backend/api.py b/apps/api/src/five08/backend/api.py index f317c6f1..36f6f411 100644 --- a/apps/api/src/five08/backend/api.py +++ b/apps/api/src/five08/backend/api.py @@ -380,6 +380,13 @@ async def _current_session(request: Request) -> tuple[str | None, AuthSession | return session_id, session +def _session_actor_provider(session: AuthSession) -> ActorProvider: + raw_provider = session.actor_provider.strip().lower() + if raw_provider == ActorProvider.DISCORD.value: + return ActorProvider.DISCORD + return ActorProvider.ADMIN_SSO + + def _set_session_cookie( response: JSONResponse | RedirectResponse, session_id: str ) -> None: @@ -1269,6 +1276,9 @@ async def auth_callback_handler( display_name = None audit_actor_subject = (email or str(claims.get("sub", "")).strip()).strip() + enforce_discord_link_identity_checks = ( + settings.discord_link_require_oidc_identity_checks + ) if pending.discord_link_token: grant = await store.get_discord_link(pending.discord_link_token) @@ -1283,58 +1293,64 @@ async def auth_callback_handler( ) return JSONResponse({"error": "link_not_found"}, status_code=404) - if not is_admin: - await _write_auth_audit_event( - action="auth.login", - result=AuditResult.DENIED, - actor_subject=audit_actor_subject, - actor_display_name=display_name, - metadata={"reason": "admin_group_required", "groups": groups}, - correlation_id=state, - ) - return JSONResponse( - {"error": "forbidden", "detail": "admin_group_required"}, - status_code=403, - ) + if enforce_discord_link_identity_checks: + if not is_admin: + await _write_auth_audit_event( + action="auth.login", + result=AuditResult.DENIED, + actor_subject=audit_actor_subject, + actor_display_name=display_name, + metadata={"reason": "admin_group_required", "groups": groups}, + correlation_id=state, + ) + return JSONResponse( + {"error": "forbidden", "detail": "admin_group_required"}, + status_code=403, + ) - if not email: - await _write_auth_audit_event( - action="auth.login", - result=AuditResult.DENIED, - actor_subject=audit_actor_subject, - actor_display_name=display_name, - metadata={"reason": "email_claim_required"}, - correlation_id=state, - ) - return JSONResponse( - {"error": "forbidden", "detail": "email_claim_required"}, - status_code=403, - ) + if not email: + await _write_auth_audit_event( + action="auth.login", + result=AuditResult.DENIED, + actor_subject=audit_actor_subject, + actor_display_name=display_name, + metadata={"reason": "email_claim_required"}, + correlation_id=state, + ) + return JSONResponse( + {"error": "forbidden", "detail": "email_claim_required"}, + status_code=403, + ) - verifier = _discord_admin_verifier_from_app(request.app) - linked = await verifier.is_admin_email_for_discord_user( - email=email, - discord_user_id=grant.discord_user_id, - ) - if not linked: - await _write_auth_audit_event( - action="auth.login", - result=AuditResult.DENIED, - actor_subject=audit_actor_subject, - actor_display_name=display_name, - metadata={ - "reason": "oidc_user_not_linked_to_discord_admin", - "discord_user_id": grant.discord_user_id, - }, - correlation_id=state, - ) - return JSONResponse( - { - "error": "forbidden", - "detail": "oidc_user_not_linked_to_discord_admin", - }, - status_code=403, + verifier = _discord_admin_verifier_from_app(request.app) + linked = await verifier.is_admin_email_for_discord_user( + email=email, + discord_user_id=grant.discord_user_id, + http_client=http_client, ) + if not linked: + await _write_auth_audit_event( + action="auth.login", + result=AuditResult.DENIED, + actor_subject=audit_actor_subject, + actor_display_name=display_name, + metadata={ + "reason": "oidc_user_not_linked_to_discord_admin", + "discord_user_id": grant.discord_user_id, + }, + correlation_id=state, + ) + return JSONResponse( + { + "error": "forbidden", + "detail": "oidc_user_not_linked_to_discord_admin", + }, + status_code=403, + ) + else: + # Discord deep links are already restricted to Discord admin users. + # In bootstrap mode, skip OIDC group/email-link checks for this path. + is_admin = True await store.delete_discord_link(pending.discord_link_token) @@ -1357,6 +1373,7 @@ async def auth_callback_handler( is_admin=is_admin, id_token=id_token, expires_at=expires_at, + actor_provider=ActorProvider.ADMIN_SSO.value, ), ttl_seconds=settings.auth_session_ttl_seconds, ) @@ -1367,16 +1384,22 @@ async def auth_callback_handler( ) response = RedirectResponse(url=redirect_to, status_code=302) _set_session_cookie(response, session_id) + login_audit_metadata: dict[str, Any] = { + "is_admin": is_admin, + "groups": groups, + "via_discord_link": bool(pending.discord_link_token), + } + if pending.discord_link_token: + login_audit_metadata["discord_link_identity_checks_enforced"] = ( + enforce_discord_link_identity_checks + ) + await _write_auth_audit_event( action="auth.login", result=AuditResult.SUCCESS, actor_subject=audit_actor_subject, actor_display_name=display_name, - metadata={ - "is_admin": is_admin, - "groups": groups, - "via_discord_link": bool(pending.discord_link_token), - }, + metadata=login_audit_metadata, resource_id=session_id, correlation_id=state, ) @@ -1399,6 +1422,7 @@ async def auth_me_handler(request: Request) -> JSONResponse: "groups": session.groups, "is_admin": session.is_admin, "expires_at": session.expires_at, + "actor_provider": session.actor_provider, } ) @@ -1411,11 +1435,16 @@ async def auth_logout_handler(request: Request) -> JSONResponse: await store.delete_session(session_id) if session is not None: + actor_provider = _session_actor_provider(session) + actor_subject = session.email or session.subject + if actor_provider == ActorProvider.DISCORD: + actor_subject = session.subject await _write_auth_audit_event( action="auth.logout", result=AuditResult.SUCCESS, - actor_subject=(session.email or session.subject), + actor_subject=actor_subject, actor_display_name=session.display_name, + actor_provider=actor_provider, metadata={"is_admin": session.is_admin}, resource_id=session_id, ) @@ -1520,7 +1549,7 @@ async def auth_discord_link_redirect_handler( request: Request, token: str, ) -> JSONResponse | RedirectResponse: - """Handle one-time Discord deep link and jump into OIDC login flow.""" + """Handle one-time Discord deep link and create or resume an admin session.""" store = _auth_store_from_app(request.app) if store is None: return JSONResponse({"error": "auth_not_ready"}, status_code=503) @@ -1530,6 +1559,56 @@ async def auth_discord_link_redirect_handler( return JSONResponse({"error": "link_not_found"}, status_code=404) _, session = await _current_session(request) + if not settings.discord_link_require_oidc_identity_checks: + verifier = _discord_admin_verifier_from_app(request.app) + http_client = _http_client_from_app(request.app) + identity = await verifier.resolve_admin_identity( + discord_user_id=grant.discord_user_id, + http_client=http_client, + ) + if identity is None: + return JSONResponse( + {"error": "forbidden", "detail": "discord_user_not_admin"}, + status_code=403, + ) + + session_id = secrets.token_urlsafe(32) + expires_at = int(time.time()) + max(1, settings.auth_session_ttl_seconds) + await store.save_session( + session_id=session_id, + payload=AuthSession( + subject=grant.discord_user_id, + email=identity.email, + display_name=identity.display_name, + groups=["discord_admin"], + is_admin=True, + id_token="", + expires_at=expires_at, + actor_provider=ActorProvider.DISCORD.value, + ), + ttl_seconds=settings.auth_session_ttl_seconds, + ) + await store.delete_discord_link(token) + + response = RedirectResponse(url=grant.next_path, status_code=302) + _set_session_cookie(response, session_id) + await _write_auth_audit_event( + action="auth.login", + result=AuditResult.SUCCESS, + actor_subject=grant.discord_user_id, + actor_display_name=identity.display_name, + actor_provider=ActorProvider.DISCORD, + metadata={ + "is_admin": True, + "groups": ["discord_admin"], + "via_discord_link": True, + "discord_link_identity_checks_enforced": False, + }, + resource_id=session_id, + correlation_id=token, + ) + return response + if session is not None: if not session.is_admin: return JSONResponse( @@ -1547,6 +1626,7 @@ async def auth_discord_link_redirect_handler( linked = await verifier.is_admin_email_for_discord_user( email=session.email, discord_user_id=grant.discord_user_id, + http_client=_http_client_from_app(request.app), ) if not linked: return JSONResponse( diff --git a/apps/api/src/five08/backend/auth.py b/apps/api/src/five08/backend/auth.py index 642b49b9..db6bb0a6 100644 --- a/apps/api/src/five08/backend/auth.py +++ b/apps/api/src/five08/backend/auth.py @@ -59,6 +59,7 @@ class AuthSession: is_admin: bool id_token: str expires_at: int + actor_provider: str = "admin_sso" @dataclass(frozen=True) @@ -69,6 +70,15 @@ class DiscordLinkGrant: next_path: str +@dataclass(frozen=True) +class DiscordAdminIdentity: + """Resolved CRM-backed Discord admin identity details.""" + + discord_user_id: str + email: str | None + display_name: str | None + + class RedisAuthStore: """Redis-backed storage for OIDC states, sessions, and Discord link grants.""" @@ -126,6 +136,7 @@ async def get_session(self, session_id: str) -> AuthSession | None: is_admin=bool(value.get("is_admin", False)), id_token=str(value["id_token"]), expires_at=int(value["expires_at"]), + actor_provider=str(value.get("actor_provider") or "admin_sso"), ) except Exception: logger.warning("Invalid auth session payload in Redis") @@ -468,7 +479,7 @@ def is_admin_from_groups( class DiscordAdminVerifier: - """Resolve whether a Discord user is admin from people cache with API fallback.""" + """Resolve whether a Discord user is an active CRM-backed Discord admin.""" def __init__(self, settings: WorkerSettings) -> None: self.settings = settings @@ -479,15 +490,41 @@ async def is_admin_discord_user( discord_user_id: str, http_client: httpx.AsyncClient, ) -> bool: - if await asyncio.to_thread( - self._is_admin_from_people_db, + identity = await self.resolve_admin_identity( + discord_user_id=discord_user_id, + http_client=http_client, + ) + return identity is not None + + async def resolve_admin_identity( + self, + *, + discord_user_id: str, + http_client: httpx.AsyncClient, + ) -> DiscordAdminIdentity | None: + """Return CRM-backed identity details when the Discord user is an admin.""" + person = await asyncio.to_thread( + self._get_active_person_record, discord_user_id, - ): - return True + ) + if person is None: + return None - return await self._is_admin_from_discord_api( + if not self._has_admin_role(person.get("discord_roles")): + is_live_admin = await self._is_admin_from_discord_api( + discord_user_id=discord_user_id, + http_client=http_client, + ) + if not is_live_admin: + return None + + email = _to_optional_str(person.get("email_508")) or _to_optional_str( + person.get("email") + ) + return DiscordAdminIdentity( discord_user_id=discord_user_id, - http_client=http_client, + email=email, + display_name=_to_optional_str(person.get("name")), ) async def is_admin_email_for_discord_user( @@ -495,37 +532,53 @@ async def is_admin_email_for_discord_user( *, email: str, discord_user_id: str, + http_client: httpx.AsyncClient | None = None, ) -> bool: """Check if OIDC email maps to an active admin row for this Discord user.""" normalized_email = email.strip().lower() if not normalized_email: return False - return await asyncio.to_thread( - self._is_admin_email_for_discord_user_sync, - normalized_email, + person = await asyncio.to_thread( + self._get_active_person_record, discord_user_id, + normalized_email, + ) + if person is None: + return False + if not self._email_matches_person(person, normalized_email): + return False + if self._has_admin_role(person.get("discord_roles")): + return True + if http_client is None: + return False + return await self._is_admin_from_discord_api( + discord_user_id=discord_user_id, + http_client=http_client, ) - def _is_admin_from_people_db(self, discord_user_id: str) -> bool: - role_names = self.settings.discord_admin_role_names + def _get_active_person_record( + self, + discord_user_id: str, + normalized_email: str | None = None, + ) -> dict[str, Any] | None: query = """ - SELECT discord_roles + SELECT name, email, email_508, discord_roles FROM people WHERE sync_status = 'active' AND discord_user_id = %s - LIMIT 1; """ + params: list[str] = [discord_user_id] + if normalized_email is not None: + query += " AND (lower(email) = %s OR lower(email_508) = %s)" + params.extend([normalized_email, normalized_email]) + query += " LIMIT 1;" + with get_postgres_connection(self.settings) as conn: with conn.cursor(row_factory=dict_row) as cursor: - cursor.execute(query, (discord_user_id,)) + cursor.execute(query, tuple(params)) row = cursor.fetchone() - if row is None: - return False - - raw_roles = row.get("discord_roles") - parsed_roles = {role.casefold() for role in _to_string_list(raw_roles)} - return bool(parsed_roles & role_names) + return row async def _is_admin_from_discord_api( self, @@ -586,32 +639,18 @@ async def _is_admin_from_discord_api( return bool(member_role_names & role_names) - def _is_admin_email_for_discord_user_sync( - self, - email: str, - discord_user_id: str, - ) -> bool: + def _has_admin_role(self, raw_roles: object) -> bool: role_names = self.settings.discord_admin_role_names - query = """ - SELECT discord_roles - FROM people - WHERE sync_status = 'active' - AND discord_user_id = %s - AND (lower(email) = %s OR lower(email_508) = %s) - LIMIT 1; - """ - with get_postgres_connection(self.settings) as conn: - with conn.cursor(row_factory=dict_row) as cursor: - cursor.execute(query, (discord_user_id, email, email)) - row = cursor.fetchone() - - if row is None: - return False - - raw_roles = row.get("discord_roles") parsed_roles = {role.casefold() for role in _to_string_list(raw_roles)} return bool(parsed_roles & role_names) + @staticmethod + def _email_matches_person(person: dict[str, Any], email: str) -> bool: + return email in { + (_to_optional_str(person.get("email")) or "").lower(), + (_to_optional_str(person.get("email_508")) or "").lower(), + } + def _to_optional_str(value: object) -> str | None: if value is None: diff --git a/apps/discord_bot/README.md b/apps/discord_bot/README.md index 29bd7594..5ae01dbf 100644 --- a/apps/discord_bot/README.md +++ b/apps/discord_bot/README.md @@ -18,6 +18,13 @@ This document captures Discord bot behavior, permissions, and slash command usag ## Slash Commands +- `/login` + - Description: Generate a one-time admin dashboard login link. + - Required role: any role listed in `DISCORD_ADMIN_ROLES` (`Admin,Owner` by default). + - Behavior: + - Calls backend `POST /auth/discord/links` using `API_SHARED_SECRET`. + - Returns an ephemeral one-time URL with expiry. + - `/create-mailbox` - Description: Create a Migadu mailbox for a 508 user and email the invitation to a backup address. - Prerequisites: `MIGADU_API_USER` and `MIGADU_API_KEY` must be configured (configured in env; command will fail if missing). diff --git a/apps/discord_bot/src/five08/discord_bot/cogs/admin_login.py b/apps/discord_bot/src/five08/discord_bot/cogs/admin_login.py new file mode 100644 index 00000000..02a06160 --- /dev/null +++ b/apps/discord_bot/src/five08/discord_bot/cogs/admin_login.py @@ -0,0 +1,184 @@ +"""Discord slash command for one-time admin dashboard login links.""" + +from __future__ import annotations + +import logging +from typing import Any + +import aiohttp +import discord +from discord import app_commands +from discord.ext import commands + +from five08.discord_bot.config import settings +from five08.discord_bot.utils.audit import DiscordAuditLogger + +logger = logging.getLogger(__name__) + + +class AdminLoginCog(commands.Cog): + """Mint one-time dashboard login links for Discord admins.""" + + def __init__(self, bot: commands.Bot) -> None: + self.bot = bot + self.audit_logger = DiscordAuditLogger( + base_url=settings.audit_api_base_url, + shared_secret=settings.api_shared_secret, + timeout_seconds=settings.audit_api_timeout_seconds, + discord_logs_webhook_url=settings.discord_logs_webhook_url, + discord_logs_webhook_wait=settings.discord_logs_webhook_wait, + ) + + def _backend_url(self, path: str) -> str: + return f"{settings.backend_api_base_url.rstrip('/')}{path}" + + def _backend_headers(self) -> dict[str, str]: + if not settings.api_shared_secret: + raise ValueError("API_SHARED_SECRET is required for backend API requests.") + return { + "X-API-Secret": settings.api_shared_secret, + "Content-Type": "application/json", + } + + @staticmethod + def _user_can_request_login_link(interaction: discord.Interaction) -> bool: + member_roles = getattr(interaction.user, "roles", None) + if member_roles is None: + return False + + allowed_role_names = settings.discord_admin_role_names + user_role_names = { + role.name.casefold() + for role in member_roles + if isinstance(role.name, str) and role.name.strip() + } + return bool(user_role_names & allowed_role_names) + + async def _create_login_link(self, *, discord_user_id: str) -> tuple[str, int]: + payload = {"discord_user_id": discord_user_id} + async with aiohttp.ClientSession() as session: + async with session.post( + self._backend_url("/auth/discord/links"), + headers=self._backend_headers(), + json=payload, + timeout=aiohttp.ClientTimeout(total=30), + ) as response: + data: Any = {} + try: + data = await response.json() + except Exception: + data = {} + + if response.status == 201 and isinstance(data, dict): + link_url = data.get("link_url") + expires_in_seconds = data.get("expires_in_seconds") + if ( + isinstance(link_url, str) + and link_url + and isinstance(expires_in_seconds, int) + ): + return link_url, expires_in_seconds + raise RuntimeError("Missing link_url or expires_in_seconds.") + + if response.status == 403 and isinstance(data, dict): + detail = data.get("detail") + if detail == "discord_user_not_admin": + raise PermissionError("discord_user_not_admin") + + raise RuntimeError( + f"Failed creating login link: status={response.status}" + ) + + def _audit( + self, + *, + interaction: discord.Interaction, + result: str, + metadata: dict[str, Any] | None = None, + ) -> None: + self.audit_logger.log_command( + interaction=interaction, + action="auth.login_link.create", + result=result, + metadata=metadata or {}, + resource_type="admin_dashboard_link", + resource_id=str(interaction.user.id), + ) + + @app_commands.command( + name="login", + description="Get a one-time admin dashboard login link.", + ) + async def login(self, interaction: discord.Interaction) -> None: + """Create and return a one-time dashboard login URL.""" + if not self._user_can_request_login_link(interaction): + self._audit( + interaction=interaction, + result="denied", + metadata={"reason": "discord_user_not_admin"}, + ) + await interaction.response.send_message( + "❌ You are not allowed to create an admin dashboard login link.", + ephemeral=True, + ) + return + + await interaction.response.defer(ephemeral=True) + + try: + link_url, expires_in_seconds = await self._create_login_link( + discord_user_id=str(interaction.user.id), + ) + except ValueError as exc: + self._audit( + interaction=interaction, + result="error", + metadata={"reason": "missing_api_secret", "error": str(exc)}, + ) + await interaction.followup.send( + "❌ Login link service is not configured yet (missing API secret).", + ephemeral=True, + ) + return + except PermissionError: + self._audit( + interaction=interaction, + result="denied", + metadata={"reason": "discord_user_not_admin"}, + ) + await interaction.followup.send( + "❌ You are not allowed to create an admin dashboard login link.", + ephemeral=True, + ) + return + except Exception as exc: + logger.error("Failed creating dashboard login link: %s", exc) + self._audit( + interaction=interaction, + result="error", + metadata={"reason": "link_create_failed", "error": str(exc)}, + ) + await interaction.followup.send( + "❌ Failed to create login link. Please try again in a minute.", + ephemeral=True, + ) + return + + self._audit( + interaction=interaction, + result="success", + metadata={"expires_in_seconds": expires_in_seconds}, + ) + await interaction.followup.send( + ( + "✅ One-time admin dashboard login link:\n" + f"{link_url}\n\n" + f"Expires in {expires_in_seconds} seconds." + ), + ephemeral=True, + ) + + +async def setup(bot: commands.Bot) -> None: + """Load the admin login command cog.""" + await bot.add_cog(AdminLoginCog(bot)) diff --git a/apps/discord_bot/src/five08/discord_bot/config.py b/apps/discord_bot/src/five08/discord_bot/config.py index ad66ecae..617f691b 100644 --- a/apps/discord_bot/src/five08/discord_bot/config.py +++ b/apps/discord_bot/src/five08/discord_bot/config.py @@ -19,6 +19,7 @@ class Settings(SharedSettings): discord_bot_token: str discord_sendmsg_character_limit: int = 2000 + discord_admin_roles: str = "Admin,Owner" # Healthcheck Configuration healthcheck_port: int = 3000 @@ -40,5 +41,11 @@ class Settings(SharedSettings): kimai_base_url: str kimai_api_token: str + @property + def discord_admin_role_names(self) -> set[str]: + """Lower-cased configured Discord admin role names.""" + values = [item.strip() for item in self.discord_admin_roles.split(",")] + return {value.casefold() for value in values if value} + settings = Settings() # type: ignore[call-arg] diff --git a/apps/worker/README.md b/apps/worker/README.md index c9680475..91c3c2aa 100644 --- a/apps/worker/README.md +++ b/apps/worker/README.md @@ -119,6 +119,13 @@ PY - `GET /auth/discord/link/{token}`: Resolve Discord deep link into authenticated dashboard redirect. - Auth flows emit best-effort human audit events (`auth.login`, `auth.logout`) under source `admin_dashboard`. +Discord deep-link identity policy: + +- `DISCORD_ADMIN_ROLES` controls who can mint/use Discord deep links (`Admin,Owner` recommended). +- `OIDC_ADMIN_GROUPS` controls normal OIDC dashboard admin membership (`authentik Admins` recommended). +- `DISCORD_LINK_REQUIRE_OIDC_IDENTITY_CHECKS=true` (default): Discord deep links also require OIDC admin group + OIDC email linked to Discord admin identity. +- `DISCORD_LINK_REQUIRE_OIDC_IDENTITY_CHECKS=false`: bootstrap mode; Discord deep links skip OIDC group/email-link checks after successful OIDC authentication. + ### Current API/Worker behavior - Worker queue configuration resolves to one effective queue via `WORKER_QUEUE_NAMES`. diff --git a/apps/worker/src/five08/worker/config.py b/apps/worker/src/five08/worker/config.py index 63cf0997..af0830fb 100644 --- a/apps/worker/src/five08/worker/config.py +++ b/apps/worker/src/five08/worker/config.py @@ -66,7 +66,7 @@ def worker_queue_name(self) -> str: oidc_client_secret: str = "" oidc_scope: str = "openid profile email groups" oidc_groups_claim: str = "groups" - oidc_admin_groups: str = "Admin,Owner,Steering Committee" + oidc_admin_groups: str = "authentik Admins" oidc_callback_path: str = "/auth/callback" oidc_redirect_base_url: str | None = None oidc_http_timeout_seconds: float = 8.0 @@ -80,9 +80,10 @@ def worker_queue_name(self) -> str: dashboard_public_base_url: str | None = None discord_bot_token: str | None = None discord_admin_guild_id: str | None = None - discord_admin_roles: str = "Admin,Owner,Steering Committee" + discord_admin_roles: str = "Admin,Owner" discord_api_timeout_seconds: float = 8.0 discord_link_ttl_seconds: int = 600 + discord_link_require_oidc_identity_checks: bool = True @property def google_forms_allowed_form_ids_set(self) -> set[str]: diff --git a/tests/unit/test_admin_login_cog.py b/tests/unit/test_admin_login_cog.py new file mode 100644 index 00000000..ca063a30 --- /dev/null +++ b/tests/unit/test_admin_login_cog.py @@ -0,0 +1,103 @@ +"""Unit tests for the admin login Discord cog.""" + +from unittest.mock import AsyncMock, Mock, patch + +import pytest + +from five08.discord_bot.cogs.admin_login import AdminLoginCog, settings + + +@pytest.fixture +def mock_interaction() -> AsyncMock: + interaction = AsyncMock() + interaction.response = AsyncMock() + interaction.response.defer = AsyncMock() + interaction.response.send_message = AsyncMock() + interaction.followup = AsyncMock() + interaction.followup.send = AsyncMock() + interaction.user = Mock() + interaction.user.id = 123456789 + role = Mock() + role.name = "Admin" + interaction.user.roles = [role] + return interaction + + +@pytest.fixture +def cog() -> AdminLoginCog: + return AdminLoginCog(Mock()) + + +@pytest.mark.asyncio +async def test_login_command_returns_link( + cog: AdminLoginCog, mock_interaction: AsyncMock +) -> None: + with ( + patch.object( + cog, + "_create_login_link", + new=AsyncMock( + return_value=("https://dash.508.dev/auth/discord/link/token", 600) + ), + ), + patch.object(cog, "_audit") as mock_audit, + ): + await cog.login.callback(cog, mock_interaction) + + mock_interaction.response.defer.assert_awaited_once_with(ephemeral=True) + mock_interaction.followup.send.assert_awaited_once() + sent_message = mock_interaction.followup.send.call_args.args[0] + assert "https://dash.508.dev/auth/discord/link/token" in sent_message + mock_audit.assert_called_once() + + +@pytest.mark.asyncio +async def test_login_command_denied_when_user_not_admin( + cog: AdminLoginCog, mock_interaction: AsyncMock +) -> None: + with ( + patch.object( + cog, + "_create_login_link", + new=AsyncMock(side_effect=PermissionError("discord_user_not_admin")), + ), + patch.object(cog, "_audit"), + ): + await cog.login.callback(cog, mock_interaction) + + sent_message = mock_interaction.followup.send.call_args.args[0] + assert "not allowed" in sent_message + + +@pytest.mark.asyncio +async def test_login_command_handles_missing_secret( + cog: AdminLoginCog, mock_interaction: AsyncMock +) -> None: + with ( + patch.object( + cog, + "_create_login_link", + new=AsyncMock(side_effect=ValueError("missing secret")), + ), + patch.object(cog, "_audit"), + ): + await cog.login.callback(cog, mock_interaction) + + sent_message = mock_interaction.followup.send.call_args.args[0] + assert "not configured" in sent_message + + +@pytest.mark.asyncio +async def test_login_command_uses_configured_admin_roles( + monkeypatch: pytest.MonkeyPatch, + cog: AdminLoginCog, + mock_interaction: AsyncMock, +) -> None: + monkeypatch.setattr(settings, "discord_admin_roles", "Operations") + mock_interaction.user.roles[0].name = "Admin" + + with patch.object(cog, "_create_login_link", new=AsyncMock()) as mock_create: + await cog.login.callback(cog, mock_interaction) + + mock_interaction.response.send_message.assert_awaited_once() + mock_create.assert_not_awaited() diff --git a/tests/unit/test_backend_api.py b/tests/unit/test_backend_api.py index b71c0168..dcb733f3 100644 --- a/tests/unit/test_backend_api.py +++ b/tests/unit/test_backend_api.py @@ -702,6 +702,8 @@ def test_auth_callback_success_writes_login_audit(client: TestClient) -> None: assert audit_payload.source == api.AuditSource.ADMIN_DASHBOARD assert audit_payload.actor_provider == api.ActorProvider.ADMIN_SSO assert audit_payload.actor_subject == "admin@508.dev" + assert audit_payload.metadata is not None + assert "discord_link_identity_checks_enforced" not in audit_payload.metadata def test_auth_callback_denied_writes_login_audit(client: TestClient) -> None: @@ -753,6 +755,114 @@ def test_auth_callback_denied_writes_login_audit(client: TestClient) -> None: assert audit_payload.actor_subject == "member@508.dev" +def test_auth_callback_discord_link_can_skip_oidc_identity_checks( + monkeypatch: pytest.MonkeyPatch, + client: TestClient, +) -> None: + monkeypatch.setattr( + api.settings, "discord_link_require_oidc_identity_checks", False + ) + store = Mock() + store.pop_oidc_state = AsyncMock( + return_value=api.PendingOIDCState( + nonce="nonce-1", + code_verifier="verifier-1", + next_path="/dashboard", + discord_link_token="link-1", + ) + ) + store.get_discord_link = AsyncMock( + return_value=api.DiscordLinkGrant( + discord_user_id="123456789", + next_path="/dashboard", + ) + ) + store.delete_discord_link = AsyncMock() + store.save_session = AsyncMock() + + oidc = Mock() + oidc.configured = True + oidc.exchange_code = AsyncMock(return_value={"id_token": "id-token-1"}) + oidc.validate_id_token = AsyncMock( + return_value={ + "sub": "authentik-user-4", + "name": "Bootstrap User", + "groups": ["not-admin-yet"], + "exp": 4_102_444_800, + } + ) + + with ( + patch("five08.backend.api._auth_store_from_app", return_value=store), + patch("five08.backend.api._oidc_client_from_app", return_value=oidc), + patch("five08.backend.api._http_client_from_app", return_value=Mock()), + patch("five08.backend.api.insert_audit_event") as mock_insert, + ): + response = client.get( + "/auth/callback?code=code-1&state=state-1", + follow_redirects=False, + ) + + assert response.status_code == 302 + saved_session = store.save_session.call_args.kwargs["payload"] + assert saved_session.is_admin is True + store.delete_discord_link.assert_awaited_once_with("link-1") + audit_payload = mock_insert.call_args.args[1] + assert audit_payload.metadata is not None + assert audit_payload.metadata["via_discord_link"] is True + assert audit_payload.metadata["discord_link_identity_checks_enforced"] is False + + +def test_auth_discord_link_redirect_creates_discord_session_when_disabled( + monkeypatch: pytest.MonkeyPatch, + client: TestClient, +) -> None: + monkeypatch.setattr( + api.settings, "discord_link_require_oidc_identity_checks", False + ) + store = Mock() + store.get_discord_link = AsyncMock( + return_value=api.DiscordLinkGrant( + discord_user_id="123456789", + next_path="/dashboard", + ) + ) + store.save_session = AsyncMock() + store.delete_discord_link = AsyncMock() + verifier = Mock() + verifier.resolve_admin_identity = AsyncMock( + return_value=Mock( + discord_user_id="123456789", + email="admin@508.dev", + display_name="Discord Admin", + ) + ) + + with ( + patch("five08.backend.api._auth_store_from_app", return_value=store), + patch( + "five08.backend.api._discord_admin_verifier_from_app", + return_value=verifier, + ), + patch("five08.backend.api._http_client_from_app", return_value=Mock()), + patch("five08.backend.api.insert_audit_event") as mock_insert, + ): + response = client.get("/auth/discord/link/link-1", follow_redirects=False) + + assert response.status_code == 302 + assert response.headers["location"] == "/dashboard" + store.delete_discord_link.assert_awaited_once_with("link-1") + saved_session = store.save_session.call_args.kwargs["payload"] + assert saved_session.subject == "123456789" + assert saved_session.actor_provider == api.ActorProvider.DISCORD.value + assert saved_session.email == "admin@508.dev" + audit_payload = mock_insert.call_args.args[1] + assert audit_payload.actor_provider == api.ActorProvider.DISCORD + assert audit_payload.actor_subject == "123456789" + assert audit_payload.metadata is not None + assert audit_payload.metadata["discord_link_identity_checks_enforced"] is False + + def test_auth_logout_writes_logout_audit(client: TestClient) -> None: store = Mock() store.delete_session = AsyncMock() diff --git a/tests/unit/test_worker_config.py b/tests/unit/test_worker_config.py index 0f7ca0d6..d31f0f71 100644 --- a/tests/unit/test_worker_config.py +++ b/tests/unit/test_worker_config.py @@ -101,6 +101,24 @@ def test_google_forms_allowed_form_ids_parses_as_set() -> None: assert settings.google_forms_allowed_form_ids_set == {"form-1", "form-2", "form-3"} +def test_oidc_admin_groups_default_matches_authentik_admins() -> None: + settings = WorkerSettings( + espo_base_url="https://crm.test.com", + espo_api_key="test-key", + ) + + assert settings.oidc_admin_group_names == {"authentik admins"} + + +def test_discord_admin_roles_default_is_admin_owner() -> None: + settings = WorkerSettings( + espo_base_url="https://crm.test.com", + espo_api_key="test-key", + ) + + assert settings.discord_admin_role_names == {"admin", "owner"} + + def test_intake_resume_fetch_timeout_must_be_positive() -> None: with pytest.raises(ValidationError): WorkerSettings(