From 2c0fc8fe3e0a67457d421694fd2bd42439dff6dd Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Wed, 5 Jul 2023 10:37:53 -0400 Subject: [PATCH] Remove legacy methods from `RegistrationServiceClient` --- .../controllers/VerificationController.java | 4 +- .../RegistrationServiceClient.java | 55 +------------------ .../src/main/proto/RegistrationService.proto | 16 ------ .../VerificationControllerTest.java | 20 +++---- 4 files changed, 14 insertions(+), 81 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java index 087a2b594..fb9572075 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java @@ -157,7 +157,7 @@ public class VerificationController { final RegistrationServiceSession registrationServiceSession; try { - registrationServiceSession = registrationServiceClient.createRegistrationSessionSession(phoneNumber, + registrationServiceSession = registrationServiceClient.createRegistrationSession(phoneNumber, accountsManager.getByE164(request.getNumber()).isPresent(), REGISTRATION_RPC_TIMEOUT).join(); } catch (final CancellationException e) { @@ -541,7 +541,7 @@ public class VerificationController { final RegistrationServiceSession resultSession; try { - resultSession = registrationServiceClient.checkVerificationCodeSession(registrationServiceSession.id(), + resultSession = registrationServiceClient.checkVerificationCode(registrationServiceSession.id(), submitVerificationCodeRequest.code(), REGISTRATION_RPC_TIMEOUT) .join(); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/registration/RegistrationServiceClient.java b/service/src/main/java/org/whispersystems/textsecuregcm/registration/RegistrationServiceClient.java index 5e83c7caf..fe1eb73bf 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/registration/RegistrationServiceClient.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/registration/RegistrationServiceClient.java @@ -78,9 +78,7 @@ public class RegistrationServiceClient implements Managed { this.callbackExecutor = callbackExecutor; } - // The …Session suffix methods distinguish the new methods, which return Sessions, from the old. - // Once the deprecated methods are removed, the names can be streamlined. - public CompletableFuture createRegistrationSessionSession( + public CompletableFuture createRegistrationSession( final Phonenumber.PhoneNumber phoneNumber, final boolean accountExistsWithPhoneNumber, final Duration timeout) { final long e164 = Long.parseLong( PhoneNumberUtil.getInstance().format(phoneNumber, PhoneNumberUtil.PhoneNumberFormat.E164).substring(1)); @@ -110,13 +108,6 @@ public class RegistrationServiceClient implements Managed { }); } - @Deprecated - public CompletableFuture createRegistrationSession(final Phonenumber.PhoneNumber phoneNumber, - final boolean accountExistsWithPhoneNumber, final Duration timeout) { - return createRegistrationSessionSession(phoneNumber, accountExistsWithPhoneNumber, timeout) - .thenApply(RegistrationServiceSession::id); - } - public CompletableFuture sendVerificationCode(final byte[] sessionId, final MessageTransport messageTransport, final ClientType clientType, @@ -169,49 +160,7 @@ public class RegistrationServiceClient implements Managed { }); } - @Deprecated - public CompletableFuture sendRegistrationCode(final byte[] sessionId, - final MessageTransport messageTransport, - final ClientType clientType, - @Nullable final String acceptLanguage, - final Duration timeout) { - return sendVerificationCode(sessionId, messageTransport, clientType, acceptLanguage, timeout) - .thenApply(RegistrationServiceSession::id); - } - - @Deprecated - public CompletableFuture checkVerificationCode(final byte[] sessionId, - final String verificationCode, - final Duration timeout) { - - return toCompletableFuture(stub.withDeadline(toDeadline(timeout)) - .legacyCheckVerificationCode(CheckVerificationCodeRequest.newBuilder() - .setSessionId(ByteString.copyFrom(sessionId)) - .setVerificationCode(verificationCode) - .build())) - .thenApply(response -> { - if (response.hasError()) { - switch (response.getError().getErrorType()) { - case CHECK_VERIFICATION_CODE_ERROR_TYPE_RATE_LIMITED -> - throw new CompletionException(new RateLimitExceededException(response.getError().getMayRetry() - ? Duration.ofSeconds(response.getError().getRetryAfterSeconds()) - : null, true)); - - case CHECK_VERIFICATION_CODE_ERROR_TYPE_NO_CODE_SENT, - CHECK_VERIFICATION_CODE_ERROR_TYPE_ATTEMPT_EXPIRED, - CHECK_VERIFICATION_CODE_ERROR_TYPE_SESSION_NOT_FOUND -> - throw new CompletionException(new RegistrationServiceException(null)); - - default -> throw new CompletionException( - new RuntimeException("Failed to check verification code: " + response.getError().getErrorType())); - } - } else { - return response.getVerified(); - } - }); - } - - public CompletableFuture checkVerificationCodeSession(final byte[] sessionId, + public CompletableFuture checkVerificationCode(final byte[] sessionId, final String verificationCode, final Duration timeout) { return toCompletableFuture(stub.withDeadline(toDeadline(timeout)) diff --git a/service/src/main/proto/RegistrationService.proto b/service/src/main/proto/RegistrationService.proto index 75c6ac3ad..551f8ea78 100644 --- a/service/src/main/proto/RegistrationService.proto +++ b/service/src/main/proto/RegistrationService.proto @@ -26,8 +26,6 @@ service RegistrationService { * session. */ rpc check_verification_code (CheckVerificationCodeRequest) returns (CheckVerificationCodeResponse) {} - - rpc legacy_check_verification_code (CheckVerificationCodeRequest) returns (LegacyCheckVerificationCodeResponse) {} } message CreateRegistrationSessionRequest { @@ -358,20 +356,6 @@ message CheckVerificationCodeResponse { CheckVerificationCodeError error = 3; } -message LegacyCheckVerificationCodeResponse { - /** - * Indicates whether the verification code given in the request that produced - * this response was correct. - */ - bool verified = 1; - - /** - * If a code could not be checked, explains the underlying error. Will be - * absent if no error occurred. - */ - CheckVerificationCodeError error = 2; -} - message CheckVerificationCodeError { /** * The type of error that prevented a verification code from being checked. diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java index ad8fe0838..dfb50292a 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java @@ -168,7 +168,7 @@ class VerificationControllerTest { @Test void createSessionRateLimited() { - when(registrationServiceClient.createRegistrationSessionSession(any(), anyBoolean(), any())) + when(registrationServiceClient.createRegistrationSession(any(), anyBoolean(), any())) .thenReturn(CompletableFuture.failedFuture(new RateLimitExceededException(null, true))); final Invocation.Builder request = resources.getJerseyTest() @@ -182,7 +182,7 @@ class VerificationControllerTest { @Test void createSessionRegistrationServiceError() { - when(registrationServiceClient.createRegistrationSessionSession(any(), anyBoolean(), any())) + when(registrationServiceClient.createRegistrationSession(any(), anyBoolean(), any())) .thenReturn(CompletableFuture.failedFuture(new RuntimeException("expected service error"))); final Invocation.Builder request = resources.getJerseyTest() @@ -198,7 +198,7 @@ class VerificationControllerTest { @MethodSource void createSessionSuccess(final String pushToken, final String pushTokenType, final List expectedRequestedInformation) { - when(registrationServiceClient.createRegistrationSessionSession(any(), anyBoolean(), any())) + when(registrationServiceClient.createRegistrationSession(any(), anyBoolean(), any())) .thenReturn( CompletableFuture.completedFuture( new RegistrationServiceSession(SESSION_ID, NUMBER, false, null, null, null, @@ -232,7 +232,7 @@ class VerificationControllerTest { @ParameterizedTest @ValueSource(booleans = {true, false}) void createSessionReregistration(final boolean isReregistration) throws NumberParseException { - when(registrationServiceClient.createRegistrationSessionSession(any(), anyBoolean(), any())) + when(registrationServiceClient.createRegistrationSession(any(), anyBoolean(), any())) .thenReturn( CompletableFuture.completedFuture( new RegistrationServiceSession(SESSION_ID, NUMBER, false, null, null, null, @@ -252,7 +252,7 @@ class VerificationControllerTest { try (final Response response = request.post(Entity.json(createSessionJson(NUMBER, null, null)))) { assertEquals(HttpStatus.SC_OK, response.getStatus()); - verify(registrationServiceClient).createRegistrationSessionSession( + verify(registrationServiceClient).createRegistrationSession( eq(PhoneNumberUtil.getInstance().parse(NUMBER, null)), eq(isReregistration), any() @@ -1102,7 +1102,7 @@ class VerificationControllerTest { Optional.of(new VerificationSession(null, Collections.emptyList(), Collections.emptyList(), true, clock.millis(), clock.millis(), registrationServiceSession.expiration())))); - when(registrationServiceClient.checkVerificationCodeSession(any(), any(), any())) + when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.failedFuture(new CompletionException(new RuntimeException()))); final Invocation.Builder request = resources.getJerseyTest() @@ -1166,7 +1166,7 @@ class VerificationControllerTest { // There is no explicit indication in the exception that no code has been sent, but we treat all RegistrationServiceExceptions // in which the response has a session object as conflicted state - when(registrationServiceClient.checkVerificationCodeSession(any(), any(), any())) + when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.failedFuture(new CompletionException( new RegistrationServiceException(new RegistrationServiceSession(SESSION_ID, NUMBER, false, 0L, null, null, SESSION_EXPIRATION_SECONDS))))); @@ -1201,7 +1201,7 @@ class VerificationControllerTest { Optional.of(new VerificationSession(null, Collections.emptyList(), Collections.emptyList(), true, clock.millis(), clock.millis(), registrationServiceSession.expiration())))); - when(registrationServiceClient.checkVerificationCodeSession(any(), any(), any())) + when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.failedFuture(new CompletionException(new RegistrationServiceException(null)))); final Invocation.Builder request = resources.getJerseyTest() @@ -1226,7 +1226,7 @@ class VerificationControllerTest { .thenReturn(CompletableFuture.completedFuture( Optional.of(new VerificationSession(null, Collections.emptyList(), Collections.emptyList(), true, clock.millis(), clock.millis(), registrationServiceSession.expiration())))); - when(registrationServiceClient.checkVerificationCodeSession(any(), any(), any())) + when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.failedFuture( new CompletionException(new VerificationSessionRateLimitExceededException(registrationServiceSession, Duration.ofMinutes(1), true)))); @@ -1262,7 +1262,7 @@ class VerificationControllerTest { final RegistrationServiceSession verifiedSession = new RegistrationServiceSession(SESSION_ID, NUMBER, true, null, null, 0L, SESSION_EXPIRATION_SECONDS); - when(registrationServiceClient.checkVerificationCodeSession(any(), any(), any())) + when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.completedFuture(verifiedSession)); final Invocation.Builder request = resources.getJerseyTest()