refactor: remove short-circuit middleware from Discord invite route#2582
refactor: remove short-circuit middleware from Discord invite route#2582AnujChhikara wants to merge 2 commits intodevelopfrom
Conversation
WalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
routes/discordactions.js (1)
40-44:⚠️ Potential issue | 🟡 MinorRemove the now-stale "Short-circuit" comment block.
The comment at Lines 40–43 explicitly states "Short-circuit the GET method for this endpoint", which directly contradicts the intent of this PR (removing that short-circuit). Leaving it in place will mislead future readers into thinking the route is still disabled.
🧹 Proposed fix
-/** - * 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", authenticate, getUserDiscordInvite);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@routes/discordactions.js` around lines 40 - 44, Remove the stale comment block above the router.get("/invite", authenticate, getUserDiscordInvite) route in routes/discordactions.js: delete the lines that say "Short-circuit the GET method for this endpoint" and the referential URL so the comment no longer contradicts the current behavior of the route; leave the route registration (router.get(...)) and middleware/handler (authenticate, getUserDiscordInvite) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@routes/discordactions.js`:
- Line 44: The /invite GET route currently calls authenticate and
getUserDiscordInvite without rate limiting; import the rate limiter (exported
function n) from middlewares/rateLimiting.js and apply it to this route so the
handler becomes router.get("/invite", authenticate, n(), getUserDiscordInvite)
(or the appropriate middleware invocation signature) to protect
getUserDiscordInvite from enumeration and Firestore exhaustion.
---
Outside diff comments:
In `@routes/discordactions.js`:
- Around line 40-44: Remove the stale comment block above the
router.get("/invite", authenticate, getUserDiscordInvite) route in
routes/discordactions.js: delete the lines that say "Short-circuit the GET
method for this endpoint" and the referential URL so the comment no longer
contradicts the current behavior of the route; leave the route registration
(router.get(...)) and middleware/handler (authenticate, getUserDiscordInvite)
unchanged.
|
@coderabbitai can you please create an ticket for these changes where i removed the disable route middleware from the get discord invite route as new application are opening and user should be able to get their invite links |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 59
You can view the issue here: #2583 |
| * 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
Show autofix suggestion
Hide autofix suggestion
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-limitat the top ofroutes/discordactions.js. - Define a limiter specifically for the Discord invite endpoint, e.g.,
getUserInviteLimiter, configured with a reasonablewindowMsandmaxappropriate for invite lookups (for example, a small number per minute or per 15 minutes). - Apply this limiter only to the
GET /inviteroute by placing it betweenauthenticateandgetUserDiscordInvitein 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, addconst rateLimit = require("express-rate-limit");alongside the otherrequirestatements. - Below the router initialization (
const router = express.Router();), defineconst getUserInviteLimiter = rateLimit({...}). - Update line 40 from
router.get("/invite", authenticate, getUserDiscordInvite);torouter.get("/invite", authenticate, getUserInviteLimiter, getUserDiscordInvite);.
This introduces rate limiting for the sensitive route without modifying existing authentication/authorization logic or controllers.
| @@ -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); |
| @@ -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", |
| Package | Version | Security advisories |
| express-rate-limit (npm) | 8.2.1 | None |
|
@AnujChhikara @MayankBansal12 |
| * Short-circuit this POST method for this endpoint | ||
| * Refer https://github.com/Real-Dev-Squad/todo-action-items/issues/269 for more details. | ||
| */ |
There was a problem hiding this comment.
This bug needs to be fixed before enabling this route, is it solved?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
By the way, are we not storing the invite link generation status? Is that not being used?
There was a problem hiding this comment.
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
3 participants
Date: 25 Feb 2026
Developer Name: @AnujChhikara
Issue Ticket Number
Tech Doc Link
Business Doc Link
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Screen.Recording.2026-02-25.at.1.33.57.AM.mov