Extend session TTL and refresh on access for dashboard - #78
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces configurable dashboard session lifetime via a new ChangesDashboard Session Lifetime & Auth Error Handling
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthAPI as /api/auth/me
participant SessionMgr as refreshDashboardAccess
Client->>AuthAPI: GET /me
AuthAPI->>SessionMgr: check session validity
alt session valid
SessionMgr->>SessionMgr: session.touch()
SessionMgr-->>AuthAPI: ok: true
else missing/expired
SessionMgr->>SessionMgr: classify reason
SessionMgr-->>AuthAPI: ok: false, reason code
end
AuthAPI-->>Client: response with reason
sequenceDiagram
participant App
participant AuthProvider
participant ToastUI
App->>AuthProvider: refreshAuth()
AuthProvider->>AuthProvider: fetch /api/auth/me
alt auth success
AuthProvider->>AuthProvider: set wasAuthenticatedRef = true
AuthProvider-->>App: status: authenticated
else auth failure
AuthProvider->>AuthProvider: resolveSessionErrorMessage
alt message exists
AuthProvider->>ToastUI: display error toast
end
AuthProvider->>AuthProvider: clear user state
AuthProvider-->>App: status: unauthenticated
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
dashboard/frontend/src/app/context/AuthContext.tsx (2)
133-146:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset auth-history ref on explicit logout.
After Line 143-145,
wasAuthenticatedRef.currentremainstrue, which can trigger an incorrect "sessão expirou" toast on a later refresh after a deliberate logout.Proposed fix
const logout = useCallback(async () => { try { await fetch('/api/auth/logout', { method: 'POST', credentials: 'include', headers: { Accept: 'application/json', }, }); } finally { setUser(null); setStatus('unauthenticated'); + wasAuthenticatedRef.current = false; navigate('/login', { replace: true }); } }, [navigate]);🤖 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 `@dashboard/frontend/src/app/context/AuthContext.tsx` around lines 133 - 146, In the logout function (logout) reset the wasAuthenticatedRef current value to false as part of the explicit logout flow so it doesn't later trigger the "sessão expirou" toast; update wasAuthenticatedRef.current before calling setUser, setStatus or navigate (i.e., on explicit logout set wasAuthenticatedRef.current = false) so any subsequent refresh checks treat this as an intentional logout rather than an expired session.
107-127:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle
/api/auth/merequest failures to avoid a stuck loading state.Line 110 can throw on network/runtime fetch failures, and
refreshAuthcurrently never recovers state in that path.Proposed fix
const refreshAuth = useCallback(async () => { setStatus('loading'); - - const payload = await fetchAuthSession(); - if (payload.authenticated && payload.user) { - setUser(payload.user); - setStatus('authenticated'); - wasAuthenticatedRef.current = true; - return true; - } - - const sessionMessage = resolveSessionErrorMessage(payload, wasAuthenticatedRef.current); - if (sessionMessage) { - toast.error(sessionMessage); - } - - setUser(null); - setStatus('unauthenticated'); - wasAuthenticatedRef.current = false; - return false; + try { + const payload = await fetchAuthSession(); + if (payload.authenticated && payload.user) { + setUser(payload.user); + setStatus('authenticated'); + wasAuthenticatedRef.current = true; + return true; + } + + const sessionMessage = resolveSessionErrorMessage(payload, wasAuthenticatedRef.current); + if (sessionMessage) { + toast.error(sessionMessage); + } + } catch { + if (wasAuthenticatedRef.current) { + toast.error('Não foi possível validar sua sessão. Tente novamente.'); + } + } + + setUser(null); + setStatus('unauthenticated'); + wasAuthenticatedRef.current = false; + return false; }, []);🤖 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 `@dashboard/frontend/src/app/context/AuthContext.tsx` around lines 107 - 127, refreshAuth currently awaits fetchAuthSession() without error handling, so network/runtime exceptions can leave the component stuck in 'loading'; wrap the fetchAuthSession call in a try/catch (or add a finally) inside refreshAuth to catch any thrown errors, log or toast the error, ensure setUser(null), setStatus('unauthenticated'), wasAuthenticatedRef.current = false are executed on error, and return false so callers don't remain waiting; update references inside the catch/finally to the existing functions/refs (refreshAuth, fetchAuthSession, setUser, setStatus, wasAuthenticatedRef, resolveSessionErrorMessage, toast.error).
🤖 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 `@dashboard/api/auth.js`:
- Around line 21-24: The current hasDashboardCookie helper (and any other checks
using cookieHeader.includes('dashboard.sid=')) can false-positive; instead parse
the cookie header into semicolon-separated pairs, trim each pair, split on the
first '=' and compare the cookie name exactly to 'dashboard.sid' (return true if
any match, false otherwise), handling missing headers safely; update
hasDashboardCookie and the other occurrence that uses includes('dashboard.sid=')
to use this exact-key parsing logic.
In `@src/core/express-app.js`:
- Around line 18-27: The parsing currently allows arbitrary floats (rawValue ->
parsed) which can become tiny fractions and lead to maxAge 0; change the
normalization to coerce to a safe positive integer by computing const days =
Math.floor(Number(rawValue)) (or Number.isFinite(parsed) && Math.floor(parsed)),
then enforce days >= 1 and return DEFAULT_DASHBOARD_SESSION_DAYS if not valid;
also update the code path that derives maxAge milliseconds (where
DASHBOARD_SESSION_MAX_AGE_DAYS is used to compute maxAge) to multiply by this
integer days value so you never get 0 ms from tiny floats; reference symbols:
rawValue, parsed (replace with days), DEFAULT_DASHBOARD_SESSION_DAYS, and the
maxAge computation that uses DASHBOARD_SESSION_MAX_AGE_DAYS.
---
Outside diff comments:
In `@dashboard/frontend/src/app/context/AuthContext.tsx`:
- Around line 133-146: In the logout function (logout) reset the
wasAuthenticatedRef current value to false as part of the explicit logout flow
so it doesn't later trigger the "sessão expirou" toast; update
wasAuthenticatedRef.current before calling setUser, setStatus or navigate (i.e.,
on explicit logout set wasAuthenticatedRef.current = false) so any subsequent
refresh checks treat this as an intentional logout rather than an expired
session.
- Around line 107-127: refreshAuth currently awaits fetchAuthSession() without
error handling, so network/runtime exceptions can leave the component stuck in
'loading'; wrap the fetchAuthSession call in a try/catch (or add a finally)
inside refreshAuth to catch any thrown errors, log or toast the error, ensure
setUser(null), setStatus('unauthenticated'), wasAuthenticatedRef.current = false
are executed on error, and return false so callers don't remain waiting; update
references inside the catch/finally to the existing functions/refs (refreshAuth,
fetchAuthSession, setUser, setStatus, wasAuthenticatedRef,
resolveSessionErrorMessage, toast.error).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 84b3bf60-c716-42a8-a921-f58d38f09491
📒 Files selected for processing (10)
.env.example.env.staging.exampleREADME.mddashboard/README.mddashboard/api/auth.jsdashboard/frontend/src/app/context/AuthContext.tsxdashboard/tests/dashboard-auth.test.jsdocs/development/SETUP.mddocs/technical/setup-discord.mdsrc/core/express-app.js
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.
This pull request introduces improved session management and observability for the dashboard authentication flow, along with enhanced error handling and configuration flexibility. The main focus is on allowing configurable session duration, providing clearer feedback to users when their session expires or access is revoked, and improving logging for authentication events. Documentation and test coverage have also been updated to reflect these changes.
Session management and configuration:
DASHBOARD_SESSION_MAX_AGE_DAYSto configure dashboard session duration (default: 30 days), updating all relevant.envexample files and documentation to include this option. Session duration now affects both cookie expiration and Redis TTL, and is set as a rolling session. [1] [2] [3] [4] [5] [6] [7]Authentication error handling and user feedback:
reasonfield to indicate specific session issues (e.g.,expired,invalid_session,no_access,missing_session), and updated the frontend to display user-friendly messages based on these reasons. [1] [2] [3] [4] [5] [6]Logging and observability:
loggerutility and helper functions for formatting log messages. [1] [2] [3] [4] [5] [6]Testing:
Documentation and environment consistency:
README.mdand related documentation to clarify the new session duration variable and ensure environment variable tables are up to date and consistently formatted. [1] [2]These changes improve the reliability, security, and transparency of dashboard authentication, making it easier to configure, debug, and use.
Summary by CodeRabbit
New Features
Documentation