Logout #22
Logout #22
Conversation
|
Warning Rate limit exceeded@likui628 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 14 seconds before requesting another review. ⌛ 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. WalkthroughThe changes encompass updates to the authentication system, including modifications to token handling, the introduction of a logout feature, and enhancements to token management. The Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (5)
src/controllers/auth.ts (3)
6-7: LGTM! Consider adding error handling.The change to make
handleTokensan async function and await the token generation is correct. This aligns well with asynchronous best practices.Consider adding a try-catch block to handle potential errors from
tokenService.generateAuthTokens(user). This would improve error handling and make the function more robust.
32-54: Good implementation. Consider adding explicit error handling.The
logoutfunction is well-implemented, covering various scenarios and consistently clearing the refresh token cookie. However, there are a few areas for improvement:
- Add explicit error handling for
tokenService.getTokenInfoandtokenService.deleteTokencalls.- Consider using a constant for the cookie name to maintain consistency across the file.
- The success response is repeated three times, which could be refactored.
Here's a suggested refactoring that addresses these points:
const REFRESH_TOKEN_COOKIE = 'refreshToken'; export const logout = asyncHandler(async (req: Request, res: Response) => { const refreshToken = req.cookies[REFRESH_TOKEN_COOKIE]; if (!refreshToken) { return successResponse(res, null, 200); } try { const foundToken = await tokenService.getTokenInfo(refreshToken); if (foundToken) { await tokenService.deleteToken(refreshToken); } } catch (error) { console.error('Error during logout:', error); // Optionally handle the error differently } finally { res.clearCookie(REFRESH_TOKEN_COOKIE, { httpOnly: true, secure: process.env.NODE_ENV === 'production', }); return successResponse(res, null, 200); } });This refactoring:
- Adds try-catch for error handling.
- Uses a constant for the cookie name.
- Reduces repetition by always clearing the cookie and sending the success response.
Line range hint
1-54: Overall, good implementation of logout functionality and async improvements.The changes in this file successfully implement the logout functionality and improve the asynchronous handling of tokens, aligning well with the PR objectives. The modifications to
handleTokens,register, andloginfunctions ensure consistent async behavior, while the newlogoutfunction provides a comprehensive logout mechanism.To further enhance the code:
- Consider implementing the suggested refactoring for the
logoutfunction.- Add error handling in the
handleTokensfunction.- Ensure that all token-related operations (generation, retrieval, deletion) are properly logged for auditing purposes.
As the authentication system grows, consider implementing a separate middleware for token validation and refresh, which could be used across different routes requiring authentication.
src/services/token.ts (2)
Line range hint
24-39: Consider handling the case when token generation fails.The
generateAuthTokensfunction should handle the scenario where token generation fails. You can wrap the token generation and saving logic in a try-catch block to catch any potential errors and handle them appropriately.Here's an example of how you can modify the function to handle errors:
export const generateAuthTokens = async (user: User) => { try { const payload = { id: user.id, username: user.name, role: user.role } const token = jwt.sign(payload, env.ACCESS_TOKEN_SECRET, { expiresIn: '1h', }) const refreshToken = jwt.sign(payload, env.REFRESH_TOKEN_SECRET, { expiresIn: '7d', }) await saveToken( refreshToken, user.id, getTokenExpiration(7, 'days'), TokenType.REFRESH, ) return { token, refreshToken } } catch (error) { console.error('Error generating auth tokens:', error) throw error } }
51-59: Consider returning null when the token is not found.The
getTokenInfofunction should handle the case when the token is not found in the database. Instead of returningundefined, you can explicitly returnnullto indicate that the token was not found.Here's an example of how you can modify the function:
export const getTokenInfo = async (token: string) => { const tokenInfo = await prisma.token.findFirst({ where: { token, }, }) return tokenInfo ?? null }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
- package.json (2 hunks)
- src/controllers/auth.ts (2 hunks)
- src/routes/v1/auth.ts (1 hunks)
- src/services/token.ts (2 hunks)
- src/utils/misc.ts (1 hunks)
🔇 Additional comments (9)
src/utils/misc.ts (2)
1-1: LGTM: Import statement is correct.The import statement correctly imports the necessary functions from the 'date-fns' library.
3-11: 🛠️ Refactor suggestionConsider enhancing error handling and type safety.
The
getTokenExpirationfunction is well-implemented, but there are a few areas for improvement:
- The error message could be more specific. Consider including the valid range for
val.- Add a runtime check for valid
durationkeys to prevent potential runtime errors.- Explicitly define the return type of the function for better type safety.
Here's a suggested implementation with these improvements:
import { add, Duration } from 'date-fns' export const getTokenExpiration = ( val: number, duration: keyof Duration = 'days', ): Date => { if (val <= 0) { throw new Error('Duration value must be a positive number greater than 0') } const validDurations: (keyof Duration)[] = ['years', 'months', 'weeks', 'days', 'hours', 'minutes', 'seconds'] if (!validDurations.includes(duration)) { throw new Error(`Invalid duration: ${duration}. Must be one of ${validDurations.join(', ')}`) } return add(new Date(), { [duration]: val }) }This implementation:
- Improves the error message for invalid
val.- Adds a runtime check for valid
durationkeys.- Explicitly defines the return type as
Date.To ensure this function is used correctly throughout the codebase, we can run the following check:
This will help identify any potential misuse of the function in other parts of the codebase.
✅ Verification successful
Usage of
getTokenExpirationis correctly implemented insrc/services/token.ts. No further issues found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for proper usage of getTokenExpiration function # Test: Search for function calls to getTokenExpiration echo "Checking usage of getTokenExpiration:" rg --type typescript "getTokenExpiration\(" -A 2Length of output: 170
Script:
#!/bin/bash # Description: Recheck usage of getTokenExpiration function in TypeScript (.ts) files # Test: Search for function calls to getTokenExpiration in .ts files echo "Rechecking usage of getTokenExpiration in .ts files:" rg "getTokenExpiration\(" -g "*.ts" -A 2Length of output: 285
src/routes/v1/auth.ts (2)
Line range hint
1-23: Overall, good addition of logout functionality.The changes to this file are minimal and focused on adding the logout functionality. The implementation is consistent with the existing code structure and follows the established patterns for route definition.
To further improve the implementation:
- Consider adding authentication middleware to the logout route.
- Ensure that the
authController.logoutmethod handles token invalidation or session termination properly.- Update any relevant documentation or API specifications to include the new logout endpoint.
20-21: Good addition of logout functionality, consider adding authentication middleware.The addition of a logout route is a good improvement to the authentication system. However, there are a few considerations:
Consider adding authentication middleware to ensure only authenticated users can access this route. This prevents potential misuse of the logout functionality.
Verify if any validation is needed for the logout request. If no request body is expected, the current implementation is fine. Otherwise, consider adding validation similar to the login and register routes.
To ensure proper implementation of the logout functionality, let's verify the
authController.logoutmethod:src/controllers/auth.ts (2)
19-19: LGTM! Proper async handling.The change to await
handleTokensis correct and consistent with the updates made to thehandleTokensfunction. This ensures that the token is properly generated before sending the response.
27-27: LGTM! Consistent async handling.The change to await
handleTokensin theloginfunction is correct and consistent with the updates made to both thehandleTokensfunction and theregisterfunction. This ensures proper token generation before sending the response.package.json (2)
28-28: LGTM: Addition of date-fns libraryThe addition of the
date-fnslibrary (version 4.1.0) is a good choice. This library is commonly used for date manipulation in JavaScript and could be beneficial for handling token expiration in the authentication system updates mentioned in the PR objectives.
42-42: LGTM: Moving @types/bcrypt to devDependenciesAdding
@types/bcryptto the devDependencies is a good practice. Type definitions are only needed during development and not in production, so this move helps to keep the production dependencies lean.src/services/token.ts (1)
Line range hint
1-59: Verify the usage of the new token-related functions in the codebase.The changes introduce new functions for token management, such as
saveToken,deleteToken, andgetTokenInfo. It's important to ensure that these functions are being used correctly throughout the codebase.Here's a script to help verify the usage of these functions:
Review the search results to ensure that the functions are being called with the correct arguments and in the appropriate contexts.
✅ Verification successful
New token-related functions are correctly implemented and used in the codebase.
The verification process has confirmed that the new token-related functions (
saveToken,deleteToken, andgetTokenInfo) are being used correctly throughout the codebase:
saveTokenis properly defined insrc/services/token.tsand used within thegenerateAuthTokensfunction with the correct arguments.deleteTokenis defined insrc/services/token.tsand utilized insrc/controllers/auth.ts, likely for user logout or token revocation purposes.getTokenInfois defined insrc/services/token.tsand used insrc/controllers/auth.ts, probably for token validation or refresh operations.The implementation and usage of these functions appear to be consistent with their intended purposes and are correctly integrated into the authentication flow.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the usage of saveToken function echo "Verifying saveToken usage:" rg --type typescript -A 5 $'saveToken' # Search for the usage of deleteToken function echo "Verifying deleteToken usage:" rg --type typescript -A 5 $'deleteToken' # Search for the usage of getTokenInfo function echo "Verifying getTokenInfo usage:" rg --type typescript -A 5 $'getTokenInfo'Length of output: 422
Script:
#!/bin/bash # Search for the usage of saveToken function echo "Verifying saveToken usage:" rg 'saveToken' -g '*.ts' -A 5 # Search for the usage of deleteToken function echo "Verifying deleteToken usage:" rg 'deleteToken' -g '*.ts' -A 5 # Search for the usage of getTokenInfo function echo "Verifying getTokenInfo usage:" rg 'getTokenInfo' -g '*.ts' -A 5Length of output: 2039
src/services/token.ts
Outdated
| export const saveToken = async ( | ||
| token: string, | ||
| userId: User['id'], | ||
| expires: Date, | ||
| type: TokenType = TokenType.REFRESH, | ||
| ) => { | ||
| const newToken = await prisma.token.create({ | ||
| data: { | ||
| userId, | ||
| token, | ||
| type, | ||
| expires, | ||
| }, | ||
| }) | ||
| return newToken | ||
| } |
There was a problem hiding this comment.
Ensure the saveToken function handles potential errors.
The saveToken function should handle potential errors that may occur during the token creation process. Consider wrapping the Prisma query in a try-catch block to catch and handle any errors gracefully.
Here's an example of how you can modify the function to handle errors:
export const saveToken = async (
token: string,
userId: User['id'],
expires: Date,
type: TokenType = TokenType.REFRESH,
) => {
try {
const newToken = await prisma.token.create({
data: {
userId,
token,
type,
expires,
},
})
return newToken
} catch (error) {
console.error('Error saving token:', error)
throw error
}
}
src/services/token.ts
Outdated
| export const deleteToken = async (token: string) => { | ||
| await prisma.token.delete({ | ||
| where: { | ||
| token, | ||
| }, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Handle the case when the token doesn't exist.
The deleteToken function should handle the scenario where the token being deleted doesn't exist in the database. You can use Prisma's deleteMany method instead of delete to avoid throwing an error if the token doesn't exist.
Here's an example of how you can modify the function:
export const deleteToken = async (token: string) => {
await prisma.token.deleteMany({
where: {
token,
},
})
}
Summary by CodeRabbit
New Features
Bug Fixes
Chores