fix: Wave 3 auth session lifecycle and timezone reset safety - #11
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
✅ Deploy Preview for hex-diva ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Sorry @TechHypeXP, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAuthentication routes now scope session cookies to ChangesAuthentication session cookie flow
UTC monthly reset calculation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RefreshRoute
participant Cookies
participant SupabaseAuth
Client->>RefreshRoute: POST refresh request
RefreshRoute->>Cookies: read sb-refresh-token
RefreshRoute->>SupabaseAuth: refresh session
SupabaseAuth-->>RefreshRoute: session and token metadata
RefreshRoute->>Cookies: set access and refresh cookies
RefreshRoute-->>Client: return refreshed session
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/api/auth/refresh/route.ts (1)
32-40: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winClear local session cookies when token refresh fails.
If the token refresh fails (e.g., because the refresh token is expired or revoked), the server currently returns a 401 error but leaves the invalid cookies intact. Proactively clearing the cookies here ensures the client state is sanitized, preventing subsequent requests from repeatedly attempting to authenticate with an invalid token.
♻️ Proposed refactor to clear cookies
if (error || !data.session) { if (!error && !data.session) { Sentry.captureException(new Error('Token refresh: no error but no session')); } - return NextResponse.json( + const errorResponse = NextResponse.json( { error: 'Token refresh failed' }, { status: 401 } ); + errorResponse.cookies.set('sb-access-token', '', { path: '/', maxAge: 0 }); + errorResponse.cookies.set('sb-refresh-token', '', { path: '/', maxAge: 0 }); + return errorResponse; }🤖 Prompt for 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. In `@src/app/api/auth/refresh/route.ts` around lines 32 - 40, Update the token-refresh failure branch in the auth route to clear the local session cookies before returning the 401 response. Preserve the existing Sentry capture for the no-error/no-session case and the current error response, using the route’s established cookie names and response mechanism.
🤖 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/api/auth/logout/route.ts`:
- Around line 23-29: Update the logout route’s supabase.auth.signOut error path
so it clears the local session cookies before returning the existing 400
response. Reuse the same cookie-clearing logic as the successful logout path,
ensuring cookies are cleared regardless of whether signOut succeeds or fails.
In `@src/lib/commissions/monthlyResetScheduler.ts`:
- Around line 35-36: Update the monthly-reset logic in the webhook route and its
commission monthly-reset tests to use UTC consistently, matching
monthlyResetScheduler’s Date.UTC and getUTC* behavior. Replace local month/year
comparisons and date construction with UTC equivalents, preserving the existing
reset conditions and expected outcomes.
---
Outside diff comments:
In `@src/app/api/auth/refresh/route.ts`:
- Around line 32-40: Update the token-refresh failure branch in the auth route
to clear the local session cookies before returning the 401 response. Preserve
the existing Sentry capture for the no-error/no-session case and the current
error response, using the route’s established cookie names and response
mechanism.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7fb325f1-1b17-4773-992e-df3fd48589ba
📒 Files selected for processing (5)
src/app/api/auth/login/route.tssrc/app/api/auth/logout/route.tssrc/app/api/auth/refresh/route.tssrc/lib/commissions/monthlyResetScheduler.tstsconfig.tsbuildinfo
ef55ec6 to
14af44b
Compare
…ken refresh lifecycle, explicit cookie path configuration, and UTC monthly reset timezone alignment
14af44b to
8cbc1f0
Compare
Addressing Wave 3 auth lifecycle, cookie path scopes, and scheduler bugs:
Summary by cubic
Fixes auth session lifecycle and UTC-based monthly reset timing. Ensures global logout, cookie-based token refresh, consistent cookie paths, and cleanup on errors.
signOut({ scope: 'global' }); clear cookies withpath: '/'on success/error; standardized error.refresh_token; set cookies withpath: '/'; clear on error.path: '/'on access/refresh cookies.Written for commit 8cbc1f0. Summary will update on new commits.
Summary by CodeRabbit