diff --git a/core/java/com/android/internal/util/Preconditions.java b/core/java/com/android/internal/util/Preconditions.java index 2f26e921d03b3..53cb56ebecd9d 100644 --- a/core/java/com/android/internal/util/Preconditions.java +++ b/core/java/com/android/internal/util/Preconditions.java @@ -99,6 +99,20 @@ public class Preconditions { return reference; } + /** + * Ensures the truth of an expression involving the state of the calling + * instance, but not involving any parameters to the calling method. + * + * @param expression a boolean expression + * @param message exception message + * @throws IllegalStateException if {@code expression} is false + */ + public static void checkState(final boolean expression, String message) { + if (!expression) { + throw new IllegalStateException(message); + } + } + /** * Ensures the truth of an expression involving the state of the calling * instance, but not involving any parameters to the calling method. @@ -107,9 +121,7 @@ public class Preconditions { * @throws IllegalStateException if {@code expression} is false */ public static void checkState(final boolean expression) { - if (!expression) { - throw new IllegalStateException(); - } + checkState(expression, null); } /** diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 9e8a21cebc696..8a2a578922094 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -2818,6 +2818,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { return; } enforceFullCrossUsersPermission(userHandle); + enforceUserUnlocked(userHandle); synchronized (this) { ActiveAdmin admin = getActiveAdminUncheckedLocked(adminReceiver, userHandle); if (admin == null) { @@ -5531,6 +5532,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { throw new SecurityException( "clearDeviceOwner can only be called by the device owner"); } + enforceUserUnlocked(deviceOwnerUserId); final ActiveAdmin admin = getDeviceOwnerAdminLocked(); if (admin != null) { @@ -5585,6 +5587,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { final UserHandle callingUser = mInjector.binderGetCallingUserHandle(); final int userId = callingUser.getIdentifier(); enforceNotManagedProfile(userId, "clear profile owner"); + enforceUserUnlocked(userId); // Check if this is the profile owner who is calling final ActiveAdmin admin = getActiveAdminForCallerLocked(who, DeviceAdminInfo.USES_POLICY_PROFILE_OWNER); @@ -5960,6 +5963,11 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { } } + private void enforceUserUnlocked(int userId) { + Preconditions.checkState(mUserManager.isUserUnlocked(userId), + "User must be running and unlocked"); + } + private void enforceManageUsers() { final int callingUid = mInjector.binderGetCallingUid(); if (!(isCallerWithSystemUid() || callingUid == Process.ROOT_UID)) { @@ -8540,6 +8548,8 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { final int userId = mInjector.userHandleGetCallingUserId(); + enforceUserUnlocked(userId); + final ComponentName profileOwner = getProfileOwner(userId); if (profileOwner != null && packageName.equals(profileOwner.getPackageName())) { throw new IllegalArgumentException("Cannot uninstall a package with a profile owner"); 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 467ecd77970ee..b23ad50ee2fcf 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -46,7 +46,6 @@ import org.mockito.stubbing.Answer; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -94,6 +93,9 @@ public class DevicePolicyManagerTest extends DpmTestBase { when(mContext.packageManager.hasSystemFeature(eq(PackageManager.FEATURE_DEVICE_ADMIN))) .thenReturn(true); + // By default, pretend all users are running and unlocked. + when(mContext.userManager.isUserUnlocked(anyInt())).thenReturn(true); + initializeDpms(); setUpPackageManagerForAdmin(admin1, DpmMockContext.CALLER_UID); @@ -414,6 +416,45 @@ public class DevicePolicyManagerTest extends DpmTestBase { } } + /** + * {@link DevicePolicyManager#removeActiveAdmin} should fail with the user is not unlocked + * (because we can't send the remove broadcast). + */ + public void testRemoveActiveAdmin_userNotRunningOrLocked() { + mContext.callerPermissions.add(android.Manifest.permission.MANAGE_DEVICE_ADMINS); + + mContext.binder.callingUid = DpmMockContext.CALLER_UID; + + // Add admin. + + dpm.setActiveAdmin(admin1, /* replace =*/ false); + + assertTrue(dpm.isAdminActive(admin1)); + + assertFalse(dpm.isRemovingAdmin(admin1, DpmMockContext.CALLER_USER_HANDLE)); + + // 1. User not unlocked. + when(mContext.userManager.isUserUnlocked(eq(DpmMockContext.CALLER_USER_HANDLE))) + .thenReturn(false); + try { + dpm.removeActiveAdmin(admin1); + fail("Didn't throw IllegalStateException"); + } catch (IllegalStateException expected) { + MoreAsserts.assertContainsRegex( + "User must be running and unlocked", expected.getMessage()); + } + + assertFalse(dpm.isRemovingAdmin(admin1, DpmMockContext.CALLER_USER_HANDLE)); + + // 2. User unlocked. + when(mContext.userManager.isUserUnlocked(eq(DpmMockContext.CALLER_USER_HANDLE))) + .thenReturn(true); + + dpm.removeActiveAdmin(admin1); + + assertTrue(dpm.isRemovingAdmin(admin1, DpmMockContext.CALLER_USER_HANDLE)); + } + /** * Test for: * {@link DevicePolicyManager#removeActiveAdmin} @@ -782,6 +823,18 @@ public class DevicePolicyManagerTest extends DpmTestBase { doReturn(DpmMockContext.CALLER_SYSTEM_USER_UID).when(mContext.packageManager).getPackageUidAsUser( eq(admin1.getPackageName()), anyInt()); + + // But first pretend the user is locked. Then it should fail. + when(mContext.userManager.isUserUnlocked(anyInt())).thenReturn(false); + try { + dpm.clearDeviceOwnerApp(admin1.getPackageName()); + fail("Didn't throw IllegalStateException"); + } catch (IllegalStateException expected) { + MoreAsserts.assertContainsRegex( + "User must be running and unlocked", expected.getMessage()); + } + + when(mContext.userManager.isUserUnlocked(anyInt())).thenReturn(true); reset(mContext.userManagerInternal); dpm.clearDeviceOwnerApp(admin1.getPackageName()); @@ -869,7 +922,19 @@ public class DevicePolicyManagerTest extends DpmTestBase { assertTrue(dpm.isProfileOwnerApp(admin1.getPackageName())); assertFalse(dpm.isRemovingAdmin(admin1, DpmMockContext.CALLER_USER_HANDLE)); - // Clear + // First try when the user is locked, which should fail. + when(mContext.userManager.isUserUnlocked(anyInt())) + .thenReturn(false); + try { + dpm.clearProfileOwner(admin1); + fail("Didn't throw IllegalStateException"); + } catch (IllegalStateException expected) { + MoreAsserts.assertContainsRegex( + "User must be running and unlocked", expected.getMessage()); + } + // Clear, really. + when(mContext.userManager.isUserUnlocked(anyInt())) + .thenReturn(true); dpm.clearProfileOwner(admin1); // Check