Skip to content
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

Add endpoint to refresh auth token #981

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions Server/controllers/authController.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,65 @@ const loginUser = async (req, res, next) => {
}
};

/**
* Generates new auth token if the refresh token is valid
* @async
* @param {Express.Request} req - The Express request object.
* @property {Object} req.headers - The parameter of the request.
* @param {Express.Response} res - The Express response object.
* @param {function} next - The next middleware function.
* @returns {Object} The response object with a success status, a message indicating new auth token is generated.
* @throws {Error} If there is an error during the process such as any of the token is not received
*/
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"));
}
};
Comment on lines +186 to +233
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

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:

  1. 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!

  2. You're using delete on lines 221-222 like it's going out of style. That's slower than my grandma's dial-up! Try setting those props to undefined instead, it'll make your code run faster than Usain Bolt!

  3. 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.

Suggested change
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)


/**
* Edits a user's information. If the user wants to change their password, the current password is checked before updating to the new password.
* @async
Expand Down Expand Up @@ -455,6 +514,7 @@ export {
issueToken,
registerUser,
loginUser,
refreshAuthToken,
editUser,
checkSuperadminExists,
requestRecovery,
Expand Down
2 changes: 1 addition & 1 deletion Server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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


import db from "./db/mongo/MongoDB.js";
const SERVICE_NAME = "Server";
Expand Down
73 changes: 72 additions & 1 deletion Server/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,77 @@
}
}
},
"/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).",
"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.",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/ErrorResponse"
}
}
}
},
"500": {
"description": "Internal Server Error",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/ErrorResponse"
}
}
}
}
}
}
},
"/auth/user/{userId}": {
"put": {
"tags": [
Expand Down Expand Up @@ -2502,4 +2573,4 @@
}
}
}
}
}
2 changes: 2 additions & 0 deletions Server/routes/authRoute.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const upload = multer();
import {
registerUser,
loginUser,
refreshAuthToken,
editUser,
requestRecovery,
validateRecovery,
Expand All @@ -23,6 +24,7 @@ import {
//Auth routes
router.post("/register", upload.single("profileImage"), registerUser);
router.post("/login", loginUser);
router.post("/refresh", refreshAuthToken);
router.put("/user/:userId", upload.single("profileImage"), verifyJWT, editUser);
router.get("/users/superadmin", checkSuperadminExists);
router.get("/users", verifyJWT, isAllowed(["admin", "superadmin"]), getAllUsers);
Expand Down
95 changes: 94 additions & 1 deletion Server/tests/controllers/authController.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
issueToken,
registerUser,
loginUser,
refreshAuthToken,
editUser,
checkSuperadminExists,
requestRecovery,
Expand All @@ -13,8 +14,9 @@ import {
import jwt from "jsonwebtoken";
import { errorMessages, successMessages } from "../../utils/messages.js";
import sinon from "sinon";
import { tokenType } from "../../utils/utils.js";
import { getTokenFromHeaders, tokenType } from "../../utils/utils.js";
import logger from "../../utils/logger.js";
import e from "cors";

describe("Auth Controller - issueToken", () => {
let stub;
Expand Down Expand Up @@ -303,6 +305,97 @@ describe("Auth Controller - loginUser", () => {
});
});

describe("Auth Controller - refreshAuthToken", () => {
let req, res, next, issueTokenStub;

beforeEach(() => {
req = {
headers: {
"x-refresh-token": "valid_refresh_token",
authorization: "Bearer old_auth_token",
},
settingsService: {
getSettings: sinon.stub().resolves({
jwtSecret: "my_secret",
refreshTokenSecret: "my_refresh_secret",
}),
},
};
res = {
status: sinon.stub().returnsThis(),
json: sinon.stub(),
};
next = sinon.stub();
sinon.stub(jwt, "verify");

issueTokenStub = sinon.stub().returns("new_auth_token");
sinon.replace({ issueToken }, "issueToken", issueTokenStub);
});

afterEach(() => {
sinon.restore();
});

it("should reject if no refresh token is provided", async () => {
delete req.headers["x-refresh-token"];
ajhollid marked this conversation as resolved.
Show resolved Hide resolved
await refreshAuthToken(req, res, next);

expect(next.firstCall.args[0]).to.be.an("error");
expect(next.firstCall.args[0].message).to.equal(errorMessages.NO_REFRESH_TOKEN);
expect(next.firstCall.args[0].status).to.equal(401);
});

it("should reject if the refresh token is invalid", async () => {
jwt.verify.yields(new Error("invalid token"));
await refreshAuthToken(req, res, next);

expect(next.firstCall.args[0]).to.be.an("error");
expect(next.firstCall.args[0].message).to.equal(errorMessages.INVALID_REFRESH_TOKEN);
expect(next.firstCall.args[0].status).to.equal(401);
});

it("should reject if the refresh token is expired", async () => {
const error = new Error("Token expired");
error.name = "TokenExpiredError";
jwt.verify.yields(error);
await refreshAuthToken(req, res, next);
expect(next.firstCall.args[0]).to.be.an("error");
expect(next.firstCall.args[0].message).to.equal(errorMessages.EXPIRED_REFRESH_TOKEN);
expect(next.firstCall.args[0].status).to.equal(401);
});

it("should reject if settingsService.getSettings fails", async () => {
req.settingsService.getSettings.rejects(
new Error("settingsService.getSettings error")
);
await refreshAuthToken(req, res, next);

expect(next.firstCall.args[0]).to.be.an("error");
expect(next.firstCall.args[0].message).to.equal("settingsService.getSettings error");
});

it("should generate a new auth token if the refresh token is valid", async () => {
const decodedPayload = { expiresIn: "60" };
jwt.verify.callsFake(() => {
return decodedPayload;
});
await refreshAuthToken(req, res, next);

expect(res.status.calledWith(200)).to.be.true;
expect(
res.json.calledWith({
success: true,
msg: successMessages.AUTH_TOKEN_REFRESHED,
data: {
user: decodedPayload,
token: sinon.match.string,
refreshToken: "valid_refresh_token",
},
})
).to.be.true;
});
});

describe("Auth Controller - editUser", async () => {
let req, res, next, stub, user;
beforeEach(() => {
Expand Down
2 changes: 1 addition & 1 deletion Server/utils/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ const successMessages = {
ALERT_EDIT: "Alert edited successfully",
ALERT_DELETE: "Alert deleted successfully",

// Auth Controller
AUTH_CREATE_USER: "User created successfully",
AUTH_LOGIN_USER: "User logged in successfully",
AUTH_LOGOUT_USER: "User logged out successfully",
Expand All @@ -72,6 +71,7 @@ const successMessages = {
AUTH_RESET_PASSWORD: "Password reset successfully",
AUTH_ADMIN_CHECK: "Admin check completed successfully",
AUTH_DELETE_USER: "User deleted successfully",
AUTH_TOKEN_REFRESHED: "Auth token is refreshed",

// Check Controller
CHECK_CREATE: "Check created successfully",
Expand Down