PiCloud/security_audit/01_authn_session.md

AuthN, Session & Token Lifecycle

Audit date: 2026-06-11 Scope: login flow, password hashing, session token lifecycle, bootstrap, token transport.

Methodology: read crates/manager-core/src/{auth,auth_api,auth_middleware,auth_bootstrap,admin_session_repo,admin_user_repo,admin_users_api}.rs, crates/picloud/src/main.rs reset-password CLI, crates/manager-core/migrations/0004_admin_auth.sql, dashboard/src/lib/{auth,api}.ts. Cross-checked against AUDIT.md to avoid duplicating remediated findings. F-S-006, F-S-007, F-S-011, F-S-009 from the prior audit overlap this scope and are noted briefly but not re-reported in full; everything below is either new or a sharper restatement.

High

Dashboard self-password-change does not invalidate other live sessions or API keys

• Where: crates/manager-core/src/admin_users_api.rs:232-241
• What: The PATCH /admin/admins/{id} password-change branch updates password_hash and immediately comments: "Best practice: rotating your own password should still keep your session alive, so we don't wipe sessions here." — but it never wipes other sessions or any api_keys rows either. By contrast, cmd_reset_password in crates/picloud/src/main.rs:189-200 does call sessions.delete_for_user, and the deactivation branch (admin_users_api.rs:296-310) wipes sessions + API keys. Only the in-product password change is silent.
• Impact: An admin who suspects compromise and rotates their password through the dashboard (the natural reaction) leaves every other live session and every minted API key valid until they hit individual TTL expiry (admin_sessions.expires_at defaults to 24h sliding, API keys until manually revoked). A stolen picloud_session token or pic_… bearer continues to authenticate. This is the classic "credential rotation that doesn't rotate credentials" pitfall — and it directly contradicts blueprint §11.4's stated invariant that a password change invalidates all other sessions for that user (memory admin_auth_design).
• Recommendation: After update_password_hash, fetch the caller's current token_hash from the request, call sessions.delete_for_user(id), then re-create the caller's session (or accept the sign-out and 401 the response). Also call keys.expire_all_for_user(id) and document the consequence in the UI ("Changing your password will revoke all of your API keys"). The current behavior is silently more dangerous than either alternative.

Bearer-token Bootstrap can be re-triggered by deleting all admins

• Where: crates/manager-core/src/auth_bootstrap.rs:89 (count gate) + crates/manager-core/src/admin_users_api.rs:340-355 (delete_admin)
• What: bootstrap_first_admin_with short-circuits when count_active() > 0. The guard counter is whatever count_active() reports — checking that repo: it counts every row (including is_active=false), so an empty admin_users table re-enables bootstrap on next process start. That alone is fine, but delete_admin performs a hard DELETE (users.delete(id), no soft-delete). The "last-active-admin" guard only blocks deactivation/deletion of the only is_active=true row. If an operator deactivates all admins (impossible — last-active guard) OR an attacker with InstanceManageUsers can wipe rows after escalation, the bootstrap env vars become live again on next restart. Combined with the fact that PICLOUD_ADMIN_USERNAME / PICLOUD_ADMIN_PASSWORD_HASH are still set in nearly every production compose file ("for the initial install"), this is a re-takeover path.
• Impact: An attacker who can either (a) gain InstanceManageUsers briefly and delete all admin rows, or (b) drop admin_users rows via SQL (separate compromise) — followed by a restart (cron, deploy, OOM) — re-seeds the env-supplied admin. The env-var leak risk (leaked from CI, .env file, docker inspect, /proc/<pid>/environ) becomes a perpetual liability rather than a one-shot.
• Recommendation: Either (1) refuse bootstrap if any row has ever existed (track with a bootstrap_done row in instance_state or a sentinel boolean in admin_users), or (2) document and enforce that the bootstrap env vars are unset after first start (start-up warning when set; harden compose templates). Option (1) is the safer default; the env var is by design a "fresh install" hatch.

Medium

Admin sessions have no absolute (hard-cap) expiry

• Where: crates/manager-core/src/admin_session_repo.rs:23-30, crates/manager-core/migrations/0004_admin_auth.sql:24-30, auth_middleware.rs:245-249
• What: admin_sessions has expires_at (sliding) and last_used_at, no absolute_expires_at. Every authenticated request bumps expires_at = NOW() + ttl. A stolen token that's exercised at least once per PICLOUD_SESSION_TTL_HOURS (default 24h) never expires. Contrast with app_user_sessions (v1.1.8) which has both expires_at and absolute_expires_at — the admin surface is the more sensitive of the two and has weaker session hygiene.
• Impact: Long-lived token theft (e.g., a stale localStorage entry on a shared workstation, a leaked support session) is materially harder to detect; nothing forces a re-auth.
• Recommendation: Add absolute_expires_at TIMESTAMPTZ NOT NULL (default NOW() + 30d or NOW() + 2*ttl) populated at create, never touched by touch. Filter WHERE absolute_expires_at > NOW() in lookup. Mirrors the shape of app_user_sessions.

Admin bearer token is stored in localStorage — XSS = account takeover with no recovery window

• Where: dashboard/src/lib/auth.ts:22-43, dashboard/src/lib/api.ts:493-495
• What: setSession() writes the raw bearer token to localStorage['picloud.admin.token']. Any XSS on the dashboard origin trivially exfiltrates it. The HttpOnly cookie is also set, so the cookie path is XSS-safe, but the localStorage echo defeats that defense — the dashboard's own auth call path reads from localStorage and injects Authorization: Bearer …, which means any XSS-injected fetch works identically. Adding Strict-Transport-Security and a CSP nonce wouldn't help if a script-tag injection lands.
• Impact: An attacker with one stored-XSS primitive in any admin-rendered surface (Markdown in script descriptions, app names, route paths in admin views, audit log fields, etc.) elevates to durable session theft. Combined with the High above, the stolen token survives a password change.
• Recommendation: Drop the localStorage echo and rely on the HttpOnly cookie alone for browser sessions. The cookie already round-trips on every request (Path=/). Keep Authorization: Bearer … injection only for non-browser CLI clients that explicitly call the login API and store the token themselves. If localStorage must stay for SPA UX reasons, at minimum: rotate the token on first request after detection of a CSP violation event, and ship a strict CSP (default-src 'self'; script-src 'self') so injection requires a CSP bypass.

Password change does not enforce current-password re-verification

• Where: crates/manager-core/src/admin_users_api.rs:204-241
• What: PATCH /admins/{id} accepts a new password without any "current password" or step-up auth. Anyone with a live session token can change the account's password to one of their choosing. Combined with the long sliding-TTL session (no absolute cap), a single hijacked session escalates to permanent ownership.
• Impact: Lost / borrowed laptop, brief session theft, XSS — all become permanent takeover because the attacker can reset the password and lock the legit user out (subject to the High finding: their existing session even survives the password change).
• Recommendation: Require the request body to include current_password when the patch carries a password field for the caller's own id. Verify with verify_password on spawn_blocking. For admin-on-admin password change, require recent step-up (e.g., the caller re-supplied their password in the last 5 minutes).

PICLOUD_COOKIE_SECURE defaults to ON but is silently disablable; no warning emitted

• Where: crates/manager-core/src/auth_api.rs:221-227
• What: build_cookie honors PICLOUD_COOKIE_SECURE=0|false|no|off. There is no startup log surfacing the resolved value, no refusal when combined with a public PICLOUD_PUBLIC_BASE_URL. An operator running prod over HTTP-only mode (reverse proxy misconfig, internal-only deploy, dev-on-prod leak — see F-S-009) silently issues plain-HTTP cookies.
• Impact: Network-adjacent attackers (LAN, MITM, hijacked Wi-Fi) capture admin session tokens off the wire.
• Recommendation: Refuse PICLOUD_COOKIE_SECURE=off when PICLOUD_PUBLIC_BASE_URL starts with https:// or contains a non-localhost host. At minimum, log an error! on every login that resolves a non-Secure cookie.

CLI reset-password reads stdin with terminal echo (no rpassword)

• Where: crates/picloud/src/main.rs:203-216
• What: prompt_password_from_stdin() prints "will be read from stdin, no echo" — but it just calls std::io::stdin().lock().read_line(&mut line). On a real TTY, that echoes characters. The "no echo" claim is false. Also: the raw password sits in line (a String allocated on the heap) with no zeroization on drop.
• Impact: Shoulder-surfing during recovery; password ends up in shell history if the operator piped via read -p wrapper; raw password lives in process heap after use (forensic concern).
• Recommendation: Use rpassword::read_password (or termion) for terminal echo suppression. Wrap the password in secrecy::SecretString so it zeroizes on drop. Update the prompt text to reflect reality if the simple-stdin path is kept for piped CI use.

Login surface has no rate limit, lockout, or captcha (recap of F-S-007)

• Where: crates/manager-core/src/auth_api.rs:74-110
• What: AUDIT.md F-S-007 already records this — POST /auth/login Argon2-verifies every attempt, no per-IP throttle, no per-username lockout. Re-flagged here only because it's the missing control for a self-host admin surface. Defense is "stand up Caddy rate-limit in front" which is documented nowhere user-visible.
• Recommendation: As in F-S-007: per-IP token bucket and per-username sliding-window lockout. At minimum, document the requirement explicitly in serverless_cloud_blueprint.md deployment notes.

Low

validate_password only enforces a length minimum (8); no character class, no breached-password check, no maximum

• Where: crates/manager-core/src/admin_users_api.rs:35,382-389
• What: 8 chars is below current NIST guidance for unrestricted-rate verifiers (NIST SP 800-63B suggests ≥8 with breached-password check OR rate-limit). Argon2 absorbs arbitrarily long passwords, so no hard upper bound = a 64MB password POST can complete the Argon2 (memory-bounded). Combined with no login rate limit, a long-password DoS is a small lever.
• Recommendation: Raise minimum to 12, add a maximum (e.g., 1024 chars) to bound input cost, and optionally wire zxcvbn for strength feedback in the dashboard.

Logout reveals nothing about whether the token was valid

• Where: crates/manager-core/src/auth_api.rs:169-187
• What: Logout always returns 204 regardless of whether the supplied token matched a row. This is correct (idempotent) — the only observation is that an attacker can use /auth/logout as a free oracle to test whether a candidate token exists by, say, timing the DELETE response. Postgres DELETE timing is data-dependent but the variance is small. Listing as Info-grade because the practical impact is negligible (32-byte random tokens are unguessable).
• Recommendation: None required; noted for completeness.

me re-reads the admin_users row on every call; no caching of fresh username

• Where: crates/manager-core/src/auth_api.rs:189-210
• What: Not a security issue. Cross-reference only because the prior audit's F-P-009 PrincipalCache might leak stale role data through /me — verified: /me deliberately bypasses the cache and re-fetches, so role changes show up promptly. Behavior is correct.

extract_token_for_logout duplicates middleware extraction logic

• Where: crates/manager-core/src/auth_api.rs:234-261 vs crates/manager-core/src/auth_middleware.rs:362-386
• What: Two copies of the same Bearer / cookie extraction. Cosmetic risk of drift; one might add a CSRF check or token-format validation and forget the other.
• Recommendation: Extract to a shared function in auth.rs.

Info

Argon2 parameters are OWASP defaults and salt is per-hash from OsRng

• Where: crates/manager-core/src/auth.rs:41-45
• Argon2id, Argon2::default() (v0x13, m=19456 KiB, t=2, p=1, 32-byte hash), salt from argon2::password_hash::rand_core::OsRng. argon2 = "0.5", rand = "0.8". Verify path is the library's verify_password (constant-time on hash bytes). PHC encoding pins parameters per-hash so a future raise of defaults doesn't break existing rows. No findings here; recorded for the audit trail.

Session token entropy and storage shape are sound

• Where: crates/manager-core/src/auth.rs:80-95, migrations/0004_admin_auth.sql:24-30
• 32 random bytes from rand::rngs::OsRng (256 bits, far above the 128-bit minimum) → base64-url-no-pad → SHA-256 hex stored as PK on admin_sessions.token_hash. Lookup is index-equality so timing on the DB side is bounded by the index path, not by token contents (the SHA-256 step happens before the DB call and is constant-time over the input). Raw token never persists; logout/touch/delete all keyed on hash. Token-equals-bearer is by design (memory admin_auth_design). No findings.

Login is timing-flat against username enumeration

• Where: crates/manager-core/src/auth.rs:35-36, auth_api.rs:79-110
• The TIMING_FLAT_DUMMY_HASH constant is verified against on missing-user lookups, so the wall-clock cost of "bad username" matches "bad password". spawn_blocking is used for the verify (F-P-002 remediation). No timing oracle on enumeration via login.

Bootstrap idempotency race is safe by accident

• Where: crates/manager-core/src/auth_bootstrap.rs:89-128
• count_active() > 0 check is not atomic with repo.create, but admin_users.username is UNIQUE, so concurrent boots can't double-seed. The second loser surfaces a Repo error rather than silently skipping — operator-visible. Acceptable.

MFA / 2FA not present

• Out of MVP per blueprint and consistent with prior decisions (single-user solo-dev target). Worth tracking as an explicit "deferred until v1.2 multi-admin scenarios" item.

Out of scope (cross-references)

• F-S-006 (API-key Argon2 amplifier), F-S-007 (no login rate-limit), F-S-009 (DEV_MODE deterministic key), F-S-011 (no revoked_at on admin_sessions) — all covered in AUDIT.md. Remediation state per memory audit_2026_06_07_remediation: F-S-006 and F-S-007 mitigated (cap + spawn_blocking); F-S-009 and F-S-011 outstanding.
• API-key authorization scopes — covered by agent 2 (authorization).
• Encryption-at-rest and key rotation (PICLOUD_SECRET_KEY) — covered by agent 3 (crypto).
• CSRF on cookie-auth endpoints — partially in this report (SameSite=Lax decision); cross-origin scenarios covered by agent 6 (network/CSRF).
• Login DoS via Argon2 memory — covered by agent 7 (DoS).
• Authorization headers in tower-http TraceLayer logs — covered by agent 10 (information disclosure).