feat: enhance Discord invite generation and role handling#2570
feat: enhance Discord invite generation and role handling#2570iamitprakash merged 7 commits intodevelopfrom
Conversation
WalkthroughThe changes refactor Discord role management to incorporate application approval workflows. They remove automatic role assignments from external account linking, introduce conditional role prefixing for group roles, add approval-based role propagation through middleware, and update corresponding tests and type definitions. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Routes as POST /invite
participant Auth as authenticate
participant CheckApproval as checkCanGenerateDiscordLink
participant Controller as generateInviteForUser
participant Discord as Discord API
Client->>Routes: POST /invite (authenticated)
Routes->>Auth: Verify authentication
Auth->>CheckApproval: Pass request
CheckApproval->>CheckApproval: Fetch approved application
alt Approved app exists and isNew=true
CheckApproval->>CheckApproval: Assign role to req.approvedApplicationRole
CheckApproval->>Controller: Pass request with role
else Approved app exists but isNew!=true
CheckApproval->>Client: 403 Forbidden
else No approved application
CheckApproval->>Client: 403 Forbidden
end
Controller->>Controller: Build inviteOptions with req.approvedApplicationRole
Controller->>Discord: Create invite with role
Discord->>Client: Return invite link
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
| * 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
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 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:
- Import
express-rate-limitat the top ofroutes/discordactions.js. - Create a specific limiter instance tailored for Discord invite generation (e.g., a small number of requests per minute/hour).
- 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 ingenerateInviteForUseris unchanged; we only restrict how frequently it can be reached.
Concretely:
-
Near the existing
requirestatements at the top, addconst 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 includesgenerateInviteLimiterin 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.
| @@ -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); |
| @@ -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 |
There was a problem hiding this comment.
Actionable comments posted: 4
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)
46-49:⚠️ Potential issue | 🟡 MinorStale comment: the short-circuit reference below is no longer accurate for POST /invite.
The comment block (Lines 46-49) about short-circuiting the POST method still references the disabled route pattern, but
disableRoutehas been removed from Line 50. Update or remove this comment.Proposed fix
-/** - * 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);
🤖 Fix all issues with AI agents
In `@middlewares/checkCanGenerateDiscordLink.ts`:
- Around line 31-33: The error message returned when an approved application
exists but isNew !== true is misleading; update the response in the
checkCanGenerateDiscordLink middleware (the branch that currently does return
res.boom.forbidden("No applications found.");) to a clear, accurate message such
as "Your approved application is not eligible for Discord invite generation."
Keep the same condition (approvedApplication.isNew !== true) but replace the
string to reflect that an application was found but is not eligible for Discord
invite generation.
- Line 35: The middleware checkCanGenerateDiscordLink assigns
approvedApplication.role directly but relies implicitly on isNew to ensure role
exists; add an explicit validation before assignment by checking
approvedApplication.role (e.g., if (!approvedApplication.role) return
res.boom.badRequest(...)) so the handler won’t proceed with an undefined role;
update the assignment of req.approvedApplicationRole to only run after this
guard and mention createApplicationService/joi.required() in the comment if you
want to keep the dependency documented.
In `@routes/discordactions.js`:
- Line 50: Add a rate limiter to the re-enabled POST /invite route to prevent
abuse; update the router.post call that currently composes authenticate,
checkCanGenerateDiscordLink, and generateInviteForUser to include the existing
commonRateLimiter middleware (or a new stricter per-user limiter) before
generateInviteForUser so requests are throttled per authenticated user, ensuring
the limiter uses user identity (from authenticate) and a tighter config than
global defaults because generateInviteForUser triggers external Discord API
calls, RS256 JWT signing, and Firestore writes.
In `@test/integration/discordactions.test.js`:
- Around line 1544-1568: The test name claims it handles string 'true' and
'false' but only sends role=true; update the test to either rename it to reflect
only the 'true' case (change the it(...) description) or extend it to include
the false case by making a second POST to "/discord-actions/groups?role=false"
(reuse roleData) and assert the created rolename includes the "group-" prefix
via the fetchStub call and parsed requestBody. Ensure you verify both
fetchStub.getCall indices (or reset/assert call counts) so you read the correct
call.args[1].body for each request and update expectations accordingly.
Date: 12 feb, 2026
Developer Name: @AnujChhikara
Issue Ticket Number
Tech Doc Link
NA
Business Doc Link
NA
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
role-assignment.mp4
verify.mp4
Test Coverage
Screenshot 1
Additional Notes