Merge "Display the system gallery storage permission as system fixed" into main

8 files changed, 135 insertions, 56 deletions

diff --git a/PermissionController/src/com/android/permissioncontroller/permission/data/AppPermGroupUiInfoLiveData.kt b/PermissionController/src/com/android/permissioncontroller/permission/data/AppPermGroupUiInfoLiveData.kt
index 394cb3eb7..ad356d5f7 100644
--- a/PermissionController/src/com/android/permissioncontroller/permission/data/AppPermGroupUiInfoLiveData.kt
+++ b/PermissionController/src/com/android/permissioncontroller/permission/data/AppPermGroupUiInfoLiveData.kt
@@ -18,6 +18,7 @@ package com.android.permissioncontroller.permission.data
 
 import android.Manifest
 import android.Manifest.permission.READ_MEDIA_VISUAL_USER_SELECTED
+import android.Manifest.permission_group.READ_MEDIA_VISUAL
 import android.Manifest.permission_group.STORAGE
 import android.app.AppOpsManager
 import android.app.Application
@@ -66,13 +67,8 @@ private constructor(
     private val isHealth = Utils.isHealthPermissionGroup(permGroupName)
 
     init {
-        isSpecialLocation =
-            LocationUtils.isLocationGroupAndProvider(app, permGroupName, packageName) ||
-                LocationUtils.isLocationGroupAndControllerExtraPackage(
-                    app,
-                    permGroupName,
-                    packageName
-                )
+        isSpecialLocation = LightAppPermGroupLiveData
+            .isSpecialLocationGranted(app, packageName, permGroupName, user) != null
 
         addSource(packageInfoLiveData) { update() }
 
@@ -258,9 +254,14 @@ private constructor(
         allPermInfos: Map<String, LightPermInfo>,
         pkg: LightPackageInfo
     ): PermGrantState {
-        val specialLocationState = getIsSpecialLocationState()
+        val specialLocationState = LightAppPermGroupLiveData
+            .isSpecialLocationGranted(app, packageName, permGroupName, user)
+        val specialFixedStorage = LightAppPermGroupLiveData
+            .isSpecialFixedStorageGranted(app, packageName, permGroupName, pkg.uid)
         if (isStorage && isFullFilesAccessGranted(pkg)) {
             return PermGrantState.PERMS_ALLOWED
+        } else if (permGroupName == READ_MEDIA_VISUAL && specialFixedStorage) {
+            return PermGrantState.PERMS_ALLOWED
         }
 
         var hasPermWithBackground = false
@@ -332,29 +333,6 @@ private constructor(
         return PermGrantState.PERMS_DENIED
     }
 
-    private fun getIsSpecialLocationState(): Boolean? {
-        if (!isSpecialLocation) {
-            return null
-        }
-
-        val userContext = Utils.getUserContext(app, user)
-        if (LocationUtils.isLocationGroupAndProvider(userContext, permGroupName, packageName)) {
-            return LocationUtils.isLocationEnabled(userContext)
-        }
-        // The permission of the extra location controller package is determined by the
-        // status of the controller package itself.
-        if (
-            LocationUtils.isLocationGroupAndControllerExtraPackage(
-                userContext,
-                permGroupName,
-                packageName
-            )
-        ) {
-            return LocationUtils.isExtraLocationControllerPackageEnabled(userContext)
-        }
-        return null
-    }
-
     private fun isFullFilesAccessGranted(pkg: LightPackageInfo): Boolean {
         val packageState =
             if (!FullStoragePermissionAppsLiveData.isStale) {
diff --git a/PermissionController/src/com/android/permissioncontroller/permission/data/LightAppPermGroupLiveData.kt b/PermissionController/src/com/android/permissioncontroller/permission/data/LightAppPermGroupLiveData.kt
index 67b765097..d66d6ec6c 100644
--- a/PermissionController/src/com/android/permissioncontroller/permission/data/LightAppPermGroupLiveData.kt
+++ b/PermissionController/src/com/android/permissioncontroller/permission/data/LightAppPermGroupLiveData.kt
@@ -16,7 +16,13 @@
 
 package com.android.permissioncontroller.permission.data
 
+import android.Manifest.permission_group.READ_MEDIA_VISUAL
+import android.Manifest.permission_group.STORAGE
+import android.app.AppOpsManager
+import android.app.AppOpsManager.MODE_ALLOWED
+import android.app.AppOpsManager.OPSTR_WRITE_MEDIA_IMAGES
 import android.app.Application
+import android.app.role.RoleManager
 import android.content.pm.PackageManager
 import android.content.pm.PermissionInfo
 import android.os.Build
@@ -121,19 +127,6 @@ private constructor(
                 LightPermission(packageInfo, permInfo, permState, foregroundPerms)
         }
 
-        // Determine if this app permission group is a special location package or provider
-        var specialLocationGrant: Boolean? = null
-        val userContext = Utils.getUserContext(app, user)
-        if (LocationUtils.isLocationGroupAndProvider(userContext, permGroupName, packageName)) {
-            specialLocationGrant = LocationUtils.isLocationEnabled(userContext)
-        } else if (
-            LocationUtils.isLocationGroupAndControllerExtraPackage(app, permGroupName, packageName)
-        ) {
-            // The permission of the extra location controller package is determined by the status
-            // of the controller package itself.
-            specialLocationGrant =
-                LocationUtils.isExtraLocationControllerPackageEnabled(userContext)
-        }
 
         val hasInstallToRuntimeSplit = hasInstallToRuntimeSplit(packageInfo, permissionMap)
         value =
@@ -142,7 +135,8 @@ private constructor(
                 permGroup.groupInfo,
                 permissionMap,
                 hasInstallToRuntimeSplit,
-                specialLocationGrant
+                isSpecialLocationGranted(app, packageName, permGroupName, user),
+                isSpecialFixedStorageGranted(app, packageName, permGroupName, packageInfo.uid)
             )
     }
 
@@ -234,5 +228,56 @@ private constructor(
                 deviceId
             )
         }
+
+        private const val SYSTEM_GALLERY_ROLE_NAME = "android.app.role.SYSTEM_GALLERY"
+
+        // Returns true if this app is the location provider or location extra package, and location
+        // access is granted, false if it is the provider/extra, and location is not granted, and
+        // null if it is not a special package
+        fun isSpecialLocationGranted(
+            app: Application,
+            packageName: String,
+            permGroupName: String,
+            user: UserHandle
+        ): Boolean? {
+            val userContext = Utils.getUserContext(app, user)
+            return if (
+                LocationUtils.isLocationGroupAndProvider(userContext, permGroupName, packageName)
+            ) {
+                LocationUtils.isLocationEnabled(userContext)
+            } else if (
+                LocationUtils.isLocationGroupAndControllerExtraPackage(app, permGroupName, packageName)
+            ) {
+                // The permission of the extra location controller package is determined by the status
+                // of the controller package itself.
+                LocationUtils.isExtraLocationControllerPackageEnabled(userContext)
+            } else {
+                null
+            }
+        }
+
+        // Gallery role is static, so we only need to get the set gallery app once
+        private val systemGalleryApps: List<String> by lazy {
+            val roleManager = PermissionControllerApplication.get()
+                .getSystemService(RoleManager::class.java) ?: return@lazy emptyList()
+            roleManager.getRoleHolders(SYSTEM_GALLERY_ROLE_NAME)
+        }
+
+        fun isSpecialFixedStorageGranted(
+            app: Application,
+            packageName: String,
+            permGroupName: String,
+            uid: Int
+        ): Boolean {
+            if (permGroupName != READ_MEDIA_VISUAL && permGroupName != STORAGE) {
+                return false
+            }
+            if (packageName !in systemGalleryApps) {
+                return false
+            }
+            // This is the storage group, and the gallery app. Check the write media app op
+            val appOps = app.getSystemService(AppOpsManager::class.java)
+            return appOps.checkOpNoThrow(OPSTR_WRITE_MEDIA_IMAGES, uid, packageName) == MODE_ALLOWED
+        }
     }
 }
diff --git a/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightAppPermGroup.kt b/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightAppPermGroup.kt
index 61a604de8..18f4ce203 100644
--- a/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightAppPermGroup.kt
+++ b/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightAppPermGroup.kt
@@ -35,19 +35,24 @@ import com.android.permissioncontroller.permission.utils.Utils
  * @param specialLocationGrant If this package is the location provider, or the extra location
  *   package, then the grant state of the group is not determined by the grant state of individual
  *   permissions, but by other system properties
+ * @param specialFixedStorageGrant If this package holds the SYSTEM_GALLERY role, and has the
+ *   WRITE_MEDIA_IMAGES app op granted, then we should show the grant state of the storage
+ *   permissions as system fixed and granted.
+ *
  */
 data class LightAppPermGroup(
     val packageInfo: LightPackageInfo,
     val permGroupInfo: LightPermGroupInfo,
     val allPermissions: Map<String, LightPermission>,
     val hasInstallToRuntimeSplit: Boolean,
-    val specialLocationGrant: Boolean?
+    val specialLocationGrant: Boolean?,
+    val specialFixedStorageGrant: Boolean,
 ) {
     constructor(
         pI: LightPackageInfo,
         pGI: LightPermGroupInfo,
         perms: Map<String, LightPermission>
-    ) : this(pI, pGI, perms, false, null)
+    ) : this(pI, pGI, perms, false, null, false)
 
     /** All unrestricted permissions. Usually restricted permissions are ignored */
     val permissions: Map<String, LightPermission> =
@@ -83,7 +88,8 @@ data class LightAppPermGroup(
             permissions.filter { it.key in foregroundPermNames },
             packageInfo,
             isPlatformPermissionGroup,
-            specialLocationGrant
+            specialLocationGrant,
+            specialFixedStorageGrant
         )
 
     val background =
@@ -91,7 +97,8 @@ data class LightAppPermGroup(
             permissions.filter { it.key in backgroundPermNames },
             packageInfo,
             isPlatformPermissionGroup,
-            specialLocationGrant
+            specialLocationGrant,
+            specialFixedStorageGrant
         )
 
     /** Whether or not this App Permission Group has a permission which has a background mode */
@@ -167,18 +174,20 @@ data class LightAppPermGroup(
      *   contained in the full group
      * @param isPlatformPermissionGroup Whether this is a platform permission group
      * @param specialLocationGrant Whether this is a special location package
+     * @param specialFixedStorageGrant Whether this is a special storage grant
      */
     data class AppPermSubGroup
     internal constructor(
         private val permissions: Map<String, LightPermission>,
         private val packageInfo: LightPackageInfo,
         private val isPlatformPermissionGroup: Boolean,
-        private val specialLocationGrant: Boolean?
+        private val specialLocationGrant: Boolean?,
+        private val specialFixedStorageGrant: Boolean
     ) {
         /** Whether any of this App Permission SubGroup's permissions are granted */
         val isGranted =
             specialLocationGrant
-                ?: permissions.any {
+                ?: specialFixedStorageGrant || permissions.any {
                     val mayGrantByPlatformOrSystem =
                         !isPlatformPermissionGroup || it.value.isPlatformOrSystem
                     it.value.isGranted && mayGrantByPlatformOrSystem
@@ -216,7 +225,7 @@ data class LightAppPermGroup(
         val isPolicyFixed = permissions.any { it.value.isPolicyFixed }
 
         /** Whether any of this App Permission Subgroup's permissions are fixed by the system */
-        val isSystemFixed = permissions.any { it.value.isSystemFixed }
+        val isSystemFixed = permissions.any { it.value.isSystemFixed } || specialFixedStorageGrant
 
         /** Whether any of this App Permission Subgroup's permissions are fixed by the user */
         val isUserFixed = permissions.any { it.value.isUserFixed }
diff --git a/PermissionController/src/com/android/permissioncontroller/permission/service/RuntimePermissionsUpgradeController.kt b/PermissionController/src/com/android/permissioncontroller/permission/service/RuntimePermissionsUpgradeController.kt
index 85145f346..184c37921 100644
--- a/PermissionController/src/com/android/permissioncontroller/permission/service/RuntimePermissionsUpgradeController.kt
+++ b/PermissionController/src/com/android/permissioncontroller/permission/service/RuntimePermissionsUpgradeController.kt
@@ -503,7 +503,8 @@ object RuntimePermissionsUpgradeController {
                         bgApp.permGroupInfo,
                         allPermissionsWithxemption,
                         bgApp.hasInstallToRuntimeSplit,
-                        bgApp.specialLocationGrant
+                        bgApp.specialLocationGrant,
+                        bgApp.specialFixedStorageGrant,
                     )
             }
 
@@ -683,7 +684,8 @@ object RuntimePermissionsUpgradeController {
                             bgSensorsGroup.permGroupInfo,
                             allPermissionsWithExemption,
                             bgSensorsGroup.hasInstallToRuntimeSplit,
-                            bgSensorsGroup.specialLocationGrant
+                            bgSensorsGroup.specialLocationGrant,
+                            bgSensorsGroup.specialFixedStorageGrant,
                         )
 
                     // Grant the background permission only if foreground permission is granted.
diff --git a/PermissionController/src/com/android/permissioncontroller/permission/utils/KotlinUtils.kt b/PermissionController/src/com/android/permissioncontroller/permission/utils/KotlinUtils.kt
index 0701045f5..51f098371 100644
--- a/PermissionController/src/com/android/permissioncontroller/permission/utils/KotlinUtils.kt
+++ b/PermissionController/src/com/android/permissioncontroller/permission/utils/KotlinUtils.kt
@@ -737,6 +737,7 @@ object KotlinUtils {
             newPerms,
             group.hasInstallToRuntimeSplit,
             group.specialLocationGrant,
+            group.specialFixedStorageGrant,
         )
     }
 
@@ -857,6 +858,7 @@ object KotlinUtils {
                 newPerms,
                 group.hasInstallToRuntimeSplit,
                 group.specialLocationGrant,
+                group.specialFixedStorageGrant,
             )
         // If any permission in the group is one time granted, start one time permission session.
         if (newGroup.permissions.any { it.value.isOneTime && it.value.isGranted }) {
@@ -1139,6 +1141,7 @@ object KotlinUtils {
                 newPerms,
                 group.hasInstallToRuntimeSplit,
                 group.specialLocationGrant,
+                group.specialFixedStorageGrant,
             )
 
         if (wasOneTime && !anyPermsOfPackageOneTimeGranted(app, newGroup.packageInfo, newGroup)) {
diff --git a/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/utils/GrantRevokeTests.kt b/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/utils/GrantRevokeTests.kt
index 28f69b136..593061a83 100644
--- a/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/utils/GrantRevokeTests.kt
+++ b/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/utils/GrantRevokeTests.kt
@@ -241,7 +241,7 @@ class GrantRevokeTests {
         perms: Map<String, LightPermission> = emptyMap()
     ): LightAppPermGroup {
         val pGi = LightPermGroupInfo(PERM_GROUP_NAME, TEST_PACKAGE_NAME, 0, 0, 0, false)
-        return LightAppPermGroup(pkgInfo, pGi, perms, false, false)
+        return LightAppPermGroup(pkgInfo, pGi, perms, false, false, false)
     }
 
     /**
diff --git a/tests/cts/permissionui/src/android/permissionui/cts/BaseUsePermissionTest.kt b/tests/cts/permissionui/src/android/permissionui/cts/BaseUsePermissionTest.kt
index bd22cb5bc..ccc5a0a5e 100644
--- a/tests/cts/permissionui/src/android/permissionui/cts/BaseUsePermissionTest.kt
+++ b/tests/cts/permissionui/src/android/permissionui/cts/BaseUsePermissionTest.kt
@@ -1129,9 +1129,16 @@ abstract class BaseUsePermissionTest : BasePermissionTest() {
             }
         }
     }
+    protected fun navigateToIndividualPermissionSetting(
+        permission: String,
+        manuallyNavigate: Boolean = false,
+    ) {
+        navigateToIndividualPermissionSetting(permission, APP_PACKAGE_NAME, manuallyNavigate)
+    }
 
     protected fun navigateToIndividualPermissionSetting(
         permission: String,
+        packageName: String,
         manuallyNavigate: Boolean = false,
     ) {
         val useLegacyNavigation = isWatch || isAutomotive || manuallyNavigate
@@ -1150,7 +1157,7 @@ abstract class BaseUsePermissionTest : BasePermissionTest() {
             runWithShellPermissionIdentity {
                 context.startActivity(
                     Intent(Intent.ACTION_MANAGE_APP_PERMISSION).apply {
-                        putExtra(Intent.EXTRA_PACKAGE_NAME, APP_PACKAGE_NAME)
+                        putExtra(Intent.EXTRA_PACKAGE_NAME, packageName)
                         putExtra(Intent.EXTRA_PERMISSION_NAME, permission)
                         putExtra(Intent.EXTRA_USER, Process.myUserHandle())
                         addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
diff --git a/tests/cts/permissionui/src/android/permissionui/cts/MediaPermissionTest.kt b/tests/cts/permissionui/src/android/permissionui/cts/MediaPermissionTest.kt
index d41c7454f..5e2e40d20 100644
--- a/tests/cts/permissionui/src/android/permissionui/cts/MediaPermissionTest.kt
+++ b/tests/cts/permissionui/src/android/permissionui/cts/MediaPermissionTest.kt
@@ -17,12 +17,18 @@
 package android.permissionui.cts
 
 import android.Manifest
+import android.app.role.RoleManager
+import android.content.pm.PackageManager
 import android.os.Build
+import android.platform.test.annotations.AsbSecurityTest
 import androidx.test.filters.FlakyTest
 import androidx.test.filters.SdkSuppress
+import androidx.test.uiautomator.By
 import com.android.compatibility.common.util.CddTest
 import com.android.compatibility.common.util.SystemUtil
+import org.junit.Assert
 import org.junit.Assume
+import org.junit.Assume.assumeTrue
 import org.junit.Test
 
 /**
@@ -180,4 +186,33 @@ class MediaPermissionTest : BaseUsePermissionTest() {
         assertAppHasPermission(Manifest.permission.READ_MEDIA_VIDEO, true)
         assertAppHasPermission(Manifest.permission.READ_MEDIA_IMAGES, true)
     }
+
+
+    @Test
+    @AsbSecurityTest(cveBugId = [315320090])
+    fun testGalleryAppListedAsFixed() {
+        val galleryPkgs = SystemUtil.callWithShellPermissionIdentity {
+            context.getSystemService(RoleManager::class.java)
+                .getRoleHolders(SYSTEM_GALLERY_ROLE_NAME)
+        }
+        assumeTrue(galleryPkgs.isNotEmpty())
+        val galleryPkg = galleryPkgs[0]
+        val checkPermissionResult = SystemUtil.callWithShellPermissionIdentity {
+            packageManager.checkPermission(Manifest.permission.READ_MEDIA_IMAGES, galleryPkg)
+        }
+        assumeTrue(checkPermissionResult == PackageManager.PERMISSION_GRANTED)
+        navigateToIndividualPermissionSetting(Manifest.permission.READ_MEDIA_IMAGES, galleryPkg)
+        // Attempt to deny the permission. It should not show the
+        // "denying default permission dialog"
+        click(By.res(DENY_RADIO_BUTTON))
+        try {
+            waitFindObject(By.res(CANCEL_BUTTON_ID), 1000L)
+            Assert.fail("expected not to find the default deny dialog")
+        } catch (_: Exception) {}
+    }
+
+    companion object {
+        private val SYSTEM_GALLERY_ROLE_NAME = "android.app.role.SYSTEM_GALLERY"
+        private val CANCEL_BUTTON_ID = "android:id/button1"
+    }
 }