diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java index 9b18ae18d..0f54d2a80 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -361,11 +361,7 @@ public class AccountController { final byte[] sessionId = maybeStoredVerificationCode.isPresent() && maybeStoredVerificationCode.get().sessionId() != null ? maybeStoredVerificationCode.get().sessionId() : createRegistrationSession(phoneNumber); - registrationServiceClient.sendRegistrationCode(sessionId, - messageTransport, - clientType, - acceptLanguage.orElse(null), - REGISTRATION_RPC_TIMEOUT).join(); + sendVerificationCode(sessionId, messageTransport, clientType, acceptLanguage); final StoredVerificationCode storedVerificationCode = new StoredVerificationCode(null, clock.millis(), @@ -405,10 +401,14 @@ public class AccountController { // Note that successful verification depends on being able to find a stored verification code for the given number. // We check that numbers are normalized before we store verification codes, and so don't need to re-assert // normalization here. - final boolean codeVerified = pendingAccounts.getCodeForNumber(number).map(storedVerificationCode -> - registrationServiceClient.checkVerificationCode(storedVerificationCode.sessionId(), - verificationCode, REGISTRATION_RPC_TIMEOUT).join()) - .orElse(false); + final boolean codeVerified; + final Optional maybeStoredVerificationCode = pendingAccounts.getCodeForNumber(number); + + if (maybeStoredVerificationCode.isPresent()) { + codeVerified = checkVerificationCode(maybeStoredVerificationCode.get().sessionId(), verificationCode); + } else { + codeVerified = false; + } if (!codeVerified) { throw new WebApplicationException(Response.status(403).build()); @@ -471,10 +471,14 @@ public class AccountController { rateLimiters.getVerifyLimiter().validate(number); - final boolean codeVerified = pendingAccounts.getCodeForNumber(number).map(storedVerificationCode -> - registrationServiceClient.checkVerificationCode(storedVerificationCode.sessionId(), - request.code(), REGISTRATION_RPC_TIMEOUT).join()) - .orElse(false); + final boolean codeVerified; + final Optional maybeStoredVerificationCode = pendingAccounts.getCodeForNumber(number); + + if (maybeStoredVerificationCode.isPresent()) { + codeVerified = checkVerificationCode(maybeStoredVerificationCode.get().sessionId(), request.code()); + } else { + codeVerified = false; + } if (!codeVerified) { throw new ForbiddenException(); @@ -964,17 +968,50 @@ public class AccountController { try { return registrationServiceClient.createRegistrationSession(phoneNumber, REGISTRATION_RPC_TIMEOUT).join(); } catch (final CompletionException e) { - Throwable cause = e; - - while (cause instanceof CompletionException) { - cause = cause.getCause(); - } - - if (cause instanceof RateLimitExceededException rateLimitExceededException) { - throw rateLimitExceededException; - } - + rethrowRateLimitException(e); throw e; } } + + private void sendVerificationCode(final byte[] sessionId, + final MessageTransport messageTransport, + final ClientType clientType, + final Optional acceptLanguage) throws RateLimitExceededException { + + try { + registrationServiceClient.sendRegistrationCode(sessionId, + messageTransport, + clientType, + acceptLanguage.orElse(null), + REGISTRATION_RPC_TIMEOUT).join(); + } catch (final CompletionException e) { + rethrowRateLimitException(e); + throw e; + } + } + + private boolean checkVerificationCode(final byte[] sessionId, final String verificationCode) + throws RateLimitExceededException { + + try { + return registrationServiceClient.checkVerificationCode(sessionId, verificationCode, REGISTRATION_RPC_TIMEOUT).join(); + } catch (final CompletionException e) { + rethrowRateLimitException(e); + throw e; + } + } + + private void rethrowRateLimitException(final CompletionException completionException) + throws RateLimitExceededException { + + Throwable cause = completionException; + + while (cause instanceof CompletionException) { + cause = cause.getCause(); + } + + if (cause instanceof RateLimitExceededException rateLimitExceededException) { + throw rateLimitExceededException; + } + } } 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 1dfb5d4ac..4dc5e10dc 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/registration/RegistrationServiceClient.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/registration/RegistrationServiceClient.java @@ -69,7 +69,8 @@ public class RegistrationServiceClient implements Managed { case ERROR -> { switch (response.getError().getErrorType()) { - case ERROR_TYPE_RATE_LIMITED -> throw new CompletionException(new RateLimitExceededException(Duration.ofSeconds(response.getError().getRetryAfterSeconds()))); + case CREATE_REGISTRATION_SESSION_ERROR_TYPE_RATE_LIMITED -> throw new CompletionException(new RateLimitExceededException(Duration.ofSeconds(response.getError().getRetryAfterSeconds()))); + case CREATE_REGISTRATION_SESSION_ERROR_TYPE_ILLEGAL_PHONE_NUMBER -> throw new IllegalArgumentException(); default -> throw new RuntimeException("Unrecognized error type from registration service: " + response.getError().getErrorType()); } } @@ -95,7 +96,18 @@ public class RegistrationServiceClient implements Managed { return toCompletableFuture(stub.withDeadline(toDeadline(timeout)) .sendVerificationCode(requestBuilder.build())) - .thenApply(response -> response.getSessionId().toByteArray()); + .thenApply(response -> { + if (response.hasError()) { + switch (response.getError().getErrorType()) { + case SEND_VERIFICATION_CODE_ERROR_TYPE_RATE_LIMITED -> + throw new CompletionException(new RateLimitExceededException(Duration.ofSeconds(response.getError().getRetryAfterSeconds()))); + + default -> throw new CompletionException(new RuntimeException("Failed to send verification code: " + response.getError().getErrorType())); + } + } else { + return response.getSessionId().toByteArray(); + } + }); } public CompletableFuture checkVerificationCode(final byte[] sessionId, @@ -107,7 +119,18 @@ public class RegistrationServiceClient implements Managed { .setSessionId(ByteString.copyFrom(sessionId)) .setVerificationCode(verificationCode) .build())) - .thenApply(CheckVerificationCodeResponse::getVerified); + .thenApply(response -> { + if (response.hasError()) { + switch (response.getError().getErrorType()) { + case CHECK_VERIFICATION_CODE_ERROR_TYPE_RATE_LIMITED -> + throw new CompletionException(new RateLimitExceededException(Duration.ofSeconds(response.getError().getRetryAfterSeconds()))); + + default -> throw new CompletionException(new RuntimeException("Failed to check verification code: " + response.getError().getErrorType())); + } + } else { + return response.getSessionMetadata().getVerified(); + } + }); } private static Deadline toDeadline(final Duration timeout) { diff --git a/service/src/main/proto/RegistrationService.proto b/service/src/main/proto/RegistrationService.proto index 5f19e9975..bcba2ee2f 100644 --- a/service/src/main/proto/RegistrationService.proto +++ b/service/src/main/proto/RegistrationService.proto @@ -50,6 +50,12 @@ message RegistrationSessionMetadata { * session associated with this registration attempt. */ bytes session_id = 1; + + /** + * Indicates whether a valid verification code has been submitted in the scope + * of this session. + */ + bool verified = 2; } message CreateRegistrationSessionError { @@ -59,37 +65,39 @@ message CreateRegistrationSessionError { CreateRegistrationSessionErrorType error_type = 1; /** - * Indicates that this error is fatal and should not be retried without - * modification. Non-fatal errors may be retried without modification after - * the duration indicated by `retry_after_seconds`. + * Indicates that this error may succeed if retried without modification after + * a delay indicated by `retry_after_seconds`. If false, callers should not + * retry the request without modification. */ - bool fatal = 2; + bool may_retry = 2; /** - * If this error is not fatal (see `fatal`), indicates the duration in seconds - * from the present after which the request may be retried without - * modification. This value has no meaning otherwise. + * If this error may be retried,, indicates the duration in seconds from the + * present after which the request may be retried without modification. This + * value has no meaning otherwise. */ uint64 retry_after_seconds = 3; } enum CreateRegistrationSessionErrorType { - ERROR_TYPE_UNSPECIFIED = 0; + CREATE_REGISTRATION_SESSION_ERROR_TYPE_UNSPECIFIED = 0; /** * Indicates that a session could not be created because too many requests to * create a session for the given phone number have been received in some * window of time. Callers should wait and try again later. */ - ERROR_TYPE_RATE_LIMITED = 1; + CREATE_REGISTRATION_SESSION_ERROR_TYPE_RATE_LIMITED = 1; + + /** + * Indicates that the provided phone number could not be parsed. + */ + CREATE_REGISTRATION_SESSION_ERROR_TYPE_ILLEGAL_PHONE_NUMBER = 2; } message SendVerificationCodeRequest { - /** - * The phone number to which to send a verification code. Ignored (and may be - * null if `session_id` is set. - */ - uint64 e164 = 1; + + reserved 1; /** * The message transport to use to send a verification code to the destination @@ -112,6 +120,12 @@ message SendVerificationCodeRequest { * The ID of a session within which to send (or re-send) a verification code. */ bytes session_id = 5; + + /** + * If provided, always attempt to use the specified sender to send + * this message. + */ + string sender_name = 6; } enum MessageTransport { @@ -133,6 +147,76 @@ message SendVerificationCodeResponse { * session associated with this registration attempt. */ bytes session_id = 1; + + /** + * Metadata for the named session. May be absent if the session could not be + * found or has expired. + */ + RegistrationSessionMetadata session_metadata = 2; + + /** + * If a code could not be sent, explains the underlying error. Will be absent + * if a code was sent successfully. Note that both an error and session + * metadata may be present in the same response because the session metadata + * may include information helpful for resolving the underlying error (i.e. + * "next attempt" times). + */ + SendVerificationCodeError error = 3; +} + +message SendVerificationCodeError { + /** + * The type of error that prevented a verification code from being sent. + */ + SendVerificationCodeErrorType error_type = 1; + + /** + * Indicates that this error may succeed if retried without modification after + * a delay indicated by `retry_after_seconds`. If false, callers should not + * retry the request without modification. + */ + bool may_retry = 2; + + /** + * If this error may be retried,, indicates the duration in seconds from the + * present after which the request may be retried without modification. This + * value has no meaning otherwise. + */ + uint64 retry_after_seconds = 3; +} + +enum SendVerificationCodeErrorType { + SEND_VERIFICATION_CODE_ERROR_TYPE_UNSPECIFIED = 0; + + /** + * The sender received and understood the request to send a verification code, + * but declined to do so (i.e. due to rate limits or suspected fraud). + */ + SEND_VERIFICATION_CODE_ERROR_TYPE_SENDER_REJECTED = 1; + + /** + * The sender could not process or would not accept some part of a request + * (e.g. a valid phone number that cannot receive SMS messages). + */ + SEND_VERIFICATION_CODE_ERROR_TYPE_SENDER_ILLEGAL_ARGUMENT = 2; + + /** + * A verification could could not be sent via the requested channel due to + * timing/rate restrictions. The response object containing this error should + * include session metadata that indicates when the next attempt is allowed. + */ + SEND_VERIFICATION_CODE_ERROR_TYPE_RATE_LIMITED = 3; + + /** + * No session was found with the given ID. + */ + SEND_VERIFICATION_CODE_ERROR_TYPE_SESSION_NOT_FOUND = 4; + + /** + * A new verification could could not be sent because the session has already + * been verified. + */ + SEND_VERIFICATION_CODE_ERROR_TYPE_SESSION_ALREADY_VERIFIED = 5; } message CheckVerificationCodeRequest { @@ -153,4 +237,70 @@ message CheckVerificationCodeResponse { * matched the expected code or false otherwise. */ bool verified = 1; + + /** + * Metadata for the named session. May be absent if the session could not be + * found or has expired. + */ + RegistrationSessionMetadata session_metadata = 2; + + /** + * If a code could not be checked, explains the underlying error. Will be + * absent if no error occurred. Note that both an error and session + * metadata may be present in the same response because the session metadata + * may include information helpful for resolving the underlying error (i.e. + * "next attempt" times). + */ + CheckVerificationCodeError error = 3; +} + +message CheckVerificationCodeError { + /** + * The type of error that prevented a verification code from being checked. + */ + CheckVerificationCodeErrorType error_type = 1; + + /** + * Indicates that this error may succeed if retried without modification after + * a delay indicated by `retry_after_seconds`. If false, callers should not + * retry the request without modification. + */ + bool may_retry = 2; + + /** + * If this error may be retried,, indicates the duration in seconds from the + * present after which the request may be retried without modification. This + * value has no meaning otherwise. + */ + uint64 retry_after_seconds = 3; +} + +enum CheckVerificationCodeErrorType { + CHECK_VERIFICATION_CODE_ERROR_TYPE_UNSPECIFIED = 0; + + /** + * The caller has made too many incorrect guesses within the scope of this + * session and may not make any further guesses. + */ + CHECK_VERIFICATION_CODE_ERROR_TYPE_ATTEMPTS_EXHAUSTED = 1; + + /** + * The caller has attempted to submit a verification code even though no + * verification codes have been sent within the scope of this session. The + * caller must issue a "send code" request before trying again. + */ + CHECK_VERIFICATION_CODE_ERROR_TYPE_NO_CODE_SENT = 2; + + /** + * The caller has made too many guesses within some period of time. Callers + * should wait for the duration prescribed in the session metadata object + * elsewhere in the response before trying again. + */ + CHECK_VERIFICATION_CODE_ERROR_TYPE_RATE_LIMITED = 3; + + /** + * The session identified in this request could not be found (possibly due to + * session expiration). + */ + CHECK_VERIFICATION_CODE_ERROR_TYPE_SESSION_NOT_FOUND = 4; }