Skip to content

refactor: remove short-circuit middleware from Discord invite route #2582

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.

Sign up for GitHub

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

Open
AnujChhikara wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

refactor: remove short-circuit middleware from Discord invite route #2582

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b69eaea
refactor: remove short-circuit middleware from Discord invite route
AnujChhikara Feb 24, 2026
b514976
refactor: remove commented short-circuit middleware for Discord invit…
AnujChhikara Feb 24, 2026
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
11 changes: 1 addition & 10 deletions routes/discordactions.js
View file
Open in desktop
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.
*/
Comment on lines -47 to -49
Copy link
Contributor

@prakashchoudhary07 prakashchoudhary07 Feb 25, 2026

Choose a reason for hiding this comment

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

This bug needs to be fixed before enabling this route, is it solved?

Copy link
Contributor Author

@AnujChhikara AnujChhikara Feb 25, 2026

Choose a reason for hiding this comment

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

No one can generate invite link until they have approved application in this new application cycle that we will start. So we are maintaining a Boolean value isNew:true for new application and all the previous application irrespective of status have isNew: false and before generating the link we verify isNew should be true otherwise you can not generate invite link

Copy link
Contributor

@prakashchoudhary07 prakashchoudhary07 Feb 26, 2026

Choose a reason for hiding this comment

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

@AnujChhikara So when does isNew become false?

Also, if that issue is fixed, please add the description, link the PR which fixes it, and update the status of it please

Copy link
Contributor Author

@AnujChhikara AnujChhikara Feb 26, 2026

Choose a reason for hiding this comment

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

We ran a migration script where in all of the old application we added isNew: false field. So if anyone have a approved application they can not generate invite code. They will be able to generate invite link if their application got approved this time as in this new application cycle we are using isNew:true field.

Copy link
Contributor

@prakashchoudhary07 prakashchoudhary07 Feb 26, 2026

Choose a reason for hiding this comment

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

By the way, are we not storing the invite link generation status? Is that not being used?

Copy link
Contributor Author

@AnujChhikara AnujChhikara Feb 26, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

When user created the invite link we store that in the discord invite collection and they can get that using this get api from where I am removing the disable route middleware. When someone try to generate invite again we check if invite exists in that collection if found then we throw error that's way they cannot generate more than one invite link

router.get("/invite", authenticate, getUserDiscordInvite);

Check failure

Code scanning / CodeQL

Missing rate limiting High

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

Copilot Autofix

AI 1 day ago

In general, this should be fixed by adding a rate‑limiting middleware to the affected route so that each client (e.g., IP) can only hit /invite a bounded number of times within a time window. In Express, a common solution is to use the well‑known express-rate-limit package, configure a limiter, and insert it into the middleware chain for the vulnerable route(s).

For this code, the least intrusive, best fix is:

  • Import express-rate-limit at the top of routes/discordactions.js.
  • Define a limiter specifically for the Discord invite endpoint, e.g., getUserInviteLimiter, configured with a reasonable windowMs and max appropriate for invite lookups (for example, a small number per minute or per 15 minutes).
  • Apply this limiter only to the GET /invite route by placing it between authenticate and getUserDiscordInvite in the middleware chain.
  • Leave all other routes unchanged, since CodeQL only complained about this one, and we don’t want to alter behavior elsewhere without need.

Concretely:

  • In routes/discordactions.js, add const rateLimit = require("express-rate-limit"); alongside the other require statements.
  • Below the router initialization (const router = express.Router();), define const getUserInviteLimiter = rateLimit({...}).
  • Update line 40 from router.get("/invite", authenticate, getUserDiscordInvite); to router.get("/invite", authenticate, getUserInviteLimiter, getUserDiscordInvite);.

This introduces rate limiting for the sensitive route without modifying existing authentication/authorization logic or controllers.

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,13 +31,21 @@
 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 getUserInviteLimiter = rateLimit({
+  windowMs: 15 * 60 * 1000, // 15 minutes
+  max: 50, // limit each IP to 50 requests per windowMs for GET /invite
+  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);
 router.post("/roles", authenticate, checkIsVerifiedDiscord, validateMemberRoleBody, addGroupRoleToMember);
-router.get("/invite", authenticate, getUserDiscordInvite);
+router.get("/invite", authenticate, getUserInviteLimiter, getUserDiscordInvite);
 router.post("/invite", authenticate, checkCanGenerateDiscordLink, generateInviteForUser);
 
 router.delete("/roles", authenticate, checkIsVerifiedDiscord, deleteRole);
EOF
@@ -31,13 +31,21 @@
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 getUserInviteLimiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 50, // limit each IP to 50 requests per windowMs for GET /invite
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);
router.post("/roles", authenticate, checkIsVerifiedDiscord, validateMemberRoleBody, addGroupRoleToMember);
router.get("/invite", authenticate, getUserDiscordInvite);
router.get("/invite", authenticate, getUserInviteLimiter, getUserDiscordInvite);
router.post("/invite", authenticate, checkCanGenerateDiscordLink, generateInviteForUser);

router.delete("/roles", authenticate, checkIsVerifiedDiscord, deleteRole);
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
Loading