diff --git a/pom.xml b/pom.xml index e4568d085..802e3e30c 100644 --- a/pom.xml +++ b/pom.xml @@ -84,7 +84,7 @@ 2024.0.10 2.3.0 3.1.0 - 0.1.0 + 0.2.0 2.0.17 30.2.0 2.2.36 diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/grpc/AuthenticationUtil.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/grpc/AuthenticationUtil.java index dacfe5c43..1690c6626 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/grpc/AuthenticationUtil.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/grpc/AuthenticationUtil.java @@ -6,8 +6,8 @@ package org.whispersystems.textsecuregcm.auth.grpc; import io.grpc.Context; -import io.grpc.Status; import javax.annotation.Nullable; +import org.whispersystems.textsecuregcm.grpc.GrpcExceptions; import org.whispersystems.textsecuregcm.storage.Device; /** @@ -18,13 +18,11 @@ public class AuthenticationUtil { static final Context.Key CONTEXT_AUTHENTICATED_DEVICE = Context.key("authenticated-device"); /** - * Returns the account/device authenticated in the current gRPC context or throws an "unauthenticated" exception if - * no authenticated account/device is available. + * Returns the account/device authenticated in the current gRPC context. Should only be called from a service run with + * the {@link RequireAuthenticationInterceptor}. * * @return the account/device identifier authenticated in the current gRPC context - * - * @throws io.grpc.StatusRuntimeException with a status of {@code UNAUTHENTICATED} if no authenticated account/device - * could be retrieved from the current gRPC context + * @throws IllegalStateException if no authenticated account/device could be retrieved from the current gRPC context */ public static AuthenticatedDevice requireAuthenticatedDevice() { @Nullable final AuthenticatedDevice authenticatedDevice = CONTEXT_AUTHENTICATED_DEVICE.get(); @@ -33,27 +31,25 @@ public class AuthenticationUtil { return authenticatedDevice; } - throw Status.UNAUTHENTICATED.asRuntimeException(); + throw new IllegalStateException( + "Configuration issue: service expects an authenticated device, but none was found. Request should have failed from an interceptor"); } /** - * Returns the account/device authenticated in the current gRPC context or throws an "unauthenticated" exception if - * no authenticated account/device is available or "permission denied" if the authenticated device is not the primary - * device for the account. + * Returns the account/device authenticated in the current gRPC context or "invalid argument" if the authenticated + * device is not the primary device for the account. * * @return the account/device identifier authenticated in the current gRPC context - * - * @throws io.grpc.StatusRuntimeException with a status of {@code UNAUTHENTICATED} if no authenticated account/device - * could be retrieved from the current gRPC context or a status of {@code PERMISSION_DENIED} if the authenticated - * device is not the primary device for the authenticated account + * @throws io.grpc.StatusRuntimeException with a status of {@code INVALID_ARGUMENT} if the authenticated device is not + * the primary device for the authenticated account + * @throws IllegalStateException if no authenticated account/device could be retrieved from the current gRPC + * context */ public static AuthenticatedDevice requireAuthenticatedPrimaryDevice() { final AuthenticatedDevice authenticatedDevice = requireAuthenticatedDevice(); - if (authenticatedDevice.deviceId() != Device.PRIMARY_ID) { - throw Status.PERMISSION_DENIED.asRuntimeException(); + throw GrpcExceptions.badAuthentication("RPC requires a primary device"); } - return authenticatedDevice; } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/grpc/ProhibitAuthenticationInterceptor.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/grpc/ProhibitAuthenticationInterceptor.java index c0ec26e82..17ce00623 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/grpc/ProhibitAuthenticationInterceptor.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/grpc/ProhibitAuthenticationInterceptor.java @@ -9,6 +9,8 @@ import io.grpc.ServerCall; import io.grpc.ServerCallHandler; import io.grpc.ServerInterceptor; import io.grpc.Status; +import org.whispersystems.textsecuregcm.grpc.GrpcExceptions; +import org.whispersystems.textsecuregcm.grpc.ServerInterceptorUtil; /** * A "prohibit authentication" interceptor ensures that requests to endpoints that should be invoked anonymously do not @@ -22,8 +24,8 @@ public class ProhibitAuthenticationInterceptor implements ServerInterceptor { final Metadata headers, final ServerCallHandler next) { final String authHeaderString = headers.get(Metadata.Key.of(RequireAuthenticationInterceptor.AUTHORIZATION_HEADER, Metadata.ASCII_STRING_MARSHALLER)); if (authHeaderString != null) { - call.close(Status.UNAUTHENTICATED.withDescription("authorization header forbidden"), new Metadata()); - return new ServerCall.Listener<>() {}; + return ServerInterceptorUtil.closeWithStatusException(call, + GrpcExceptions.badAuthentication("The service forbids requests with an authentication header")); } return next.startCall(call, headers); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/auth/grpc/RequireAuthenticationInterceptor.java b/service/src/main/java/org/whispersystems/textsecuregcm/auth/grpc/RequireAuthenticationInterceptor.java index 14ebec7cb..42331c5dc 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/auth/grpc/RequireAuthenticationInterceptor.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/auth/grpc/RequireAuthenticationInterceptor.java @@ -14,6 +14,7 @@ import io.grpc.ServerInterceptor; import io.grpc.Status; import java.util.Optional; import org.whispersystems.textsecuregcm.auth.AccountAuthenticator; +import org.whispersystems.textsecuregcm.grpc.GrpcExceptions; import org.whispersystems.textsecuregcm.grpc.ServerInterceptorUtil; import org.whispersystems.textsecuregcm.util.HeaderUtils; @@ -40,21 +41,21 @@ public class RequireAuthenticationInterceptor implements ServerInterceptor { Metadata.Key.of(AUTHORIZATION_HEADER, Metadata.ASCII_STRING_MARSHALLER)); if (authHeaderString == null) { - return ServerInterceptorUtil.closeWithStatus(call, - Status.UNAUTHENTICATED.withDescription("missing authorization header")); + return ServerInterceptorUtil.closeWithStatusException(call, + GrpcExceptions.invalidCredentials("missing authorization header")); } final Optional basicCredentials = HeaderUtils.basicCredentialsFromAuthHeader(authHeaderString); if (basicCredentials.isEmpty()) { - return ServerInterceptorUtil.closeWithStatus(call, - Status.UNAUTHENTICATED.withDescription("malformed authorization header")); + return ServerInterceptorUtil.closeWithStatusException(call, + GrpcExceptions.invalidCredentials("malformed authorization header")); } final Optional authenticated = authenticator.authenticate(basicCredentials.get()); if (authenticated.isEmpty()) { - return ServerInterceptorUtil.closeWithStatus(call, - Status.UNAUTHENTICATED.withDescription("invalid credentials")); + return ServerInterceptorUtil.closeWithStatusException(call, + GrpcExceptions.invalidCredentials("invalid credentials")); } final AuthenticatedDevice authenticatedDevice = new AuthenticatedDevice( diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/RateLimitExceededException.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/RateLimitExceededException.java index c0538709a..a52115a3f 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/RateLimitExceededException.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/RateLimitExceededException.java @@ -4,28 +4,15 @@ */ package org.whispersystems.textsecuregcm.controllers; -import io.grpc.Metadata; -import io.grpc.Status; import java.time.Duration; import java.util.Optional; import javax.annotation.Nullable; +import io.grpc.StatusRuntimeException; import org.whispersystems.textsecuregcm.grpc.ConvertibleToGrpcStatus; +import org.whispersystems.textsecuregcm.grpc.GrpcExceptions; public class RateLimitExceededException extends Exception implements ConvertibleToGrpcStatus { - public static final Metadata.Key RETRY_AFTER_DURATION_KEY = - Metadata.Key.of("retry-after", new Metadata.AsciiMarshaller<>() { - @Override - public String toAsciiString(final Duration value) { - return value.toString(); - } - - @Override - public Duration parseAsciiString(final String serialized) { - return Duration.parse(serialized); - } - }); - @Nullable private final Duration retryDuration; @@ -44,17 +31,7 @@ public class RateLimitExceededException extends Exception implements Convertible } @Override - public Status grpcStatus() { - return Status.RESOURCE_EXHAUSTED; - } - - @Override - public Optional grpcMetadata() { - return getRetryDuration() - .map(duration -> { - final Metadata metadata = new Metadata(); - metadata.put(RETRY_AFTER_DURATION_KEY, duration); - return metadata; - }); + public StatusRuntimeException toStatusRuntimeException() { + return GrpcExceptions.rateLimitExceeded(retryDuration); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/filters/RemoteDeprecationFilter.java b/service/src/main/java/org/whispersystems/textsecuregcm/filters/RemoteDeprecationFilter.java index ee5afbe41..aa5e402d8 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/filters/RemoteDeprecationFilter.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/filters/RemoteDeprecationFilter.java @@ -27,8 +27,9 @@ import java.util.Set; import javax.annotation.Nullable; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicRemoteDeprecationConfiguration; +import org.whispersystems.textsecuregcm.grpc.GrpcExceptions; import org.whispersystems.textsecuregcm.grpc.RequestAttributesUtil; -import org.whispersystems.textsecuregcm.grpc.StatusConstants; +import org.whispersystems.textsecuregcm.grpc.ServerInterceptorUtil; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.util.ua.ClientPlatform; import org.whispersystems.textsecuregcm.util.ua.UnrecognizedUserAgentException; @@ -91,8 +92,7 @@ public class RemoteDeprecationFilter implements Filter, ServerInterceptor { }).orElse(null); if (shouldBlock(userAgent)) { - call.close(StatusConstants.UPGRADE_NEEDED_STATUS, new Metadata()); - return new ServerCall.Listener<>() {}; + return ServerInterceptorUtil.closeWithStatusException(call, GrpcExceptions.upgradeRequired()); } else { return next.startCall(call, headers); } 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 8936cb89d..c4f88c6b2 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsAnonymousGrpcService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsAnonymousGrpcService.java @@ -6,7 +6,6 @@ package org.whispersystems.textsecuregcm.grpc; import com.google.protobuf.ByteString; -import io.grpc.Status; import org.signal.chat.account.CheckAccountExistenceRequest; import org.signal.chat.account.CheckAccountExistenceResponse; import org.signal.chat.account.LookupUsernameHashRequest; @@ -14,6 +13,7 @@ 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.errors.NotFound; import org.whispersystems.textsecuregcm.controllers.AccountController; import org.whispersystems.textsecuregcm.identity.AciServiceIdentifier; import org.whispersystems.textsecuregcm.identity.ServiceIdentifier; @@ -51,18 +51,18 @@ public class AccountsAnonymousGrpcService extends ReactorAccountsAnonymousGrpc.A @Override public Mono lookupUsernameHash(final LookupUsernameHashRequest request) { if (request.getUsernameHash().size() != AccountController.USERNAME_HASH_LENGTH) { - throw Status.INVALID_ARGUMENT - .withDescription(String.format("Illegal username hash length; expected %d bytes, but got %d bytes", - AccountController.USERNAME_HASH_LENGTH, request.getUsernameHash().size())) - .asRuntimeException(); + 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.orElseThrow(Status.NOT_FOUND::asRuntimeException)) - .map(account -> LookupUsernameHashResponse.newBuilder() - .setServiceIdentifier(ServiceIdentifierUtil.toGrpcServiceIdentifier(new AciServiceIdentifier(account.getUuid()))) - .build()); + .map(maybeAccount -> maybeAccount + .map(account -> LookupUsernameHashResponse.newBuilder() + .setServiceIdentifier(ServiceIdentifierUtil.toGrpcServiceIdentifier(new AciServiceIdentifier(account.getUuid()))) + .build()) + .orElseGet(() -> LookupUsernameHashResponse.newBuilder().setNotFound(NotFound.getDefaultInstance()).build())); } @Override @@ -72,19 +72,16 @@ public class AccountsAnonymousGrpcService extends ReactorAccountsAnonymousGrpc.A try { linkHandle = UUIDUtil.fromByteString(request.getUsernameLinkHandle()); } catch (final IllegalArgumentException e) { - throw Status.INVALID_ARGUMENT - .withDescription("Could not interpret link handle as UUID") - .withCause(e) - .asRuntimeException(); + 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) - .orElseThrow(Status.NOT_FOUND::asRuntimeException)) - .map(usernameCiphertext -> LookupUsernameLinkResponse.newBuilder() - .setUsernameCiphertext(ByteString.copyFrom(usernameCiphertext)) - .build()); + .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 1209f1323..ef36593a5 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java @@ -6,7 +6,6 @@ package org.whispersystems.textsecuregcm.grpc; import com.google.protobuf.ByteString; -import io.grpc.Status; import java.util.ArrayList; import java.util.HexFormat; import java.util.List; @@ -26,8 +25,6 @@ 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.ReserveUsernameHashError; -import org.signal.chat.account.ReserveUsernameHashErrorType; import org.signal.chat.account.ReserveUsernameHashRequest; import org.signal.chat.account.ReserveUsernameHashResponse; import org.signal.chat.account.SetDiscoverableByPhoneNumberRequest; @@ -38,7 +35,9 @@ 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.UsernameNotAvailable; import org.signal.chat.common.AccountIdentifiers; +import org.signal.chat.errors.FailedPrecondition; import org.signal.libsignal.usernames.BaseUsernameException; import org.whispersystems.textsecuregcm.auth.SaltedTokenHash; import org.whispersystems.textsecuregcm.auth.UnidentifiedAccessUtil; @@ -50,6 +49,7 @@ import org.whispersystems.textsecuregcm.identity.AciServiceIdentifier; import org.whispersystems.textsecuregcm.identity.IdentityType; import org.whispersystems.textsecuregcm.identity.PniServiceIdentifier; import org.whispersystems.textsecuregcm.limits.RateLimiters; +import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.RegistrationRecoveryPasswordsManager; import org.whispersystems.textsecuregcm.storage.UsernameHashNotAvailableException; @@ -78,10 +78,7 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { @Override public Mono getAccountIdentity(final GetAccountIdentityRequest request) { - final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice(); - - return Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier())) - .map(maybeAccount -> maybeAccount.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException)) + return getAccount() .map(account -> { final AccountIdentifiers.Builder accountIdentifiersBuilder = AccountIdentifiers.newBuilder() .addServiceIdentifiers(ServiceIdentifierUtil.toGrpcServiceIdentifier(new AciServiceIdentifier(account.getUuid()))) @@ -99,10 +96,7 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { @Override public Mono deleteAccount(final DeleteAccountRequest request) { - final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedPrimaryDevice(); - - return Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier())) - .map(maybeAccount -> maybeAccount.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException)) + return getAccount(AuthenticationUtil.requireAuthenticatedPrimaryDevice()) .flatMap(account -> Mono.fromFuture(() -> accountsManager.delete(account, AccountsManager.DeletionReason.USER_REQUEST))) .thenReturn(DeleteAccountResponse.newBuilder().build()); } @@ -112,11 +106,10 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedPrimaryDevice(); if (request.getRegistrationLock().isEmpty()) { - throw Status.INVALID_ARGUMENT.withDescription("Registration lock secret must not be empty").asRuntimeException(); + throw GrpcExceptions.fieldViolation("registration_lock", "Registration lock secret must not be empty"); } - return Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier())) - .map(maybeAccount -> maybeAccount.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException)) + 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. @@ -131,10 +124,7 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { @Override public Mono clearRegistrationLock(final ClearRegistrationLockRequest request) { - final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedPrimaryDevice(); - - return Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier())) - .map(maybeAccount -> maybeAccount.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException)) + return getAccount(AuthenticationUtil.requireAuthenticatedPrimaryDevice()) .flatMap(account -> Mono.fromFuture(() -> accountsManager.updateAsync(account, a -> a.setRegistrationLock(null, null)))) .map(ignored -> ClearRegistrationLockResponse.newBuilder().build()); @@ -145,42 +135,34 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice(); if (request.getUsernameHashesCount() == 0) { - throw Status.INVALID_ARGUMENT - .withDescription("List of username hashes must not be empty") - .asRuntimeException(); + throw GrpcExceptions.fieldViolation("username_hashes", "List of username hashes must not be empty"); } if (request.getUsernameHashesCount() > AccountController.MAXIMUM_USERNAME_HASHES_LIST_LENGTH) { - throw Status.INVALID_ARGUMENT - .withDescription(String.format("List of username hashes may have at most %d elements, but actually had %d", - AccountController.MAXIMUM_USERNAME_HASHES_LIST_LENGTH, request.getUsernameHashesCount())) - .asRuntimeException(); + throw GrpcExceptions.fieldViolation("username_hashes", + String.format("List of username hashes may have at most %d elements, but actually had %d", + AccountController.MAXIMUM_USERNAME_HASHES_LIST_LENGTH, request.getUsernameHashesCount())); } final List usernameHashes = new ArrayList<>(request.getUsernameHashesCount()); for (final ByteString usernameHash : request.getUsernameHashesList()) { if (usernameHash.size() != AccountController.USERNAME_HASH_LENGTH) { - throw Status.INVALID_ARGUMENT - .withDescription(String.format("Username hash length must be %d bytes, but was actually %d", - AccountController.USERNAME_HASH_LENGTH, usernameHash.size())) - .asRuntimeException(); + throw GrpcExceptions.fieldViolation("username_hashes", + String.format("Username hash length must be %d bytes, but was actually %d", + AccountController.USERNAME_HASH_LENGTH, usernameHash.size())); } - usernameHashes.add(usernameHash.toByteArray()); } return rateLimiters.getUsernameReserveLimiter().validateReactive(authenticatedDevice.accountIdentifier()) - .then(Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier()))) - .map(maybeAccount -> maybeAccount.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException)) + .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() - .setError(ReserveUsernameHashError.newBuilder() - .setErrorType(ReserveUsernameHashErrorType.RESERVE_USERNAME_HASH_ERROR_TYPE_NO_HASHES_AVAILABLE) - .build()) + .setUsernameNotAvailable(UsernameNotAvailable.getDefaultInstance()) .build()); } @@ -189,61 +171,57 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice(); if (request.getUsernameHash().isEmpty()) { - throw Status.INVALID_ARGUMENT - .withDescription("Username hash must not be empty") - .asRuntimeException(); + throw GrpcExceptions.fieldViolation("username_hash", "Username hash must not be empty"); } if (request.getUsernameHash().size() != AccountController.USERNAME_HASH_LENGTH) { - throw Status.INVALID_ARGUMENT - .withDescription(String.format("Username hash length must be %d bytes, but was actually %d", - AccountController.USERNAME_HASH_LENGTH, request.getUsernameHash().size())) - .asRuntimeException(); + throw GrpcExceptions.fieldViolation("username_hash", + String.format("Username hash length must be %d bytes, but was actually %d", + AccountController.USERNAME_HASH_LENGTH, request.getUsernameHash().size())); } if (request.getZkProof().isEmpty()) { - throw Status.INVALID_ARGUMENT - .withDescription("Zero-knowledge proof must not be empty") - .asRuntimeException(); + throw GrpcExceptions.fieldViolation("zk_proof", "Zero-knowledge proof must not be empty"); } if (request.getUsernameCiphertext().isEmpty()) { - throw Status.INVALID_ARGUMENT - .withDescription("Username ciphertext must not be empty") - .asRuntimeException(); + throw GrpcExceptions.fieldViolation("username_ciphertext", "Username ciphertext must not be empty"); } if (request.getUsernameCiphertext().size() > AccountController.MAXIMUM_USERNAME_CIPHERTEXT_LENGTH) { - throw Status.INVALID_ARGUMENT - .withDescription(String.format("Username hash length must at most %d bytes, but was actually %d", - AccountController.MAXIMUM_USERNAME_CIPHERTEXT_LENGTH, request.getUsernameCiphertext().size())) - .asRuntimeException(); + throw GrpcExceptions.fieldViolation("username_ciphertext", + String.format("Username ciphertext length must at most %d bytes, but was actually %d", + AccountController.MAXIMUM_USERNAME_CIPHERTEXT_LENGTH, request.getUsernameCiphertext().size())); } try { usernameHashZkProofVerifier.verifyProof(request.getZkProof().toByteArray(), request.getUsernameHash().toByteArray()); } catch (final BaseUsernameException e) { - throw Status.INVALID_ARGUMENT.withDescription("Could not verify proof").asRuntimeException(); + throw GrpcExceptions.constraintViolation("Could not verify proof"); } return rateLimiters.getUsernameSetLimiter().validateReactive(authenticatedDevice.accountIdentifier()) - .then(Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier()))) - .map(maybeAccount -> maybeAccount.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException)) + .then(getAccount()) .flatMap(account -> Mono.fromFuture(() -> accountsManager.confirmReservedUsernameHash(account, request.getUsernameHash().toByteArray(), request.getUsernameCiphertext().toByteArray()))) .map(updatedAccount -> ConfirmUsernameHashResponse.newBuilder() - .setUsernameHash(ByteString.copyFrom(updatedAccount.getUsernameHash().orElseThrow())) - .setUsernameLinkHandle(UUIDUtil.toByteString(updatedAccount.getUsernameLinkHandle())) + .setConfirmedUsernameHash(ConfirmUsernameHashResponse.ConfirmedUsernameHash.newBuilder() + .setUsernameHash(ByteString.copyFrom(updatedAccount.getUsernameHash().orElseThrow())) + .setUsernameLinkHandle(UUIDUtil.toByteString(updatedAccount.getUsernameLinkHandle())) + .build()) .build()) - .onErrorMap(UsernameReservationNotFoundException.class, throwable -> Status.FAILED_PRECONDITION.asRuntimeException()) - .onErrorMap(UsernameHashNotAvailableException.class, throwable -> Status.NOT_FOUND.asRuntimeException()); + .onErrorResume(UsernameReservationNotFoundException.class, _ -> Mono.just(ConfirmUsernameHashResponse + .newBuilder() + .setReservationNotFound(FailedPrecondition.getDefaultInstance()) + .build())) + .onErrorResume(UsernameHashNotAvailableException.class, _ -> Mono.just(ConfirmUsernameHashResponse + .newBuilder() + .setUsernameNotAvailable(UsernameNotAvailable.getDefaultInstance()) + .build())); } @Override public Mono deleteUsernameHash(final DeleteUsernameHashRequest request) { - final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice(); - - return Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier())) - .map(maybeAccount -> maybeAccount.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException)) + return getAccount() .flatMap(account -> Mono.fromFuture(() -> accountsManager.clearUsernameHash(account))) .thenReturn(DeleteUsernameHashResponse.newBuilder().build()); } @@ -253,19 +231,16 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice(); if (request.getUsernameCiphertext().isEmpty() || request.getUsernameCiphertext().size() > EncryptedUsername.MAX_SIZE) { - throw Status.INVALID_ARGUMENT - .withDescription(String.format("Username ciphertext must not be empty and must be shorter than %d bytes", EncryptedUsername.MAX_SIZE)) - .asRuntimeException(); + throw GrpcExceptions.fieldViolation("username_ciphertext", + 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(Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier()))) - .map(maybeAccount -> maybeAccount.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException)) + .then(getAccount()) .flatMap(account -> { + final SetUsernameLinkResponse.Builder responseBuilder = SetUsernameLinkResponse.newBuilder(); if (account.getUsernameHash().isEmpty()) { - return Mono.error(Status.FAILED_PRECONDITION - .withDescription("Account does not have a username hash") - .asRuntimeException()); + return Mono.just(responseBuilder.setNoUsernameSet(FailedPrecondition.getDefaultInstance()).build()); } final UUID linkHandle; @@ -276,37 +251,28 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { } return Mono.fromFuture(() -> accountsManager.updateAsync(account, a -> a.setUsernameLinkDetails(linkHandle, request.getUsernameCiphertext().toByteArray()))) - .thenReturn(linkHandle); - }) - .map(linkHandle -> SetUsernameLinkResponse.newBuilder() - .setUsernameLinkHandle(UUIDUtil.toByteString(linkHandle)) - .build()); + .thenReturn(responseBuilder.setUsernameLinkHandle(UUIDUtil.toByteString(linkHandle)).build()); + }); } @Override public Mono deleteUsernameLink(final DeleteUsernameLinkRequest request) { final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice(); - return rateLimiters.getUsernameLinkOperationLimiter().validateReactive(authenticatedDevice.accountIdentifier()) - .then(Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier()))) - .map(maybeAccount -> maybeAccount.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException)) + .then(getAccount()) .flatMap(account -> Mono.fromFuture(() -> accountsManager.updateAsync(account, a -> a.setUsernameLinkDetails(null, null)))) .thenReturn(DeleteUsernameLinkResponse.newBuilder().build()); } @Override public Mono configureUnidentifiedAccess(final ConfigureUnidentifiedAccessRequest request) { - final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice(); - if (!request.getAllowUnrestrictedUnidentifiedAccess() && request.getUnidentifiedAccessKey().size() != UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH) { - throw Status.INVALID_ARGUMENT - .withDescription(String.format("Unidentified access key must be %d bytes, but was actually %d", - UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH, request.getUnidentifiedAccessKey().size())) - .asRuntimeException(); + 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 Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier())) - .map(maybeAccount -> maybeAccount.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException)) + return getAccount() .flatMap(account -> Mono.fromFuture(() -> accountsManager.updateAsync(account, a -> { a.setUnrestrictedUnidentifiedAccess(request.getAllowUnrestrictedUnidentifiedAccess()); a.setUnidentifiedAccessKey(request.getAllowUnrestrictedUnidentifiedAccess() ? null : request.getUnidentifiedAccessKey().toByteArray()); @@ -316,10 +282,7 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { @Override public Mono setDiscoverableByPhoneNumber(final SetDiscoverableByPhoneNumberRequest request) { - final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice(); - - return Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier())) - .map(maybeAccount -> maybeAccount.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException)) + return getAccount() .flatMap(account -> Mono.fromFuture(() -> accountsManager.updateAsync(account, a -> a.setDiscoverableByPhoneNumber(request.getDiscoverableByPhoneNumber())))) .thenReturn(SetDiscoverableByPhoneNumberResponse.newBuilder().build()); @@ -327,17 +290,22 @@ public class AccountsGrpcService extends ReactorAccountsGrpc.AccountsImplBase { @Override public Mono setRegistrationRecoveryPassword(final SetRegistrationRecoveryPasswordRequest request) { - final AuthenticatedDevice authenticatedDevice = AuthenticationUtil.requireAuthenticatedDevice(); - if (request.getRegistrationRecoveryPassword().isEmpty()) { - throw Status.INVALID_ARGUMENT - .withDescription("Registration recovery password must not be empty") - .asRuntimeException(); + throw GrpcExceptions.fieldViolation("registration_recovery_password", "Registration recovery password must not be empty"); } - return Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier())) - .map(maybeAccount -> maybeAccount.orElseThrow(Status.UNAUTHENTICATED::asRuntimeException)) + return getAccount() .flatMap(account -> Mono.fromFuture(() -> registrationRecoveryPasswordsManager.store(account.getIdentifier(IdentityType.PNI), request.getRegistrationRecoveryPassword().toByteArray()))) .thenReturn(SetRegistrationRecoveryPasswordResponse.newBuilder().build()); } + + private Mono getAccount() { + return getAccount(AuthenticationUtil.requireAuthenticatedDevice()); + } + + private Mono getAccount(AuthenticatedDevice authenticatedDevice) { + return Mono.fromFuture(() -> accountsManager.getByAccountIdentifierAsync(authenticatedDevice.accountIdentifier())) + .map(maybeAccount -> maybeAccount + .orElseThrow(() -> GrpcExceptions.invalidCredentials("invalid credentials"))); + } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ConvertibleToGrpcStatus.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ConvertibleToGrpcStatus.java index 73fb1cb85..4d35c9977 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ConvertibleToGrpcStatus.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ConvertibleToGrpcStatus.java @@ -7,14 +7,12 @@ package org.whispersystems.textsecuregcm.grpc; import io.grpc.Metadata; import io.grpc.Status; +import io.grpc.StatusRuntimeException; import java.util.Optional; /** * Interface to be implemented by our custom exceptions that are consistently mapped to a gRPC status. */ public interface ConvertibleToGrpcStatus { - - Status grpcStatus(); - - Optional grpcMetadata(); + StatusRuntimeException toStatusRuntimeException(); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ErrorMappingInterceptor.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ErrorMappingInterceptor.java index 75d25d498..eeddbe203 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ErrorMappingInterceptor.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ErrorMappingInterceptor.java @@ -11,6 +11,11 @@ import io.grpc.ServerCall; import io.grpc.ServerCallHandler; import io.grpc.ServerInterceptor; import io.grpc.Status; +import io.grpc.StatusRuntimeException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.io.IOException; +import java.io.UncheckedIOException; /** * This interceptor observes responses from the service and if the response status is {@link Status#UNKNOWN} @@ -21,6 +26,8 @@ import io.grpc.Status; */ public class ErrorMappingInterceptor implements ServerInterceptor { + private static final Logger log = LoggerFactory.getLogger(ErrorMappingInterceptor.class); + @Override public ServerCall.Listener interceptCall( final ServerCall call, @@ -34,15 +41,33 @@ public class ErrorMappingInterceptor implements ServerInterceptor { // I.e. if at this point we see anything but the `UNKNOWN`, // that means that some logic in the service made this decision already // and automatic conversion may conflict with it. - if (status.getCode().equals(Status.Code.UNKNOWN) - && status.getCause() instanceof ConvertibleToGrpcStatus convertibleToGrpcStatus) { - super.close( - convertibleToGrpcStatus.grpcStatus(), - convertibleToGrpcStatus.grpcMetadata().orElseGet(Metadata::new) - ); - } else { + if (!status.getCode().equals(Status.Code.UNKNOWN)) { super.close(status, trailers); + return; } + + final StatusRuntimeException statusException = switch (status.getCause()) { + case ConvertibleToGrpcStatus e -> e.toStatusRuntimeException(); + case UncheckedIOException e -> { + log.warn("RPC {} encountered UncheckedIOException", call.getMethodDescriptor().getFullMethodName(), e.getCause()); + yield GrpcExceptions.unavailable(e.getCause().getMessage()); + } + case IOException e -> { + log.warn("RPC {} encountered IOException", call.getMethodDescriptor().getFullMethodName(), e); + yield GrpcExceptions.unavailable(e.getMessage()); + } + case null -> { + log.error("RPC {} finished with status UNKNOWN: {}", + call.getMethodDescriptor().getFullMethodName(), status.getDescription()); + yield GrpcExceptions.unavailable(status.getDescription()); + } + default -> { + log.error("RPC {} finished with status UNKNOWN", + call.getMethodDescriptor().getFullMethodName(), status.getCause()); + yield GrpcExceptions.unavailable(status.getCause().getMessage()); + } + }; + super.close(statusException.getStatus(), statusException.getTrailers()); } }, headers); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/GrpcExceptions.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/GrpcExceptions.java new file mode 100644 index 000000000..731902974 --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/GrpcExceptions.java @@ -0,0 +1,164 @@ +/* + * Copyright 2025 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.grpc; + +import com.google.protobuf.Any; +import com.google.rpc.BadRequest; +import com.google.rpc.ErrorInfo; +import com.google.rpc.RetryInfo; +import io.grpc.Status; +import io.grpc.StatusRuntimeException; +import io.grpc.protobuf.StatusProto; +import java.time.Duration; +import javax.annotation.Nullable; + +public class GrpcExceptions { + + public static final String DOMAIN = "grpc.chat.signal.org"; + + private static final Any ERROR_INFO_CONSTRAINT_VIOLATED = Any.pack(ErrorInfo.newBuilder() + .setDomain(DOMAIN) + .setReason("CONSTRAINT_VIOLATED") + .build()); + + private static final Any ERROR_INFO_RESOURCE_EXHAUSTED = Any.pack(ErrorInfo.newBuilder() + .setDomain(DOMAIN) + .setReason("RESOURCE_EXHAUSTED") + .build()); + + private static final Any ERROR_INFO_INVALID_CREDENTIALS = Any.pack(ErrorInfo.newBuilder() + .setDomain(DOMAIN) + .setReason("INVALID_CREDENTIALS") + .build()); + + private static final Any ERROR_INFO_BAD_AUTHENTICATION = Any.pack(ErrorInfo.newBuilder() + .setDomain(DOMAIN) + .setReason("BAD_AUTHENTICATION") + .build()); + + private static final com.google.rpc.Status UPGRADE_REQUIRED = com.google.rpc.Status.newBuilder() + .setCode(Status.Code.INVALID_ARGUMENT.value()) + .setMessage("Upgrade required") + .addDetails(Any.pack(ErrorInfo.newBuilder() + .setDomain(DOMAIN) + .setReason("UPGRADE_REQUIRED") + .build())) + .build(); + + + private GrpcExceptions() { + } + + /// The client version provided in the User-Agent is no longer supported. The client must upgrade to use the service. + /// + /// @return A [StatusRuntimeException] encoding the error + public static StatusRuntimeException upgradeRequired() { + return StatusProto.toStatusRuntimeException(UPGRADE_REQUIRED); + } + + /// The RPC argument violated a constraint that was annotated or documented in the service definition. It is always + /// possible to check this constraint without communicating with the chat server. This always represents a client bug + /// or out of date client. Additional information about the violating field will be included in the metadata. + /// + /// @param fieldName The name of the field that violated a service constraint + /// @param message Additional context about the constraint violation + /// @return A [StatusRuntimeException] encoding the error + public static StatusRuntimeException fieldViolation(final String fieldName, @Nullable final String message) { + return StatusProto.toStatusRuntimeException(com.google.rpc.Status.newBuilder() + .setCode(Status.Code.INVALID_ARGUMENT.value()) + .setMessage(messageOrDefault(message, Status.Code.INVALID_ARGUMENT)) + .addDetails(ERROR_INFO_CONSTRAINT_VIOLATED) + .addDetails(Any.pack(BadRequest.newBuilder() + .addFieldViolations(BadRequest.FieldViolation.newBuilder() + .setField(fieldName) + .setDescription(messageOrDefault(message, Status.Code.INVALID_ARGUMENT))) + .build())) + .build()); + } + + /// The RPC argument violated a constraint that was annotated or documented in the service definition. It is always + /// possible to check this constraint without communicating with the chat server. This always represents a client bug + /// or out of date client. + /// + /// @param message Additional context about the constraint violation + /// @return A [StatusRuntimeException] encoding the error + public static StatusRuntimeException constraintViolation(@Nullable final String message) { + return StatusProto.toStatusRuntimeException(com.google.rpc.Status.newBuilder() + .setCode(Status.Code.INVALID_ARGUMENT.value()) + .setMessage(messageOrDefault(message, Status.Code.INVALID_ARGUMENT)) + .addDetails(ERROR_INFO_CONSTRAINT_VIOLATED) + .build()); + } + + /// The request has incorrectly set authentication credentials for the RPC. This represents a client bug where the + /// authorization header is not correct for the RPC. For example, + /// + /// - The RPC was for an anonymous service, but included an Authentication header in the RPC metadata + /// - The RPC should only be made by the primary device, but the request had linked device credentials + /// + /// @param message indicating why the credentials were set incorrectly + /// @return A [StatusRuntimeException] encoding the error + public static StatusRuntimeException badAuthentication(@Nullable final String message) { + return StatusProto.toStatusRuntimeException(com.google.rpc.Status.newBuilder() + .setCode(Status.Code.INVALID_ARGUMENT.value()) + .setMessage(messageOrDefault(message, Status.Code.INVALID_ARGUMENT)) + .addDetails(ERROR_INFO_BAD_AUTHENTICATION) + .build()); + } + + /// The account credentials provided in the authorization header are no longer valid. + /// + /// @param message indicating why the credentials were invalid + /// @return A [StatusRuntimeException] encoding the error + public static StatusRuntimeException invalidCredentials(@Nullable final String message) { + return StatusProto.toStatusRuntimeException(com.google.rpc.Status.newBuilder() + .setCode(Status.Code.UNAUTHENTICATED.value()) + .setMessage(messageOrDefault(message, Status.Code.UNAUTHENTICATED)) + .addDetails(ERROR_INFO_INVALID_CREDENTIALS) + .build()); + } + + /// A server-side resource was exhausted. The details field may include a RetryInfo message that includes the amount + /// of time in seconds the client should wait before retrying the request. + /// + /// If a RetryInfo is present, the client must wait the indicated time before retrying the request. If absent, the + /// client should retry with an exponential backoff. + /// + /// @param retryDuration If present, the duration the client should wait before retrying the request + /// @return A [StatusRuntimeException] encoding the error + public static StatusRuntimeException rateLimitExceeded(@Nullable final Duration retryDuration) { + final com.google.rpc.Status.Builder builder = com.google.rpc.Status.newBuilder() + .setCode(Status.Code.RESOURCE_EXHAUSTED.value()) + .addDetails(ERROR_INFO_RESOURCE_EXHAUSTED); + + if (retryDuration != null) { + builder.addDetails(Any.pack(RetryInfo.newBuilder() + .setRetryDelay(com.google.protobuf.Duration.newBuilder() + .setSeconds(retryDuration.getSeconds()) + .setNanos(retryDuration.getNano())) + .build())); + } + return StatusProto.toStatusRuntimeException(builder.build()); + } + + /// There was an internal error processing the RPC. The client should retry the request with exponential backoff. + /// + /// @return A [StatusRuntimeException] encoding the error + public static StatusRuntimeException unavailable(@Nullable final String message) { + return StatusProto.toStatusRuntimeException(com.google.rpc.Status.newBuilder() + .setCode(Status.Code.UNAVAILABLE.value()) + .setMessage(messageOrDefault(message, Status.Code.UNAVAILABLE)) + .addDetails(Any.pack(ErrorInfo.newBuilder() + .setDomain(DOMAIN) + .setReason("UNAVAILABLE") + .build())) + .build()); + } + + private static String messageOrDefault(@Nullable final String message, Status.Code code) { + return message == null ? code.toString() : message; + } +} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptor.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptor.java index e458657e7..e96767116 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptor.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptor.java @@ -6,26 +6,50 @@ package org.whispersystems.textsecuregcm.grpc; import com.google.common.annotations.VisibleForTesting; -import io.grpc.*; +import com.google.protobuf.Descriptors; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.Message; +import com.google.rpc.ErrorInfo; +import io.grpc.ForwardingServerCall; +import io.grpc.ForwardingServerCallListener; +import io.grpc.Metadata; +import io.grpc.ServerCall; +import io.grpc.ServerCallHandler; +import io.grpc.ServerInterceptor; +import io.grpc.Status; +import io.grpc.protobuf.StatusProto; import io.micrometer.core.instrument.Counter; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Tags; import io.micrometer.core.instrument.Timer; +import java.io.UncheckedIOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Stream; +import javax.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.whispersystems.textsecuregcm.metrics.MetricsUtil; import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil; import org.whispersystems.textsecuregcm.storage.ClientReleaseManager; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Optional; public class MetricServerInterceptor implements ServerInterceptor { + private static final Logger log = LoggerFactory.getLogger(MetricServerInterceptor.class); + private static final String TAG_SERVICE_NAME = "grpcService"; private static final String TAG_METHOD_NAME = "method"; private static final String TAG_METHOD_TYPE = "methodType"; private static final String TAG_STATUS_CODE = "statusCode"; + private static final String TAG_REASON = "reason"; + + @VisibleForTesting + static final String DEFAULT_SUCCESS_REASON = "success"; + @VisibleForTesting + static final String DEFAULT_ERROR_REASON = "n/a"; @VisibleForTesting static final String REQUEST_MESSAGE_COUNTER_NAME = MetricsUtil.name(MetricServerInterceptor.class, "requestMessage"); @@ -77,6 +101,7 @@ public class MetricServerInterceptor implements ServerInterceptor { private final Counter responseMessageCounter; private final Tags tags; + private @Nullable String reason = null; MetricServerCall(final ServerCall delegate, final Tags tags) { super(delegate); @@ -86,15 +111,59 @@ public class MetricServerInterceptor implements ServerInterceptor { @Override public void close(final Status status, final Metadata responseHeaders) { - meterRegistry.counter(RPC_COUNTER_NAME, tags.and(TAG_STATUS_CODE, status.getCode().name())).increment(); + if (!status.isOk()) { + reason = errorInfo(StatusProto.fromStatusAndTrailers(status, responseHeaders)) + .map(ErrorInfo::getReason) + .orElse(DEFAULT_ERROR_REASON); + } + Tags responseTags = tags.and(Tag.of(TAG_STATUS_CODE, status.getCode().name())); + if (reason != null) { + responseTags = responseTags.and(TAG_REASON, reason); + } + meterRegistry.counter(RPC_COUNTER_NAME, responseTags).increment(); super.close(status, responseHeaders); } @Override public void sendMessage(final RespT responseMessage) { this.responseMessageCounter.increment(); + // Extract the annotated reason (if any) from the message + final String messageReason = MetricServerCall.reason(responseMessage); + + // If there are multiple messages sent on this RPC (server-side streaming), just use the most recent reason + this.reason = messageReason == null ? DEFAULT_SUCCESS_REASON : messageReason; + super.sendMessage(responseMessage); } + + @Nullable + private static String reason(final Object obj) { + if (!(obj instanceof Message msg)) { + return null; + } + // iterate through all fields on the message + for (Map.Entry field : msg.getAllFields().entrySet()) { + // iterate through all options on the field + for (Map.Entry option : field.getKey().getOptions().getAllFields().entrySet()) { + if (option.getKey().getFullName().equals("org.signal.chat.tag.reason")) { + if (!(option.getValue() instanceof String s)) { + log.error("Invalid value for option tag.reason {}", option.getValue()); + continue; + } + // return the first tag we see + return s; + } + } + + // No reason on this field. Recursively check subfields of this field for a reason + final String subReason = reason(field.getValue()); + if (subReason != null) { + return subReason; + } + } + // No field or subfield contained an annotated reason + return null; + } } /** @@ -131,4 +200,17 @@ public class MetricServerInterceptor implements ServerInterceptor { super.onCancel(); } } + + private static Optional errorInfo(final com.google.rpc.Status statusProto) { + return statusProto.getDetailsList().stream() + .filter(any -> any.is(ErrorInfo.class)) + .map(errorInfo -> { + try { + return errorInfo.unpack(ErrorInfo.class); + } catch (final InvalidProtocolBufferException e) { + throw new UncheckedIOException(e); + } + }) + .findFirst(); + } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ServerInterceptorUtil.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ServerInterceptorUtil.java index 816319b4c..dca04e6f2 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ServerInterceptorUtil.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ServerInterceptorUtil.java @@ -8,6 +8,7 @@ package org.whispersystems.textsecuregcm.grpc; import io.grpc.Metadata; import io.grpc.ServerCall; import io.grpc.Status; +import io.grpc.StatusRuntimeException; public class ServerInterceptorUtil { @@ -36,4 +37,23 @@ public class ServerInterceptorUtil { //noinspection unchecked return NO_OP_LISTENER; } + + /** + * Closes the given server call with the status and metadata from the provided exception, returning a no-op listener. + * + * @param call the server call to close + * @param exception the {@link StatusRuntimeException} with which to close the call + * + * @return a no-op server call listener + * + * @param the type of request object handled by the server call + * @param the type of response object returned by the server call + */ + public static ServerCall.Listener closeWithStatusException(final ServerCall call, final StatusRuntimeException exception) { + call.close(exception.getStatus(), exception.getTrailers()); + + //noinspection unchecked + return NO_OP_LISTENER; + } + } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/StatusConstants.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/StatusConstants.java deleted file mode 100644 index 78836ff20..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/StatusConstants.java +++ /dev/null @@ -1,12 +0,0 @@ -/* - * Copyright 2023 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.grpc; - -import io.grpc.Status; - -public abstract class StatusConstants { - public static final Status UPGRADE_NEEDED_STATUS = Status.INVALID_ARGUMENT.withDescription("signal-upgrade-required"); -} diff --git a/service/src/main/proto/org/signal/chat/account.proto b/service/src/main/proto/org/signal/chat/account.proto index 66dc89b1a..7b6b0998e 100644 --- a/service/src/main/proto/org/signal/chat/account.proto +++ b/service/src/main/proto/org/signal/chat/account.proto @@ -5,6 +5,7 @@ option java_multiple_files = true; package org.signal.chat.account; import "org/signal/chat/common.proto"; +import "org/signal/chat/errors.proto"; /** * Provides methods for working with Signal accounts. @@ -34,23 +35,13 @@ service Accounts { /** * Attempts to reserve one of multiple given username hashes. Reserved - * usernames may be claimed later via `ConfirmUsernameHash`. This RPC may - * fail with a `RESOURCE_EXHAUSTED` status if a rate limit for modifying - * usernames has been exceeded, in which case a `retry-after` header - * containing an ISO 8601 duration string will be present in the response - * trailers. + * usernames may be claimed later via `ConfirmUsernameHash`. */ rpc ReserveUsernameHash(ReserveUsernameHashRequest) returns (ReserveUsernameHashResponse) {} /** * Sets the username hash/encrypted username to a previously-reserved value - * (see `ReserveUsernameHash`). This RPC may fail with a status of - * `FAILED_PRECONDITION` if no reserved username hash was found for the given - * account or `NOT_FOUND` if the reservation has lapsed and been claimed by - * another caller. It may also fail with a `RESOURCE_EXHAUSTED` if a rate - * limit for modifying usernames has been exceeded, in which case a - * `retry-after` header containing an ISO 8601 duration string will be present - * in the response trailers. + * (see `ReserveUsernameHash`). */ rpc ConfirmUsernameHash(ConfirmUsernameHashRequest) returns (ConfirmUsernameHashResponse) {} @@ -64,21 +55,11 @@ service Accounts { * Associates the given username ciphertext with the account, replacing any * previously stored ciphertext. A new link handle will optionally be created, * and the link handle to use will be returned in any event. - * - * This RPC may fail with a status of `FAILED_PRECONDITION` if the - * authenticated account does not have a username. It may also fail with - * `RESOURCE_EXHAUSTED` if a rate limit for modifying username links has been - * exceeded, in which case a `retry-after` header containing an ISO 8601 - * duration string will be present in the response trailers. */ rpc SetUsernameLink(SetUsernameLinkRequest) returns (SetUsernameLinkResponse) {} /** - * Clears any username link associated with the authenticated account. This - * RPC may fail with `RESOURCE_EXHAUSTED` if a rate limit for modifying - * username links has been exceeded, in which case a `retry-after` header - * containing an ISO 8601 duration string will be present in the response - * trailers. + * Clears any username link associated with the authenticated account. */ rpc DeleteUsernameLink(DeleteUsernameLinkRequest) returns (DeleteUsernameLinkResponse) {} @@ -114,8 +95,7 @@ service AccountsAnonymous { /** * Finds the service identifier of the account associated with the given - * username hash. This method will return a `NOT_FOUND` status if no account - * was found for the given username hash. + * username hash. */ rpc LookupUsernameHash(LookupUsernameHashRequest) returns (LookupUsernameHashResponse) {} @@ -166,6 +146,8 @@ message ReserveUsernameHashRequest { repeated bytes username_hashes = 1; } +message UsernameNotAvailable {} + message ReserveUsernameHashResponse { oneof response { /** @@ -174,26 +156,13 @@ message ReserveUsernameHashResponse { bytes username_hash = 1; /** - * An error indicating why a username hash could not be reserved. + * Indicates that, of all of the candidate hashes provided, none were + * available. Callers may generate a new set of hashes and and retry. */ - ReserveUsernameHashError error = 2; + UsernameNotAvailable username_not_available = 2; } } -message ReserveUsernameHashError { - ReserveUsernameHashErrorType error_type = 1; -} - -enum ReserveUsernameHashErrorType { - RESERVE_USERNAME_HASH_ERROR_TYPE_UNSPECIFIED = 0; - - /** - * Indicates that, of all of the candidate hashes provided, none were - * available. Callers may generate a new set of hashes and and retry. - */ - RESERVE_USERNAME_HASH_ERROR_TYPE_NO_HASHES_AVAILABLE = 1; -} - message ConfirmUsernameHashRequest { /** * The username hash to claim for the authenticated account. @@ -214,15 +183,35 @@ message ConfirmUsernameHashRequest { } message ConfirmUsernameHashResponse { - /** - * The newly-confirmed username hash. - */ - bytes username_hash = 1; + message ConfirmedUsernameHash { + /** + * The newly-confirmed username hash. + */ + bytes username_hash = 1; - /** - * The server-generated username link handle for the newly-confirmed username. - */ - bytes username_link_handle = 2; + /** + * The server-generated username link handle for the newly-confirmed username. + */ + bytes username_link_handle = 2; + } + + oneof response { + /** + * The details of the successfully confirmed username. + */ + ConfirmedUsernameHash confirmed_username_hash = 1; + + /** + * The provided hash was not reserved for the account. + */ + errors.FailedPrecondition reservation_not_found = 2; + + /** + * The reservation has lapsed and the requested username has been claimed by + * another caller. + */ + UsernameNotAvailable username_not_available = 3; + } } message DeleteUsernameHashRequest { @@ -245,11 +234,20 @@ message SetUsernameLinkRequest { bool keep_link_handle = 2; } + message SetUsernameLinkResponse { - /** - * A new link handle for the given username ciphertext. - */ - bytes username_link_handle = 1; + oneof response { + /** + * A new link handle for the given username ciphertext. + */ + bytes username_link_handle = 1; + + /** + * The authenticated account did not have a username set. + */ + errors.FailedPrecondition no_username_set = 2; + + } } message DeleteUsernameLinkRequest { @@ -323,10 +321,17 @@ message LookupUsernameHashRequest { } message LookupUsernameHashResponse { - /** - * The service identifier associated with a given username hash. - */ - common.ServiceIdentifier service_identifier = 1; + oneof response { + /** + * The service identifier associated with the provided username hash. + */ + common.ServiceIdentifier service_identifier = 1; + + /** + * No account was found for the provided username hash. + */ + errors.NotFound not_found = 2; + } } message LookupUsernameLinkRequest { @@ -338,8 +343,16 @@ message LookupUsernameLinkRequest { } message LookupUsernameLinkResponse { - /** - * The ciphertext of the username identified by the given link handle. - */ - bytes username_ciphertext = 1; + oneof response { + /** + * The ciphertext of the username identified by the provided link handle. + */ + bytes username_ciphertext = 1; + + + /** + * No username was found for the provided link handle. + */ + errors.NotFound not_found = 2; + } } diff --git a/service/src/main/proto/org/signal/chat/common.proto b/service/src/main/proto/org/signal/chat/common.proto index 5bad50990..3bf4d12d9 100644 --- a/service/src/main/proto/org/signal/chat/common.proto +++ b/service/src/main/proto/org/signal/chat/common.proto @@ -115,3 +115,4 @@ message ZkCredential { */ bytes credential = 2; } + diff --git a/service/src/main/proto/org/signal/chat/errors.proto b/service/src/main/proto/org/signal/chat/errors.proto new file mode 100644 index 000000000..b851dd18f --- /dev/null +++ b/service/src/main/proto/org/signal/chat/errors.proto @@ -0,0 +1,28 @@ +/* + * Copyright 2025 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +syntax = "proto3"; + +option java_multiple_files = true; + +package org.signal.chat.errors; + +/** + * Response message that indicates a particular resource was not found. + */ +message NotFound {} + +/** + * Response message that indicates that some precondition of the request was not + * met. For example, if there was a request to update foo, but foo had not been + * set, this would be an appropriate error. + */ + +message FailedPrecondition { + /** + * An optional description indicating what precondition failed. + */ + string description = 1; +} diff --git a/service/src/main/proto/org/signal/chat/tag.proto b/service/src/main/proto/org/signal/chat/tag.proto new file mode 100644 index 000000000..0697f5a9f --- /dev/null +++ b/service/src/main/proto/org/signal/chat/tag.proto @@ -0,0 +1,40 @@ +/* + * Copyright 2025 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +syntax = "proto3"; + +option java_multiple_files = true; + +package org.signal.chat.tag; + +import "google/protobuf/descriptor.proto"; + +extend google.protobuf.FieldOptions { + /** + * Indicate that a message which includes this field (directly or indirectly) + * was generated for a particular reason. + * + * ``` + * import "org/signal/chat/tag.proto" + * + * message LookupThingResponse { + * oneof response { + * string thing = 1; + * Error not_found = 2 [(tag.reason) = "not_found"]; + * Error forbidden = 3 [(tag.reason) = "forbidden"]; + * } + * } + * ``` + * + * Metrics middleware may then inspect `LookupThingResponse` and tag responses + * with the provided reason. This is useful when multiple outcomes are + * potentially represented with a status = "OK" RPC response. + * + * Valid messages should only have a single reason set. If a message has + * multiple fields present that have a reason option set, no guarantees are + * made about the reason that is selected. + */ + optional string reason = 71000; +} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/auth/grpc/ProhibitAuthenticationInterceptorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/auth/grpc/ProhibitAuthenticationInterceptorTest.java index 31ecfd1f8..372b3516a 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/auth/grpc/ProhibitAuthenticationInterceptorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/auth/grpc/ProhibitAuthenticationInterceptorTest.java @@ -53,7 +53,7 @@ class ProhibitAuthenticationInterceptorTest { final StatusRuntimeException e = assertThrows(StatusRuntimeException.class, () -> client.echo(EchoRequest.getDefaultInstance())); - assertEquals(Status.Code.UNAUTHENTICATED, e.getStatus().getCode()); + assertEquals(Status.Code.INVALID_ARGUMENT, e.getStatus().getCode()); } @Test diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/filters/RemoteDeprecationFilterTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/filters/RemoteDeprecationFilterTest.java index 54422b47f..cd645c87e 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/filters/RemoteDeprecationFilterTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/filters/RemoteDeprecationFilterTest.java @@ -5,6 +5,7 @@ package org.whispersystems.textsecuregcm.filters; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; @@ -17,12 +18,14 @@ import static org.mockito.Mockito.when; import com.google.common.net.HttpHeaders; import com.google.common.net.InetAddresses; import com.google.protobuf.ByteString; +import com.google.rpc.ErrorInfo; import com.vdurmont.semver4j.Semver; import io.grpc.ManagedChannel; import io.grpc.Server; import io.grpc.StatusRuntimeException; import io.grpc.inprocess.InProcessChannelBuilder; import io.grpc.inprocess.InProcessServerBuilder; +import io.grpc.protobuf.StatusProto; import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; @@ -40,9 +43,9 @@ import org.signal.chat.rpc.EchoServiceGrpc; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicRemoteDeprecationConfiguration; import org.whispersystems.textsecuregcm.grpc.EchoServiceImpl; +import org.whispersystems.textsecuregcm.grpc.GrpcExceptions; import org.whispersystems.textsecuregcm.grpc.MockRequestAttributesInterceptor; import org.whispersystems.textsecuregcm.grpc.RequestAttributes; -import org.whispersystems.textsecuregcm.grpc.StatusConstants; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.util.ua.ClientPlatform; @@ -153,7 +156,14 @@ class RemoteDeprecationFilterTest { final StatusRuntimeException e = assertThrows( StatusRuntimeException.class, () -> client.echo(req)); - assertEquals(StatusConstants.UPGRADE_NEEDED_STATUS.toString(), e.getStatus().toString()); + final com.google.rpc.Status status = StatusProto.fromThrowable(e); + final ErrorInfo errorInfo = assertDoesNotThrow(() -> status.getDetailsList().stream() + .filter(any -> any.is(ErrorInfo.class)).findFirst() + .orElseThrow(() -> new AssertionError("No error info found")) + .unpack(ErrorInfo.class)); + assertEquals(GrpcExceptions.DOMAIN, errorInfo.getDomain()); + assertEquals(io.grpc.Status.Code.INVALID_ARGUMENT.value(), status.getCode()); + assertEquals("UPGRADE_REQUIRED", errorInfo.getReason()); } else { assertEquals("cluck cluck, i'm a parrot", client.echo(req).getPayload().toStringUtf8()); } 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 12eeec8ba..cec9036f9 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsAnonymousGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsAnonymousGrpcServiceTest.java @@ -29,8 +29,11 @@ import org.mockito.Mock; import org.signal.chat.account.AccountsAnonymousGrpc; import org.signal.chat.account.CheckAccountExistenceRequest; 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.common.IdentityType; +import org.signal.chat.errors.NotFound; import org.signal.chat.common.ServiceIdentifier; import org.whispersystems.textsecuregcm.controllers.AccountController; import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException; @@ -151,8 +154,8 @@ class AccountsAnonymousGrpcServiceTest extends .getServiceIdentifier()); //noinspection ResultOfMethodCallIgnored - GrpcTestUtils.assertStatusException(Status.NOT_FOUND, - () -> unauthenticatedServiceStub().lookupUsernameHash(LookupUsernameHashRequest.newBuilder() + assertEquals(LookupUsernameHashResponse.newBuilder().setNotFound(NotFound.getDefaultInstance()).build(), + unauthenticatedServiceStub().lookupUsernameHash(LookupUsernameHashRequest.newBuilder() .setUsernameHash(ByteString.copyFrom(new byte[AccountController.USERNAME_HASH_LENGTH])) .build())); } @@ -217,15 +220,16 @@ class AccountsAnonymousGrpcServiceTest extends when(account.getEncryptedUsername()).thenReturn(Optional.empty()); - //noinspection ResultOfMethodCallIgnored - GrpcTestUtils.assertStatusException(Status.NOT_FOUND, - () -> unauthenticatedServiceStub().lookupUsernameLink(LookupUsernameLinkRequest.newBuilder() + final LookupUsernameLinkResponse notFoundResponse = LookupUsernameLinkResponse.newBuilder() + .setNotFound(NotFound.getDefaultInstance()) + .build(); + + assertEquals(notFoundResponse, + unauthenticatedServiceStub().lookupUsernameLink(LookupUsernameLinkRequest.newBuilder() .setUsernameLinkHandle(UUIDUtil.toByteString(linkHandle)) .build())); - - //noinspection ResultOfMethodCallIgnored - GrpcTestUtils.assertStatusException(Status.NOT_FOUND, - () -> unauthenticatedServiceStub().lookupUsernameLink(LookupUsernameLinkRequest.newBuilder() + assertEquals(notFoundResponse, + unauthenticatedServiceStub().lookupUsernameLink(LookupUsernameLinkRequest.newBuilder() .setUsernameLinkHandle(UUIDUtil.toByteString(UUID.randomUUID())) .build())); } 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 dda17ffc2..a3fd6393c 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java @@ -47,8 +47,6 @@ import org.signal.chat.account.DeleteUsernameHashRequest; import org.signal.chat.account.DeleteUsernameLinkRequest; import org.signal.chat.account.GetAccountIdentityRequest; import org.signal.chat.account.GetAccountIdentityResponse; -import org.signal.chat.account.ReserveUsernameHashError; -import org.signal.chat.account.ReserveUsernameHashErrorType; import org.signal.chat.account.ReserveUsernameHashRequest; import org.signal.chat.account.ReserveUsernameHashResponse; import org.signal.chat.account.SetDiscoverableByPhoneNumberRequest; @@ -57,7 +55,9 @@ import org.signal.chat.account.SetRegistrationLockResponse; import org.signal.chat.account.SetRegistrationRecoveryPasswordRequest; import org.signal.chat.account.SetUsernameLinkRequest; import org.signal.chat.account.SetUsernameLinkResponse; +import org.signal.chat.account.UsernameNotAvailable; import org.signal.chat.common.AccountIdentifiers; +import org.signal.chat.errors.FailedPrecondition; import org.signal.libsignal.usernames.BaseUsernameException; import org.whispersystems.textsecuregcm.auth.SaltedTokenHash; import org.whispersystems.textsecuregcm.auth.UnidentifiedAccessUtil; @@ -173,7 +173,7 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest authenticatedServiceStub().deleteAccount(DeleteAccountRequest.newBuilder().build())); verify(accountsManager, never()).delete(any(), any()); @@ -217,7 +217,7 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest authenticatedServiceStub().setRegistrationLock(SetRegistrationLockRequest.newBuilder() .build())); @@ -242,7 +242,7 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest authenticatedServiceStub().clearRegistrationLock(ClearRegistrationLockRequest.newBuilder().build())); verify(accountsManager, never()).updateAsync(any(), any()); @@ -288,9 +288,7 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest authenticatedServiceStub().confirmUsernameHash(ConfirmUsernameHashRequest.newBuilder() + final ConfirmUsernameHashResponse actualResponse = authenticatedServiceStub() + .confirmUsernameHash(ConfirmUsernameHashRequest.newBuilder() .setUsernameHash(ByteString.copyFrom(usernameHash)) .setUsernameCiphertext(ByteString.copyFrom(usernameCiphertext)) .setZkProof(ByteString.copyFrom(zkProof)) - .build())); + .build()); + + assertEquals(expectedResponse, actualResponse); } private static Stream confirmUsernameHashConfirmationException() { return Stream.of( - Arguments.of(new UsernameHashNotAvailableException(), Status.NOT_FOUND), - Arguments.of(new UsernameReservationNotFoundException(), Status.FAILED_PRECONDITION) + Arguments.of( new UsernameHashNotAvailableException(), + ConfirmUsernameHashResponse.newBuilder() + .setUsernameNotAvailable(UsernameNotAvailable.getDefaultInstance()) + .build()), + Arguments.of(new UsernameReservationNotFoundException(), + ConfirmUsernameHashResponse.newBuilder() + .setReservationNotFound(FailedPrecondition.getDefaultInstance()) + .build()) ); } @@ -546,9 +553,11 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest authenticatedServiceStub().setUsernameLink(SetUsernameLinkRequest.newBuilder() + assertEquals( + SetUsernameLinkResponse.newBuilder() + .setNoUsernameSet(FailedPrecondition.getDefaultInstance()) + .build(), + authenticatedServiceStub().setUsernameLink(SetUsernameLinkRequest.newBuilder() .setUsernameCiphertext(ByteString.copyFrom(usernameCiphertext)) .build())); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/BackupsAnonymousGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/BackupsAnonymousGrpcServiceTest.java index 1808ca185..549c36d3c 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/BackupsAnonymousGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/BackupsAnonymousGrpcServiceTest.java @@ -17,6 +17,7 @@ import com.google.protobuf.ByteString; import io.grpc.Status; import io.grpc.StatusRuntimeException; import java.time.Clock; +import java.time.Duration; import java.util.Arrays; import java.util.Base64; import java.util.Iterator; @@ -295,12 +296,10 @@ class BackupsAnonymousGrpcServiceTest extends assertThat(uploadForm.getSignedUploadLocation()).isEqualTo("example.org"); // rate limit + Duration duration = Duration.ofSeconds(10); when(backupManager.createTemporaryAttachmentUploadDescriptor(any())) - .thenReturn(CompletableFuture.failedFuture(new RateLimitExceededException(null))); - assertThatExceptionOfType(StatusRuntimeException.class) - .isThrownBy(() -> unauthenticatedServiceStub().getUploadForm(request)) - .extracting(StatusRuntimeException::getStatus) - .isEqualTo(Status.RESOURCE_EXHAUSTED); + .thenReturn(CompletableFuture.failedFuture(new RateLimitExceededException(duration))); + GrpcTestUtils.assertRateLimitExceeded(duration, () -> unauthenticatedServiceStub().getUploadForm(request)); } static Stream messagesUploadForm() { diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ErrorMappingInterceptorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ErrorMappingInterceptorTest.java new file mode 100644 index 000000000..a97525df4 --- /dev/null +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ErrorMappingInterceptorTest.java @@ -0,0 +1,157 @@ +/* + * Copyright 2025 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ +package org.whispersystems.textsecuregcm.grpc; + +import com.google.protobuf.Any; +import com.google.rpc.ErrorInfo; +import io.grpc.ManagedChannel; +import io.grpc.Server; +import io.grpc.Status; +import io.grpc.StatusRuntimeException; +import io.grpc.inprocess.InProcessChannelBuilder; +import io.grpc.inprocess.InProcessServerBuilder; +import io.grpc.protobuf.StatusProto; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.signal.chat.rpc.EchoRequest; +import org.signal.chat.rpc.EchoResponse; +import org.signal.chat.rpc.EchoServiceGrpc; +import org.signal.chat.rpc.ReactorEchoServiceGrpc; +import org.signal.chat.rpc.SimpleEchoServiceGrpc; +import reactor.core.publisher.Mono; + +class ErrorMappingInterceptorTest { + + private Server server; + private ManagedChannel channel; + + + @BeforeEach + void setUp() throws Exception { + channel = InProcessChannelBuilder.forName("ErrorMappingInterceptorTest") + .directExecutor() + .build(); + } + + @AfterEach + void tearDown() throws Exception { + server.shutdownNow(); + channel.shutdownNow(); + server.awaitTermination(1, TimeUnit.SECONDS); + channel.awaitTermination(1, TimeUnit.SECONDS); + } + + @Test + public void includeDetailsSimpleGrpc() throws Exception { + final StatusRuntimeException e = StatusProto.toStatusRuntimeException(com.google.rpc.Status.newBuilder() + .setCode(Status.Code.INVALID_ARGUMENT.value()) + .addDetails(Any.pack(ErrorInfo.newBuilder() + .setDomain("test") + .setReason("TEST") + .build())) + .build()); + + server = InProcessServerBuilder.forName("ErrorMappingInterceptorTest") + .directExecutor() + .addService(new SimpleEchoServiceErrorImpl(e)) + .intercept(new ErrorMappingInterceptor()) + .build() + .start(); + + final EchoServiceGrpc.EchoServiceBlockingStub client = EchoServiceGrpc.newBlockingStub(channel); + GrpcTestUtils.assertStatusException(Status.INVALID_ARGUMENT, "TEST", () -> + client.echo(EchoRequest.getDefaultInstance())); + } + + @Test + public void includeDetailsReactiveGrpc() throws Exception { + final StatusRuntimeException e = StatusProto.toStatusRuntimeException(com.google.rpc.Status.newBuilder() + .setCode(Status.Code.INVALID_ARGUMENT.value()) + .addDetails(Any.pack(ErrorInfo.newBuilder() + .setDomain("test") + .setReason("TEST") + .build())) + .build()); + + server = InProcessServerBuilder.forName("ErrorMappingInterceptorTest") + .directExecutor() + .addService(new ReactorEchoServiceErrorImpl(e)) + .intercept(new ErrorMappingInterceptor()) + .build() + .start(); + + final EchoServiceGrpc.EchoServiceBlockingStub client = EchoServiceGrpc.newBlockingStub(channel); + GrpcTestUtils.assertStatusException(Status.INVALID_ARGUMENT, "TEST", () -> + client.echo(EchoRequest.getDefaultInstance())); + } + + + @Test + public void mapIOExceptionsReactive() throws Exception { + server = InProcessServerBuilder.forName("ErrorMappingInterceptorTest") + .directExecutor() + .addService(new ReactorEchoServiceErrorImpl(new IOException("test"))) + .intercept(new ErrorMappingInterceptor()) + .build() + .start(); + + final EchoServiceGrpc.EchoServiceBlockingStub client = EchoServiceGrpc.newBlockingStub(channel); + GrpcTestUtils.assertStatusException(Status.UNAVAILABLE, "UNAVAILABLE", () -> + client.echo(EchoRequest.getDefaultInstance())); + } + + @Test + public void mapIOExceptionsSimple() throws Exception { + server = InProcessServerBuilder.forName("ErrorMappingInterceptorTest") + .directExecutor() + .addService(new SimpleEchoServiceErrorImpl(new UncheckedIOException(new IOException("test")))) + .intercept(new ErrorMappingInterceptor()) + .build() + .start(); + + final EchoServiceGrpc.EchoServiceBlockingStub client = EchoServiceGrpc.newBlockingStub(channel); + GrpcTestUtils.assertStatusException(Status.UNAVAILABLE, "UNAVAILABLE", () -> + client.echo(EchoRequest.getDefaultInstance())); + } + + + static class ReactorEchoServiceErrorImpl extends ReactorEchoServiceGrpc.EchoServiceImplBase { + + private final Exception exception; + + ReactorEchoServiceErrorImpl(final Exception exception) { + this.exception = exception; + } + + @Override + public Mono echo(final EchoRequest echoRequest) { + return Mono.error(exception); + } + + @Override + public Throwable onErrorMap(Throwable throwable) { + return new IllegalArgumentException(throwable); + } + } + + static class SimpleEchoServiceErrorImpl extends SimpleEchoServiceGrpc.EchoServiceImplBase { + + private final RuntimeException exception; + + SimpleEchoServiceErrorImpl(final RuntimeException exception) { + this.exception = exception; + } + + @Override + public EchoResponse echo(final EchoRequest echoRequest) { + throw exception; + } + + } +} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsGrpcServiceTest.java index e2498cb3e..98739be03 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ExternalServiceCredentialsGrpcServiceTest.java @@ -122,14 +122,6 @@ public class ExternalServiceCredentialsGrpcServiceTest ); } - @Test - public void testUnauthenticatedCall() throws Exception { - assertStatusUnauthenticated(() -> unauthenticatedServiceStub().getExternalServiceCredentials( - GetExternalServiceCredentialsRequest.newBuilder() - .setExternalService(ExternalServiceType.EXTERNAL_SERVICE_TYPE_DIRECTORY) - .build())); - } - /** * `ExternalServiceDefinitions` enum is supposed to have entries for all values in `ExternalServiceType`, * except for the `EXTERNAL_SERVICE_TYPE_UNSPECIFIED` and `UNRECOGNIZED`. diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/GrpcTestUtils.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/GrpcTestUtils.java index 65c3c97a3..8ebbe2f8a 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/GrpcTestUtils.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/GrpcTestUtils.java @@ -5,16 +5,24 @@ package org.whispersystems.textsecuregcm.grpc; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.mockito.Mockito.verifyNoInteractions; +import com.google.protobuf.Any; +import com.google.protobuf.Message; +import com.google.rpc.ErrorInfo; +import com.google.rpc.RetryInfo; import io.grpc.BindableService; import io.grpc.ServerInterceptors; import io.grpc.Status; +import io.grpc.StatusException; import io.grpc.StatusRuntimeException; import java.time.Duration; +import java.util.List; import java.util.UUID; +import io.grpc.protobuf.StatusProto; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.function.Executable; import org.whispersystems.textsecuregcm.auth.grpc.MockAuthenticationInterceptor; @@ -51,6 +59,12 @@ public final class GrpcTestUtils { assertEquals(expected.getCode(), exception.getStatus().getCode()); } + public static void assertStatusException(final Status expected, final String expectedReason, final Executable serviceCall) { + final StatusRuntimeException exception = Assertions.assertThrows(StatusRuntimeException.class, serviceCall); + assertEquals(expected.getCode(), exception.getStatus().getCode()); + assertEquals(expectedReason, extractErrorInfo(exception).getReason()); + } + public static void assertStatusInvalidArgument(final Executable serviceCall) { assertStatusException(Status.INVALID_ARGUMENT, serviceCall); } @@ -68,11 +82,31 @@ public final class GrpcTestUtils { final Executable serviceCall, final Object... mocksToCheckForNoInteraction) { final StatusRuntimeException exception = Assertions.assertThrows(StatusRuntimeException.class, serviceCall); - assertEquals(Status.RESOURCE_EXHAUSTED, exception.getStatus()); + assertEquals(Status.RESOURCE_EXHAUSTED.getCode(), exception.getStatus().getCode()); assertNotNull(exception.getTrailers()); - assertEquals(expectedRetryAfter, exception.getTrailers().get(RateLimitExceededException.RETRY_AFTER_DURATION_KEY)); + + final ErrorInfo errorInfo = extractErrorInfo(exception); + final RetryInfo retryInfo = extractDetail(RetryInfo.class, exception); + final Duration actual = Duration.ofSeconds(retryInfo.getRetryDelay().getSeconds(), retryInfo.getRetryDelay().getNanos()); + assertEquals(errorInfo.getDomain(), GrpcExceptions.DOMAIN); + assertEquals(errorInfo.getReason(), "RESOURCE_EXHAUSTED"); + assertEquals(expectedRetryAfter, actual); + for (final Object mock: mocksToCheckForNoInteraction) { verifyNoInteractions(mock); } } + + public static ErrorInfo extractErrorInfo(final StatusRuntimeException exception) { + return extractDetail(ErrorInfo.class, exception); + } + + public static T extractDetail(final Class detailCls, final StatusRuntimeException exception) { + final com.google.rpc.Status status = StatusProto.fromThrowable(exception); + + return assertDoesNotThrow(() -> status.getDetailsList().stream() + .filter(any -> any.is(detailCls)).findFirst() + .orElseThrow(() -> new AssertionError("No error info found")) + .unpack(detailCls)); + } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptorTest.java index 3281cbb59..faafc2412 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/MetricServerInterceptorTest.java @@ -10,12 +10,17 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import com.google.common.net.InetAddresses; +import com.google.protobuf.Any; import com.google.protobuf.ByteString; +import com.google.protobuf.Empty; +import com.google.rpc.ErrorInfo; import io.grpc.ManagedChannel; import io.grpc.Server; +import io.grpc.Status; import io.grpc.StatusException; import io.grpc.inprocess.InProcessChannelBuilder; import io.grpc.inprocess.InProcessServerBuilder; +import io.grpc.protobuf.StatusProto; import io.grpc.stub.BlockingClientCall; import io.micrometer.core.instrument.Counter; import io.micrometer.core.instrument.Meter; @@ -24,34 +29,48 @@ import io.micrometer.core.instrument.Tags; import io.micrometer.core.instrument.Timer; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import java.util.List; +import java.util.concurrent.Flow; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; +import java.util.function.Supplier; +import java.util.stream.Stream; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; +import org.junitpioneer.jupiter.cartesian.CartesianTest; import org.signal.chat.rpc.EchoRequest; import org.signal.chat.rpc.EchoResponse; import org.signal.chat.rpc.EchoServiceGrpc; +import org.signal.chat.rpc.SimpleTagTestServiceGrpc; +import org.signal.chat.rpc.TagResponse; +import org.signal.chat.rpc.TagTestServiceGrpc; import org.whispersystems.textsecuregcm.storage.ClientReleaseManager; import org.whispersystems.textsecuregcm.util.ua.UnrecognizedUserAgentException; import org.whispersystems.textsecuregcm.util.ua.UserAgent; import org.whispersystems.textsecuregcm.util.ua.UserAgentUtil; +import reactor.adapter.JdkFlowAdapter; +import reactor.core.publisher.Flux; +import reactor.core.scheduler.Schedulers; public class MetricServerInterceptorTest { - private static String USER_AGENT = "Signal-Android/4.53.7 (Android 8.1; libsignal)"; + private static final String USER_AGENT = "Signal-Android/4.53.7 (Android 8.1; libsignal)"; private Server server; private ManagedChannel channel; private SimpleMeterRegistry simpleMeterRegistry; private ClientReleaseManager clientReleaseManager; + private Supplier tagResponseSupplier; + @BeforeEach void setUp() throws Exception { simpleMeterRegistry = new SimpleMeterRegistry(); clientReleaseManager = mock(ClientReleaseManager.class); + tagResponseSupplier = mock(Supplier.class); final MockRequestAttributesInterceptor mockRequestAttributesInterceptor = new MockRequestAttributesInterceptor(); mockRequestAttributesInterceptor.setRequestAttributes( new RequestAttributes(InetAddresses.forString("127.0.0.1"), USER_AGENT, null)); @@ -59,9 +78,9 @@ public class MetricServerInterceptorTest { server = InProcessServerBuilder.forName("MetricServerInterceptorTest") .directExecutor() .addService(new EchoServiceImpl()) + .addService(new TagTestServiceImpl(tagResponseSupplier)) .intercept(new MetricServerInterceptor(simpleMeterRegistry, clientReleaseManager)) .intercept(mockRequestAttributesInterceptor) - .intercept(mockRequestAttributesInterceptor) .build() .start(); @@ -111,7 +130,7 @@ public class MetricServerInterceptorTest { } @Test - void streaming() throws StatusException, InterruptedException, TimeoutException { + void streaming() throws StatusException, InterruptedException { final EchoServiceGrpc.EchoServiceBlockingV2Stub client = EchoServiceGrpc.newBlockingV2Stub(channel); final BlockingClientCall echoStream = client.echoStream(); echoStream.write(EchoRequest.newBuilder().setPayload(ByteString.copyFromUtf8("1")).build()); @@ -152,6 +171,99 @@ public class MetricServerInterceptorTest { assertThat(expectedClientVersion).isEqualTo(actualClientVersion); } + static Stream testUnaryOkResponseReason() { + return Stream.of( + Arguments.argumentSet("Default reason", TagResponse.newBuilder().build(), "success"), + Arguments.argumentSet("No reason", TagResponse.newBuilder().setNoReason(true).build(), "success"), + Arguments.argumentSet("Explicitly set reason", TagResponse.newBuilder().setReason1(true).build(), "reason_1"), + Arguments.argumentSet("Nested reason", TagResponse.newBuilder().setNestedReason(TagResponse.NestedReason.newBuilder().setReason(true)).build(), "nested_reason")); + } + + @ParameterizedTest + @MethodSource + void testUnaryOkResponseReason(TagResponse response, String expectedReason) throws InterruptedException { + final TagTestServiceGrpc.TagTestServiceBlockingStub tagTestServiceBlockingStub = + TagTestServiceGrpc.newBlockingStub(channel); + when(tagResponseSupplier.get()).thenReturn(response); + tagTestServiceBlockingStub.tagEndpoint(Empty.getDefaultInstance()); + + final Counter rpcCount = find(Counter.class, MetricServerInterceptor.RPC_COUNTER_NAME); + assertThat(rpcCount.count()).isCloseTo(1.0, offset(0.01)); + assertThat(rpcCount.getId().getTag("statusCode")).isEqualTo("OK"); + assertThat(rpcCount.getId().getTag("reason")).isEqualTo(expectedReason); + } + + @Test + public void testConflictingReasons() { + final TagTestServiceGrpc.TagTestServiceBlockingStub tagTestServiceBlockingStub = + TagTestServiceGrpc.newBlockingStub(channel); + when(tagResponseSupplier.get()) + .thenReturn(TagResponse.newBuilder().setReason1(true).setConflictingReason(true).build()); + tagTestServiceBlockingStub.tagEndpoint(Empty.getDefaultInstance()); + + // We make no promises if proto fields that have reason tags are present on a message, but this tests for the sane + // behavior that at least one of these tags makes it into the metric. + assertThat(find(Counter.class, MetricServerInterceptor.RPC_COUNTER_NAME).getId().getTag("reason")) + .isIn("duplicate_reason", "reason_1"); + } + + @CartesianTest + public void testStatusErrorResponseReason( + @CartesianTest.Enum(mode = CartesianTest.Enum.Mode.EXCLUDE, names = {"OK"}) Status.Code statusCode, + @CartesianTest.Values(strings = {"test", "", "null"}) String reasonParam) { + + final String reason, expectedReasonTag; + if (reasonParam.equals("null")) { + reason = null; + expectedReasonTag = MetricServerInterceptor.DEFAULT_ERROR_REASON; + } else { + reason = reasonParam; + expectedReasonTag = reasonParam; + } + + final TagTestServiceGrpc.TagTestServiceBlockingStub tagTestServiceBlockingStub = + TagTestServiceGrpc.newBlockingStub(channel); + + final com.google.rpc.Status.Builder builder = com.google.rpc.Status.newBuilder() + .setCode(statusCode.value()) + .setMessage("test"); + if (reason != null) { + builder.addDetails(Any.pack(ErrorInfo.newBuilder() + .setDomain("domain") + .setReason(reason) + .build())); + } + + when(tagResponseSupplier.get()).thenThrow(StatusProto.toStatusRuntimeException(builder.build())); + + GrpcTestUtils.assertStatusException(statusCode.toStatus(), + () -> tagTestServiceBlockingStub.tagEndpoint(Empty.getDefaultInstance())); + + final Counter rpcCount = find(Counter.class, MetricServerInterceptor.RPC_COUNTER_NAME); + assertThat(rpcCount.count()).isCloseTo(1.0, offset(0.01)); + assertThat(rpcCount.getId().getTag("statusCode")).isEqualTo(statusCode.name()); + assertThat(rpcCount.getId().getTag("reason")).isEqualTo(expectedReasonTag); + } + + @Test + public void testStreamingResponseReason() { + final TagTestServiceGrpc.TagTestServiceBlockingStub tagTestServiceBlockingStub = + TagTestServiceGrpc.newBlockingStub(channel); + when(tagResponseSupplier.get()) + .thenReturn(TagResponse.newBuilder().setReason1(true).build()) + .thenReturn(TagResponse.newBuilder().setNoReason(true).build()) + .thenReturn(null); + + tagTestServiceBlockingStub.streamingTagEndpoint(Empty.getDefaultInstance()).forEachRemaining(_ -> {}); + final Counter messageCounter = find(Counter.class, MetricServerInterceptor.RESPONSE_COUNTER_NAME); + assertThat(messageCounter.count()).isCloseTo(2.0, offset(0.01)); + + final Counter rpcCount = find(Counter.class, MetricServerInterceptor.RPC_COUNTER_NAME); + assertThat(rpcCount.count()).isCloseTo(1.0, offset(0.01)); + assertThat(rpcCount.getId().getTag("statusCode")).isEqualTo("OK"); + assertThat(rpcCount.getId().getTag("reason")).isEqualTo(MetricServerInterceptor.DEFAULT_SUCCESS_REASON); + } + private T find(Class cls, final String name) { final Meter meter = simpleMeterRegistry.getMeters().stream() .filter(m -> m.getId().getName().equals(name)) @@ -162,4 +274,32 @@ public class MetricServerInterceptorTest { } throw new IllegalArgumentException("Meter " + name + " should be an instance of " + cls); } + + class TagTestServiceImpl extends SimpleTagTestServiceGrpc.TagTestServiceImplBase { + + private Supplier tagResponseSupplier; + TagTestServiceImpl(Supplier tagResponseSupplier) { + this.tagResponseSupplier = tagResponseSupplier; + } + + @Override + public TagResponse tagEndpoint(final Empty request) { + return tagResponseSupplier.get(); + } + + @Override + public Flow.Publisher streamingTagEndpoint(com.google.protobuf.Empty request) { + return JdkFlowAdapter.publisherToFlowPublisher(Flux.create(sink -> { + while (!sink.isCancelled()) { + TagResponse item = tagResponseSupplier.get(); + if (item == null) { + sink.complete(); + break; + } + sink.next(item); + } + }) + .subscribeOn(Schedulers.boundedElastic())); + } + } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/PaymentsGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/PaymentsGrpcServiceTest.java index 1c70d1ebb..8ea3db743 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/PaymentsGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/PaymentsGrpcServiceTest.java @@ -63,10 +63,4 @@ class PaymentsGrpcServiceTest extends SimpleBaseGrpcTest authenticatedServiceStub().getCurrencyConversions( GetCurrencyConversionsRequest.newBuilder().build())); } - - @Test - public void testUnauthenticated() throws Exception { - assertStatusException(Status.UNAUTHENTICATED, () -> unauthenticatedServiceStub().getCurrencyConversions( - GetCurrencyConversionsRequest.newBuilder().build())); - } } diff --git a/service/src/test/proto/tag_test.proto b/service/src/test/proto/tag_test.proto new file mode 100644 index 000000000..733162b4a --- /dev/null +++ b/service/src/test/proto/tag_test.proto @@ -0,0 +1,32 @@ +/* + * Copyright 2025 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +syntax = "proto3"; + +option java_multiple_files = true; + +package org.signal.chat.rpc; + +import "org/signal/chat/tag.proto"; +import "google/protobuf/empty.proto"; + +service TagTestService { + rpc TagEndpoint(google.protobuf.Empty) returns (TagResponse) {} + rpc StreamingTagEndpoint(google.protobuf.Empty) returns (stream TagResponse) {} +} + +message TagResponse { + oneof response { + bool no_reason = 1; + bool reason_1 = 2 [(tag.reason) = "reason_1"]; + } + + bool conflicting_reason = 4 [(tag.reason) = "duplicate_reason"]; + + message NestedReason { + bool reason = 1 [(tag.reason) = "nested_reason"]; + } + NestedReason nested_reason = 5; +}