DO / PO Shouldn't be removed as active admin...
even if they asked.
Also clear(Device|Profile)Owner should remove them as the active admin
too.
Also add some more unit tests.
Bug 26858840
Change-Id: I7b3ed92e1b4cbe803381ed6e3f64d8de17b2ebb0
diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java
index f04e76e..9bad2f9 100644
--- a/core/java/android/app/admin/DevicePolicyManager.java
+++ b/core/java/android/app/admin/DevicePolicyManager.java
@@ -3474,8 +3474,7 @@
public boolean isProfileOwnerApp(String packageName) {
if (mService != null) {
try {
- ComponentName profileOwner = mService.getProfileOwner(
- Process.myUserHandle().getIdentifier());
+ ComponentName profileOwner = mService.getProfileOwner(myUserId());
return profileOwner != null
&& profileOwner.getPackageName().equals(packageName);
} catch (RemoteException re) {
diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
index 572843e..cfe147e 100644
--- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
+++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
@@ -2013,9 +2013,8 @@
void removeActiveAdminLocked(final ComponentName adminReceiver, final int userHandle) {
final ActiveAdmin admin = getActiveAdminUncheckedLocked(adminReceiver, userHandle);
if (admin != null) {
- synchronized (this) {
- getUserData(userHandle).mRemovingAdmins.add(adminReceiver);
- }
+ getUserData(userHandle).mRemovingAdmins.add(adminReceiver);
+
sendAdminCommandLocked(admin,
DeviceAdminReceiver.ACTION_DEVICE_ADMIN_DISABLED,
new BroadcastReceiver() {
@@ -2821,14 +2820,14 @@
if (admin == null) {
return;
}
+ // Active device/profile owners must remain active admins.
+ if (isDeviceOwner(adminReceiver, userHandle)
+ || isProfileOwner(adminReceiver, userHandle)) {
+ Slog.e(LOG_TAG, "Device/profile owner cannot be removed: component=" +
+ adminReceiver);
+ return;
+ }
if (admin.getUid() != mInjector.binderGetCallingUid()) {
- // Active device/profile owners must remain active admins.
- if (isDeviceOwner(adminReceiver, userHandle)
- || isProfileOwner(adminReceiver, userHandle)) {
- Slog.e(LOG_TAG, "Device/profile owner cannot be removed: component=" +
- adminReceiver);
- return;
- }
mContext.enforceCallingOrSelfPermission(
android.Manifest.permission.MANAGE_DEVICE_ADMINS, null);
}
@@ -5481,9 +5480,11 @@
throw new SecurityException(e);
}
synchronized (this) {
+ final ComponentName deviceOwnerComponent = mOwners.getDeviceOwnerComponent();
+ final int deviceOwnerUserId = mOwners.getDeviceOwnerUserId();
if (!mOwners.hasDeviceOwner()
- || !mOwners.getDeviceOwnerComponent().getPackageName().equals(packageName)
- || (mOwners.getDeviceOwnerUserId() != UserHandle.getUserId(callingUid))) {
+ || !deviceOwnerComponent.getPackageName().equals(packageName)
+ || (deviceOwnerUserId != UserHandle.getUserId(callingUid))) {
throw new SecurityException(
"clearDeviceOwner can only be called by the device owner");
}
@@ -5495,8 +5496,7 @@
admin.forceEphemeralUsers = false;
mUserManagerInternal.setForceEphemeralUsers(admin.forceEphemeralUsers);
}
-
- clearUserPoliciesLocked(new UserHandle(UserHandle.USER_SYSTEM));
+ clearUserPoliciesLocked(deviceOwnerUserId);
mOwners.clearDeviceOwner();
mOwners.writeDeviceOwner();
@@ -5506,6 +5506,8 @@
long ident = mInjector.binderClearCallingIdentity();
try {
mInjector.getIBackupManager().setBackupServiceActive(UserHandle.USER_SYSTEM, true);
+
+ removeActiveAdminLocked(deviceOwnerComponent, deviceOwnerUserId);
} catch (RemoteException e) {
throw new IllegalStateException("Failed reactivating backup service.", e);
} finally {
@@ -5546,9 +5548,16 @@
synchronized (this) {
admin.disableCamera = false;
admin.userRestrictions = null;
- clearUserPoliciesLocked(callingUser);
+ clearUserPoliciesLocked(userId);
mOwners.removeProfileOwner(userId);
mOwners.writeProfileOwner(userId);
+
+ final long ident = mInjector.binderClearCallingIdentity();
+ try {
+ removeActiveAdminLocked(who, userId);
+ } finally {
+ mInjector.binderRestoreCallingIdentity(ident);
+ }
}
}
@@ -5576,8 +5585,7 @@
return mLockPatternUtils.getDeviceOwnerInfo();
}
- private void clearUserPoliciesLocked(UserHandle userHandle) {
- int userId = userHandle.getIdentifier();
+ private void clearUserPoliciesLocked(int userId) {
// Reset some of the user-specific policies
DevicePolicyData policy = getUserData(userId);
policy.mPermissionPolicy = DevicePolicyManager.PERMISSION_POLICY_PROMPT;
@@ -5591,8 +5599,8 @@
try {
mIPackageManager.updatePermissionFlagsForAllApps(
PackageManager.FLAG_PERMISSION_POLICY_FIXED,
- 0 /* flagValues */, userHandle.getIdentifier());
- pushUserRestrictions(userHandle.getIdentifier());
+ 0 /* flagValues */, userId);
+ pushUserRestrictions(userId);
} catch (RemoteException re) {
} finally {
mInjector.binderRestoreCallingIdentity(ident);
diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
index 87569b7..64f60d93 100644
--- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
@@ -559,6 +559,10 @@
expected.getMessage().contains("already has a device owner"));
}
+ // DO admin can't be deactivated.
+ dpm.removeActiveAdmin(admin1);
+ assertTrue(dpm.isAdminActive(admin1));
+
// TODO Test getDeviceOwnerName() too. To do so, we need to change
// DPMS.getApplicationLabel() because Context.createPackageContextAsUser() is not mockable.
}
@@ -763,6 +767,11 @@
assertEquals(admin1, dpm.getDeviceOwnerComponentOnAnyUser());
+ dpm.addUserRestriction(admin1, UserManager.DISALLOW_ADD_USER);
+
+ assertTrue(dpm.isAdminActive(admin1));
+ assertFalse(dpm.isRemovingAdmin(admin1, UserHandle.USER_SYSTEM));
+
// Set up other mocks.
when(mContext.userManager.getUserRestrictions()).thenReturn(new Bundle());
@@ -770,11 +779,21 @@
doReturn(DpmMockContext.CALLER_SYSTEM_USER_UID).when(mContext.packageManager).getPackageUidAsUser(
eq(admin1.getPackageName()),
anyInt());
+ reset(mContext.userManagerInternal);
dpm.clearDeviceOwnerApp(admin1.getPackageName());
// Now DO shouldn't be set.
assertNull(dpm.getDeviceOwnerComponentOnAnyUser());
+ verify(mContext.userManagerInternal).setDevicePolicyUserRestrictions(
+ eq(UserHandle.USER_SYSTEM),
+ MockUtils.checkUserRestrictions(),
+ MockUtils.checkUserRestrictions()
+ );
+
+ assertTrue(dpm.isAdminActive(admin1));
+ assertTrue(dpm.isRemovingAdmin(admin1, UserHandle.USER_SYSTEM));
+
// TODO Check other calls.
}
@@ -823,6 +842,10 @@
public void testSetProfileOwner() throws Exception {
setAsProfileOwner(admin1);
+ // PO admin can't be deactivated.
+ dpm.removeActiveAdmin(admin1);
+ assertTrue(dpm.isAdminActive(admin1));
+
// Try setting DO on the same user, which should fail.
setUpPackageManagerForAdmin(admin2, DpmMockContext.CALLER_UID);
dpm.setActiveAdmin(admin2, /* refreshing= */ true, DpmMockContext.CALLER_USER_HANDLE);
@@ -835,6 +858,22 @@
}
}
+ public void testClearProfileOwner() throws Exception {
+ setAsProfileOwner(admin1);
+
+ mContext.binder.callingUid = DpmMockContext.CALLER_UID;
+
+ assertTrue(dpm.isProfileOwnerApp(admin1.getPackageName()));
+ assertFalse(dpm.isRemovingAdmin(admin1, DpmMockContext.CALLER_USER_HANDLE));
+
+ // Clear
+ dpm.clearProfileOwner(admin1);
+
+ // Check
+ assertFalse(dpm.isProfileOwnerApp(admin1.getPackageName()));
+ assertTrue(dpm.isRemovingAdmin(admin1, DpmMockContext.CALLER_USER_HANDLE));
+ }
+
public void testSetProfileOwner_failures() throws Exception {
// TODO Test more failure cases. Basically test all chacks in enforceCanSetProfileOwner().
}