summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Nate Myren <ntmyren@google.com> 2025-01-09 16:38:46 -0800
committer Nate Myren <ntmyren@google.com> 2025-01-31 16:04:38 -0800
commit17f747d7ea2e2bb9ac5bbb7654bd5354eb4632d0 (patch)
tree093389227c1e601493397dba85b39a883d868524
parent4d9853a88e879df10068caaca5acd1f2f4258760 (diff)
Display the system gallery storage permission as system fixed
Due to the WRITE_MEDIA_IMAGES app op, photos access cannot actually be denied. Since the role is fixed, the permission effectively is, too Bug: 315320090 Test: atest MediaPermissionTest#testGalleryAppListedAsFixed Flag: EXEMPT see bug Relnote: none Change-Id: If9ae5366fe80fcd9eaf9c45dcee13e163207fbbe
-rw-r--r--PermissionController/src/com/android/permissioncontroller/permission/data/AppPermGroupUiInfoLiveData.kt40
-rw-r--r--PermissionController/src/com/android/permissioncontroller/permission/data/LightAppPermGroupLiveData.kt73
-rw-r--r--PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightAppPermGroup.kt23
-rw-r--r--PermissionController/src/com/android/permissioncontroller/permission/service/RuntimePermissionsUpgradeController.kt6
-rw-r--r--PermissionController/src/com/android/permissioncontroller/permission/utils/KotlinUtils.kt3
-rw-r--r--PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/utils/GrantRevokeTests.kt2
-rw-r--r--tests/cts/permissionui/src/android/permissionui/cts/BaseUsePermissionTest.kt9
-rw-r--r--tests/cts/permissionui/src/android/permissionui/cts/MediaPermissionTest.kt35
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"
+ }
}