Merge "DPMS setDeviceOwner 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 8b3a8b9ad846..ddcb2bf7be22 100644
--- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
+++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
@@ -8119,20 +8119,16 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
                     + " as device owner for user " + userId);
             return false;
         }
-        if (admin == null
-                || !isPackageInstalledForUser(admin.getPackageName(), userId)) {
-            throw new IllegalArgumentException("Invalid component " + admin
-                    + " for device owner");
-        }
+        Preconditions.checkArgument(admin != null);
 
         final CallerIdentity caller = getCallerIdentity();
         synchronized (getLockObject()) {
             enforceCanSetDeviceOwnerLocked(caller, admin, userId);
+            Preconditions.checkArgument(isPackageInstalledForUser(admin.getPackageName(), userId),
+                    "Invalid component " + admin + " for device owner");
             final ActiveAdmin activeAdmin = getActiveAdminUncheckedLocked(admin, userId);
-            if (activeAdmin == null
-                    || getUserData(userId).mRemovingAdmins.contains(admin)) {
-                throw new IllegalArgumentException("Not active admin: " + admin);
-            }
+            Preconditions.checkArgument(activeAdmin != null && !getUserData(
+                    userId).mRemovingAdmins.contains(admin), "Not active admin: " + admin);
 
             // Shutting down backup manager service permanently.
             toggleBackupServiceActive(UserHandle.USER_SYSTEM, /* makeActive= */ false);
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 78e2dee7acf8..73ec5b8d3522 100644
--- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
@@ -1304,6 +1304,16 @@ public class DevicePolicyManagerTest extends DpmTestBase {
     @Test
     public void testSetDeviceOwner_failures() throws Exception {
         // TODO Test more failure cases.  Basically test all chacks in enforceCanSetDeviceOwner().
+        // Package doesn't exist and caller is not system
+        assertExpectException(SecurityException.class,
+                /* messageRegex= */ "Calling identity is not authorized",
+                () -> dpm.setDeviceOwner(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.setDeviceOwner(admin1, "owner-name", UserHandle.USER_SYSTEM));
     }
 
     @Test