fix(auth): normalise email to lowercase before rate-limit key lookup
Case variants of the same address (e.g. User@EXAMPLE.COM vs user@example.com) now share a single Bucket4j bucket, preventing a trivial bypass of per-email limits via mixed-case submissions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -10,6 +10,7 @@ import org.raddatz.familienarchiv.exception.ErrorCode;
|
|||||||
import org.springframework.stereotype.Service;
|
import org.springframework.stereotype.Service;
|
||||||
|
|
||||||
import java.time.Duration;
|
import java.time.Duration;
|
||||||
|
import java.util.Locale;
|
||||||
import java.util.concurrent.TimeUnit;
|
import java.util.concurrent.TimeUnit;
|
||||||
|
|
||||||
@Service
|
@Service
|
||||||
@@ -41,20 +42,21 @@ public class LoginRateLimiter {
|
|||||||
// For the current single-VPS setup this is the correct, simplest implementation.
|
// For the current single-VPS setup this is the correct, simplest implementation.
|
||||||
|
|
||||||
public void checkAndConsume(String ip, String email) {
|
public void checkAndConsume(String ip, String email) {
|
||||||
if (!byIpEmail.get(ip + ":" + email).tryConsume(1)) {
|
String key = ip + ":" + email.toLowerCase(Locale.ROOT);
|
||||||
|
if (!byIpEmail.get(key).tryConsume(1)) {
|
||||||
throw DomainException.tooManyRequests(ErrorCode.TOO_MANY_LOGIN_ATTEMPTS,
|
throw DomainException.tooManyRequests(ErrorCode.TOO_MANY_LOGIN_ATTEMPTS,
|
||||||
"Too many login attempts from " + ip);
|
"Too many login attempts from " + ip);
|
||||||
}
|
}
|
||||||
if (!byIp.get(ip).tryConsume(1)) {
|
if (!byIp.get(ip).tryConsume(1)) {
|
||||||
// Refund the ipEmail token so IP-level blocking does not erode the per-email quota.
|
// Refund the ipEmail token so IP-level blocking does not erode the per-email quota.
|
||||||
byIpEmail.get(ip + ":" + email).addTokens(1);
|
byIpEmail.get(key).addTokens(1);
|
||||||
throw DomainException.tooManyRequests(ErrorCode.TOO_MANY_LOGIN_ATTEMPTS,
|
throw DomainException.tooManyRequests(ErrorCode.TOO_MANY_LOGIN_ATTEMPTS,
|
||||||
"Too many login attempts from " + ip);
|
"Too many login attempts from " + ip);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
public void invalidateOnSuccess(String ip, String email) {
|
public void invalidateOnSuccess(String ip, String email) {
|
||||||
byIpEmail.invalidate(ip + ":" + email);
|
byIpEmail.invalidate(ip + ":" + email.toLowerCase(Locale.ROOT));
|
||||||
byIp.invalidate(ip);
|
byIp.invalidate(ip);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -78,6 +78,30 @@ class LoginRateLimiterTest {
|
|||||||
() -> rateLimiter.checkAndConsume("1.2.3.4", "other@example.com"));
|
() -> rateLimiter.checkAndConsume("1.2.3.4", "other@example.com"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void email_lookup_is_case_insensitive_so_mixed_case_shares_the_same_bucket() {
|
||||||
|
for (int i = 0; i < 10; i++) {
|
||||||
|
rateLimiter.checkAndConsume("1.2.3.4", "User@Example.COM");
|
||||||
|
}
|
||||||
|
|
||||||
|
assertThatThrownBy(() -> rateLimiter.checkAndConsume("1.2.3.4", "user@example.com"))
|
||||||
|
.isInstanceOf(DomainException.class)
|
||||||
|
.satisfies(ex -> org.assertj.core.api.Assertions.assertThat(((DomainException) ex).getCode())
|
||||||
|
.isEqualTo(ErrorCode.TOO_MANY_LOGIN_ATTEMPTS));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void invalidateOnSuccess_is_case_insensitive_so_mixed_case_clears_the_bucket() {
|
||||||
|
for (int i = 0; i < 10; i++) {
|
||||||
|
rateLimiter.checkAndConsume("1.2.3.4", "user@example.com");
|
||||||
|
}
|
||||||
|
|
||||||
|
rateLimiter.invalidateOnSuccess("1.2.3.4", "User@Example.COM");
|
||||||
|
|
||||||
|
assertThatNoException().isThrownBy(
|
||||||
|
() -> rateLimiter.checkAndConsume("1.2.3.4", "user@example.com"));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void ip_exhaustion_does_not_consume_ipEmail_tokens_for_blocked_attempts() {
|
void ip_exhaustion_does_not_consume_ipEmail_tokens_for_blocked_attempts() {
|
||||||
// Use a tighter limiter so the phantom-consumption effect is observable.
|
// Use a tighter limiter so the phantom-consumption effect is observable.
|
||||||
|
|||||||
Reference in New Issue
Block a user