Skip to content
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
4 changes: 3 additions & 1 deletion controllers/discordactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ const DISCORD_BASE_URL = config.get("services.discordBot.baseUrl");

const createGroupRole = async (req, res) => {
try {
const rolename = `group-${req.body.rolename}`;
const { role = false } = req.query;
const rolename = role ? req.body.rolename : `group-${req.body.rolename}`;

const { roleExists } = await discordRolesModel.isGroupRoleExists({ rolename });

Expand Down Expand Up @@ -505,6 +506,7 @@ const generateInviteForUser = async (req, res) => {

const inviteOptions = {
channelId: channelId,
role: req.approvedApplicationRole,
};
const response = await fetch(`${DISCORD_BASE_URL}/invite`, {
method: "POST",
Expand Down
26 changes: 1 addition & 25 deletions controllers/external-accounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ const { addOrUpdate, getUsersByRole, updateUsersInBatch } = require("../models/u
const { retrieveDiscordUsers, fetchUsersForKeyValues } = require("../services/dataAccessLayer");
const { EXTERNAL_ACCOUNTS_POST_ACTIONS } = require("../constants/external-accounts");
const removeDiscordRoleUtils = require("../utils/removeDiscordRoleFromUser");
const addDiscordRoleUtils = require("../utils/addDiscordRoleToUser");
const config = require("config");
const logger = require("../utils/logger");
const { markUnDoneTasksOfArchivedUsersBacklog } = require("../models/tasks");
Expand Down Expand Up @@ -80,30 +79,7 @@ const linkExternalAccount = async (req, res) => {
);

if (!unverifiedRoleRemovalResponse.success) {
// Tolerable errors that should not block role assignment
const tolerableErrors = ["Role doesn't exist", "Role deletion from database failed"];
const isTolerableError = tolerableErrors.some((err) => unverifiedRoleRemovalResponse.message.includes(err));

if (!isTolerableError) {
const message = `User details updated but ${unverifiedRoleRemovalResponse.message}. Please contact admin`;
return res.boom.internal(message, { message });
}
logger.info(
`Tolerable error during unverified role removal for Discord ID: ${attributes.discordId}: ${unverifiedRoleRemovalResponse.message}`
);
}

const developerRoleId = config.get("discordDeveloperRoleId");
const newRoleId = config.get("discordNewRoleId");

try {
await addDiscordRoleUtils.addDiscordRoleToUser(attributes.discordId, developerRoleId, "Developer");
await addDiscordRoleUtils.addDiscordRoleToUser(attributes.discordId, newRoleId, "New");
logger.info(`Roles (Developer, New) assigned successfully for Discord ID: ${attributes.discordId}`);
} catch (roleError) {
logger.error(`Error assigning roles after verification: ${roleError}`);
const message = `Your discord profile has been linked but role assignment failed. Please contact admin`;
return res.boom.internal(message, { message });
return res.boom.internal(null, { message: "Unverified role removal failed. Please contact admin" });
}

return res.status(200).json({ message: "Your discord profile has been linked successfully" });
Expand Down
7 changes: 6 additions & 1 deletion middlewares/checkCanGenerateDiscordLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,16 @@ const checkCanGenerateDiscordLink = async (req: CustomRequest, res: CustomRespon
}

const approvedApplication = applications.find((application: { status: string; }) => application.status === 'accepted');

if (!approvedApplication) {
return res.boom.forbidden("Only users with an accepted application can generate a Discord invite link.");
}

if (approvedApplication.isNew !== true || !approvedApplication.role) {
return res.boom.forbidden("You are not eligible to generate a Discord invite link.");
}

req.approvedApplicationRole = approvedApplication.role;
return next();
} catch (error) {
return res.boom.badImplementation("An error occurred while checking user applications.");
Expand Down
10 changes: 8 additions & 2 deletions middlewares/validators/discordactions.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
const Joi = require("joi");
const { validateMillisecondsTimestamp } = require("./utils");
const logger = require("../../utils/logger");

const validateGroupRoleBody = async (req, res, next) => {
const schema = Joi.object({
const bodySchema = Joi.object({
rolename: Joi.string().trim().required(),
description: Joi.string().trim(),
});

const querySchema = Joi.object({
role: Joi.boolean().default(false).optional(),
});

try {
await schema.validateAsync(req.body);
await bodySchema.validateAsync(req.body);
req.query = await querySchema.validateAsync(req.query);
next();
} catch (error) {
logger.error(`Error validating createGroupRole payload : ${error}`);
Expand Down
2 changes: 1 addition & 1 deletion routes/discordactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
* Short-circuit this POST method for this endpoint
* Refer https://github.com/Real-Dev-Squad/todo-action-items/issues/269 for more details.
*/
router.post("/invite", disableRoute, authenticate, checkCanGenerateDiscordLink, generateInviteForUser);
router.post("/invite", authenticate, checkCanGenerateDiscordLink, generateInviteForUser);

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
authorization
, but is not rate-limited.
This route handler performs
authorization
, but is not rate-limited.
This route handler performs
authorization
, but is not rate-limited.

Copilot Autofix

AI 19 days ago

In general, the fix is to introduce a rate-limiting middleware and apply it to the sensitive route(s) that perform expensive operations or external calls, in this case the POST /invite route. For Express applications, a standard solution is to use the widely adopted express-rate-limit library. This allows configuring per-route or global limits such as “N requests per window per IP,” which mitigates abuse without changing the route’s core behavior.

For this file, the least invasive fix is:

  1. Import express-rate-limit at the top of routes/discordactions.js.
  2. Create a specific limiter instance tailored for Discord invite generation (e.g., a small number of requests per minute/hour).
  3. Apply this limiter as a middleware on the router.post("/invite", ...) route, before the existing authentication/authorization/controller middlewares, or at least in the middleware chain for this route. This way, the business logic in generateInviteForUser is unchanged; we only restrict how frequently it can be reached.

Concretely:

  • Near the existing require statements at the top, add const rateLimit = require("express-rate-limit");.

  • After the router is created (const router = express.Router();), define a limiter like:

    const generateInviteLimiter = rateLimit({
      windowMs: 15 * 60 * 1000, // 15 minutes
      max: 20, // e.g. limit each IP to 20 invite-generation requests per window
      standardHeaders: true,
      legacyHeaders: false,
    });
  • Update the router.post("/invite", ...) line so that it includes generateInviteLimiter in the middleware chain, e.g.:

    router.post(
      "/invite",
      generateInviteLimiter,
      authenticate,
      checkCanGenerateDiscordLink,
      generateInviteForUser
    );

This targets the flagged route specifically and addresses all alert variants referencing missing rate limiting on this handler, without altering other routes’ behavior.


Suggested changeset 2
routes/discordactions.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/routes/discordactions.js b/routes/discordactions.js
--- a/routes/discordactions.js
+++ b/routes/discordactions.js
@@ -32,8 +32,16 @@
 const { verifyCronJob } = require("../middlewares/authorizeBot");
 const { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndService");
 const { disableRoute } = require("../middlewares/shortCircuit");
+const rateLimit = require("express-rate-limit");
 const router = express.Router();
 
+const generateInviteLimiter = rateLimit({
+  windowMs: 15 * 60 * 1000, // 15 minutes
+  max: 20, // limit each IP to 20 invite generation requests per window
+  standardHeaders: true,
+  legacyHeaders: false,
+});
+
 router.post("/groups", authenticate, checkIsVerifiedDiscord, validateGroupRoleBody, createGroupRole);
 router.get("/groups", authenticate, checkIsVerifiedDiscord, validateLazyLoadingParams, getPaginatedAllGroupRoles);
 router.delete("/groups/:groupId", authenticate, checkIsVerifiedDiscord, authorizeRoles([SUPERUSER]), deleteGroupRole);
@@ -47,7 +53,13 @@
  * Short-circuit this POST method for this endpoint
  * Refer https://github.com/Real-Dev-Squad/todo-action-items/issues/269 for more details.
  */
-router.post("/invite", authenticate, checkCanGenerateDiscordLink, generateInviteForUser);
+router.post(
+  "/invite",
+  generateInviteLimiter,
+  authenticate,
+  checkCanGenerateDiscordLink,
+  generateInviteForUser
+);
 
 router.delete("/roles", authenticate, checkIsVerifiedDiscord, deleteRole);
 router.get("/roles", authenticate, checkIsVerifiedDiscord, getGroupsRoleId);
EOF
@@ -32,8 +32,16 @@
const { verifyCronJob } = require("../middlewares/authorizeBot");
const { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndService");
const { disableRoute } = require("../middlewares/shortCircuit");
const rateLimit = require("express-rate-limit");
const router = express.Router();

const generateInviteLimiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 20, // limit each IP to 20 invite generation requests per window
standardHeaders: true,
legacyHeaders: false,
});

router.post("/groups", authenticate, checkIsVerifiedDiscord, validateGroupRoleBody, createGroupRole);
router.get("/groups", authenticate, checkIsVerifiedDiscord, validateLazyLoadingParams, getPaginatedAllGroupRoles);
router.delete("/groups/:groupId", authenticate, checkIsVerifiedDiscord, authorizeRoles([SUPERUSER]), deleteGroupRole);
@@ -47,7 +53,13 @@
* Short-circuit this POST method for this endpoint
* Refer https://github.com/Real-Dev-Squad/todo-action-items/issues/269 for more details.
*/
router.post("/invite", authenticate, checkCanGenerateDiscordLink, generateInviteForUser);
router.post(
"/invite",
generateInviteLimiter,
authenticate,
checkCanGenerateDiscordLink,
generateInviteForUser
);

router.delete("/roles", authenticate, checkIsVerifiedDiscord, deleteRole);
router.get("/roles", authenticate, checkIsVerifiedDiscord, getGroupsRoleId);
package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/package.json b/package.json
--- a/package.json
+++ b/package.json
@@ -42,7 +42,8 @@
     "passport-github2": "0.1.12",
     "passport-google-oauth20": "^2.0.0",
     "rate-limiter-flexible": "5.0.3",
-    "winston": "3.13.0"
+    "winston": "3.13.0",
+    "express-rate-limit": "^8.2.1"
   },
   "devDependencies": {
     "@types/chai": "4.3.16",
EOF
@@ -42,7 +42,8 @@
"passport-github2": "0.1.12",
"passport-google-oauth20": "^2.0.0",
"rate-limiter-flexible": "5.0.3",
"winston": "3.13.0"
"winston": "3.13.0",
"express-rate-limit": "^8.2.1"
},
"devDependencies": {
"@types/chai": "4.3.16",
This fix introduces these dependencies
Package Version Security advisories
express-rate-limit (npm) 8.2.1 None
Copilot is powered by AI and may make mistakes. Always verify output.

router.delete("/roles", authenticate, checkIsVerifiedDiscord, deleteRole);
router.get("/roles", authenticate, checkIsVerifiedDiscord, getGroupsRoleId);
Expand Down
188 changes: 188 additions & 0 deletions test/integration/discordactions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1377,4 +1377,192 @@ describe("Discord actions", function () {
});
});
});

describe("POST /discord-actions/groups (createGroupRole)", function () {
let testUserId;
let testUserAuthToken;

beforeEach(async function () {
testUserId = await addUser(userData[0]);
testUserAuthToken = authService.generateAuthToken({ userId: testUserId });

fetchStub.returns(
Promise.resolve({
ok: true,
json: () => Promise.resolve({ id: "discord-role-id-123" }),
})
);

sinon.stub(discordRolesModel, "isGroupRoleExists").resolves({
roleExists: false,
});
});

afterEach(async function () {
await cleanDb();
});

it("should create a group role with 'group-' prefix when role query param is not provided", function (done) {
const roleData = {
rolename: "developers",
description: "Test group role",
};

chai
.request(app)
.post("/discord-actions/groups")
.set("cookie", `${cookieName}=${testUserAuthToken}`)
.send(roleData)
.end((err, res) => {
if (err) {
return done(err);
}
expect(res).to.have.status(201);
expect(res.body).to.be.an("object");
expect(res.body.message).to.equal("Role created successfully!");
expect(res.body).to.have.property("id");
expect(fetchStub.calledOnce).to.equal(true);
const fetchCall = fetchStub.getCall(0);
const requestBody = JSON.parse(fetchCall.args[1].body);
expect(requestBody.rolename).to.equal("group-developers");

return done();
});
});

it("should create a group role with 'group-' prefix when role=false", function (done) {
const roleData = {
rolename: "developers",
description: "Test group role",
};

chai
.request(app)
.post("/discord-actions/groups?role=false")
.set("cookie", `${cookieName}=${testUserAuthToken}`)
.send(roleData)
.end((err, res) => {
if (err) {
return done(err);
}
expect(res).to.have.status(201);
expect(res.body).to.be.an("object");
expect(res.body.message).to.equal("Role created successfully!");
const fetchCall = fetchStub.getCall(0);
const requestBody = JSON.parse(fetchCall.args[1].body);
expect(requestBody.rolename).to.equal("group-developers");

return done();
});
});

it("should create a custom role without prefix when role=true", function (done) {
const roleData = {
rolename: "developers",
description: "Test custom role",
};

chai
.request(app)
.post("/discord-actions/groups?role=true")
.set("cookie", `${cookieName}=${testUserAuthToken}`)
.send(roleData)
.end((err, res) => {
if (err) {
return done(err);
}
expect(res).to.have.status(201);
expect(res.body).to.be.an("object");
expect(res.body.message).to.equal("Role created successfully!");
const fetchCall = fetchStub.getCall(0);
const requestBody = JSON.parse(fetchCall.args[1].body);
expect(requestBody.rolename).to.equal("developers");

return done();
});
});

it("should return 400 when role query param is not a boolean", function (done) {
const roleData = {
rolename: "developers",
description: "Test role",
};

chai
.request(app)
.post("/discord-actions/groups?role=invalid")
.set("cookie", `${cookieName}=${testUserAuthToken}`)
.send(roleData)
.end((err, res) => {
if (err) {
return done(err);
}
expect(res).to.have.status(400);
expect(res.body).to.be.an("object");
expect(res.body.error).to.equal("Bad Request");

return done();
});
});

it("should return 400 when role already exists", function (done) {
sinon.restore();
fetchStub = sinon.stub(global, "fetch");
fetchStub.returns(
Promise.resolve({
ok: true,
json: () => Promise.resolve({ id: "discord-role-id-123" }),
})
);

sinon.stub(discordRolesModel, "isGroupRoleExists").resolves({
roleExists: true,
});

const roleData = {
rolename: "developers",
description: "Test role",
};

chai
.request(app)
.post("/discord-actions/groups?role=true")
.set("cookie", `${cookieName}=${testUserAuthToken}`)
.send(roleData)
.end((err, res) => {
if (err) {
return done(err);
}
expect(res).to.have.status(400);
expect(res.body).to.be.an("object");
expect(res.body.message).to.equal("Role already exists!");

return done();
});
});

it("should handle string 'true' as true boolean value", function (done) {
const roleData = {
rolename: "testrole",
description: "Test role",
};

chai
.request(app)
.post("/discord-actions/groups?role=true")
.set("cookie", `${cookieName}=${testUserAuthToken}`)
.send(roleData)
.end((err, res) => {
if (err) {
return done(err);
}
expect(res).to.have.status(201);
const fetchCall = fetchStub.getCall(0);
const requestBody = JSON.parse(fetchCall.args[1].body);
expect(requestBody.rolename).to.equal("testrole");

return done();
});
});
});
});
Loading
Loading