fix(auth): device persistence improvements and auth loop prevention

[2witstudios]

Summary

• Fix auth loop prevention: clear authFailedPermanently flag on successful OAuth/session load
• Link device tokens to refresh tokens in OAuth callback and One Tap flows for proper device revocation
• Extend device token persistence to web browsers (90-day stay-logged-in)
• Add comprehensive loop detection (5+ auth attempts in 10 seconds triggers circuit breaker)

Code Review Follow-up Fixes

Issue 1: authFailedPermanently not cleared on successful session load

File: useAuthStore.ts:332-354

When response.ok succeeds in loadSession(), the set() calls now clear both authFailedPermanently and authAttemptTimestamps. This fixes the scenario where a user:

1. Hits the loop breaker (sets authFailedPermanently: true)
2. Logs in via OAuth (which forces loadSession)
3. Session loads successfully but the flag stayed true, causing logout on next auth check

Issue 2 & 3: OAuth callback and One Tap don't link device token to refresh token

Files: callback/route.ts:347-358, one-tap/route.ts:325-336

The refresh token is inserted BEFORE the device token is created. Now after creating the device token, we update the refresh token row to link it:

await db.update(refreshTokens)
  .set({ deviceTokenId: result.deviceTokenRecordId })
  .where(...)

This ensures when a user revokes a device via Connected Devices, the associated refresh token is also deleted.

Other Changes (from other devs)

Web Device Token Persistence

• Extended device token creation to web platform (previously desktop-only)
• Browser fingerprint utility generates deviceId and deviceName
• Device token stored in localStorage for 90-day persistence
• Modified: signin/page.tsx, GoogleOneTap.tsx, signin/route.ts

Desktop Auth Improvements

• Reduced JWT cache TTL from 30s to 5s for freshness after token rotation
• Clear authFailedPermanently on login attempt in useAuth.ts

Auth Loop Prevention

• Added authFailedPermanently flag (persisted) to break Zustand rehydration loops
• Added authAttemptTimestamps for rapid auth attempt detection
• Added onRehydrateStorage handler to clear stale auth state

Test plan

• Auth failure flag clearing: Set authFailedPermanently: true in localStorage, login via OAuth, verify flag is cleared
• Device token linking: Sign in via Google, verify device_token_id is NOT NULL in refresh_tokens table
• Device revocation: Revoke device via Connected Devices, verify refresh token is also deleted
• Tests pass: pnpm vitest run src/stores/__tests__/useAuthStore.test.ts (46 tests)
• Tests pass: pnpm vitest run src/app/api/auth/ (335 auth tests total)

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Sign-in now captures device name + ID and issues optional device tokens (90‑day client persistence) for web and desktop; tokens are surfaced to the client for client-side storage.

• Bug Fixes

  • Added open‑redirect protection for sign-in flows.
  • Added loop‑detection and permanent‑failure guard to avoid repeated blocked login attempts.
  • Reduced JWT cache TTL for fresher token reads.

• Tests

  • Extensive OAuth, device‑token, and open‑redirect protection test suites added.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Adds device-aware sign‑in: client-sent deviceId/deviceName are carried through signin/one‑tap/callback; server may create 90‑day web device tokens, link them to refresh tokens, and return deviceToken. Also adds open‑redirect validation, an auth store circuit‑breaker, and shortens an in‑memory JWT TTL.

Changes

Cohort / File(s)	Summary
OAuth Callback & One‑Tap apps/web/src/app/api/auth/google/callback/route.ts, apps/web/src/app/api/auth/google/one-tap/route.ts	Parse legacy/new state for deviceId/deviceName; validate returnUrl; create/validate web device token (90d) when deviceId present; on success update refreshTokens (AND userId & tokenHash) to set deviceTokenId; include deviceToken in redirect/response; warn and continue on failures.
OAuth Sign‑in Schema & Protection apps/web/src/app/api/auth/google/signin/route.ts, apps/web/src/app/api/auth/google/__tests__/open-redirect-protection.test.ts	Add optional deviceName to signin schema; include deviceName in signed state; add isSafeReturnUrl checks and tests covering safe/unsafe returnUrl cases.
Client Sign‑in & One‑Tap Component apps/web/src/app/auth/signin/page.tsx, apps/web/src/components/auth/GoogleOneTap.tsx	Always collect deviceId and deviceName (Electron or web fingerprint); include both in signin/one‑tap requests; persist returned deviceToken on web (localStorage) or desktop secure storage; add storage error handling.
Auth Store & Hooks apps/web/src/stores/useAuthStore.ts, apps/web/src/hooks/useAuth.ts, apps/web/src/stores/__tests__/useAuthStore.test.ts, apps/web/src/hooks/__tests__/useAuth.test.ts	Add authFailedPermanently: boolean and authAttemptTimestamps: number[]; detect auth loops (>=5 attempts in 10s) to set permanent failure and skip session loads; clear flags on successful auth/reset; expose setAuthFailedPermanently; tests added/updated.
JWT Cache apps/web/src/lib/auth/auth-fetch.ts	Reduce JWT_CACHE_TTL from 30000ms to 5000ms (shorter in‑memory JWT TTL).
Tests — OAuth flows apps/web/src/app/api/auth/google/__tests__/* (e.g., google-callback-redirect.test.ts, one-tap.test.ts, open-redirect-protection.test.ts)	Add extensive contract/integration tests and mocks for callback/one‑tap flows, device-token behavior, redirect construction, and open‑redirect protection; note duplicated one‑tap test suite observed in one-tap.test.ts.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Client
    participant SigninRoute as Signin Route
    participant OAuthProvider as Google OAuth
    participant CallbackRoute as OAuth Callback Route
    participant DeviceSvc as Device Token Service
    participant DB as Database
    participant ClientRedirect as Client Redirect

    Client->>SigninRoute: POST /api/auth/google/signin (platform, deviceId, deviceName, returnUrl)
    SigninRoute->>OAuthProvider: initiate OAuth (signed state includes deviceId/deviceName/returnUrl)
    OAuthProvider-->>Client: redirect to callback URL with state
    Client->>CallbackRoute: GET /api/auth/google/callback (state)
    CallbackRoute->>CallbackRoute: verify state, isSafeReturnUrl
    alt web + deviceId present
        CallbackRoute->>DeviceSvc: validateOrCreateDeviceToken(userId, deviceId, deviceName, ip, ua)
        DeviceSvc->>DB: insert/ensure device token (expires ~90d)
        DB-->>DeviceSvc: deviceToken + recordId
        DeviceSvc-->>CallbackRoute: deviceToken
        CallbackRoute->>DB: update refreshTokens WHERE userId AND tokenHash SET deviceTokenId
    end
    CallbackRoute->>ClientRedirect: build redirect URL (auth=success [+ deviceToken])
    CallbackRoute-->>Client: 302 Redirect
    Client->>Client: persist deviceToken (localStorage / Electron secure store)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• PR #204: Overlaps Google OAuth route handlers and device‑token create/link flow (signin, callback, one‑tap) and related tests.
• PR #167: Edits the same auth/google routes (callback/signin/one‑tap) and test scaffolding; strong code‑level overlap.
• PR #203: Modifies auth store failure/circuit‑breaker logic similar to the new authFailedPermanently and attempt tracking changes.

Poem

🐰 I hopped through state and signed the key,
Carried device names across the sea,
Ninety days to keep the thread,
Loops are stopped and flags are shed,
A joyful hop for auth and me 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: device persistence improvements and auth loop prevention are both central features of this PR.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@apps/web/src/app/api/auth/google/__tests__/google-callback-redirect.test.ts`:
- Line 120: Remove the unused imports causing ESLint failures: delete the
`users` specifier from the import from `@pagespace/db` and remove the
`resetDistributedRateLimit` import wherever it’s declared in this test file
(`google-callback-redirect.test.ts`); if either is intended to be used, instead
reference them in the test (e.g., invoke `resetDistributedRateLimit()` or use
`users` fixture) otherwise simply remove the unused import entries to satisfy
the linter.

In `@apps/web/src/components/auth/GoogleOneTap.tsx`:
- Around line 86-98: The localStorage write for deviceToken can throw and should
not abort a successful sign-in; wrap the localStorage.setItem call (in the
branch where !isDesktop && data.deviceToken) in a try/catch (or call a safe
helper like safeSetItem) so any storage error is caught, logged (or reported)
but not rethrown, and the flow continues to the redirect; keep the existing
Electron path (window.electron.auth.storeSession) unchanged and only apply the
guard around the web storage write for data.deviceToken.

🧹 Nitpick comments (1)

apps/web/src/stores/__tests__/useAuthStore.test.ts (1)

875-898: Persistence tests may have unreliable assertions.

These tests check mockLocalStorage.getItem('auth-storage') immediately after setState, but Zustand's persist middleware may not have written to storage synchronously. The conditional if (stored) masks potential failures if the assertion never runs.

Consider using Zustand's persist API to trigger/verify persistence explicitly, or await the storage write:

// More reliable approach:
it('given authFailedPermanently true, should be included in persisted state', async () => {
  useAuthStore.setState({ authFailedPermanently: true });
  
  // Allow persist middleware to complete
  await new Promise(resolve => setTimeout(resolve, 0));
  
  const stored = mockLocalStorage.getItem('auth-storage');
  expect(stored).toBeTruthy();
  const parsed = JSON.parse(stored!);
  expect(parsed.state.authFailedPermanently).toBe(true);
});

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1bedbfe064

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Prevent leaking deviceToken via unvalidated returnUrl

The new redirectUrl.searchParams.set('deviceToken', ...) appends a long‑lived device token to the redirect URL, but redirectUrl is built from returnUrl in the OAuth state without any origin validation. That means an attacker can call /api/auth/google/signin with an external returnUrl and chosen deviceId, get a signed state, and trick a victim into completing Google OAuth; the callback will then redirect to the attacker’s domain with the victim’s deviceToken. Since /api/auth/device/refresh accepts deviceToken + deviceId to mint new access/refresh tokens, this can lead to account takeover. Consider restricting returnUrl to same‑origin paths (or stripping deviceToken for external redirects / storing it server‑side) to avoid leaking the token.

Useful? React with 👍 / 👎.

[2witstudios]

Code review

Found 1 issue:

1. Missing try-catch around decodeURIComponent() in isSafeReturnUrl() can cause unhandled URIError on malformed input. The callback route handles this correctly with a try-catch block, but signin route does not. This inconsistency means malformed percent-encoding (e.g., /%E0%A4%A) will throw an exception instead of returning false as intended by the defense-in-depth pattern.

PageSpace/apps/web/src/app/api/auth/google/signin/route.ts

Lines 30 to 34 in 8fcb014

	// Reject URLs with encoded characters that could bypass validation
	// %2f = /, %5c = \, %3a = :
	const decoded = decodeURIComponent(url);
	if (decoded.startsWith('//') || decoded.startsWith('/\\')) return false;
	if (/[a-z]+:/i.test(decoded)) return false;

Compare with the correct implementation in callback/route.ts:

PageSpace/apps/web/src/app/api/auth/google/callback/route.ts

Lines 31 to 37 in 8fcb014

	try {
	const decoded = decodeURIComponent(url);
	if (decoded.startsWith('//') || decoded.startsWith('/\\')) return false;
	if (/[a-z]+:/i.test(decoded)) return false;
	} catch {
	return false; // Invalid encoding
	}

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}