Skip to content

Commit 0852107

Browse files
committed
refactor(login): move rate limiter after successful login
Before, we weren't checking if a login was successful before counting it against the rate limiter. With this change, we only count unsuccessful logins against the rate limiter. We did this because this was a bug but also because it caused problems with our e2e tests hitting the rate limit.
1 parent 83cfbf8 commit 0852107

File tree

1 file changed

+6
-4
lines changed

1 file changed

+6
-4
lines changed

src/node/routes/login.ts

+6-4
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,6 @@ router.get("/", async (req, res) => {
5959

6060
router.post("/", async (req, res) => {
6161
try {
62-
if (!limiter.try()) {
63-
throw new Error("Login rate limited!")
64-
}
65-
6662
if (!req.body.password) {
6763
throw new Error("Missing password")
6864
}
@@ -84,6 +80,12 @@ router.post("/", async (req, res) => {
8480
return redirect(req, res, to, { to: undefined })
8581
}
8682

83+
// Note: successful logins should not count against the RateLimiter
84+
// which is why this logic must come after the successful login logic
85+
if (!limiter.try()) {
86+
throw new Error("Login rate limited!")
87+
}
88+
8789
console.error(
8890
"Failed login attempt",
8991
JSON.stringify({

0 commit comments

Comments
 (0)