From 4f01bacb499da150e9cd9f826b90e34c73a23a3d Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Thu, 25 Feb 2021 12:19:58 -0500 Subject: [PATCH] Add recipient protections and logging to media send flow. --- .../conversation/ConversationActivity.java | 28 +++++++++++++------ .../mediasend/MediaSendActivity.java | 22 +++++++++++++-- .../mediasend/MediaSendActivityResult.java | 22 +++++++++++---- .../mediasend/MediaSendViewModel.java | 7 +++-- .../securesms/sms/MessageSender.java | 7 ++++- app/src/main/res/values/strings.xml | 2 ++ 6 files changed, 69 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActivity.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActivity.java index b282dabc38b..2121943f681 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActivity.java @@ -677,6 +677,13 @@ public void onActivityResult(final int reqCode, int resultCode, Intent data) { break; case MEDIA_SENDER: MediaSendActivityResult result = data.getParcelableExtra(MediaSendActivity.EXTRA_RESULT); + + if (!Objects.equals(result.getRecipientId(), recipient.getId())) { + Log.w(TAG, "Result's recipientId did not match ours! Result: " + result.getRecipientId() + ", Activity: " + recipient.getId()); + Toast.makeText(this, R.string.ConversationActivity_error_sending_media, Toast.LENGTH_SHORT).show(); + return; + } + sendButton.setTransport(result.getTransport()); if (result.isPushPreUpload()) { @@ -705,7 +712,8 @@ public void onActivityResult(final int reqCode, int resultCode, Intent data) { final Context context = ConversationActivity.this.getApplicationContext(); - sendMediaMessage(result.getTransport().isSms(), + sendMediaMessage(result.getRecipientId(), + result.getTransport().isSms(), result.getBody(), slideDeck, quote, @@ -2425,7 +2433,7 @@ private void sendSharedContact(List contacts) { long expiresIn = recipient.get().getExpireMessages() * 1000L; boolean initiating = threadId == -1; - sendMediaMessage(isSmsForced(), "", attachmentManager.buildSlideDeck(), null, contacts, Collections.emptyList(), Collections.emptyList(), expiresIn, false, subscriptionId, initiating, false); + sendMediaMessage(recipient.getId(), isSmsForced(), "", attachmentManager.buildSlideDeck(), null, contacts, Collections.emptyList(), Collections.emptyList(), expiresIn, false, subscriptionId, initiating, false); } private void selectContactInfo(ContactData contactData) { @@ -2751,7 +2759,8 @@ private void sendMediaMessage(final boolean forceSms, final long expiresIn, fina throws InvalidMessageException { Log.i(TAG, "Sending media message..."); - sendMediaMessage(forceSms, + sendMediaMessage(recipient.getId(), + forceSms, getMessage(), attachmentManager.buildSlideDeck(), inputPanel.getQuote().orNull(), @@ -2765,7 +2774,8 @@ private void sendMediaMessage(final boolean forceSms, final long expiresIn, fina true); } - private ListenableFuture sendMediaMessage(final boolean forceSms, + private ListenableFuture sendMediaMessage(@NonNull RecipientId recipientId, + final boolean forceSms, @NonNull String body, SlideDeck slideDeck, QuoteModel quote, @@ -2794,7 +2804,7 @@ private ListenableFuture sendMediaMessage(final boolean forceSms, } } - OutgoingMediaMessage outgoingMessageCandidate = new OutgoingMediaMessage(recipient.get(), slideDeck, body, System.currentTimeMillis(), subscriptionId, expiresIn, viewOnce, distributionType, quote, contacts, previews, mentions); + OutgoingMediaMessage outgoingMessageCandidate = new OutgoingMediaMessage(Recipient.resolved(recipientId), slideDeck, body, System.currentTimeMillis(), subscriptionId, expiresIn, viewOnce, distributionType, quote, contacts, previews, mentions); final SettableFuture future = new SettableFuture<>(); final Context context = getApplicationContext(); @@ -2985,7 +2995,8 @@ public void onSuccess(final @NonNull Pair result) { SlideDeck slideDeck = new SlideDeck(); slideDeck.addSlide(audioSlide); - ListenableFuture sendResult = sendMediaMessage(forceSms, + ListenableFuture sendResult = sendMediaMessage(recipient.getId(), + forceSms, "", slideDeck, inputPanel.getQuote().orNull(), @@ -3128,7 +3139,7 @@ private void sendSticker(@NonNull StickerLocator stickerLocator, @NonNull String slideDeck.addSlide(stickerSlide); - sendMediaMessage(transport.isSms(), "", slideDeck, null, Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), expiresIn, false, subscriptionId, initiating, clearCompose); + sendMediaMessage(recipient.getId(), transport.isSms(), "", slideDeck, null, Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), expiresIn, false, subscriptionId, initiating, clearCompose); } private void silentlySetComposeText(String text) { @@ -3600,7 +3611,8 @@ private void sendKeyboardImage(@NonNull Uri uri, @NonNull String contentType, @N throw new AssertionError("Only images are supported!"); } - sendMediaMessage(isSmsForced(), + sendMediaMessage(recipient.getId(), + isSmsForced(), "", slideDeck, null, diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivity.java b/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivity.java index 88752260549..d0b5f43d058 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivity.java @@ -245,9 +245,16 @@ protected void onCreate(Bundle savedInstanceState, boolean ready) { RecipientId recipientId = getIntent().getParcelableExtra(KEY_RECIPIENT); if (recipientId != null) { + Log.i(TAG, "Preparing for " + recipientId); recipient = Recipient.live(recipientId); } + List recipientIds = getIntent().getParcelableArrayListExtra(KEY_RECIPIENTS); + if (recipientIds != null && recipientIds.size() > 0) { + Log.i(TAG, "Preparing for " + recipientIds); + } + + viewModel = ViewModelProviders.of(this, new MediaSendViewModel.Factory(getApplication(), new MediaRepository())).get(MediaSendViewModel.class); transport = getIntent().getParcelableExtra(KEY_TRANSPORT); @@ -348,7 +355,6 @@ public void onTextChanged(String text) { revealButton.setOnClickListener(v -> viewModel.onRevealButtonToggled()); - List recipientIds = getIntent().getParcelableArrayListExtra(KEY_RECIPIENTS); continueButton.setOnClickListener(v -> { continueButton.setEnabled(false); if (recipientIds == null || recipientIds.isEmpty()) { @@ -595,7 +601,11 @@ private void onSend(@NonNull List recipients) { viewModel.onSendClicked(buildModelsToTransform(fragment), recipients, composeText.getMentions()) .observe(this, result -> { dialog.dismiss(); - setActivityResultAndFinish(result); + if (recipients.size() > 1) { + finishWithoutSettingResults(); + } else { + setActivityResultAndFinish(result); + } }); } @@ -977,6 +987,14 @@ public void onEmojiSelected(String emoji) { return (MediaSendFragment) getSupportFragmentManager().findFragmentByTag(TAG_SEND); } + private void finishWithoutSettingResults() { + Intent intent = new Intent(); + setResult(RESULT_OK, intent); + + finish(); + overridePendingTransition(R.anim.stationary, R.anim.camera_slide_to_bottom); + } + private void setActivityResultAndFinish(@NonNull MediaSendActivityResult result) { Intent intent = new Intent(); intent.putExtra(EXTRA_RESULT, result); diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivityResult.java b/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivityResult.java index 5750516d5a3..63892d63229 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivityResult.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivityResult.java @@ -8,6 +8,7 @@ import org.thoughtcrime.securesms.TransportOption; import org.thoughtcrime.securesms.conversation.ConversationActivity; import org.thoughtcrime.securesms.database.model.Mention; +import org.thoughtcrime.securesms.recipients.RecipientId; import org.thoughtcrime.securesms.sms.MessageSender.PreUploadResult; import org.thoughtcrime.securesms.util.ParcelUtil; import org.whispersystems.libsignal.util.guava.Preconditions; @@ -20,6 +21,7 @@ * A class that lets us nicely format data that we'll send back to {@link ConversationActivity}. */ public class MediaSendActivityResult implements Parcelable { + private final RecipientId recipientId; private final Collection uploadResults; private final Collection nonUploadedMedia; private final String body; @@ -27,33 +29,37 @@ public class MediaSendActivityResult implements Parcelable { private final boolean viewOnce; private final Collection mentions; - static @NonNull MediaSendActivityResult forPreUpload(@NonNull Collection uploadResults, + static @NonNull MediaSendActivityResult forPreUpload(@NonNull RecipientId recipientId, + @NonNull Collection uploadResults, @NonNull String body, @NonNull TransportOption transport, boolean viewOnce, @NonNull List mentions) { Preconditions.checkArgument(uploadResults.size() > 0, "Must supply uploadResults!"); - return new MediaSendActivityResult(uploadResults, Collections.emptyList(), body, transport, viewOnce, mentions); + return new MediaSendActivityResult(recipientId, uploadResults, Collections.emptyList(), body, transport, viewOnce, mentions); } - static @NonNull MediaSendActivityResult forTraditionalSend(@NonNull List nonUploadedMedia, + static @NonNull MediaSendActivityResult forTraditionalSend(@NonNull RecipientId recipientId, + @NonNull List nonUploadedMedia, @NonNull String body, @NonNull TransportOption transport, boolean viewOnce, @NonNull List mentions) { Preconditions.checkArgument(nonUploadedMedia.size() > 0, "Must supply media!"); - return new MediaSendActivityResult(Collections.emptyList(), nonUploadedMedia, body, transport, viewOnce, mentions); + return new MediaSendActivityResult(recipientId, Collections.emptyList(), nonUploadedMedia, body, transport, viewOnce, mentions); } - private MediaSendActivityResult(@NonNull Collection uploadResults, + private MediaSendActivityResult(@NonNull RecipientId recipientId, + @NonNull Collection uploadResults, @NonNull List nonUploadedMedia, @NonNull String body, @NonNull TransportOption transport, boolean viewOnce, @NonNull List mentions) { + this.recipientId = recipientId; this.uploadResults = uploadResults; this.nonUploadedMedia = nonUploadedMedia; this.body = body; @@ -63,6 +69,7 @@ private MediaSendActivityResult(@NonNull Collection uploadResul } private MediaSendActivityResult(Parcel in) { + this.recipientId = in.readParcelable(RecipientId.class.getClassLoader()); this.uploadResults = ParcelUtil.readParcelableCollection(in, PreUploadResult.class); this.nonUploadedMedia = ParcelUtil.readParcelableCollection(in, Media.class); this.body = in.readString(); @@ -71,6 +78,10 @@ private MediaSendActivityResult(Parcel in) { this.mentions = ParcelUtil.readParcelableCollection(in, Mention.class); } + public @NonNull RecipientId getRecipientId() { + return recipientId; + } + public boolean isPushPreUpload() { return uploadResults.size() > 0; } @@ -118,6 +129,7 @@ public int describeContents() { @Override public void writeToParcel(Parcel dest, int flags) { + dest.writeParcelable(recipientId, 0); ParcelUtil.writeParcelableCollection(dest, uploadResults); ParcelUtil.writeParcelableCollection(dest, nonUploadedMedia); dest.writeString(body); diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendViewModel.java b/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendViewModel.java index 5a7d2eccb7f..776f92b3a77 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendViewModel.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendViewModel.java @@ -475,7 +475,7 @@ void saveDrawState(@NonNull Map state) { if (isSms || MessageSender.isLocalSelfSend(application, recipient, isSms)) { Log.i(TAG, "SMS or local self-send. Skipping pre-upload."); - result.postValue(MediaSendActivityResult.forTraditionalSend(updatedMedia, trimmedBody, transport, isViewOnce(), trimmedMentions)); + result.postValue(MediaSendActivityResult.forTraditionalSend(recipient.getId(), updatedMedia, trimmedBody, transport, isViewOnce(), trimmedMentions)); return; } @@ -494,9 +494,10 @@ void saveDrawState(@NonNull Map state) { if (recipients.size() > 0) { sendMessages(recipients, splitBody, uploadResults, trimmedMentions); uploadRepository.deleteAbandonedAttachments(); + result.postValue(null); + } else { + result.postValue(MediaSendActivityResult.forPreUpload(recipient.getId(), uploadResults, splitBody, transport, isViewOnce(), trimmedMentions)); } - - result.postValue(MediaSendActivityResult.forPreUpload(uploadResults, splitBody, transport, isViewOnce(), trimmedMentions)); }); }); diff --git a/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java b/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java index 71896ce7b21..a0b364548f5 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java +++ b/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java @@ -100,6 +100,7 @@ public static long send(final Context context, final boolean forceSms, final SmsDatabase.InsertListener insertListener) { + Log.i(TAG, "Sending text message to " + message.getRecipient().getId() + ", thread: " + threadId); MessageDatabase database = DatabaseFactory.getSmsDatabase(context); Recipient recipient = message.getRecipient(); boolean keyExchange = message.isKeyExchange(); @@ -119,6 +120,7 @@ public static long send(final Context context, final boolean forceSms, final SmsDatabase.InsertListener insertListener) { + Log.i(TAG, "Sending media message to " + message.getRecipient().getId() + ", thread: " + threadId); try { ThreadDatabase threadDatabase = DatabaseFactory.getThreadDatabase(context); MessageDatabase database = DatabaseFactory.getMmsDatabase(context); @@ -144,6 +146,7 @@ public static long sendPushWithPreUploadedMedia(final Context context, final long threadId, final SmsDatabase.InsertListener insertListener) { + Log.i(TAG, "Sending media message with pre-uploads to " + message.getRecipient().getId() + ", thread: " + threadId); Preconditions.checkArgument(message.getAttachments().isEmpty(), "If the media is pre-uploaded, there should be no attachments on the message."); try { @@ -178,6 +181,7 @@ public static long sendPushWithPreUploadedMedia(final Context context, } public static void sendMediaBroadcast(@NonNull Context context, @NonNull List messages, @NonNull Collection preUploadResults) { + Log.i(TAG, "Sending media broadcast to " + Stream.of(messages).map(m -> m.getRecipient().getId()).toList()); Preconditions.checkArgument(messages.size() > 0, "No messages!"); Preconditions.checkArgument(Stream.of(messages).allMatch(m -> m.getAttachments().isEmpty()), "Messages can't have attachments! They should be pre-uploaded."); @@ -260,9 +264,10 @@ public static void sendMediaBroadcast(@NonNull Context context, @NonNull ListJoin Full + Error sending media + %d unread message