Add endpoint to refresh auth token #981
Conversation
WalkthroughThe changes in this pull request introduce a new function, Changes
Possibly related PRs
Suggested reviewers
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- Server/controllers/authController.js (2 hunks)
- Server/routes/authRoute.js (2 hunks)
- Server/utils/messages.js (1 hunks)
🧰 Additional context used
🪛 Biome
Server/controllers/authController.js
[error] 222-222: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 223-223: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (2)
Server/routes/authRoute.js (1)
14-14: Yo, this import's lookin' fresh, dawg!The addition of
refreshAuthTokento the import list is on point. It's settin' us up for that sweet token refresh action we've been waitin' for.Server/utils/messages.js (1)
78-78: Yo, this message is straight fire! 🔥Aight, check it out, homie. This new success message is droppin' some serious heat in the Auth Controller section. It's like mom's spaghetti, but for auth tokens, ya feel me?
Let's make sure this message is the real MVP. Run this script to see if it's used properly:
✅ Verification successful
Uh oh, the search didn't go as planned! 🚧
Looks like
rgdoesn’t recognize "javascript" as a file type. Let’s switch it up to "js" and give it another shot:
Everything's smooth with AUTH_TOKEN_REFRESHED! 🎉
Looks like the new success message is shining bright and being used just right in the Auth Controller. No hiccups found!
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Yo, we're gonna search for this fresh message usage, dawg rg --type javascript "AUTH_TOKEN_REFRESHED" -C 3Length of output: 83
Script:
#!/bin/bash # Correcting the file type to JS for proper search rg --type js "AUTH_TOKEN_REFRESHED" -C 3Length of output: 975
Server/controllers/authController.js
Outdated
| const payloadData = jwt.verify(oldAuthToken, jwtSecret, { ignoreExpiration: true }); | ||
| // delete old token related data | ||
| delete payloadData.iat; | ||
| delete payloadData.exp; |
There was a problem hiding this comment.
Don't let your security get shaky with ignoreExpiration: true
By verifying oldAuthToken with ignoreExpiration: true, you're potentially allowing expired or tampered tokens to slip through, which could make your security knees weak. This could be exploited by an attacker using an old or malicious token. Instead, consider validating the token without ignoring expiration or fetch the user data securely from a trusted source before issuing a new token.
🧰 Tools
🪛 Biome
[error] 222-222: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 223-223: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Server/controllers/authController.js
Outdated
| jwt.verify(refreshToken, refreshTokenSecret, (refreshErr, refreshDecoded) => { | ||
| if (refreshErr) { | ||
| // Invalid or expired refresh token, trigger logout | ||
| const errorMessage = | ||
| refreshErr.name === "TokenExpiredError" | ||
| ? errorMessages.EXPIRED_REFRESH_TOKEN | ||
| : errorMessages.INVALID_REFRESH_TOKEN; | ||
| const error = new Error(errorMessage); | ||
| error.status = 401; | ||
| error.service = SERVICE_NAME; | ||
| return next(error); | ||
| } | ||
| }); |
There was a problem hiding this comment.
You're missing a beat with the asynchronous jwt.verify callback
The way the code is structured, jwt.verify is used with a callback, making it asynchronous. However, the code that generates the new access token marches on outside this callback. This could lead to generating a new access token even if the refresh token is invalid—a security misstep you don't want to take. To keep the flow steady and secure, move the dependent code inside the callback after successful verification.
Apply this diff to keep the rhythm on point:
const refreshAuthToken = (req, res, next) => {
try {
// check for refreshToken
const refreshToken = req.headers["x-refresh-token"];
if (!refreshToken) {
// No refresh token provided
const error = new Error(errorMessages.NO_REFRESH_TOKEN);
error.status = 401;
error.service = SERVICE_NAME;
error.method = "handleExpiredJwtToken";
return next(error);
}
// Verify refresh token
const { refreshTokenSecret } = req.settingsService.getSettings();
- jwt.verify(refreshToken, refreshTokenSecret, (refreshErr, refreshDecoded) => {
- if (refreshErr) {
- // Invalid or expired refresh token, trigger logout
- const errorMessage =
- refreshErr.name === "TokenExpiredError"
- ? errorMessages.EXPIRED_REFRESH_TOKEN
- : errorMessages.INVALID_REFRESH_TOKEN;
- const error = new Error(errorMessage);
- error.status = 401;
- error.service = SERVICE_NAME;
- return next(error);
- }
- });
-
+ jwt.verify(refreshToken, refreshTokenSecret, (refreshErr, refreshDecoded) => {
+ if (refreshErr) {
+ // Invalid or expired refresh token, trigger logout
+ const errorMessage =
+ refreshErr.name === "TokenExpiredError"
+ ? errorMessages.EXPIRED_REFRESH_TOKEN
+ : errorMessages.INVALID_REFRESH_TOKEN;
+ const error = new Error(errorMessage);
+ error.status = 401;
+ error.service = SERVICE_NAME;
+ return next(error);
+ }
+
+ // Refresh token is valid and unexpired, generate new access token
+ const oldAuthToken = getTokenFromHeaders(req.headers);
+ const { jwtSecret } = req.settingsService.getSettings();
+
+ const payloadData = jwt.verify(oldAuthToken, jwtSecret, { ignoreExpiration: true });
+ // delete old token related data
+ delete payloadData.iat;
+ delete payloadData.exp;
+
+ const newAuthToken = issueToken(payloadData, tokenType.ACCESS_TOKEN, req.settingsService.getSettings());
+
+ return res.status(200).json({
+ success: true,
+ msg: successMessages.AUTH_TOKEN_REFRESHED,
+ data: { user: payloadData, token: newAuthToken, refreshToken: refreshToken },
+ });
+ });
} catch (error) {
next(handleError(error, SERVICE_NAME, "refreshAuthToken"));
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jwt.verify(refreshToken, refreshTokenSecret, (refreshErr, refreshDecoded) => { | |
| if (refreshErr) { | |
| // Invalid or expired refresh token, trigger logout | |
| const errorMessage = | |
| refreshErr.name === "TokenExpiredError" | |
| ? errorMessages.EXPIRED_REFRESH_TOKEN | |
| : errorMessages.INVALID_REFRESH_TOKEN; | |
| const error = new Error(errorMessage); | |
| error.status = 401; | |
| error.service = SERVICE_NAME; | |
| return next(error); | |
| } | |
| }); | |
| jwt.verify(refreshToken, refreshTokenSecret, (refreshErr, refreshDecoded) => { | |
| if (refreshErr) { | |
| // Invalid or expired refresh token, trigger logout | |
| const errorMessage = | |
| refreshErr.name === "TokenExpiredError" | |
| ? errorMessages.EXPIRED_REFRESH_TOKEN | |
| : errorMessages.INVALID_REFRESH_TOKEN; | |
| const error = new Error(errorMessage); | |
| error.status = 401; | |
| error.service = SERVICE_NAME; | |
| return next(error); | |
| } | |
| // Refresh token is valid and unexpired, generate new access token | |
| const oldAuthToken = getTokenFromHeaders(req.headers); | |
| const { jwtSecret } = req.settingsService.getSettings(); | |
| const payloadData = jwt.verify(oldAuthToken, jwtSecret, { ignoreExpiration: true }); | |
| // delete old token related data | |
| delete payloadData.iat; | |
| delete payloadData.exp; | |
| const newAuthToken = issueToken(payloadData, tokenType.ACCESS_TOKEN, req.settingsService.getSettings()); | |
| return res.status(200).json({ | |
| success: true, | |
| msg: successMessages.AUTH_TOKEN_REFRESHED, | |
| data: { user: payloadData, token: newAuthToken, refreshToken: refreshToken }, | |
| }); | |
| }); |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- Server/controllers/authController.js (2 hunks)
🧰 Additional context used
🪛 Biome
Server/controllers/authController.js
[error] 221-221: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 222-222: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (3)
Server/controllers/authController.js (3)
176-198: Yo, this function declaration is straight fire! 🔥The JSDoc comments are on point, givin' us the 411 on what this function's all about. The initial error handling is tight, checkin' for that refresh token like a bouncer at the club. Keep it up, homie!
232-235: Error handling's tighter than Mom's spaghetti grip! 👌You're wrappin' this function in a try-catch like Eminem wraps his lyrics around a beat. That
handleErrorfunction is probably handlin' errors across the whole app like a pro. Keep that consistency flowin', it's music to a developer's ears!
520-520: Exportin' like a boss! 🚀You've added
refreshAuthTokento the export list smoother than Eminem adds a new track to his album. It's ready to drop some fresh authentication beats in other parts of the app. Nice work, MC Dev!
Server/controllers/authController.js
Outdated
| const error = new Error(errorMessages.NO_REFRESH_TOKEN); | ||
| error.status = 401; | ||
| error.service = SERVICE_NAME; | ||
| error.method = "handleExpiredJwtToken"; |
There was a problem hiding this comment.
Here we do not have a refreshToken. should the method be handleExpiredJwtToken?
There was a problem hiding this comment.
error.method is meant to refer to the method that is throwing the error so we have more information in the logs to quickly track down where errors are being thrown.
In this case the method name should be refreshAuthToken
There was a problem hiding this comment.
error.methodis meant to refer to the method that is throwing the error so we have more information in the logs to quickly track down where errors are being thrown.In this case the method name should be
refreshAuthToken
Yeah. I am aware of that, I copied the code from the middleware, so forgot to update the method name there. I'll update it.
There was a problem hiding this comment.
Looks pretty good to me for the most part 👍
The error.method value should be the name of the method that is throwing the error, so that ca be updated.
Please include updated tests for authController and document the new endpoint in the openapi.json file so we have complete documentation and tests.
Server/controllers/authController.js
Outdated
| const error = new Error(errorMessages.NO_REFRESH_TOKEN); | ||
| error.status = 401; | ||
| error.service = SERVICE_NAME; | ||
| error.method = "handleExpiredJwtToken"; |
There was a problem hiding this comment.
error.method is meant to refer to the method that is throwing the error so we have more information in the logs to quickly track down where errors are being thrown.
In this case the method name should be refreshAuthToken
Hello @ajhollid, |
Hi @Rushi1109, I'd be happy to help with some test cases, but I would need you to grant me access to your forked repo so I can push to this branch. |
Hello @ajhollid,
|
OK ill give it a try when I have a chance. It's the weekend coming up so it might be a couple days. I will get to it as soon as I can 👍 |
Sure. |
|
Hey @Rushi1109 , I refactored the function slightly, I'm not sure why all code following verifying refresh token was carried out in the callback, but I don't think there's any reason to. Please review the changes I made and make sure everything is still functioning as you expect. I also added the missing test cases so you've got 100% coverage again now. |
|
@Rushi1109 do you still want to finish this PR? I will close it otherwise. Thanks! |
|
Hello @ajhollid,
Thanks for adding those test cases.
Yeah I guess it should not be inside the callback. Thanks for correction. I have reviewed the changes and it looks good to me. |
|
I think the PR is ready to be merged now. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
Server/index.js (1)
Line range hint
147-158: Yo, this service initialization is fire, but let's make it bulletproof!Aight, I see what you did there with the service initialization. It's like you're cookin' up mom's spaghetti, but in the right order this time. That's dope!
But listen up, we gotta make sure this pasta don't stick to the pot, you feel me? Here's what I'm thinkin':
- After
connectDbAndRunServer, throw in some error handling. What if the DB connection fails? We don't want our whole app to choke.- Same deal with
loadSettings. If that fails, we're in trouble, dawg.- For the
emailService, what if the settings ain't loaded right? We gotta have a plan B.Here's a little somethin' to make it even more fire:
await connectDbAndRunServer(app, db); +if (!db.isConnected()) { + throw new Error("DB connection failed. Can't serve mom's spaghetti without a plate, yo!"); +} const settingsService = new SettingsService(AppSettings); await settingsService.loadSettings(); +if (!settingsService.isLoaded()) { + throw new Error("Failed to load settings. We're lost in the sauce, homie!"); +} const emailService = new EmailService( settingsService, fs, path, compile, mjml2html, nodemailer, logger ); +if (!emailService.isInitialized()) { + logger.warn("Email service init failed. We might not be able to send out invites to the spaghetti party."); +}This way, if somethin' goes wrong, we know exactly where we dropped the spaghetti, you feel me?
Server/openapi.json (3)
264-334: Yo, this new endpoint's got some fire, but let's spice it up a bit more!The
/auth/refreshendpoint is lookin' fresh, but there's room for improvement, ya feel me? Here's what's cookin':
- That empty request body? It's like servin' an empty plate. Let's ditch it if we ain't usin' it.
- The
authorizationheader needs more explanation. What's it for? Old access token? Let's spell it out.- The 401 response could use some extra flavor. What kinda unauthorized we talkin' about?
Here's how we can make it pop:
"/auth/refresh": { "post": { "tags": [ "auth" ], "description": "Generates a new auth token if the refresh token is valid.", - "requestBody": { - "content": { - "application/json": { - "schema": { - "type": "object", - "properties": {} - } - } - }, - "required": false - }, "parameters": [ { "name": "x-refresh-token", "in": "header", "description": "Refresh token required to generate a new auth token.", "required": true, "schema": { "type": "string" } }, { "name": "authorization", "in": "header", - "description": "Old access token, used to extract payload).", + "description": "Old access token (Bearer token), used to extract payload for validation.", "required": true, "schema": { "type": "string" } } ], "responses": { "200": { "description": "New access token generated.", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/SuccessResponse" } } } }, "401": { - "description": "Unauthorized or invalid refresh token.", + "description": "Unauthorized. Either the refresh token is invalid or expired, or the old access token is invalid.", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/ErrorResponse" } } } }, "500": { "description": "Internal Server Error", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/ErrorResponse" } } } } } } },These changes gonna make your API documentation as smooth as mom's spaghetti, you dig?
Line range hint
1-2576: Yo, we gotta tighten up our security game, it's looking shakier than my knees right now!Listen up, fam! Our static analysis tools are throwin' up red flags about our security setup. Here's the 411:
- We're missin' some global security rules. It's like leavin' the front door open, you feel me?
- Some of our operations might be rockin' empty security. That's a no-go, chief.
But hold up, let's not panic yet. I peeped that we got a
bearerAuthscheme defined in our components. That's solid. But we gotta make sure we're usin' it consistently across the board.Here's how we can lock this down tighter than mom's spaghetti recipe:
- Add a global security requirement. This'll be our default bouncer at the door.
- Double-check all our endpoints. Make sure they're either using the global security or have their own VIP list.
Try adding this at the root level of your OpenAPI spec:
"security": [ { "bearerAuth": [] } ]Then, for any public endpoints that don't need auth, you can override it like this:
"security": []Let's get this security tight, so our API's not sweatin' like it just ran a marathon, ya dig?
You want me to whip up a script to check all our endpoints for proper security? Just say the word, and I'll cook up something faster than you can say "mom's spaghetti"!
Line range hint
1-2576: Yo, our API's structure is tight, but we can make it flow smoother than Eminem's rhymes!Aight, check it. Our API's got a solid foundation, but we can take it from good to straight fire. Here's the breakdown:
- Descriptions be hittin' different levels. Some are droppin' knowledge bombs, others are keepin' it too real (as in, too brief).
- Error responses are all over the place. Some are specific like mom's spaghetti recipe, others are vague like "what's in this mystery meat?"
- Tags are consistent, but we could group these endpoints tighter than a rapper's crew.
Here's how we level up:
- Standardize them descriptions. Every endpoint should tell a story - what it does, what it needs, what it gives back.
- Error response game strong. Let's break down possible errors for each endpoint. Users should know what went wrong, you feel me?
- Consider adding some sub-categories to our tags. Like, under "monitors", we could have "create", "read", "update", "delete". Makes it easier to navigate than finding your way outta 8 Mile.
Example for standardizing descriptions:
"/monitors/{monitorId}": { "get": { "tags": [ "monitors" ], - "description": "Get monitor by id", + "description": "Retrieves detailed information about a specific monitor. Requires a valid monitor ID. Returns monitor configuration, status, and recent check results.", ... } }For error responses, let's get specific:
"responses": { "200": { "description": "OK", ... }, + "400": { + "description": "Bad Request - Invalid monitor ID format", + ... + }, "422": { - "description": "Unprocessable Content", + "description": "Unprocessable Entity - Monitor not found or user lacks permission", ... }, "500": { "description": "Internal Server Error", ... } }These changes gonna make our API docs clearer than Eminem's diction in Rap God. Users'll navigate this easier than B-Rabbit in a rap battle!
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
- Server/controllers/authController.js (2 hunks)
- Server/index.js (1 hunks)
- Server/openapi.json (2 hunks)
- Server/tests/controllers/authController.test.js (3 hunks)
- Server/utils/messages.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Server/utils/messages.js
🧰 Additional context used
🪛 Biome
Server/controllers/authController.js
[error] 221-221: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 222-222: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Server/tests/controllers/authController.test.js
[error] 340-340: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🪛 checkov
Server/openapi.json
[HIGH] 1-2576: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[HIGH] 1-2576: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
| @@ -36,7 +36,7 @@ import mjml2html from "mjml"; | |||
|
|
|||
| // Settings Service and dependencies | |||
| import SettingsService from "./service/settingsService.js"; | |||
| import AppSettings from "../db/models/AppSettings.js"; | |||
| import AppSettings from "./db/models/AppSettings.js"; | |||
There was a problem hiding this comment.
💡 Codebase verification
AppSettings.js import path is invalid!
The new path "./db/models/AppSettings.js" does not exist. Please verify the correct location of AppSettings.js and update the import path accordingly.
🔗 Analysis chain
Yo, check that import path, homie!
Aight, so we switched up the import path for AppSettings. It's like we're tryin' to find mom's spaghetti in a new kitchen, you feel me? Make sure this new path "./db/models/AppSettings.js" is legit and we ain't gonna trip when we try to serve up those settings.
Let's run this script to make sure we ain't lost in the sauce:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Yo, we're gonna check if that AppSettings file is where we think it is
# First, let's see if the file exists
if [ -f "./db/models/AppSettings.js" ]; then
echo "AppSettings.js is right where it should be, fam!"
else
echo "Uh oh, AppSettings.js is MIA. We might have a problem, dawg."
fi
# Now, let's make sure it's got the goods
grep -q "export default" "./db/models/AppSettings.js" && echo "AppSettings is exportin' something, we good!" || echo "Hold up, AppSettings ain't exportin'. We might need to fix that, yo."
Length of output: 437
| const refreshAuthToken = async (req, res, next) => { | ||
| try { | ||
| // check for refreshToken | ||
| const refreshToken = req.headers["x-refresh-token"]; | ||
|
|
||
| if (!refreshToken) { | ||
| // No refresh token provided | ||
| const error = new Error(errorMessages.NO_REFRESH_TOKEN); | ||
| error.status = 401; | ||
| error.service = SERVICE_NAME; | ||
| error.method = "refreshAuthToken"; | ||
| return next(error); | ||
| } | ||
|
|
||
| // Verify refresh token | ||
| const appSettings = await req.settingsService.getSettings(); | ||
| const { refreshTokenSecret } = appSettings; | ||
| jwt.verify(refreshToken, refreshTokenSecret, async (refreshErr, refreshDecoded) => { | ||
| if (refreshErr) { | ||
| // Invalid or expired refresh token, trigger logout | ||
| const errorMessage = | ||
| refreshErr.name === "TokenExpiredError" | ||
| ? errorMessages.EXPIRED_REFRESH_TOKEN | ||
| : errorMessages.INVALID_REFRESH_TOKEN; | ||
| const error = new Error(errorMessage); | ||
| error.status = 401; | ||
| error.service = SERVICE_NAME; | ||
| return next(error); | ||
| } | ||
| }); | ||
| // Refresh token is valid and unexpired, generate new access token | ||
| const oldAuthToken = getTokenFromHeaders(req.headers); | ||
| const { jwtSecret } = await req.settingsService.getSettings(); | ||
| const payloadData = jwt.verify(oldAuthToken, jwtSecret, { ignoreExpiration: true }); | ||
| // delete old token related data | ||
| delete payloadData.iat; | ||
| delete payloadData.exp; | ||
| const newAuthToken = issueToken(payloadData, tokenType.ACCESS_TOKEN, appSettings); | ||
|
|
||
| return res.status(200).json({ | ||
| success: true, | ||
| msg: successMessages.AUTH_TOKEN_REFRESHED, | ||
| data: { user: payloadData, token: newAuthToken, refreshToken: refreshToken }, | ||
| }); | ||
| } catch (error) { | ||
| next(handleError(error, SERVICE_NAME, "refreshAuthToken")); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Yo, dawg! Your token refresh game's weak, knees shaky!
Listen up, homie! Your token refresh function's got more issues than Eminem's got rhymes. Let's break it down:
-
You're lettin' expired tokens slide with
ignoreExpiration: true. That's like leaving your front door wide open in the hood! Fix that security hole, fam! -
You're using
deleteon lines 221-222 like it's going out of style. That's slower than my grandma's dial-up! Try setting those props toundefinedinstead, it'll make your code run faster than Usain Bolt! -
Your error handling's as basic as vanilla ice cream. Spice it up with some proper logging, ya feel me?
Here's how you can clean up your act:
- const payloadData = jwt.verify(oldAuthToken, jwtSecret, { ignoreExpiration: true });
+ const payloadData = jwt.verify(oldAuthToken, jwtSecret);
- delete payloadData.iat;
- delete payloadData.exp;
+ payloadData.iat = undefined;
+ payloadData.exp = undefined;
+ logger.info('Auth token refreshed', { userId: payloadData.userId });Don't let your code be a one-hit wonder, make it a classic!
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const refreshAuthToken = async (req, res, next) => { | |
| try { | |
| // check for refreshToken | |
| const refreshToken = req.headers["x-refresh-token"]; | |
| if (!refreshToken) { | |
| // No refresh token provided | |
| const error = new Error(errorMessages.NO_REFRESH_TOKEN); | |
| error.status = 401; | |
| error.service = SERVICE_NAME; | |
| error.method = "refreshAuthToken"; | |
| return next(error); | |
| } | |
| // Verify refresh token | |
| const appSettings = await req.settingsService.getSettings(); | |
| const { refreshTokenSecret } = appSettings; | |
| jwt.verify(refreshToken, refreshTokenSecret, async (refreshErr, refreshDecoded) => { | |
| if (refreshErr) { | |
| // Invalid or expired refresh token, trigger logout | |
| const errorMessage = | |
| refreshErr.name === "TokenExpiredError" | |
| ? errorMessages.EXPIRED_REFRESH_TOKEN | |
| : errorMessages.INVALID_REFRESH_TOKEN; | |
| const error = new Error(errorMessage); | |
| error.status = 401; | |
| error.service = SERVICE_NAME; | |
| return next(error); | |
| } | |
| }); | |
| // Refresh token is valid and unexpired, generate new access token | |
| const oldAuthToken = getTokenFromHeaders(req.headers); | |
| const { jwtSecret } = await req.settingsService.getSettings(); | |
| const payloadData = jwt.verify(oldAuthToken, jwtSecret, { ignoreExpiration: true }); | |
| // delete old token related data | |
| delete payloadData.iat; | |
| delete payloadData.exp; | |
| const newAuthToken = issueToken(payloadData, tokenType.ACCESS_TOKEN, appSettings); | |
| return res.status(200).json({ | |
| success: true, | |
| msg: successMessages.AUTH_TOKEN_REFRESHED, | |
| data: { user: payloadData, token: newAuthToken, refreshToken: refreshToken }, | |
| }); | |
| } catch (error) { | |
| next(handleError(error, SERVICE_NAME, "refreshAuthToken")); | |
| } | |
| }; | |
| const refreshAuthToken = async (req, res, next) => { | |
| try { | |
| // check for refreshToken | |
| const refreshToken = req.headers["x-refresh-token"]; | |
| if (!refreshToken) { | |
| // No refresh token provided | |
| const error = new Error(errorMessages.NO_REFRESH_TOKEN); | |
| error.status = 401; | |
| error.service = SERVICE_NAME; | |
| error.method = "refreshAuthToken"; | |
| return next(error); | |
| } | |
| // Verify refresh token | |
| const appSettings = await req.settingsService.getSettings(); | |
| const { refreshTokenSecret } = appSettings; | |
| jwt.verify(refreshToken, refreshTokenSecret, async (refreshErr, refreshDecoded) => { | |
| if (refreshErr) { | |
| // Invalid or expired refresh token, trigger logout | |
| const errorMessage = | |
| refreshErr.name === "TokenExpiredError" | |
| ? errorMessages.EXPIRED_REFRESH_TOKEN | |
| : errorMessages.INVALID_REFRESH_TOKEN; | |
| const error = new Error(errorMessage); | |
| error.status = 401; | |
| error.service = SERVICE_NAME; | |
| return next(error); | |
| } | |
| }); | |
| // Refresh token is valid and unexpired, generate new access token | |
| const oldAuthToken = getTokenFromHeaders(req.headers); | |
| const { jwtSecret } = await req.settingsService.getSettings(); | |
| const payloadData = jwt.verify(oldAuthToken, jwtSecret); | |
| // delete old token related data | |
| payloadData.iat = undefined; | |
| payloadData.exp = undefined; | |
| const newAuthToken = issueToken(payloadData, tokenType.ACCESS_TOKEN, appSettings); | |
| logger.info('Auth token refreshed', { userId: payloadData.userId }); | |
| return res.status(200).json({ | |
| success: true, | |
| msg: successMessages.AUTH_TOKEN_REFRESHED, | |
| data: { user: payloadData, token: newAuthToken, refreshToken: refreshToken }, | |
| }); | |
| } catch (error) { | |
| next(handleError(error, SERVICE_NAME, "refreshAuthToken")); | |
| } | |
| }; |
🧰 Tools
🪛 Biome
[error] 221-221: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 222-222: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
No problem, just wanted to make sure you were still with us 😂 |
Awesome thanks for your work! I will review as soon as I can 👍 |
Yeah, It was a weekend. So, I wasn't working on it. |
Rushi1109
deleted the
915-be-add-new-endpoint-for-refreshing-access-token
branch
Changes
Completes the backend part for #915