From 7d2f0fa017d4c677728586ebb5b775fdec780bb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Brey=20Vilas?= Date: Thu, 10 Mar 2022 13:44:11 +0100 Subject: [PATCH] Make external storage permission optional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Álvaro Brey Vilas --- .../client/GrantStoragePermissionRule.kt | 5 +- .../client/preferences/AppPreferences.java | 4 + .../preferences/AppPreferencesImpl.java | 12 ++ .../android/datamodel/MediaProvider.java | 19 +-- .../ui/activity/FileDisplayActivity.java | 36 +---- .../ui/activity/SyncedFoldersActivity.kt | 47 ++++--- .../ui/activity/UploadFilesActivity.java | 45 +++--- .../owncloud/android/utils/PermissionUtil.kt | 132 ++++++++++++------ app/src/main/res/values/strings.xml | 1 + 9 files changed, 165 insertions(+), 136 deletions(-) diff --git a/app/src/androidTest/java/com/nextcloud/client/GrantStoragePermissionRule.kt b/app/src/androidTest/java/com/nextcloud/client/GrantStoragePermissionRule.kt index e1064e6f86..8b16204764 100644 --- a/app/src/androidTest/java/com/nextcloud/client/GrantStoragePermissionRule.kt +++ b/app/src/androidTest/java/com/nextcloud/client/GrantStoragePermissionRule.kt @@ -21,10 +21,10 @@ package com.nextcloud.client +import android.Manifest import android.os.Build import androidx.test.platform.app.InstrumentationRegistry import androidx.test.rule.GrantPermissionRule -import com.owncloud.android.utils.PermissionUtil import org.junit.rules.TestRule import org.junit.runner.Description import org.junit.runners.model.Statement @@ -35,8 +35,7 @@ class GrantStoragePermissionRule private constructor() { @JvmStatic fun grant(): TestRule = when { Build.VERSION.SDK_INT < Build.VERSION_CODES.R -> GrantPermissionRule.grant( - PermissionUtil - .getExternalStoragePermission() + Manifest.permission.WRITE_EXTERNAL_STORAGE ) else -> GrantManageExternalStoragePermissionRule() } diff --git a/app/src/main/java/com/nextcloud/client/preferences/AppPreferences.java b/app/src/main/java/com/nextcloud/client/preferences/AppPreferences.java index bd72625476..7266894c6e 100644 --- a/app/src/main/java/com/nextcloud/client/preferences/AppPreferences.java +++ b/app/src/main/java/com/nextcloud/client/preferences/AppPreferences.java @@ -373,4 +373,8 @@ public interface AppPreferences { void setPdfZoomTipShownCount(int count); int getPdfZoomTipShownCount(); + + boolean isStoragePermissionRequested(); + + void setStoragePermissionRequested(boolean value); } diff --git a/app/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java b/app/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java index 79f5ac8c23..6754c379c7 100644 --- a/app/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java +++ b/app/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java @@ -97,6 +97,8 @@ public final class AppPreferencesImpl implements AppPreferences { private static final String PREF__PDF_ZOOM_TIP_SHOWN = "pdf_zoom_tip_shown"; + private static final String PREF__STORAGE_PERMISSION_REQUESTED = "storage_permission_requested"; + private final Context context; private final SharedPreferences preferences; private final CurrentAccountProvider currentAccountProvider; @@ -696,6 +698,16 @@ public final class AppPreferencesImpl implements AppPreferences { return preferences.getInt(PREF__PDF_ZOOM_TIP_SHOWN, 0); } + @Override + public boolean isStoragePermissionRequested() { + return preferences.getBoolean(PREF__STORAGE_PERMISSION_REQUESTED, false); + } + + @Override + public void setStoragePermissionRequested(boolean value) { + preferences.edit().putBoolean(PREF__STORAGE_PERMISSION_REQUESTED, value).apply(); + } + @VisibleForTesting public int computeBruteForceDelay(int count) { return (int) Math.min(count / 3d, 10); diff --git a/app/src/main/java/com/owncloud/android/datamodel/MediaProvider.java b/app/src/main/java/com/owncloud/android/datamodel/MediaProvider.java index d72e7a6232..0586a1caed 100644 --- a/app/src/main/java/com/owncloud/android/datamodel/MediaProvider.java +++ b/app/src/main/java/com/owncloud/android/datamodel/MediaProvider.java @@ -28,11 +28,8 @@ import android.net.Uri; import android.provider.MediaStore; import android.util.Log; -import com.google.android.material.snackbar.Snackbar; import com.owncloud.android.MainApp; -import com.owncloud.android.R; import com.owncloud.android.utils.PermissionUtil; -import com.owncloud.android.utils.theme.ThemeSnackbarUtils; import java.io.File; import java.util.ArrayList; @@ -174,21 +171,7 @@ public final class MediaProvider { private static void checkPermissions(@Nullable Activity activity) { if (activity != null && !PermissionUtil.checkExternalStoragePermission(activity.getApplicationContext())) { - // Check if we should show an explanation - if (PermissionUtil - .shouldShowRequestPermissionRationale(activity, PermissionUtil.getExternalStoragePermission())) { - // Show explanation to the user and then request permission - Snackbar snackbar = Snackbar.make(activity.findViewById(R.id.ListLayout), - R.string.permission_storage_access, Snackbar.LENGTH_INDEFINITE) - .setAction(R.string.common_ok, v -> PermissionUtil.requestExternalStoragePermission(activity)); - - ThemeSnackbarUtils.colorSnackbar(activity.getApplicationContext(), snackbar); - - snackbar.show(); - } else { - // No explanation needed, request the permission. - PermissionUtil.requestExternalStoragePermission(activity); - } + PermissionUtil.requestExternalStoragePermission(activity); } } diff --git a/app/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.java b/app/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.java index 04d414ecb9..dd4ab336c7 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.java +++ b/app/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.java @@ -121,7 +121,6 @@ import com.owncloud.android.utils.PermissionUtil; import com.owncloud.android.utils.PushUtils; import com.owncloud.android.utils.StringUtils; import com.owncloud.android.utils.theme.ThemeButtonUtils; -import com.owncloud.android.utils.theme.ThemeSnackbarUtils; import com.owncloud.android.utils.theme.ThemeToolbarUtils; import org.greenrobot.eventbus.EventBus; @@ -316,22 +315,7 @@ public class FileDisplayActivity extends FileActivity super.onPostCreate(savedInstanceState); - if (!PermissionUtil.checkExternalStoragePermission(this)) { - // Check if we should show an explanation - if (PermissionUtil.shouldShowRequestPermissionRationale(this, - PermissionUtil.getExternalStoragePermission())) { - // Show explanation to the user and then request permission - Snackbar snackbar = Snackbar.make(binding.rootLayout, - R.string.permission_storage_access, - Snackbar.LENGTH_INDEFINITE) - .setAction(R.string.common_ok, v -> PermissionUtil.requestExternalStoragePermission(this)); - ThemeSnackbarUtils.colorSnackbar(this, snackbar); - snackbar.show(); - } else { - // No explanation needed, request the permission. - PermissionUtil.requestExternalStoragePermission(this); - } - } + PermissionUtil.requestExternalStoragePermission(this); if (getIntent().getParcelableExtra(OCFileListFragment.SEARCH_EVENT) != null) { switchToSearchFragment(savedInstanceState); @@ -399,7 +383,7 @@ public class FileDisplayActivity extends FileActivity public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { switch (requestCode) { - case PermissionUtil.PERMISSIONS_EXTERNAL_STORAGE: { + case PermissionUtil.PERMISSIONS_EXTERNAL_STORAGE: // If request is cancelled, result arrays are empty. if (grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) { @@ -407,24 +391,16 @@ public class FileDisplayActivity extends FileActivity EventBus.getDefault().post(new TokenPushEvent()); syncAndUpdateFolder(true); // toggle on is save since this is the only scenario this code gets accessed - } else { - // permission denied --> do nothing - return; } - return; - } - case PermissionUtil.PERMISSIONS_CAMERA: { + break; + case PermissionUtil.PERMISSIONS_CAMERA: // If request is cancelled, result arrays are empty. if (grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) { // permission was granted getFileOperationsHelper() .uploadFromCamera(this, FileDisplayActivity.REQUEST_CODE__UPLOAD_FROM_CAMERA); - } else { - // permission denied - return; } - return; - } + break; default: super.onRequestPermissionsResult(requestCode, permissions, grantResults); } @@ -856,6 +832,8 @@ public class FileDisplayActivity extends FileActivity }, DELAY_TO_REQUEST_OPERATIONS_LATER ); + } else if (requestCode == PermissionUtil.REQUEST_CODE_MANAGE_ALL_FILES) { + syncAndUpdateFolder(true); } else { super.onActivityResult(requestCode, resultCode, data); } diff --git a/app/src/main/java/com/owncloud/android/ui/activity/SyncedFoldersActivity.kt b/app/src/main/java/com/owncloud/android/ui/activity/SyncedFoldersActivity.kt index 2844e12dee..0f0a9e8bc3 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/SyncedFoldersActivity.kt +++ b/app/src/main/java/com/owncloud/android/ui/activity/SyncedFoldersActivity.kt @@ -514,24 +514,28 @@ class SyncedFoldersActivity : android.R.id.home -> finish() R.id.action_create_custom_folder -> { Log.d(TAG, "Show custom folder dialog") - val emptyCustomFolder = SyncedFolderDisplayItem( - SyncedFolder.UNPERSISTED_ID, - null, - null, - true, - false, - true, - false, - account.name, - FileUploader.LOCAL_BEHAVIOUR_FORGET, - NameCollisionPolicy.ASK_USER.serialize(), - false, - clock.currentTime, - null, - MediaFolderType.CUSTOM, - false - ) - onSyncFolderSettingsClick(0, emptyCustomFolder) + if (PermissionUtil.checkExternalStoragePermission(this)) { + val emptyCustomFolder = SyncedFolderDisplayItem( + SyncedFolder.UNPERSISTED_ID, + null, + null, + true, + false, + true, + false, + account.name, + FileUploader.LOCAL_BEHAVIOUR_FORGET, + NameCollisionPolicy.ASK_USER.serialize(), + false, + clock.currentTime, + null, + MediaFolderType.CUSTOM, + false + ) + onSyncFolderSettingsClick(0, emptyCustomFolder) + } else { + PermissionUtil.requestExternalStoragePermission(this, true) + } result = super.onOptionsItemSelected(item) } else -> result = super.onOptionsItemSelected(item) @@ -751,17 +755,14 @@ class SyncedFoldersActivity : ) { when (requestCode) { PermissionUtil.PERMISSIONS_EXTERNAL_STORAGE -> { - // If request is cancelled, result arrays are empty. if (grantResults.isNotEmpty() && grantResults[0] == PackageManager.PERMISSION_GRANTED) { // permission was granted - load(getItemsDisplayedPerFolder(), true) } else { - // permission denied --> do nothing - return + // permission denied --> request again + PermissionUtil.requestExternalStoragePermission(this, true) } - return } else -> super.onRequestPermissionsResult(requestCode, permissions, grantResults) } diff --git a/app/src/main/java/com/owncloud/android/ui/activity/UploadFilesActivity.java b/app/src/main/java/com/owncloud/android/ui/activity/UploadFilesActivity.java index 92cff9401c..a563a87908 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/UploadFilesActivity.java +++ b/app/src/main/java/com/owncloud/android/ui/activity/UploadFilesActivity.java @@ -37,7 +37,6 @@ import android.widget.Spinner; import android.widget.TextView; import com.google.android.material.button.MaterialButton; -import com.google.android.material.snackbar.Snackbar; import com.nextcloud.client.account.User; import com.nextcloud.client.di.Injectable; import com.nextcloud.client.preferences.AppPreferences; @@ -59,7 +58,6 @@ import com.owncloud.android.utils.PermissionUtil; import com.owncloud.android.utils.theme.ThemeButtonUtils; import com.owncloud.android.utils.theme.ThemeColorUtils; import com.owncloud.android.utils.theme.ThemeDrawableUtils; -import com.owncloud.android.utils.theme.ThemeSnackbarUtils; import com.owncloud.android.utils.theme.ThemeToolbarUtils; import com.owncloud.android.utils.theme.ThemeUtils; @@ -265,6 +263,16 @@ public class UploadFilesActivity extends DrawerActivity implements LocalFileList Log_OC.d(TAG, "onCreate() end"); } + @Override + protected void onPostCreate(Bundle savedInstanceState) { + super.onPostCreate(savedInstanceState); + requestPermissions(); + } + + private void requestPermissions() { + PermissionUtil.requestExternalStoragePermission(this, true); + } + public void showToolbarSpinner() { mToolbarSpinner.setVisibility(View.VISIBLE); } @@ -324,25 +332,10 @@ public class UploadFilesActivity extends DrawerActivity implements LocalFileList private void checkLocalStoragePathPickerPermission() { if (!PermissionUtil.checkExternalStoragePermission(this)) { - // Check if we should show an explanation - if (PermissionUtil.shouldShowRequestPermissionRationale(this, - PermissionUtil.getExternalStoragePermission())) { - // Show explanation to the user and then request permission - Snackbar snackbar = Snackbar.make(findViewById(android.R.id.content), - R.string.permission_storage_access, - Snackbar.LENGTH_INDEFINITE) - .setAction(R.string.common_ok, v -> PermissionUtil.requestExternalStoragePermission(this)); - ThemeSnackbarUtils.colorSnackbar(this, snackbar); - snackbar.show(); - } else { - // No explanation needed, request the permission. - PermissionUtil.requestExternalStoragePermission(this); - } - - return; + requestPermissions(); + } else { + showLocalStoragePathPickerDialog(); } - - showLocalStoragePathPickerDialog(); } private void showLocalStoragePathPickerDialog() { @@ -370,6 +363,18 @@ public class UploadFilesActivity extends DrawerActivity implements LocalFileList } } + @Override + protected void onActivityResult(int requestCode, int resultCode, Intent data) { + super.onActivityResult(requestCode, resultCode, data); + if (requestCode == PermissionUtil.REQUEST_CODE_MANAGE_ALL_FILES) { + if (resultCode == Activity.RESULT_OK) { + showLocalStoragePathPickerDialog(); + } else { + DisplayUtils.showSnackMessage(this, R.string.permission_storage_access); + } + } + } + @Override public void onSortingOrderChosen(FileSortOrder selection) { preferences.setSortOrder(FileSortOrder.Type.localFileListView, selection); diff --git a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt index 5dc9f8a7a2..77a105a9b9 100644 --- a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt +++ b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt @@ -36,8 +36,12 @@ import androidx.annotation.RequiresApi import androidx.appcompat.app.AlertDialog import androidx.core.app.ActivityCompat import androidx.core.content.ContextCompat +import com.google.android.material.snackbar.Snackbar +import com.nextcloud.client.preferences.AppPreferences +import com.nextcloud.client.preferences.AppPreferencesImpl import com.owncloud.android.R import com.owncloud.android.utils.theme.ThemeButtonUtils +import com.owncloud.android.utils.theme.ThemeSnackbarUtils object PermissionUtil { const val PERMISSIONS_EXTERNAL_STORAGE = 1 @@ -76,68 +80,110 @@ object PermissionUtil { ActivityCompat.shouldShowRequestPermissionRationale(activity, permission) /** - * For SDK < 30, we can do whatever we want using WRITE_EXTERNAL_STORAGE. - * For SDK above 30, scoped storage is in effect, and WRITE_EXTERNAL_STORAGE is useless. However, we do still need - * READ_EXTERNAL_STORAGE to read and upload files from folders that we don't manage and are not public access. + * Determine whether the app has been granted external storage permissions depending on SDK. * - * @return The relevant external storage permission, depending on SDK - */ - @JvmStatic - fun getExternalStoragePermission(): String = when { - Build.VERSION.SDK_INT >= Build.VERSION_CODES.R -> Manifest.permission.MANAGE_EXTERNAL_STORAGE - else -> Manifest.permission.WRITE_EXTERNAL_STORAGE - } - - /** - * Determine whether *the app* has been granted external storage permissions depending on SDK. + * For sdk >= 30 we use the storage manager special permissin + * Under sdk 30 we use WRITE_EXTERNAL_STORAGE * * @return `true` if app has the permission, or `false` if not. */ @JvmStatic fun checkExternalStoragePermission(context: Context): Boolean = when { Build.VERSION.SDK_INT >= Build.VERSION_CODES.R -> Environment.isExternalStorageManager() - else -> checkSelfPermission(context, getExternalStoragePermission()) + else -> checkSelfPermission(context, Manifest.permission.WRITE_EXTERNAL_STORAGE) } /** - * Request relevant external storage permission depending on SDK. + * Request relevant external storage permission depending on SDK, if needed. + * + * Activities should implement [Activity.onRequestPermissionsResult] + * and handle the [PERMISSIONS_EXTERNAL_STORAGE] code, as well ass [Activity.onActivityResult] + * with `requestCode=`[REQUEST_CODE_MANAGE_ALL_FILES] * * @param activity The target activity. + * @param force for MANAGE_ALL_FILES specifically, show again even if already denied in the past */ @JvmStatic - fun requestExternalStoragePermission(activity: Activity) = when { - Build.VERSION.SDK_INT >= Build.VERSION_CODES.R -> requestManageFilesPermission(activity) - else -> { - ActivityCompat.requestPermissions( - activity, arrayOf(getExternalStoragePermission()), - PERMISSIONS_EXTERNAL_STORAGE - ) + @JvmOverloads + fun requestExternalStoragePermission(activity: Activity, force: Boolean = false) { + if (!checkExternalStoragePermission(activity)) { + when { + Build.VERSION.SDK_INT >= Build.VERSION_CODES.R -> requestManageFilesPermission(activity, force) + else -> requestWriteExternalStoragePermission(activity) + } } } - @RequiresApi(Build.VERSION_CODES.R) - private fun requestManageFilesPermission(activity: Activity) { - val alertDialog = AlertDialog.Builder(activity, R.style.Theme_ownCloud_Dialog) - .setTitle(R.string.file_management_permission) - .setMessage( - String.format( - activity.getString(R.string.file_management_permission_text), - activity.getString(R.string.app_name) - ) + /** + * For sdk < 30: request WRITE_EXTERNAL_STORAGE + */ + private fun requestWriteExternalStoragePermission(activity: Activity) { + fun doRequest() { + ActivityCompat.requestPermissions( + activity, arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE), + PERMISSIONS_EXTERNAL_STORAGE ) - .setCancelable(false) - .setPositiveButton(R.string.common_ok) { dialog, _ -> - val intent = Intent().apply { - action = Settings.ACTION_MANAGE_APP_ALL_FILES_ACCESS_PERMISSION - data = Uri.parse("package:${activity.applicationContext.packageName}") - } - activity.startActivityForResult(intent, REQUEST_CODE_MANAGE_ALL_FILES) - dialog.dismiss() - } - .create() + } - alertDialog.show() - ThemeButtonUtils.themeBorderlessButton(alertDialog.getButton(AlertDialog.BUTTON_POSITIVE)) + // Check if we should show an explanation + if (shouldShowRequestPermissionRationale(activity, Manifest.permission.WRITE_EXTERNAL_STORAGE)) { + // Show explanation to the user and then request permission + Snackbar + .make( + activity.findViewById(android.R.id.content), + R.string.permission_storage_access, + Snackbar.LENGTH_INDEFINITE + ) + .setAction(R.string.common_ok) { + doRequest() + } + .also { + ThemeSnackbarUtils.colorSnackbar(activity, it) + } + .show() + } else { + // No explanation needed, request the permission. + doRequest() + } + } + + /** + * For sdk < 30: request MANAGE_EXTERNAL_STORAGE through system preferences + */ + @RequiresApi(Build.VERSION_CODES.R) + private fun requestManageFilesPermission(activity: Activity, force: Boolean) { + + val preferences: AppPreferences = AppPreferencesImpl.fromContext(activity) + + if (!preferences.isStoragePermissionRequested || force) { + + val alertDialog = AlertDialog.Builder(activity, R.style.Theme_ownCloud_Dialog) + .setTitle(R.string.file_management_permission) + .setMessage( + String.format( + activity.getString(R.string.file_management_permission_optional_text), + activity.getString(R.string.app_name) + ) + ) + .setPositiveButton(R.string.common_ok) { dialog, _ -> + val intent = Intent().apply { + action = Settings.ACTION_MANAGE_APP_ALL_FILES_ACCESS_PERMISSION + data = Uri.parse("package:${activity.applicationContext.packageName}") + } + activity.startActivityForResult(intent, REQUEST_CODE_MANAGE_ALL_FILES) + preferences.isStoragePermissionRequested = true + dialog.dismiss() + } + .setNegativeButton(R.string.common_cancel) { dialog, _ -> + preferences.isStoragePermissionRequested = true + dialog.dismiss() + } + .create() + + alertDialog.show() + ThemeButtonUtils.themeBorderlessButton(alertDialog.getButton(AlertDialog.BUTTON_POSITIVE)) + ThemeButtonUtils.themeBorderlessButton(alertDialog.getButton(AlertDialog.BUTTON_NEGATIVE)) + } } /** diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 7f15aa36c4..11c2133ddf 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -999,6 +999,7 @@ Load more results Permissions needed %1$s needs file management permissions to work properly. Please enable it in the following screen to continue. + %1$s needs file management permissions to upload files from this device. Please enable it in the following screen if appropriate. No results found for your query Found no images or videos Error creating file from template