Migrate to 429 for all ratelimit responses

This commit is contained in:
Katherine
2024-08-05 12:02:11 -07:00
committed by GitHub
parent 10d559bbb5
commit 0e4625ef88
33 changed files with 63 additions and 110 deletions

View File

@@ -102,9 +102,8 @@ public class BackupAuthManager {
return CompletableFuture.completedFuture(null);
}
return RateLimiter.adaptLegacyException(rateLimiters
.forDescriptor(RateLimiters.For.SET_BACKUP_ID)
.validateAsync(account.getUuid()))
return rateLimiters.forDescriptor(RateLimiters.For.SET_BACKUP_ID)
.validateAsync(account.getUuid())
.thenCompose(ignored -> this.accountsManager
.updateAsync(account, acc -> acc.setBackupCredentialRequest(serializedRequest))
.thenRun(Util.NOOP))

View File

@@ -156,9 +156,8 @@ public class BackupManager {
final AuthenticatedBackupUser backupUser) {
checkBackupLevel(backupUser, BackupLevel.MEDIA);
return RateLimiter.adaptLegacyException(rateLimiters
.forDescriptor(RateLimiters.For.BACKUP_ATTACHMENT)
.validateAsync(rateLimitKey(backupUser))).thenApply(ignored -> {
return rateLimiters.forDescriptor(RateLimiters.For.BACKUP_ATTACHMENT)
.validateAsync(rateLimitKey(backupUser)).thenApply(ignored -> {
final byte[] bytes = new byte[15];
secureRandom.nextBytes(bytes);
final String attachmentKey = Base64.getUrlEncoder().encodeToString(bytes);

View File

@@ -134,7 +134,7 @@ public class AccountControllerV2 {
// Only verify and check reglock if there's a data change to be made...
if (!authenticatedAccount.getAccount().getNumber().equals(number)) {
RateLimiter.adaptLegacyException(() -> rateLimiters.getRegistrationLimiter().validate(number));
rateLimiters.getRegistrationLimiter().validate(number);
final PhoneVerificationRequest.VerificationType verificationType = phoneVerificationTokenManager.verify(number,
request);

View File

@@ -28,28 +28,21 @@ public class RateLimitExceededException extends Exception implements Convertible
@Nullable
private final Duration retryDuration;
private final boolean legacy;
/**
* Constructs a new exception indicating when it may become safe to retry
*
* @param retryDuration A duration to wait before retrying, null if no duration can be indicated
* @param legacy whether to use a legacy status code when mapping the exception to an HTTP response
*/
public RateLimitExceededException(@Nullable final Duration retryDuration, final boolean legacy) {
public RateLimitExceededException(@Nullable final Duration retryDuration) {
super(null, null, true, false);
this.retryDuration = retryDuration;
this.legacy = legacy;
}
public Optional<Duration> getRetryDuration() {
return Optional.ofNullable(retryDuration);
}
public boolean isLegacy() {
return legacy;
}
@Override
public Status grpcStatus() {
return Status.RESOURCE_EXHAUSTED;

View File

@@ -111,7 +111,7 @@ public class RegistrationController {
throw new WebApplicationException("Invalid signature", 422);
}
RateLimiter.adaptLegacyException(() -> rateLimiters.getRegistrationLimiter().validate(number));
rateLimiters.getRegistrationLimiter().validate(number);
final PhoneVerificationRequest.VerificationType verificationType = phoneVerificationTokenManager.verify(number,
registrationRequest);

View File

@@ -173,9 +173,7 @@ public class VerificationController {
} catch (final CompletionException e) {
if (ExceptionUtils.unwrap(e) instanceof RateLimitExceededException re) {
RateLimiter.adaptLegacyException(() -> {
throw re;
});
throw re;
}
throw new ServerErrorException(Response.Status.INTERNAL_SERVER_ERROR, e);
@@ -318,9 +316,8 @@ public class VerificationController {
final boolean pushChallengePresent = updateVerificationSessionRequest.pushChallenge() != null;
if (pushChallengePresent) {
RateLimiter.adaptLegacyException(
() -> rateLimiters.getVerificationPushChallengeLimiter()
.validate(registrationServiceSession.encodedSessionId()));
rateLimiters.getVerificationPushChallengeLimiter()
.validate(registrationServiceSession.encodedSessionId());
}
final boolean pushChallengeMatches;
@@ -383,8 +380,7 @@ public class VerificationController {
return verificationSession;
}
RateLimiter.adaptLegacyException(
() -> rateLimiters.getVerificationCaptchaLimiter().validate(registrationServiceSession.encodedSessionId()));
rateLimiters.getVerificationCaptchaLimiter().validate(registrationServiceSession.encodedSessionId());
final AssessmentResult assessmentResult;
try {
@@ -507,7 +503,7 @@ public class VerificationController {
throw new ClientErrorException(response);
}
throw new RateLimitExceededException(rateLimitExceededException.getRetryDuration().orElse(null), false);
throw new RateLimitExceededException(rateLimitExceededException.getRetryDuration().orElse(null));
} else if (unwrappedException instanceof RegistrationServiceException registrationServiceException) {
throw registrationServiceException.getRegistrationSession()
@@ -584,7 +580,7 @@ public class VerificationController {
throw new ClientErrorException(response);
}
throw new RateLimitExceededException(rateLimitExceededException.getRetryDuration().orElse(null), false);
throw new RateLimitExceededException(rateLimitExceededException.getRetryDuration().orElse(null));
} else if (unwrappedException instanceof RegistrationServiceException registrationServiceException) {

View File

@@ -24,7 +24,7 @@ public class VerificationSessionRateLimitExceededException extends RateLimitExce
public VerificationSessionRateLimitExceededException(
final RegistrationServiceSession registrationServiceSession, @Nullable final Duration retryDuration,
final boolean legacy) {
super(retryDuration, legacy);
super(retryDuration);
this.registrationServiceSession = registrationServiceSession;
}

View File

@@ -28,8 +28,8 @@ public class RateLimitByIpFilter implements ContainerRequestFilter {
private static final Logger logger = LoggerFactory.getLogger(RateLimitByIpFilter.class);
@VisibleForTesting
static final RateLimitExceededException INVALID_HEADER_EXCEPTION = new RateLimitExceededException(Duration.ofHours(1),
true);
static final RateLimitExceededException INVALID_HEADER_EXCEPTION = new RateLimitExceededException(Duration.ofHours(1)
);
private static final ExceptionMapper<RateLimitExceededException> EXCEPTION_MAPPER = new RateLimitExceededExceptionMapper();

View File

@@ -78,32 +78,4 @@ public interface RateLimiter {
default CompletionStage<Void> clearAsync(final UUID accountUuid) {
return clearAsync(accountUuid.toString());
}
/**
* If the future throws a {@link RateLimitExceededException}, it will adapt it to ensure that
* {@link RateLimitExceededException#isLegacy()} returns {@code false}
*/
static CompletionStage<Void> adaptLegacyException(final CompletionStage<Void> rateLimitFuture) {
return rateLimitFuture.exceptionally(ExceptionUtils.exceptionallyHandler(RateLimitExceededException.class, e -> {
throw ExceptionUtils.wrap(new RateLimitExceededException(e.getRetryDuration().orElse(null), false));
}));
}
/**
* If the wrapped {@code validate()} call throws a {@link RateLimitExceededException}, it will adapt it to ensure that
* {@link RateLimitExceededException#isLegacy()} returns {@code false}
*/
static void adaptLegacyException(final RateLimitValidator validator) throws RateLimitExceededException {
try {
validator.validate();
} catch (final RateLimitExceededException e) {
throw new RateLimitExceededException(e.getRetryDuration().orElse(null), false);
}
}
@FunctionalInterface
interface RateLimitValidator {
void validate() throws RateLimitExceededException;
}
}

View File

@@ -65,7 +65,7 @@ public class StaticRateLimiter implements RateLimiter {
counter.increment();
final Duration retryAfter = Duration.ofMillis(
(long) Math.ceil((double) deficitPermitsAmount / config.leakRatePerMillis()));
throw new RateLimitExceededException(retryAfter, true);
throw new RateLimitExceededException(retryAfter);
}
} catch (RedisException e) {
if (!failOpen()) {
@@ -84,7 +84,7 @@ public class StaticRateLimiter implements RateLimiter {
counter.increment();
final Duration retryAfter = Duration.ofMillis(
(long) Math.ceil((double) deficitPermitsAmount / config.leakRatePerMillis()));
return failedFuture(new RateLimitExceededException(retryAfter, true));
return failedFuture(new RateLimitExceededException(retryAfter));
})
.exceptionally(throwable -> {
if (ExceptionUtils.unwrap(throwable) instanceof RedisException && failOpen()) {

View File

@@ -16,11 +16,8 @@ public class RateLimitExceededExceptionMapper implements ExceptionMapper<RateLim
private static final Logger logger = LoggerFactory.getLogger(RateLimitExceededExceptionMapper.class);
private static final int LEGACY_STATUS_CODE = 413;
private static final int STATUS_CODE = 429;
/**
* Convert a RateLimitExceededException to a {@value STATUS_CODE} (or legacy {@value LEGACY_STATUS_CODE}) response
* Convert a RateLimitExceededException to a 429 response
* with a Retry-After header.
*
* @param e A RateLimitExceededException potentially containing a recommended retry duration
@@ -28,7 +25,6 @@ public class RateLimitExceededExceptionMapper implements ExceptionMapper<RateLim
*/
@Override
public Response toResponse(RateLimitExceededException e) {
final int statusCode = e.isLegacy() ? LEGACY_STATUS_CODE : STATUS_CODE;
return e.getRetryDuration()
.filter(d -> {
if (d.isNegative()) {
@@ -38,7 +34,7 @@ public class RateLimitExceededExceptionMapper implements ExceptionMapper<RateLim
// only include non-negative durations in retry headers
return !d.isNegative();
})
.map(d -> Response.status(statusCode).header("Retry-After", d.toSeconds()))
.orElseGet(() -> Response.status(statusCode)).build();
.map(d -> Response.status(Response.Status.TOO_MANY_REQUESTS).header("Retry-After", d.toSeconds()))
.orElseGet(() -> Response.status(Response.Status.TOO_MANY_REQUESTS)).build();
}
}

View File

@@ -97,8 +97,8 @@ public class RegistrationServiceClient implements Managed {
case CREATE_REGISTRATION_SESSION_ERROR_TYPE_RATE_LIMITED -> throw new CompletionException(
new RateLimitExceededException(response.getError().getMayRetry()
? Duration.ofSeconds(response.getError().getRetryAfterSeconds())
: null,
true));
: null
));
case CREATE_REGISTRATION_SESSION_ERROR_TYPE_ILLEGAL_PHONE_NUMBER -> throw new IllegalArgumentException();
default -> throw new RuntimeException(
"Unrecognized error type from registration service: " + response.getError().getErrorType());