-
Notifications
You must be signed in to change notification settings - Fork 5
feat: implement remember me functionality for authentication #71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
saffiullah200
commented
Jan 25, 2026
- Add rememberMe field to RefreshToken model (defaults to false)
- Update sign-in validation to accept optional rememberMe parameter
- Implement dual token expiration:
- Without remember me: 7 days
- With remember me: 90 days
- Apply to all auth methods: email, Google OAuth, Facebook OAuth
- Token refresh respects original rememberMe preference
- Add rememberMe field to RefreshToken model (defaults to false) - Update sign-in validation to accept optional rememberMe parameter - Implement dual token expiration: * Without remember me: 7 days * With remember me: 90 days - Apply to all auth methods: email, Google OAuth, Facebook OAuth - Token refresh respects original rememberMe preference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Implements “remember me” semantics for auth by persisting a rememberMe flag on refresh tokens and using it to control access/refresh token lifetimes.
Changes:
- Added optional
rememberMevalidation for sign-in and OAuth sign-in payloads. - Stored
rememberMeonRefreshTokenand used it to set JWT + refresh token expiration (7d vs 90d). - Updated token refresh (
generate-token) to issue JWTs using the originalrememberMepreference.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/auth/validations.js | Accepts/validates optional rememberMe boolean in auth payloads. |
| src/routes/auth/sign-in.js | Applies rememberMe to JWT/refresh expiry and persists flag on refresh token. |
| src/routes/auth/google-sign-in.js | Applies rememberMe to JWT/refresh expiry and persists flag on refresh token. |
| src/routes/auth/facebook-sign-in.js | Applies rememberMe to JWT/refresh expiry and persists flag on refresh token. |
| src/routes/auth/generate-token.js | Uses refresh token’s rememberMe to determine refreshed JWT expiry. |
| src/models/refresh-token.js | Adds rememberMe field (default false) to refresh token schema. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ); | ||
| return next(err); | ||
| } | ||
| console.log(process.env.JWT_SECRET) |
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
console.log(process.env.JWT_SECRET) prints the JWT signing secret to logs, which is a credential leak. Remove this line (and avoid logging secrets in general).
| console.log(process.env.JWT_SECRET) |
| // Set token expiration based on rememberMe | ||
| // If rememberMe is true: 90 days, otherwise: 7 days (short-lived session) | ||
| const tokenExpirationDays = rememberMe ? 90 : 7; | ||
| const jwtExpiration = rememberMe ? '90d' : '7d'; | ||
|
|
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The token lifetime logic is duplicated across multiple auth handlers (email/google/facebook) and requires keeping tokenExpirationDays and jwtExpiration in sync. Consider extracting a shared helper/constant (e.g., getAuthExpirations(rememberMe)) to avoid future drift and make changes to durations in one place.
| // } | ||
|
|
||
| let token = req?.body?.code; | ||
| const rememberMe = req.body.rememberMe || false; |
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rememberMe is taken directly from req.body here, but the Facebook sign-in validation is currently commented out, so non-boolean values (e.g., the string "false") can slip through and be treated as truthy, resulting in a 90-day session unintentionally. Either re-enable validateFacebookSignIn or explicitly coerce/parse rememberMe to a boolean in a strict way.
| const rememberMe = req.body.rememberMe || false; | |
| const rememberMeRaw = req?.body?.rememberMe; | |
| const rememberMe = | |
| rememberMeRaw === true || | |
| rememberMeRaw === "true" || | |
| rememberMeRaw === 1 || | |
| rememberMeRaw === "1"; |
| // Generate JWT with expiration based on rememberMe flag | ||
| // If rememberMe is true: 90 days, otherwise: 7 days | ||
| const jwtExpiration = refreshToken.rememberMe ? '90d' : '7d'; | ||
|
|
||
| const token = jwt.sign( | ||
| { userId: refreshToken.userId }, | ||
| process.env.JWT_SECRET, | ||
| { | ||
| expiresIn: '30d', | ||
| expiresIn: jwtExpiration, | ||
| } |
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generate-token signs a new JWT for a full 7/90 days from now regardless of how much time is left on the refresh token. If a refresh token is near expiry, this can mint an access token that remains valid past refreshToken.expiresAt, effectively extending the session beyond the refresh token lifetime. Consider bounding expiresIn to the remaining time until refreshToken.expiresAt (or rotating/extending the refresh token in the same operation).
| // Generate JWT with expiration based on rememberMe flag | ||
| // If rememberMe is true: 90 days, otherwise: 7 days | ||
| const jwtExpiration = refreshToken.rememberMe ? '90d' : '7d'; | ||
|
|
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backward-compatibility: existing RefreshToken documents created before this change likely won't have rememberMe set, so refreshToken.rememberMe ? '90d' : '7d' will default them to 7 days on refresh (previously 30d). If that behavior change isn't intended, add an explicit fallback for rememberMe === undefined (e.g., keep legacy 30d) or run a migration to populate rememberMe on existing tokens.