fix: persist API_SECRET + cross-device session restore (#259) - #260
Open
AmintaCCCP wants to merge 2 commits into
Open
fix: persist API_SECRET + cross-device session restore (#259)#260AmintaCCCP wants to merge 2 commits into
AmintaCCCP wants to merge 2 commits into
Conversation
Resolve the repeated PAT / API_SECRET re-prompt after Docker or tab restart (#259, reporter raphaelbahat explicitly requested persisting API_SECRET). Client persistence: - Partialize backendApiSecret to IndexedDB (persist v10) and keep a synchronous localStorage "auth mirror" as a fallback when the async IndexedDB unload write never lands; persisted values always win over the mirror. - v9→v10 migrate initializes backendApiSecret to null for older snapshots. Cross-device restore: - New POST /api/sync/auth route (API_SECRET-gated) returns the stored GitHub token; tryRestoreAuthFromBackend re-validates it against the GitHub API before logging in, with re-entrancy + double-check guards and a no-clobber rule for any existing local session. Audit fixes layered on top: - logout() now fully tears down backendApiSecret (memory + sessionStorage + localStorage mirror); previously it survived logout and got re-persisted, leaving the backend authenticating a logged-out user. - /api/sync/auth sets Cache-Control: no-store on the PAT-bearing response (defence-in-depth beyond the global helmet() default). - jsdom localStorage/sessionStorage shim in test/setup.ts for the auth mirror. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe change adds a backend authentication restore endpoint, client-side session recovery, and a localStorage authentication mirror. Application startup and backend connection flows now restore GitHub sessions when local credentials are unavailable. ChangesAuthentication restoration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant RestoreService
participant BackendAdapter
participant AuthRestoreRouter
participant Database
participant GitHubApiService
participant AppStore
App->>RestoreService: tryRestoreAuthFromBackend()
RestoreService->>BackendAdapter: restoreAuth()
BackendAdapter->>AuthRestoreRouter: POST /api/sync/auth
AuthRestoreRouter->>Database: read and decrypt github_token
Database-->>AuthRestoreRouter: token or null
AuthRestoreRouter-->>BackendAdapter: github_token response
RestoreService->>GitHubApiService: validate token
GitHubApiService-->>RestoreService: GitHub user
RestoreService->>AppStore: store token and user
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App.tsx`:
- Around line 159-162: Update the startup flow in App so
tryRestoreAuthFromBackend() runs immediately after backend.init() and before
syncFromBackend(), instead of waiting for the backend data pull to finish. Keep
the existing backend initialization and session-recovery behavior intact, but
reorder the calls so auth restoration can complete before the app decides to
render LoginScreen.
In `@src/store/useAppStore.test.ts`:
- Line 319: Update the localStorage parsing line in useAppStore.test so parsed
is declared with const instead of let, since it is never reassigned. Keep the
JSON.parse and AUTH_MIRROR_KEY logic unchanged, and adjust only the declaration
in that test block to satisfy prefer-const.
In `@src/test/setup.ts`:
- Around line 56-60: Configure the jsdom test environment in vitest.config.ts
with an explicit non-opaque URL via test.environmentOptions.url, and remove the
storage-shim fallback around window.localStorage and window.sessionStorage in
setup.ts. Preserve normal jsdom storage behavior once the origin is configured.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d1e6b79-e9a1-4704-8f71-11306c8cb761
📒 Files selected for processing (10)
server/src/index.tsserver/src/routes/authRestore.tsserver/tests/routes/authRestore.test.tssrc/App.tsxsrc/components/settings/BackendPanel.tsxsrc/services/autoSync.tssrc/services/backendAdapter.tssrc/store/useAppStore.test.tssrc/store/useAppStore.tssrc/test/setup.ts
- App.tsx: run tryRestoreAuthFromBackend before syncFromBackend so auth restoration completes before the app decides to render LoginScreen - useAppStore.test.ts: use const for non-reassigned parsed var (prefer-const) - vitest.config.ts: set non-opaque jsdom test URL via environmentOptions - setup.ts: document why a guarded storage shim is still required (Node's experimental localStorage global shadows jsdom Storage on this runtime)
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.
Summary
Fixes #259 — the repeated prompts to re-enter the GitHub PAT and
API_SECRETafter a Docker restart / tab close. Per the reporter's (@raphaelbahat) request in the issue,API_SECRETis now persisted rather than cleared on tab close.This PR also layers a code-audit pass on the changeset; the audit findings were applied as fixes within the same PR.
What changed
Client persistence
backendApiSecretis now persisted to IndexedDB (persist v10) and, additionally, written to a synchronous localStorage “auth mirror”. The mirror back-fills when the async IndexedDB unload-write never lands (the failure mode behind the bug). Persisted values always win over the mirror.backendApiSecrettonullfor older snapshots.Cross-device session restore
POST /api/sync/authroute (gated by the existingauthMiddlewareon/api/*) returns the stored, decrypted GitHub token so a fresh browser/device holding onlyAPI_SECRETcan re-establish a session.tryRestoreAuthFromBackendre-validates the restored token against the GitHub API before logging in, with re-entrancy (_isRestoringAuth) and double-check guards, and never clobbers an existing local session.Audit fixes (applied here, not separate PR)
logout()full credential teardown (security) — After persisting the secret, the oldlogout()leftbackendApiSecretin memory and in IndexedDB, so the backend kept authenticating a logged-out user. Nowlogout()clears the localStorage mirror, the sessionStorage cache, and the in-memory secret./api/sync/authCache-Control: no-store— the response carries the decrypted PAT; defence-in-depth beyond the globalhelmet()default.Notes / non-blocking
/api/sync/authroute deliberately echoes the PAT (unlike/sync/exportwhich masks it) — the restore path needs the raw token to re-validate against the GitHub API. Echo is gated behindAPI_SECRETand documented in the route. No PAT is ever logged (verified againstlogSanitizer).Test plan
src/store/useAppStore.test.ts— 18/18 pass (incl. 4 new Issue [Bug] Repeated prompts to re-enter PAT and API_SECRET; values not persisted after Docker restart #259 mirror tests)tsc --noEmit(app + server) — cleanCloses #259
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests