Repository navigation
Unlock encryption keys once per sign-in - #190
Merged
Merged
Conversation
Keys were held in memory only, so a new tab or reload lost them and the authorize screen failed with "Missing export key" even while signed in. The browser now keeps ARK as an AES-GCM envelope in localStorage whose key lives on the sign-in's server session, modelled on Proton's persisted sessions. Neither half alone reveals ARK, and logout, expiry, or revoking a sign-in destroys the server half. Also adds remembered app approvals with a per-client toggle and OIDC prompt support, a portal list of sign-ins and app approvals with revoke, and fixes device-approval polling, password error reporting, and unguarded browser storage access.
The embedded pglite driver returns bytea columns as Uint8Array, where
toString("base64url") yields a comma-separated byte list. Key envelopes
were therefore served as "123,34,..." and the browser failed to decode
them, so post-login key unlock silently failed.
Found while verifying session unlock end to end against a local instance.
- Access tokens carry sid, so sessions created later from a token (organization switching) inherit the sign-in and are revoked with it. - Expired authorization requests return to the client through POST /authorize/restart with error=invalid_request instead of stranding the browser on an error page. - Guard every browser storage access; an unguarded clearLegacyTokens in api.ts ran on each 401/403 and could replace a real error with a storage exception, which explains a wrong-password report during device-approval polling. - Add a cross-site Playwright test (DarkAuth on localhost, app on 127.0.0.1) covering new-tab authorize with no unlock prompt and silent finalization on reload.
- Remembered consent no longer auto-approves when the user has more than one active organization, or when the request names an organization other than the consented one. A client could otherwise get silent approval for an organization the user never approved. - prompt=login and prompt=select_account now require a fresh authentication; previously they only disabled remembered consent while the existing session could still approve the request. - prompt=none returns interaction_required to the client when the browser cannot restore the key without asking the user, instead of showing an interactive unlock step for a silent request.
…-side - Remembered consent now requires the sole active organization to be the one the consent was granted for. Previously, if that organization was removed and another became the only active one, it was silently approved. - pending_auth records the request's prompt, and finalization rejects prompt=login or prompt=select_account unless the session signed in after the request was created. The UI check alone could be bypassed by calling /authorize/finalize directly with the request id.
The api and branding test scripts used an unquoted src/**/*.test.ts glob. pnpm runs scripts with sh, where ** behaves like *, so 27 of 64 API test files never ran in CI, including everything under src/controllers/user. Quoting the pattern lets Node expand it. Eight of those files then hung: closing the embedded database while a fire-and-forget audit write is still in flight deadlocks inside the WASM runtime, so cleanup never returned. Closing now lets in-flight work settle first. Also repairs the stale tests this exposed: missing scimUsers.findMany and getHeader in test doubles, dark-theme branding values that changed with semantic tokens, and an authorization-code helper that never set requireOrganizationSelection. Also lets prompt=select_account finish as a different account: the pending request is no longer bound to the account that started it. 341 API tests now run and pass, in 68 seconds.
- ThemeToggle.readStoredTheme called itself instead of reading localStorage, so a saved theme was never restored. Introduced by the storage guard pass. - The authorize route was not behind OtpGate, so a user with enforced OTP who signed in from an app redirect reached the consent screen with an unverified session and every call 401'd. - restoreSessionArk deleted the stored envelope when the key or keybag request failed, so one network blip forced the password again. Only a real decrypt, subject, key or policy failure clears it now. - Consent was recorded even for clients with rememberConsent disabled, so the portal listed an approval that never skips the screen and revoking it signed the user out of that app. - The prompt=login marker lived in component state, so reloading the authorize page sent an already re-authenticated user back through the password form. It is now kept for the tab.
- unlockOrCreatePasswordArk treated any failed wrapped-DRK request as "no key exists" and generated a replacement, so a transient error could overwrite the account root and strand existing encrypted data. It now creates a key only on a 404, and API errors carry their status. - Recording consent unioned the previous scopes while replacing the organization, so scopes approved in one organization could be reused in another. Scopes now reset when the organization changes. - First-time OTP setup reloaded the page, discarding the password-derived key held in memory and forcing another unlock. The OTP journey now navigates within the app.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Encryption keys lived in one tab's memory. Sign in on the portal, close the tab, open an app in a new tab, and Authorize failed with "Missing export key" — even though the session was still valid. Signed in and unlocked had very different lifetimes: days versus one tab.
Approach
Modelled on Proton's persisted sessions (
auth/v4/sessions/local/key). The browser stores ARK as an AES-GCM envelope inlocalStorage; the key that opens it is generated by the API and lives in the sign-in's session data. Neither half alone reveals ARK. Logout, expiry, password reset, SCIM deactivation and sign-in revocation all destroy the server half.Trade-off, stated in
specs/SECURITY.md: a copied browser profile or same-origin script can restore ARK while the sign-in is live, where memory-only custody would have required the password again. Memory-only still lost keys to XSS whenever the page was in use, and cost a prompt per tab.Also in this PR
user_client_consents), per-clientrememberConsent, and OIDCpromptsupport. Not remembered when the user has several eligible organizations.signInIdcarried across refresh rotation; portal list with per-sign-in and "all others" revoke; relying-party refresh sessions inherit the sign-in so revoking ends them too.SECURITY.md, key management, sessions, OIDC, portal) plustasks/session-bound-unlock.mdandtasks/sdk-session-bound-cak.md.Tests
pnpm tidy,pnpm build, 212 API + 72 user UI + 17 admin UI unit tests, and the 126 existing Playwright tests all pass.Not verified in a browser yet. A Playwright test for the new-tab flow is still open: the demo harness serves DarkAuth and the demo on
localhost, so the demo reuses the refresh cookie and never reaches/authorize. Reproducing it needs distinct sites.Known gaps (tracked in the task)
/token/organizationcreates a relying-party session with no sign-in link, so revoking a sign-in does not end it.