diff --git a/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.kt b/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.kt index 568e3e4d28..4dc3b12b05 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.kt @@ -150,6 +150,9 @@ class MediaPreviewActivity : ScreenLockActionBarActivity(), @Inject lateinit var mediaDatabase: MediaDatabase + @Inject + lateinit var shareIntentTokenStore: ShareIntentTokenStore + override val applyDefaultWindowInsets: Boolean get() = false @@ -488,6 +491,12 @@ class MediaPreviewActivity : ScreenLockActionBarActivity(), ) composeIntent.setAction(Intent.ACTION_SEND) composeIntent.putExtra(Intent.EXTRA_STREAM, mediaItem.uri) + // ShareActivity passes one of our own attachment URIs along untouched only for the exact + // URIs a token vouches for; without this it would have nothing to read. + composeIntent.putExtra( + ShareActivity.EXTRA_SHARE_TOKEN, + shareIntentTokenStore.mint(authorisedUris = setOf(mediaItem.uri)) + ) composeIntent.setType(mediaItem.mimeType) startActivity(composeIntent) } diff --git a/app/src/main/java/org/thoughtcrime/securesms/ScreenLockActionBarActivity.kt b/app/src/main/java/org/thoughtcrime/securesms/ScreenLockActionBarActivity.kt index 4aae429d06..7cdf9c7e1f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/ScreenLockActionBarActivity.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/ScreenLockActionBarActivity.kt @@ -2,12 +2,14 @@ package org.thoughtcrime.securesms import android.content.BroadcastReceiver import android.content.ClipData +import android.content.ContentResolver import android.content.Context import android.content.Intent import android.content.IntentFilter import android.net.Uri import android.os.Bundle import androidx.annotation.IdRes +import androidx.annotation.VisibleForTesting import androidx.core.content.ContextCompat import androidx.core.content.IntentCompat import androidx.fragment.app.Fragment @@ -21,6 +23,7 @@ import org.session.libsignal.utilities.Log import org.thoughtcrime.securesms.auth.LoginStateRepository import org.thoughtcrime.securesms.home.HomeActivity import org.thoughtcrime.securesms.migration.DatabaseMigrationManager +import org.thoughtcrime.securesms.mms.PartAuthority import org.thoughtcrime.securesms.migration.DatabaseMigrationStateActivity import org.thoughtcrime.securesms.onboarding.landing.LandingActivity import org.thoughtcrime.securesms.service.KeyCachingService @@ -345,14 +348,36 @@ abstract class ScreenLockActionBarActivity : BaseActionBarActivity() { private suspend fun copyFileToCache(uri: Uri, filename: String): Uri? = withContext(Dispatchers.IO) { try { + // A URI grant is the only thing that makes the sender's content readable to us, and only + // content:// carries one. openInputStream also accepts file:// and android.resource://, + // both of which it opens as this app with nothing consulted - and this runs before the + // user has authenticated, so it must not be able to reach anything of ours. + if (ContentResolver.SCHEME_CONTENT != uri.scheme) { + Log.w(TAG, "Refusing to cache a shared URI that carries no content grant - aborting.") + return@withContext null + } + + // Our own providers answer us regardless of being unexported, and our FileProvider's + // configured roots include this very cache directory - so without this the copy below + // would read our own data back for the sender, still before they have authenticated. + if (PartAuthority.isLocalUri(uri) || FileProviderUtil.AUTHORITY == uri.authority) { + Log.w(TAG, "Refusing to cache a shared URI that names one of our own providers - aborting.") + return@withContext null + } + + val cacheFilename = cacheFilenameFrom(filename) + if (cacheFilename == null) { + Log.w(TAG, "Shared content did not provide a usable filename - aborting.") + return@withContext null + } + val inputStream = contentResolver.openInputStream(uri) if (inputStream == null) { Log.w(TAG, "Could not open input stream to cache shared content - aborting.") return@withContext null } - // Create a File in your cache directory using the retrieved name - val tempFile = File(cacheDir, filename) + val tempFile = File(cacheDir, cacheFilename) inputStream.use { input -> FileOutputStream(tempFile).use { output -> input.copyTo(output) @@ -413,4 +438,16 @@ abstract class ScreenLockActionBarActivity : BaseActionBarActivity() { clearKeyReceiver = null } } -} \ No newline at end of file +} + +/** + * Reduces a sending app's `OpenableColumns.DISPLAY_NAME` to a name that can only land directly in the + * directory it is joined to, or null when nothing usable is left of it. + * + * The display name reaches us verbatim from the sending app and is not a path segment until it is + * made one: joined as given it lets "../" out of the directory, and the two relative names survive + * the reduction still naming a directory rather than a file. + */ +@VisibleForTesting +internal fun cacheFilenameFrom(displayName: String): String? = + File(displayName).name.takeUnless { it.isEmpty() || it == "." || it == ".." } diff --git a/app/src/main/java/org/thoughtcrime/securesms/ShareActivity.kt b/app/src/main/java/org/thoughtcrime/securesms/ShareActivity.kt index ee259d5132..dd1a1ff8f8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/ShareActivity.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/ShareActivity.kt @@ -32,7 +32,7 @@ class ShareActivity : FullComposeScreenLockActivity() { private val viewModel: ShareViewModel by viewModels() companion object { - const val EXTRA_ADDRESS = "address" + const val EXTRA_SHARE_TOKEN = "share_token" } diff --git a/app/src/main/java/org/thoughtcrime/securesms/ShareIntentTokenStore.kt b/app/src/main/java/org/thoughtcrime/securesms/ShareIntentTokenStore.kt new file mode 100644 index 0000000000..2b77ef8e4f --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/ShareIntentTokenStore.kt @@ -0,0 +1,66 @@ +package org.thoughtcrime.securesms + +import android.net.Uri +import org.session.libsession.utilities.Address +import java.security.SecureRandom +import java.util.Base64 +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Issues opaque tokens that mark a share Intent as one this app built itself, and carries the + * conversation such an Intent should open. + * + * `ShareActivity` is exported, so the Intent it receives is composed by whichever app invoked the + * share sheet. A token stands in for the destination because it means nothing outside this process, + * where the table that resolves it lives. + * + * A token names the URIs it speaks for rather than merely existing, because the two do not arrive + * together: the system chooser merges a direct-share target's extras into the *sender's* Intent, so + * a token minted here can reach us alongside URIs chosen by another app. + */ +@Singleton +class ShareIntentTokenStore @Inject constructor() { + + /** + * Present only for a token this store issued. A null [address] means "no destination chosen", + * and [authorisedUris] is the exact set of our own URIs the Intent carrying it may pass along - + * usually empty. + */ + class Minted(val address: Address?, val authorisedUris: Set) { + fun authorises(uri: Uri): Boolean = uri in authorisedUris + } + + private val random = SecureRandom() + + private val issued = LinkedHashMap() + + @JvmOverloads + @Synchronized + fun mint(address: Address? = null, authorisedUris: Set = emptySet()): String { + // A chooser refresh mints one token per conversation, so retention is capped rather than + // left to grow with however many times the share sheet has been opened this process. + while (issued.size >= MAX_RETAINED) { + issued.remove(issued.keys.first()) + } + + val token = Base64.getUrlEncoder().withoutPadding() + .encodeToString(ByteArray(TOKEN_BYTES).also(random::nextBytes)) + + issued[token] = Minted(address, authorisedUris) + return token + } + + /** + * Resolution deliberately does not retire the token: one user action can create `ShareActivity` + * twice - once before app lock routes it away, once from the Intent the lock screen replays - + * and each instance resolves the Intent independently. + */ + @Synchronized + fun resolve(token: String?): Minted? = token?.let(issued::get) + + private companion object { + private const val TOKEN_BYTES = 32 + private const val MAX_RETAINED = 512 + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/ShareViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/ShareViewModel.kt index 5ad30e9dfd..4b3d411e43 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/ShareViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/ShareViewModel.kt @@ -1,10 +1,11 @@ package org.thoughtcrime.securesms +import android.content.ContentResolver import android.content.Context import android.content.Intent import android.net.Uri import android.provider.OpenableColumns -import androidx.core.content.IntentCompat +import androidx.annotation.VisibleForTesting import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel @@ -35,9 +36,9 @@ import org.thoughtcrime.securesms.mms.PartAuthority import org.thoughtcrime.securesms.providers.BlobUtils import org.thoughtcrime.securesms.repository.ConversationRepository import org.thoughtcrime.securesms.util.AvatarUIData +import org.thoughtcrime.securesms.util.FileProviderUtil import org.thoughtcrime.securesms.util.AvatarUtils import org.thoughtcrime.securesms.util.MediaUtil -import java.io.FileInputStream import java.io.IOException import javax.inject.Inject @@ -46,6 +47,7 @@ class ShareViewModel @Inject constructor( @ApplicationContext private val context: Context, private val avatarUtils: AvatarUtils, private val deprecationManager: LegacyGroupDeprecationManager, + private val shareIntentTokenStore: ShareIntentTokenStore, conversationRepository: ConversationRepository, ): ViewModel(){ @@ -55,6 +57,8 @@ class ShareViewModel @Inject constructor( private var resolvedPlaintext: CharSequence? = null private var mimeType: String? = null private var isPassingAlongMedia = false + private var minted: ShareIntentTokenStore.Minted? = null + private var shareDestination: Address? = null // Input: The search query private val mutableSearchQuery = MutableStateFlow("") @@ -144,6 +148,10 @@ class ShareViewModel @Inject constructor( mimeType = null isPassingAlongMedia = false + val minted = shareIntentTokenStore.resolve(intent.getStringExtra(ShareActivity.EXTRA_SHARE_TOKEN)) + this.minted = minted + shareDestination = minted?.address + val action = intent.action val type = intent.type val incomingUris = ArrayList() @@ -176,27 +184,32 @@ class ShareViewModel @Inject constructor( isPassingAlongMedia = false mimeType = getMimeType(uris.firstOrNull(), type) - if (uris.isNotEmpty() && uris.all { PartAuthority.isLocalUri(it) }) { + // A URI naming one of our own providers is passed to the attachment manager verbatim, which + // reads it as us - so it resolves to the viewer's own message history rather than to anything + // the sender holds. Only the exact URIs a token was minted for may take that route: holding a + // token is not enough, because the chooser merges our direct-share extras into the sender's + // own Intent, so a valid token can arrive alongside URIs we never vouched for. + if (minted != null && uris.isNotEmpty() && uris.all { minted.authorises(it) }) { isPassingAlongMedia = true resolvedExtras = uris - handleResolvedMedia(intent) + handleResolvedMedia() } else if ( uris.isEmpty() && charSequenceExtra != null && (mimeType?.startsWith("text/") == true) ) { resolvedPlaintext = charSequenceExtra - handleResolvedMedia(intent) + handleResolvedMedia() } else if (uris.isNotEmpty()) { _uiState.update { it.copy(showLoader = true) } - resolveMedia(intent, uris) + resolveMedia(uris) } else { _uiState.update { it.copy(showLoader = false) } } } - private fun handleResolvedMedia(intent: Intent) { - val address = IntentCompat.getParcelableExtra(intent, ShareActivity.EXTRA_ADDRESS, Address::class.java) + private fun handleResolvedMedia() { + val address = shareDestination if (address is Address.Conversable) { createConversation(address) } else { @@ -204,22 +217,46 @@ class ShareViewModel @Inject constructor( } } - private fun resolveMedia(intent: Intent, uris: List){ + private fun resolveMedia(uris: List){ viewModelScope.launch(Dispatchers.Default){ resolvedExtras = uris.mapNotNull { processSingleUri(it) } - handleResolvedMedia(intent) + handleResolvedMedia() + } + } + + /** + * Whether a URI offered by whoever sent the share Intent may be opened on their behalf. + */ + @VisibleForTesting + internal fun canReadSharedUri(uri: Uri): Boolean { + // A URI grant is what makes the sender's content readable to us, and only content:// carries + // one. openInputStream also accepts file:// and android.resource://, both of which it opens + // as this app with nothing consulted, so anything this app can reach would be readable by + // whoever sent the Intent. + if (ContentResolver.SCHEME_CONTENT != uri.scheme) { + Log.w(TAG, "Refusing a shared URI that carries no content grant.") + return false + } + + // Our own providers answer us whether or not they are exported, so these resolve to our own + // data rather than to anything the sender holds. That covers the attachment and blob + // providers, and equally our FileProvider, whose configured roots include the cache + // directory and external storage. + if (PartAuthority.isLocalUri(uri) || FileProviderUtil.AUTHORITY == uri.authority) { + Log.w(TAG, "Refusing a shared URI that names one of our own providers.") + return false } + + return true } private fun processSingleUri(uri: Uri): Uri? { try { Log.i(TAG, "Resolving URI: " + uri.toString() + " - " + uri.path) - val inputStream = if ("file" == uri.scheme) { - FileInputStream(uri.path) - } else { - context.contentResolver.openInputStream(uri) - } + if (!canReadSharedUri(uri)) return null + + val inputStream = context.contentResolver.openInputStream(uri) if (inputStream == null) { Log.w(TAG, "Failed to create input stream during ShareActivity - bailing.") diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt index e7d2bbcd7e..83c559c4d3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt @@ -1109,9 +1109,9 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate, } else { prepMediaForSending(mediaURI, mediaType).addListener(object : ListenableFuture.Listener { - override fun onSuccess(result: Boolean?) { - sendAttachments(attachmentManager.buildSlideDeck().asAttachments(), null) - } + // Nothing to do on success: prepMediaForSending stages the attachment in the input + // bar, and the send is the user's to make once they can see what they shared. + override fun onSuccess(result: Boolean?) {} override fun onFailure(e: ExecutionException?) { Toast.makeText(this@ConversationActivityV2, R.string.attachmentsErrorLoad, Toast.LENGTH_LONG).show() @@ -2370,7 +2370,14 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate, viewModel.beforeSendMessage() - if (binding.inputBar.linkPreview != null || binding.inputBar.quote != null) { + if (attachmentManager.isAttachmentPresent()) { + sendAttachments( + attachmentManager.buildSlideDeck().asAttachments(), + getMessageBody(), + binding.inputBar.quote, + binding.inputBar.linkPreview + ) + } else if (binding.inputBar.linkPreview != null || binding.inputBar.quote != null) { sendAttachments(listOf(), getMessageBody(), binding.inputBar.quote, binding.inputBar.linkPreview) } else { sendTextOnlyMessage() @@ -2612,7 +2619,16 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate, ) } - override fun onAttachmentChanged() { /* Do nothing */ } + override fun onAttachmentChanged() { + val slide = attachmentManager.getSlide() + if (slide != null) binding.inputBar.showAttachmentDraft(glide, slide) + else binding.inputBar.clearAttachmentDraft() + } + + override fun cancelAttachmentDraft() { + attachmentManager.clear() + if (isShowingAttachmentOptions) { toggleAttachmentOptions() } + } override fun onRequestPermissionsResult(requestCode: Int, permissions: Array, grantResults: IntArray) { super.onRequestPermissionsResult(requestCode, permissions, grantResults) @@ -2633,18 +2649,12 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate, // If the attachment was too large or MediaConstraints.isSatisfied failed for some // other reason then we reset the attachment manager & shown buttons then bail.. + // Otherwise it is left staged in the input bar, so that the user can see what they + // picked, add a message to it, and choose to send. if (!result) { attachmentManager.clear() if (isShowingAttachmentOptions) { toggleAttachmentOptions() } - return } - - // ..otherwise we can attempt to send the attachment(s). - // Note: The only multi-attachment message type is when sending images - all others - // attempt send the attachment immediately upon file selection. - sendAttachments(attachmentManager.buildSlideDeck().asAttachments(), null) - //todo: The current system sends the document the moment it has been selected, without text (body is set to null above) - We will want to fix this and allow the user to add text with a document AND be able to confirm before sending - //todo: Simply setting body to getMessageBody() above isn't good enough as it doesn't give the user a chance to confirm their message before sending it. } override fun onFailure(e: ExecutionException?) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/components/AttachmentDraftView.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/components/AttachmentDraftView.kt new file mode 100644 index 0000000000..cba8ee53a9 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/components/AttachmentDraftView.kt @@ -0,0 +1,52 @@ +package org.thoughtcrime.securesms.conversation.v2.components + +import android.content.Context +import android.text.format.Formatter +import android.util.AttributeSet +import android.view.LayoutInflater +import android.widget.LinearLayout +import androidx.core.view.isVisible +import com.bumptech.glide.RequestManager +import network.loki.messenger.R +import network.loki.messenger.databinding.ViewAttachmentDraftBinding +import org.thoughtcrime.securesms.mms.ImageSlide +import org.thoughtcrime.securesms.mms.Slide +import org.thoughtcrime.securesms.util.toPx + +/** + * The attachment waiting in the input bar to be sent, with the means to drop it again. + */ +class AttachmentDraftView : LinearLayout { + private lateinit var binding: ViewAttachmentDraftBinding + var delegate: AttachmentDraftViewDelegate? = null + + constructor(context: Context) : super(context) { initialize() } + constructor(context: Context, attrs: AttributeSet) : super(context, attrs) { initialize() } + constructor(context: Context, attrs: AttributeSet, defStyleAttr: Int) : super(context, attrs, defStyleAttr) { initialize() } + + private fun initialize() { + binding = ViewAttachmentDraftBinding.inflate(LayoutInflater.from(context), this, true) + binding.attachmentDraftThumbnail.root.clipToOutline = true + binding.attachmentDraftCancelButton.contentDescription = context.getString(R.string.remove) + binding.attachmentDraftCancelButton.setOnClickListener { delegate?.cancelAttachmentDraft() } + } + + fun update(glide: RequestManager, slide: Slide) { + binding.attachmentDraftFilenameTextView.text = slide.filename + binding.attachmentDraftFileSizeTextView.text = Formatter.formatFileSize(context, slide.fileSize) + + // A document carries a thumbnailUri as readily as a photo does, so hasImage() rather than the + // URI decides: without it the empty thumbnail view covers the file icon standing in for it. + val thumbnail = slide.thumbnailUri.takeIf { slide.hasImage() } + binding.attachmentDraftThumbnail.root.isVisible = thumbnail != null + if (thumbnail != null) { + binding.attachmentDraftThumbnail.root.setRoundedCorners(toPx(4, resources)) + binding.attachmentDraftThumbnail.root.setImageResource(glide, ImageSlide(context, thumbnail, slide.filename, slide.fileSize, 0, 0, null), false) + } + } +} + +interface AttachmentDraftViewDelegate { + + fun cancelAttachmentDraft() +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBar.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBar.kt index 84553fe26d..79bd6a3b7f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBar.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBar.kt @@ -26,9 +26,12 @@ import org.session.libsession.utilities.recipients.Recipient import org.thoughtcrime.securesms.InputbarViewModel import org.thoughtcrime.securesms.InputbarViewModel.InputBarContentState import org.thoughtcrime.securesms.conversation.v2.ViewUtil +import org.thoughtcrime.securesms.conversation.v2.components.AttachmentDraftView +import org.thoughtcrime.securesms.conversation.v2.components.AttachmentDraftViewDelegate import org.thoughtcrime.securesms.conversation.v2.components.LinkPreviewDraftView import org.thoughtcrime.securesms.conversation.v2.components.LinkPreviewDraftViewDelegate import org.thoughtcrime.securesms.conversation.v2.messages.QuoteView +import org.thoughtcrime.securesms.mms.Slide import org.thoughtcrime.securesms.conversation.v2.messages.QuoteViewDelegate import org.thoughtcrime.securesms.database.RecipientRepository import org.thoughtcrime.securesms.database.model.MessageRecord @@ -62,10 +65,12 @@ class InputBar @JvmOverloads constructor( ), InputBarEditTextDelegate, QuoteViewDelegate, LinkPreviewDraftViewDelegate, + AttachmentDraftViewDelegate, TextView.OnEditorActionListener { private var binding: ViewInputBarBinding = ViewInputBarBinding.inflate(LayoutInflater.from(context), this, true) private var linkPreviewDraftView: LinkPreviewDraftView? = null + private var attachmentDraftView: AttachmentDraftView? = null private var quoteView: QuoteView? = null var delegate: InputBarDelegate? = null var quote: MessageRecord? = null @@ -225,11 +230,18 @@ class InputBar @JvmOverloads constructor( } override fun inputBarEditTextContentChanged(text: CharSequence) { - microphoneButton.isVisible = text.trim().isEmpty() && !sendOnly - sendButton.isVisible = microphoneButton.isGone || sendOnly + updateMicrophoneOrSendButton() delegate?.inputBarEditTextContentChanged(text) } + // A staged attachment is sendable with no text at all, so the microphone has to give way to the + // send button for it as well - keying off the text alone strands the attachment with no way to send. + private fun updateMicrophoneOrSendButton() { + val hasSendableContent = text.trim().isNotEmpty() || attachmentDraftView != null + microphoneButton.isVisible = !hasSendableContent && !sendOnly + sendButton.isVisible = microphoneButton.isGone || sendOnly + } + override fun commitInputContent(contentUri: Uri) { delegate?.commitInputContent(contentUri) } private fun toggleAttachmentOptions() { delegate?.toggleAttachmentOptions() } @@ -301,6 +313,35 @@ class InputBar @JvmOverloads constructor( linkPreview = updatedLinkPreview.also { linkPreviewDraftView?.update(glide, it) } } + fun showAttachmentDraft(glide: RequestManager, slide: Slide) { + val existing = attachmentDraftView + if (existing != null) { + existing.update(glide, slide) + return + } + + attachmentDraftView = AttachmentDraftView(context).also { + it.delegate = this + it.update(glide, slide) + binding.inputBarAdditionalContentContainer.addView(it) + } + updateMicrophoneOrSendButton() + requestLayout() + } + + // Driven by the attachment manager rather than called directly: it owns whether an attachment + // exists, and clearing it reports back through onAttachmentChanged. + fun clearAttachmentDraft() { + attachmentDraftView?.let(binding.inputBarAdditionalContentContainer::removeView) + attachmentDraftView = null + updateMicrophoneOrSendButton() + requestLayout() + } + + override fun cancelAttachmentDraft() { + delegate?.cancelAttachmentDraft() + } + override fun cancelLinkPreviewDraft() { binding.inputBarAdditionalContentContainer.removeView(linkPreviewDraftView) linkPreview = null @@ -407,5 +448,6 @@ interface InputBarDelegate { fun onMicrophoneButtonUp(event: MotionEvent) fun sendMessage() fun commitInputContent(contentUri: Uri) + fun cancelAttachmentDraft() {} // no-op by default: only a bar backed by an attachment manager has one fun onCharLimitTapped() } diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/utilities/AttachmentManager.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/utilities/AttachmentManager.java index c2961675fc..5e1f6eb2d9 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/utilities/AttachmentManager.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/utilities/AttachmentManager.java @@ -225,6 +225,14 @@ protected void onPostExecute(@Nullable final Slide slide) { return result; } + public boolean isAttachmentPresent() { + return slide != null; + } + + public @Nullable Slide getSlide() { + return slide; + } + public @NonNull SlideDeck buildSlideDeck() { SlideDeck deck = new SlideDeck(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/giph/ui/GiphyActivity.kt b/app/src/main/java/org/thoughtcrime/securesms/giph/ui/GiphyActivity.kt index 6e1a747bbc..f4704774cb 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/giph/ui/GiphyActivity.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/giph/ui/GiphyActivity.kt @@ -21,6 +21,7 @@ import org.session.libsignal.utilities.Log import org.thoughtcrime.securesms.ScreenLockActionBarActivity import org.thoughtcrime.securesms.giph.ui.compose.GiphyTabsCompose import org.thoughtcrime.securesms.providers.BlobUtils +import org.thoughtcrime.securesms.util.FilenameUtils import org.thoughtcrime.securesms.ui.setThemedContent class GiphyActivity : @@ -113,6 +114,9 @@ class GiphyActivity : BlobUtils.getInstance() .forData(data) .withMimeType(MediaTypes.IMAGE_GIF) + // Giphy gives us bytes and no name, and a blob without one is named "null" + // verbatim - which the attachment then carries to the recipient. + .withFileName(FilenameUtils.getFilenameFromUri(this@GiphyActivity, null, MediaTypes.IMAGE_GIF)) .createForSingleSessionOnDisk( this@GiphyActivity ) { e -> Log.w(TAG, "Failed to write to disk.", e) } diff --git a/app/src/main/java/org/thoughtcrime/securesms/service/DirectShareService.java b/app/src/main/java/org/thoughtcrime/securesms/service/DirectShareService.java index b8ea438ebc..5c487d2410 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/service/DirectShareService.java +++ b/app/src/main/java/org/thoughtcrime/securesms/service/DirectShareService.java @@ -19,6 +19,7 @@ import org.session.libsession.utilities.recipients.RecipientNamesKt; import org.session.libsignal.utilities.Log; import org.thoughtcrime.securesms.ShareActivity; +import org.thoughtcrime.securesms.ShareIntentTokenStore; import org.thoughtcrime.securesms.database.RecipientRepository; import org.thoughtcrime.securesms.database.model.ThreadRecord; import org.thoughtcrime.securesms.repository.ConversationRepository; @@ -46,6 +47,9 @@ public class DirectShareService extends ChooserTargetService { @Inject ConversationRepository conversationRepository; + @Inject + ShareIntentTokenStore shareIntentTokenStore; + private static final String TAG = DirectShareService.class.getSimpleName(); @Override @@ -79,8 +83,7 @@ public List onGetChooserTargets(ComponentName targetActivityName, } Bundle bundle = new Bundle(1); - bundle.putParcelable(ShareActivity.EXTRA_ADDRESS, recipient.getAddress()); - bundle.setClassLoader(getClassLoader()); + bundle.putString(ShareActivity.EXTRA_SHARE_TOKEN, shareIntentTokenStore.mint(recipient.getAddress())); results.add(new ChooserTarget(RecipientNamesKt.displayName(recipient), Icon.createWithBitmap(avatar), 1.0f, componentName, bundle)); } diff --git a/app/src/main/res/layout/view_attachment_draft.xml b/app/src/main/res/layout/view_attachment_draft.xml new file mode 100644 index 0000000000..e8c3f5efdf --- /dev/null +++ b/app/src/main/res/layout/view_attachment_draft.xml @@ -0,0 +1,81 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/app/src/test/java/org/thoughtcrime/securesms/ShareIntentCacheFilenameTest.kt b/app/src/test/java/org/thoughtcrime/securesms/ShareIntentCacheFilenameTest.kt new file mode 100644 index 0000000000..5d56315321 --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/ShareIntentCacheFilenameTest.kt @@ -0,0 +1,72 @@ +package org.thoughtcrime.securesms + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.io.File + +/** + * The filename `ScreenLockActionBarActivity` caches a shared file under is the sending app's + * `OpenableColumns.DISPLAY_NAME`, verbatim and unvalidated. + */ +@RunWith(RobolectricTestRunner::class) +class ShareIntentCacheFilenameTest { + + private val cacheDir = ApplicationProvider.getApplicationContext().cacheDir + + private fun cachedFileFor(displayName: String): File? = + cacheFilenameFrom(displayName)?.let { File(cacheDir, it) } + + @Test + fun `a traversing display name lands in the cache directory`() { + val cached = cachedFileFor("../../outside.xml") + + assertThat(cached).isNotNull() + assertThat(cached!!.name).isEqualTo("outside.xml") + assertThat(cached.canonicalFile.parentFile).isEqualTo(cacheDir.canonicalFile) + } + + @Test + fun `no display name can land outside the cache directory`() { + val displayNames = listOf( + "../../outside.xml", + "../outside.xml", + "a/b/../../../../outside.xml", + "/an/absolute/path/outside.xml", + "subdir/outside.xml", + "foo/", + "..", + ".", + "", + "./../outside.xml", + ) + + displayNames.forEach { displayName -> + val cached = cachedFileFor(displayName) + + if (cached != null) { + assertThat(cached.canonicalFile.parentFile).isEqualTo(cacheDir.canonicalFile) + } + } + } + + @Test + fun `an ordinary display name is kept as it is`() { + assertThat(cacheFilenameFrom("cat.jpeg")).isEqualTo("cat.jpeg") + assertThat(cacheFilenameFrom("holiday photo (1).png")).isEqualTo("holiday photo (1).png") + assertThat(cacheFilenameFrom("..leading-dots.pdf")).isEqualTo("..leading-dots.pdf") + assertThat(cacheFilenameFrom("report.pdf/")).isEqualTo("report.pdf") + } + + @Test + fun `a display name that reduces to no filename at all is refused`() { + assertThat(cacheFilenameFrom("")).isNull() + assertThat(cacheFilenameFrom(".")).isNull() + assertThat(cacheFilenameFrom("..")).isNull() + assertThat(cacheFilenameFrom("../..")).isNull() + assertThat(cacheFilenameFrom("/")).isNull() + } +} diff --git a/app/src/test/java/org/thoughtcrime/securesms/ShareIntentTokenStoreTest.kt b/app/src/test/java/org/thoughtcrime/securesms/ShareIntentTokenStoreTest.kt new file mode 100644 index 0000000000..afe13305f3 --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/ShareIntentTokenStoreTest.kt @@ -0,0 +1,87 @@ +package org.thoughtcrime.securesms + +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import android.net.Uri +import org.session.libsession.utilities.Address.Companion.toAddress + +@RunWith(RobolectricTestRunner::class) +class ShareIntentTokenStoreTest { + + private val store = ShareIntentTokenStore() + + private val address = "05${"1".repeat(64)}".toAddress() + + @Test + fun `resolves a token it minted, with the address it was minted for`() { + val token = store.mint(address) + + assertThat(store.resolve(token)?.address).isEqualTo(address) + } + + @Test + fun `a token authorises only the uris it was minted for`() { + val authorised = Uri.parse("content://network.loki.provider.securesms/part/1/2") + val other = Uri.parse("content://network.loki.provider.securesms/part/1/3") + + val minted = store.resolve(store.mint(authorisedUris = setOf(authorised)))!! + + assertThat(minted.authorises(authorised)).isTrue() + assertThat(minted.authorises(other)).isFalse() + } + + @Test + fun `a token minted for a destination authorises no uris`() { + val minted = store.resolve(store.mint(address))!! + + assertThat(minted.authorisedUris).isEmpty() + assertThat(minted.authorises(Uri.parse("content://network.loki.provider.securesms/part/1/2"))).isFalse() + } + + @Test + fun `resolves a token minted without an address`() { + val token = store.mint() + + val minted = store.resolve(token) + + assertThat(minted).isNotNull() + assertThat(minted?.address).isNull() + } + + @Test + fun `does not resolve a token it did not mint`() { + store.mint(address) + + assertThat(store.resolve("not-a-token-this-store-issued")).isNull() + assertThat(store.resolve("")).isNull() + assertThat(store.resolve(null)).isNull() + } + + @Test + fun `mints a distinct unguessable token each time`() { + val tokens = List(500) { store.mint(address) } + + assertThat(tokens.toSet()).hasSize(tokens.size) + tokens.forEach { assertThat(it.length).isAtLeast(32) } + } + + @Test + fun `a token stays resolvable across repeated lookups`() { + // App lock creates ShareActivity twice for one share - once before it routes to the lock + // screen, once from the Intent the lock screen replays - and each resolves independently. + val token = store.mint(address) + + repeat(3) { assertThat(store.resolve(token)?.address).isEqualTo(address) } + } + + @Test + fun `retention is bounded, oldest first`() { + val first = store.mint(address) + + repeat(1_000) { store.mint(address) } + + assertThat(store.resolve(first)).isNull() + } +} diff --git a/app/src/test/java/org/thoughtcrime/securesms/ShareViewModelTest.kt b/app/src/test/java/org/thoughtcrime/securesms/ShareViewModelTest.kt new file mode 100644 index 0000000000..79d647f4dc --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/ShareViewModelTest.kt @@ -0,0 +1,184 @@ +package org.thoughtcrime.securesms + +import android.content.Context +import android.content.Intent +import android.net.Uri +import androidx.test.core.app.ApplicationProvider +import app.cash.turbine.test +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.runTest +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.mock +import org.robolectric.RobolectricTestRunner +import org.session.libsession.messaging.groups.LegacyGroupDeprecationManager +import org.session.libsession.messaging.sending_receiving.attachments.AttachmentId +import org.session.libsession.utilities.Address +import org.thoughtcrime.securesms.mms.PartAuthority +import org.thoughtcrime.securesms.providers.BlobUtils +import org.thoughtcrime.securesms.repository.ConversationRepository +import org.thoughtcrime.securesms.util.AvatarUtils +import org.thoughtcrime.securesms.util.FileProviderUtil +import java.io.File + +@RunWith(RobolectricTestRunner::class) +class ShareViewModelTest : BaseViewModelTest() { + + @OptIn(ExperimentalCoroutinesApi::class) + @get:Rule + val mainCoroutineRule = MainCoroutineRule() + + private val context = ApplicationProvider.getApplicationContext() + + private val tokenStore = ShareIntentTokenStore() + + private val recipient: Address.Conversable = + Address.fromSerialized("0538e63512fd78c04d45b83ec7f0f3d593f60276ce535d1160eb589a00cca7db59") + as Address.Conversable + + private val viewModel = ShareViewModel( + context = context, + avatarUtils = mock(), + deprecationManager = mock(), + shareIntentTokenStore = tokenStore, + conversationRepository = mock { + on { observeConversationList() } doReturn flowOf(emptyList()) + }, + ) + + // One of our own blob URIs, shaped as BlobUtils mints them: blob///// + private val ownBlobUri: Uri = BlobUtils.CONTENT_URI.buildUpon() + .appendPath("multi-session-disk") + .appendPath("text/plain") + .appendPath("note.txt") + .appendPath("12") + .appendPath("11111111-2222-3333-4444-555555555555") + .build() + + private fun sharedTextIntent() = Intent(Intent.ACTION_SEND) + .setType("text/plain") + .putExtra(Intent.EXTRA_TEXT, "hello") + + private fun sharedUriIntent(uri: Uri) = Intent(Intent.ACTION_SEND) + .setType("text/plain") + .putExtra(Intent.EXTRA_STREAM, uri) + + // --- What may be opened on the sender's behalf ------------------------------------------------ + + @Test + fun `refuses a file uri, including one naming a file of ours that exists`() { + val real = File(context.filesDir, "fixture.txt").apply { + parentFile?.mkdirs() + writeText("synthetic fixture content") + } + assertThat(real.exists()).isTrue() // positive control: the path is readable to this process + + assertThat(viewModel.canReadSharedUri(Uri.fromFile(real))).isFalse() + assertThat(viewModel.canReadSharedUri(Uri.parse("file:///storage/emulated/0/Download/example.txt"))).isFalse() + } + + @Test + fun `refuses schemes that openInputStream would serve without a grant`() { + assertThat(viewModel.canReadSharedUri(Uri.parse("android.resource://com.example.app/raw/1"))).isFalse() + } + + @Test + fun `refuses a content uri naming one of our own providers`() { + val attachment = AttachmentId(rowId = 42, uniqueId = 1700000000000) + + assertThat(viewModel.canReadSharedUri(ownBlobUri)).isFalse() + assertThat(viewModel.canReadSharedUri(PartAuthority.getAttachmentDataUri(attachment))).isFalse() + assertThat(viewModel.canReadSharedUri(PartAuthority.getAttachmentThumbnailUri(attachment))).isFalse() + } + + @Test + fun `accepts a content uri from another app's provider`() { + assertThat(viewModel.canReadSharedUri(Uri.parse("content://com.example.documents/document/12"))).isTrue() + assertThat(viewModel.canReadSharedUri(Uri.parse("content://media/external/images/media/7"))).isTrue() + } + + // --- Where the share is allowed to go --------------------------------------------------------- + + @Test + fun `an address supplied by the caller does not choose the conversation`() = runTest { + val intent = sharedTextIntent().putExtra(ShareActivity.EXTRA_SHARE_TOKEN, "forged") + .putExtra("address", recipient as Address) + + viewModel.uiEvents.test { + viewModel.initialiseMedia(intent) + + expectNoEvents() + } + assertThat(viewModel.uiState.value.showLoader).isFalse() + } + + @Test + fun `a minted token chooses the conversation it was minted for`() = runTest { + val intent = sharedTextIntent() + .putExtra(ShareActivity.EXTRA_SHARE_TOKEN, tokenStore.mint(recipient)) + + viewModel.uiEvents.test { + viewModel.initialiseMedia(intent) + + val event = awaitItem() as ShareViewModel.ShareUIEvent.GoToScreen + assertThat(event.intent.getStringExtra(Intent.EXTRA_TEXT)).isEqualTo("hello") + } + } + + // --- Passing one of our own attachments along ------------------------------------------------- + + @Test + fun `one of our own uris is not passed along for an intent we did not build`() = runTest { + val intent = sharedUriIntent(ownBlobUri).putExtra("address", recipient as Address) + + viewModel.uiEvents.test { + viewModel.initialiseMedia(intent) + + expectNoEvents() + } + // Nothing was staged, so there is nothing for onPause to tidy up either. + assertThat(viewModel.onPause()).isFalse() + } + + // The chooser merges a direct-share target's extras into the sender's own Intent, so a token we + // minted arrives attached to URIs the sender chose. The token carries a destination here, so the + // pass-through gate is the only thing that can stop it - without it this reaches the conversation. + @Test + fun `a token does not authorise uris it was not minted for`() = runTest { + val intent = sharedUriIntent(ownBlobUri) + .putExtra(ShareActivity.EXTRA_SHARE_TOKEN, tokenStore.mint(recipient)) + + viewModel.uiEvents.test { + viewModel.initialiseMedia(intent) + + expectNoEvents() + } + assertThat(viewModel.onPause()).isFalse() + } + + @Test + fun `one of our own uris is passed along when the token was minted for it`() = runTest { + val intent = sharedUriIntent(ownBlobUri).putExtra( + ShareActivity.EXTRA_SHARE_TOKEN, + tokenStore.mint(recipient, authorisedUris = setOf(ownBlobUri)) + ) + + viewModel.uiEvents.test { + viewModel.initialiseMedia(intent) + + val event = awaitItem() as ShareViewModel.ShareUIEvent.GoToScreen + assertThat(event.intent.data).isEqualTo(ownBlobUri) + } + } + + @Test + fun `refuses a content uri naming our own FileProvider`() { + val ours = Uri.parse("content://${FileProviderUtil.AUTHORITY}/internal_cache/example.txt") + + assertThat(viewModel.canReadSharedUri(ours)).isFalse() + } +}