diff --git a/src/main/java/org/signal/storageservice/controllers/GroupsController.java b/src/main/java/org/signal/storageservice/controllers/GroupsController.java index 3b5da9e..3013dc8 100644 --- a/src/main/java/org/signal/storageservice/controllers/GroupsController.java +++ b/src/main/java/org/signal/storageservice/controllers/GroupsController.java @@ -478,7 +478,12 @@ public CompletableFuture modifyGroup( return CompletableFuture.completedFuture(Response.status(Response.Status.CONFLICT).entity(group.get()).build()); } + if (!submittedActions.getGroupId().isEmpty()) { + throw new BadRequestException("requested actions must not set group id"); + } + Actions actions = submittedActions.toBuilder() + .setGroupId(user.getGroupId()) .clearAddMembers() .addAllAddMembers(groupValidator.validateAddMember(user, inviteLinkPassword, group.get(), submittedActions.getAddMembersList())) .clearAddMembersPendingProfileKey() diff --git a/src/main/proto/Groups.proto b/src/main/proto/Groups.proto index f5f68e0..34191b5 100644 --- a/src/main/proto/Groups.proto +++ b/src/main/proto/Groups.proto @@ -204,6 +204,9 @@ message GroupChange { } bytes sourceUuid = 1; + // clients should not provide this value; the server will provide it in the response buffer to ensure the signature is binding to a particular group + // if clients set it during a request the server will respond with 400. + bytes group_id = 25; uint32 version = 2; repeated AddMemberAction addMembers = 3; @@ -228,7 +231,7 @@ message GroupChange { repeated AddMemberBannedAction add_members_banned = 22; // change epoch = 4 repeated DeleteMemberBannedAction delete_members_banned = 23; // change epoch = 4 repeated PromoteMemberPendingPniAciProfileKeyAction promote_members_pending_pni_aci_profile_key = 24; // change epoch = 5 - // next: 25 + // next: 26 } bytes actions = 1; diff --git a/src/test/java/org/signal/storageservice/controllers/GroupsControllerBannedMembersTest.java b/src/test/java/org/signal/storageservice/controllers/GroupsControllerBannedMembersTest.java index 0ff0423..6a0ac0c 100644 --- a/src/test/java/org/signal/storageservice/controllers/GroupsControllerBannedMembersTest.java +++ b/src/test/java/org/signal/storageservice/controllers/GroupsControllerBannedMembersTest.java @@ -139,6 +139,7 @@ public void testModifyGroupBanMember() throws Exception { setupGroupsManagerForWrites(); Response response = modifyGroup(AuthHelper.VALID_USER_AUTH_CREDENTIAL, actionsBuilder); + actionsBuilder.setGroupId(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); actionsBuilder.getAddMembersBannedBuilder(0).getAddedBuilder().setTimestamp(clock.millis()); groupBuilder.setVersion(1).addMembersBannedBuilder().setUserId(validUserThreeId).setTimestamp(clock.millis()); assertThat(response.getStatus()).isEqualTo(200); @@ -174,6 +175,7 @@ public void testModifyGroupUnbanMember() throws Exception { setupGroupsManagerForWrites(); Response response = modifyGroup(AuthHelper.VALID_USER_AUTH_CREDENTIAL, actionsBuilder); + actionsBuilder.setGroupId(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); groupBuilder.setVersion(1).clearMembersBanned(); assertThat(response.getStatus()).isEqualTo(200); verifyGroupModification(groupBuilder, actionsBuilder, 4, response, validUserId); @@ -210,6 +212,7 @@ public void testModifyGroupUnbanMemberAsNonAdminWithOpenGroup() throws Exception setupGroupsManagerForWrites(); Response response = modifyGroup(AuthHelper.VALID_USER_TWO_AUTH_CREDENTIAL, actionsBuilder); + actionsBuilder.setGroupId(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); groupBuilder.setVersion(1).clearMembersBanned(); assertThat(response.getStatus()).isEqualTo(200); verifyGroupModification(groupBuilder, actionsBuilder, 4, response, validUserTwoId); diff --git a/src/test/java/org/signal/storageservice/controllers/GroupsControllerInviteLinkTest.java b/src/test/java/org/signal/storageservice/controllers/GroupsControllerInviteLinkTest.java index ab5edae..b392e3f 100644 --- a/src/test/java/org/signal/storageservice/controllers/GroupsControllerInviteLinkTest.java +++ b/src/test/java/org/signal/storageservice/controllers/GroupsControllerInviteLinkTest.java @@ -84,9 +84,10 @@ void testModifyAddFromInviteLinkAccessControl() throws Exception { .isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(GroupChange.Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(GroupChange.Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(GroupChange.Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(GroupChange.Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(GroupChange.Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); AuthHelper.GROUPS_SERVER_KEY.getPublicParams().verifySignature(signedChange.getActions().toByteArray(), new NotarySignature(signedChange.getServerSignature().toByteArray())); diff --git a/src/test/java/org/signal/storageservice/controllers/GroupsControllerPhoneNumberPrivacyTest.java b/src/test/java/org/signal/storageservice/controllers/GroupsControllerPhoneNumberPrivacyTest.java index e45729d..86696b2 100644 --- a/src/test/java/org/signal/storageservice/controllers/GroupsControllerPhoneNumberPrivacyTest.java +++ b/src/test/java/org/signal/storageservice/controllers/GroupsControllerPhoneNumberPrivacyTest.java @@ -2,6 +2,7 @@ import javax.ws.rs.client.Entity; import javax.ws.rs.core.Response; +import com.google.protobuf.ByteString; import org.junit.jupiter.api.Test; import org.signal.storageservice.providers.ProtocolBufferMediaType; import org.signal.storageservice.storage.protos.groups.Group; @@ -41,6 +42,7 @@ void testRejectInvitationFromPni() throws Exception { .header("Authorization", AuthHelper.getAuthHeader(groupSecretParams, AuthHelper.VALID_USER_THREE_AUTH_CREDENTIAL)) .method("PATCH", Entity.entity(actionsBuilder.build().toByteArray(), ProtocolBufferMediaType.APPLICATION_PROTOBUF))) { + actionsBuilder.setGroupId(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); verifyGroupModification(groupBuilder, actionsBuilder, 0, response, validUserThreePniId); } } diff --git a/src/test/java/org/signal/storageservice/controllers/GroupsControllerTest.java b/src/test/java/org/signal/storageservice/controllers/GroupsControllerTest.java index fb234c0..d8156e7 100644 --- a/src/test/java/org/signal/storageservice/controllers/GroupsControllerTest.java +++ b/src/test/java/org/signal/storageservice/controllers/GroupsControllerTest.java @@ -954,9 +954,10 @@ void testModifyGroupTitle(final Instant issueTime, final Instant lastValidTime) .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); AuthHelper.GROUPS_SERVER_KEY.getPublicParams().verifySignature(signedChange.getActions().toByteArray(), new NotarySignature(signedChange.getServerSignature().toByteArray())); @@ -1198,9 +1199,10 @@ void testModifyGroupDescription(final Instant issueTime, final Instant lastValid assertThat(signedChange).isEqualTo(changeCaptor.getValue()); assertThat(signedChange.getChangeEpoch()).isEqualTo(2); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); AuthHelper.GROUPS_SERVER_KEY.getPublicParams().verifySignature(signedChange.getActions().toByteArray(), new NotarySignature(signedChange.getServerSignature().toByteArray())); @@ -1292,9 +1294,10 @@ void testModifyGroupAnnouncementsOnly(final Instant issueTime, final Instant las assertThat(signedChange).isEqualTo(changeCaptor.getValue()); assertThat(signedChange.getChangeEpoch()).isEqualTo(3); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); AuthHelper.GROUPS_SERVER_KEY.getPublicParams().verifySignature(signedChange.getActions().toByteArray(), new NotarySignature(signedChange.getServerSignature().toByteArray())); @@ -1384,9 +1387,10 @@ void testModifyGroupAvatarAndTitle(final Instant issueTime, final Instant lastVa .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(2); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); AuthHelper.GROUPS_SERVER_KEY.getPublicParams().verifySignature(signedChange.getActions().toByteArray(), new NotarySignature(signedChange.getServerSignature().toByteArray())); @@ -1470,9 +1474,10 @@ void testModifyGroupTimer(final Instant issueTime, final Instant lastValidTime) .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); AuthHelper.GROUPS_SERVER_KEY.getPublicParams().verifySignature(signedChange.getActions().toByteArray(), new NotarySignature(signedChange.getServerSignature().toByteArray())); @@ -1530,6 +1535,58 @@ void testModifyGroupTimerUnauthorized() { verifyNoMoreInteractions(groupsManager); } + @Test + void testModifyGroupWithGroupId() { + GroupSecretParams groupSecretParams = GroupSecretParams.generate(); + GroupPublicParams groupPublicParams = groupSecretParams.getPublicParams(); + + ProfileKeyCredentialPresentation validUserPresentation = new ClientZkProfileOperations(AuthHelper.GROUPS_SERVER_KEY.getPublicParams()).createProfileKeyCredentialPresentation(groupSecretParams, AuthHelper.VALID_USER_PROFILE_CREDENTIAL ); + ProfileKeyCredentialPresentation validUserTwoPresentation = new ClientZkProfileOperations(AuthHelper.GROUPS_SERVER_KEY.getPublicParams()).createProfileKeyCredentialPresentation(groupSecretParams, AuthHelper.VALID_USER_TWO_PROFILE_CREDENTIAL); + + Group group = Group.newBuilder() + .setPublicKey(ByteString.copyFrom(groupPublicParams.serialize())) + .setAccessControl(AccessControl.newBuilder() + .setMembers(AccessControl.AccessRequired.MEMBER) + .setAttributes(AccessControl.AccessRequired.MEMBER)) + .setTitle(ByteString.copyFromUtf8("Some title")) + .setAvatar(avatarFor(groupPublicParams.getGroupIdentifier().serialize())) + .setVersion(0) + .addMembers(Member.newBuilder() + .setUserId(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())) + .setProfileKey(ByteString.copyFrom(validUserPresentation.getProfileKeyCiphertext().serialize())) + .setRole(Member.Role.ADMINISTRATOR) + .build()) + .addMembers(Member.newBuilder() + .setUserId(ByteString.copyFrom(validUserTwoPresentation.getUuidCiphertext().serialize())) + .setProfileKey(ByteString.copyFrom(validUserTwoPresentation.getProfileKeyCiphertext().serialize())) + .setRole(Member.Role.DEFAULT) + .build()) + .build(); + + + when(groupsManager.getGroup(eq(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())))) + .thenReturn(CompletableFuture.completedFuture(Optional.of(group))); + + GroupChange.Actions groupChange = Actions.newBuilder() + .setVersion(1) + .setModifyTitle(ModifyTitleAction.newBuilder() + .setTitle(ByteString.copyFromUtf8("Another title"))) + .setGroupId(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())) + .build(); + + try (Response response = resources.getJerseyTest() + .target("/v2/groups/") + .request(ProtocolBufferMediaType.APPLICATION_PROTOBUF) + .header("Authorization", AuthHelper.getAuthHeader(groupSecretParams, AuthHelper.VALID_USER_AUTH_CREDENTIAL)) + .method("PATCH", Entity.entity(groupChange.toByteArray(), ProtocolBufferMediaType.APPLICATION_PROTOBUF))) { + + assertThat(response.getStatus()).isEqualTo(400); + } + + verify(groupsManager).getGroup(eq(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize()))); + verifyNoMoreInteractions(groupsManager); + } + @ParameterizedTest @MethodSource("sendCredentialTimes") void testDeleteMember(final Instant issueTime, final Instant lastValidTime) throws Exception { @@ -1610,9 +1667,10 @@ void testDeleteMember(final Instant issueTime, final Instant lastValidTime) thro .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); assertValidSendEndorsements( AuthHelper.VALID_USER, List.of(), groupSecretParams, responseProto.getGroupSendEndorsementsResponse(), issueTime, lastValidTime); @@ -1747,10 +1805,11 @@ void testAddMember(final Instant issueTime, final Instant lastValidTime) throws .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearVersion().clearSourceUuid().build()) + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearVersion().clearSourceUuid().build()) .isEqualTo(Actions.newBuilder().addAddMembers(Actions.AddMemberAction.newBuilder().setAdded(Member.newBuilder().setUserId(ByteString.copyFrom(validUserTwoPresentation.getUuidCiphertext().serialize())) .setProfileKey(ByteString.copyFrom(validUserTwoPresentation.getProfileKeyCiphertext().serialize())) .setRole(Member.Role.DEFAULT) @@ -1902,10 +1961,11 @@ void testAddMemberWhoIsAlreadyPendingProfileKey(final Instant issueTime, final I .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearVersion().clearSourceUuid().build()) + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearVersion().clearSourceUuid().build()) .isEqualTo(Actions.newBuilder().addAddMembers(Actions.AddMemberAction.newBuilder().setAdded(Member.newBuilder().setUserId(ByteString.copyFrom(validUserTwoPresentation.getUuidCiphertext().serialize())) .setProfileKey(ByteString.copyFrom(validUserTwoPresentation.getProfileKeyCiphertext().serialize())) .setRole(Member.Role.DEFAULT) @@ -2002,10 +2062,11 @@ void testAddMemberWhoIsAlreadyPendingAdminApproval(final Instant issueTime, fina .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearVersion().clearSourceUuid().build()) + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearVersion().clearSourceUuid().build()) .isEqualTo(Actions.newBuilder().addAddMembers(Actions.AddMemberAction.newBuilder().setAdded(Member.newBuilder().setUserId(ByteString.copyFrom(validUserTwoPresentation.getUuidCiphertext().serialize())) .setProfileKey(ByteString.copyFrom(validUserTwoPresentation.getProfileKeyCiphertext().serialize())) .setRole(Member.Role.DEFAULT) @@ -2211,6 +2272,7 @@ void testModifyMemberPresentation(final Instant issueTime, final Instant lastVal .build(); GroupChange.Actions.Builder expectedGroupChangeResponseBuilder = groupChange.toBuilder() + .setGroupId(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())) .setSourceUuid(ByteString.copyFrom(validUserTwoPresentation.getUuidCiphertext().serialize())); expectedGroupChangeResponseBuilder.getModifyMemberProfileKeysBuilder(0) .setUserId(ByteString.copyFrom(validUserTwoPresentationUpdate.getUuidCiphertext().serialize())) @@ -2247,6 +2309,7 @@ void testModifyMemberPresentation(final Instant issueTime, final Instant lastVal .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserTwoPresentation.getUuidCiphertext().serialize())); assertThat(Actions.parseFrom(signedChange.getActions())).isEqualTo(expectedGroupChangeResponseBuilder.build()); @@ -3151,9 +3214,10 @@ void testModifyMembersRole(final Instant issueTime, final Instant lastValidTime) .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); assertValidSendEndorsements( AuthHelper.VALID_USER, List.of(AuthHelper.VALID_USER_TWO), groupSecretParams, responseProto.getGroupSendEndorsementsResponse(), issueTime, lastValidTime); @@ -3295,9 +3359,10 @@ void testModifyMemberRole(final Instant issueTime, final Instant lastValidTime) .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); assertValidSendEndorsements( AuthHelper.VALID_USER, List.of(AuthHelper.VALID_USER_TWO), groupSecretParams, responseProto.getGroupSendEndorsementsResponse(), issueTime, lastValidTime); @@ -4004,10 +4069,11 @@ void testLastAdminLeavesGroup() throws Exception { assertThat(signedChange).as("check returned change matches the saved change").isEqualTo(changeCaptor.getValue()); Actions resultActions = Actions.parseFrom(signedChange.getActions()); + assertThat(resultActions.getGroupId()).as("check group id").isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(resultActions.getVersion()).as("check change version").isEqualTo(1); assertThat(resultActions.getSourceUuid()).as("check source of the change is correct").isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(resultActions.toBuilder().clearSourceUuid().build()).as("check that the actions were modified from the input").isNotEqualTo(actions); - assertThat(resultActions.toBuilder().clearSourceUuid().clearModifyMemberRoles().build()).as("check that the actions were modified by adding modify member roles").isEqualTo(actions); + assertThat(resultActions.toBuilder().clearGroupId().clearSourceUuid().build()).as("check that the actions were modified from the input").isNotEqualTo(actions); + assertThat(resultActions.toBuilder().clearGroupId().clearSourceUuid().clearModifyMemberRoles().build()).as("check that the actions were modified by adding modify member roles").isEqualTo(actions); assertThat(resultActions.getModifyMemberRolesCount()).as("check only one modify member role action").isEqualTo(1); assertThat(resultActions.getModifyMemberRoles(0).getRole()).as("check setting the remaining member to admin").isEqualTo(Role.ADMINISTRATOR); assertThat(resultActions.getModifyMemberRoles(0).getUserId()).as("check user id promoted to admin was the remaining group member").isEqualTo(group.getMembers(1).getUserId()); diff --git a/src/test/java/org/signal/storageservice/controllers/GroupsV1ControllerTest.java b/src/test/java/org/signal/storageservice/controllers/GroupsV1ControllerTest.java index 2c721e6..2a7c139 100644 --- a/src/test/java/org/signal/storageservice/controllers/GroupsV1ControllerTest.java +++ b/src/test/java/org/signal/storageservice/controllers/GroupsV1ControllerTest.java @@ -867,9 +867,10 @@ void testModifyGroupTitle() throws Exception { .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); AuthHelper.GROUPS_SERVER_KEY.getPublicParams().verifySignature(signedChange.getActions().toByteArray(), new NotarySignature(signedChange.getServerSignature().toByteArray())); @@ -1106,9 +1107,10 @@ void testModifyGroupDescription() throws Exception { assertThat(signedChange).isEqualTo(changeCaptor.getValue()); assertThat(signedChange.getChangeEpoch()).isEqualTo(2); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); AuthHelper.GROUPS_SERVER_KEY.getPublicParams().verifySignature(signedChange.getActions().toByteArray(), new NotarySignature(signedChange.getServerSignature().toByteArray())); @@ -1195,9 +1197,10 @@ void testModifyGroupAnnouncementsOnly() throws Exception { assertThat(signedChange).isEqualTo(changeCaptor.getValue()); assertThat(signedChange.getChangeEpoch()).isEqualTo(3); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); AuthHelper.GROUPS_SERVER_KEY.getPublicParams().verifySignature(signedChange.getActions().toByteArray(), new NotarySignature(signedChange.getServerSignature().toByteArray())); @@ -1282,9 +1285,10 @@ void testModifyGroupAvatarAndTitle() throws Exception { .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(2); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); AuthHelper.GROUPS_SERVER_KEY.getPublicParams().verifySignature(signedChange.getActions().toByteArray(), new NotarySignature(signedChange.getServerSignature().toByteArray())); @@ -1363,9 +1367,10 @@ void testModifyGroupTimer() throws Exception { .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); AuthHelper.GROUPS_SERVER_KEY.getPublicParams().verifySignature(signedChange.getActions().toByteArray(), new NotarySignature(signedChange.getServerSignature().toByteArray())); @@ -1498,9 +1503,10 @@ void testDeleteMember() throws Exception { .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); AuthHelper.GROUPS_SERVER_KEY.getPublicParams().verifySignature(signedChange.getActions().toByteArray(), new NotarySignature(signedChange.getServerSignature().toByteArray())); @@ -1630,10 +1636,11 @@ void testAddMember() throws Exception { .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearVersion().clearSourceUuid().build()) + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearVersion().clearGroupId().clearSourceUuid().build()) .isEqualTo(Actions.newBuilder().addAddMembers(Actions.AddMemberAction.newBuilder().setAdded(Member.newBuilder().setUserId(ByteString.copyFrom(validUserTwoPresentation.getUuidCiphertext().serialize())) .setProfileKey(ByteString.copyFrom(validUserTwoPresentation.getProfileKeyCiphertext().serialize())) .setRole(Member.Role.DEFAULT) @@ -1777,10 +1784,11 @@ void testAddMemberWhoIsAlreadyPendingProfileKey() throws Exception { .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearVersion().clearSourceUuid().build()) + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearVersion().clearSourceUuid().build()) .isEqualTo(Actions.newBuilder().addAddMembers(Actions.AddMemberAction.newBuilder().setAdded(Member.newBuilder().setUserId(ByteString.copyFrom(validUserTwoPresentation.getUuidCiphertext().serialize())) .setProfileKey(ByteString.copyFrom(validUserTwoPresentation.getProfileKeyCiphertext().serialize())) .setRole(Member.Role.DEFAULT) @@ -1872,10 +1880,11 @@ void testAddMemberWhoIsAlreadyPendingAdminApproval() throws Exception { .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearVersion().clearSourceUuid().build()) + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearVersion().clearSourceUuid().build()) .isEqualTo(Actions.newBuilder().addAddMembers(Actions.AddMemberAction.newBuilder().setAdded(Member.newBuilder().setUserId(ByteString.copyFrom(validUserTwoPresentation.getUuidCiphertext().serialize())) .setProfileKey(ByteString.copyFrom(validUserTwoPresentation.getProfileKeyCiphertext().serialize())) .setRole(Member.Role.DEFAULT) @@ -2078,6 +2087,7 @@ void testModifyMemberPresentation() throws Exception { .build(); GroupChange.Actions.Builder expectedGroupChangeResponseBuilder = groupChange.toBuilder() + .setGroupId(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())) .setSourceUuid(ByteString.copyFrom(validUserTwoPresentation.getUuidCiphertext().serialize())); expectedGroupChangeResponseBuilder.getModifyMemberProfileKeysBuilder(0) .setUserId(ByteString.copyFrom(validUserTwoPresentationUpdate.getUuidCiphertext().serialize())) @@ -2112,6 +2122,7 @@ void testModifyMemberPresentation() throws Exception { .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserTwoPresentation.getUuidCiphertext().serialize())); assertThat(Actions.parseFrom(signedChange.getActions())).isEqualTo(expectedGroupChangeResponseBuilder.build()); @@ -2991,9 +3002,10 @@ void testModifyMembersRole() throws Exception { .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); AuthHelper.GROUPS_SERVER_KEY.getPublicParams().verifySignature(signedChange.getActions().toByteArray(), new NotarySignature(signedChange.getServerSignature().toByteArray())); @@ -3130,9 +3142,10 @@ void testModifyMemberRole() throws Exception { .build()).isEqualTo(group); assertThat(signedChange).isEqualTo(changeCaptor.getValue()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(Actions.parseFrom(signedChange.getActions()).getVersion()).isEqualTo(1); assertThat(Actions.parseFrom(signedChange.getActions()).getSourceUuid()).isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearSourceUuid().build()).isEqualTo(groupChange); + assertThat(Actions.parseFrom(signedChange.getActions()).toBuilder().clearGroupId().clearSourceUuid().build()).isEqualTo(groupChange); AuthHelper.GROUPS_SERVER_KEY.getPublicParams().verifySignature(signedChange.getActions().toByteArray(), new NotarySignature(signedChange.getServerSignature().toByteArray())); @@ -3821,10 +3834,11 @@ void testLastAdminLeavesGroup() throws Exception { assertThat(signedChange).as("check returned change matches the saved change").isEqualTo(changeCaptor.getValue()); Actions resultActions = Actions.parseFrom(signedChange.getActions()); + assertThat(Actions.parseFrom(signedChange.getActions()).getGroupId()).as("check change group id").isEqualTo(ByteString.copyFrom(groupPublicParams.getGroupIdentifier().serialize())); assertThat(resultActions.getVersion()).as("check change version").isEqualTo(1); assertThat(resultActions.getSourceUuid()).as("check source of the change is correct").isEqualTo(ByteString.copyFrom(validUserPresentation.getUuidCiphertext().serialize())); - assertThat(resultActions.toBuilder().clearSourceUuid().build()).as("check that the actions were modified from the input").isNotEqualTo(actions); - assertThat(resultActions.toBuilder().clearSourceUuid().clearModifyMemberRoles().build()).as("check that the actions were modified by adding modify member roles").isEqualTo(actions); + assertThat(resultActions.toBuilder().clearGroupId().clearSourceUuid().build()).as("check that the actions were modified from the input").isNotEqualTo(actions); + assertThat(resultActions.toBuilder().clearGroupId().clearSourceUuid().clearModifyMemberRoles().build()).as("check that the actions were modified by adding modify member roles").isEqualTo(actions); assertThat(resultActions.getModifyMemberRolesCount()).as("check only one modify member role action").isEqualTo(1); assertThat(resultActions.getModifyMemberRoles(0).getRole()).as("check setting the remaining member to admin").isEqualTo(Role.ADMINISTRATOR); assertThat(resultActions.getModifyMemberRoles(0).getUserId()).as("check user id promoted to admin was the remaining group member").isEqualTo(group.getMembers(1).getUserId());