Repository navigation
Fix/device auth review followup - #225
Conversation
This PR addresses issues found in code review and adds comprehensive device token persistence for web browsers. ## Auth Loop Prevention - Add `authFailedPermanently` flag to break Zustand rehydration loops - Add `authAttemptTimestamps` for rapid auth attempt detection (5+ in 10s) - Clear failure flags on successful session load (OAuth/login flow fix) - Set permanent failure flag when device token is definitively revoked ## Device Token Linking - Link device tokens to refresh tokens in OAuth callback and One Tap - Ensures device revocation also revokes associated refresh tokens - Prevents revoked devices from refreshing via orphaned refresh tokens ## Web Device Token Persistence - Extend device token creation to web platform (was desktop-only) - Pass deviceId and deviceName from browser to OAuth endpoints - Store web device token in localStorage for 90-day persistence - Add fingerprint utility integration for web device identification ## Desktop Auth Improvements - Reduce JWT cache TTL from 30s to 5s for freshness after rotation - Clear `authFailedPermanently` on login attempt in useAuth hook ## Tests - Add comprehensive tests for auth loop detection - Add tests for web device token creation in One Tap and OAuth callback - Add tests for `authFailedPermanently` flag behavior Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The useAuth hook calls useAuthStore.getState().setAuthFailedPermanently() during login, but the test mock was missing this method causing CI failures. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Prevent attackers from hijacking OAuth callbacks to external domains: - Validate returnUrl in signin to reject absolute/protocol-relative URLs - Defense-in-depth: re-validate returnUrl in callback before redirect - Add localStorage error handling in GoogleOneTap for private browsing - Add comprehensive tests for redirect protection Attack vector prevented: attacker sets returnUrl to external domain, tricks victim through OAuth, captures deviceToken from redirect. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Wrap decodeURIComponent() in try-catch to handle malformed percent-encoding gracefully. Without this, URLs like /%E0%A4%A would throw URIError instead of being rejected as unsafe. This matches the pattern already used in callback/route.ts for defense-in-depth. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add token refresh attempt before marking auth as failed in useAuthStore - Persist device token after successful refresh in auth-fetch - Track shouldLogout flag to distinguish fatal vs transient failures - Pause retry attempts instead of forcing logout on max retries - Update test mock to include shouldLogout in refreshAuthSession response Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe pull request enhances token refresh logic by introducing refresh outcome tracking to control retry behavior, adding device token and CSRF token persistence from device-token fallback responses with error handling, and implementing a web-token refresh flow in the auth store that mirrors desktop behavior for improved session recovery without forced logout on temporary failures. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
✨ Finishing touches
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 |
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.