Request READ_EXTERNAL_STORAGE in API >= 30

Although scoped storage is in effect and WRITE_EXTERNAL_STORAGE does not do anything,
we still need READ_EXTERNAL_STORAGE to read files from folders we don't own.

Signed-off-by: Álvaro Brey Vilas <alvaro.brey@nextcloud.com>

src/androidTest/java/com/nextcloud/client/GrantStoragePermissionRule.kt

@@ -21,27 +21,14 @@
 
 package com.nextcloud.client
 
-import android.Manifest
-import android.os.Build
 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
 
 class GrantStoragePermissionRule private constructor() {
 
     companion object {
         @JvmStatic
-        fun grant(): TestRule = when {
-            Build.VERSION.SDK_INT < Build.VERSION_CODES.R ->
-                GrantPermissionRule.grant(Manifest.permission.WRITE_EXTERNAL_STORAGE)
-            else -> DoNothingTestRule
-        }
-    }
-
-    private object DoNothingTestRule : TestRule {
-        override fun apply(base: Statement, description: Description?): Statement {
-            return base
-        }
+        fun grant(): TestRule = GrantPermissionRule.grant(PermissionUtil.getExternalStoragePermission())
     }
 }

src/main/AndroidManifest.xml

@@ -28,7 +28,11 @@
 
     <!-- WRITE_EXTERNAL_STORAGE may be enabled or disabled by the user after installation in
         API >= 23; the app needs to handle this -->
-    <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
+    <uses-permission
+        android:name="android.permission.WRITE_EXTERNAL_STORAGE"
+        android:maxSdkVersion="29"
+        tools:ignore="ScopedStorage" />
+    <uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" />
     <uses-permission android:name="android.permission.CAMERA" />
     <uses-permission android:name="android.permission.VIBRATE" />
 

src/main/java/com/owncloud/android/MainApp.java

@@ -439,8 +439,7 @@ public class MainApp extends MultiDexApplication implements HasAndroidInjector {
         updateAutoUploadEntries(clock);
 
         if (getAppContext() != null) {
-            if (PermissionUtil.checkSelfPermission(getAppContext(),
-                                                   Manifest.permission.WRITE_EXTERNAL_STORAGE)) {
+            if (PermissionUtil.checkExternalStoragePermission(getAppContext())) {
                 splitOutAutoUploadEntries(clock);
             } else {
                 preferences.setAutoUploadSplitEntriesEnabled(true);

src/main/java/com/owncloud/android/datamodel/MediaProvider.java

@@ -21,7 +21,6 @@
 
 package com.owncloud.android.datamodel;
 
-import android.Manifest;
 import android.app.Activity;
 import android.content.ContentResolver;
 import android.database.Cursor;
@@ -79,8 +78,8 @@ public final class MediaProvider {
 
         // query media/image folders
         Cursor cursorFolders = null;
-        if ((activity != null && PermissionUtil.checkSelfPermission(activity.getApplicationContext(),
-                Manifest.permission.WRITE_EXTERNAL_STORAGE)) || getWithoutActivity) {
+        if ((activity != null && PermissionUtil.checkExternalStoragePermission(activity.getApplicationContext()))
+            || getWithoutActivity) {
             cursorFolders = ContentResolverHelper.queryResolver(contentResolver, IMAGES_MEDIA_URI,
                                                                 IMAGES_FOLDER_PROJECTION, null, null,
                                                                 IMAGES_FOLDER_SORT_COLUMN, IMAGES_SORT_DIRECTION, null);
@@ -172,22 +171,21 @@ public final class MediaProvider {
 
     private static void checkPermissions(@Nullable Activity activity) {
         if (activity != null &&
-                !PermissionUtil.checkSelfPermission(activity.getApplicationContext(),
-                        Manifest.permission.WRITE_EXTERNAL_STORAGE)) {
+            !PermissionUtil.checkExternalStoragePermission(activity.getApplicationContext())) {
             // Check if we should show an explanation
-            if (PermissionUtil.shouldShowRequestPermissionRationale(activity,
-                    Manifest.permission.WRITE_EXTERNAL_STORAGE)) {
+            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.requestWriteExternalStoreagePermission(activity));
+                                                  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.requestWriteExternalStoreagePermission(activity);
+                PermissionUtil.requestExternalStoragePermission(activity);
             }
         }
     }
@@ -199,10 +197,10 @@ public final class MediaProvider {
 
         // query media/image folders
         Cursor cursorFolders = null;
-        if ((activity != null && PermissionUtil.checkSelfPermission(activity.getApplicationContext(),
-                Manifest.permission.WRITE_EXTERNAL_STORAGE)) || getWithoutActivity) {
+        if ((activity != null && PermissionUtil.checkExternalStoragePermission(activity.getApplicationContext()))
+            || getWithoutActivity) {
             cursorFolders = contentResolver.query(MediaStore.Video.Media.EXTERNAL_CONTENT_URI, VIDEOS_FOLDER_PROJECTION,
-                    null, null, null);
+                                                  null, null, null);
         }
 
         List<MediaFolder> mediaFolders = new ArrayList<>();

src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.java

@@ -25,7 +25,6 @@
 
 package com.owncloud.android.ui.activity;
 
-import android.Manifest;
 import android.accounts.AuthenticatorException;
 import android.app.Activity;
 import android.content.BroadcastReceiver;
@@ -320,20 +319,20 @@ public class FileDisplayActivity extends FileActivity
         super.onPostCreate(savedInstanceState);
 
 
-        if (!PermissionUtil.checkSelfPermission(this, Manifest.permission.WRITE_EXTERNAL_STORAGE)) {
+        if (!PermissionUtil.checkExternalStoragePermission(this)) {
             // Check if we should show an explanation
             if (PermissionUtil.shouldShowRequestPermissionRationale(this,
-                                                                    Manifest.permission.WRITE_EXTERNAL_STORAGE)) {
+                                                                    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.requestWriteExternalStoreagePermission(this));
+                    .setAction(R.string.common_ok, v -> PermissionUtil.requestExternalStoragePermission(this));
                 ThemeSnackbarUtils.colorSnackbar(this, snackbar);
                 snackbar.show();
             } else {
                 // No explanation needed, request the permission.
-                PermissionUtil.requestWriteExternalStoreagePermission(this);
+                PermissionUtil.requestExternalStoragePermission(this);
             }
         }
 
@@ -403,7 +402,7 @@ public class FileDisplayActivity extends FileActivity
     public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions,
                                            @NonNull int[] grantResults) {
         switch (requestCode) {
-            case PermissionUtil.PERMISSIONS_WRITE_EXTERNAL_STORAGE: {
+            case PermissionUtil.PERMISSIONS_EXTERNAL_STORAGE: {
                 // If request is cancelled, result arrays are empty.
                 if (grantResults.length > 0
                     && grantResults[0] == PackageManager.PERMISSION_GRANTED) {

src/main/java/com/owncloud/android/ui/activity/SyncedFoldersActivity.java

@@ -764,7 +764,7 @@ public class SyncedFoldersActivity extends FileActivity implements SyncedFolderA
     public void onRequestPermissionsResult(int requestCode, @NonNull String permissions[],
                                            @NonNull int[] grantResults) {
         switch (requestCode) {
-            case PermissionUtil.PERMISSIONS_WRITE_EXTERNAL_STORAGE: {
+            case PermissionUtil.PERMISSIONS_EXTERNAL_STORAGE: {
                 // If request is cancelled, result arrays are empty.
                 if (grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
                     // permission was granted

src/main/java/com/owncloud/android/utils/PermissionUtil.java

@@ -1,73 +0,0 @@
-package com.owncloud.android.utils;
-
-import android.Manifest;
-import android.app.Activity;
-import android.content.Context;
-
-import androidx.core.app.ActivityCompat;
-import androidx.core.content.ContextCompat;
-
-/**
- * Created by scherzia on 29.12.2015.
- */
-public final class PermissionUtil {
-    public static final int PERMISSIONS_WRITE_EXTERNAL_STORAGE = 1;
-    public static final int PERMISSIONS_READ_CONTACTS_AUTOMATIC = 2;
-    public static final int PERMISSIONS_WRITE_CONTACTS = 4;
-    public static final int PERMISSIONS_CAMERA = 5;
-    public static final int PERMISSIONS_READ_CALENDAR_AUTOMATIC = 6;
-    public static final int PERMISSIONS_WRITE_CALENDAR = 7;
-
-    private PermissionUtil() {
-        // utility class -> private constructor
-    }
-
-    /**
-     * Wrapper method for ContextCompat.checkSelfPermission().
-     * Determine whether <em>the app</em> has been granted a particular permission.
-     *
-     * @param permission The name of the permission being checked.
-     * @return <code>true</code> if app has the permission, or <code>false</code> if not.
-     */
-    public static boolean checkSelfPermission(Context context, String permission) {
-        return ContextCompat.checkSelfPermission(context, permission)
-                == android.content.pm.PackageManager.PERMISSION_GRANTED;
-    }
-
-    /**
-     * Wrapper method for ActivityCompat.shouldShowRequestPermissionRationale().
-     * Gets whether you should show UI with rationale for requesting a permission.
-     * You should do this only if you do not have the permission and the context in
-     * which the permission is requested does not clearly communicate to the user
-     * what would be the benefit from granting this permission.
-     *
-     * @param activity   The target activity.
-     * @param permission A permission to be requested.
-     * @return Whether to show permission rationale UI.
-     */
-    public static boolean shouldShowRequestPermissionRationale(Activity activity, String permission) {
-        return ActivityCompat.shouldShowRequestPermissionRationale(activity, permission);
-    }
-
-    /**
-     * request the write permission for external storage.
-     *
-     * @param activity The target activity.
-     */
-    public static void requestWriteExternalStoreagePermission(Activity activity) {
-        ActivityCompat.requestPermissions(activity,
-                new String[]{Manifest.permission.WRITE_EXTERNAL_STORAGE},
-                PERMISSIONS_WRITE_EXTERNAL_STORAGE);
-    }
-
-    /**
-     * request camera permission.
-     *
-     * @param activity The target activity.
-     */
-    public static void requestCameraPermission(Activity activity) {
-        ActivityCompat.requestPermissions(activity,
-                                          new String[]{Manifest.permission.CAMERA},
-                                          PERMISSIONS_CAMERA);
-    }
-}

src/main/java/com/owncloud/android/utils/PermissionUtil.kt

@@ -0,0 +1,94 @@
+package com.owncloud.android.utils
+
+import android.Manifest
+import android.app.Activity
+import android.content.Context
+import android.content.pm.PackageManager
+import android.os.Build
+import androidx.core.app.ActivityCompat
+import androidx.core.content.ContextCompat
+
+/**
+ * Created by scherzia on 29.12.2015.
+ */
+object PermissionUtil {
+    const val PERMISSIONS_EXTERNAL_STORAGE = 1
+    const val PERMISSIONS_READ_CONTACTS_AUTOMATIC = 2
+    const val PERMISSIONS_READ_CONTACTS_MANUALLY = 3
+    const val PERMISSIONS_WRITE_CONTACTS = 4
+    const val PERMISSIONS_CAMERA = 5
+
+    /**
+     * Wrapper method for ContextCompat.checkSelfPermission().
+     * Determine whether *the app* has been granted a particular permission.
+     *
+     * @param permission The name of the permission being checked.
+     * @return `true` if app has the permission, or `false` if not.
+     */
+    @JvmStatic
+    fun checkSelfPermission(context: Context, permission: String): Boolean =
+        ContextCompat.checkSelfPermission(context, permission) == PackageManager.PERMISSION_GRANTED
+
+    /**
+     * Wrapper method for ActivityCompat.shouldShowRequestPermissionRationale().
+     * Gets whether you should show UI with rationale for requesting a permission.
+     * You should do this only if you do not have the permission and the context in
+     * which the permission is requested does not clearly communicate to the user
+     * what would be the benefit from granting this permission.
+     *
+     * @param activity   The target activity.
+     * @param permission A permission to be requested.
+     * @return Whether to show permission rationale UI.
+     */
+    @JvmStatic
+    fun shouldShowRequestPermissionRationale(activity: Activity, permission: String): Boolean =
+        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.
+     *
+     * @return The relevant external storage permission, depending on SDK
+     */
+    @JvmStatic
+    fun getExternalStoragePermission(): String = when {
+        Build.VERSION.SDK_INT < Build.VERSION_CODES.R -> Manifest.permission.WRITE_EXTERNAL_STORAGE
+        else -> Manifest.permission.READ_EXTERNAL_STORAGE
+    }
+
+    /**
+     * Determine whether *the app* has been granted external storage permissions depending on SDK.
+     *
+     * @return `true` if app has the permission, or `false` if not.
+     */
+    @JvmStatic
+    fun checkExternalStoragePermission(context: Context): Boolean =
+        ContextCompat.checkSelfPermission(context, getExternalStoragePermission()) == PackageManager.PERMISSION_GRANTED
+
+    /**
+     * Request relevant external storage permission depending on SDK.
+     *
+     * @param activity The target activity.
+     */
+    @JvmStatic
+    fun requestExternalStoragePermission(activity: Activity) {
+        ActivityCompat.requestPermissions(
+            activity, arrayOf(getExternalStoragePermission()),
+            PERMISSIONS_EXTERNAL_STORAGE
+        )
+    }
+
+    /**
+     * request camera permission.
+     *
+     * @param activity The target activity.
+     */
+    @JvmStatic
+    fun requestCameraPermission(activity: Activity) {
+        ActivityCompat.requestPermissions(
+            activity, arrayOf(Manifest.permission.CAMERA),
+            PERMISSIONS_CAMERA
+        )
+    }
+}