PSA audit: auth + tenancy security fixes #48
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/m1-vas"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Remediates verified-open PSA-audit findings in the legacy PSA auth/tenancy layer (re-verified against origin/main).
{user_id}.{secret}), closing the cross-user account-takeover where the newest unused token across ALL users was accepted.Deferred follow-ups: staged RLS per-query enforcement (needs a live DB to verify); routing Google through the SSO federation path (legacy HS256 kept for now); a DB-backed integration test harness.
Two PSA-audit P0 fixes in src/modules/auth/service.rs: reset_password previously selected the most-recent unused password_reset_tokens row across ALL users (ORDER BY created_at DESC LIMIT 1) and verified the presented token against it - a leaked or guessed token could reset another account, and concurrent resets locked out legitimate users. Tokens are salted Argon2 hashes so they can't be looked up by value; the emailed token is now `{user_id}.{secret}` and reset_password scopes the candidate lookup to that user before verifying the secret. Both mint sites (request_password_reset and the create_user welcome/setup flow) emit the user-bound shape. login, login_with_google, and refresh_token checked the user's status but never the owning tenant's. Added ensure_tenant_active(), called on every session-minting path, so a suspended/cancelled tenant is rejected immediately instead of lingering until token expiry. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>The RLS tenant GUC (app.current_tenant) was set via a format!-interpolated SQL string (injection-shaped, though tenant_id is a Uuid), and the non-transaction with_tenant helper ran SET LOCAL on a pooled connection - a no-op outside a transaction, and a cross-request tenant-context leak if it ever took effect. Parameterize begin_with_tenant via set_config('app.current_tenant', $1, true) and remove with_tenant plus the now-orphaned TenantConnection. The transaction-scoped begin_with_tenant remains as the building block for staged RLS enforcement; the policy stays fail-open so existing explicit `WHERE tenant_id` queries are unaffected. Follow-up (staged RLS, second half): route tickets/contacts read paths through begin_with_tenant for real row-level isolation, verify with a psql smoke, then flip the tenant_isolation policy fail-closed (the migration/bootstrap role needs BYPASSRLS). The cross-tenant FK validation already landed closes the write-side leak in the meantime. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>Extract the user-bound token split ({user_id}.{secret}) and the case-insensitive email-allowlist match into pure helpers (parse_user_bound_token, is_allowlisted_email) used by reset_password and provision_user_from_google, and cover them with unit tests: valid/invalid token shapes, dotted secrets, empty-secret rejection, and case-insensitive / empty-allowlist matching. Tenant-status and DB-backed paths still need an integration harness (deferred). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>The fallback page derived the bunyip hub link via strip_prefix("msp-api."), which fails on the api.msp.a8n.systems host and silently falls back to the hardcoded apex. Match api.msp. (the current topology) and update the stale msp-api.<tld> comments. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>