Send 508 status code for legacy clients that produce rate limit challenges

This commit is contained in:
Chris Eager
2021-08-10 10:10:59 -05:00
committed by Chris Eager
parent d29764d11f
commit b3e6a50dee
11 changed files with 126 additions and 66 deletions

View File

@@ -112,25 +112,25 @@ class RateLimitChallengeManagerTest {
@ParameterizedTest
@MethodSource
void shouldIssueRateLimitChallenge(final String userAgent, final boolean expectIssueChallenge) {
void isClientBelowMinimumVersion(final String userAgent, final boolean expectBelowMinimumVersion) {
when(rateLimitChallengeConfiguration.getMinimumSupportedVersion(any())).thenReturn(Optional.empty());
when(rateLimitChallengeConfiguration.getMinimumSupportedVersion(ClientPlatform.ANDROID))
.thenReturn(Optional.of(new Semver("5.6.0")));
when(rateLimitChallengeConfiguration.getMinimumSupportedVersion(ClientPlatform.DESKTOP))
.thenReturn(Optional.of(new Semver("5.0.0-beta.2")));
assertEquals(expectIssueChallenge, rateLimitChallengeManager.shouldIssueRateLimitChallenge(userAgent));
assertEquals(expectBelowMinimumVersion, rateLimitChallengeManager.isClientBelowMinimumVersion(userAgent));
}
private static Stream<Arguments> shouldIssueRateLimitChallenge() {
private static Stream<Arguments> isClientBelowMinimumVersion() {
return Stream.of(
Arguments.of("Signal-Android/5.1.2 Android/30", false),
Arguments.of("Signal-Android/5.6.0 Android/30", true),
Arguments.of("Signal-Android/5.11.1 Android/30", true),
Arguments.of("Signal-Desktop/5.0.0-beta.3 macOS/11", true),
Arguments.of("Signal-Desktop/5.0.0-beta.1 Windows/3.1", false),
Arguments.of("Signal-Desktop/5.2.0 Debian/11", true),
Arguments.of("Signal-iOS/5.1.2 iOS/12.2", false),
Arguments.of("Signal-Android/5.1.2 Android/30", true),
Arguments.of("Signal-Android/5.6.0 Android/30", false),
Arguments.of("Signal-Android/5.11.1 Android/30", false),
Arguments.of("Signal-Desktop/5.0.0-beta.3 macOS/11", false),
Arguments.of("Signal-Desktop/5.0.0-beta.1 Windows/3.1", true),
Arguments.of("Signal-Desktop/5.2.0 Debian/11", false),
Arguments.of("Signal-iOS/5.1.2 iOS/12.2", true),
Arguments.of("anything-else", false)
);
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013-2020 Signal Messenger, LLC
* Copyright 2013-2021 Signal Messenger, LLC
* SPDX-License-Identifier: AGPL-3.0-only
*/
@@ -39,6 +39,8 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatcher;
import org.whispersystems.textsecuregcm.auth.AmbiguousIdentifier;
@@ -57,6 +59,7 @@ import org.whispersystems.textsecuregcm.limits.RateLimitChallengeManager;
import org.whispersystems.textsecuregcm.limits.RateLimiter;
import org.whispersystems.textsecuregcm.limits.RateLimiters;
import org.whispersystems.textsecuregcm.mappers.RateLimitChallengeExceptionMapper;
import org.whispersystems.textsecuregcm.mappers.ServerRejectedExceptionMapper;
import org.whispersystems.textsecuregcm.storage.Account;
import org.whispersystems.textsecuregcm.storage.AccountsManager;
import org.whispersystems.textsecuregcm.storage.Device;
@@ -98,12 +101,15 @@ class KeysControllerTest {
private static final RateLimiter rateLimiter = mock(RateLimiter.class );
private static final ResourceExtension resources = ResourceExtension.builder()
.addProvider(AuthHelper.getAuthFilter())
.addProvider(new PolymorphicAuthValueFactoryProvider.Binder<>(ImmutableSet.of(Account.class, DisabledPermittedAccount.class)))
.setTestContainerFactory(new GrizzlyWebTestContainerFactory())
.addResource(new RateLimitChallengeExceptionMapper(rateLimitChallengeManager))
.addResource(new KeysController(rateLimiters, keysDynamoDb, accounts, preKeyRateLimiter, rateLimitChallengeManager))
.build();
.addProvider(AuthHelper.getAuthFilter())
.addProvider(new PolymorphicAuthValueFactoryProvider.Binder<>(
ImmutableSet.of(Account.class, DisabledPermittedAccount.class)))
.setTestContainerFactory(new GrizzlyWebTestContainerFactory())
.addResource(new RateLimitChallengeExceptionMapper(rateLimitChallengeManager))
.addResource(new ServerRejectedExceptionMapper())
.addResource(
new KeysController(rateLimiters, keysDynamoDb, accounts, preKeyRateLimiter, rateLimitChallengeManager))
.build();
@BeforeEach
void setup() {
@@ -622,16 +628,20 @@ class KeysControllerTest {
verify(accounts).update(eq(AuthHelper.DISABLED_ACCOUNT), any());
}
@Test
void testRateLimitChallenge() throws RateLimitExceededException {
@ParameterizedTest
@ValueSource(booleans = {true, false})
void testRateLimitChallenge(boolean clientBelowMinimumVersion) throws RateLimitExceededException {
Duration retryAfter = Duration.ofMinutes(1);
doThrow(new RateLimitExceededException(retryAfter))
.when(preKeyRateLimiter).validate(any());
when(rateLimitChallengeManager.shouldIssueRateLimitChallenge("Signal-Android/5.1.2 Android/30")).thenReturn(true);
when(
rateLimitChallengeManager.isClientBelowMinimumVersion("Signal-Android/5.1.2 Android/30")).thenReturn(
clientBelowMinimumVersion);
when(rateLimitChallengeManager.getChallengeOptions(AuthHelper.VALID_ACCOUNT))
.thenReturn(List.of(RateLimitChallengeManager.OPTION_PUSH_CHALLENGE, RateLimitChallengeManager.OPTION_RECAPTCHA));
.thenReturn(
List.of(RateLimitChallengeManager.OPTION_PUSH_CHALLENGE, RateLimitChallengeManager.OPTION_RECAPTCHA));
Response result = resources.getJerseyTest()
.target(String.format("/v2/keys/%s/*", EXISTS_UUID.toString()))
@@ -650,14 +660,18 @@ class KeysControllerTest {
.header("User-Agent", "Signal-Android/5.1.2 Android/30")
.get();
assertThat(result.getStatus()).isEqualTo(428);
if (clientBelowMinimumVersion) {
assertThat(result.getStatus()).isEqualTo(508);
} else {
assertThat(result.getStatus()).isEqualTo(428);
RateLimitChallenge rateLimitChallenge = result.readEntity(RateLimitChallenge.class);
RateLimitChallenge rateLimitChallenge = result.readEntity(RateLimitChallenge.class);
assertThat(rateLimitChallenge.getToken()).isNotBlank();
assertThat(rateLimitChallenge.getOptions()).isNotEmpty();
assertThat(rateLimitChallenge.getOptions()).contains("recaptcha");
assertThat(rateLimitChallenge.getOptions()).contains("pushChallenge");
assertThat(Long.parseLong(result.getHeaderString("Retry-After"))).isEqualTo(retryAfter.toSeconds());
assertThat(rateLimitChallenge.getToken()).isNotBlank();
assertThat(rateLimitChallenge.getOptions()).isNotEmpty();
assertThat(rateLimitChallenge.getOptions()).contains("recaptcha");
assertThat(rateLimitChallenge.getOptions()).contains("pushChallenge");
assertThat(Long.parseLong(result.getHeaderString("Retry-After"))).isEqualTo(retryAfter.toSeconds());
}
}
}

View File

@@ -294,8 +294,9 @@ class MessageControllerTest {
}
@ParameterizedTest
@CsvSource({"true, 5.1.0, 413", "true, 5.6.4, 428", "false, 5.6.4, 200"})
void testUnsealedSenderCardinalityRateLimited(final boolean rateLimited, final String clientVersion, final int expectedStatusCode) throws Exception {
@CsvSource({"true, true, 413", "true, false, 428", "false, false, 200"})
void testUnsealedSenderCardinalityRateLimited(final boolean rateLimited, final boolean legacyClient,
final int expectedStatusCode) throws Exception {
final DynamicConfiguration dynamicConfiguration = mock(DynamicConfiguration.class);
final DynamicMessageRateConfiguration messageRateConfiguration = mock(DynamicMessageRateConfiguration.class);
@@ -323,8 +324,8 @@ class MessageControllerTest {
doThrow(new RateLimitExceededException(Duration.ofHours(1)))
.when(unsealedSenderRateLimiter).validate(eq(AuthHelper.VALID_ACCOUNT), eq(internationalAccount));
when(rateLimitChallengeManager.shouldIssueRateLimitChallenge(String.format("Signal-Android/%s Android/30", clientVersion)))
.thenReturn(true);
when(rateLimitChallengeManager.isClientBelowMinimumVersion(anyString()))
.thenReturn(legacyClient);
}
Response response =
@@ -352,9 +353,10 @@ class MessageControllerTest {
doThrow(new RateLimitExceededException(retryAfter))
.when(unsealedSenderRateLimiter).validate(any(), any());
when(rateLimitChallengeManager.shouldIssueRateLimitChallenge("Signal-Android/5.1.2 Android/30")).thenReturn(true);
when(rateLimitChallengeManager.isClientBelowMinimumVersion("Signal-Android/5.1.2 Android/30")).thenReturn(false);
when(rateLimitChallengeManager.getChallengeOptions(AuthHelper.VALID_ACCOUNT))
.thenReturn(List.of(RateLimitChallengeManager.OPTION_PUSH_CHALLENGE, RateLimitChallengeManager.OPTION_RECAPTCHA));
.thenReturn(
List.of(RateLimitChallengeManager.OPTION_PUSH_CHALLENGE, RateLimitChallengeManager.OPTION_RECAPTCHA));
Response response =
resources.getJerseyTest()