Skip to content

Conversation

@HarshP4585
Copy link
Collaborator

Describe your changes

  • Merge develop-enterprise (Entra ID SSO) into develop

Write your issue number after "Fixes "

Enter the corresponding issue number after "Fixes #"

Please ensure all items are checked off before requesting a review:

  • I deployed the code locally.
  • I have performed a self-review of my code.
  • I have included the issue # in the PR.
  • I have labelled the PR correctly.
  • The issue I am working on is assigned to me.
  • I have avoided using hardcoded values to ensure scalability and maintain consistency across the application.
  • I have ensured that font sizes, color choices, and other UI elements are referenced from the theme.
  • My pull request is focused and addresses a single, specific feature.
  • If there are UI changes, I have attached a screenshot or video to this PR.

@HarshP4585 HarshP4585 marked this pull request as ready for review January 9, 2026 22:03
@HarshP4585 HarshP4585 requested a review from gorkem-bwl January 9, 2026 22:03
@HarshP4585 HarshP4585 marked this pull request as draft January 9, 2026 22:05
});
router.post("/login", loginLimiter, loginUser);

router.post("/login-microsoft", loginUserWithMicrosoft);

Check failure

Code scanning / CodeQL

Missing rate limiting High

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

Copilot Autofix

AI about 1 month ago

In general, each authentication or authorization endpoint that may trigger expensive operations (database queries, password processing, external IdP calls) should be protected with an appropriate rate‑limiting middleware. This is already done for /login via a dedicated loginLimiter and for other sensitive endpoints via authLimiter. The fix is to ensure /login-microsoft is also wrapped with a rate limiter.

The most consistent and least intrusive fix is to reuse an existing limiter (authLimiter or loginLimiter) rather than inventing a new pattern. Since /login-microsoft is a login endpoint, reusing the same loginLimiter used for /login makes sense and preserves behavior: both login routes will now share the same “5 requests per minute per IP” policy and error message. Technically, this only requires updating the route definition on line 136 to insert loginLimiter as middleware. No new imports or helper functions are needed because loginLimiter is declared just above and rateLimit is already imported.

Concretely, in Servers/routes/user.route.ts, modify the /login-microsoft route so that:

router.post("/login-microsoft", loginLimiter, loginUserWithMicrosoft);

replaces the original line router.post("/login-microsoft", loginUserWithMicrosoft);. No other lines need to change.

Suggested changeset 1
Servers/routes/user.route.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Servers/routes/user.route.ts b/Servers/routes/user.route.ts
--- a/Servers/routes/user.route.ts
+++ b/Servers/routes/user.route.ts
@@ -133,7 +133,7 @@
 });
 router.post("/login", loginLimiter, loginUser);
 
-router.post("/login-microsoft", loginUserWithMicrosoft);
+router.post("/login-microsoft", loginLimiter, loginUserWithMicrosoft);
 
 router.post("/refresh-token", authLimiter, refreshAccessToken);
 
EOF
@@ -133,7 +133,7 @@
});
router.post("/login", loginLimiter, loginUser);

router.post("/login-microsoft", loginUserWithMicrosoft);
router.post("/login-microsoft", loginLimiter, loginUserWithMicrosoft);

router.post("/refresh-token", authLimiter, refreshAccessToken);

Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants