From ad9c03186ab488f99b151c275b55d8fdac560adc Mon Sep 17 00:00:00 2001 From: Jon Chambers <63609320+jon-signal@users.noreply.github.com> Date: Thu, 26 Feb 2026 17:08:54 -0500 Subject: [PATCH] Use `simple-grpc` in `AccountsGrpcService`/`AccountsAnonymousGrpcService` --- .../grpc/AccountsAnonymousGrpcService.java | 61 ++--- .../grpc/AccountsGrpcService.java | 244 ++++++++++-------- .../textsecuregcm/grpc/RateLimitUtil.java | 6 +- .../AccountsAnonymousGrpcServiceTest.java | 28 +- .../grpc/AccountsGrpcServiceTest.java | 104 ++++---- 5 files changed, 234 insertions(+), 209 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsAnonymousGrpcService.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsAnonymousGrpcService.java index c4f88c6b2..2bf9f8fa3 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsAnonymousGrpcService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsAnonymousGrpcService.java @@ -6,26 +6,25 @@ package org.whispersystems.textsecuregcm.grpc; import com.google.protobuf.ByteString; +import java.util.UUID; import org.signal.chat.account.CheckAccountExistenceRequest; import org.signal.chat.account.CheckAccountExistenceResponse; import org.signal.chat.account.LookupUsernameHashRequest; import org.signal.chat.account.LookupUsernameHashResponse; import org.signal.chat.account.LookupUsernameLinkRequest; import org.signal.chat.account.LookupUsernameLinkResponse; -import org.signal.chat.account.ReactorAccountsAnonymousGrpc; +import org.signal.chat.account.SimpleAccountsAnonymousGrpc; import org.signal.chat.errors.NotFound; import org.whispersystems.textsecuregcm.controllers.AccountController; +import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException; import org.whispersystems.textsecuregcm.identity.AciServiceIdentifier; import org.whispersystems.textsecuregcm.identity.ServiceIdentifier; import org.whispersystems.textsecuregcm.limits.RateLimiters; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.util.UUIDUtil; -import reactor.core.publisher.Mono; -import java.util.Optional; -import java.util.UUID; -public class AccountsAnonymousGrpcService extends ReactorAccountsAnonymousGrpc.AccountsAnonymousImplBase { +public class AccountsAnonymousGrpcService extends SimpleAccountsAnonymousGrpc.AccountsAnonymousImplBase { private final AccountsManager accountsManager; private final RateLimiters rateLimiters; @@ -36,37 +35,41 @@ public class AccountsAnonymousGrpcService extends ReactorAccountsAnonymousGrpc.A } @Override - public Mono checkAccountExistence(final CheckAccountExistenceRequest request) { + public CheckAccountExistenceResponse checkAccountExistence(final CheckAccountExistenceRequest request) + throws RateLimitExceededException { + final ServiceIdentifier serviceIdentifier = ServiceIdentifierUtil.fromGrpcServiceIdentifier(request.getServiceIdentifier()); - return RateLimitUtil.rateLimitByRemoteAddress(rateLimiters.getCheckAccountExistenceLimiter()) - .then(Mono.fromFuture(() -> accountsManager.getByServiceIdentifierAsync(serviceIdentifier))) - .map(Optional::isPresent) - .map(accountExists -> CheckAccountExistenceResponse.newBuilder() - .setAccountExists(accountExists) - .build()); + RateLimitUtil.rateLimitByRemoteAddress(rateLimiters.getCheckAccountExistenceLimiter()); + + return CheckAccountExistenceResponse.newBuilder() + .setAccountExists(accountsManager.getByServiceIdentifier(serviceIdentifier).isPresent()) + .build(); } @Override - public Mono lookupUsernameHash(final LookupUsernameHashRequest request) { + public LookupUsernameHashResponse lookupUsernameHash(final LookupUsernameHashRequest request) + throws RateLimitExceededException { + if (request.getUsernameHash().size() != AccountController.USERNAME_HASH_LENGTH) { throw GrpcExceptions.fieldViolation("username_hash", String.format("Illegal username hash length; expected %d bytes, but got %d bytes", AccountController.USERNAME_HASH_LENGTH, request.getUsernameHash().size())); } - return RateLimitUtil.rateLimitByRemoteAddress(rateLimiters.getUsernameLookupLimiter()) - .then(Mono.fromFuture(() -> accountsManager.getByUsernameHash(request.getUsernameHash().toByteArray()))) - .map(maybeAccount -> maybeAccount - .map(account -> LookupUsernameHashResponse.newBuilder() - .setServiceIdentifier(ServiceIdentifierUtil.toGrpcServiceIdentifier(new AciServiceIdentifier(account.getUuid()))) - .build()) - .orElseGet(() -> LookupUsernameHashResponse.newBuilder().setNotFound(NotFound.getDefaultInstance()).build())); + RateLimitUtil.rateLimitByRemoteAddress(rateLimiters.getUsernameLookupLimiter()); + + return accountsManager.getByUsernameHash(request.getUsernameHash().toByteArray()).join() + .map(account -> LookupUsernameHashResponse.newBuilder() + .setServiceIdentifier(ServiceIdentifierUtil.toGrpcServiceIdentifier(new AciServiceIdentifier(account.getUuid()))) + .build()) + .orElseGet(() -> LookupUsernameHashResponse.newBuilder().setNotFound(NotFound.getDefaultInstance()).build()); } @Override - public Mono lookupUsernameLink(final LookupUsernameLinkRequest request) { + public LookupUsernameLinkResponse lookupUsernameLink(final LookupUsernameLinkRequest request) + throws RateLimitExceededException { final UUID linkHandle; try { @@ -75,13 +78,13 @@ public class AccountsAnonymousGrpcService extends ReactorAccountsAnonymousGrpc.A throw GrpcExceptions.fieldViolation("username_link_handle", "Could not interpret link handle as UUID"); } - return RateLimitUtil.rateLimitByRemoteAddress(rateLimiters.getUsernameLinkLookupLimiter()) - .then(Mono.fromFuture(() -> accountsManager.getByUsernameLinkHandle(linkHandle))) - .map(maybeAccount -> maybeAccount - .flatMap(Account::getEncryptedUsername) - .map(usernameCiphertext -> LookupUsernameLinkResponse.newBuilder() - .setUsernameCiphertext(ByteString.copyFrom(usernameCiphertext)) - .build()) - .orElseGet(() -> LookupUsernameLinkResponse.newBuilder().setNotFound(NotFound.getDefaultInstance()).build())); + RateLimitUtil.rateLimitByRemoteAddress(rateLimiters.getUsernameLinkLookupLimiter()); + + return accountsManager.getByUsernameLinkHandle(linkHandle).join() + .flatMap(Account::getEncryptedUsername) + .map(usernameCiphertext -> LookupUsernameLinkResponse.newBuilder() + .setUsernameCiphertext(ByteString.copyFrom(usernameCiphertext)) + .build()) + .orElseGet(() -> LookupUsernameLinkResponse.newBuilder().setNotFound(NotFound.getDefaultInstance()).build()); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java index ef36593a5..6119c1189 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java @@ -10,6 +10,7 @@ import java.util.ArrayList; import java.util.HexFormat; import java.util.List; import java.util.UUID; +import java.util.concurrent.CompletionException; import org.signal.chat.account.ClearRegistrationLockRequest; import org.signal.chat.account.ClearRegistrationLockResponse; import org.signal.chat.account.ConfigureUnidentifiedAccessRequest; @@ -24,7 +25,6 @@ import org.signal.chat.account.DeleteUsernameLinkRequest; import org.signal.chat.account.DeleteUsernameLinkResponse; import org.signal.chat.account.GetAccountIdentityRequest; import org.signal.chat.account.GetAccountIdentityResponse; -import org.signal.chat.account.ReactorAccountsGrpc; import org.signal.chat.account.ReserveUsernameHashRequest; import org.signal.chat.account.ReserveUsernameHashResponse; import org.signal.chat.account.SetDiscoverableByPhoneNumberRequest; @@ -35,6 +35,7 @@ import org.signal.chat.account.SetRegistrationRecoveryPasswordRequest; import org.signal.chat.account.SetRegistrationRecoveryPasswordResponse; import org.signal.chat.account.SetUsernameLinkRequest; import org.signal.chat.account.SetUsernameLinkResponse; +import org.signal.chat.account.SimpleAccountsGrpc; import org.signal.chat.account.UsernameNotAvailable; import org.signal.chat.common.AccountIdentifiers; import org.signal.chat.errors.FailedPrecondition; @@ -44,6 +45,7 @@ import org.whispersystems.textsecuregcm.auth.UnidentifiedAccessUtil; import org.whispersystems.textsecuregcm.auth.grpc.AuthenticatedDevice; import org.whispersystems.textsecuregcm.auth.grpc.AuthenticationUtil; import org.whispersystems.textsecuregcm.controllers.AccountController; +import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException; import org.whispersystems.textsecuregcm.entities.EncryptedUsername; import org.whispersystems.textsecuregcm.identity.AciServiceIdentifier; import org.whispersystems.textsecuregcm.identity.IdentityType; @@ -56,9 +58,8 @@ import org.whispersystems.textsecuregcm.storage.UsernameHashNotAvailableExceptio import org.whispersystems.textsecuregcm.storage.UsernameReservationNotFoundException; import org.whispersystems.textsecuregcm.util.UUIDUtil; import org.whispersystems.textsecuregcm.util.UsernameHashZkProofVerifier; -import reactor.core.publisher.Mono; -public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { +public class AccountsGrpcService extends SimpleAccountsGrpc.AccountsImplBase { private final AccountsManager accountsManager; private final RateLimiters rateLimiters; @@ -77,61 +78,59 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { } @Override - public Mono getAccountIdentity(final GetAccountIdentityRequest request) { - return getAccount() - .map(account -> { - final AccountIdentifiers.Builder accountIdentifiersBuilder = AccountIdentifiers.newBuilder() - .addServiceIdentifiers(ServiceIdentifierUtil.toGrpcServiceIdentifier(new AciServiceIdentifier(account.getUuid()))) - .addServiceIdentifiers(ServiceIdentifierUtil.toGrpcServiceIdentifier(new PniServiceIdentifier(account.getPhoneNumberIdentifier()))) - .setE164(account.getNumber()); + public GetAccountIdentityResponse getAccountIdentity(final GetAccountIdentityRequest request) { + final Account account = getAuthenticatedAccount(); - account.getUsernameHash().ifPresent(usernameHash -> - accountIdentifiersBuilder.setUsernameHash(ByteString.copyFrom(usernameHash))); + final AccountIdentifiers.Builder accountIdentifiersBuilder = AccountIdentifiers.newBuilder() + .addServiceIdentifiers(ServiceIdentifierUtil.toGrpcServiceIdentifier(new AciServiceIdentifier(account.getUuid()))) + .addServiceIdentifiers(ServiceIdentifierUtil.toGrpcServiceIdentifier(new PniServiceIdentifier(account.getPhoneNumberIdentifier()))) + .setE164(account.getNumber()); - return GetAccountIdentityResponse.newBuilder() - .setAccountIdentifiers(accountIdentifiersBuilder.build()) - .build(); - }); + account.getUsernameHash().ifPresent(usernameHash -> + accountIdentifiersBuilder.setUsernameHash(ByteString.copyFrom(usernameHash))); + + return GetAccountIdentityResponse.newBuilder() + .setAccountIdentifiers(accountIdentifiersBuilder) + .build(); } @Override - public Mono deleteAccount(final DeleteAccountRequest request) { - return getAccount(AuthenticationUtil.requireAuthenticatedPrimaryDevice()) - .flatMap(account -> Mono.fromFuture(() -> accountsManager.delete(account, AccountsManager.DeletionReason.USER_REQUEST))) - .thenReturn(DeleteAccountResponse.newBuilder().build()); + public DeleteAccountResponse deleteAccount(final DeleteAccountRequest request) { + accountsManager.delete(getAuthenticatedAccount(AuthenticationUtil.requireAuthenticatedPrimaryDevice()), + AccountsManager.DeletionReason.USER_REQUEST) + .join(); + + return DeleteAccountResponse.getDefaultInstance(); } @Override - public Mono setRegistrationLock(final SetRegistrationLockRequest request) { - final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedPrimaryDevice(); - + public SetRegistrationLockResponse setRegistrationLock(final SetRegistrationLockRequest request) { if (request.getRegistrationLock().isEmpty()) { throw GrpcExceptions.fieldViolation("registration_lock", "Registration lock secret must not be empty"); } - return getAccount(authenticatedDevice) - .flatMap(account -> { - // In the previous REST-based API, clients would send hex strings directly. For backward compatibility, we - // convert the registration lock secret to a lowercase hex string before turning it into a salted hash. - final SaltedTokenHash credentials = - SaltedTokenHash.generateFor(HexFormat.of().withLowerCase().formatHex(request.getRegistrationLock().toByteArray())); + // In the previous REST-based API, clients would send hex strings directly. For backward compatibility, we + // convert the registration lock secret to a lowercase hex string before turning it into a salted hash. + final SaltedTokenHash credentials = + SaltedTokenHash.generateFor(HexFormat.of().withLowerCase().formatHex(request.getRegistrationLock().toByteArray())); - return Mono.fromFuture(() -> accountsManager.updateAsync(account, - a -> a.setRegistrationLock(credentials.hash(), credentials.salt()))); - }) - .map(ignored -> SetRegistrationLockResponse.newBuilder().build()); + accountsManager.update(getAuthenticatedAccount(AuthenticationUtil.requireAuthenticatedPrimaryDevice()), + account -> account.setRegistrationLock(credentials.hash(), credentials.salt())); + + return SetRegistrationLockResponse.getDefaultInstance(); } @Override - public Mono clearRegistrationLock(final ClearRegistrationLockRequest request) { - return getAccount(AuthenticationUtil.requireAuthenticatedPrimaryDevice()) - .flatMap(account -> Mono.fromFuture(() -> accountsManager.updateAsync(account, - a -> a.setRegistrationLock(null, null)))) - .map(ignored -> ClearRegistrationLockResponse.newBuilder().build()); + public ClearRegistrationLockResponse clearRegistrationLock(final ClearRegistrationLockRequest request) { + accountsManager.update(getAuthenticatedAccount(AuthenticationUtil.requireAuthenticatedPrimaryDevice()), + account -> account.setRegistrationLock(null, null)); + + return ClearRegistrationLockResponse.getDefaultInstance(); } @Override - public Mono reserveUsernameHash(final ReserveUsernameHashRequest request) { + public ReserveUsernameHashResponse reserveUsernameHash(final ReserveUsernameHashRequest request) + throws RateLimitExceededException { final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice(); if (request.getUsernameHashesCount() == 0) { @@ -155,19 +154,31 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { usernameHashes.add(usernameHash.toByteArray()); } - return rateLimiters.getUsernameReserveLimiter().validateReactive(authenticatedDevice.accountIdentifier()) - .then(getAccount()) - .flatMap(account -> Mono.fromFuture(() -> accountsManager.reserveUsernameHash(account, usernameHashes))) - .map(reservation -> ReserveUsernameHashResponse.newBuilder() - .setUsernameHash(ByteString.copyFrom(reservation.reservedUsernameHash())) - .build()) - .onErrorReturn(UsernameHashNotAvailableException.class, ReserveUsernameHashResponse.newBuilder() + rateLimiters.getUsernameReserveLimiter().validate(authenticatedDevice.accountIdentifier()); + + final Account account = getAuthenticatedAccount(); + + try { + final AccountsManager.UsernameReservation usernameReservation = + accountsManager.reserveUsernameHash(account, usernameHashes).join(); + + return ReserveUsernameHashResponse.newBuilder() + .setUsernameHash(ByteString.copyFrom(usernameReservation.reservedUsernameHash())) + .build(); + } catch (final CompletionException e) { + if (e.getCause() instanceof UsernameHashNotAvailableException) { + return ReserveUsernameHashResponse.newBuilder() .setUsernameNotAvailable(UsernameNotAvailable.getDefaultInstance()) - .build()); + .build(); + } + + throw e; + } } @Override - public Mono confirmUsernameHash(final ConfirmUsernameHashRequest request) { + public ConfirmUsernameHashResponse confirmUsernameHash(final ConfirmUsernameHashRequest request) + throws RateLimitExceededException { final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice(); if (request.getUsernameHash().isEmpty()) { @@ -200,34 +211,46 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { throw GrpcExceptions.constraintViolation("Could not verify proof"); } - return rateLimiters.getUsernameSetLimiter().validateReactive(authenticatedDevice.accountIdentifier()) - .then(getAccount()) - .flatMap(account -> Mono.fromFuture(() -> accountsManager.confirmReservedUsernameHash(account, request.getUsernameHash().toByteArray(), request.getUsernameCiphertext().toByteArray()))) - .map(updatedAccount -> ConfirmUsernameHashResponse.newBuilder() - .setConfirmedUsernameHash(ConfirmUsernameHashResponse.ConfirmedUsernameHash.newBuilder() - .setUsernameHash(ByteString.copyFrom(updatedAccount.getUsernameHash().orElseThrow())) - .setUsernameLinkHandle(UUIDUtil.toByteString(updatedAccount.getUsernameLinkHandle())) - .build()) - .build()) - .onErrorResume(UsernameReservationNotFoundException.class, _ -> Mono.just(ConfirmUsernameHashResponse + rateLimiters.getUsernameSetLimiter().validate(authenticatedDevice.accountIdentifier()); + + try { + final Account updatedAccount = accountsManager.confirmReservedUsernameHash(getAuthenticatedAccount(), + request.getUsernameHash().toByteArray(), + request.getUsernameCiphertext().toByteArray()) + .join(); + + return ConfirmUsernameHashResponse.newBuilder() + .setConfirmedUsernameHash(ConfirmUsernameHashResponse.ConfirmedUsernameHash.newBuilder() + .setUsernameHash(ByteString.copyFrom(updatedAccount.getUsernameHash().orElseThrow())) + .setUsernameLinkHandle(UUIDUtil.toByteString(updatedAccount.getUsernameLinkHandle()))) + .build(); + } catch (final CompletionException e) { + if (e.getCause() instanceof UsernameReservationNotFoundException) { + return ConfirmUsernameHashResponse .newBuilder() .setReservationNotFound(FailedPrecondition.getDefaultInstance()) - .build())) - .onErrorResume(UsernameHashNotAvailableException.class, _ -> Mono.just(ConfirmUsernameHashResponse + .build(); + } else if (e.getCause() instanceof UsernameHashNotAvailableException) { + return ConfirmUsernameHashResponse .newBuilder() .setUsernameNotAvailable(UsernameNotAvailable.getDefaultInstance()) - .build())); + .build(); + } + + throw e; + } } @Override - public Mono deleteUsernameHash(final DeleteUsernameHashRequest request) { - return getAccount() - .flatMap(account -> Mono.fromFuture(() -> accountsManager.clearUsernameHash(account))) - .thenReturn(DeleteUsernameHashResponse.newBuilder().build()); + public DeleteUsernameHashResponse deleteUsernameHash(final DeleteUsernameHashRequest request) { + accountsManager.clearUsernameHash(getAuthenticatedAccount()).join(); + + return DeleteUsernameHashResponse.getDefaultInstance(); } @Override - public Mono setUsernameLink(final SetUsernameLinkRequest request) { + public SetUsernameLinkResponse setUsernameLink(final SetUsernameLinkRequest request) + throws RateLimitExceededException { final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice(); if (request.getUsernameCiphertext().isEmpty() || request.getUsernameCiphertext().size() > EncryptedUsername.MAX_SIZE) { @@ -235,77 +258,80 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { String.format("Username ciphertext must not be empty and must be shorter than %d bytes", EncryptedUsername.MAX_SIZE)); } - return rateLimiters.getUsernameLinkOperationLimiter().validateReactive(authenticatedDevice.accountIdentifier()) - .then(getAccount()) - .flatMap(account -> { - final SetUsernameLinkResponse.Builder responseBuilder = SetUsernameLinkResponse.newBuilder(); - if (account.getUsernameHash().isEmpty()) { - return Mono.just(responseBuilder.setNoUsernameSet(FailedPrecondition.getDefaultInstance()).build()); - } + rateLimiters.getUsernameLinkOperationLimiter().validate(authenticatedDevice.accountIdentifier()); - final UUID linkHandle; - if (request.getKeepLinkHandle() && account.getUsernameLinkHandle() != null) { - linkHandle = account.getUsernameLinkHandle(); - } else { - linkHandle = UUID.randomUUID(); - } + final Account account = getAuthenticatedAccount(); - return Mono.fromFuture(() -> accountsManager.updateAsync(account, a -> a.setUsernameLinkDetails(linkHandle, request.getUsernameCiphertext().toByteArray()))) - .thenReturn(responseBuilder.setUsernameLinkHandle(UUIDUtil.toByteString(linkHandle)).build()); - }); + final SetUsernameLinkResponse.Builder responseBuilder = SetUsernameLinkResponse.newBuilder(); + + if (account.getUsernameHash().isEmpty()) { + return responseBuilder.setNoUsernameSet(FailedPrecondition.getDefaultInstance()).build(); + } + + final UUID linkHandle = (request.getKeepLinkHandle() && account.getUsernameLinkHandle() != null) + ? account.getUsernameLinkHandle() + : UUID.randomUUID(); + + accountsManager.update(account, a -> a.setUsernameLinkDetails(linkHandle, request.getUsernameCiphertext().toByteArray())); + + return responseBuilder.setUsernameLinkHandle(UUIDUtil.toByteString(linkHandle)).build(); } @Override - public Mono deleteUsernameLink(final DeleteUsernameLinkRequest request) { + public DeleteUsernameLinkResponse deleteUsernameLink(final DeleteUsernameLinkRequest request) + throws RateLimitExceededException { final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice(); - return rateLimiters.getUsernameLinkOperationLimiter().validateReactive(authenticatedDevice.accountIdentifier()) - .then(getAccount()) - .flatMap(account -> Mono.fromFuture(() -> accountsManager.updateAsync(account, a -> a.setUsernameLinkDetails(null, null)))) - .thenReturn(DeleteUsernameLinkResponse.newBuilder().build()); + + rateLimiters.getUsernameLinkOperationLimiter().validate(authenticatedDevice.accountIdentifier()); + + accountsManager.update(getAuthenticatedAccount(), a -> a.setUsernameLinkDetails(null, null)); + + return DeleteUsernameLinkResponse.getDefaultInstance(); } @Override - public Mono configureUnidentifiedAccess(final ConfigureUnidentifiedAccessRequest request) { + public ConfigureUnidentifiedAccessResponse configureUnidentifiedAccess(final ConfigureUnidentifiedAccessRequest request) { if (!request.getAllowUnrestrictedUnidentifiedAccess() && request.getUnidentifiedAccessKey().size() != UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH) { throw GrpcExceptions.fieldViolation("unidentified_access_key", String.format("Unidentified access key must be %d bytes, but was actually %d", UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH, request.getUnidentifiedAccessKey().size())); } - return getAccount() - .flatMap(account -> Mono.fromFuture(() -> accountsManager.updateAsync(account, a -> { - a.setUnrestrictedUnidentifiedAccess(request.getAllowUnrestrictedUnidentifiedAccess()); - a.setUnidentifiedAccessKey(request.getAllowUnrestrictedUnidentifiedAccess() ? null : request.getUnidentifiedAccessKey().toByteArray()); - }))) - .thenReturn(ConfigureUnidentifiedAccessResponse.newBuilder().build()); + accountsManager.update(getAuthenticatedAccount(), account -> { + account.setUnrestrictedUnidentifiedAccess(request.getAllowUnrestrictedUnidentifiedAccess()); + account.setUnidentifiedAccessKey(request.getAllowUnrestrictedUnidentifiedAccess() ? null : request.getUnidentifiedAccessKey().toByteArray()); + }); + + return ConfigureUnidentifiedAccessResponse.getDefaultInstance(); } @Override - public Mono setDiscoverableByPhoneNumber(final SetDiscoverableByPhoneNumberRequest request) { - return getAccount() - .flatMap(account -> Mono.fromFuture(() -> accountsManager.updateAsync(account, - a -> a.setDiscoverableByPhoneNumber(request.getDiscoverableByPhoneNumber())))) - .thenReturn(SetDiscoverableByPhoneNumberResponse.newBuilder().build()); + public SetDiscoverableByPhoneNumberResponse setDiscoverableByPhoneNumber(final SetDiscoverableByPhoneNumberRequest request) { + accountsManager.update(getAuthenticatedAccount(), + account -> account.setDiscoverableByPhoneNumber(request.getDiscoverableByPhoneNumber())); + + return SetDiscoverableByPhoneNumberResponse.getDefaultInstance(); } @Override - public Mono setRegistrationRecoveryPassword(final SetRegistrationRecoveryPasswordRequest request) { + public SetRegistrationRecoveryPasswordResponse setRegistrationRecoveryPassword(final SetRegistrationRecoveryPasswordRequest request) { if (request.getRegistrationRecoveryPassword().isEmpty()) { throw GrpcExceptions.fieldViolation("registration_recovery_password", "Registration recovery password must not be empty"); } - return getAccount() - .flatMap(account -> Mono.fromFuture(() -> registrationRecoveryPasswordsManager.store(account.getIdentifier(IdentityType.PNI), request.getRegistrationRecoveryPassword().toByteArray()))) - .thenReturn(SetRegistrationRecoveryPasswordResponse.newBuilder().build()); + registrationRecoveryPasswordsManager.store(getAuthenticatedAccount().getIdentifier(IdentityType.PNI), + request.getRegistrationRecoveryPassword().toByteArray()) + .join(); + + return SetRegistrationRecoveryPasswordResponse.getDefaultInstance(); } - private Mono getAccount() { - return getAccount(AuthenticationUtil.requireAuthenticatedDevice()); + private Account getAuthenticatedAccount() { + return getAuthenticatedAccount(AuthenticationUtil.requireAuthenticatedDevice()); } - private Mono getAccount(AuthenticatedDevice authenticatedDevice) { - return Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier())) - .map(maybeAccount -> maybeAccount - .orElseThrow(() -> GrpcExceptions.invalidCredentials("invalid credentials"))); + private Account getAuthenticatedAccount(final AuthenticatedDevice authenticatedDevice) { + return accountsManager.getByAccountIdentifier(authenticatedDevice.accountIdentifier()) + .orElseThrow(() -> GrpcExceptions.invalidCredentials("invalid credentials")); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/RateLimitUtil.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/RateLimitUtil.java index e3b12f690..636066c4e 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/RateLimitUtil.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/RateLimitUtil.java @@ -5,12 +5,12 @@ package org.whispersystems.textsecuregcm.grpc; +import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException; import org.whispersystems.textsecuregcm.limits.RateLimiter; -import reactor.core.publisher.Mono; class RateLimitUtil { - static Mono rateLimitByRemoteAddress(final RateLimiter rateLimiter) { - return rateLimiter.validateReactive(RequestAttributesUtil.getRemoteAddress().getHostAddress()); + static void rateLimitByRemoteAddress(final RateLimiter rateLimiter) throws RateLimitExceededException { + rateLimiter.validate(RequestAttributesUtil.getRemoteAddress().getHostAddress()); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsAnonymousGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsAnonymousGrpcServiceTest.java index cec9036f9..7c4848cc8 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsAnonymousGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsAnonymousGrpcServiceTest.java @@ -10,6 +10,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -60,8 +61,8 @@ class AccountsAnonymousGrpcServiceTest extends @Override protected AccountsAnonymousGrpcService createServiceBeforeEachTest() { - when(accountsManager.getByServiceIdentifierAsync(any())) - .thenReturn(CompletableFuture.completedFuture(Optional.empty())); + when(accountsManager.getByServiceIdentifier(any())) + .thenReturn(Optional.empty()); when(accountsManager.getByUsernameHash(any())) .thenReturn(CompletableFuture.completedFuture(Optional.empty())); @@ -85,8 +86,8 @@ class AccountsAnonymousGrpcServiceTest extends void checkAccountExistence() { final AciServiceIdentifier serviceIdentifier = new AciServiceIdentifier(UUID.randomUUID()); - when(accountsManager.getByServiceIdentifierAsync(serviceIdentifier)) - .thenReturn(CompletableFuture.completedFuture(Optional.of(mock(Account.class)))); + when(accountsManager.getByServiceIdentifier(serviceIdentifier)) + .thenReturn(Optional.of(mock(Account.class))); assertTrue(unauthenticatedServiceStub().checkAccountExistence(CheckAccountExistenceRequest.newBuilder() .setServiceIdentifier(ServiceIdentifierUtil.toGrpcServiceIdentifier(serviceIdentifier)) @@ -121,11 +122,11 @@ class AccountsAnonymousGrpcServiceTest extends } @Test - void checkAccountExistenceRateLimited() { + void checkAccountExistenceRateLimited() throws RateLimitExceededException { final Duration retryAfter = Duration.ofSeconds(11); - when(rateLimiter.validateReactive(anyString())) - .thenReturn(Mono.error(new RateLimitExceededException(retryAfter))); + doThrow(new RateLimitExceededException(retryAfter)) + .when(rateLimiter).validate(anyString()); //noinspection ResultOfMethodCallIgnored GrpcTestUtils.assertRateLimitExceeded(retryAfter, @@ -153,7 +154,6 @@ class AccountsAnonymousGrpcServiceTest extends .build()) .getServiceIdentifier()); - //noinspection ResultOfMethodCallIgnored assertEquals(LookupUsernameHashResponse.newBuilder().setNotFound(NotFound.getDefaultInstance()).build(), unauthenticatedServiceStub().lookupUsernameHash(LookupUsernameHashRequest.newBuilder() .setUsernameHash(ByteString.copyFrom(new byte[AccountController.USERNAME_HASH_LENGTH])) @@ -186,11 +186,11 @@ class AccountsAnonymousGrpcServiceTest extends } @Test - void lookupUsernameHashRateLimited() { + void lookupUsernameHashRateLimited() throws RateLimitExceededException { final Duration retryAfter = Duration.ofSeconds(13); - when(rateLimiter.validateReactive(anyString())) - .thenReturn(Mono.error(new RateLimitExceededException(retryAfter))); + doThrow(new RateLimitExceededException(retryAfter)) + .when(rateLimiter).validate(anyString()); //noinspection ResultOfMethodCallIgnored GrpcTestUtils.assertRateLimitExceeded(retryAfter, @@ -255,11 +255,11 @@ class AccountsAnonymousGrpcServiceTest extends } @Test - void lookupUsernameLinkRateLimited() { + void lookupUsernameLinkRateLimited() throws RateLimitExceededException { final Duration retryAfter = Duration.ofSeconds(17); - when(rateLimiter.validateReactive(anyString())) - .thenReturn(Mono.error(new RateLimitExceededException(retryAfter))); + doThrow(new RateLimitExceededException(retryAfter)) + .when(rateLimiter).validate(anyString()); //noinspection ResultOfMethodCallIgnored GrpcTestUtils.assertRateLimitExceeded(retryAfter, diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java index a3fd6393c..eb067dbd0 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java @@ -9,7 +9,6 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -78,7 +77,6 @@ import org.whispersystems.textsecuregcm.storage.UsernameReservationNotFoundExcep import org.whispersystems.textsecuregcm.util.TestRandomUtil; import org.whispersystems.textsecuregcm.util.UUIDUtil; import org.whispersystems.textsecuregcm.util.UsernameHashZkProofVerifier; -import reactor.core.publisher.Mono; class AccountsGrpcServiceTest extends SimpleBaseGrpcTest { @@ -96,14 +94,14 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest { final Account account = invocation.getArgument(0); final Consumer updater = invocation.getArgument(1); updater.accept(account); - return CompletableFuture.completedFuture(account); + return account; }); final RateLimiters rateLimiters = mock(RateLimiters.class); @@ -111,9 +109,6 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest authenticatedServiceStub().setRegistrationLock(SetRegistrationLockRequest.newBuilder() .build())); - verify(accountsManager, never()).updateAsync(any(), any()); + verify(accountsManager, never()).update(any(), any()); } @Test @@ -219,17 +214,18 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest authenticatedServiceStub().setRegistrationLock(SetRegistrationLockRequest.newBuilder() + .setRegistrationLock(ByteString.copyFrom(TestRandomUtil.nextBytes(16))) .build())); - verify(accountsManager, never()).updateAsync(any(), any()); + verify(accountsManager, never()).update(any(), any()); } @Test void clearRegistrationLock() { final Account account = mock(Account.class); - when(accountsManager.getByAccountIdentifierAsync(AUTHENTICATED_ACI)) - .thenReturn(CompletableFuture.completedFuture(Optional.of(account))); + when(accountsManager.getByAccountIdentifier(AUTHENTICATED_ACI)) + .thenReturn(Optional.of(account)); final ClearRegistrationLockResponse ignored = authenticatedServiceStub().clearRegistrationLock(ClearRegistrationLockRequest.newBuilder().build()); @@ -252,8 +248,8 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest { + .thenAnswer(_ -> { final Account updatedAccount = mock(Account.class); when(updatedAccount.getUsernameHash()).thenReturn(Optional.of(usernameHash)); @@ -402,8 +398,8 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest authenticatedServiceStub().deleteUsernameLink(DeleteUsernameLinkRequest.newBuilder().build())); @@ -613,11 +609,11 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest authenticatedServiceStub().configureUnidentifiedAccess(ConfigureUnidentifiedAccessRequest.newBuilder() .setAllowUnrestrictedUnidentifiedAccess(unrestrictedUnidentifiedAccess) @@ -681,8 +677,8 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest authenticatedServiceStub().setDiscoverableByPhoneNumber(SetDiscoverableByPhoneNumberRequest.newBuilder() @@ -699,8 +695,8 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest