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
16 changes: 13 additions & 3 deletions controllers/discordactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -488,9 +488,15 @@ const setRoleToUsersWith31DaysPlusOnboarding = async (req, res) => {
const generateInviteForUser = async (req, res) => {
try {
const { userId } = req.query;
const isSuperUser = req.userData.roles.super_user;

const applicationId = req.approvedApplicationId || (isSuperUser ? req.query.applicationId : undefined);
const role = req.approvedApplicationRole || (isSuperUser ? req.query.role : undefined);

if (!applicationId || !role) return res.boom.forbidden("Application data is required to generate an invite.");
const userIdForInvite = userId || req.userData.id;

const modelResponse = await discordRolesModel.getUserDiscordInvite(userIdForInvite);
const modelResponse = await discordRolesModel.getUserDiscordInviteByApplication(userIdForInvite, applicationId);

if (!modelResponse.notFound) {
return res.status(409).json({
Expand All @@ -506,7 +512,7 @@ const generateInviteForUser = async (req, res) => {

const inviteOptions = {
channelId: channelId,
role: req.approvedApplicationRole,
role,
};
const response = await fetch(`${DISCORD_BASE_URL}/invite`, {
method: "POST",
Expand All @@ -518,7 +524,11 @@ const generateInviteForUser = async (req, res) => {
const inviteCode = discordInviteResponse.data.code;
const inviteLink = `discord.gg/${inviteCode}`;

await discordRolesModel.addInviteToInviteModel({ userId: userIdForInvite, inviteLink });
await discordRolesModel.addInviteToInviteModel({
userId: userIdForInvite,
inviteLink,
applicationId,
});

return res.status(201).json({
message: "invite generated successfully",
Expand Down
1 change: 1 addition & 0 deletions middlewares/checkCanGenerateDiscordLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const checkCanGenerateDiscordLink = async (req: CustomRequest, res: CustomRespon
}

req.approvedApplicationRole = approvedApplication.role;
req.approvedApplicationId = approvedApplication.id;
return next();
} catch (error) {
return res.boom.badImplementation("An error occurred while checking user applications.");
Expand Down
21 changes: 20 additions & 1 deletion models/discordactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,7 @@ const getUserDiscordInvite = async (userId) => {
return { notFound: true };
}
} catch (err) {
logger.log("error in getting user invite", err);
logger.error("error in getting user invite", err);
throw err;
}
};
Expand All @@ -1264,6 +1264,24 @@ const groupUpdateLastJoinDate = async ({ id }) => {
return { updated: true };
};

const getUserDiscordInviteByApplication = async (userId, applicationId) => {
try {
const invite = await discordInvitesModel
.where("userId", "==", userId)
.where("applicationId", "==", applicationId)
.limit(1)
.get();
const [inviteDoc] = invite.docs;
if (inviteDoc) {
return { id: inviteDoc.id, ...inviteDoc.data(), notFound: false };
}
return { notFound: true };
} catch (err) {
logger.error("error in getting user invite by application", err);
throw err;
}
};

module.exports = {
createNewRole,
removeMemberGroup,
Expand All @@ -1288,4 +1306,5 @@ module.exports = {
groupUpdateLastJoinDate,
deleteGroupRole,
skipOnboardingUsersHavingApprovedExtensionRequest,
getUserDiscordInviteByApplication,
};
11 changes: 1 addition & 10 deletions routes/discordactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,13 @@
const { Services } = require("../constants/bot");
const { verifyCronJob } = require("../middlewares/authorizeBot");
const { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndService");
const { disableRoute } = require("../middlewares/shortCircuit");
const router = express.Router();

router.post("/groups", authenticate, checkIsVerifiedDiscord, validateGroupRoleBody, createGroupRole);
router.get("/groups", authenticate, checkIsVerifiedDiscord, validateLazyLoadingParams, getPaginatedAllGroupRoles);
router.delete("/groups/:groupId", authenticate, checkIsVerifiedDiscord, authorizeRoles([SUPERUSER]), deleteGroupRole);
router.post("/roles", authenticate, checkIsVerifiedDiscord, validateMemberRoleBody, addGroupRoleToMember);
/**
* Short-circuit the GET method for this endpoint
* Refer https://github.com/Real-Dev-Squad/todo-action-items/issues/269 for more details.
*/
router.get("/invite", disableRoute, authenticate, getUserDiscordInvite);
/**
* Short-circuit this POST method for this endpoint
* Refer https://github.com/Real-Dev-Squad/todo-action-items/issues/269 for more details.
*/
router.get("/invite", authenticate, getUserDiscordInvite);

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 about 14 hours ago

In general, the fix is to add a rate-limiting middleware to the sensitive route(s) so that repeated requests from the same client (or IP) are capped within a time window. In Express, this is typically done using a library like express-rate-limit, configured with reasonable windowMs and max settings, and then passed as a middleware in the route definition.

For this file, the minimal, targeted change that preserves existing behavior is:

  • Import express-rate-limit at the top of routes/discordactions.js.
  • Define one or more limiter instances, for example inviteLimiter, configured specifically for the /invite endpoints (and optionally reuse the same limiter for both GET and POST on /invite).
  • Apply that limiter as a middleware before authenticate on the /invite routes (line 40 and 41). This ensures requests are rate-limited even if authentication fails and keeps the rest of the pipeline unchanged.
  • Leave all other routes untouched unless we want broader protection; to keep the change minimal and precisely aligned with the alert, we will limit it to the invite routes.

Concretely:

  • Add a const rateLimit = require("express-rate-limit"); near the other require calls.
  • Add a new const inviteLimiter = rateLimit({ ... }) after the imports, with a conservative configuration such as a few tens of requests per 15 minutes.
  • Update router.get("/invite", authenticate, getUserDiscordInvite); to include inviteLimiter (e.g., router.get("/invite", inviteLimiter, authenticate, getUserDiscordInvite);), and similarly for the POST /invite route.
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
@@ -31,14 +31,20 @@
 const { Services } = require("../constants/bot");
 const { verifyCronJob } = require("../middlewares/authorizeBot");
 const { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndService");
+const rateLimit = require("express-rate-limit");
 const router = express.Router();
 
+const inviteLimiter = rateLimit({
+  windowMs: 15 * 60 * 1000, // 15 minutes
+  max: 50, // limit each IP to 50 invite requests per windowMs
+});
+
 router.post("/groups", authenticate, checkIsVerifiedDiscord, validateGroupRoleBody, createGroupRole);
 router.get("/groups", authenticate, checkIsVerifiedDiscord, validateLazyLoadingParams, getPaginatedAllGroupRoles);
 router.delete("/groups/:groupId", authenticate, checkIsVerifiedDiscord, authorizeRoles([SUPERUSER]), deleteGroupRole);
 router.post("/roles", authenticate, checkIsVerifiedDiscord, validateMemberRoleBody, addGroupRoleToMember);
-router.get("/invite", authenticate, getUserDiscordInvite);
-router.post("/invite", authenticate, checkCanGenerateDiscordLink, generateInviteForUser);
+router.get("/invite", inviteLimiter, authenticate, getUserDiscordInvite);
+router.post("/invite", inviteLimiter, authenticate, checkCanGenerateDiscordLink, generateInviteForUser);
 
 router.delete("/roles", authenticate, checkIsVerifiedDiscord, deleteRole);
 router.get("/roles", authenticate, checkIsVerifiedDiscord, getGroupsRoleId);
EOF
@@ -31,14 +31,20 @@
const { Services } = require("../constants/bot");
const { verifyCronJob } = require("../middlewares/authorizeBot");
const { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndService");
const rateLimit = require("express-rate-limit");
const router = express.Router();

const inviteLimiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 50, // limit each IP to 50 invite requests per windowMs
});

router.post("/groups", authenticate, checkIsVerifiedDiscord, validateGroupRoleBody, createGroupRole);
router.get("/groups", authenticate, checkIsVerifiedDiscord, validateLazyLoadingParams, getPaginatedAllGroupRoles);
router.delete("/groups/:groupId", authenticate, checkIsVerifiedDiscord, authorizeRoles([SUPERUSER]), deleteGroupRole);
router.post("/roles", authenticate, checkIsVerifiedDiscord, validateMemberRoleBody, addGroupRoleToMember);
router.get("/invite", authenticate, getUserDiscordInvite);
router.post("/invite", authenticate, checkCanGenerateDiscordLink, generateInviteForUser);
router.get("/invite", inviteLimiter, authenticate, getUserDiscordInvite);
router.post("/invite", inviteLimiter, 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.post("/invite", authenticate, checkCanGenerateDiscordLink, generateInviteForUser);

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

describe("POST /discord-actions/invite (application-scoped)", function () {
it("should create invite and return 201 for an eligible non-super user", async function () {
sinon
.stub(ApplicationModel, "getUserApplications")
.resolves([{ id: "app-1", status: "accepted", role: "developer", isNew: true }]);
sinon.stub(discordRolesModel, "getUserDiscordInviteByApplication").resolves({ notFound: true });
sinon.stub(discordRolesModel, "addInviteToInviteModel").resolves("invite-doc-id");
fetchStub.returns(
Promise.resolve({
status: 201,
json: () => Promise.resolve({ data: { code: "new-code" } }),
})
);

const res = await chai
.request(app)
.post("/discord-actions/invite")
.set("cookie", `${cookieName}=${userAuthToken}`);

expect(res).to.have.status(201);
expect(res.body.message).to.be.equal("invite generated successfully");
expect(res.body.inviteLink).to.be.equal("discord.gg/new-code");
});

it("should return 409 when invite already exists for the same application", async function () {
sinon
.stub(ApplicationModel, "getUserApplications")
.resolves([{ id: "app-1", status: "accepted", role: "developer", isNew: true }]);
sinon.stub(discordRolesModel, "getUserDiscordInviteByApplication").resolves({
notFound: false,
id: "invite-doc-id",
userId,
applicationId: "app-1",
inviteLink: "discord.gg/existing",
});

const res = await chai
.request(app)
.post("/discord-actions/invite")
.set("cookie", `${cookieName}=${userAuthToken}`);

expect(res).to.have.status(409);
expect(res.body.message).to.be.equal("User invite is already present!");
});

it("should return 403 for super user when application data is not provided", async function () {
const res = await chai
.request(app)
.post("/discord-actions/invite")
.set("cookie", `${cookieName}=${superUserAuthToken}`);

expect(res).to.have.status(403);
expect(res.body.message).to.be.equal("Application data is required to generate an invite.");
});

it("should allow super user to create invite when application data is provided", async function () {
sinon.stub(discordRolesModel, "getUserDiscordInviteByApplication").resolves({ notFound: true });
sinon.stub(discordRolesModel, "addInviteToInviteModel").resolves("invite-doc-id");
fetchStub.returns(
Promise.resolve({
status: 201,
json: () => Promise.resolve({ data: { code: "super-code" } }),
})
);

const res = await chai
.request(app)
.post("/discord-actions/invite?applicationId=app-super-1&role=developer")
.set("cookie", `${cookieName}=${superUserAuthToken}`);

expect(res).to.have.status(201);
expect(res.body.message).to.be.equal("invite generated successfully");
expect(res.body.inviteLink).to.be.equal("discord.gg/super-code");
});
});

describe("PUT /discord-actions/group-idle", function () {
let allIds;

Expand Down
39 changes: 39 additions & 0 deletions test/unit/models/discordactions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const {
getMissedProgressUpdatesUsers,
addInviteToInviteModel,
getUserDiscordInvite,
getUserDiscordInviteByApplication,
groupUpdateLastJoinDate,
updateIdleUsersOnDiscord,
updateIdle7dUsersOnDiscord,
Expand Down Expand Up @@ -877,6 +878,10 @@ describe("discordactions", function () {
await addInviteToInviteModel(inviteObject);
});

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

it("should return invite for the user when the userId of a user is passed at it exists in the db", async function () {
const invite = await getUserDiscordInvite("kfjkasdfl");
expect(invite).to.have.property("id");
Expand All @@ -891,6 +896,40 @@ describe("discordactions", function () {
});
});

describe("getUserDiscordInviteByApplication", function () {
afterEach(async function () {
await cleanDb();
});

it("should return invite when userId and applicationId match an existing doc", async function () {
const userId = "user-app-invite";
const applicationId = "app-123";
await addInviteToInviteModel({
userId,
inviteLink: "discord.gg/abc",
applicationId,
createdAt: new Date().toISOString(),
});
const invite = await getUserDiscordInviteByApplication(userId, applicationId);
expect(invite.notFound).to.be.equal(false);
expect(invite.userId).to.be.equal(userId);
expect(invite.applicationId).to.be.equal(applicationId);
expect(invite.inviteLink).to.be.equal("discord.gg/abc");
});

it("should return notFound when no invite exists for that applicationId", async function () {
const invite = await getUserDiscordInviteByApplication("nonexistent-user", "app-456");
expect(invite.notFound).to.be.equal(true);
});

it("should return notFound for applicationId when only legacy doc without applicationId exists", async function () {
const userId = "user-legacy-only";
await addInviteToInviteModel({ userId, inviteLink: "discord.gg/legacy" });
const invite = await getUserDiscordInviteByApplication(userId, "new-app-id");
expect(invite.notFound).to.be.equal(true);
});
});

describe("groupUpdateLastJoinDate", function () {
beforeEach(function () {
sinon.stub(discordRoleModel, "doc").returns({
Expand Down
6 changes: 5 additions & 1 deletion types/global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,8 @@ export type userData = {
};

export type CustomResponse = Response & { boom: Boom };
export type CustomRequest = Request & { userData; approvedApplicationRole?: string };
export type CustomRequest = Request & {
userData;
approvedApplicationRole?: string;
approvedApplicationId?: string;
};
Loading