From 9a1c450458bd5452595ad9dabef397ed8e5ae1d4 Mon Sep 17 00:00:00 2001 From: Chris Eager Date: Mon, 2 Feb 2026 14:05:07 -0600 Subject: [PATCH] Make VerificationSession.sessionId not-null --- .../controllers/VerificationController.java | 14 +++--- .../registration/VerificationSession.java | 3 +- .../storage/VerificationSessionManager.java | 8 ++-- .../VerificationControllerTest.java | 44 +++++++++---------- 4 files changed, 34 insertions(+), 35 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 1e2510796..dc625eefb 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/VerificationController.java @@ -254,7 +254,7 @@ public class VerificationController { // if a push challenge sent in `handlePushToken` doesn't arrive in time verificationSession.requestedInformation().add(VerificationSession.Information.CAPTCHA); - storeVerificationSession(registrationServiceSession, verificationSession); + storeVerificationSession(verificationSession); return buildResponse(registrationServiceSession, verificationSession); } @@ -327,22 +327,20 @@ public class VerificationController { } finally { // Each of the handle* methods may update requestedInformation, submittedInformation, and allowedToRequestCode, // and we want to be sure to store a changes, even if a later method throws - updateStoredVerificationSession(registrationServiceSession, verificationSession); + updateStoredVerificationSession(verificationSession); } return buildResponse(registrationServiceSession, verificationSession); } - private void storeVerificationSession(final RegistrationServiceSession registrationServiceSession, - final VerificationSession verificationSession) { - verificationSessionManager.insert(registrationServiceSession.encodedSessionId(), verificationSession) + private void storeVerificationSession(final VerificationSession verificationSession) { + verificationSessionManager.insert(verificationSession) .orTimeout(DYNAMODB_TIMEOUT.toSeconds(), TimeUnit.SECONDS) .join(); } - private void updateStoredVerificationSession(final RegistrationServiceSession registrationServiceSession, - final VerificationSession verificationSession) { - verificationSessionManager.update(registrationServiceSession.encodedSessionId(), verificationSession) + private void updateStoredVerificationSession(final VerificationSession verificationSession) { + verificationSessionManager.update(verificationSession) .orTimeout(DYNAMODB_TIMEOUT.toSeconds(), TimeUnit.SECONDS) .join(); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/registration/VerificationSession.java b/service/src/main/java/org/whispersystems/textsecuregcm/registration/VerificationSession.java index 799076fc0..fe15accea 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/registration/VerificationSession.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/registration/VerificationSession.java @@ -18,6 +18,7 @@ import org.whispersystems.textsecuregcm.telephony.CarrierData; * requesting codes from Registration Service, in order to get a verified session to be provided to * {@link org.whispersystems.textsecuregcm.controllers.RegistrationController}. * + * @param sessionId the session ID returned by Registration Service * @param pushChallenge the value of a push challenge sent to a client, after it submitted a push token * @param carrierData information about the phone number's carrier if available * @param requestedInformation information requested that a client send to the server @@ -36,7 +37,7 @@ import org.whispersystems.textsecuregcm.telephony.CarrierData; * @see org.whispersystems.textsecuregcm.entities.VerificationSessionResponse */ public record VerificationSession( - @Nullable String sessionId, + String sessionId, @Nullable String pushChallenge, @Nullable CarrierData carrierData, List requestedInformation, diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationSessionManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationSessionManager.java index ed9d29cf4..77a6528c7 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationSessionManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationSessionManager.java @@ -17,12 +17,12 @@ public class VerificationSessionManager { this.verificationSessions = verificationSessions; } - public CompletableFuture insert(final String encodedSessionId, final VerificationSession verificationSession) { - return verificationSessions.insert(encodedSessionId, verificationSession); + public CompletableFuture insert(final VerificationSession verificationSession) { + return verificationSessions.insert(verificationSession.sessionId(), verificationSession); } - public CompletableFuture update(final String encodedSessionId, final VerificationSession verificationSession) { - return verificationSessions.update(encodedSessionId, verificationSession); + public CompletableFuture update(final VerificationSession verificationSession) { + return verificationSessions.update(verificationSession.sessionId(), verificationSession); } public CompletableFuture> findForId(final String encodedSessionId) { 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 e2b1bcf82..415823134 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java @@ -231,7 +231,7 @@ class VerificationControllerTest { CompletableFuture.completedFuture( new RegistrationServiceSession(SESSION_ID, requestedNumber, false, null, null, null, SESSION_EXPIRATION_SECONDS))); - when(verificationSessionManager.insert(any(), any())) + when(verificationSessionManager.insert(any())) .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() @@ -272,7 +272,7 @@ class VerificationControllerTest { CompletableFuture.completedFuture( new RegistrationServiceSession(SESSION_ID, NUMBER, false, null, null, null, SESSION_EXPIRATION_SECONDS))); - when(verificationSessionManager.insert(any(), any())) + when(verificationSessionManager.insert(any())) .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() @@ -293,7 +293,7 @@ class VerificationControllerTest { CompletableFuture.completedFuture( new RegistrationServiceSession(SESSION_ID, NUMBER, false, null, null, null, SESSION_EXPIRATION_SECONDS))); - when(verificationSessionManager.insert(any(), any())) + when(verificationSessionManager.insert(any())) .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() @@ -328,7 +328,7 @@ class VerificationControllerTest { new RegistrationServiceSession(SESSION_ID, NUMBER, false, null, null, null, SESSION_EXPIRATION_SECONDS))); - when(verificationSessionManager.insert(any(), any())) + when(verificationSessionManager.insert(any())) .thenReturn(CompletableFuture.completedFuture(null)); when(accountsManager.getByE164(NUMBER)) @@ -396,7 +396,7 @@ class VerificationControllerTest { new VerificationSession(encodedSessionId, null, null, List.of(VerificationSession.Information.CAPTCHA), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), registrationServiceSession.expiration())))); - when(verificationSessionManager.update(any(), any())) + when(verificationSessionManager.update(any())) .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() @@ -408,7 +408,7 @@ class VerificationControllerTest { final ArgumentCaptor verificationSessionArgumentCaptor = ArgumentCaptor.forClass( VerificationSession.class); - verify(verificationSessionManager).update(any(), verificationSessionArgumentCaptor.capture()); + verify(verificationSessionManager).update(verificationSessionArgumentCaptor.capture()); final VerificationSession updatedSession = verificationSessionArgumentCaptor.getValue(); assertEquals(List.of(VerificationSession.Information.PUSH_CHALLENGE, VerificationSession.Information.CAPTCHA), @@ -440,7 +440,7 @@ class VerificationControllerTest { Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), registrationServiceSession.expiration())))); - when(verificationSessionManager.update(any(), any())) + when(verificationSessionManager.update(any())) .thenReturn(CompletableFuture.completedFuture(null)); doThrow(RateLimitExceededException.class) @@ -476,7 +476,7 @@ class VerificationControllerTest { Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), registrationServiceSession.expiration())))); - when(verificationSessionManager.update(any(), any())) + when(verificationSessionManager.update(any())) .thenReturn(CompletableFuture.completedFuture(null)); doThrow(RateLimitExceededException.class) @@ -512,7 +512,7 @@ class VerificationControllerTest { Optional.of(new VerificationSession(encodedSessionId, "challenge", null, List.of(VerificationSession.Information.PUSH_CHALLENGE), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), registrationServiceSession.expiration())))); - when(verificationSessionManager.update(any(), any())) + when(verificationSessionManager.update(any())) .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() @@ -550,7 +550,7 @@ class VerificationControllerTest { when(registrationCaptchaManager.assessCaptcha(any(), any(), any(), any())) .thenReturn(Optional.of(AssessmentResult.invalid())); - when(verificationSessionManager.update(any(), any())) + when(verificationSessionManager.update(any())) .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() @@ -563,7 +563,7 @@ class VerificationControllerTest { final ArgumentCaptor verificationSessionArgumentCaptor = ArgumentCaptor.forClass( VerificationSession.class); - verify(verificationSessionManager).update(any(), verificationSessionArgumentCaptor.capture()); + verify(verificationSessionManager).update(verificationSessionArgumentCaptor.capture()); final VerificationSession updatedSession = verificationSessionArgumentCaptor.getValue(); assertEquals(List.of(VerificationSession.Information.CAPTCHA), @@ -597,7 +597,7 @@ class VerificationControllerTest { List.of(VerificationSession.Information.PUSH_CHALLENGE), null, null, false, clock.millis(), clock.millis(), registrationServiceSession.expiration())))); - when(verificationSessionManager.update(any(), any())) + when(verificationSessionManager.update(any())) .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() @@ -610,7 +610,7 @@ class VerificationControllerTest { final ArgumentCaptor verificationSessionArgumentCaptor = ArgumentCaptor.forClass( VerificationSession.class); - verify(verificationSessionManager).update(any(), verificationSessionArgumentCaptor.capture()); + verify(verificationSessionManager).update(verificationSessionArgumentCaptor.capture()); final VerificationSession updatedSession = verificationSessionArgumentCaptor.getValue(); assertEquals(List.of(VerificationSession.Information.PUSH_CHALLENGE), @@ -640,7 +640,7 @@ class VerificationControllerTest { Optional.of(new VerificationSession(encodedSessionId, "challenge", null, List.of(), List.of(), null, null, true, clock.millis(), clock.millis(), registrationServiceSession.expiration())))); - when(verificationSessionManager.update(any(), any())) + when(verificationSessionManager.update(any())) .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() @@ -679,7 +679,7 @@ class VerificationControllerTest { List.of(VerificationSession.Information.PUSH_CHALLENGE, VerificationSession.Information.CAPTCHA), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), registrationServiceSession.expiration())))); - when(verificationSessionManager.update(any(), any())) + when(verificationSessionManager.update(any())) .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() @@ -692,7 +692,7 @@ class VerificationControllerTest { final ArgumentCaptor verificationSessionArgumentCaptor = ArgumentCaptor.forClass( VerificationSession.class); - verify(verificationSessionManager).update(any(), verificationSessionArgumentCaptor.capture()); + verify(verificationSessionManager).update(verificationSessionArgumentCaptor.capture()); final VerificationSession updatedSession = verificationSessionArgumentCaptor.getValue(); assertEquals(List.of(VerificationSession.Information.PUSH_CHALLENGE), @@ -726,7 +726,7 @@ class VerificationControllerTest { when(registrationCaptchaManager.assessCaptcha(any(), any(), any(), any())) .thenReturn(Optional.of(AssessmentResult.alwaysValid())); - when(verificationSessionManager.update(any(), any())) + when(verificationSessionManager.update(any())) .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() @@ -739,7 +739,7 @@ class VerificationControllerTest { final ArgumentCaptor verificationSessionArgumentCaptor = ArgumentCaptor.forClass( VerificationSession.class); - verify(verificationSessionManager).update(any(), verificationSessionArgumentCaptor.capture()); + verify(verificationSessionManager).update(verificationSessionArgumentCaptor.capture()); final VerificationSession updatedSession = verificationSessionArgumentCaptor.getValue(); assertEquals(List.of(VerificationSession.Information.CAPTCHA), @@ -776,7 +776,7 @@ class VerificationControllerTest { when(registrationCaptchaManager.assessCaptcha(any(), any(), any(), any())) .thenReturn(Optional.of(AssessmentResult.alwaysValid())); - when(verificationSessionManager.update(any(), any())) + when(verificationSessionManager.update(any())) .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() @@ -790,7 +790,7 @@ class VerificationControllerTest { final ArgumentCaptor verificationSessionArgumentCaptor = ArgumentCaptor.forClass( VerificationSession.class); - verify(verificationSessionManager).update(any(), verificationSessionArgumentCaptor.capture()); + verify(verificationSessionManager).update(verificationSessionArgumentCaptor.capture()); final VerificationSession updatedSession = verificationSessionArgumentCaptor.getValue(); assertEquals(List.of(VerificationSession.Information.PUSH_CHALLENGE, VerificationSession.Information.CAPTCHA), @@ -825,7 +825,7 @@ class VerificationControllerTest { when(registrationCaptchaManager.assessCaptcha(any(), any(), any(), any())) .thenThrow(new IOException("expected service error")); - when(verificationSessionManager.update(any(), any())) + when(verificationSessionManager.update(any())) .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() @@ -839,7 +839,7 @@ class VerificationControllerTest { final ArgumentCaptor verificationSessionArgumentCaptor = ArgumentCaptor.forClass( VerificationSession.class); - verify(verificationSessionManager).update(any(), verificationSessionArgumentCaptor.capture()); + verify(verificationSessionManager).update(verificationSessionArgumentCaptor.capture()); final VerificationSession updatedSession = verificationSessionArgumentCaptor.getValue(); assertTrue(updatedSession.submittedInformation().isEmpty());