Fix various corner case block/reject join request bugs.
This commit is contained in:
parent
b8e98350c1
commit
e6ac40a07c
8 changed files with 83 additions and 26 deletions
|
@ -1934,6 +1934,8 @@ public class ConversationFragment extends LoggingFragment implements Multiselect
|
|||
if (result.isFailure()) {
|
||||
int failureReason = GroupErrors.getUserDisplayMessage(((GroupBlockJoinRequestResult.Failure) result).getReason());
|
||||
Toast.makeText(requireContext(), failureReason, Toast.LENGTH_SHORT).show();
|
||||
} else {
|
||||
Toast.makeText(requireContext(), R.string.ConversationFragment__blocked, Toast.LENGTH_SHORT).show();
|
||||
}
|
||||
})
|
||||
);
|
||||
|
|
|
@ -17,6 +17,7 @@ import androidx.core.content.ContextCompat;
|
|||
import androidx.lifecycle.LifecycleOwner;
|
||||
import androidx.lifecycle.LiveData;
|
||||
import androidx.lifecycle.Observer;
|
||||
import androidx.lifecycle.Transformations;
|
||||
|
||||
import com.google.android.material.button.MaterialButton;
|
||||
import com.google.common.collect.Sets;
|
||||
|
@ -57,7 +58,9 @@ import java.util.Locale;
|
|||
import java.util.Objects;
|
||||
import java.util.Optional;
|
||||
import java.util.Set;
|
||||
import java.util.UUID;
|
||||
import java.util.concurrent.ExecutionException;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
public final class ConversationUpdateItem extends FrameLayout
|
||||
implements BindableConversationItem
|
||||
|
@ -85,7 +88,7 @@ public final class ConversationUpdateItem extends FrameLayout
|
|||
private final PresentOnChange presentOnChange = new PresentOnChange();
|
||||
private final RecipientObserverManager senderObserver = new RecipientObserverManager(presentOnChange);
|
||||
private final RecipientObserverManager groupObserver = new RecipientObserverManager(presentOnChange);
|
||||
private final GroupDataManager groupData = new GroupDataManager(presentOnChange);
|
||||
private final GroupDataManager groupData = new GroupDataManager();
|
||||
|
||||
public ConversationUpdateItem(Context context) {
|
||||
super(context);
|
||||
|
@ -272,42 +275,68 @@ public final class ConversationUpdateItem extends FrameLayout
|
|||
|
||||
private final class GroupDataManager {
|
||||
|
||||
private final Observer<Recipient> recipientObserver;
|
||||
private final Observer<Boolean> isSelfAdminSetter;
|
||||
private final Observer<Object> updater;
|
||||
|
||||
private LiveGroup liveGroup;
|
||||
private LiveData<Boolean> liveIsSelfAdmin;
|
||||
private boolean isSelfAdmin;
|
||||
private LiveData<Set<UUID>> liveBannedMembers;
|
||||
private LiveData<Set<UUID>> liveFullMembers;
|
||||
private Recipient conversationRecipient;
|
||||
|
||||
GroupDataManager(@NonNull Observer<Recipient> observer) {
|
||||
this.recipientObserver = observer;
|
||||
this.isSelfAdminSetter = isSelfAdmin -> {
|
||||
this.isSelfAdmin = isSelfAdmin;
|
||||
present(conversationMessage, nextMessageRecord, conversationRecipient, isMessageRequestAccepted);
|
||||
};
|
||||
GroupDataManager() {
|
||||
this.updater = unused -> update();
|
||||
}
|
||||
|
||||
public void observe(@NonNull LifecycleOwner lifecycleOwner, @Nullable Recipient recipient) {
|
||||
if (liveGroup != null) {
|
||||
liveIsSelfAdmin.removeObserver(isSelfAdminSetter);
|
||||
liveIsSelfAdmin = null;
|
||||
liveIsSelfAdmin.removeObserver(updater);
|
||||
liveBannedMembers.removeObserver(updater);
|
||||
liveFullMembers.removeObserver(updater);
|
||||
}
|
||||
|
||||
if (recipient != null) {
|
||||
conversationRecipient = recipient;
|
||||
liveGroup = new LiveGroup(recipient.requireGroupId());
|
||||
liveIsSelfAdmin = liveGroup.isSelfAdmin();
|
||||
liveBannedMembers = liveGroup.getBannedMembers();
|
||||
liveFullMembers = Transformations.map(liveGroup.getFullMembers(),
|
||||
members -> members.stream()
|
||||
.map(m -> m.getMember().requireServiceId().uuid())
|
||||
.collect(Collectors.toSet()));
|
||||
|
||||
liveIsSelfAdmin.observe(lifecycleOwner, isSelfAdminSetter);
|
||||
liveIsSelfAdmin.observe(lifecycleOwner, updater);
|
||||
liveBannedMembers.observe(lifecycleOwner, updater);
|
||||
liveFullMembers.observe(lifecycleOwner, updater);
|
||||
} else {
|
||||
conversationRecipient = null;
|
||||
liveGroup = null;
|
||||
liveBannedMembers = null;
|
||||
liveIsSelfAdmin = null;
|
||||
}
|
||||
}
|
||||
|
||||
public boolean isSelfAdmin() {
|
||||
return isSelfAdmin;
|
||||
return liveIsSelfAdmin.getValue() != null ? liveIsSelfAdmin.getValue() : false;
|
||||
}
|
||||
|
||||
public boolean isBanned(Recipient recipient) {
|
||||
Set<UUID> bannedMembers = liveBannedMembers.getValue();
|
||||
if (bannedMembers != null) {
|
||||
return recipient.getServiceId().isPresent() && bannedMembers.contains(recipient.requireServiceId().uuid());
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
public boolean isFullMember(Recipient recipient) {
|
||||
Set<UUID> members = liveFullMembers.getValue();
|
||||
if (members != null) {
|
||||
return recipient.getServiceId().isPresent() && members.contains(recipient.requireServiceId().uuid());
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private void update() {
|
||||
present(conversationMessage, nextMessageRecord, conversationRecipient, isMessageRequestAccepted);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -469,7 +498,10 @@ public final class ConversationUpdateItem extends FrameLayout
|
|||
eventListener.onChangeNumberUpdateContact(conversationMessage.getMessageRecord().getIndividualRecipient());
|
||||
}
|
||||
});
|
||||
} else if (conversationMessage.getMessageRecord().isCollapsedGroupV2JoinUpdate() && groupData.isSelfAdmin()) {
|
||||
} else if (conversationMessage.getMessageRecord().isCollapsedGroupV2JoinUpdate() &&
|
||||
groupData.isSelfAdmin() &&
|
||||
!groupData.isBanned(conversationMessage.getMessageRecord().getIndividualRecipient()) &&
|
||||
!groupData.isFullMember(conversationMessage.getMessageRecord().getIndividualRecipient())) {
|
||||
actionButton.setText(R.string.ConversationUpdateItem_block_request);
|
||||
actionButton.setVisibility(VISIBLE);
|
||||
actionButton.setOnClickListener(v -> {
|
||||
|
|
|
@ -6,6 +6,8 @@ import androidx.annotation.NonNull;
|
|||
import androidx.annotation.Nullable;
|
||||
import androidx.annotation.WorkerThread;
|
||||
|
||||
import com.google.protobuf.ByteString;
|
||||
|
||||
import org.signal.core.util.logging.Log;
|
||||
import org.signal.storageservice.protos.groups.GroupExternalCredential;
|
||||
import org.signal.storageservice.protos.groups.local.DecryptedGroup;
|
||||
|
@ -22,6 +24,7 @@ import org.thoughtcrime.securesms.profiles.AvatarHelper;
|
|||
import org.thoughtcrime.securesms.recipients.Recipient;
|
||||
import org.thoughtcrime.securesms.recipients.RecipientId;
|
||||
import org.whispersystems.signalservice.api.groupsv2.GroupLinkNotActiveException;
|
||||
import org.whispersystems.signalservice.api.util.UuidUtil;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Collection;
|
||||
|
@ -280,8 +283,19 @@ public final class GroupManager {
|
|||
@NonNull RecipientId recipientId)
|
||||
throws GroupChangeBusyException, IOException, GroupChangeFailedException, GroupNotAMemberException, GroupInsufficientRightsException
|
||||
{
|
||||
GroupDatabase.GroupRecord groupRecord = SignalDatabase.groups().requireGroup(groupId);
|
||||
Recipient recipient = Recipient.resolved(recipientId);
|
||||
|
||||
if (groupRecord.requireV2GroupProperties().getBannedMembers().contains(recipient.requireServiceId().uuid())) {
|
||||
Log.i(TAG, "Attempt to ban already banned recipient: " + recipientId);
|
||||
return;
|
||||
}
|
||||
|
||||
ByteString uuid = UuidUtil.toByteString(recipient.requireServiceId().uuid());
|
||||
boolean rejectJoinRequest = groupRecord.requireV2GroupProperties().getDecryptedGroup().getRequestingMembersList().stream().anyMatch(m -> m.getUuid().equals(uuid));
|
||||
|
||||
try (GroupManagerV2.GroupEditor editor = new GroupManagerV2(context).edit(groupId.requireV2())) {
|
||||
editor.ban(Collections.singleton(Recipient.resolved(recipientId).requireServiceId().uuid()));
|
||||
editor.ban(Collections.singleton(recipient.requireServiceId().uuid()), rejectJoinRequest);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -530,8 +530,8 @@ final class GroupManagerV2 {
|
|||
return commitChangeWithConflictResolution(groupOperations.createAcceptInviteChange(groupCandidate.getProfileKeyCredential().get()));
|
||||
}
|
||||
|
||||
public GroupManager.GroupActionResult ban(Set<UUID> uuids) throws GroupChangeFailedException, GroupNotAMemberException, GroupInsufficientRightsException, IOException {
|
||||
return commitChangeWithConflictResolution(groupOperations.createBanUuidsChange(uuids));
|
||||
public GroupManager.GroupActionResult ban(Set<UUID> uuids, boolean rejectJoinRequest) throws GroupChangeFailedException, GroupNotAMemberException, GroupInsufficientRightsException, IOException {
|
||||
return commitChangeWithConflictResolution(groupOperations.createBanUuidsChange(uuids, rejectJoinRequest));
|
||||
}
|
||||
|
||||
public GroupManager.GroupActionResult unban(Set<UUID> uuids) throws GroupChangeFailedException, GroupNotAMemberException, GroupInsufficientRightsException, IOException {
|
||||
|
|
|
@ -32,6 +32,8 @@ import org.whispersystems.signalservice.api.push.ServiceId;
|
|||
import java.util.Collections;
|
||||
import java.util.Comparator;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
import java.util.UUID;
|
||||
|
||||
public final class LiveGroup {
|
||||
|
||||
|
@ -141,6 +143,10 @@ public final class LiveGroup {
|
|||
return Transformations.map(groupRecord, g -> g.isAdmin(Recipient.self()));
|
||||
}
|
||||
|
||||
public LiveData<Set<UUID>> getBannedMembers() {
|
||||
return Transformations.map(groupRecord, g -> g.isV2Group() ? g.requireV2GroupProperties().getBannedMembers() : Collections.emptySet());
|
||||
}
|
||||
|
||||
public LiveData<Boolean> isActive() {
|
||||
return Transformations.map(groupRecord, GroupDatabase.GroupRecord::isActive);
|
||||
}
|
||||
|
|
|
@ -375,6 +375,8 @@
|
|||
<string name="ConversationFragment__block_request_button">Block request</string>
|
||||
<!-- Dialog cancel block request button -->
|
||||
<string name="ConversationFragment__cancel">Cancel</string>
|
||||
<!-- Message shown after successfully blocking join requests for a user -->
|
||||
<string name="ConversationFragment__blocked">Blocked</string>
|
||||
|
||||
<plurals name="ConversationListFragment_delete_selected_conversations">
|
||||
<item quantity="one">Delete selected conversation?</item>
|
||||
|
|
|
@ -210,7 +210,7 @@ public final class GroupsV2Operations {
|
|||
}
|
||||
|
||||
public GroupChange.Actions.Builder createRefuseGroupJoinRequest(Set<UUID> requestsToRemove, boolean alsoBan) {
|
||||
GroupChange.Actions.Builder actions = alsoBan ? createBanUuidsChange(requestsToRemove)
|
||||
GroupChange.Actions.Builder actions = alsoBan ? createBanUuidsChange(requestsToRemove, false)
|
||||
: GroupChange.Actions.newBuilder();
|
||||
|
||||
for (UUID uuid : requestsToRemove) {
|
||||
|
@ -236,7 +236,7 @@ public final class GroupsV2Operations {
|
|||
}
|
||||
|
||||
public GroupChange.Actions.Builder createRemoveMembersChange(final Set<UUID> membersToRemove, boolean alsoBan) {
|
||||
GroupChange.Actions.Builder actions = alsoBan ? createBanUuidsChange(membersToRemove)
|
||||
GroupChange.Actions.Builder actions = alsoBan ? createBanUuidsChange(membersToRemove, false)
|
||||
: GroupChange.Actions.newBuilder();
|
||||
|
||||
for (UUID remove: membersToRemove) {
|
||||
|
@ -350,8 +350,9 @@ public final class GroupsV2Operations {
|
|||
.setAnnouncementsOnly(isAnnouncementGroup));
|
||||
}
|
||||
|
||||
public GroupChange.Actions.Builder createBanUuidsChange(Set<UUID> banUuids) {
|
||||
GroupChange.Actions.Builder builder = GroupChange.Actions.newBuilder();
|
||||
public GroupChange.Actions.Builder createBanUuidsChange(Set<UUID> banUuids, boolean rejectJoinRequest) {
|
||||
GroupChange.Actions.Builder builder = rejectJoinRequest ? createRefuseGroupJoinRequest(banUuids, false)
|
||||
: GroupChange.Actions.newBuilder();
|
||||
|
||||
for (UUID uuid : banUuids) {
|
||||
builder.addAddBannedMembers(GroupChange.Actions.AddBannedMemberAction.newBuilder().setAdded(BannedMember.newBuilder().setUserId(encryptUuid(uuid)).build()));
|
||||
|
|
|
@ -393,7 +393,7 @@ public final class GroupsV2Operations_decrypt_change_Test {
|
|||
public void can_decrypt_member_bans_field22() {
|
||||
UUID ban = UUID.randomUUID();
|
||||
|
||||
assertDecryption(groupOperations.createBanUuidsChange(Collections.singleton(ban))
|
||||
assertDecryption(groupOperations.createBanUuidsChange(Collections.singleton(ban), false)
|
||||
.setRevision(13),
|
||||
DecryptedGroupChange.newBuilder()
|
||||
.setRevision(13)
|
||||
|
|
Loading…
Add table
Reference in a new issue