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

am: 24f62ea634 * commit '24f62ea63482e6c5d5ba3357951e13e25221eb2e': Fix bugs in user restriction migration
2016-02-18 20:57:10 +00:00
parent 689fe4444f 24f62ea634
commit 5cee7cafde
4 changed files with 138 additions and 39 deletions
--- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
+++ b/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.
-        final Set<String> normalExceptionList = Sets.newArraySet(
+        // Migrate for POs.
+
+        // 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) {
-                        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);
+                if (VERBOSE_LOG) {
+                    Log.v(LOG_TAG, "Migrating PO user restrictions for user " + 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);
--- a/services/tests/servicestests/assets/DevicePolicyManagerServiceMigrationTest2/legacy_device_owner.xml
+++ b/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" />
--- a/services/tests/servicestests/assets/DevicePolicyManagerServiceMigrationTest2/legacy_device_policies.xml
+++ b/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>
--- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceMigrationTest.java
+++ b/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_RECORD_AUDIO
+                        UserManager.DISALLOW_ADD_USER
                ),
                dpms.getDeviceOwnerAdminLocked().ensureUserRestrictions());

        DpmTestUtils.assertRestrictions(
                DpmTestUtils.newRestrictions(
-                        UserManager.DISALLOW_REMOVE_USER,
-                        UserManager.DISALLOW_WALLPAPER,
-                        UserManager.DISALLOW_RECORD_AUDIO
+                        UserManager.DISALLOW_REMOVE_USER
                ),
                dpms.getProfileOwnerAdminLocked(10).ensureUserRestrictions());

        DpmTestUtils.assertRestrictions(
                DpmTestUtils.newRestrictions(
-                        UserManager.DISALLOW_REMOVE_USER,
-                        UserManager.DISALLOW_RECORD_AUDIO
+                        UserManager.DISALLOW_REMOVE_USER
                ),
                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());
+    }
 }