-
Notifications
You must be signed in to change notification settings - Fork 83
Merge develop-enterprise into develop (Entra ID SSO) #3032
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.
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
base: develop
Are you sure you want to change the base?
Conversation
| }); | ||
| router.post("/login", loginLimiter, loginUser); | ||
|
|
||
| router.post("/login-microsoft", loginUserWithMicrosoft); |
Check failure
Code scanning / CodeQL
Missing rate limiting High
authorization
Show autofix suggestion
Hide autofix suggestion
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.
-
Copy modified line R136
| @@ -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); | ||
|
|
Describe your changes
Write your issue number after "Fixes "
Enter the corresponding issue number after "Fixes #"
Please ensure all items are checked off before requesting a review: