Browse Source

Make external storage permission optional

Signed-off-by: Álvaro Brey Vilas <alvaro.brey@nextcloud.com>
Álvaro Brey Vilas 3 years ago
parent
commit
7d2f0fa017

+ 2 - 3
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()
         }

+ 4 - 0
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);
 }

+ 12 - 0
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);

+ 1 - 18
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);
         }
     }
 

+ 7 - 29
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);
         }

+ 24 - 23
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)
         }

+ 25 - 20
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);

+ 84 - 38
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 -> {
+    @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)
+            }
+        }
+    }
+
+    /**
+     * For sdk < 30: request WRITE_EXTERNAL_STORAGE
+     */
+    private fun requestWriteExternalStoragePermission(activity: Activity) {
+        fun doRequest() {
             ActivityCompat.requestPermissions(
-                activity, arrayOf(getExternalStoragePermission()),
+                activity, arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE),
                 PERMISSIONS_EXTERNAL_STORAGE
             )
         }
+
+        // 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) {
-        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)
+    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)
+                    )
                 )
-            )
-            .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}")
+                .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()
                 }
-                activity.startActivityForResult(intent, REQUEST_CODE_MANAGE_ALL_FILES)
-                dialog.dismiss()
-            }
-            .create()
+                .setNegativeButton(R.string.common_cancel) { dialog, _ ->
+                    preferences.isStoragePermissionRequested = true
+                    dialog.dismiss()
+                }
+                .create()
 
-        alertDialog.show()
-        ThemeButtonUtils.themeBorderlessButton(alertDialog.getButton(AlertDialog.BUTTON_POSITIVE))
+            alertDialog.show()
+            ThemeButtonUtils.themeBorderlessButton(alertDialog.getButton(AlertDialog.BUTTON_POSITIVE))
+            ThemeButtonUtils.themeBorderlessButton(alertDialog.getButton(AlertDialog.BUTTON_NEGATIVE))
+        }
     }
 
     /**

+ 1 - 0
app/src/main/res/values/strings.xml

@@ -999,6 +999,7 @@
     <string name="load_more_results">Load more results</string>
     <string name="file_management_permission">Permissions needed</string>
     <string name="file_management_permission_text">%1$s needs file management permissions to work properly. Please enable it in the following screen to continue.</string>
+    <string name="file_management_permission_optional_text">%1$s needs file management permissions to upload files from this device. Please enable it in the following screen if appropriate.</string>
     <string name="file_list_empty_unified_search_no_results">No results found for your query</string>
     <string name="file_list_empty_gallery">Found no images or videos</string>
     <string name="error_creating_file_from_template">Error creating file from template</string>