Merge "DPMS setProfileOwner access control" into sc-dev

2 files changed, 15 insertions, 9 deletions

diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
index ddcb2bf7be22..cf9b88a33527 100644
--- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
+++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
@@ -8517,20 +8517,16 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
                     + " as profile owner for user " + userHandle);
             return false;
         }
-        if (who == null
-                || !isPackageInstalledForUser(who.getPackageName(), userHandle)) {
-            throw new IllegalArgumentException("Component " + who
-                    + " not installed for userId:" + userHandle);
-        }
+        Preconditions.checkArgument(who != null);
 
         final CallerIdentity caller = getCallerIdentity();
         synchronized (getLockObject()) {
             enforceCanSetProfileOwnerLocked(caller, who, userHandle);
-
+            Preconditions.checkArgument(isPackageInstalledForUser(who.getPackageName(), userHandle),
+                    "Component " + who + " not installed for userId:" + userHandle);
             final ActiveAdmin admin = getActiveAdminUncheckedLocked(who, userHandle);
-            if (admin == null || getUserData(userHandle).mRemovingAdmins.contains(who)) {
-                throw new IllegalArgumentException("Not active admin: " + who);
-            }
+            Preconditions.checkArgument(admin != null && !getUserData(
+                    userHandle).mRemovingAdmins.contains(who), "Not active admin: " + who);
 
             final int parentUserId = getProfileParentId(userHandle);
             // When trying to set a profile owner on a new user, it may be that this user is
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 e03144da4127..89798ce12d4c 100644
--- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
@@ -1551,6 +1551,16 @@ public class DevicePolicyManagerTest extends DpmTestBase {
     @Test
     public void testSetProfileOwner_failures() throws Exception {
         // TODO Test more failure cases.  Basically test all chacks in enforceCanSetProfileOwner().
+        // Package doesn't exist and caller is not system
+        assertExpectException(SecurityException.class,
+                /* messageRegex= */ "Calling identity is not authorized",
+                () -> dpm.setProfileOwner(admin1, "owner-name", UserHandle.USER_SYSTEM));
+
+        // Package exists, but caller is not system
+        setUpPackageManagerForAdmin(admin1, DpmMockContext.CALLER_SYSTEM_USER_UID);
+        assertExpectException(SecurityException.class,
+                /* messageRegex= */ "Calling identity is not authorized",
+                () -> dpm.setProfileOwner(admin1, "owner-name", UserHandle.USER_SYSTEM));
     }
 
     @Test