feat: implement refresh token authentication#121
Conversation
|
@Jaykolate is attempting to deploy a commit to the Akash Santra 's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds refresh-token authentication with short-lived access tokens, HTTP-only cookies, hashed persistence, a refresh endpoint, logout revocation, environment configuration, and integration coverage. ChangesRefresh-token authentication
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthRoute
participant AuthController
participant UserDatabase
Client->>AuthRoute: POST /api/auth/refresh with refreshToken cookie
AuthRoute->>AuthController: Invoke refresh
AuthController->>UserDatabase: Match stored refreshTokenHash
AuthController-->>Client: Return new access token or error
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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: 1
🧹 Nitpick comments (3)
backend/test/auth/refresh_test_runner.js (2)
55-61: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAssert the refresh cookie’s security contract.
Presence alone does not verify the PR’s guarantees. Assert
HttpOnly,SameSite=Strict, and the seven-day lifetime so regressions cannot silently weaken cookie security.🤖 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 `@backend/test/auth/refresh_test_runner.js` around lines 55 - 61, Extend the refreshCookie validation in the login response checks to assert that the cookie includes HttpOnly, SameSite=Strict, and a seven-day lifetime. Keep the existing missing-cookie error and success logging, and fail with a clear error if any security attribute is absent or incorrect.
96-105: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd the claimed expired-token test case.
This only tests a malformed token. Sign an already-expired refresh JWT and assert
401to exercise expiration verification explicitly.🤖 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 `@backend/test/auth/refresh_test_runner.js` around lines 96 - 105, Add an expired-token case alongside the invalid-token scenario in the refresh test runner: create a refresh JWT with an expiration time in the past using the existing signing configuration, send it to POST /api/auth/refresh, and assert that the response status is 401. Keep the malformed-token assertion intact and clearly label the new expiration-specific test.backend/src/controllers/auth.controller.js (1)
410-426: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftRotate refresh tokens atomically after successful renewal.
The same cookie and hash remain valid for seven days, allowing a stolen token to be replayed repeatedly. Atomically replace the matching hash, issue a new refresh JWT, and update the cookie; reject concurrent reuse of the old hash.
🤖 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 `@backend/src/controllers/auth.controller.js` around lines 410 - 426, Update the refresh-token renewal flow around User.findOne and generateToken to atomically replace the matching refreshTokenHash, so concurrent requests cannot reuse the old hash. After the atomic update succeeds, generate a new refresh JWT, hash it, set the replacement token in the cookie, and return the new access token; reject failed updates with the existing unauthorized response.
🤖 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 `@backend/src/controllers/auth.controller.js`:
- Around line 411-422: Update the User.findOne lookup in refresh() to require
isBanned: false alongside the existing user ID and refreshTokenHash criteria, so
banned accounts fail the existing invalid-token response and cannot receive a
new access token.
---
Nitpick comments:
In `@backend/src/controllers/auth.controller.js`:
- Around line 410-426: Update the refresh-token renewal flow around User.findOne
and generateToken to atomically replace the matching refreshTokenHash, so
concurrent requests cannot reuse the old hash. After the atomic update succeeds,
generate a new refresh JWT, hash it, set the replacement token in the cookie,
and return the new access token; reject failed updates with the existing
unauthorized response.
In `@backend/test/auth/refresh_test_runner.js`:
- Around line 55-61: Extend the refreshCookie validation in the login response
checks to assert that the cookie includes HttpOnly, SameSite=Strict, and a
seven-day lifetime. Keep the existing missing-cookie error and success logging,
and fail with a clear error if any security attribute is absent or incorrect.
- Around line 96-105: Add an expired-token case alongside the invalid-token
scenario in the refresh test runner: create a refresh JWT with an expiration
time in the past using the existing signing configuration, send it to POST
/api/auth/refresh, and assert that the response status is 401. Keep the
malformed-token assertion intact and clearly label the new expiration-specific
test.
🪄 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 Plus
Run ID: 8c490f3c-948f-4f67-b1c0-34a4ffc7187b
📒 Files selected for processing (6)
backend/.env.examplebackend/src/controllers/auth.controller.jsbackend/src/lib/utils.jsbackend/src/models/user.model.jsbackend/src/routes/auth.route.jsbackend/test/auth/refresh_test_runner.js
Akash504-ai
left a comment
There was a problem hiding this comment.
Thanks for the PR! I noticed the CI checks are currently failing. Could you please run the tests locally, check the GitHub Actions logs, and address the failing checks? Once the CI is green, I'll start reviewing the implementation.
Summary
Implements refresh token-based authentication to provide secure session management and automatic access token renewal.
Closes #49
Changes Made
POST /api/auth/refreshendpoint to issue new access tokens using a valid refresh token.REFRESH_TOKEN_SECRETto.env.example.Authentication Flow
Login / Email Verification
Refresh
401 Unauthorizedfor invalid, expired, or revoked refresh tokens.Logout
Testing
POST /api/auth/refreshreturns a new access token for valid refresh tokens.401 Unauthorized.Summary by CodeRabbit