Different usernames not allowed when correct user credentials applied, after some of wrong credentials with particular username password

[MohamedNashad]

import ErrorMessages from "./constants";
import { CommonResponseWithError } from "./response";
import { timeZoneName } from "./timezone.constants";
import { AuthSettings } from "../models";

const rateLimit = require("express-rate-limit");

export async function loginLimiter() {
  try {
    const attempts = await AuthSettings.findAll();
    let maxAuthAttemptsValue: number = 0;
    let maxTimeLimitValue: number = 0;

    for (const setting of attempts) {
      if (setting.name === "max_no_of_login_attempts") {
        maxAuthAttemptsValue = Number(setting.value);
      } else if (setting.name === "login_attempts_exceed_time_limit_minutes") {
        maxTimeLimitValue = Number(setting.value);
      }
    }

    return rateLimit({
      max: maxAuthAttemptsValue !== 0 && maxAuthAttemptsValue,
      windowMs: maxTimeLimitValue !== 0 && maxTimeLimitValue * 60 * 1000,

      handler: (req: any, res: any, next: any) => {
        const currentTime = new Date();
        const resetTime = new Date(
          currentTime.getTime() + maxTimeLimitValue * 60 * 1000
        );

        const formattedResetTime = resetTime.toLocaleTimeString("en-US", {
          timeZone: timeZoneName, // Assuming timezone is sent in headers
          timeStyle: "medium",
        });

        let message: string = `${ErrorMessages.tooManyAttempts} ${maxTimeLimitValue} minutes`;

        let errorMsg = {
          name: "Error",
          errors: [{ message: message }],
        };
        let response = new CommonResponseWithError(
          {},
          message,
          errorMsg,
          false
        );

        res.status(429).send(response);
      },
      requestWasSuccessful: (request: any, response: any) =>
        response.statusCode < 400,
      skipSuccessfulRequests: true,
    });
  } catch (error) {
    console.error("Error fetching authentication settings:", error);
    throw error;
  }
}

2. routes

(async () => {
      try {
        const limitMiddleware = await loginLimiter();
        this.app.post(
          "/login",
          limitMiddleware,
          mul.none(),
          UserController.login
        );
      } catch (error) {
        console.error("Error setting up login limiter:", error);
      }
    })();

the limiter applied globally for all users which is not good, when i try with wrong credentials it blocks the login for certain amount of time as expected, then if i try with different username and password which is completely different from the previously tried credentials it doesn't allow the user to login.

for instance,

payload:

wrong credentials used .
{
"username":"johnss",
"password":"12345"
}

another user correct credentials,

{
"username":"kevin",
"password":"4825"
}

[nfriedly]

For the main issue, I think I see what's happening: skipSuccessfulRequests only works for IPs that aren't already blocked. Once the IP is blocked, it doesn't get to the point where it can evaluate the credentials.

You could add a custom keyGenerator() that takes the username into account, but I wouldn't recommend it, as it would leave you vulnerable to certain forms of attack (e.g. try a whole bunch of different username & password combos from a leaked database.)

Ultimately, I think you should just accept that once an IP is blocked for too many invalid username/password combos, they stay blocked until the timer expires.

---

Also, just as an aside, this code will set the max and windowMs values to false if maxAuthAttemptsValue and maxTimeLimitValue are 0 respectively:

      max: maxAuthAttemptsValue !== 0 && maxAuthAttemptsValue,
      windowMs: maxTimeLimitValue !== 0 && maxTimeLimitValue * 60 * 1000,

I think you should settle on some reasonable default value and then do something like this:

      max: maxAuthAttemptsValue  || 5,
      windowMs: (maxTimeLimitValue || 1) * 60 * 1000,

That would use the maxAuthAttemptsValue if it's non-zero, or else default to 5, and similarly use a default of 1 for maxTimeLimitValue