From 957d31d1b5fab827fa3af6c990ec33355db1219a Mon Sep 17 00:00:00 2001 From: Eran Messeri Date: Thu, 16 Jan 2020 10:21:26 +0000 Subject: [PATCH] Remove unnecessary parent restriction check When checking whether provisioning of a managed profile is allowed, it is unnecessary to check whether there's a restriction on the parent user because the check is done from the primary user. If the check is done from inside a managed profile, then the check should return false because a managed profile cannot be provisioned from within another managed profile. The DevicePolicyManagerTest was incorrectly returning user 0 as the "parent user" for user 0, so changed the test to return null as the profile parent for user 0. Bug: 147631026 Test: atest FrameworksServicesTests:DevicePolicyManagerTest Test: atest CtsDevicePolicyManagerTestCases:com.android.cts.devicepolicy.ManagedProfileTest#testIsProvisioningAllowed Test: atest CtsDevicePolicyManagerTestCases:com.android.cts.devicepolicy.DeviceOwnerTest#testIsManagedDeviceProvisioningAllowed Test: atest CtsDevicePolicyManagerTestCases:com.android.cts.devicepolicy.DeviceOwnerPlusProfileOwnerTest#testProvisioningNotAllowedWithDeviceOwner Test: atest CtsDevicePolicyManagerTestCases:com.android.cts.devicepolicy.CustomDeviceOwnerTest#testIsProvisioningAllowed Test: atest CtsDevicePolicyManagerTestCases:com.android.cts.devicepolicy.CustomManagedProfileTest#testIsProvisioningAllowed Change-Id: Ia62dce93265ec65b61a048c4d96f96baa4598a57 --- .../DevicePolicyManagerService.java | 33 ++++++++++--------- .../devicepolicy/DevicePolicyManagerTest.java | 6 ++-- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 6ab5303b41e4f..7012d24f067af 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -13041,26 +13041,25 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { final boolean addingProfileRestricted = mUserManager.hasUserRestriction( UserManager.DISALLOW_ADD_MANAGED_PROFILE, callingUserHandle); - UserInfo parentUser = mUserManager.getProfileParent(callingUserId); - final boolean addingProfileRestrictedOnParent = (parentUser != null) - && mUserManager.hasUserRestriction( - UserManager.DISALLOW_ADD_MANAGED_PROFILE, - UserHandle.of(parentUser.id)); + if (mUserManager.getUserInfo(callingUserId).isProfile()) { + Slog.i(LOG_TAG, + String.format("Calling user %d is a profile, cannot add another.", + callingUserId)); + // The check is called from inside a managed profile. A managed profile cannot + // be provisioned from within another managed profile. + return CODE_CANNOT_ADD_MANAGED_PROFILE; + } - Slog.i(LOG_TAG, String.format( - "When checking for managed profile provisioning: Has device owner? %b, adding" - + " profile restricted? %b, adding profile restricted on parent? %b", - hasDeviceOwner, addingProfileRestricted, addingProfileRestrictedOnParent)); - - // If there's a device owner, the restriction on adding a managed profile must be set - // somewhere. - if (hasDeviceOwner && !addingProfileRestricted && !addingProfileRestrictedOnParent) { + // If there's a device owner, the restriction on adding a managed profile must be set. + if (hasDeviceOwner && !addingProfileRestricted) { Slog.wtf(LOG_TAG, "Has a device owner but no restriction on adding a profile."); } - // Do not allow adding a managed profile if there's a restriction, either on the current - // user or its parent user. - if (addingProfileRestricted || addingProfileRestrictedOnParent) { + // Do not allow adding a managed profile if there's a restriction. + if (addingProfileRestricted) { + Slog.i(LOG_TAG, String.format( + "Adding a profile is restricted: User %s Has device owner? %b", + callingUserHandle, hasDeviceOwner)); return CODE_CANNOT_ADD_MANAGED_PROFILE; } // If there's a restriction on removing the managed profile then we have to take it @@ -13069,6 +13068,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { !mUserManager.hasUserRestriction(UserManager.DISALLOW_REMOVE_MANAGED_PROFILE, callingUserHandle); if (!mUserManager.canAddMoreManagedProfiles(callingUserId, canRemoveProfile)) { + Slog.i(LOG_TAG, String.format( + "Cannot add more profiles: Can remove current? %b", canRemoveProfile)); return CODE_CANNOT_ADD_MANAGED_PROFILE; } } finally { 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 f57b5f246978e..c990e98a7af8d 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -3193,6 +3193,7 @@ public class DevicePolicyManagerTest extends DpmTestBase { when(getServices().userManager.canAddMoreManagedProfiles(UserHandle.USER_SYSTEM, true)) .thenReturn(true); setUserSetupCompleteForUser(false, UserHandle.USER_SYSTEM); + when(getServices().userManager.getProfileParent(UserHandle.USER_SYSTEM)).thenReturn(null); mContext.binder.callingUid = DpmMockContext.CALLER_SYSTEM_USER_UID; } @@ -3234,6 +3235,7 @@ public class DevicePolicyManagerTest extends DpmTestBase { when(getServices().userManager.canAddMoreManagedProfiles(UserHandle.USER_SYSTEM, true)) .thenReturn(true); setUserSetupCompleteForUser(true, UserHandle.USER_SYSTEM); + when(getServices().userManager.getProfileParent(UserHandle.USER_SYSTEM)).thenReturn(null); mContext.binder.callingUid = DpmMockContext.CALLER_SYSTEM_USER_UID; } @@ -3605,14 +3607,14 @@ public class DevicePolicyManagerTest extends DpmTestBase { when(getServices().ipackageManager.hasSystemFeature(PackageManager.FEATURE_MANAGED_USERS, 0)) .thenReturn(true); - when(getServices().userManagerForMock.isSplitSystemUser()).thenReturn(true); + when(getServices().userManagerForMock.isSplitSystemUser()).thenReturn(false); when(getServices().userManager.getProfileParent(DpmMockContext.CALLER_USER_HANDLE)) .thenReturn(new UserInfo(UserHandle.USER_SYSTEM, "user system", 0)); when(getServices().userManager.canAddMoreManagedProfiles(DpmMockContext.CALLER_USER_HANDLE, true)).thenReturn(true); setUserSetupCompleteForUser(false, DpmMockContext.CALLER_USER_HANDLE); - mContext.binder.callingUid = DpmMockContext.CALLER_UID; + mContext.binder.callingUid = DpmMockContext.ANOTHER_UID; } public void testIsProvisioningAllowed_provisionManagedProfileWithDeviceOwner_primaryUser()