From 3ff41ca354ff7dd881e894aa65fb7ad91030330f Mon Sep 17 00:00:00 2001 From: Leo Vasanko Date: Mon, 7 Sep 2026 07:53:27 +0000 Subject: [PATCH] Review fixes: validation hardening, dead code, stale comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - validate_config: reject multiple auth-host marks per domain; sanitize_config clears extras (first wins) and coerces junk entry values to presence-only - origin_key: lowercase keys, strip trailing dots (bare hosts/wildcards) - Passkey._allowlisted: tolerate trailing-dot wildcard bases - wschat: stamp remote-flow sessions with the session host's domain, not the approver's - auth_host redirects: keep the port (redirect to the configured auth host instead of the normalized, port-less current host) - update_domain: required fields (wholesale replace) — no silent wipes - admin: fix pre-existing lockout-guard order in org permission removal; permission PATCH keeps domain restriction when omitted; 400 instead of 500 on unknown permission UUIDs - Drop dead code: db.update_config/set_session_host/delete_reset_token, Session.metadata, oidjwt.clear_key, background aliases, avatar.current_avatar_url/media_root, wsutil.require_pow - Prune stale/duplicated comments and docstrings --- e2e/README.md | 25 +++++-- e2e/tests/50-multidomain.spec.ts | 112 +++++++++++++++++++++++++++- paskia/__main__.py | 9 +-- paskia/bootstrap.py | 2 +- paskia/db/__init__.py | 6 -- paskia/db/background.py | 5 -- paskia/db/operations.py | 34 ++------- paskia/db/structs.py | 31 +++----- paskia/domains.py | 46 ++++++++++-- paskia/fastapi/admin/orgs.py | 10 +-- paskia/fastapi/admin/permissions.py | 9 ++- paskia/fastapi/api.py | 2 - paskia/fastapi/auth_host.py | 12 ++- paskia/fastapi/dispatch.py | 3 +- paskia/fastapi/oid.py | 7 +- paskia/fastapi/remote.py | 2 +- paskia/fastapi/session.py | 2 - paskia/fastapi/wschat.py | 7 +- paskia/fastapi/wsutil.py | 41 ---------- paskia/sansio.py | 2 +- paskia/util/apistructs.py | 3 +- paskia/util/avatar.py | 17 +---- paskia/util/oidjwt.py | 6 -- paskia/util/startupbox.py | 7 +- 24 files changed, 224 insertions(+), 176 deletions(-) diff --git a/e2e/README.md b/e2e/README.md index e946d2c..9ce8faf 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -75,18 +75,22 @@ Runs tests with Playwright Inspector for step-by-step debugging. ``` e2e/ -├── playwright.config.ts # Playwright configuration +├── playwright.config.js # Playwright configuration ├── package.json ├── tsconfig.json ├── test-data/ # Test database (created at runtime) -│ └── test.sqlite +│ └── paskia.kantadb └── tests/ - ├── global-setup.ts # Creates fresh DB, captures reset token + ├── global-setup.ts # Creates fresh DB (localhost + test.localhost domains), captures reset token ├── global-teardown.ts # Cleanup - ├── passkey.spec.ts # Main E2E tests + ├── 10-passkey.spec.ts # Registration, authentication, session tests + ├── 20-api-auth.spec.ts # API-mode iframe flows (401/403/reauth) + ├── 50-multidomain.spec.ts# Multi-domain dispatch, related origins, auth hosts, remote login + ├── 99-logout.spec.ts # Logout (runs last) └── fixtures/ ├── virtual-authenticator.ts # Virtual authenticator setup - └── passkey-helpers.ts # WebSocket helpers + ├── passkey-helpers.ts # WebSocket helpers + └── remote-auth.ts # Pairing-code remote auth helpers ``` ## What's Tested @@ -107,6 +111,13 @@ e2e/ - Logout (`/auth/api/logout`) - Invalid/missing token rejection +### Multi-Domain +- Host-based domain dispatch (`localhost` vs `test.localhost`, 421 for unknown hosts) +- Related Origin Requests well-known endpoint and admin domain API +- Per-domain auth hosts (UI at the site root) +- WebSocket cross-domain rules +- Cross-domain remote login via pairing code + ## How Virtual Authenticator Works The tests use Chrome DevTools Protocol (CDP) to create a virtual authenticator: @@ -142,12 +153,12 @@ This creates an in-browser authenticator that: ## Limitations 1. **Chromium only**: Virtual authenticator is a Chrome DevTools feature -2. **No cross-origin**: Tests run on localhost; production-like origins need additional setup +2. **Multi-domain via `*.localhost`**: Chrome resolves any `*.localhost` hostname to loopback, which the tests use for cross-domain scenarios; non-localhost domains are exercised only via explicit Host headers (Node-side requests) 3. **Single user per run**: Bootstrap creates one admin user; additional users need admin API ## Debugging Tips -1. **Check test database**: `e2e/test-data/test.sqlite` persists after tests +1. **Check test database**: `e2e/test-data/paskia.kantadb` is removed during teardown; comment out the cleanup in `global-teardown.ts` to inspect it after a run 2. **View server output**: Global setup echoes server bootstrap to console 3. **Use trace viewer**: `npx playwright show-trace` on failure traces diff --git a/e2e/tests/50-multidomain.spec.ts b/e2e/tests/50-multidomain.spec.ts index d54c43f..635de15 100644 --- a/e2e/tests/50-multidomain.spec.ts +++ b/e2e/tests/50-multidomain.spec.ts @@ -19,7 +19,11 @@ import { * * Covers: * - Host-based domain dispatch (settings, 421 for unknown hosts) - * - Related Origin Requests well-known endpoint + admin domain API + * - Related Origin Requests well-known endpoint + admin domain API, + * including HTTP dispatch to a related hostname + * - Per-domain auth hosts: settings, UI at the site root, /auth/ redirect + * - WebSocket cross-domain rule: rejected unless the Host is the origin + * domain's own auth host * - Cross-domain remote login: a passkey registered on localhost permits a * session on test.localhost via pairing code * - The profile enrollment prompt on a domain where the user has no passkey @@ -38,11 +42,15 @@ test.describe('Multi-domain E2E', () => { const domainSettings = await domainResp?.json() expect(domainSettings.rp_id).toBe('test.localhost') expect(domainSettings.own_auth_host).toBeNull() + expect(domainSettings.auth_host).toBeNull() + expect(domainSettings.ui_base_path).toBe('/auth/') const defaultResp = await page.goto(`${baseUrl}/auth/api/settings`) expect(defaultResp?.status()).toBe(200) const defaultSettings = await defaultResp?.json() expect(defaultSettings.rp_id).toBe('localhost') + expect(defaultSettings.auth_host).toBeNull() + expect(defaultSettings.ui_base_path).toBe('/auth/') // Unknown host is rejected with 421 Misdirected Request. // page.request is Node-side, so target loopback with an explicit Host. @@ -89,6 +97,14 @@ test.describe('Multi-domain E2E', () => { const wkJson = await wk.json() expect(wkJson.origins).toContain('https://app.example.com') + // The related hostname now dispatches to the listing domain (HTTP). + // page.request is Node-side, so target loopback with an explicit Host. + const relResp = await page.request.get(`${baseUrl}/auth/api/settings`, { + headers: { Host: 'app.example.com' }, + }) + expect(relResp.ok()).toBeTruthy() + expect((await relResp.json()).rp_id).toBe('localhost') + // Restore: remove related origins again so later tests see the pristine state const restore = await page.request.patch(`${baseUrl}/auth/api/admin/domains/localhost`, { headers, @@ -97,6 +113,100 @@ test.describe('Multi-domain E2E', () => { expect(restore.ok()).toBeTruthy() const after = await page.request.get(`${baseUrl}/.well-known/webauthn`) expect(after.status()).toBe(404) + + // ...and the related hostname is unknown again + const relGone = await page.request.get(`${baseUrl}/auth/api/settings`, { + headers: { Host: 'app.example.com' }, + }) + expect(relGone.status()).toBe(421) + }) + + test('per-domain auth host serves the domain UI at its site root', async ({ page, virtualAuthenticator }) => { + // Fresh session via device token (domain writes require recent auth) + const deviceToken = popDeviceToken() + test.skip(!deviceToken, 'No device tokens available') + await page.goto('/auth/') + const reg = await registerPasskey(page, baseUrl, { resetToken: deviceToken }) + expect(reg.session_token).toBeTruthy() + + const headers = { Cookie: `${getSessionCookieName()}=${reg.session_token}` } + const authHost = 'auth.test.localhost:4404' + + try { + // Mark an auth host on the test.localhost domain. Chrome resolves any + // *.localhost hostname to loopback, so the auth host is reachable. + const patch = await page.request.patch(`${baseUrl}/auth/api/admin/domains/test.localhost`, { + headers, + data: { rp_name: '', origins: { [`http://${authHost}`]: { auth_host: true } }, related: {} }, + }) + expect(patch.ok()).toBeTruthy() + + // The auth host dispatches to its domain and reports itself in settings + const settingsResp = await page.goto(`http://${authHost}/auth/api/settings`) + expect(settingsResp?.status()).toBe(200) + const settings = await settingsResp?.json() + expect(settings.rp_id).toBe('test.localhost') + expect(settings.auth_host).toBe(authHost) + expect(settings.own_auth_host).toBe(authHost) + expect(settings.ui_base_path).toBe('/') + + // The UI lives at the site root on the auth host + const rootResp = await page.goto(`http://${authHost}/`) + expect(rootResp?.status()).toBe(200) + expect(rootResp?.headers()['content-type']).toContain('text/html') + + // /auth/ on the auth host redirects to the root + const redir = await page.request.get(`${baseUrl}/auth/`, { + headers: { Host: authHost }, + maxRedirects: 0, + }) + expect(redir.status()).toBe(307) + expect(redir.headers()['location']).toMatch(/^http:\/\/auth\.test\.localhost(:\d+)?\/$/) + } finally { + // Restore: no origins, no auth host (later tests expect pristine state) + const restore = await page.request.patch(`${baseUrl}/auth/api/admin/domains/test.localhost`, { + headers, + data: { rp_name: '', origins: {}, related: {} }, + }) + expect(restore.ok()).toBeTruthy() + } + + const after = await page.request.get(`${baseUrl}/auth/api/settings`, { + headers: { Host: 'test.localhost:4404' }, + }) + expect((await after.json()).auth_host).toBeNull() + }) + + test('WebSocket cross-domain connections require the origin domain\'s own auth host', async ({ page }) => { + await page.goto(`${domainUrl}/auth/`) + + // Same-domain WebSocket receives authentication options... + const sameDomain: any = await page.evaluate(async () => { + return new Promise((resolve) => { + const ws = new WebSocket(`ws://${location.host}/auth/ws/authenticate`) + const timer = setTimeout(() => { ws.close(); resolve({ message: false }) }, 5000) + ws.onmessage = () => { clearTimeout(timer); ws.close(); resolve({ message: true }) } + ws.onerror = () => { clearTimeout(timer); resolve({ message: false }) } + }) + }) + expect(sameDomain.message).toBe(true) + + // ...but a cross-domain connection is closed pre-accept: test.localhost + // has no auth host of its own, so no other host may serve its logins + const crossDomain: any = await page.evaluate(async (host) => { + return new Promise((resolve) => { + const ws = new WebSocket(`ws://${host}/auth/ws/authenticate`) + let message = false + const timer = setTimeout(() => { ws.close(); resolve({ message, code: -1 }) }, 5000) + ws.onmessage = () => { message = true } + ws.onclose = (event) => { + clearTimeout(timer) + resolve({ message, code: event.code, wasClean: event.wasClean }) + } + }) + }, new URL(baseUrl).host) + expect(crossDomain.message).toBe(false) + expect(crossDomain.wasClean).toBe(false) }) test('cross-domain remote login via pairing code', async ({ page, virtualAuthenticator }) => { diff --git a/paskia/__main__.py b/paskia/__main__.py index 3bcf134..3314fa2 100644 --- a/paskia/__main__.py +++ b/paskia/__main__.py @@ -199,13 +199,8 @@ def cmd_serve(args: argparse.Namespace) -> None: registry = build_registry(config) except ValueError as e: raise SystemExit(f"Invalid stored configuration: {e}") from e - for warning in registry.warnings: - # Serving is best-effort; fixing the stored config is the admin's - # job via the admin interface on any working domain. - print( - f"⚠️ Config problem (fix via the admin interface): {warning}", - file=sys.stderr, - ) + # Sanitization warnings (serving is best-effort; fixing the stored config + # is the admin's job via the admin interface) are logged by build(). # Pass process-global serve parameters to the server process(es) os.environ["PASKIA_CONFIG"] = msgspec.json.encode( diff --git a/paskia/bootstrap.py b/paskia/bootstrap.py index a2bd6a0..c000318 100644 --- a/paskia/bootstrap.py +++ b/paskia/bootstrap.py @@ -5,7 +5,7 @@ The initial database seeding (admin user, organization, permissions, registration reset token) is performed by ``paskia init`` via :func:`paskia.db.bootstrap.bootstrap`. This module provides the serve-time check that re-prints a registration link when the admin user still has no -passkey under the default domain. +passkey on any configured domain. """ import logging diff --git a/paskia/db/__init__.py b/paskia/db/__init__.py index c1c3967..949cc96 100644 --- a/paskia/db/__init__.py +++ b/paskia/db/__init__.py @@ -37,7 +37,6 @@ from paskia.db.operations import ( delete_oid_client, delete_org, delete_permission, - delete_reset_token, delete_role, delete_session, delete_sessions_for_user, @@ -48,8 +47,6 @@ from paskia.db.operations import ( remove_permission_from_org, remove_permission_from_role, reset_oid_client_secret, - set_session_host, - update_config, update_credential_sign_count, update_domain, update_oid_client, @@ -117,7 +114,6 @@ __all__ = [ "delete_org", "delete_permission", "delete_domain", - "delete_reset_token", "delete_role", "delete_session", "delete_sessions_for_user", @@ -126,8 +122,6 @@ __all__ = [ "oidc_login", "remove_permission_from_org", "remove_permission_from_role", - "set_session_host", - "update_config", "update_credential_sign_count", "update_org_name", "update_permission", diff --git a/paskia/db/background.py b/paskia/db/background.py index b819fa5..85538ff 100644 --- a/paskia/db/background.py +++ b/paskia/db/background.py @@ -73,8 +73,3 @@ async def stop_background(): except asyncio.CancelledError: pass _background_task = None - - -# Aliases for backwards compatibility -start_cleanup = start_background -stop_cleanup = stop_background diff --git a/paskia/db/operations.py b/paskia/db/operations.py index 904dff7..976b2f2 100644 --- a/paskia/db/operations.py +++ b/paskia/db/operations.py @@ -1,7 +1,7 @@ """ Database for WebAuthn passkey authentication. -Read operations: Access _db directly, use build_* helpers to get public structs. +Read operations: Access _db directly. Context lookup: _db.session_ctx() returns full SessionContext with effective permissions. Write operations: Functions that validate and commit, or raise ValueError. """ @@ -18,7 +18,6 @@ from paskia.config import SESSION_LIFETIME from paskia.db.structs import ( DB, Client, - Config, Credential, DomainConfig, Org, @@ -78,12 +77,6 @@ def is_username_taken(username: str, exclude_uuid: UUID | None = None) -> bool: # ------------------------------------------------------------------------- -def update_config(config: Config) -> None: - """Update the stored configuration.""" - with _transaction("update_config"): - _db.config = config - - def create_permission(perm: Permission, *, ctx: SessionContext | None = None) -> None: """Create a new permission.""" if perm.uuid in _db.permissions: @@ -485,11 +478,6 @@ def update_session( s.issuer = issuer -def set_session_host(key: str, host: str, *, ctx: SessionContext | None = None) -> None: - """Set the host for a session (first-time binding).""" - update_session(key, host=host, ctx=ctx) - - def delete_session( key: str, *, ctx: SessionContext | None = None, action: str = "delete_session" ) -> None: @@ -558,14 +546,6 @@ def create_reset_token( return passphrase -def delete_reset_token(key: bytes, *, ctx: SessionContext | None = None) -> None: - """Delete a reset token.""" - if key not in _db.reset_tokens: - raise ValueError("Reset token not found") - with _transaction("delete_reset_token", ctx): - _db.reset_tokens[key].delete() - - # ------------------------------------------------------------------------- # Composite operations (used by app code) # ------------------------------------------------------------------------- @@ -732,12 +712,12 @@ def create_domain( def update_domain( rp_id: str, *, - rp_name: str | None = None, - origins: dict[str, bool | OriginEntry] | None = None, - related: dict[str, bool] | None = None, + rp_name: str | None, + origins: dict[str, bool | OriginEntry], + related: dict[str, bool], ctx: SessionContext | None = None, ) -> None: - """Update a domain's rp_name, origins and related origins. + """Replace a domain's rp_name, origins and related origins (wholesale). The rp-id itself is immutable: credentials are stamped with it, so changing it would orphan them — delete and recreate the domain instead. @@ -748,8 +728,8 @@ def update_domain( raise ValueError(f"Domain {rp_id} not found") with _transaction("admin:update_domain", ctx): domain.rp_name = rp_name - domain.origins = origins or {} - domain.related = related or {} + domain.origins = origins + domain.related = related def delete_domain(rp_id: str, *, ctx: SessionContext | None = None) -> None: diff --git a/paskia/db/structs.py b/paskia/db/structs.py index 3ca0997..254f1f7 100644 --- a/paskia/db/structs.py +++ b/paskia/db/structs.py @@ -374,8 +374,8 @@ class Credential(msgspec.Struct, dict=True): class Session(msgspec.Struct, dict=True, omit_defaults=True): """Session data structure. - Mutable fields: validated (updated on session refresh) - Immutable fields: user_uuid, credential_uuid, host, ip, user_agent, client_uuid + Mutable fields: host, ip, user_agent, validated, issuer (update_session) + Immutable fields: user_uuid, credential_uuid, client_uuid, rp_id key is the hashed db_key, stored in the dict key, not in the struct. If client_uuid is set, this is an OIDC session. @@ -408,14 +408,6 @@ class Session(msgspec.Struct, dict=True, omit_defaults=True): """Get the Credential object for this session.""" return db.data().credentials[self.credential_uuid] - def metadata(self) -> dict: - """Return session metadata for backwards compatibility.""" - return { - "ip": self.ip, - "user_agent": self.user_agent, - "validated": self.validated.isoformat(), - } - def store(self, last_seen: datetime) -> None: """Store this session in the database and record a visit. @@ -644,7 +636,8 @@ class DomainConfig(msgspec.Struct, omit_defaults=True): the UI. ``related`` lists other domain names that may assert this rp-id - (WebAuthn Related Origin Requests), with the same key rule. + (WebAuthn Related Origin Requests): individual hosts or full origins + only (no wildcards, no "*"). """ rp_name: str | None = None @@ -714,7 +707,8 @@ class DB(msgspec.Struct, dict=True, omit_defaults=False): Args: session_secret: The session secret (cookie value) - will be hashed for lookup - host: Optional host for binding/validation and domain-scoped permissions + host: The request host; sessions are host-bound and domain-scoped + permissions are filtered by it Returns: SessionContext if valid, None if session not found, expired, or host mismatch @@ -730,10 +724,8 @@ class DB(msgspec.Struct, dict=True, omit_defaults=False): if s.client_uuid is not None: return None - # Validate host matches (sessions are always created with a host) - normalized_input = host - if s.host != normalized_input: - # Session bound to different host + # Sessions are host-bound + if s.host != host: return None try: @@ -744,8 +736,8 @@ class DB(msgspec.Struct, dict=True, omit_defaults=False): except KeyError: return None - # Effective permissions: role's permissions that the org can grant - # Also filter by domain if host is provided + # Effective permissions: role's permissions that the org can grant, + # filtered by domain restriction org_perm_uuids = {p.uuid for p in org.permissions} effective_perms = [] @@ -756,8 +748,7 @@ class DB(msgspec.Struct, dict=True, omit_defaults=False): p = self.permissions[perm_uuid] except KeyError: continue - # Check domain restriction (normalized_input already has port stripped) - if p.domain is not None and p.domain != normalized_input: + if p.domain is not None and p.domain != host: continue effective_perms.append(p) diff --git a/paskia/domains.py b/paskia/domains.py index b9d16f3..c39cbb9 100644 --- a/paskia/domains.py +++ b/paskia/domains.py @@ -28,15 +28,25 @@ DEFAULT_RELATED_ORIGIN_CAP = 5 def origin_url(key: str) -> str: - """Full origin URL for an origins-dict key (https:// is implied).""" + """URL form of an origins-dict key (https:// is implied); wildcards and + '*' pass through unchanged.""" if hostutil.is_wildcard_pattern(key) or key == "*" or "://" in key: return key return f"https://{key}" def origin_key(origin: str) -> str: - """Origins-dict key for a full origin URL (https:// omitted).""" - return origin.removeprefix("https://").rstrip("/") + """Origins-dict key for a full origin URL (https:// omitted). + + Keys are canonicalized: lowercased, and bare hosts/wildcards lose any + trailing dot. + """ + key = origin.removeprefix("https://").rstrip("/") + if hostutil.is_wildcard_pattern(key): + return "*." + key[2:].rstrip(".").lower() + if "://" not in key: + key = key.rstrip(".") + return key.lower() def auth_host_url(domain: DomainConfig) -> str | None: @@ -181,6 +191,7 @@ def validate_config( for rp_id, domain in config.domains.items(): hostutil.validate_rp_id(rp_id) + domain_auth_host: str | None = None for key, props in domain.origins.items(): is_auth = isinstance(props, OriginEntry) and props.auth_host if key == "*": @@ -207,6 +218,12 @@ def validate_config( f"'{rp_id}' — configure it as a related origin instead" ) if is_auth: + if domain_auth_host is not None: + raise ValueError( + f"Domain '{rp_id}' marks several origins as the auth " + f"host ('{domain_auth_host}' and '{key}') — only one allowed" + ) + domain_auth_host = key ah = hostutil.normalize_host( hostutil.auth_host_netloc(origin_url(key)) or "" ) @@ -239,8 +256,10 @@ def validate_config( f"'{rp_id}' — subdomains need no related origin entry" ) # A related host may be (or fall inside) another domain's - # rp-id: the owning domain wins dispatch, and the listing - # domain's well-known document still authorizes ROR logins. + # rp-id: a host that *is* a configured rp-id always serves its + # own domain; otherwise the related listing wins dispatch over + # suffix matching, so ROR logins from the listed host keep + # working. covered_by_rp_id = any( hostutil.is_subdomain(hn, other) for other in config.domains ) @@ -288,8 +307,11 @@ def sanitize_config( origins: dict[str, bool | OriginEntry] = {} related: dict[str, bool] = dict(domain.related) + auth_seen = False for key, props in domain.origins.items(): is_auth = isinstance(props, OriginEntry) and props.auth_host + if not is_auth: + props = True # canonicalize junk/empty entries to presence-only if key == "*": # Shorthand for '*.{rp_id}'; wildcards cannot be auth hosts if is_auth: @@ -324,6 +346,15 @@ def sanitize_config( warn(f"Domain '{rp_id}': invalid origin '{key}' dropped") continue if hostutil.is_subdomain(hn, rp_id): + if is_auth: + if auth_seen: + warn( + f"Domain '{rp_id}': several origins marked as " + f"auth host — extra mark on '{key}' cleared" + ) + props = True + else: + auth_seen = True origins[key] = props else: warn( @@ -376,8 +407,9 @@ def sanitize_config( # (the rp-id always wins dispatch) — clear the mark. Sharing one auth # host between domains is allowed (login consolidation); resolution # picks the best suffix match. Related origins may point at or inside - # other domains' rp-ids (the owner wins dispatch; the listing domain's - # well-known document still authorizes ROR logins). + # other domains' rp-ids: a host that *is* a configured rp-id serves its + # own domain; otherwise the related listing wins dispatch over suffix + # matching. rp_ids = set(domains) seen_auth_hosts: dict[str, str] = {} for rp_id, domain in domains.items(): diff --git a/paskia/fastapi/admin/orgs.py b/paskia/fastapi/admin/orgs.py index 1165aee..be6bcd2 100644 --- a/paskia/fastapi/admin/orgs.py +++ b/paskia/fastapi/admin/orgs.py @@ -137,13 +137,11 @@ async def admin_remove_org_permission( auth, ["auth:admin"], host=request.headers.get("host"), match=permutil.has_all ) - db.remove_permission_from_org(org_uuid, permission_uuid, ctx=ctx) - - # Guard rail: prevent removing auth:admin from your own org if it would lock you out + # Guard rail: prevent removing auth:admin from your own org (lockout) perm = db.data().permissions.get(permission_uuid) - if perm and perm.scope == "auth:admin" and ctx.org.uuid == org_uuid: - # Check if any other org grants auth:admin that we're a member of - # (we only know our current org, so this effectively means we can't remove it from our own org) + if perm is None: + raise ValueError(f"Permission {permission_uuid} not found") + if perm.scope == "auth:admin" and ctx.org.uuid == org_uuid: raise ValueError( "Cannot remove auth:admin from your own organization. " "This would lock you out of admin access." diff --git a/paskia/fastapi/admin/permissions.py b/paskia/fastapi/admin/permissions.py index b33d8c4..0e4316f 100644 --- a/paskia/fastapi/admin/permissions.py +++ b/paskia/fastapi/admin/permissions.py @@ -166,11 +166,14 @@ async def admin_update_permission( # Get existing permission perm = db.data().permissions.get(permission_uuid) + if perm is None: + raise ValueError(f"Permission {permission_uuid} not found") - # Update fields that were provided + # Update fields that were provided (omitted domain keeps the existing + # restriction; an explicit empty domain clears it) new_scope = scope if scope is not None else perm.scope new_display_name = display_name if display_name is not None else perm.display_name - domain_value = domain if domain else None + domain_value = perm.domain if domain is None else domain or None # Sanity check: prevent changing the auth:admin permission scope if perm.scope == "auth:admin" and new_scope != "auth:admin": @@ -211,6 +214,8 @@ async def admin_delete_permission( # Get the permission to check its scope perm = db.data().permissions.get(permission_uuid) + if perm is None: + raise ValueError(f"Permission {permission_uuid} not found") # Sanity check: prevent deleting critical permissions if it would lock out admin if perm.scope == "auth:admin": diff --git a/paskia/fastapi/api.py b/paskia/fastapi/api.py index 9455800..63f2e23 100644 --- a/paskia/fastapi/api.py +++ b/paskia/fastapi/api.py @@ -399,7 +399,6 @@ async def api_set_session( if not auth or not auth.credentials: raise HTTPException(400, "Bearer token required") - # Verify host is provided host = hostutil.normalize_host(request.headers.get("host", "")) if not host: raise HTTPException(400, "Host header required") @@ -412,7 +411,6 @@ async def api_set_session( secret = a.session_key - # Verify the session exists ctx = session_ctx(secret, host) if not ctx: raise HTTPException(401, f"Session not found on {host}") diff --git a/paskia/fastapi/auth_host.py b/paskia/fastapi/auth_host.py index af0c8b0..32e6a43 100644 --- a/paskia/fastapi/auth_host.py +++ b/paskia/fastapi/auth_host.py @@ -66,19 +66,17 @@ def should_redirect_auth_path_to_root(path: str) -> bool: return bool(token and "/" not in token and passphrase.is_well_formed(token)) -def redirect_to_root_on_auth_host(request: Request, cur: str, path: str) -> Response: +def redirect_to_root_on_auth_host(request: Request, host: str, path: str) -> Response: """Create a redirect response to root path on the same host.""" new_path = path[5:] or "/" - return RedirectResponse(f"{request.url.scheme}://{cur}{new_path}", 307) + return RedirectResponse(f"{request.url.scheme}://{host}{new_path}", 307) async def redirect_middleware(request: Request, call_next): """Middleware to handle auth host redirects. Only the current domain's *own* auth host triggers redirects; a domain - without one serves its UI under /auth/ on its own hosts. Domains - relying on a shared (fallback) auth host use it for WS/restricted - API calls, not for redirects. + without one serves its UI under /auth/ on its own hosts. """ cfg = current_domain().own_auth_host if not cfg: @@ -98,7 +96,7 @@ async def redirect_middleware(request: Request, call_next): return await call_next(request) return redirect_to_auth_host(request, cfg, path) else: - # On auth host: force UI endpoints at root + # On auth host: force UI endpoints at root (cfg keeps any port) if should_redirect_auth_path_to_root(path): - return redirect_to_root_on_auth_host(request, cur, path) + return redirect_to_root_on_auth_host(request, cfg, path) return await call_next(request) diff --git a/paskia/fastapi/dispatch.py b/paskia/fastapi/dispatch.py index ae015a7..4b82300 100644 --- a/paskia/fastapi/dispatch.py +++ b/paskia/fastapi/dispatch.py @@ -4,8 +4,7 @@ Every HTTP request and WebSocket connection is dispatched to exactly one domain, resolved from the Host header via the domain registry. The resolved domain is exposed as ``request.state.domain`` and through the :func:`paskia.domains.current_domain` contextvar, which endpoint code uses -for all domain-dependent behavior (passkey configuration, OIDC provider, -site URLs). +for all domain-dependent behavior (passkey configuration, site URLs). Unknown hosts are rejected before routing: diff --git a/paskia/fastapi/oid.py b/paskia/fastapi/oid.py index 291bcc1..62d8e49 100644 --- a/paskia/fastapi/oid.py +++ b/paskia/fastapi/oid.py @@ -265,9 +265,6 @@ async def _handle_refresh_token( - Validates session exists and belongs to client - Extends session expiry (24h sliding window) - Issues new access_token and id_token - - Note: ip and user_agent are NOT updated because the refresh request - comes from the OIDC client's backend, not the end user's browser. """ if not refresh_token_value: return JSONResponse( @@ -367,7 +364,7 @@ def _build_token_response( name=user.display_name, preferred_username=user.preferred_username, email=user.email, - picture=avatar.current_avatar_url(user.uuid), + picture=avatar.avatar_url(user.uuid), groups=groups or None, auth_time=auth_time, ) @@ -455,7 +452,7 @@ async def userinfo( response["name"] = user.display_name if user.preferred_username: response["preferred_username"] = user.preferred_username - picture = avatar.current_avatar_url(user.uuid) + picture = avatar.avatar_url(user.uuid) if picture: response["picture"] = picture diff --git a/paskia/fastapi/remote.py b/paskia/fastapi/remote.py index 083fcf2..95036ee 100644 --- a/paskia/fastapi/remote.py +++ b/paskia/fastapi/remote.py @@ -6,7 +6,7 @@ wants to log in and another device (authenticating) provides the passkey. Endpoints: - /request: Called by the device wanting to be authenticated -- /pair: Called by the authenticating device to complete the request +- /permit: Called by the authenticating device to complete the request """ import asyncio diff --git a/paskia/fastapi/session.py b/paskia/fastapi/session.py index 2d87b0e..d44b816 100644 --- a/paskia/fastapi/session.py +++ b/paskia/fastapi/session.py @@ -4,8 +4,6 @@ FastAPI-specific session management for WebAuthn authentication. This module provides FastAPI-specific session management functionality: - Extracting client information from FastAPI requests - Setting and clearing HTTP-only cookies via FastAPI Response objects - -Generic session management functions have been moved to authsession.py """ from ipaddress import IPv4Address, IPv6Address diff --git a/paskia/fastapi/wschat.py b/paskia/fastapi/wschat.py index feb4aed..209088a 100644 --- a/paskia/fastapi/wschat.py +++ b/paskia/fastapi/wschat.py @@ -120,7 +120,10 @@ async def authenticate_and_login( session_user_agent if session_user_agent is not None else metadata["user_agent"] ) - # Create session and update user/credential + # Create session and update user/credential; stamp it with the domain of + # the session's host (in remote flows the connection domain is the + # approver's, but the session belongs to the requesting device's domain) + login_domain = registry().resolve(login_host) or domain secret = db.login( user_uuid=cred.user_uuid, credential_uuid=cred.uuid, @@ -128,7 +131,7 @@ async def authenticate_and_login( host=login_host, ip=login_ip, user_agent=login_user_agent, - rp_id=domain.rp_id, + rp_id=login_domain.rp_id, ) # Fetch and return the full session context (using the same host the session was created with) diff --git a/paskia/fastapi/wsutil.py b/paskia/fastapi/wsutil.py index 5515285..10c9a75 100644 --- a/paskia/fastapi/wsutil.py +++ b/paskia/fastapi/wsutil.py @@ -5,13 +5,11 @@ Shared WebSocket utilities for FastAPI endpoints. import logging from functools import wraps -import base64url from fastapi import WebSocket, WebSocketDisconnect from webauthn.helpers.exceptions import InvalidAuthenticationResponse from paskia.domains import current_domain from paskia.fastapi import authz -from paskia.util import pow def websocket_error_handler(func): @@ -40,45 +38,6 @@ def websocket_error_handler(func): return wrapper -async def require_pow(ws: WebSocket, work: int | None = None) -> None: - """Send a PoW challenge and verify the client's solution. - - Sends: {"pow": {"challenge": "", "work": 10}} - Expects: {"pow": ""} - - Args: - ws: WebSocket connection - work: PoW difficulty level (default: pow.DEFAULT_WORK) - - Raises: - ValueError: If the PoW solution is invalid - """ - challenge = pow.generate_challenge() - if work is None: - work = pow.DEFAULT_WORK - - await ws.send_json( - { - "pow": { - "challenge": base64url.enc(challenge), - "work": work, - } - } - ) - - response = await ws.receive_json() - solution_b64 = response.get("pow") - if not solution_b64: - raise ValueError("PoW solution required") - - try: - solution = base64url.dec(solution_b64) - except Exception: - raise ValueError("Invalid PoW solution encoding") - - pow.verify_pow(challenge, solution, work) - - def validate_origin(ws: WebSocket) -> str: """Extract and validate origin from WebSocket request headers. diff --git a/paskia/sansio.py b/paskia/sansio.py index 6dae9cf..d84024e 100644 --- a/paskia/sansio.py +++ b/paskia/sansio.py @@ -141,7 +141,7 @@ class Passkey: for entry in self.allowed_origins: if not hostutil.is_wildcard_pattern(entry): continue - base = entry[2:] + base = entry[2:].rstrip(".") if not hostutil.is_subdomain(hostname, base): continue if hostutil.is_subdomain(base, "localhost"): diff --git a/paskia/util/apistructs.py b/paskia/util/apistructs.py index 0891f62..a10012d 100644 --- a/paskia/util/apistructs.py +++ b/paskia/util/apistructs.py @@ -1,7 +1,8 @@ """API response utilities using msgspec for JSON serialization. msgspec handles UUID and datetime conversion automatically. -API structs inherit from db structs with kw_only=True to add uuid/key fields. +Some API structs inherit from db structs with kw_only=True to add uuid/key +fields; others are standalone response shapes. """ from __future__ import annotations diff --git a/paskia/util/avatar.py b/paskia/util/avatar.py index 001a472..7fb77ec 100644 --- a/paskia/util/avatar.py +++ b/paskia/util/avatar.py @@ -15,19 +15,9 @@ from paskia.domains import current_domain MAX_UPLOAD_BYTES = 10 * 1024 * 1024 -def media_root() -> Path: - """Return the filesystem root for auxiliary media files.""" - return users_root_path(create_root=True) - - -def avatars_root() -> Path: - """Return the filesystem root for stored avatar images.""" - return media_root() - - def avatar_path(user_uuid: UUID) -> Path: """Return the avatar file path for a user.""" - return avatars_root() / str(user_uuid) / "profile.webp" + return users_root_path(create_root=True) / str(user_uuid) / "profile.webp" def avatar_public_path(user_uuid: UUID) -> str: @@ -49,11 +39,6 @@ def avatar_url(user_uuid: UUID) -> str | None: return current_domain().api_url(f"user/{user_uuid}/profile.webp") -def current_avatar_url(user_uuid: UUID) -> str | None: - """Return the current absolute avatar URL for a user UUID.""" - return avatar_url(user_uuid) - - def remove_avatar_file(user_uuid: UUID) -> None: """Delete a stored avatar file if it exists.""" with contextlib.suppress(FileNotFoundError): diff --git a/paskia/util/oidjwt.py b/paskia/util/oidjwt.py index 230dfa1..1c57018 100644 --- a/paskia/util/oidjwt.py +++ b/paskia/util/oidjwt.py @@ -54,12 +54,6 @@ def _ensure_key() -> tuple[object, object, str]: return _key -def clear_key() -> None: - """Drop the cached signing key (key rotated).""" - global _key - _key = None - - def get_jwks() -> dict: """Get JWKS (JSON Web Key Set) for public key verification.""" private_key, _, kid = _ensure_key() diff --git a/paskia/util/startupbox.py b/paskia/util/startupbox.py index 8a3fc50..52022c8 100644 --- a/paskia/util/startupbox.py +++ b/paskia/util/startupbox.py @@ -16,6 +16,7 @@ from paskia.util.hostutil import format_endpoint if TYPE_CHECKING: from paskia.domains import DomainRegistry +from paskia.db.structs import OriginEntry from paskia.domains import origin_url BOX_WIDTH = 60 # Inner width (excluding box chars) @@ -97,7 +98,11 @@ def print_startup_config( if len(domains) > 1: lines.append(line(f" URL: {domain.site_url}{domain.site_path}")) for key, props in sorted(domain.config.origins.items()): - marker = " (auth host)" if props is not True and props.auth_host else "" + marker = ( + " (auth host)" + if isinstance(props, OriginEntry) and props.auth_host + else "" + ) lines.append(line(f" Origin: {origin_url(key)}{marker}")) if not domain.config.origins: lines.append(line(f" Origin: {domain.rp_id} and subdomains"))