Merge "Fix bugs in user restriction migration" into nyc-dev

am: 24f62ea634

* commit '24f62ea63482e6c5d5ba3357951e13e25221eb2e':
  Fix bugs in user restriction migration

services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java

@@ -1689,7 +1689,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
             final ActiveAdmin deviceOwnerAdmin = getDeviceOwnerAdminLocked();
 
             migrateUserRestrictionsForUser(UserHandle.SYSTEM, deviceOwnerAdmin,
-                    /* exceptionList =*/ null);
+                    /* exceptionList =*/ null, /* isDeviceOwner =*/ true);
 
             // Push DO user restrictions to user manager.
             pushUserRestrictions(UserHandle.USER_SYSTEM);
@@ -1697,39 +1697,36 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
             mOwners.setDeviceOwnerUserRestrictionsMigrated();
         }
 
-        // Migrate for POs.  We have a few more exceptions.
+        // Migrate for POs.
-        final Set<String> normalExceptionList = Sets.newArraySet(
+
+        // The following restrictions can be set on secondary users by the device owner, so we
+        // assume they're not from the PO.
+        final Set<String> secondaryUserExceptionList = Sets.newArraySet(
                 UserManager.DISALLOW_OUTGOING_CALLS,
                 UserManager.DISALLOW_SMS);
 
-        final Set<String> managedExceptionList = new ArraySet<>(normalExceptionList.size() + 1);
-        managedExceptionList.addAll(normalExceptionList);
-        managedExceptionList.add(UserManager.DISALLOW_WALLPAPER);
-
         for (UserInfo ui : mUserManager.getUsers()) {
             final int userId = ui.id;
             if (mOwners.getProfileOwnerUserRestrictionsNeedsMigration(userId)) {
-                if (userId != UserHandle.USER_SYSTEM) {
+                if (VERBOSE_LOG) {
-                    if (VERBOSE_LOG) {
+                    Log.v(LOG_TAG, "Migrating PO user restrictions for user " + userId);
-                        Log.v(LOG_TAG, "Migrating PO user restrictions for user " + userId);
-                    }
-                    migrated = true;
-
-                    final ActiveAdmin profileOwnerAdmin = getProfileOwnerAdminLocked(userId);
-
-                    final Set<String> exceptionList =
-                            ui.isManagedProfile() ? managedExceptionList : normalExceptionList;
-
-                    migrateUserRestrictionsForUser(ui.getUserHandle(), profileOwnerAdmin,
-                            exceptionList);
-
-                    // Note if a secondary user has no PO but has a DA that disables camera, we
-                    // don't get here and won't push the camera user restriction to UserManager
-                    // here.  That's okay because we'll push user restrictions anyway when a user
-                    // starts.  But we still do it because we want to let user manager persist
-                    // upon migration.
-                    pushUserRestrictions(userId);
                 }
+                migrated = true;
+
+                final ActiveAdmin profileOwnerAdmin = getProfileOwnerAdminLocked(userId);
+
+                final Set<String> exceptionList =
+                        (userId == UserHandle.USER_SYSTEM) ? null : secondaryUserExceptionList;
+
+                migrateUserRestrictionsForUser(ui.getUserHandle(), profileOwnerAdmin,
+                        exceptionList, /* isDeviceOwner =*/ false);
+
+                // Note if a secondary user has no PO but has a DA that disables camera, we
+                // don't get here and won't push the camera user restriction to UserManager
+                // here.  That's okay because we'll push user restrictions anyway when a user
+                // starts.  But we still do it because we want to let user manager persist
+                // upon migration.
+                pushUserRestrictions(userId);
 
                 mOwners.setProfileOwnerUserRestrictionsMigrated(userId);
             }
@@ -1740,7 +1737,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
     }
 
     private void migrateUserRestrictionsForUser(UserHandle user, ActiveAdmin admin,
-            Set<String> exceptionList) {
+            Set<String> exceptionList, boolean isDeviceOwner) {
         final Bundle origRestrictions = mUserManagerInternal.getBaseUserRestrictions(
                 user.getIdentifier());
 
@@ -1751,7 +1748,11 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
             if (!origRestrictions.getBoolean(key)) {
                 continue;
             }
-            if (exceptionList!= null && exceptionList.contains(key)) {
+            final boolean canOwnerChange = isDeviceOwner
+                    ? UserRestrictionsUtils.canDeviceOwnerChange(key)
+                    : UserRestrictionsUtils.canProfileOwnerChange(key, user.getIdentifier());
+
+            if (!canOwnerChange || (exceptionList!= null && exceptionList.contains(key))) {
                 newBaseRestrictions.putBoolean(key, true);
             } else {
                 newOwnerRestrictions.putBoolean(key, true);

services/tests/servicestests/assets/DevicePolicyManagerServiceMigrationTest2/legacy_device_owner.xml

@@ -0,0 +1,2 @@
+<?xml version='1.0' encoding='utf-8' standalone='yes' ?>
+<profile-owner package="com.android.frameworks.servicestests" name="0" userId="0" component="com.android.frameworks.servicestests/com.android.server.devicepolicy.DummyDeviceAdmins$Admin2" />

services/tests/servicestests/assets/DevicePolicyManagerServiceMigrationTest2/legacy_device_policies.xml

@@ -0,0 +1,5 @@
+<?xml version='1.0' encoding='utf-8' standalone='yes' ?>
+<policies setup-complete="true">
+    <admin name="com.android.frameworks.servicestests/com.android.server.devicepolicy.DummyDeviceAdmins$Admin2">
+    </admin>
+</policies>

services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceMigrationTest.java

@@ -92,6 +92,7 @@ public class DevicePolicyManagerServiceMigrationTest extends DpmTestBase {
         when(mMockContext.userManagerInternal.getBaseUserRestrictions(
                 eq(10))).thenReturn(DpmTestUtils.newRestrictions(
                 UserManager.DISALLOW_REMOVE_USER,
+                UserManager.DISALLOW_ADD_USER,
                 UserManager.DISALLOW_SMS,
                 UserManager.DISALLOW_OUTGOING_CALLS,
                 UserManager.DISALLOW_WALLPAPER,
@@ -100,6 +101,7 @@ public class DevicePolicyManagerServiceMigrationTest extends DpmTestBase {
         when(mMockContext.userManagerInternal.getBaseUserRestrictions(
                 eq(11))).thenReturn(DpmTestUtils.newRestrictions(
                 UserManager.DISALLOW_REMOVE_USER,
+                UserManager.DISALLOW_ADD_USER,
                 UserManager.DISALLOW_SMS,
                 UserManager.DISALLOW_OUTGOING_CALLS,
                 UserManager.DISALLOW_WALLPAPER,
@@ -137,53 +139,142 @@ public class DevicePolicyManagerServiceMigrationTest extends DpmTestBase {
             mContext.binder.restoreCallingIdentity(ident);
         }
 
+        assertTrue(dpms.mOwners.hasDeviceOwner());
+        assertFalse(dpms.mOwners.hasProfileOwner(UserHandle.USER_SYSTEM));
+        assertTrue(dpms.mOwners.hasProfileOwner(10));
+        assertTrue(dpms.mOwners.hasProfileOwner(11));
+        assertFalse(dpms.mOwners.hasProfileOwner(12));
+
         // Now all information should be migrated.
         assertFalse(dpms.mOwners.getDeviceOwnerUserRestrictionsNeedsMigration());
+        assertFalse(dpms.mOwners.getProfileOwnerUserRestrictionsNeedsMigration(
+                UserHandle.USER_SYSTEM));
         assertFalse(dpms.mOwners.getProfileOwnerUserRestrictionsNeedsMigration(10));
         assertFalse(dpms.mOwners.getProfileOwnerUserRestrictionsNeedsMigration(11));
         assertFalse(dpms.mOwners.getProfileOwnerUserRestrictionsNeedsMigration(12));
 
         // Check the new base restrictions.
         DpmTestUtils.assertRestrictions(
-                DpmTestUtils.newRestrictions(),
+                DpmTestUtils.newRestrictions(
+                        UserManager.DISALLOW_RECORD_AUDIO
+                ),
                 newBaseRestrictions.get(UserHandle.USER_SYSTEM));
 
         DpmTestUtils.assertRestrictions(
                 DpmTestUtils.newRestrictions(
+                        UserManager.DISALLOW_ADD_USER,
                         UserManager.DISALLOW_SMS,
-                        UserManager.DISALLOW_OUTGOING_CALLS
+                        UserManager.DISALLOW_OUTGOING_CALLS,
+                        UserManager.DISALLOW_RECORD_AUDIO,
+                        UserManager.DISALLOW_WALLPAPER
                 ),
                 newBaseRestrictions.get(10));
 
         DpmTestUtils.assertRestrictions(
                 DpmTestUtils.newRestrictions(
+                        UserManager.DISALLOW_ADD_USER,
                         UserManager.DISALLOW_SMS,
                         UserManager.DISALLOW_OUTGOING_CALLS,
-                        UserManager.DISALLOW_WALLPAPER
+                        UserManager.DISALLOW_WALLPAPER,
+                        UserManager.DISALLOW_RECORD_AUDIO
                 ),
                 newBaseRestrictions.get(11));
 
         // Check the new owner restrictions.
         DpmTestUtils.assertRestrictions(
                 DpmTestUtils.newRestrictions(
-                        UserManager.DISALLOW_ADD_USER,
+                        UserManager.DISALLOW_ADD_USER
-                        UserManager.DISALLOW_RECORD_AUDIO
                 ),
                 dpms.getDeviceOwnerAdminLocked().ensureUserRestrictions());
 
         DpmTestUtils.assertRestrictions(
                 DpmTestUtils.newRestrictions(
-                        UserManager.DISALLOW_REMOVE_USER,
+                        UserManager.DISALLOW_REMOVE_USER
-                        UserManager.DISALLOW_WALLPAPER,
-                        UserManager.DISALLOW_RECORD_AUDIO
                 ),
                 dpms.getProfileOwnerAdminLocked(10).ensureUserRestrictions());
 
         DpmTestUtils.assertRestrictions(
                 DpmTestUtils.newRestrictions(
-                        UserManager.DISALLOW_REMOVE_USER,
+                        UserManager.DISALLOW_REMOVE_USER
-                        UserManager.DISALLOW_RECORD_AUDIO
                 ),
                 dpms.getProfileOwnerAdminLocked(11).ensureUserRestrictions());
     }
+
+    public void testMigration2_profileOwnerOnUser0() throws Exception {
+        setUpPackageManagerForAdmin(admin2, DpmMockContext.CALLER_SYSTEM_USER_UID);
+
+        // Create the legacy owners & policies file.
+        DpmTestUtils.writeToFile(
+                (new File(mContext.dataDir, OwnersTestable.LEGACY_FILE)).getAbsoluteFile(),
+                DpmTestUtils.readAsset(mRealTestContext,
+                        "DevicePolicyManagerServiceMigrationTest2/legacy_device_owner.xml"));
+
+        DpmTestUtils.writeToFile(
+                (new File(mContext.systemUserDataDir, "device_policies.xml")).getAbsoluteFile(),
+                DpmTestUtils.readAsset(mRealTestContext,
+                        "DevicePolicyManagerServiceMigrationTest2/legacy_device_policies.xml"));
+
+        // Set up UserManager
+        when(mMockContext.userManagerInternal.getBaseUserRestrictions(
+                eq(UserHandle.USER_SYSTEM))).thenReturn(DpmTestUtils.newRestrictions(
+                UserManager.DISALLOW_ADD_USER,
+                UserManager.DISALLOW_RECORD_AUDIO,
+                UserManager.DISALLOW_SMS,
+                UserManager.DISALLOW_OUTGOING_CALLS));
+
+        final Map<Integer, Bundle> newBaseRestrictions = new HashMap<>();
+
+        doAnswer(new Answer<Void>() {
+            @Override
+            public Void answer(InvocationOnMock invocation) throws Throwable {
+                Integer userId = (Integer) invocation.getArguments()[0];
+                Bundle bundle = (Bundle) invocation.getArguments()[1];
+
+                newBaseRestrictions.put(userId, bundle);
+
+                return null;
+            }
+        }).when(mContext.userManagerInternal).setBaseUserRestrictionsByDpmsForMigration(
+                anyInt(), any(Bundle.class));
+
+        // Initialize DPM/DPMS and let it migrate the persisted information.
+        // (Need clearCallingIdentity() to pass permission checks.)
+
+        final DevicePolicyManagerServiceTestable dpms;
+
+        final long ident = mContext.binder.clearCallingIdentity();
+        try {
+            LocalServices.removeServiceForTest(DevicePolicyManagerInternal.class);
+
+            dpms = new DevicePolicyManagerServiceTestable(mContext, dataDir);
+
+            dpms.systemReady(SystemService.PHASE_LOCK_SETTINGS_READY);
+            dpms.systemReady(SystemService.PHASE_BOOT_COMPLETED);
+        } finally {
+            mContext.binder.restoreCallingIdentity(ident);
+        }
+        assertFalse(dpms.mOwners.hasDeviceOwner());
+        assertTrue(dpms.mOwners.hasProfileOwner(UserHandle.USER_SYSTEM));
+
+        // Now all information should be migrated.
+        assertFalse(dpms.mOwners.getDeviceOwnerUserRestrictionsNeedsMigration());
+        assertFalse(dpms.mOwners.getProfileOwnerUserRestrictionsNeedsMigration(
+                UserHandle.USER_SYSTEM));
+
+        // Check the new base restrictions.
+        DpmTestUtils.assertRestrictions(
+                DpmTestUtils.newRestrictions(
+                        UserManager.DISALLOW_RECORD_AUDIO
+                ),
+                newBaseRestrictions.get(UserHandle.USER_SYSTEM));
+
+        // Check the new owner restrictions.
+        DpmTestUtils.assertRestrictions(
+                DpmTestUtils.newRestrictions(
+                        UserManager.DISALLOW_ADD_USER,
+                        UserManager.DISALLOW_SMS,
+                        UserManager.DISALLOW_OUTGOING_CALLS
+                ),
+                dpms.getProfileOwnerAdminLocked(UserHandle.USER_SYSTEM).ensureUserRestrictions());
+    }
 }