Remove REVIEW_REQUIRED temporary grant for notifications

Bug: 231715786
Test: atest PermissionHelperTest, NotificationPermissionsTest
Change-Id: I342a5e78e8c33250f632ce802dd897a1380c4679
Merged-In: I342a5e78e8c33250f632ce802dd897a1380c4679
This commit is contained in:
Nate Myren
2022-05-09 14:57:53 -07:00
parent eccd61221b
commit f37429291e
5 changed files with 19 additions and 136 deletions

View File

@@ -16,7 +16,6 @@
package com.android.server.notification;
import static android.content.pm.PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED;
import static android.content.pm.PackageManager.FLAG_PERMISSION_USER_SET;
import static android.content.pm.PackageManager.GET_PERMISSIONS;
import static android.content.pm.PackageManager.PERMISSION_GRANTED;
@@ -70,8 +69,7 @@ public final class PermissionHelper {
public boolean hasPermission(int uid) {
final long callingId = Binder.clearCallingIdentity();
try {
return mPmi.checkPostNotificationsPermissionGrantedOrLegacyAccess(uid)
== PERMISSION_GRANTED;
return mPmi.checkUidPermission(uid, NOTIFICATION_PERMISSION) == PERMISSION_GRANTED;
} finally {
Binder.restoreCallingIdentity(callingId);
}
@@ -150,14 +148,6 @@ public final class PermissionHelper {
return notifPermissions;
}
/**
* @see setNotificationPermission(String, int, boolean, boolean, boolean)
*/
public void setNotificationPermission(String packageName, @UserIdInt int userId, boolean grant,
boolean userSet) {
setNotificationPermission(packageName, userId, grant, userSet, false);
}
/**
* Grants or revokes the notification permission for a given package/user. UserSet should
* only be true if this method is being called to migrate existing user choice, because it
@@ -165,7 +155,7 @@ public final class PermissionHelper {
* with a lock held.
*/
public void setNotificationPermission(String packageName, @UserIdInt int userId, boolean grant,
boolean userSet, boolean reviewRequired) {
boolean userSet) {
final long callingId = Binder.clearCallingIdentity();
try {
// Do not change the permission if the package doesn't request it, do not change fixed
@@ -179,7 +169,7 @@ public final class PermissionHelper {
boolean currentlyGranted = mPmi.checkPermission(packageName, NOTIFICATION_PERMISSION,
userId) != PackageManager.PERMISSION_DENIED;
if (grant && !reviewRequired && !currentlyGranted) {
if (grant && !currentlyGranted) {
mPermManager.grantRuntimePermission(packageName, NOTIFICATION_PERMISSION, userId);
} else if (!grant && currentlyGranted) {
mPermManager.revokeRuntimePermission(packageName, NOTIFICATION_PERMISSION,
@@ -187,12 +177,10 @@ public final class PermissionHelper {
}
if (userSet) {
mPermManager.updatePermissionFlags(packageName, NOTIFICATION_PERMISSION,
FLAG_PERMISSION_USER_SET | FLAG_PERMISSION_REVIEW_REQUIRED,
FLAG_PERMISSION_USER_SET, true, userId);
} else if (reviewRequired) {
FLAG_PERMISSION_USER_SET, FLAG_PERMISSION_USER_SET, true, userId);
} else {
mPermManager.updatePermissionFlags(packageName, NOTIFICATION_PERMISSION,
FLAG_PERMISSION_REVIEW_REQUIRED, FLAG_PERMISSION_REVIEW_REQUIRED, true,
userId);
0, FLAG_PERMISSION_USER_SET, true, userId);
}
} catch (RemoteException e) {
Slog.e(TAG, "Could not reach system server", e);

View File

@@ -17,7 +17,6 @@
package com.android.server.pm.permission;
import static android.Manifest.permission.CAPTURE_AUDIO_HOTWORD;
import static android.Manifest.permission.POST_NOTIFICATIONS;
import static android.Manifest.permission.RECORD_AUDIO;
import static android.Manifest.permission.UPDATE_APP_OPS_STATS;
import static android.app.AppOpsManager.ATTRIBUTION_CHAIN_ID_NONE;
@@ -51,7 +50,6 @@ import android.content.pm.PermissionGroupInfo;
import android.content.pm.PermissionInfo;
import android.content.pm.permission.SplitPermissionInfoParcelable;
import android.os.Binder;
import android.os.Build;
import android.os.IBinder;
import android.os.Process;
import android.os.RemoteException;
@@ -595,26 +593,6 @@ public class PermissionManagerService extends IPermissionManager.Stub {
return PermissionManagerService.this.checkUidPermission(uid, permissionName);
}
@Override
public int checkPostNotificationsPermissionGrantedOrLegacyAccess(int uid) {
int granted = PermissionManagerService.this.checkUidPermission(uid,
POST_NOTIFICATIONS);
AndroidPackage pkg = mPackageManagerInt.getPackage(uid);
if (pkg == null) {
Slog.e(LOG_TAG, "No package for uid " + uid);
return granted;
}
if (granted != PackageManager.PERMISSION_GRANTED
&& pkg.getTargetSdkVersion() >= Build.VERSION_CODES.M) {
int flags = PermissionManagerService.this.getPermissionFlags(pkg.getPackageName(),
POST_NOTIFICATIONS, UserHandle.getUserId(uid));
if ((flags & PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED) != 0) {
return PackageManager.PERMISSION_GRANTED;
}
}
return granted;
}
@Override
public void startShellPermissionIdentityDelegation(int uid, @NonNull String packageName,
@Nullable List<String> permissionNames) {

View File

@@ -62,17 +62,6 @@ public interface PermissionManagerServiceInternal extends PermissionManagerInter
//@SystemApi(client = SystemApi.Client.SYSTEM_SERVER)
int checkUidPermission(int uid, @NonNull String permissionName);
/**
* Check whether a particular UID has been granted the POST_NOTIFICATIONS permission, or if
* access should be granted based on legacy access (currently symbolized by the REVIEW_REQUIRED
* permission flag
*
* @param uid the UID
* @return {@code PERMISSION_GRANTED} if the permission is granted, or legacy access is granted,
* {@code PERMISSION_DENIED} otherwise
*/
int checkPostNotificationsPermissionGrantedOrLegacyAccess(int uid);
/**
* Adds a listener for runtime permission state (permissions or flags) changes.
*

View File

@@ -915,8 +915,7 @@ public final class PermissionPolicyService extends SystemService {
int permissionFlags = mPackageManager.getPermissionFlags(permissionName,
packageName, mContext.getUser());
boolean isReviewRequired = (permissionFlags & FLAG_PERMISSION_REVIEW_REQUIRED) != 0;
if (isReviewRequired && !CompatChanges.isChangeEnabled(
NOTIFICATION_PERM_CHANGE_ID, packageName, user)) {
if (isReviewRequired) {
return;
}
@@ -1118,48 +1117,13 @@ public final class PermissionPolicyService extends SystemService {
private class Internal extends PermissionPolicyInternal {
// UIDs that, if a grant dialog is shown for POST_NOTIFICATIONS before next reboot,
// should display a "continue allowing" message, rather than an "allow" message
private final ArraySet<Integer> mContinueNotifGrantMessageUids = new ArraySet<>();
private final ActivityInterceptorCallback mActivityInterceptorCallback =
new ActivityInterceptorCallback() {
@Nullable
@Override
public ActivityInterceptorCallback.ActivityInterceptResult intercept(
ActivityInterceptorInfo info) {
String action = info.intent.getAction();
ActivityInterceptResult result = null;
if (!ACTION_REQUEST_PERMISSIONS_FOR_OTHER.equals(action)
&& !PackageManager.ACTION_REQUEST_PERMISSIONS.equals(action)) {
return null;
}
// Only this interceptor can add LEGACY_ACCESS_PERMISSION_NAMES
if (info.intent.getStringArrayExtra(PackageManager
.EXTRA_REQUEST_PERMISSIONS_LEGACY_ACCESS_PERMISSION_NAMES)
!= null) {
result = new ActivityInterceptResult(
new Intent(info.intent), info.checkedOptions);
result.intent.removeExtra(PackageManager
.EXTRA_REQUEST_PERMISSIONS_LEGACY_ACCESS_PERMISSION_NAMES);
}
if (PackageManager.ACTION_REQUEST_PERMISSIONS.equals(action)
&& !mContinueNotifGrantMessageUids.contains(info.realCallingUid)) {
return result;
}
if (ACTION_REQUEST_PERMISSIONS_FOR_OTHER.equals(action)) {
String otherPkg = info.intent.getStringExtra(Intent.EXTRA_PACKAGE_NAME);
if (otherPkg == null || (mPackageManager.getPermissionFlags(
POST_NOTIFICATIONS, otherPkg, UserHandle.of(info.userId))
& FLAG_PERMISSION_REVIEW_REQUIRED) == 0) {
return result;
}
}
mContinueNotifGrantMessageUids.remove(info.realCallingUid);
return new ActivityInterceptResult(info.intent.putExtra(PackageManager
.EXTRA_REQUEST_PERMISSIONS_LEGACY_ACCESS_PERMISSION_NAMES,
new String[] { POST_NOTIFICATIONS }), info.checkedOptions);
return null;
}
@Override
@@ -1173,10 +1137,8 @@ public final class PermissionPolicyService extends SystemService {
return;
}
UserHandle user = UserHandle.of(taskInfo.userId);
if (CompatChanges.isChangeEnabled(NOTIFICATION_PERM_CHANGE_ID,
if (!CompatChanges.isChangeEnabled(NOTIFICATION_PERM_CHANGE_ID,
activityInfo.packageName, user)) {
clearNotificationReviewFlagsIfNeeded(activityInfo.packageName, user);
} else {
// Post the activity start checks to ensure the notification channel
// checks happen outside the WindowManager global lock.
mHandler.post(() -> showNotificationPromptIfNeeded(
@@ -1337,22 +1299,6 @@ public final class PermissionPolicyService extends SystemService {
&& isLauncherIntent(taskInfo.baseIntent);
}
private void clearNotificationReviewFlagsIfNeeded(String packageName, UserHandle user) {
if ((mPackageManager.getPermissionFlags(POST_NOTIFICATIONS, packageName, user)
& FLAG_PERMISSION_REVIEW_REQUIRED) == 0) {
return;
}
try {
int uid = mPackageManager.getPackageUidAsUser(packageName, 0,
user.getIdentifier());
mContinueNotifGrantMessageUids.add(uid);
mPackageManager.updatePermissionFlags(POST_NOTIFICATIONS, packageName,
FLAG_PERMISSION_REVIEW_REQUIRED, 0, user);
} catch (PackageManager.NameNotFoundException e) {
// Do nothing
}
}
private void launchNotificationPermissionRequestDialog(String pkgName, UserHandle user,
int taskId, @Nullable ActivityInterceptorInfo info) {
Intent grantPermission = mPackageManager
@@ -1469,8 +1415,7 @@ public final class PermissionPolicyService extends SystemService {
== PackageManager.PERMISSION_GRANTED;
int flags = mPackageManager.getPermissionFlags(POST_NOTIFICATIONS, pkgName, user);
boolean explicitlySet = (flags & PermissionManager.EXPLICIT_SET_FLAGS) != 0;
boolean needsReview = (flags & FLAG_PERMISSION_REVIEW_REQUIRED) != 0;
return !granted && hasCreatedNotificationChannels && (needsReview || !explicitlySet);
return !granted && hasCreatedNotificationChannels && !explicitlySet;
}
}
}

View File

@@ -17,7 +17,6 @@ package com.android.server.notification;
import static android.content.pm.PackageManager.FLAG_PERMISSION_GRANTED_BY_DEFAULT;
import static android.content.pm.PackageManager.FLAG_PERMISSION_POLICY_FIXED;
import static android.content.pm.PackageManager.FLAG_PERMISSION_REVIEW_REQUIRED;
import static android.content.pm.PackageManager.FLAG_PERMISSION_SYSTEM_FIXED;
import static android.content.pm.PackageManager.FLAG_PERMISSION_USER_SET;
import static android.content.pm.PackageManager.GET_PERMISSIONS;
@@ -87,12 +86,12 @@ public class PermissionHelperTest extends UiServiceTestCase {
@Test
public void testHasPermission() throws Exception {
when(mPmi.checkPostNotificationsPermissionGrantedOrLegacyAccess(anyInt()))
when(mPmi.checkUidPermission(anyInt(), anyString()))
.thenReturn(PERMISSION_GRANTED);
assertThat(mPermissionHelper.hasPermission(1)).isTrue();
when(mPmi.checkPostNotificationsPermissionGrantedOrLegacyAccess(anyInt()))
when(mPmi.checkUidPermission(anyInt(), anyString()))
.thenReturn(PERMISSION_DENIED);
assertThat(mPermissionHelper.hasPermission(1)).isFalse();
@@ -184,21 +183,7 @@ public class PermissionHelperTest extends UiServiceTestCase {
verify(mPermManager).grantRuntimePermission(
"pkg", Manifest.permission.POST_NOTIFICATIONS, 10);
verify(mPermManager).updatePermissionFlags("pkg", Manifest.permission.POST_NOTIFICATIONS,
FLAG_PERMISSION_USER_SET | FLAG_PERMISSION_REVIEW_REQUIRED,
FLAG_PERMISSION_USER_SET, true, 10);
}
@Test
public void testSetNotificationPermission_grantReviewRequired() throws Exception {
when(mPmi.checkPermission(anyString(), anyString(), anyInt()))
.thenReturn(PERMISSION_DENIED);
mPermissionHelper.setNotificationPermission("pkg", 10, true, false, true);
verify(mPermManager, never()).revokeRuntimePermission(
"pkg", Manifest.permission.POST_NOTIFICATIONS, 10, "PermissionHelper");
verify(mPermManager).updatePermissionFlags("pkg", Manifest.permission.POST_NOTIFICATIONS,
FLAG_PERMISSION_REVIEW_REQUIRED, FLAG_PERMISSION_REVIEW_REQUIRED, true, 10);
FLAG_PERMISSION_USER_SET, FLAG_PERMISSION_USER_SET, true, 10);
}
@Test
@@ -216,8 +201,7 @@ public class PermissionHelperTest extends UiServiceTestCase {
verify(mPermManager).grantRuntimePermission(
"pkg", Manifest.permission.POST_NOTIFICATIONS, 10);
verify(mPermManager).updatePermissionFlags("pkg", Manifest.permission.POST_NOTIFICATIONS,
FLAG_PERMISSION_USER_SET | FLAG_PERMISSION_REVIEW_REQUIRED,
FLAG_PERMISSION_USER_SET, true, 10);
FLAG_PERMISSION_USER_SET, FLAG_PERMISSION_USER_SET, true, 10);
}
@Test
@@ -230,8 +214,7 @@ public class PermissionHelperTest extends UiServiceTestCase {
verify(mPermManager).revokeRuntimePermission(
eq("pkg"), eq(Manifest.permission.POST_NOTIFICATIONS), eq(10), anyString());
verify(mPermManager).updatePermissionFlags("pkg", Manifest.permission.POST_NOTIFICATIONS,
FLAG_PERMISSION_USER_SET | FLAG_PERMISSION_REVIEW_REQUIRED,
FLAG_PERMISSION_USER_SET, true, 10);
FLAG_PERMISSION_USER_SET, FLAG_PERMISSION_USER_SET, true, 10);
}
@Test
@@ -243,8 +226,8 @@ public class PermissionHelperTest extends UiServiceTestCase {
verify(mPermManager).grantRuntimePermission(
"pkg", Manifest.permission.POST_NOTIFICATIONS, 10);
verify(mPermManager, never()).updatePermissionFlags(
anyString(), anyString(), anyInt(), anyInt(), anyBoolean(), anyInt());
verify(mPermManager).updatePermissionFlags("pkg", Manifest.permission.POST_NOTIFICATIONS,
0, FLAG_PERMISSION_USER_SET, true, 10);
}
@Test
@@ -256,8 +239,8 @@ public class PermissionHelperTest extends UiServiceTestCase {
verify(mPermManager).revokeRuntimePermission(
eq("pkg"), eq(Manifest.permission.POST_NOTIFICATIONS), eq(10), anyString());
verify(mPermManager, never()).updatePermissionFlags(
anyString(), anyString(), anyInt(), anyInt(), anyBoolean(), anyInt());
verify(mPermManager).updatePermissionFlags("pkg", Manifest.permission.POST_NOTIFICATIONS,
0, FLAG_PERMISSION_USER_SET, true, 10);
}
@Test