Fix several issues with Storage permission UI
Ensure UserPackageInfos live data updates on permission changes, ensure
that UI correctly handles case where app may have special app access,
but not permission, ensure special app access is calculated correctly,
kills app on app op change.
Fixes: 160166191
Test: atest CtsPermissionTestCases CtsPermission3TestCases
Change-Id: Ie194f41d11da3316350e2715049c55d596f45b19
diff --git a/PermissionController/src/com/android/permissioncontroller/permission/data/FullStoragePermissionAppsLiveData.kt b/PermissionController/src/com/android/permissioncontroller/permission/data/FullStoragePermissionAppsLiveData.kt
index 99bddff..531600e 100644
--- a/PermissionController/src/com/android/permissioncontroller/permission/data/FullStoragePermissionAppsLiveData.kt
+++ b/PermissionController/src/com/android/permissioncontroller/permission/data/FullStoragePermissionAppsLiveData.kt
@@ -16,10 +16,12 @@
package com.android.permissioncontroller.permission.data
-import android.Manifest
+import android.Manifest.permission.MANAGE_EXTERNAL_STORAGE
import android.Manifest.permission_group.STORAGE
import android.app.AppOpsManager
import android.app.AppOpsManager.MODE_ALLOWED
+import android.app.AppOpsManager.MODE_DEFAULT
+import android.app.AppOpsManager.MODE_FOREGROUND
import android.app.AppOpsManager.OPSTR_LEGACY_STORAGE
import android.app.AppOpsManager.OPSTR_MANAGE_EXTERNAL_STORAGE
import android.app.Application
@@ -64,7 +66,7 @@
for ((user, packageInfoList) in AllPackageInfosLiveData.value ?: emptyMap()) {
val userPackages = packageInfoList.filter {
storagePackages.contains(it.packageName to user) ||
- it.requestedPermissions.contains(Manifest.permission.MANAGE_EXTERNAL_STORAGE)
+ it.requestedPermissions.contains(MANAGE_EXTERNAL_STORAGE)
}
for (packageInfo in userPackages) {
@@ -80,10 +82,12 @@
isLegacy = true, isGranted = true))
continue
}
- if (packageInfo.requestedPermissions.contains(
- Manifest.permission.MANAGE_EXTERNAL_STORAGE)) {
- val granted = appOpsManager.unsafeCheckOpNoThrow(OPSTR_MANAGE_EXTERNAL_STORAGE,
- packageInfo.uid, packageInfo.packageName) == MODE_ALLOWED
+ if (MANAGE_EXTERNAL_STORAGE in packageInfo.requestedPermissions) {
+ val mode = appOpsManager.unsafeCheckOpNoThrow(OPSTR_MANAGE_EXTERNAL_STORAGE,
+ packageInfo.uid, packageInfo.packageName)
+ val granted = mode == MODE_ALLOWED || mode == MODE_FOREGROUND ||
+ (mode == MODE_DEFAULT &&
+ MANAGE_EXTERNAL_STORAGE in packageInfo.grantedPermissions)
fullStoragePackages.add(FullStoragePackageState(packageInfo.packageName, user,
isLegacy = false, isGranted = granted))
}
@@ -97,4 +101,12 @@
super.onActive()
updateAsync()
}
+
+ /**
+ * Recalculate the LiveData
+ * TODO ntmyren: Make livedata properly observe app ops
+ */
+ fun recalculate() {
+ updateAsync()
+ }
}
\ No newline at end of file
diff --git a/PermissionController/src/com/android/permissioncontroller/permission/data/LightPackageInfoLiveData.kt b/PermissionController/src/com/android/permissioncontroller/permission/data/LightPackageInfoLiveData.kt
index 437b0e7..4c8039e 100644
--- a/PermissionController/src/com/android/permissioncontroller/permission/data/LightPackageInfoLiveData.kt
+++ b/PermissionController/src/com/android/permissioncontroller/permission/data/LightPackageInfoLiveData.kt
@@ -118,7 +118,8 @@
registeredUid = uid
PermissionListenerMultiplexer.addCallback(it, this)
}
- if (userPackagesLiveData.hasActiveObservers() && !watchingUserPackagesLiveData) {
+ if (userPackagesLiveData.hasActiveObservers() && !watchingUserPackagesLiveData &&
+ !userPackagesLiveData.permChangeStale) {
watchingUserPackagesLiveData = true
addSource(userPackagesLiveData, userPackageInfosObserver)
} else {
diff --git a/PermissionController/src/com/android/permissioncontroller/permission/data/UserPackageInfosLiveData.kt b/PermissionController/src/com/android/permissioncontroller/permission/data/UserPackageInfosLiveData.kt
index 7016164..caeb0fa 100644
--- a/PermissionController/src/com/android/permissioncontroller/permission/data/UserPackageInfosLiveData.kt
+++ b/PermissionController/src/com/android/permissioncontroller/permission/data/UserPackageInfosLiveData.kt
@@ -35,12 +35,39 @@
private val app: Application,
private val user: UserHandle
) : SmartAsyncMediatorLiveData<@kotlin.jvm.JvmSuppressWildcards List<LightPackageInfo>>(),
- PackageBroadcastReceiver.PackageBroadcastListener {
+ PackageBroadcastReceiver.PackageBroadcastListener,
+ PermissionListenerMultiplexer.PermissionChangeCallback {
+
+ /**
+ * Whether or not the permissions in this liveData are out of date
+ */
+ var permChangeStale = false
override fun onPackageUpdate(packageName: String) {
updateAsync()
}
+ // TODO ntmyren: replace with correctly updating
+ override fun onPermissionChange() {
+ permChangeStale = true
+ for (packageInfo in value ?: emptyList()) {
+ PermissionListenerMultiplexer.removeCallback(packageInfo.uid, this)
+ }
+ }
+
+ override fun setValue(newValue: List<LightPackageInfo>?) {
+ if (newValue != value) {
+ for (packageInfo in value ?: emptyList()) {
+ PermissionListenerMultiplexer.removeCallback(packageInfo.uid, this)
+ }
+ for (packageInfo in newValue ?: emptyList()) {
+ PermissionListenerMultiplexer.addCallback(packageInfo.uid, this)
+ }
+ }
+ super.setValue(newValue)
+ permChangeStale = false
+ }
+
/**
* Get all of the packages in the system, organized by user.
*/
@@ -53,6 +80,7 @@
"${user.identifier}")
val packageInfos = app.applicationContext.packageManager
.getInstalledPackagesAsUser(GET_PERMISSIONS or MATCH_ALL, user.identifier)
+
postValue(packageInfos.map { packageInfo -> LightPackageInfo(packageInfo) })
}
@@ -60,12 +88,21 @@
super.onActive()
PackageBroadcastReceiver.addAllCallback(this)
+
+ for (packageInfo in value ?: emptyList()) {
+ PermissionListenerMultiplexer.addCallback(packageInfo.uid, this)
+ }
+
updateAsync()
}
override fun onInactive() {
super.onInactive()
+ for (packageInfo in value ?: emptyList()) {
+ PermissionListenerMultiplexer.removeCallback(packageInfo.uid, this)
+ }
+
PackageBroadcastReceiver.removeAllCallback(this)
}
diff --git a/PermissionController/src/com/android/permissioncontroller/permission/ui/handheld/AppPermissionFragment.java b/PermissionController/src/com/android/permissioncontroller/permission/ui/handheld/AppPermissionFragment.java
index 13d111e..6a7002d 100644
--- a/PermissionController/src/com/android/permissioncontroller/permission/ui/handheld/AppPermissionFragment.java
+++ b/PermissionController/src/com/android/permissioncontroller/permission/ui/handheld/AppPermissionFragment.java
@@ -359,6 +359,11 @@
setResult(DENIED);
});
mDenyButton.setOnClickListener((v) -> {
+
+ if (mViewModel.getFullStorageStateLiveData().getValue() != null
+ && !mViewModel.getFullStorageStateLiveData().getValue().isLegacy()) {
+ mViewModel.setAllFilesAccess(false);
+ }
mViewModel.requestChange(false, this, this, ChangeRequest.REVOKE_BOTH,
APP_PERMISSION_FRAGMENT_ACTION_REPORTED__BUTTON_PRESSED__DENY);
setResult(DENIED_DO_NOT_ASK_AGAIN);
@@ -421,12 +426,8 @@
return;
}
- if (storageState.isGranted()) {
- textView.setText(R.string.app_permission_footer_special_file_access);
- textView.setVisibility(View.VISIBLE);
- } else {
- textView.setVisibility(View.GONE);
- }
+ textView.setText(R.string.app_permission_footer_special_file_access);
+ textView.setVisibility(View.VISIBLE);
}
private void setResult(@GrantPermissionsViewHandler.Result int result) {
@@ -532,7 +533,7 @@
private static final String KEY = ConfirmDialog.class.getName() + ".arg.key";
private static final String BUTTON = ConfirmDialog.class.getName() + ".arg.button";
private static final String ONE_TIME = ConfirmDialog.class.getName() + ".arg.onetime";
-
+ private static int sCode = APP_PERMISSION_FRAGMENT_ACTION_REPORTED__BUTTON_PRESSED__ALLOW;
@Override
public Dialog onCreateDialog(Bundle savedInstanceState) {
AppPermissionFragment fragment = (AppPermissionFragment) getParentFragment();
@@ -550,6 +551,8 @@
(DialogInterface dialog, int which) -> {
if (isGrantFileAccess) {
fragment.mViewModel.setAllFilesAccess(true);
+ fragment.mViewModel.requestChange(false, fragment,
+ fragment, ChangeRequest.GRANT_BOTH, sCode);
} else {
fragment.mViewModel.onDenyAnyWay((ChangeRequest)
getArguments().getSerializable(CHANGE_REQUEST),
diff --git a/PermissionController/src/com/android/permissioncontroller/permission/ui/handheld/PermissionAppsFragment.java b/PermissionController/src/com/android/permissioncontroller/permission/ui/handheld/PermissionAppsFragment.java
index c192166..b73b426 100644
--- a/PermissionController/src/com/android/permissioncontroller/permission/ui/handheld/PermissionAppsFragment.java
+++ b/PermissionController/src/com/android/permissioncontroller/permission/ui/handheld/PermissionAppsFragment.java
@@ -131,8 +131,6 @@
setLoading(true /* loading */, false /* animate */);
}
}, SHOW_LOAD_DELAY_MS);
- } else if (mViewModel.getCategorizedAppsLiveData().getValue() != null) {
- onPackagesLoaded(mViewModel.getCategorizedAppsLiveData().getValue());
}
setHasOptionsMenu(true);
diff --git a/PermissionController/src/com/android/permissioncontroller/permission/ui/model/AppPermissionViewModel.kt b/PermissionController/src/com/android/permissioncontroller/permission/ui/model/AppPermissionViewModel.kt
index 0642495..46af48a 100644
--- a/PermissionController/src/com/android/permissioncontroller/permission/ui/model/AppPermissionViewModel.kt
+++ b/PermissionController/src/com/android/permissioncontroller/permission/ui/model/AppPermissionViewModel.kt
@@ -19,7 +19,7 @@
import android.Manifest
import android.app.AppOpsManager
import android.app.AppOpsManager.MODE_ALLOWED
-import android.app.AppOpsManager.MODE_IGNORED
+import android.app.AppOpsManager.MODE_ERRORED
import android.app.AppOpsManager.OPSTR_MANAGE_EXTERNAL_STORAGE
import android.app.Application
import android.content.Intent
@@ -33,7 +33,6 @@
import androidx.lifecycle.ViewModel
import androidx.lifecycle.ViewModelProvider
import androidx.navigation.fragment.findNavController
-import com.android.permissioncontroller.PermissionControllerApplication
import com.android.permissioncontroller.PermissionControllerStatsLog
import com.android.permissioncontroller.PermissionControllerStatsLog.APP_PERMISSION_FRAGMENT_ACTION_REPORTED
import com.android.permissioncontroller.PermissionControllerStatsLog.APP_PERMISSION_FRAGMENT_VIEWED
@@ -343,6 +342,7 @@
allowedAllFilesState.isShown = true
if (storageState.isGranted) {
allowedAllFilesState.isChecked = true
+ deniedState.isChecked = false
}
} else {
allowedAllFilesState.isEnabled = false
@@ -576,6 +576,10 @@
}
logPermissionChanges(oldGroup, newGroup, buttonClicked)
+
+ fullStorageStateLiveData.value?.let {
+ FullStoragePermissionAppsLiveData.recalculate()
+ }
}
/**
@@ -620,18 +624,30 @@
if (hasDefaultPermissions || !group.supportsRuntimePerms) {
hasConfirmedRevoke = true
}
+
+ fullStorageStateLiveData.value?.let {
+ FullStoragePermissionAppsLiveData.recalculate()
+ }
}
+ /**
+ * Set the All Files access for this app
+ *
+ * @param granted Whether to grant or revoke access
+ */
fun setAllFilesAccess(granted: Boolean) {
- val aom =
- PermissionControllerApplication.get().getSystemService(AppOpsManager::class.java)!!
+ val aom = app.getSystemService(AppOpsManager::class.java)!!
val uid = lightAppPermGroup?.packageInfo?.uid ?: return
val mode = if (granted) {
MODE_ALLOWED
} else {
- MODE_IGNORED
+ MODE_ERRORED
}
- aom.setUidMode(OPSTR_MANAGE_EXTERNAL_STORAGE, uid, mode)
+ val fullStorageGrant = fullStorageStateLiveData.value?.isGranted
+ if (fullStorageGrant != null && fullStorageGrant != granted) {
+ aom.setUidMode(OPSTR_MANAGE_EXTERNAL_STORAGE, uid, mode)
+ FullStoragePermissionAppsLiveData.recalculate()
+ }
}
/**
diff --git a/PermissionController/src/com/android/permissioncontroller/permission/ui/model/PermissionAppsViewModel.kt b/PermissionController/src/com/android/permissioncontroller/permission/ui/model/PermissionAppsViewModel.kt
index 3772cd7..c7a13db 100644
--- a/PermissionController/src/com/android/permissioncontroller/permission/ui/model/PermissionAppsViewModel.kt
+++ b/PermissionController/src/com/android/permissioncontroller/permission/ui/model/PermissionAppsViewModel.kt
@@ -88,15 +88,6 @@
init {
var fullStorageLiveData: FullStoragePermissionAppsLiveData? = null
- addSource(packagesUiInfoLiveData) {
- if (fullStorageLiveData == null || fullStorageLiveData?.isInitialized == true)
- update()
- }
- addSource(shouldShowSystemLiveData) {
- if (fullStorageLiveData == null || fullStorageLiveData?.isInitialized == true)
- update()
- }
-
// If this is the Storage group, observe a FullStoragePermissionAppsLiveData, update
// the packagesWithFullFileAccess list, and call update to populate the subtitles.
if (groupName == Manifest.permission_group.STORAGE) {
@@ -104,7 +95,6 @@
addSource(FullStoragePermissionAppsLiveData) { fullAccessPackages ->
if (fullAccessPackages != packagesWithFullFileAccess) {
packagesWithFullFileAccess = fullAccessPackages.filter { it.isGranted }
- ?: emptyList()
if (packagesUiInfoLiveData.isInitialized) {
update()
}
@@ -112,8 +102,19 @@
}
}
+ addSource(packagesUiInfoLiveData) {
+ if (fullStorageLiveData == null || fullStorageLiveData.isInitialized)
+ update()
+ }
+ addSource(shouldShowSystemLiveData) {
+ if (fullStorageLiveData == null || fullStorageLiveData.isInitialized)
+ update()
+ }
+
if ((fullStorageLiveData == null || fullStorageLiveData.isInitialized) &&
packagesUiInfoLiveData.isInitialized) {
+ packagesWithFullFileAccess = fullStorageLiveData?.value?.filter { it.isGranted }
+ ?: emptyList()
update()
}
}
@@ -155,13 +156,19 @@
showAlwaysAllowedString = true
}
- val category = when (uiInfo.permGrantState) {
+ var category = when (uiInfo.permGrantState) {
PermGrantState.PERMS_ALLOWED -> Category.ALLOWED
PermGrantState.PERMS_ALLOWED_FOREGROUND_ONLY -> Category.ALLOWED_FOREGROUND
PermGrantState.PERMS_ALLOWED_ALWAYS -> Category.ALLOWED
PermGrantState.PERMS_DENIED -> Category.DENIED
PermGrantState.PERMS_ASK -> Category.ASK
}
+
+ if (groupName == Manifest.permission_group.STORAGE &&
+ packagesWithFullFileAccess.any { !it.isLegacy && it.isGranted &&
+ it.packageName to it.user == packageUserPair }) {
+ category = Category.ALLOWED
+ }
categoryMap[category]!!.add(packageUserPair)
}
showAllowAlwaysStringLiveData.value = showAlwaysAllowedString
