Skip to content

Commit

Permalink
Add recipient protections and logging to media send flow.
Browse files Browse the repository at this point in the history
  • Loading branch information
@greyson-signal
greyson-signal committed Feb 25, 2021
1 parent e6f4b09 commit 4f01bac
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 19 deletions.
28 changes: 20 additions & 8 deletions app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationActivity.java
View file
Expand Up @@ -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()) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -2425,7 +2433,7 @@ private void sendSharedContact(List<Contact> 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) {
Expand Down Expand Up @@ -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(),
Expand All @@ -2765,7 +2774,8 @@ private void sendMediaMessage(final boolean forceSms, final long expiresIn, fina
true);
}

private ListenableFuture<Void> sendMediaMessage(final boolean forceSms,
private ListenableFuture<Void> sendMediaMessage(@NonNull RecipientId recipientId,
final boolean forceSms,
@NonNull String body,
SlideDeck slideDeck,
QuoteModel quote,
Expand Down Expand Up @@ -2794,7 +2804,7 @@ private ListenableFuture<Void> 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<Void> future = new SettableFuture<>();
final Context context = getApplicationContext();
Expand Down Expand Up @@ -2985,7 +2995,8 @@ public void onSuccess(final @NonNull Pair<Uri, Long> result) {
SlideDeck slideDeck = new SlideDeck();
slideDeck.addSlide(audioSlide);

ListenableFuture<Void> sendResult = sendMediaMessage(forceSms,
ListenableFuture<Void> sendResult = sendMediaMessage(recipient.getId(),
forceSms,
"",
slideDeck,
inputPanel.getQuote().orNull(),
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
22 changes: 20 additions & 2 deletions app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivity.java
View file
Expand Up @@ -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<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);
transport = getIntent().getParcelableExtra(KEY_TRANSPORT);

Expand Down Expand Up @@ -348,7 +355,6 @@ public void onTextChanged(String text) {

revealButton.setOnClickListener(v -> viewModel.onRevealButtonToggled());

List<RecipientId> recipientIds = getIntent().getParcelableArrayListExtra(KEY_RECIPIENTS);
continueButton.setOnClickListener(v -> {
continueButton.setEnabled(false);
if (recipientIds == null || recipientIds.isEmpty()) {
Expand Down Expand Up @@ -595,7 +601,11 @@ private void onSend(@NonNull List<Recipient> recipients) {
viewModel.onSendClicked(buildModelsToTransform(fragment), recipients, composeText.getMentions())
.observe(this, result -> {
dialog.dismiss();
setActivityResultAndFinish(result);
if (recipients.size() > 1) {
finishWithoutSettingResults();
} else {
setActivityResultAndFinish(result);
}
});
}

Expand Down Expand Up @@ -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);
Expand Down
22 changes: 17 additions & 5 deletions app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivityResult.java
View file
Expand Up @@ -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;
Expand All @@ -20,40 +21,45 @@
* 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<PreUploadResult> uploadResults;
private final Collection<Media> nonUploadedMedia;
private final String body;
private final TransportOption transport;
private final boolean viewOnce;
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 TransportOption transport,
boolean viewOnce,
@NonNull List<Mention> 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<Media> nonUploadedMedia,
static @NonNull MediaSendActivityResult forTraditionalSend(@NonNull RecipientId recipientId,
@NonNull List<Media> nonUploadedMedia,
@NonNull String body,
@NonNull TransportOption transport,
boolean viewOnce,
@NonNull List<Mention> 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<PreUploadResult> uploadResults,
private MediaSendActivityResult(@NonNull RecipientId recipientId,
@NonNull Collection<PreUploadResult> uploadResults,
@NonNull List<Media> nonUploadedMedia,
@NonNull String body,
@NonNull TransportOption transport,
boolean viewOnce,
@NonNull List<Mention> mentions)
{
this.recipientId = recipientId;
this.uploadResults = uploadResults;
this.nonUploadedMedia = nonUploadedMedia;
this.body = body;
Expand All @@ -63,6 +69,7 @@ private MediaSendActivityResult(@NonNull Collection<PreUploadResult> 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();
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 4 additions & 3 deletions app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendViewModel.java
View file
Expand Up @@ -475,7 +475,7 @@ void saveDrawState(@NonNull Map<Uri, Object> 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;
}

Expand All @@ -494,9 +494,10 @@ void saveDrawState(@NonNull Map<Uri, Object> 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));
});
});

Expand Down
7 changes: 6 additions & 1 deletion app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java
View file
Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -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 {
Expand Down Expand Up @@ -178,6 +181,7 @@ public static long sendPushWithPreUploadedMedia(final Context context,
}

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(Stream.of(messages).allMatch(m -> m.getAttachments().isEmpty()), "Messages can't have attachments! They should be pre-uploaded.");

Expand Down Expand Up @@ -260,9 +264,10 @@ public static void sendMediaBroadcast(@NonNull Context context, @NonNull List<Ou
* 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) {
if (recipient != null && isLocalSelfSend(context, recipient, false)) {
if (isLocalSelfSend(context, recipient, false)) {
return null;
}
Log.i(TAG, "Pre-uploading attachment for " + (recipient != null ? recipient.getId() : "null"));

try {
AttachmentDatabase attachmentDatabase = DatabaseFactory.getAttachmentDatabase(context);
Expand Down
2 changes: 2 additions & 0 deletions app/src/main/res/values/strings.xml
View file
Expand Up @@ -275,6 +275,8 @@
<string name="ConversationActivity_join">Join</string>
<string name="ConversationActivity_full">Full</string>

<string name="ConversationActivity_error_sending_media">Error sending media</string>

<!-- ConversationAdapter -->
<plurals name="ConversationAdapter_n_unread_messages">
<item quantity="one">%d unread message</item>
Expand Down

0 comments on commit 4f01bac

Please sign in to comment.