diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsAnonymousGrpcService.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsAnonymousGrpcService.java index 8536b7de8..76f65fcbc 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsAnonymousGrpcService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsAnonymousGrpcService.java @@ -15,19 +15,16 @@ import java.util.concurrent.TimeUnit; import org.signal.chat.credentials.AuthCheckResult; import org.signal.chat.credentials.CheckSvrCredentialsRequest; import org.signal.chat.credentials.CheckSvrCredentialsResponse; -import org.signal.chat.credentials.ReactorExternalServiceCredentialsAnonymousGrpc; +import org.signal.chat.credentials.SimpleExternalServiceCredentialsAnonymousGrpc; import org.whispersystems.textsecuregcm.WhisperServerConfiguration; import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentials; import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentialsGenerator; import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentialsSelector; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; -import reactor.core.publisher.Flux; -import reactor.core.publisher.Mono; -import reactor.util.function.Tuples; public class ExternalServiceCredentialsAnonymousGrpcService extends - ReactorExternalServiceCredentialsAnonymousGrpc.ExternalServiceCredentialsAnonymousImplBase { + SimpleExternalServiceCredentialsAnonymousGrpc.ExternalServiceCredentialsAnonymousImplBase { private static final long MAX_SVR_PASSWORD_AGE_SECONDS = TimeUnit.DAYS.toSeconds(30); @@ -54,36 +51,34 @@ public class ExternalServiceCredentialsAnonymousGrpcService extends } @Override - public Mono checkSvrCredentials(final CheckSvrCredentialsRequest request) { + public CheckSvrCredentialsResponse checkSvrCredentials(final CheckSvrCredentialsRequest request) { final List tokens = request.getPasswordsList(); + if (tokens.size() > 10) { + throw GrpcExceptions.fieldViolation("passwordsList", "At most 10 passwords may be provided"); + } final List credentials = ExternalServiceCredentialsSelector.check( tokens, svrCredentialsGenerator, MAX_SVR_PASSWORD_AGE_SECONDS); - - return Mono.fromFuture(() -> accountsManager.getByE164Async(request.getNumber())) - // the username associated with the provided number - .map(maybeAccount -> maybeAccount.map(Account::getUuid) - .map(svrCredentialsGenerator::generateForUuid) - .map(ExternalServiceCredentials::username)) - .flatMapMany(maybeMatchingUsername -> Flux.fromIterable(credentials) - .map(credential -> Tuples.of(maybeMatchingUsername, credential))) - .reduce(CheckSvrCredentialsResponse.newBuilder(), ((builder, usernameAndCredentialInfo) -> { - final Optional matchingUsername = usernameAndCredentialInfo.getT1(); - final ExternalServiceCredentialsSelector.CredentialInfo credentialInfo = usernameAndCredentialInfo.getT2(); - - final AuthCheckResult authCheckResult; - if (!credentialInfo.valid()) { - authCheckResult = AuthCheckResult.AUTH_CHECK_RESULT_INVALID; - } else { - final String username = credentialInfo.credentials().username(); - // does this credential match the account id for the e164 provided in the request? - authCheckResult = matchingUsername.map(username::equals).orElse(false) - ? AuthCheckResult.AUTH_CHECK_RESULT_MATCH - : AuthCheckResult.AUTH_CHECK_RESULT_NO_MATCH; - } - return builder.putMatches(credentialInfo.token(), authCheckResult); - })) - .map(CheckSvrCredentialsResponse.Builder::build); + // the username associated with the provided number + final Optional maybeUsername = accountsManager.getByE164(request.getNumber()) + .map(Account::getUuid) + .map(svrCredentialsGenerator::generateForUuid) + .map(ExternalServiceCredentials::username); + final CheckSvrCredentialsResponse.Builder builder = CheckSvrCredentialsResponse.newBuilder(); + for (ExternalServiceCredentialsSelector.CredentialInfo credentialInfo : credentials) { + final AuthCheckResult authCheckResult; + if (!credentialInfo.valid()) { + authCheckResult = AuthCheckResult.AUTH_CHECK_RESULT_INVALID; + } else { + final String username = credentialInfo.credentials().username(); + // does this credential match the account id for the e164 provided in the request? + authCheckResult = maybeUsername.map(username::equals).orElse(false) + ? AuthCheckResult.AUTH_CHECK_RESULT_MATCH + : AuthCheckResult.AUTH_CHECK_RESULT_NO_MATCH; + } + builder.putMatches(credentialInfo.token(), authCheckResult); + } + return builder.build(); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsGrpcService.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsGrpcService.java index df54391e9..bf9da3d4c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsGrpcService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsGrpcService.java @@ -8,22 +8,21 @@ package org.whispersystems.textsecuregcm.grpc; import static java.util.Objects.requireNonNull; import com.google.common.annotations.VisibleForTesting; -import io.grpc.Status; import java.time.Clock; import java.util.Map; import org.signal.chat.credentials.ExternalServiceType; import org.signal.chat.credentials.GetExternalServiceCredentialsRequest; import org.signal.chat.credentials.GetExternalServiceCredentialsResponse; -import org.signal.chat.credentials.ReactorExternalServiceCredentialsGrpc; +import org.signal.chat.credentials.SimpleExternalServiceCredentialsGrpc; import org.whispersystems.textsecuregcm.WhisperServerConfiguration; import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentials; import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentialsGenerator; import org.whispersystems.textsecuregcm.auth.grpc.AuthenticatedDevice; import org.whispersystems.textsecuregcm.auth.grpc.AuthenticationUtil; +import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException; import org.whispersystems.textsecuregcm.limits.RateLimiters; -import reactor.core.publisher.Mono; -public class ExternalServiceCredentialsGrpcService extends ReactorExternalServiceCredentialsGrpc.ExternalServiceCredentialsImplBase { +public class ExternalServiceCredentialsGrpcService extends SimpleExternalServiceCredentialsGrpc.ExternalServiceCredentialsImplBase { private final Map credentialsGeneratorByType; @@ -48,21 +47,20 @@ public class ExternalServiceCredentialsGrpcService extends ReactorExternalServic } @Override - public Mono getExternalServiceCredentials(final GetExternalServiceCredentialsRequest request) { + public GetExternalServiceCredentialsResponse getExternalServiceCredentials(final GetExternalServiceCredentialsRequest request) + throws RateLimitExceededException { final ExternalServiceCredentialsGenerator credentialsGenerator = this.credentialsGeneratorByType .get(request.getExternalService()); if (credentialsGenerator == null) { - return Mono.error(Status.INVALID_ARGUMENT.asException()); + throw GrpcExceptions.fieldViolation("externalService", "Invalid external service type"); } final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice(); - return rateLimiters.forDescriptor(RateLimiters.For.EXTERNAL_SERVICE_CREDENTIALS).validateReactive(authenticatedDevice.accountIdentifier()) - .then(Mono.fromSupplier(() -> { - final ExternalServiceCredentials externalServiceCredentials = credentialsGenerator - .generateForUuid(authenticatedDevice.accountIdentifier()); - return GetExternalServiceCredentialsResponse.newBuilder() - .setUsername(externalServiceCredentials.username()) - .setPassword(externalServiceCredentials.password()) - .build(); - })); + rateLimiters.forDescriptor(RateLimiters.For.EXTERNAL_SERVICE_CREDENTIALS).validate(authenticatedDevice.accountIdentifier()); + final ExternalServiceCredentials externalServiceCredentials = credentialsGenerator + .generateForUuid(authenticatedDevice.accountIdentifier()); + return GetExternalServiceCredentialsResponse.newBuilder() + .setUsername(externalServiceCredentials.username()) + .setPassword(externalServiceCredentials.password()) + .build(); } } diff --git a/service/src/main/proto/org/signal/chat/calling.proto b/service/src/main/proto/org/signal/chat/calling.proto index f4ee93048..7b566a4f1 100644 --- a/service/src/main/proto/org/signal/chat/calling.proto +++ b/service/src/main/proto/org/signal/chat/calling.proto @@ -13,11 +13,6 @@ package org.signal.chat.calling; service Calling { // Generates and returns TURN credentials for the caller. - // - // This RPC may fail with a `RESOURCE_EXHAUSTED` status if a rate limit for - // generating TURN credentials has been exceeded, in which case a - // `retry-after` header containing an ISO 8601 duration string will be present - // in the response trailers. rpc GetTurnCredentials(GetTurnCredentialsRequest) returns (GetTurnCredentialsResponse) {} } diff --git a/service/src/main/proto/org/signal/chat/credentials.proto b/service/src/main/proto/org/signal/chat/credentials.proto index 16fa0e4e0..596e2a48e 100644 --- a/service/src/main/proto/org/signal/chat/credentials.proto +++ b/service/src/main/proto/org/signal/chat/credentials.proto @@ -15,16 +15,6 @@ package org.signal.chat.credentials; service ExternalServiceCredentials { // Generates and returns an external service credentials for the caller. - // - // `UNAUTHENTICATED` status is returned if the call is made on unauthenticated channel. - // - // `INVALID_ARGUMENT` status is returned if service is not configured for the service type - // found in the request OR if `externalService` is not specified in the request. - // - // `RESOURCE_EXHAUSTED` status is returned if a rate limit for - // generating credentials has been exceeded, in which case a - // `retry-after` header containing an ISO 8601 duration string will be present - // in the response trailers. rpc GetExternalServiceCredentials(GetExternalServiceCredentialsRequest) returns (GetExternalServiceCredentialsResponse) {} } @@ -33,10 +23,6 @@ service ExternalServiceCredentialsAnonymous { // Given a list of secure value recovery (SVR) service credentials and a phone number, // checks, which of the provided credentials were generated by the user with the given phone number // and have not yet expired. - // - // `UNAUTHENTICATED` status is returned if the call is made on unauthenticated channel. - // - // `INVALID_ARGUMENT` status is returned if request contains more than 10 passwords to be checked. rpc CheckSvrCredentials(CheckSvrCredentialsRequest) returns (CheckSvrCredentialsResponse) {} } @@ -81,7 +67,8 @@ message CheckSvrCredentialsRequest { string number = 1; // A list of credentials from previously made calls to `ExternalServiceCredentials.GetExternalServiceCredentials()` - // for `EXTERNAL_SERVICE_TYPE_SVR`. This list may contain credentials generated by different users. + // for `EXTERNAL_SERVICE_TYPE_SVR`. This list may contain credentials generated by different users. Up to 10 credentials + // can be checked. repeated string passwords = 2; } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsAnonymousGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsAnonymousGrpcServiceTest.java index f1994c8fe..50779182f 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsAnonymousGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsAnonymousGrpcServiceTest.java @@ -6,13 +6,16 @@ package org.whispersystems.textsecuregcm.grpc; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.i18n.phonenumbers.PhoneNumberUtil; +import io.grpc.Status; +import io.grpc.StatusRuntimeException; import java.time.Duration; import java.util.Map; import java.util.Optional; import java.util.UUID; -import java.util.concurrent.CompletableFuture; +import java.util.stream.IntStream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; @@ -59,8 +62,7 @@ class ExternalServiceCredentialsAnonymousGrpcServiceTest extends @BeforeEach public void setup() { - Mockito.when(accountsManager.getByE164Async(USER_E164)) - .thenReturn(CompletableFuture.completedFuture(Optional.of(account(USER_UUID)))); + Mockito.when(accountsManager.getByE164(USER_E164)).thenReturn(Optional.of(account(USER_UUID))); } @Test @@ -125,6 +127,17 @@ class ExternalServiceCredentialsAnonymousGrpcServiceTest extends ), day(25)); } + @Test + public void testTooManyPasswords() { + final CheckSvrCredentialsRequest request = CheckSvrCredentialsRequest.newBuilder() + .setNumber(USER_E164) + .addAllPasswords(IntStream.range(0, 12).mapToObj(i -> token(UUID.randomUUID(), day(10))).toList()) + .build(); + final StatusRuntimeException status = assertThrows(StatusRuntimeException.class, + () -> unauthenticatedServiceStub().checkSvrCredentials(request)); + assertEquals(Status.INVALID_ARGUMENT.getCode(), status.getStatus().getCode()); + } + private void assertExpectedCredentialCheckResponse( final Map expected, final long nowMillis) throws Exception {