From 57c69d1e30e5eb7f41d1d02ddea35c38f9f73ff1 Mon Sep 17 00:00:00 2001 From: Pavel Grafov Date: Wed, 8 Apr 2020 20:06:01 +0100 Subject: [PATCH] Copy the remaining policies on migration. * accountTypesWithManagementDisabled * disableScreenCapture For security logging nothing has to be done since the state is stored in a system property, just changed it so that the logging will be started after the migration and only events for the right user are logged. Also removed the todo about hardening for power cut case, the risk of additional complexity sees to outweight the benefit. Bug: 149075700 Test: atest DevicePolicyManagerServiceMigrationTest Change-Id: I3a58325f2d6f415e51998c5096c5fc123d26602d --- .../app/admin/DevicePolicyManager.java | 2 +- .../DevicePolicyManagerService.java | 20 ++++++------------- .../res/raw/comp_policies_primary.xml | 4 ++++ .../comp_policies_profile_same_package.xml | 3 +++ ...vicePolicyManagerServiceMigrationTest.java | 11 +++++++++- 5 files changed, 24 insertions(+), 16 deletions(-) diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java index faf9ec61ffde3..fb9adb730314b 100644 --- a/core/java/android/app/admin/DevicePolicyManager.java +++ b/core/java/android/app/admin/DevicePolicyManager.java @@ -8606,7 +8606,7 @@ public class DevicePolicyManager { *

* This method may be called on the {@code DevicePolicyManager} instance returned from * {@link #getParentProfileInstance(ComponentName)}. Note that only a profile owner on - * an organization-deviced can affect account types on the parent profile instance. + * an organization-owned device can affect account types on the parent profile instance. * * @return a list of account types for which account management has been disabled. * diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index d1d353dfbf39b..1da0740024560 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -2702,10 +2702,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { Slog.i(LOG_TAG, "Clearing the DO..."); final ComponentName doAdminReceiver = doAdmin.info.getComponent(); clearDeviceOwnerLocked(doAdmin, doUserId); - // TODO(b/143516163): If we have a power cut here, we might leave active admin. Consider if - // it is worth the complexity to make it more robust. Slog.i(LOG_TAG, "Removing admin artifacts..."); - // TODO(b/143516163): Clean up application restrictions in UserManager. + // TODO(b/149075700): Clean up application restrictions in UserManager. removeAdminArtifacts(doAdminReceiver, doUserId); Slog.i(LOG_TAG, "Migration complete."); @@ -2747,18 +2745,12 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { // The following policies weren't available to PO, but will be available after migration. parentAdmin.disableCamera = doAdmin.disableCamera; - parentAdmin.requireAutoTime = doAdmin.requireAutoTime; - - // TODO(b/143516163): Uncomment once corresponding APIs are available via parent instance. - // parentAdmin.disableScreenCapture = doAdmin.disableScreenCapture; - // parentAdmin.accountTypesWithManagementDisabled.addAll( - // doAdmin.accountTypesWithManagementDisabled); + parentAdmin.disableScreenCapture = doAdmin.disableScreenCapture; + parentAdmin.accountTypesWithManagementDisabled.addAll( + doAdmin.accountTypesWithManagementDisabled); moveDoUserRestrictionsToCopeParent(doAdmin, parentAdmin); - - // TODO(b/143516163): migrate network and security logging state, currently they are - // turned off when DO is removed. } private void moveDoUserRestrictionsToCopeParent(ActiveAdmin doAdmin, ActiveAdmin parentAdmin) { @@ -2778,7 +2770,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { * a managed profile. */ @GuardedBy("getLockObject()") - void applyManagedProfileRestrictionIfDeviceOwnerLocked() { + private void applyManagedProfileRestrictionIfDeviceOwnerLocked() { final int doUserId = mOwners.getDeviceOwnerUserId(); if (doUserId == UserHandle.USER_NULL) { logIfVerbose("No DO found, skipping application of restriction."); @@ -4002,11 +3994,11 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { mOwners.systemReady(); break; case SystemService.PHASE_ACTIVITY_MANAGER_READY: - maybeStartSecurityLogMonitorOnActivityManagerReady(); synchronized (getLockObject()) { migrateToProfileOnOrganizationOwnedDeviceIfCompLocked(); applyManagedProfileRestrictionIfDeviceOwnerLocked(); } + maybeStartSecurityLogMonitorOnActivityManagerReady(); final int userId = getManagedUserId(UserHandle.USER_SYSTEM); if (userId >= 0) { updatePersonalAppSuspension(userId, false /* running */); diff --git a/services/tests/servicestests/res/raw/comp_policies_primary.xml b/services/tests/servicestests/res/raw/comp_policies_primary.xml index 8b7709e0a14f4..395b8ab4ba757 100644 --- a/services/tests/servicestests/res/raw/comp_policies_primary.xml +++ b/services/tests/servicestests/res/raw/comp_policies_primary.xml @@ -5,5 +5,9 @@ + + + + diff --git a/services/tests/servicestests/res/raw/comp_policies_profile_same_package.xml b/services/tests/servicestests/res/raw/comp_policies_profile_same_package.xml index c874dcca2c73b..c65d05693f106 100644 --- a/services/tests/servicestests/res/raw/comp_policies_profile_same_package.xml +++ b/services/tests/servicestests/res/raw/comp_policies_profile_same_package.xml @@ -2,5 +2,8 @@ + + + diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceMigrationTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceMigrationTest.java index c9bd01a31af09..74e7f8c44d1a4 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceMigrationTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceMigrationTest.java @@ -19,6 +19,7 @@ import static android.os.UserHandle.USER_SYSTEM; import static com.android.server.devicepolicy.DpmTestUtils.writeInputStreamToFile; +import static org.junit.Assert.assertArrayEquals; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.eq; @@ -378,6 +379,15 @@ public class DevicePolicyManagerServiceMigrationTest extends DpmTestBase { 33, dpm.getParentProfileInstance(admin1).getPasswordHistoryLength(admin1)); assertEquals("Password history policy was put into non-parent PO instance", 0, dpm.getPasswordHistoryLength(admin1)); + assertTrue("Screen capture restriction wasn't migrated to PO parent instance", + dpm.getParentProfileInstance(admin1).getScreenCaptureDisabled(admin1)); + + assertArrayEquals("Accounts with management disabled weren't migrated to PO parent", + new String[] {"com.google-primary"}, + dpm.getParentProfileInstance(admin1).getAccountTypesWithManagementDisabled()); + assertArrayEquals("Accounts with management disabled for profile were lost", + new String[] {"com.google-profile"}, + dpm.getAccountTypesWithManagementDisabled()); assertTrue("User restriction wasn't migrated to PO parent instance", dpm.getParentProfileInstance(admin1).getUserRestrictions(admin1) @@ -394,7 +404,6 @@ public class DevicePolicyManagerServiceMigrationTest extends DpmTestBase { dpms.getProfileOwnerAdminLocked(COPE_PROFILE_USER_ID) .getEffectiveRestrictions() .containsKey(UserManager.DISALLOW_CONFIG_DATE_TIME)); - // TODO(b/143516163): verify more policies. }); }