From c6126634905dc3cba923751a6b87019614534a05 Mon Sep 17 00:00:00 2001 From: Jon Chambers <63609320+jon-signal@users.noreply.github.com> Date: Fri, 14 Jan 2022 14:47:46 -0500 Subject: [PATCH] Handle `null` `AccountAttributes` when verifying linked devices --- .../controllers/DeviceController.java | 9 ++++--- .../controllers/DeviceControllerTest.java | 25 +++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java index 0d4f69036..77c7b3cdf 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java @@ -13,6 +13,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import javax.validation.Valid; +import javax.validation.constraints.NotNull; import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; import javax.ws.rs.GET; @@ -152,7 +153,7 @@ public class DeviceController { public DeviceResponse verifyDeviceToken(@PathParam("verification_code") String verificationCode, @HeaderParam("Authorization") BasicAuthorizationHeader authorizationHeader, @HeaderParam("User-Agent") String userAgent, - @Valid AccountAttributes accountAttributes, + @NotNull @Valid AccountAttributes accountAttributes, @Context ContainerRequest containerRequest) throws RateLimitExceededException, DeviceLimitExceededException { @@ -164,17 +165,17 @@ public class DeviceController { Optional storedVerificationCode = pendingDevices.getCodeForNumber(number); - if (!storedVerificationCode.isPresent() || !storedVerificationCode.get().isValid(verificationCode)) { + if (storedVerificationCode.isEmpty() || !storedVerificationCode.get().isValid(verificationCode)) { throw new WebApplicationException(Response.status(403).build()); } Optional account = accounts.getByE164(number); - if (!account.isPresent()) { + if (account.isEmpty()) { throw new WebApplicationException(Response.status(403).build()); } - // Normally, the the "do we need to refresh somebody's websockets" listener can do this on its own. In this case, + // Normally, the "do we need to refresh somebody's websockets" listener can do this on its own. In this case, // we're not using the conventional authentication system, and so we need to give it a hint so it knows who the // active user is and what their device states look like. AuthEnablementRefreshRequirementProvider.setAccount(containerRequest, account.get()); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java index 84d0876ca..cce33dfa6 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DeviceControllerTest.java @@ -186,6 +186,31 @@ class DeviceControllerTest { verify(clientPresenceManager).disconnectPresence(AuthHelper.VALID_UUID, Device.MASTER_ID); } + @Test + void verifyDeviceWithNullAccountAttributes() { + when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)).thenReturn(Optional.of(AuthHelper.VALID_ACCOUNT)); + + final Device existingDevice = mock(Device.class); + when(existingDevice.getId()).thenReturn(Device.MASTER_ID); + when(AuthHelper.VALID_ACCOUNT.getDevices()).thenReturn(Set.of(existingDevice)); + + VerificationCode deviceCode = resources.getJerseyTest() + .target("/v1/devices/provisioning/code") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) + .get(VerificationCode.class); + + assertThat(deviceCode).isEqualTo(new VerificationCode(5678901)); + + final Response response = resources.getJerseyTest() + .target("/v1/devices/5678901") + .request() + .header("Authorization", AuthHelper.getProvisioningAuthHeader(AuthHelper.VALID_NUMBER, "password1")) + .put(Entity.json("")); + + assertThat(response.getStatus()).isNotEqualTo(500); + } + @Test void verifyDeviceTokenBadCredentials() { final Response response = resources.getJerseyTest()