From 1c0cf347f19ac48ff28c19ce9f9821ef9a3efd93 Mon Sep 17 00:00:00 2001
From: Dipankar Bhardwaj <dipankarb@google.com>
Date: Fri, 7 Feb 2025 16:58:52 +0000
Subject: Fix for legacy storage app op

There is a bug in AppOpsManager that keeps legacy storage granted even
when an app updates its targetSdkVersion from value <30 to >=30.
If an app upgrades from targetSdk 29 to targetSdk 33, legacy storage
remains granted and in targetSdk 33, app are required to replace R_E_S
with R_M_*. If an app updates its manifest with R_M_*, permission check
in MediaProvider will look for R_E_S and will not grant read access as
the app would be still treated as legacy.

Test: atest PermissionUtilsTest
Bug: 315914683
Flag: EXEMPT bug fix
Change-Id: I22d0a329fbea5afe9dea2bc5179596db7a2ef402
---
 tests/Android.bp                                   | 20 +++++++++
 tests/AndroidManifest.xml                          |  1 +
 tests/AndroidTest.xml                              |  1 +
 .../providers/media/util/PermissionUtilsTest.java  | 49 ++++++++++++++++++++++
 tests/test_app/LegacyTestAppWithTargetSdk33.xml    | 40 ++++++++++++++++++
 5 files changed, 111 insertions(+)
 create mode 100644 tests/test_app/LegacyTestAppWithTargetSdk33.xml

(limited to 'tests')

diff --git a/tests/Android.bp b/tests/Android.bp
index cdb44b814..3de35a077 100644
--- a/tests/Android.bp
+++ b/tests/Android.bp
@@ -155,6 +155,25 @@ android_test_helper_app {
     ],
 }
 
+android_test_helper_app {
+    name: "LegacyMediaProviderTestAppFor33",
+    manifest: "test_app/LegacyTestAppWithTargetSdk33.xml",
+    srcs: [
+        "test_app/src/**/*.java",
+        "src/com/android/providers/media/util/TestUtils.java",
+    ],
+    static_libs: [
+        "cts-install-lib",
+    ],
+    sdk_version: "test_current",
+    target_sdk_version: "33",
+    min_sdk_version: "30",
+    test_suites: [
+        "general-tests",
+        "mts-mediaprovider",
+    ],
+}
+
 // This looks a bit awkward, but we need our tests to run against either
 // MediaProvider or MediaProviderGoogle, and we don't know which one is
 // on the device being tested, so we can't sign our tests with a key that
@@ -250,6 +269,7 @@ android_test {
 
     data: [
         ":LegacyMediaProviderTestApp",
+        ":LegacyMediaProviderTestAppFor33",
         ":LegacyMediaProviderTestAppFor35",
         ":MediaProviderTestAppForPermissionActivity",
         ":MediaProviderTestAppForPermissionActivity33",
diff --git a/tests/AndroidManifest.xml b/tests/AndroidManifest.xml
index acd1a21af..e3902d6eb 100644
--- a/tests/AndroidManifest.xml
+++ b/tests/AndroidManifest.xml
@@ -13,6 +13,7 @@
         <package android:name="com.android.providers.media.testapp.withuserselectedperms" />
         <package android:name="com.android.providers.media.testapp.legacy" />
         <package android:name="com.android.providers.media.testapp.legacywithtargetsdk35" />
+        <package android:name="com.android.providers.media.testapp.legacywithtargetsdk33" />
     </queries>
 
     <uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" />
diff --git a/tests/AndroidTest.xml b/tests/AndroidTest.xml
index 31d9e1535..44fb27930 100644
--- a/tests/AndroidTest.xml
+++ b/tests/AndroidTest.xml
@@ -30,6 +30,7 @@
         <option name="test-file-name" value="MediaProviderTestAppWithUserSelectedPerms.apk" />
         <option name="test-file-name" value="MediaProviderTestAppWithoutPerms.apk" />
         <option name="test-file-name" value="LegacyMediaProviderTestApp.apk" />
+        <option name="test-file-name" value="LegacyMediaProviderTestAppFor33.apk" />
         <option name="test-file-name" value="LegacyMediaProviderTestAppFor35.apk" />
         <option name="install-arg" value="-g" />
     </target_preparer>
diff --git a/tests/src/com/android/providers/media/util/PermissionUtilsTest.java b/tests/src/com/android/providers/media/util/PermissionUtilsTest.java
index f58f1bbf6..7eb6c667b 100644
--- a/tests/src/com/android/providers/media/util/PermissionUtilsTest.java
+++ b/tests/src/com/android/providers/media/util/PermissionUtilsTest.java
@@ -21,6 +21,7 @@ import static android.Manifest.permission.MANAGE_EXTERNAL_STORAGE;
 import static android.Manifest.permission.MANAGE_MEDIA;
 import static android.Manifest.permission.UPDATE_APP_OPS_STATS;
 import static android.app.AppOpsManager.OPSTR_ACCESS_MEDIA_LOCATION;
+import static android.app.AppOpsManager.OPSTR_LEGACY_STORAGE;
 import static android.app.AppOpsManager.OPSTR_NO_ISOLATED_STORAGE;
 import static android.app.AppOpsManager.OPSTR_READ_MEDIA_AUDIO;
 import static android.app.AppOpsManager.OPSTR_READ_MEDIA_IMAGES;
@@ -44,6 +45,7 @@ import static com.android.providers.media.util.PermissionUtils.checkPermissionIn
 import static com.android.providers.media.util.PermissionUtils.checkPermissionManageMedia;
 import static com.android.providers.media.util.PermissionUtils.checkPermissionManager;
 import static com.android.providers.media.util.PermissionUtils.checkPermissionReadAudio;
+import static com.android.providers.media.util.PermissionUtils.checkPermissionReadForLegacyStorage;
 import static com.android.providers.media.util.PermissionUtils.checkPermissionReadImages;
 import static com.android.providers.media.util.PermissionUtils.checkPermissionReadStorage;
 import static com.android.providers.media.util.PermissionUtils.checkPermissionReadVideo;
@@ -105,6 +107,10 @@ public class PermissionUtilsTest {
             "com.android.providers.media.testapp.legacy", 1, false,
             "LegacyMediaProviderTestApp.apk");
 
+    private static final TestApp LEGACY_TEST_APP_33 = new TestApp("LegacyTestAppWithTargetSdk33",
+            "com.android.providers.media.testapp.legacywithtargetsdk33", 1, false,
+            "LegacyMediaProviderTestAppFor33.apk");
+
     private static final TestApp LEGACY_TEST_APP_35 = new TestApp("LegacyTestAppWithTargetSdk35",
             "com.android.providers.media.testapp.legacywithtargetsdk35", 1, false,
             "LegacyMediaProviderTestAppFor35.apk");
@@ -251,6 +257,44 @@ public class PermissionUtilsTest {
                     getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse();
             assertThat(checkPermissionReadStorage(
                     getContext(), TEST_APP_PID, testAppUid, packageName, null)).isTrue();
+            assertThat(checkPermissionReadForLegacyStorage(
+                    getContext(), TEST_APP_PID, testAppUid, packageName,
+                    null, /* isTargetSdkAtLeastT */ true)).isTrue();
+        } finally {
+            dropShellPermission();
+        }
+    }
+
+    @Test
+    @SdkSuppress(minSdkVersion = Build.VERSION_CODES.TIRAMISU, codeName = "Tiramisu")
+    public void testDefaultPermissionsOnLegacyTestAppWithTargetSdk33() throws Exception {
+        String packageName = LEGACY_TEST_APP_33.getPackageName();
+        int testAppUid = getContext().getPackageManager().getPackageUid(packageName, 0);
+        adoptShellPermission(UPDATE_APP_OPS_STATS, MANAGE_APP_OPS_MODES);
+
+        try {
+            assertThat(checkPermissionSelf(getContext(), TEST_APP_PID, testAppUid)).isFalse();
+            assertThat(checkPermissionShell(testAppUid)).isFalse();
+            assertThat(checkPermissionInstallPackages(
+                    getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse();
+            assertThat(checkPermissionAccessMtp(
+                    getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse();
+            assertThat(checkPermissionWriteStorage(
+                    getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse();
+
+            modifyAppOp(testAppUid, OPSTR_READ_MEDIA_IMAGES, AppOpsManager.MODE_ALLOWED);
+            modifyAppOp(testAppUid, OPSTR_READ_MEDIA_VIDEO, AppOpsManager.MODE_ALLOWED);
+            modifyAppOp(testAppUid, OPSTR_READ_MEDIA_AUDIO, AppOpsManager.MODE_ALLOWED);
+            modifyAppOp(testAppUid, OPSTR_LEGACY_STORAGE, AppOpsManager.MODE_ALLOWED);
+
+            assertThat(checkIsLegacyStorageGranted(getContext(), testAppUid,
+                    packageName,  /* isTargetSdkAtLeastV */ false)).isTrue();
+            // Since R_E_S is not granted, this is should return false
+            assertThat(checkPermissionReadStorage(
+                    getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse();
+            assertThat(checkPermissionReadForLegacyStorage(
+                    getContext(), TEST_APP_PID, testAppUid, packageName,
+                    null, /* isTargetSdkAtLeastT */ true)).isTrue();
         } finally {
             dropShellPermission();
         }
@@ -329,6 +373,11 @@ public class PermissionUtilsTest {
             assertThat(
                     checkIsLegacyStorageGranted(getContext(), testAppUid, packageName,
                             /* isTargetSdkAtLeastS */ false)).isTrue();
+            assertThat(
+                    checkPermissionReadForLegacyStorage(getContext(), TEST_APP_PID,
+                            testAppUid, packageName,
+                            null, /* isTargetSdkAtLeastT */ false)).isTrue();
+
             assertThat(
                     checkPermissionInstallPackages(getContext(), TEST_APP_PID, testAppUid,
                             packageName, null)).isFalse();
diff --git a/tests/test_app/LegacyTestAppWithTargetSdk33.xml b/tests/test_app/LegacyTestAppWithTargetSdk33.xml
new file mode 100644
index 000000000..541e5abcf
--- /dev/null
+++ b/tests/test_app/LegacyTestAppWithTargetSdk33.xml
@@ -0,0 +1,40 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+  ~ Copyright (C) 2025 The Android Open Source Project
+  ~
+  ~ Licensed under the Apache License, Version 2.0 (the "License");
+  ~ you may not use this file except in compliance with the License.
+  ~ You may obtain a copy of the License at
+  ~
+  ~      http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing, software
+  ~ distributed under the License is distributed on an "AS IS" BASIS,
+  ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  ~ See the License for the specific language governing permissions and
+  ~ limitations under the License.
+  -->
+
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+          package="com.android.providers.media.testapp.legacywithtargetsdk33"
+          android:versionCode="1"
+          android:versionName="1.0">
+
+    <uses-sdk android:minSdkVersion="30" android:targetSdkVersion="33" />
+
+    <uses-permission android:name="android.permission.READ_MEDIA_IMAGES" />
+    <uses-permission android:name="android.permission.READ_MEDIA_VIDEO" />
+    <uses-permission android:name="android.permission.READ_MEDIA_AUDIO" />
+
+    <application android:label="LegacyTestAppWithTargetSdk33"
+                 android:requestLegacyExternalStorage="true">
+        <activity android:name="com.android.providers.media.util.TestAppActivity"
+                  android:exported="true">
+            <intent-filter>
+                <action android:name="android.intent.action.MAIN"/>
+                <category android:name="android.intent.category.DEFAULT"/>
+                <category android:name="android.intent.category.LAUNCHER"/>
+            </intent-filter>
+        </activity>
+    </application>
+</manifest>
-- 
cgit v1.2.3-59-g8ed1b