Merge pull request #891 from numerique-gouv/fix-misconfigured-rate-li…

…miter

Fix misconfigured rate limiter

cypress/e2e/reset_password/env.conf

@@ -0,0 +1 @@
+FEATURE_RATE_LIMIT=True

cypress/e2e/reset_password/index.cy.ts

@@ -59,4 +59,20 @@ describe("sign-in with magic link", () => {
 
     cy.contains("Votre compte ProConnect");
   });
+
+  it("should trigger totp rate limiting", function () {
+    // Set email in unauthenticated session
+    cy.visit("/users/start-sign-in");
+    cy.get('[name="login"]').type("unused@yopmail.com");
+    cy.get('[type="submit"]').click();
+
+    // trigger reset password rate limiter
+    for (let i = 0; i <= 5; i++) {
+      cy.visit("/users/reset-password");
+
+      cy.get('[action="/users/reset-password"] [type="submit"]').click();
+    }
+
+    cy.contains("Too Many Requests");
+  });
 });

src/controllers/user/update-password.ts

@@ -12,7 +12,10 @@ import {
   getUserFromAuthenticatedSession,
   isWithinAuthenticatedSession,
 } from "../../managers/session/authenticated";
-import { getEmailFromUnauthenticatedSession } from "../../managers/session/unauthenticated";
+import {
+  getEmailFromUnauthenticatedSession,
+  setEmailInUnauthenticatedSession,
+} from "../../managers/session/unauthenticated";
 import { changePassword, sendResetPasswordEmail } from "../../managers/user";
 import { csrfToken } from "../../middlewares/csrf-protection";
 import { emailSchema } from "../../services/custom-zod-schemas";
@@ -28,11 +31,9 @@ export const getResetPasswordController = async (
     return res.render("user/reset-password", {
       pageTitle: "Réinitialiser mon mot de passe",
       notifications: await getNotificationsFromRequest(req),
-      loginHint:
-        getEmailFromUnauthenticatedSession(req) ||
-        (isWithinAuthenticatedSession(req.session)
-          ? getUserFromAuthenticatedSession(req).email
-          : null),
+      loginHint: isWithinAuthenticatedSession(req.session)
+        ? getUserFromAuthenticatedSession(req).email
+        : getEmailFromUnauthenticatedSession(req) || null,
       csrfToken: csrfToken(req),
     });
   } catch (error) {
@@ -59,6 +60,10 @@ export const postResetPasswordController = async (
       const parsedBody = await schema.parseAsync(req.body);
 
       email = parsedBody.login;
+
+      // When the user is redirected to start of the sign-in process, the email value will be updated accordingly.
+      // The email rate limiter will rely on the email value set in the session here.
+      setEmailInUnauthenticatedSession(req, email);
     }
 
     await sendResetPasswordEmail(email, MONCOMPTEPRO_HOST);

src/middlewares/rate-limiter.ts

@@ -50,42 +50,59 @@ const emailRateLimiterMiddlewareFactory =
     }
   };
 
-const defaultRateLimiter = new RateLimiterRedis({
-  storeClient: redisClient,
-  keyPrefix: "rate-limiter",
-  points: 20, // 20 requests
-  duration: 60, // per minute per IP
-});
-
-export const rateLimiterMiddleware =
-  ipRateLimiterMiddlewareFactory(defaultRateLimiter);
-
-const apiRateLimiter = new RateLimiterRedis({
-  storeClient: redisClient,
-  keyPrefix: "rate-limiter-api",
-  points: 42, // 4 API requests
-  duration: 1, // per second per IP
-});
+export const rateLimiterMiddleware = ipRateLimiterMiddlewareFactory(
+  new RateLimiterRedis({
+    storeClient: redisClient,
+    keyPrefix: "rate-limiter",
+    points: 20, // 20 requests
+    duration: 60, // per minute per IP
+  }),
+);
 
-export const apiRateLimiterMiddleware =
-  ipRateLimiterMiddlewareFactory(apiRateLimiter);
+export const apiRateLimiterMiddleware = ipRateLimiterMiddlewareFactory(
+  new RateLimiterRedis({
+    storeClient: redisClient,
+    keyPrefix: "rate-limiter-api",
+    points: 42, // 42 API requests
+    duration: 1, // per second per IP
+  }),
+);
 
-const passwordRateLimiter = new RateLimiterRedis({
-  storeClient: redisClient,
-  keyPrefix: "rate-limiter-password",
-  points: 10, // 10 requests
-  duration: 5 * 60, // per 5 minutes per email
-});
+export const passwordRateLimiterMiddleware = emailRateLimiterMiddlewareFactory(
+  new RateLimiterRedis({
+    storeClient: redisClient,
+    keyPrefix: "rate-limiter-password",
+    points: 10, // 10 requests
+    duration: 5 * 60, // per 5 minutes per email
+  }),
+);
 
-export const passwordRateLimiterMiddleware =
-  emailRateLimiterMiddlewareFactory(passwordRateLimiter);
+export const authenticatorRateLimiterMiddleware =
+  emailRateLimiterMiddlewareFactory(
+    new RateLimiterRedis({
+      storeClient: redisClient,
+      keyPrefix: "rate-limiter-totp",
+      points: 5, // 5 requests
+      duration: 15 * 60, // per 15 minutes per email
+    }),
+  );
 
-const authenticatorRateLimiter = new RateLimiterRedis({
-  storeClient: redisClient,
-  keyPrefix: "rate-limiter-totp",
-  points: 5, // 5 requests
-  duration: 15 * 60, // per 15 minutes per email
-});
+export const resetPasswordRateLimiterMiddleware =
+  emailRateLimiterMiddlewareFactory(
+    new RateLimiterRedis({
+      storeClient: redisClient,
+      keyPrefix: "rate-limiter-reset-password",
+      points: 5, // 5 requests
+      duration: 15 * 60, // per 15 minutes per email
+    }),
+  );
 
-export const authenticatorRateLimiterMiddleware =
-  emailRateLimiterMiddlewareFactory(authenticatorRateLimiter);
+export const sendMagicLinkRateLimiterMiddleware =
+  emailRateLimiterMiddlewareFactory(
+    new RateLimiterRedis({
+      storeClient: redisClient,
+      keyPrefix: "rate-limiter-send-magic-link",
+      points: 5, // 5 requests
+      duration: 15 * 60, // per 15 minutes per email
+    }),
+  );

src/routers/user.ts

@@ -67,6 +67,8 @@ import {
   authenticatorRateLimiterMiddleware,
   passwordRateLimiterMiddleware,
   rateLimiterMiddleware,
+  resetPasswordRateLimiterMiddleware,
+  sendMagicLinkRateLimiterMiddleware,
 } from "../middlewares/rate-limiter";
 import {
   checkCredentialPromptRequirementsMiddleware,
@@ -208,6 +210,7 @@ export const userRouter = () => {
     rateLimiterMiddleware,
     checkCredentialPromptRequirementsMiddleware,
     csrfProtectionMiddleware,
+    sendMagicLinkRateLimiterMiddleware,
     postSendMagicLinkController,
     checkUserSignInRequirementsMiddleware,
     issueSessionOrRedirectController,
@@ -255,6 +258,7 @@ export const userRouter = () => {
     "/reset-password",
     rateLimiterMiddleware,
     csrfProtectionMiddleware,
+    resetPasswordRateLimiterMiddleware,
     postResetPasswordController,
   );
   userRouter.get(