Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix file access permission #1022

Merged
merged 10 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class AudioPicker : Picker<MultiPickerAudioType>() {
*/
override fun getSelectedFiles(context: Context, data: Intent?): List<MultiPickerAudioType> {
return getSelectedUriList(data).mapNotNull { selectedUri ->
// Tchap: Grant permission to access the selected file.
context.grantUriPermission(context.applicationContext.packageName, selectedUri, Intent.FLAG_GRANT_READ_URI_PERMISSION)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourrais être mutualisé dans la classe parente multipicker/Picker::getSelectedUriList

selectedUri.toMultiPickerAudioType(context)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class CameraPicker {
val photoUri = createPhotoUri(context)
val intent = createIntent().apply {
putExtra(MediaStore.EXTRA_OUTPUT, photoUri)
addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION)
}
activityResultLauncher.launch(intent)
return photoUri
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ class ContactPicker : Picker<MultiPickerContactType>() {
val contactList = mutableListOf<MultiPickerContactType>()

data?.data?.let { selectedUri ->
// Tchap: Grant permission to access the selected URI.
context.grantUriPermission(context.applicationContext.packageName, selectedUri, Intent.FLAG_GRANT_READ_URI_PERMISSION)

val projection = arrayOf(
ContactsContract.Contacts.DISPLAY_NAME,
ContactsContract.Contacts.PHOTO_URI,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ class FilePicker : Picker<MultiPickerBaseType>() {
*/
override fun getSelectedFiles(context: Context, data: Intent?): List<MultiPickerBaseType> {
return getSelectedUriList(data).mapNotNull { selectedUri ->
// Tchap: Grant permission to access the selected file.
context.grantUriPermission(context.applicationContext.packageName, selectedUri, Intent.FLAG_GRANT_READ_URI_PERMISSION)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourrais être mutualisé dans la classe parente multipicker/Picker::getSelectedUriList


val type = context.contentResolver.getType(selectedUri)

when {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class ImagePicker : Picker<MultiPickerImageType>() {
*/
override fun getSelectedFiles(context: Context, data: Intent?): List<MultiPickerImageType> {
return getSelectedUriList(data).mapNotNull { selectedUri ->
// Tchap: Grant permission to access the selected file.
context.grantUriPermission(context.applicationContext.packageName, selectedUri, Intent.FLAG_GRANT_READ_URI_PERMISSION)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourrais être mutualisé dans la classe parente multipicker/Picker::getSelectedUriList

selectedUri.toMultiPickerImageType(context)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ class MediaPicker : Picker<MultiPickerBaseMediaType>() {
*/
override fun getSelectedFiles(context: Context, data: Intent?): List<MultiPickerBaseMediaType> {
return getSelectedUriList(data).mapNotNull { selectedUri ->
// Tchap: Grant permission to access the selected file.
context.grantUriPermission(context.applicationContext.packageName, selectedUri, Intent.FLAG_GRANT_READ_URI_PERMISSION)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourrais être mutualisé dans la classe parente multipicker/Picker::getSelectedUriList


val mimeType = context.contentResolver.getType(selectedUri)

if (mimeType.isMimeTypeVideo()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package im.vector.lib.multipicker

import android.content.ComponentName
import android.content.Context
import android.content.Intent
import android.content.pm.PackageManager
Expand Down Expand Up @@ -58,7 +59,15 @@ abstract class Picker<T> {
uriList.forEach {
for (resolveInfo in resInfoList) {
val packageName: String = resolveInfo.activityInfo.packageName
context.grantUriPermission(packageName, it, Intent.FLAG_GRANT_READ_URI_PERMISSION)
// Tchap: Replace implicit intent by an explicit to fix crash on some devices like Xiaomi.
try {
context.grantUriPermission(packageName, it, Intent.FLAG_GRANT_READ_URI_PERMISSION)
} catch (e: Exception) {
continue
}
data.action = null
data.component = ComponentName(packageName, resolveInfo.activityInfo.name)
break
}
}
return getSelectedFiles(context, data)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class VideoPicker : Picker<MultiPickerVideoType>() {
*/
override fun getSelectedFiles(context: Context, data: Intent?): List<MultiPickerVideoType> {
return getSelectedUriList(data).mapNotNull { selectedUri ->
// Tchap: Grant permission to access the selected file.
context.grantUriPermission(context.applicationContext.packageName, selectedUri, Intent.FLAG_GRANT_READ_URI_PERMISSION)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourrais être mutualisé dans la classe parente multipicker/Picker::getSelectedUriList

selectedUri.toMultiPickerVideoType(context)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
package org.matrix.android.sdk.internal.session.content

import android.content.Context
import android.content.Intent
import android.graphics.BitmapFactory
import android.media.MediaMetadataRetriever
import android.os.Build
import androidx.core.net.toUri
import androidx.work.WorkerParameters
import com.squareup.moshi.JsonClass
Expand Down Expand Up @@ -115,7 +117,16 @@ internal class UploadContentWorker(val context: Context, params: WorkerParameter
if (allCancelled) {
// there is no point in uploading the image!
return Result.success(inputData)
.also { Timber.e("## Send: Work cancelled by user") }
.also {
Timber.e("## Send: Work cancelled by user")

// Tchap: Revoke read permission to the local file.
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
context.revokeUriPermission(context.packageName, params.attachment.queryUri, Intent.FLAG_GRANT_READ_URI_PERMISSION)
} else {
context.revokeUriPermission(params.attachment.queryUri, Intent.FLAG_GRANT_READ_URI_PERMISSION)
}
}
}

val attachment = params.attachment
Expand Down Expand Up @@ -396,6 +407,13 @@ internal class UploadContentWorker(val context: Context, params: WorkerParameter
)
return Result.success(WorkerParamsFactory.toData(sendParams)).also {
Timber.v("## handleSuccess $attachmentUrl, work is stopped $isStopped")

// Tchap: Revoke read permission to the local file.
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
context.revokeUriPermission(context.packageName, params.attachment.queryUri, Intent.FLAG_GRANT_READ_URI_PERMISSION)
} else {
context.revokeUriPermission(params.attachment.queryUri, Intent.FLAG_GRANT_READ_URI_PERMISSION)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ sealed class RoomDetailAction : VectorViewModelAction {

data class ResendMessage(val eventId: String) : RoomDetailAction()
data class RemoveFailedEcho(val eventId: String) : RoomDetailAction()
data class CancelSend(val eventId: String, val force: Boolean) : RoomDetailAction()
data class CancelSend(val event: TimelineEvent, val force: Boolean) : RoomDetailAction()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ajouter un Tchap commentaire

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

je préfère ne pas en mettre. le lint oblige d'ajouter un saut de ligne au dessus du commentaire. ca rajoute de la complexité inutile en cas de conflit.


data class VoteToPoll(val eventId: String, val optionKey: String) : RoomDetailAction()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ sealed class RoomDetailViewEvents : VectorViewEvents {
val mimeType: String?
) : RoomDetailViewEvents()

// Tchap: Revoke read permission to the local file.
data class RevokeFilePermission(
val uri: Uri
) : RoomDetailViewEvents()

data class DisplayAndAcceptCall(val call: WebRtcCall) : RoomDetailViewEvents()

object DisplayPromptForIntegrationManager : RoomDetailViewEvents()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ class TimelineFragment :

timelineViewModel.observeViewEvents {
when (it) {
is RoomDetailViewEvents.RevokeFilePermission -> revokeFilePermission(it)
is RoomDetailViewEvents.SendCallFeedback -> bugReporter.openBugReportScreen(requireActivity(), ReportType.VOIP)
is RoomDetailViewEvents.Failure -> displayErrorMessage(it)
is RoomDetailViewEvents.OnNewTimelineEvents -> scrollOnNewMessageCallback.addNewTimelineEventIds(it.eventIds)
Expand Down Expand Up @@ -547,6 +548,22 @@ class TimelineFragment :
)
}

// Tchap: Revoke read permission to the local file.
private fun revokeFilePermission(revokeFilePermission: RoomDetailViewEvents.RevokeFilePermission) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
requireContext().revokeUriPermission(
requireContext().applicationContext.packageName,
revokeFilePermission.uri,
Intent.FLAG_GRANT_READ_URI_PERMISSION
)
} else {
requireContext().revokeUriPermission(
revokeFilePermission.uri,
Intent.FLAG_GRANT_READ_URI_PERMISSION
)
}
}

private fun displayErrorMessage(error: RoomDetailViewEvents.Failure) {
if (error.showInDialog) displayErrorDialog(error.throwable) else showErrorInSnackbar(error.throwable)
}
Expand Down Expand Up @@ -1578,14 +1595,16 @@ class TimelineFragment :

private fun handleCancelSend(action: EventSharedAction.Cancel) {
if (action.force) {
timelineViewModel.handle(RoomDetailAction.CancelSend(action.eventId, true))
// Tchap: Revoke read permission to the local file.
timelineViewModel.handle(RoomDetailAction.CancelSend(action.event, true))
} else {
MaterialAlertDialogBuilder(requireContext())
.setTitle(R.string.dialog_title_confirmation)
.setMessage(getString(R.string.event_status_cancel_sending_dialog_message))
.setNegativeButton(R.string.no, null)
.setPositiveButton(R.string.yes) { _, _ ->
timelineViewModel.handle(RoomDetailAction.CancelSend(action.eventId, false))
// Tchap: Revoke read permission to the local file.
timelineViewModel.handle(RoomDetailAction.CancelSend(action.event, false))
}
.show()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package im.vector.app.features.home.room.detail

import android.net.Uri
import androidx.annotation.IdRes
import androidx.core.net.toUri
import androidx.lifecycle.asFlow
import com.airbnb.mvrx.Async
import com.airbnb.mvrx.Fail
Expand Down Expand Up @@ -85,6 +86,7 @@ import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import org.matrix.android.sdk.api.MatrixPatterns
import org.matrix.android.sdk.api.MatrixUrls.isMxcUrl
import org.matrix.android.sdk.api.extensions.orFalse
import org.matrix.android.sdk.api.extensions.tryOrNull
import org.matrix.android.sdk.api.query.QueryStringValue
Expand Down Expand Up @@ -112,6 +114,8 @@ import org.matrix.android.sdk.api.session.room.model.Membership
import org.matrix.android.sdk.api.session.room.model.RoomMemberSummary
import org.matrix.android.sdk.api.session.room.model.RoomSummary
import org.matrix.android.sdk.api.session.room.model.localecho.RoomLocalEcho
import org.matrix.android.sdk.api.session.room.model.message.MessageContent
import org.matrix.android.sdk.api.session.room.model.message.MessageWithAttachmentContent
import org.matrix.android.sdk.api.session.room.model.message.getFileUrl
import org.matrix.android.sdk.api.session.room.model.relation.RelationDefaultContent
import org.matrix.android.sdk.api.session.room.model.tombstone.RoomTombstoneContent
Expand Down Expand Up @@ -1097,18 +1101,18 @@ class TimelineViewModel @AssistedInject constructor(

private fun handleCancel(action: RoomDetailAction.CancelSend) {
if (room == null) return
if (action.force) {
room.sendService().cancelSend(action.eventId)
return
}
val targetEventId = action.eventId
room.getTimelineEvent(targetEventId)?.let {
// State must be in one of the sending states
if (!it.root.sendState.isSending()) {
Timber.e("Cannot cancel message, it is not sending")
return
// Tchap: Revoke read permission to the local file.
// State must be in one of the sending states
if (action.force || action.event.root.sendState.isSending()) {
room.sendService().cancelSend(action.event.eventId)

val clearContent = action.event.root.getClearContent()
val messageContent = clearContent?.toModel<MessageContent>() as? MessageWithAttachmentContent
messageContent?.getFileUrl()?.takeIf { !it.isMxcUrl() }?.let {
_viewEvents.post(RoomDetailViewEvents.RevokeFilePermission(it.toUri()))
}
room.sendService().cancelSend(targetEventId)
} else {
Timber.e("Cannot cancel message, it is not sending")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import im.vector.app.core.platform.VectorSharedAction
import im.vector.app.features.home.room.detail.timeline.item.MessageInformationData
import org.matrix.android.sdk.api.session.room.model.message.MessageContent
import org.matrix.android.sdk.api.session.room.model.message.MessageWithAttachmentContent
import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent

sealed class EventSharedAction(
@StringRes val titleRes: Int,
Expand Down Expand Up @@ -71,7 +72,7 @@ sealed class EventSharedAction(
data class Redact(val eventId: String, val askForReason: Boolean, val dialogTitleRes: Int, val dialogDescriptionRes: Int) :
EventSharedAction(R.string.message_action_item_redact, R.drawable.ic_delete, true)

data class Cancel(val eventId: String, val force: Boolean) :
data class Cancel(val event: TimelineEvent, val force: Boolean) :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add Tchap commentaire

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

je préfère ne pas en mettre. le lint oblige d'ajouter un saut de ligne au dessus du commentaire. ca rajoute de la complexité inutile en cas de conflit.

EventSharedAction(R.string.action_cancel, R.drawable.ic_close_round)

data class ViewSource(val content: String) :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,15 +313,17 @@ class MessageActionsViewModel @AssistedInject constructor(
private fun ArrayList<EventSharedAction>.addActionsForSendingState(timelineEvent: TimelineEvent) {
// TODO is uploading attachment?
if (canCancel(timelineEvent)) {
add(EventSharedAction.Cancel(timelineEvent.eventId, false))
// Tchap: Revoke read permission to the local file.
add(EventSharedAction.Cancel(timelineEvent, false))
}
}

private fun ArrayList<EventSharedAction>.addActionsForSentNotSyncedState(timelineEvent: TimelineEvent) {
// If sent but not synced (synapse stuck at bottom bug)
// Still offer action to cancel (will only remove local echo)
timelineEvent.root.eventId?.let {
add(EventSharedAction.Cancel(it, true))
// Tchap: Revoke read permission to the local file.
add(EventSharedAction.Cancel(timelineEvent, true))
}

// TODO Can be redacted
Expand Down
Loading