From 9ace11127c334296ddf502ee0f6b7f908b2e7006 Mon Sep 17 00:00:00 2001 From: Alex Johnston Date: Thu, 30 Apr 2020 15:06:19 +0100 Subject: [PATCH] Restrict creation of secondary users Background * Secondary users should be disabled when the device is an organization-owned managed profile device. * This is because supporting secondary users would complicate the semantics of user restrictions. Changes * Add DISALLOW_ADD_USER as a base restriction when the device is an organization-owned managed profile device. * Handle removal case when the device is no longer in this mode. * Remove the ability of other admins to apply DISALLOW_ADD_USER. Manual Testing Steps * Provision an organization-owned managed profile device. * Check Settings > System > Multiple users and verify that a user cannot be added. * Check WP TestDPC 'Set user restrictions on parent' and verify 'Disallow add user' is not present. Bug: 155281701 Test: Manual testing atest com.android.server.devicepolicy.DevicePolicyManagerTest Change-Id: I83348fc8b854cef20383803124000540b5b130cb --- core/java/android/os/UserManager.java | 5 +-- .../server/pm/UserRestrictionsUtils.java | 1 - .../DevicePolicyManagerService.java | 10 ++++-- .../devicepolicy/DevicePolicyManagerTest.java | 36 ++++++++++++++++++- 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/core/java/android/os/UserManager.java b/core/java/android/os/UserManager.java index 187274a837a0a..7912dacac3770 100644 --- a/core/java/android/os/UserManager.java +++ b/core/java/android/os/UserManager.java @@ -635,10 +635,11 @@ public class UserManager { /** * Specifies if a user is disallowed from adding new users. This can only be set by device - * owners, profile owners on the primary user or profile owners of organization-owned managed - * profiles on the parent profile. The default value is false. + * owners or profile owners on the primary user. The default value is false. *

This restriction has no effect on secondary users and managed profiles since only the * primary user can add other users. + *

When the device is an organization-owned device provisioned with a managed profile, + * this restriction will be set as a base restriction which cannot be removed by any admin. * *

Key for user restrictions. *

Type: Boolean diff --git a/services/core/java/com/android/server/pm/UserRestrictionsUtils.java b/services/core/java/com/android/server/pm/UserRestrictionsUtils.java index 0b6024a84f784..1fec8aa0a3ff2 100644 --- a/services/core/java/com/android/server/pm/UserRestrictionsUtils.java +++ b/services/core/java/com/android/server/pm/UserRestrictionsUtils.java @@ -208,7 +208,6 @@ public class UserRestrictionsUtils { Sets.newArraySet( UserManager.DISALLOW_CONFIG_DATE_TIME, UserManager.DISALLOW_CAMERA, - UserManager.DISALLOW_ADD_USER, UserManager.DISALLOW_BLUETOOTH, UserManager.DISALLOW_BLUETOOTH_SHARING, UserManager.DISALLOW_CONFIG_CELL_BROADCASTS, diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 3323fa4b53e3e..966694ad346c0 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -4567,9 +4567,11 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } if (isProfileOwner(adminReceiver, userHandle)) { if (isProfileOwnerOfOrganizationOwnedDevice(userHandle)) { + UserHandle parentUserHandle = UserHandle.of(getProfileParentId(userHandle)); mUserManager.setUserRestriction(UserManager.DISALLOW_REMOVE_MANAGED_PROFILE, - false, - UserHandle.of(getProfileParentId(userHandle))); + false, parentUserHandle); + mUserManager.setUserRestriction(UserManager.DISALLOW_ADD_USER, + false, parentUserHandle); } final ActiveAdmin admin = getActiveAdminUncheckedLocked(adminReceiver, userHandle, /* parent */ false); @@ -7213,6 +7215,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { mUserManager.setUserRestriction( UserManager.DISALLOW_REMOVE_MANAGED_PROFILE, false, UserHandle.SYSTEM); + mUserManager.setUserRestriction( + UserManager.DISALLOW_ADD_USER, false, UserHandle.SYSTEM); // Device-wide policies set by the profile owner need to be cleaned up here. mLockPatternUtils.setDeviceOwnerInfo(null); @@ -13825,6 +13829,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { mUserManager.setUserRestriction(UserManager.DISALLOW_REMOVE_MANAGED_PROFILE, true, parentUser); + mUserManager.setUserRestriction(UserManager.DISALLOW_ADD_USER, true, + parentUser); }); // markProfileOwnerOfOrganizationOwnedDevice will trigger writing of the profile owner 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 6b36bc591b780..ed40fe756ea16 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -1998,7 +1998,6 @@ public class DevicePolicyManagerTest extends DpmTestBase { private static final Set PROFILE_OWNER_ORGANIZATION_OWNED_GLOBAL_RESTRICTIONS = Sets.newSet( UserManager.DISALLOW_CONFIG_DATE_TIME, - UserManager.DISALLOW_ADD_USER, UserManager.DISALLOW_BLUETOOTH_SHARING, UserManager.DISALLOW_CONFIG_CELL_BROADCASTS, UserManager.DISALLOW_CONFIG_MOBILE_NETWORKS, @@ -4005,6 +4004,12 @@ public class DevicePolicyManagerTest extends DpmTestBase { // Any caller should be able to call this method. assertFalse(dpm.isOrganizationOwnedDeviceWithManagedProfile()); configureProfileOwnerOfOrgOwnedDevice(admin1, CALLER_USER_HANDLE); + + verify(getServices().userManager).setUserRestriction( + eq(UserManager.DISALLOW_ADD_USER), + eq(true), + eq(UserHandle.of(UserHandle.USER_SYSTEM))); + assertTrue(dpm.isOrganizationOwnedDeviceWithManagedProfile()); // A random caller from another user should also be able to get the right result. @@ -4012,6 +4017,35 @@ public class DevicePolicyManagerTest extends DpmTestBase { assertTrue(dpm.isOrganizationOwnedDeviceWithManagedProfile()); } + public void testMarkOrganizationOwnedDevice_baseRestrictionsAdded() throws Exception { + addManagedProfile(admin1, DpmMockContext.CALLER_UID, admin1); + + configureProfileOwnerOfOrgOwnedDevice(admin1, CALLER_USER_HANDLE); + + // Base restriction DISALLOW_REMOVE_MANAGED_PROFILE added + verify(getServices().userManager).setUserRestriction( + eq(UserManager.DISALLOW_REMOVE_MANAGED_PROFILE), + eq(true), + eq(UserHandle.of(UserHandle.USER_SYSTEM))); + + // Base restriction DISALLOW_ADD_USER added + verify(getServices().userManager).setUserRestriction( + eq(UserManager.DISALLOW_ADD_USER), + eq(true), + eq(UserHandle.of(UserHandle.USER_SYSTEM))); + + // Assert base restrictions cannot be added or removed by admin + assertExpectException(SecurityException.class, null, () -> + parentDpm.addUserRestriction(admin1, UserManager.DISALLOW_REMOVE_MANAGED_PROFILE)); + assertExpectException(SecurityException.class, null, () -> + parentDpm.clearUserRestriction(admin1, + UserManager.DISALLOW_REMOVE_MANAGED_PROFILE)); + assertExpectException(SecurityException.class, null, () -> + parentDpm.addUserRestriction(admin1, UserManager.DISALLOW_ADD_USER)); + assertExpectException(SecurityException.class, null, () -> + parentDpm.clearUserRestriction(admin1, UserManager.DISALLOW_ADD_USER)); + } + public void testSetTime() throws Exception { mContext.binder.callingUid = DpmMockContext.CALLER_SYSTEM_USER_UID; setupDeviceOwner();