Add recipient protections and logging to media send flow.

This commit is contained in:
Greyson Parrelli 2021-02-25 12:19:58 -05:00 committed by GitHub
parent e6f4b0976f
commit 4f01bacb49
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 69 additions and 19 deletions

View file

@ -677,6 +677,13 @@ public class ConversationActivity extends PassphraseRequiredActivity
break; break;
case MEDIA_SENDER: case MEDIA_SENDER:
MediaSendActivityResult result = data.getParcelableExtra(MediaSendActivity.EXTRA_RESULT); 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()); sendButton.setTransport(result.getTransport());
if (result.isPushPreUpload()) { if (result.isPushPreUpload()) {
@ -705,7 +712,8 @@ public class ConversationActivity extends PassphraseRequiredActivity
final Context context = ConversationActivity.this.getApplicationContext(); final Context context = ConversationActivity.this.getApplicationContext();
sendMediaMessage(result.getTransport().isSms(), sendMediaMessage(result.getRecipientId(),
result.getTransport().isSms(),
result.getBody(), result.getBody(),
slideDeck, slideDeck,
quote, quote,
@ -2425,7 +2433,7 @@ public class ConversationActivity extends PassphraseRequiredActivity
long expiresIn = recipient.get().getExpireMessages() * 1000L; long expiresIn = recipient.get().getExpireMessages() * 1000L;
boolean initiating = threadId == -1; 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) { private void selectContactInfo(ContactData contactData) {
@ -2751,7 +2759,8 @@ public class ConversationActivity extends PassphraseRequiredActivity
throws InvalidMessageException throws InvalidMessageException
{ {
Log.i(TAG, "Sending media message..."); Log.i(TAG, "Sending media message...");
sendMediaMessage(forceSms, sendMediaMessage(recipient.getId(),
forceSms,
getMessage(), getMessage(),
attachmentManager.buildSlideDeck(), attachmentManager.buildSlideDeck(),
inputPanel.getQuote().orNull(), inputPanel.getQuote().orNull(),
@ -2765,7 +2774,8 @@ public class ConversationActivity extends PassphraseRequiredActivity
true); true);
} }
private ListenableFuture<Void> sendMediaMessage(final boolean forceSms, private ListenableFuture<Void> sendMediaMessage(@NonNull RecipientId recipientId,
final boolean forceSms,
@NonNull String body, @NonNull String body,
SlideDeck slideDeck, SlideDeck slideDeck,
QuoteModel quote, QuoteModel quote,
@ -2794,7 +2804,7 @@ public class ConversationActivity extends PassphraseRequiredActivity
} }
} }
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<Void> future = new SettableFuture<>(); final SettableFuture<Void> future = new SettableFuture<>();
final Context context = getApplicationContext(); final Context context = getApplicationContext();
@ -2985,7 +2995,8 @@ public class ConversationActivity extends PassphraseRequiredActivity
SlideDeck slideDeck = new SlideDeck(); SlideDeck slideDeck = new SlideDeck();
slideDeck.addSlide(audioSlide); slideDeck.addSlide(audioSlide);
ListenableFuture<Void> sendResult = sendMediaMessage(forceSms, ListenableFuture<Void> sendResult = sendMediaMessage(recipient.getId(),
forceSms,
"", "",
slideDeck, slideDeck,
inputPanel.getQuote().orNull(), inputPanel.getQuote().orNull(),
@ -3128,7 +3139,7 @@ public class ConversationActivity extends PassphraseRequiredActivity
slideDeck.addSlide(stickerSlide); 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) { private void silentlySetComposeText(String text) {
@ -3600,7 +3611,8 @@ public class ConversationActivity extends PassphraseRequiredActivity
throw new AssertionError("Only images are supported!"); throw new AssertionError("Only images are supported!");
} }
sendMediaMessage(isSmsForced(), sendMediaMessage(recipient.getId(),
isSmsForced(),
"", "",
slideDeck, slideDeck,
null, null,

View file

@ -245,9 +245,16 @@ public class MediaSendActivity extends PassphraseRequiredActivity implements Med
RecipientId recipientId = getIntent().getParcelableExtra(KEY_RECIPIENT); RecipientId recipientId = getIntent().getParcelableExtra(KEY_RECIPIENT);
if (recipientId != null) { if (recipientId != null) {
Log.i(TAG, "Preparing for " + recipientId);
recipient = Recipient.live(recipientId); recipient = Recipient.live(recipientId);
} }
List<RecipientId> 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); viewModel = ViewModelProviders.of(this, new MediaSendViewModel.Factory(getApplication(), new MediaRepository())).get(MediaSendViewModel.class);
transport = getIntent().getParcelableExtra(KEY_TRANSPORT); transport = getIntent().getParcelableExtra(KEY_TRANSPORT);
@ -348,7 +355,6 @@ public class MediaSendActivity extends PassphraseRequiredActivity implements Med
revealButton.setOnClickListener(v -> viewModel.onRevealButtonToggled()); revealButton.setOnClickListener(v -> viewModel.onRevealButtonToggled());
List<RecipientId> recipientIds = getIntent().getParcelableArrayListExtra(KEY_RECIPIENTS);
continueButton.setOnClickListener(v -> { continueButton.setOnClickListener(v -> {
continueButton.setEnabled(false); continueButton.setEnabled(false);
if (recipientIds == null || recipientIds.isEmpty()) { if (recipientIds == null || recipientIds.isEmpty()) {
@ -595,7 +601,11 @@ public class MediaSendActivity extends PassphraseRequiredActivity implements Med
viewModel.onSendClicked(buildModelsToTransform(fragment), recipients, composeText.getMentions()) viewModel.onSendClicked(buildModelsToTransform(fragment), recipients, composeText.getMentions())
.observe(this, result -> { .observe(this, result -> {
dialog.dismiss(); dialog.dismiss();
setActivityResultAndFinish(result); if (recipients.size() > 1) {
finishWithoutSettingResults();
} else {
setActivityResultAndFinish(result);
}
}); });
} }
@ -977,6 +987,14 @@ public class MediaSendActivity extends PassphraseRequiredActivity implements Med
return (MediaSendFragment) getSupportFragmentManager().findFragmentByTag(TAG_SEND); 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) { private void setActivityResultAndFinish(@NonNull MediaSendActivityResult result) {
Intent intent = new Intent(); Intent intent = new Intent();
intent.putExtra(EXTRA_RESULT, result); intent.putExtra(EXTRA_RESULT, result);

View file

@ -8,6 +8,7 @@ import androidx.annotation.NonNull;
import org.thoughtcrime.securesms.TransportOption; import org.thoughtcrime.securesms.TransportOption;
import org.thoughtcrime.securesms.conversation.ConversationActivity; import org.thoughtcrime.securesms.conversation.ConversationActivity;
import org.thoughtcrime.securesms.database.model.Mention; import org.thoughtcrime.securesms.database.model.Mention;
import org.thoughtcrime.securesms.recipients.RecipientId;
import org.thoughtcrime.securesms.sms.MessageSender.PreUploadResult; import org.thoughtcrime.securesms.sms.MessageSender.PreUploadResult;
import org.thoughtcrime.securesms.util.ParcelUtil; import org.thoughtcrime.securesms.util.ParcelUtil;
import org.whispersystems.libsignal.util.guava.Preconditions; import org.whispersystems.libsignal.util.guava.Preconditions;
@ -20,6 +21,7 @@ import java.util.List;
* A class that lets us nicely format data that we'll send back to {@link ConversationActivity}. * A class that lets us nicely format data that we'll send back to {@link ConversationActivity}.
*/ */
public class MediaSendActivityResult implements Parcelable { public class MediaSendActivityResult implements Parcelable {
private final RecipientId recipientId;
private final Collection<PreUploadResult> uploadResults; private final Collection<PreUploadResult> uploadResults;
private final Collection<Media> nonUploadedMedia; private final Collection<Media> nonUploadedMedia;
private final String body; private final String body;
@ -27,33 +29,37 @@ public class MediaSendActivityResult implements Parcelable {
private final boolean viewOnce; private final boolean viewOnce;
private final Collection<Mention> mentions; private final Collection<Mention> mentions;
static @NonNull MediaSendActivityResult forPreUpload(@NonNull Collection<PreUploadResult> uploadResults, static @NonNull MediaSendActivityResult forPreUpload(@NonNull RecipientId recipientId,
@NonNull Collection<PreUploadResult> uploadResults,
@NonNull String body, @NonNull String body,
@NonNull TransportOption transport, @NonNull TransportOption transport,
boolean viewOnce, boolean viewOnce,
@NonNull List<Mention> mentions) @NonNull List<Mention> mentions)
{ {
Preconditions.checkArgument(uploadResults.size() > 0, "Must supply uploadResults!"); 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<Media> nonUploadedMedia, static @NonNull MediaSendActivityResult forTraditionalSend(@NonNull RecipientId recipientId,
@NonNull List<Media> nonUploadedMedia,
@NonNull String body, @NonNull String body,
@NonNull TransportOption transport, @NonNull TransportOption transport,
boolean viewOnce, boolean viewOnce,
@NonNull List<Mention> mentions) @NonNull List<Mention> mentions)
{ {
Preconditions.checkArgument(nonUploadedMedia.size() > 0, "Must supply media!"); 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<PreUploadResult> uploadResults, private MediaSendActivityResult(@NonNull RecipientId recipientId,
@NonNull Collection<PreUploadResult> uploadResults,
@NonNull List<Media> nonUploadedMedia, @NonNull List<Media> nonUploadedMedia,
@NonNull String body, @NonNull String body,
@NonNull TransportOption transport, @NonNull TransportOption transport,
boolean viewOnce, boolean viewOnce,
@NonNull List<Mention> mentions) @NonNull List<Mention> mentions)
{ {
this.recipientId = recipientId;
this.uploadResults = uploadResults; this.uploadResults = uploadResults;
this.nonUploadedMedia = nonUploadedMedia; this.nonUploadedMedia = nonUploadedMedia;
this.body = body; this.body = body;
@ -63,6 +69,7 @@ public class MediaSendActivityResult implements Parcelable {
} }
private MediaSendActivityResult(Parcel in) { private MediaSendActivityResult(Parcel in) {
this.recipientId = in.readParcelable(RecipientId.class.getClassLoader());
this.uploadResults = ParcelUtil.readParcelableCollection(in, PreUploadResult.class); this.uploadResults = ParcelUtil.readParcelableCollection(in, PreUploadResult.class);
this.nonUploadedMedia = ParcelUtil.readParcelableCollection(in, Media.class); this.nonUploadedMedia = ParcelUtil.readParcelableCollection(in, Media.class);
this.body = in.readString(); this.body = in.readString();
@ -71,6 +78,10 @@ public class MediaSendActivityResult implements Parcelable {
this.mentions = ParcelUtil.readParcelableCollection(in, Mention.class); this.mentions = ParcelUtil.readParcelableCollection(in, Mention.class);
} }
public @NonNull RecipientId getRecipientId() {
return recipientId;
}
public boolean isPushPreUpload() { public boolean isPushPreUpload() {
return uploadResults.size() > 0; return uploadResults.size() > 0;
} }
@ -118,6 +129,7 @@ public class MediaSendActivityResult implements Parcelable {
@Override @Override
public void writeToParcel(Parcel dest, int flags) { public void writeToParcel(Parcel dest, int flags) {
dest.writeParcelable(recipientId, 0);
ParcelUtil.writeParcelableCollection(dest, uploadResults); ParcelUtil.writeParcelableCollection(dest, uploadResults);
ParcelUtil.writeParcelableCollection(dest, nonUploadedMedia); ParcelUtil.writeParcelableCollection(dest, nonUploadedMedia);
dest.writeString(body); dest.writeString(body);

View file

@ -475,7 +475,7 @@ class MediaSendViewModel extends ViewModel {
if (isSms || MessageSender.isLocalSelfSend(application, recipient, isSms)) { if (isSms || MessageSender.isLocalSelfSend(application, recipient, isSms)) {
Log.i(TAG, "SMS or local self-send. Skipping pre-upload."); 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; return;
} }
@ -494,9 +494,10 @@ class MediaSendViewModel extends ViewModel {
if (recipients.size() > 0) { if (recipients.size() > 0) {
sendMessages(recipients, splitBody, uploadResults, trimmedMentions); sendMessages(recipients, splitBody, uploadResults, trimmedMentions);
uploadRepository.deleteAbandonedAttachments(); 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));
}); });
}); });

View file

@ -100,6 +100,7 @@ public class MessageSender {
final boolean forceSms, final boolean forceSms,
final SmsDatabase.InsertListener insertListener) final SmsDatabase.InsertListener insertListener)
{ {
Log.i(TAG, "Sending text message to " + message.getRecipient().getId() + ", thread: " + threadId);
MessageDatabase database = DatabaseFactory.getSmsDatabase(context); MessageDatabase database = DatabaseFactory.getSmsDatabase(context);
Recipient recipient = message.getRecipient(); Recipient recipient = message.getRecipient();
boolean keyExchange = message.isKeyExchange(); boolean keyExchange = message.isKeyExchange();
@ -119,6 +120,7 @@ public class MessageSender {
final boolean forceSms, final boolean forceSms,
final SmsDatabase.InsertListener insertListener) final SmsDatabase.InsertListener insertListener)
{ {
Log.i(TAG, "Sending media message to " + message.getRecipient().getId() + ", thread: " + threadId);
try { try {
ThreadDatabase threadDatabase = DatabaseFactory.getThreadDatabase(context); ThreadDatabase threadDatabase = DatabaseFactory.getThreadDatabase(context);
MessageDatabase database = DatabaseFactory.getMmsDatabase(context); MessageDatabase database = DatabaseFactory.getMmsDatabase(context);
@ -144,6 +146,7 @@ public class MessageSender {
final long threadId, final long threadId,
final SmsDatabase.InsertListener insertListener) 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."); Preconditions.checkArgument(message.getAttachments().isEmpty(), "If the media is pre-uploaded, there should be no attachments on the message.");
try { try {
@ -178,6 +181,7 @@ public class MessageSender {
} }
public static void sendMediaBroadcast(@NonNull Context context, @NonNull List<OutgoingSecureMediaMessage> messages, @NonNull Collection<PreUploadResult> preUploadResults) { public static void sendMediaBroadcast(@NonNull Context context, @NonNull List<OutgoingSecureMediaMessage> messages, @NonNull Collection<PreUploadResult> 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(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."); 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 class MessageSender {
* be enqueued (like in the case of a local self-send). * be enqueued (like in the case of a local self-send).
*/ */
public static @Nullable PreUploadResult preUploadPushAttachment(@NonNull Context context, @NonNull Attachment attachment, @Nullable Recipient recipient) { public static @Nullable PreUploadResult preUploadPushAttachment(@NonNull Context context, @NonNull Attachment attachment, @Nullable Recipient recipient) {
if (recipient != null && isLocalSelfSend(context, recipient, false)) { if (isLocalSelfSend(context, recipient, false)) {
return null; return null;
} }
Log.i(TAG, "Pre-uploading attachment for " + (recipient != null ? recipient.getId() : "null"));
try { try {
AttachmentDatabase attachmentDatabase = DatabaseFactory.getAttachmentDatabase(context); AttachmentDatabase attachmentDatabase = DatabaseFactory.getAttachmentDatabase(context);

View file

@ -275,6 +275,8 @@
<string name="ConversationActivity_join">Join</string> <string name="ConversationActivity_join">Join</string>
<string name="ConversationActivity_full">Full</string> <string name="ConversationActivity_full">Full</string>
<string name="ConversationActivity_error_sending_media">Error sending media</string>
<!-- ConversationAdapter --> <!-- ConversationAdapter -->
<plurals name="ConversationAdapter_n_unread_messages"> <plurals name="ConversationAdapter_n_unread_messages">
<item quantity="one">%d unread message</item> <item quantity="one">%d unread message</item>