From 6c9116a6430ca5cd55b1b926213a5e8de77e4fc6 Mon Sep 17 00:00:00 2001 From: Esteban Talavera Date: Thu, 24 Nov 2016 16:12:44 +0000 Subject: [PATCH] Create DISALLOW_{ADD,REMOVE}_MANAGED_PROFILE user restrictions Bug: 31952368 Test: runtest -c com.android.server.devicepolicy.DevicePolicyManagerTest frameworks-services Test: runtest -c com.android.server.pm.UserManagerTest frameworks-services Test: cts-tradefed run cts --module DevicePolicyManager --test com.android.cts.devicepolicy.UserRestrictionsTest Change-Id: I240ab99c2409bbabffbc574bef202f2457026905 --- api/current.txt | 2 + api/system-current.txt | 2 + api/test-current.txt | 2 + .../app/admin/DevicePolicyManager.java | 4 +- core/java/android/os/UserManager.java | 34 +++- core/java/android/os/UserManagerInternal.java | 6 +- .../android/server/pm/UserManagerService.java | 20 ++- .../server/pm/UserRestrictionsUtils.java | 18 +++ .../DevicePolicyManagerService.java | 145 +++++++++++------- .../devicepolicy/DevicePolicyManagerTest.java | 25 ++- .../android/server/pm/UserManagerTest.java | 45 +++++- 11 files changed, 228 insertions(+), 75 deletions(-) diff --git a/api/current.txt b/api/current.txt index c294d8ae4b974..a8bb3f2eb5120 100644 --- a/api/current.txt +++ b/api/current.txt @@ -29719,6 +29719,7 @@ package android.os { method public deprecated void setUserRestrictions(android.os.Bundle, android.os.UserHandle); method public static boolean supportsMultipleUsers(); field public static final java.lang.String ALLOW_PARENT_PROFILE_APP_LINKING = "allow_parent_profile_app_linking"; + field public static final java.lang.String DISALLOW_ADD_MANAGED_PROFILE = "no_add_managed_profile"; field public static final java.lang.String DISALLOW_ADD_USER = "no_add_user"; field public static final java.lang.String DISALLOW_ADJUST_VOLUME = "no_adjust_volume"; field public static final java.lang.String DISALLOW_APPS_CONTROL = "no_control_apps"; @@ -29743,6 +29744,7 @@ package android.os { field public static final java.lang.String DISALLOW_NETWORK_RESET = "no_network_reset"; field public static final java.lang.String DISALLOW_OUTGOING_BEAM = "no_outgoing_beam"; field public static final java.lang.String DISALLOW_OUTGOING_CALLS = "no_outgoing_calls"; + field public static final java.lang.String DISALLOW_REMOVE_MANAGED_PROFILE = "no_remove_managed_profile"; field public static final java.lang.String DISALLOW_REMOVE_USER = "no_remove_user"; field public static final java.lang.String DISALLOW_SAFE_BOOT = "no_safe_boot"; field public static final java.lang.String DISALLOW_SET_USER_ICON = "no_set_user_icon"; diff --git a/api/system-current.txt b/api/system-current.txt index a3672b3145b17..60fdda4979036 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -32367,6 +32367,7 @@ package android.os { method public deprecated void setUserRestrictions(android.os.Bundle, android.os.UserHandle); method public static boolean supportsMultipleUsers(); field public static final java.lang.String ALLOW_PARENT_PROFILE_APP_LINKING = "allow_parent_profile_app_linking"; + field public static final java.lang.String DISALLOW_ADD_MANAGED_PROFILE = "no_add_managed_profile"; field public static final java.lang.String DISALLOW_ADD_USER = "no_add_user"; field public static final java.lang.String DISALLOW_ADJUST_VOLUME = "no_adjust_volume"; field public static final java.lang.String DISALLOW_APPS_CONTROL = "no_control_apps"; @@ -32391,6 +32392,7 @@ package android.os { field public static final java.lang.String DISALLOW_NETWORK_RESET = "no_network_reset"; field public static final java.lang.String DISALLOW_OUTGOING_BEAM = "no_outgoing_beam"; field public static final java.lang.String DISALLOW_OUTGOING_CALLS = "no_outgoing_calls"; + field public static final java.lang.String DISALLOW_REMOVE_MANAGED_PROFILE = "no_remove_managed_profile"; field public static final java.lang.String DISALLOW_REMOVE_USER = "no_remove_user"; field public static final java.lang.String DISALLOW_SAFE_BOOT = "no_safe_boot"; field public static final java.lang.String DISALLOW_SET_USER_ICON = "no_set_user_icon"; diff --git a/api/test-current.txt b/api/test-current.txt index 11f1c5c99c11c..bd2d101eb2975 100644 --- a/api/test-current.txt +++ b/api/test-current.txt @@ -29808,6 +29808,7 @@ package android.os { method public deprecated void setUserRestrictions(android.os.Bundle, android.os.UserHandle); method public static boolean supportsMultipleUsers(); field public static final java.lang.String ALLOW_PARENT_PROFILE_APP_LINKING = "allow_parent_profile_app_linking"; + field public static final java.lang.String DISALLOW_ADD_MANAGED_PROFILE = "no_add_managed_profile"; field public static final java.lang.String DISALLOW_ADD_USER = "no_add_user"; field public static final java.lang.String DISALLOW_ADJUST_VOLUME = "no_adjust_volume"; field public static final java.lang.String DISALLOW_APPS_CONTROL = "no_control_apps"; @@ -29832,6 +29833,7 @@ package android.os { field public static final java.lang.String DISALLOW_NETWORK_RESET = "no_network_reset"; field public static final java.lang.String DISALLOW_OUTGOING_BEAM = "no_outgoing_beam"; field public static final java.lang.String DISALLOW_OUTGOING_CALLS = "no_outgoing_calls"; + field public static final java.lang.String DISALLOW_REMOVE_MANAGED_PROFILE = "no_remove_managed_profile"; field public static final java.lang.String DISALLOW_REMOVE_USER = "no_remove_user"; field public static final java.lang.String DISALLOW_SAFE_BOOT = "no_safe_boot"; field public static final java.lang.String DISALLOW_SET_USER_ICON = "no_set_user_icon"; diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java index 39f415e6ad8f0..d72c96f58f43d 100644 --- a/core/java/android/app/admin/DevicePolicyManager.java +++ b/core/java/android/app/admin/DevicePolicyManager.java @@ -1254,8 +1254,8 @@ public class DevicePolicyManager { /** * Result code for {@link checkProvisioningPreCondition}. * - *

Returned for {@link #ACTION_PROVISION_MANAGED_PROFILE} when the device has a device owner - * and the user is a system user on a split system user device. + *

Returned for {@link #ACTION_PROVISION_MANAGED_PROFILE} when the device the user is a + * system user on a split system user device. * * @hide */ diff --git a/core/java/android/os/UserManager.java b/core/java/android/os/UserManager.java index 0d3b32858f85e..19b49b2e7da5b 100644 --- a/core/java/android/os/UserManager.java +++ b/core/java/android/os/UserManager.java @@ -252,6 +252,20 @@ public class UserManager { */ public static final String DISALLOW_REMOVE_USER = "no_remove_user"; + /** + * Specifies if managed profiles of this user can be removed, other than by its profile owner. + * The default value is false. + *

+ * This restriction can only be set by device owners. + * + *

Key for user restrictions. + *

Type: Boolean + * @see DevicePolicyManager#addUserRestriction(ComponentName, String) + * @see DevicePolicyManager#clearUserRestriction(ComponentName, String) + * @see #getUserRestrictions() + */ + public static final String DISALLOW_REMOVE_MANAGED_PROFILE = "no_remove_managed_profile"; + /** * Specifies if a user is disallowed from enabling or * accessing debugging features. The default value is false. @@ -322,8 +336,8 @@ public class UserManager { public static final String DISALLOW_FACTORY_RESET = "no_factory_reset"; /** - * Specifies if a user is disallowed from adding new users and - * profiles. This can only be set by device owners and profile owners on the primary user. + * Specifies if a user is disallowed from adding new users. This can only be set by device + * owners and 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. @@ -336,6 +350,20 @@ public class UserManager { */ public static final String DISALLOW_ADD_USER = "no_add_user"; + /** + * Specifies if a user is disallowed from adding managed profiles. + *

The default value for an unmanaged user is false. + * For users with a device owner set, the default is true + *

This restriction can only be set by device owners. + * + *

Key for user restrictions. + *

Type: Boolean + * @see DevicePolicyManager#addUserRestriction(ComponentName, String) + * @see DevicePolicyManager#clearUserRestriction(ComponentName, String) + * @see #getUserRestrictions() + */ + public static final String DISALLOW_ADD_MANAGED_PROFILE = "no_add_managed_profile"; + /** * Specifies if a user is disallowed from disabling application * verification. The default value is false. @@ -1403,7 +1431,7 @@ public class UserManager { /** * Similar to {@link #createProfileForUser(String, int, int, String[])} - * except bypassing the checking of {@link UserManager#DISALLOW_ADD_USER}. + * except bypassing the checking of {@link UserManager#DISALLOW_ADD_MANAGED_PROFILE}. * Requires {@link android.Manifest.permission#MANAGE_USERS} permission. * * @see #createProfileForUser(String, int, int, String[]) diff --git a/core/java/android/os/UserManagerInternal.java b/core/java/android/os/UserManagerInternal.java index 1447e7d3fa63f..466a7e35be68c 100644 --- a/core/java/android/os/UserManagerInternal.java +++ b/core/java/android/os/UserManagerInternal.java @@ -120,7 +120,8 @@ public abstract class UserManagerInternal { public abstract void onEphemeralUserStop(int userId); /** - * Same as UserManager.createUser(), but bypasses the check for DISALLOW_ADD_USER. + * Same as UserManager.createUser(), but bypasses the check for + * {@link UserManager#DISALLOW_ADD_USER} and {@link UserManager#DISALLOW_ADD_MANAGED_PROFILE} * *

Called by the {@link com.android.server.devicepolicy.DevicePolicyManagerService} when * createAndManageUser is called by the device owner. @@ -129,7 +130,8 @@ public abstract class UserManagerInternal { /** * Same as {@link UserManager#removeUser(int userHandle)}, but bypasses the check for - * {@link UserManager#DISALLOW_REMOVE_USER} and does not require the + * {@link UserManager#DISALLOW_REMOVE_USER} and + * {@link UserManager#DISALLOW_REMOVE_MANAGED_PROFILE} and does not require the * {@link android.Manifest.permission#MANAGE_USERS} permission. */ public abstract boolean removeUserEvenWhenDisallowed(int userId); diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index bac7a76e01cc5..05228ec50e5d4 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -2269,8 +2269,11 @@ public class UserManagerService extends IUserManager.Stub { private UserInfo createUserInternal(String name, int flags, int parentId, String[] disallowedPackages) { - if (hasUserRestriction(UserManager.DISALLOW_ADD_USER, UserHandle.getCallingUserId())) { - Log.w(LOG_TAG, "Cannot add user. DISALLOW_ADD_USER is enabled."); + String restriction = ((flags & UserInfo.FLAG_MANAGED_PROFILE) != 0) + ? UserManager.DISALLOW_ADD_MANAGED_PROFILE + : UserManager.DISALLOW_ADD_USER; + if (hasUserRestriction(restriction, UserHandle.getCallingUserId())) { + Log.w(LOG_TAG, "Cannot add user. " + restriction + " is enabled."); return null; } return createUserInternalUnchecked(name, flags, parentId, disallowedPackages); @@ -2541,9 +2544,16 @@ public class UserManagerService extends IUserManager.Stub { public boolean removeUser(int userHandle) { Slog.i(LOG_TAG, "removeUser u" + userHandle); checkManageOrCreateUsersPermission("Only the system can remove users"); - if (getUserRestrictions(UserHandle.getCallingUserId()).getBoolean( - UserManager.DISALLOW_REMOVE_USER, false)) { - Log.w(LOG_TAG, "Cannot remove user. DISALLOW_REMOVE_USER is enabled."); + + final boolean isManagedProfile; + synchronized (mUsersLock) { + UserInfo userInfo = getUserInfoLU(userHandle); + isManagedProfile = userInfo != null && userInfo.isManagedProfile(); + } + String restriction = isManagedProfile + ? UserManager.DISALLOW_REMOVE_MANAGED_PROFILE : UserManager.DISALLOW_REMOVE_USER; + if (getUserRestrictions(UserHandle.getCallingUserId()).getBoolean(restriction, false)) { + Log.w(LOG_TAG, "Cannot remove user. " + restriction + " is enabled."); return false; } return removeUserUnchecked(userHandle); diff --git a/services/core/java/com/android/server/pm/UserRestrictionsUtils.java b/services/core/java/com/android/server/pm/UserRestrictionsUtils.java index 7ec3c19bbae32..e91cce11f7df3 100644 --- a/services/core/java/com/android/server/pm/UserRestrictionsUtils.java +++ b/services/core/java/com/android/server/pm/UserRestrictionsUtils.java @@ -75,12 +75,14 @@ public class UserRestrictionsUtils { UserManager.DISALLOW_USB_FILE_TRANSFER, UserManager.DISALLOW_CONFIG_CREDENTIALS, UserManager.DISALLOW_REMOVE_USER, + UserManager.DISALLOW_REMOVE_MANAGED_PROFILE, UserManager.DISALLOW_DEBUGGING_FEATURES, UserManager.DISALLOW_CONFIG_VPN, UserManager.DISALLOW_CONFIG_TETHERING, UserManager.DISALLOW_NETWORK_RESET, UserManager.DISALLOW_FACTORY_RESET, UserManager.DISALLOW_ADD_USER, + UserManager.DISALLOW_ADD_MANAGED_PROFILE, UserManager.ENSURE_VERIFY_APPS, UserManager.DISALLOW_CONFIG_CELL_BROADCASTS, UserManager.DISALLOW_CONFIG_MOBILE_NETWORKS, @@ -124,6 +126,8 @@ public class UserRestrictionsUtils { UserManager.DISALLOW_NETWORK_RESET, UserManager.DISALLOW_FACTORY_RESET, UserManager.DISALLOW_ADD_USER, + UserManager.DISALLOW_ADD_MANAGED_PROFILE, + UserManager.DISALLOW_REMOVE_MANAGED_PROFILE, UserManager.DISALLOW_CONFIG_CELL_BROADCASTS, UserManager.DISALLOW_CONFIG_MOBILE_NETWORKS, UserManager.DISALLOW_MOUNT_PHYSICAL_MEDIA, @@ -154,6 +158,13 @@ public class UserRestrictionsUtils { UserManager.DISALLLOW_UNMUTE_DEVICE ); + /** + * User restrictions that default to {@code true} for device owners. + */ + private static final Set DEFAULT_ENABLED_FOR_DEVICE_OWNERS = Sets.newArraySet( + UserManager.DISALLOW_ADD_MANAGED_PROFILE + ); + /** * Throws {@link IllegalArgumentException} if the given restriction name is invalid. */ @@ -248,6 +259,13 @@ public class UserRestrictionsUtils { && DEVICE_OWNER_ONLY_RESTRICTIONS.contains(restriction)); } + /** + * Returns the user restrictions that default to {@code true} for device owners. + */ + public static @NonNull Set getDefaultEnabledForDeviceOwner() { + return DEFAULT_ENABLED_FOR_DEVICE_OWNERS; + } + /** * Takes restrictions that can be set by device owner, and sort them into what should be applied * globally and what should be applied only on the current user. diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index aafc432f9a554..4061036c6c97b 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -153,7 +153,6 @@ import com.android.internal.R; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.logging.MetricsLogger; import com.android.internal.statusbar.IStatusBarService; -import com.android.internal.util.ArrayUtils; import com.android.internal.util.FastXmlSerializer; import com.android.internal.util.JournaledFile; import com.android.internal.util.ParcelableString; @@ -4806,6 +4805,20 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { long ident = mInjector.binderClearCallingIdentity(); try { + final String restriction; + if (userHandle == UserHandle.USER_SYSTEM) { + restriction = UserManager.DISALLOW_FACTORY_RESET; + } else if (isManagedProfile(userHandle)) { + restriction = UserManager.DISALLOW_REMOVE_MANAGED_PROFILE; + } else { + restriction = UserManager.DISALLOW_REMOVE_USER; + } + if (isAdminAffectedByRestriction( + admin.info.getComponent(), restriction, userHandle)) { + throw new SecurityException("Cannot wipe data. " + restriction + + " restriction is set for user " + userHandle); + } + if ((flags & WIPE_RESET_PROTECTION_DATA) != 0) { if (!isDeviceOwner(admin.info.getComponent(), userHandle)) { throw new SecurityException( @@ -4817,34 +4830,21 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { manager.wipe(); } } + boolean wipeExtRequested = (flags & WIPE_EXTERNAL_STORAGE) != 0; - // If the admin is the only one who has set the restriction: force wipe, even if - // {@link UserManager.DISALLOW_FACTORY_RESET} is set. Reason is that the admin - // could remove this user restriction anyway. - boolean force = (userHandle == UserHandle.USER_SYSTEM) - && isAdminOnlyOneWhoSetRestriction(admin, - UserManager.DISALLOW_FACTORY_RESET, UserHandle.USER_SYSTEM); wipeDeviceOrUserLocked(wipeExtRequested, userHandle, - "DevicePolicyManager.wipeData() from " + source, force); + "DevicePolicyManager.wipeData() from " + source, /*force=*/ true); } finally { mInjector.binderRestoreCallingIdentity(ident); } } } - private boolean isAdminOnlyOneWhoSetRestriction(ActiveAdmin admin, String userRestriction, - int userId) { - int source = mUserManager.getUserRestrictionSource(userRestriction, UserHandle.of(userId)); - if (isDeviceOwner(admin.info.getComponent(), userId)) { - return source == UserManager.RESTRICTION_SOURCE_DEVICE_OWNER; - } else if (isProfileOwner(admin.info.getComponent(), userId)) { - return source == UserManager.RESTRICTION_SOURCE_PROFILE_OWNER; - } - return false; - } - - private void wipeDeviceOrUserLocked(boolean wipeExtRequested, final int userHandle, - String reason, boolean force) { + private void wipeDeviceOrUserLocked( + boolean wipeExtRequested, final int userHandle, String reason, boolean force) { + // TODO If split user is enabled and the device owner is set in the primary user (rather + // than system), we should probably trigger factory reset. Current code just remove + // that user (but still clears FRP...) if (userHandle == UserHandle.USER_SYSTEM) { wipeDataLocked(wipeExtRequested, reason, force); } else { @@ -4857,10 +4857,12 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { am.switchUser(UserHandle.USER_SYSTEM); } - boolean isManagedProfile = isManagedProfile(userHandle); - if (!mUserManager.removeUser(userHandle)) { + boolean userRemoved = force + ? mUserManagerInternal.removeUserEvenWhenDisallowed(userHandle) + : mUserManager.removeUser(userHandle); + if (!userRemoved) { Slog.w(LOG_TAG, "Couldn't remove user " + userHandle); - } else if (isManagedProfile) { + } else if (isManagedProfile(userHandle)) { sendWipeProfileNotification(); } } catch (RemoteException re) { @@ -5948,7 +5950,8 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { } synchronized (this) { enforceCanSetDeviceOwnerLocked(admin, userId); - if (getActiveAdminUncheckedLocked(admin, userId) == null + final ActiveAdmin activeAdmin = getActiveAdminUncheckedLocked(admin, userId); + if (activeAdmin == null || getUserData(userId).mRemovingAdmins.contains(admin)) { throw new IllegalArgumentException("Not active admin: " + admin); } @@ -5975,12 +5978,23 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { mOwners.writeDeviceOwner(); updateDeviceOwnerLocked(); setDeviceOwnerSystemPropertyLocked(); - Intent intent = new Intent(DevicePolicyManager.ACTION_DEVICE_OWNER_CHANGED); + + // STOPSHIP(b/31952368) Also set this restriction for existing DOs on OTA to Android OC. + final Set restrictions = + UserRestrictionsUtils.getDefaultEnabledForDeviceOwner(); + if (!restrictions.isEmpty()) { + for (String restriction : restrictions) { + activeAdmin.ensureUserRestrictions().putBoolean(restriction, true); + } + saveUserRestrictionsLocked(userId); + } ident = mInjector.binderClearCallingIdentity(); try { // TODO Send to system too? - mContext.sendBroadcastAsUser(intent, new UserHandle(userId)); + mContext.sendBroadcastAsUser( + new Intent(DevicePolicyManager.ACTION_DEVICE_OWNER_CHANGED), + UserHandle.of(userId)); } finally { mInjector.binderRestoreCallingIdentity(ident); } @@ -6176,6 +6190,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { mOwners.setProfileOwner(who, ownerName, userHandle); mOwners.writeProfileOwner(userHandle); Slog.i(LOG_TAG, "Profile owner set: " + who + " on user " + userHandle); + return true; } } @@ -7486,28 +7501,41 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { @Override public boolean removeUser(ComponentName who, UserHandle userHandle) { Preconditions.checkNotNull(who, "ComponentName is null"); - UserHandle callingUserHandle = mInjector.binderGetCallingUserHandle(); synchronized (this) { getActiveAdminForCallerLocked(who, DeviceAdminInfo.USES_POLICY_DEVICE_OWNER); } + + final int callingUserId = mInjector.userHandleGetCallingUserId(); final long id = mInjector.binderClearCallingIdentity(); try { - int restrictionSource = mUserManager.getUserRestrictionSource( - UserManager.DISALLOW_REMOVE_USER, callingUserHandle); - if (restrictionSource != UserManager.RESTRICTION_NOT_SET - && restrictionSource != UserManager.RESTRICTION_SOURCE_DEVICE_OWNER) { + String restriction = isManagedProfile(userHandle.getIdentifier()) + ? UserManager.DISALLOW_REMOVE_MANAGED_PROFILE + : UserManager.DISALLOW_REMOVE_USER; + if (isAdminAffectedByRestriction(who, restriction, callingUserId)) { Log.w(LOG_TAG, "The device owner cannot remove a user because " - + "DISALLOW_REMOVE_USER is enabled, and was not set by the device " - + "owner"); + + restriction + " is enabled, and was not set by the device owner"); return false; } - return mUserManagerInternal.removeUserEvenWhenDisallowed( - userHandle.getIdentifier()); + return mUserManagerInternal.removeUserEvenWhenDisallowed(userHandle.getIdentifier()); } finally { mInjector.binderRestoreCallingIdentity(id); } } + private boolean isAdminAffectedByRestriction( + ComponentName admin, String userRestriction, int userId) { + switch(mUserManager.getUserRestrictionSource(userRestriction, UserHandle.of(userId))) { + case UserManager.RESTRICTION_NOT_SET: + return false; + case UserManager.RESTRICTION_SOURCE_DEVICE_OWNER: + return !isDeviceOwner(admin, userId); + case UserManager.RESTRICTION_SOURCE_PROFILE_OWNER: + return !isProfileOwner(admin, userId); + default: + return true; + } + } + @Override public boolean switchUser(ComponentName who, UserHandle userHandle) { Preconditions.checkNotNull(who, "ComponentName is null"); @@ -7613,14 +7641,16 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { // Save the restriction to ActiveAdmin. activeAdmin.ensureUserRestrictions().putBoolean(key, enabledFromThisOwner); - saveSettingsLocked(userHandle); - - pushUserRestrictions(userHandle); - - sendChangedNotification(userHandle); + saveUserRestrictionsLocked(userHandle); } } + private void saveUserRestrictionsLocked(int userId) { + saveSettingsLocked(userId); + pushUserRestrictions(userId); + sendChangedNotification(userId); + } + private void pushUserRestrictions(int userId) { synchronized (this) { final Bundle global; @@ -8862,26 +8892,33 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { if (!hasFeatureManagedUsers()) { return CODE_MANAGED_USERS_NOT_SUPPORTED; } - synchronized (this) { - if (mOwners.hasDeviceOwner()) { - // STOPSHIP Only allow creating a managed profile if allowed by the device - // owner. http://b/31952368 - if (mInjector.userManagerIsSplitSystemUser()) { - if (callingUserId == UserHandle.USER_SYSTEM) { - // Managed-profiles cannot be setup on the system user. - return CODE_SPLIT_SYSTEM_USER_DEVICE_SYSTEM_USER; - } - } - } + if (callingUserId == UserHandle.USER_SYSTEM + && mInjector.userManagerIsSplitSystemUser()) { + // Managed-profiles cannot be setup on the system user. + return CODE_SPLIT_SYSTEM_USER_DEVICE_SYSTEM_USER; } if (getProfileOwner(callingUserId) != null) { // Managed user cannot have a managed profile. return CODE_USER_HAS_PROFILE_OWNER; } - boolean canRemoveProfile = - !mUserManager.hasUserRestriction(UserManager.DISALLOW_REMOVE_USER); final long ident = mInjector.binderClearCallingIdentity(); try { + /* STOPSHIP(b/31952368) Reinstate a check similar to this once ManagedProvisioning + uses checkProvisioningPreCondition (see ag/1607846) and passes the packageName + there. In isProvisioningAllowed we should check isCallerDeviceOwner, but for + managed provisioning we need to check the package that is going to be set as PO + if (mUserManager.hasUserRestriction(UserManager.DISALLOW_ADD_MANAGED_PROFILE)) { + if (!isCallerDeviceOwner(callingUid) + || isAdminAffectedByRestriction(mOwners.getDeviceOwnerComponent(), + UserManager.DISALLOW_ADD_MANAGED_PROFILE, callingUserId)) { + // Caller is not DO or the restriction was set by the system. + return false; + } + } */ + // TODO: Allow it if the caller is the DO? DO could just call removeUser() before + // provisioning, so not strictly required... + boolean canRemoveProfile = !mUserManager.hasUserRestriction( + UserManager.DISALLOW_REMOVE_MANAGED_PROFILE, UserHandle.of(callingUserId)); if (!mUserManager.canAddMoreManagedProfiles(callingUserId, canRemoveProfile)) { return CODE_CANNOT_ADD_MANAGED_PROFILE; } 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 0c008861b346c..5b6d31b464e2d 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -49,6 +49,7 @@ import android.util.Pair; import com.android.internal.R; import com.android.server.LocalServices; import com.android.server.SystemService; +import com.android.server.pm.UserRestrictionsUtils; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; @@ -1190,6 +1191,22 @@ public class DevicePolicyManagerTest extends DpmTestBase { assertTrue(dpm.setDeviceOwner(admin1, "owner-name", UserHandle.USER_SYSTEM)); + // Check that the user restrictions that are enabled by default are set. Then unset them. + String[] defaultRestrictions = UserRestrictionsUtils + .getDefaultEnabledForDeviceOwner().toArray(new String[0]); + DpmTestUtils.assertRestrictions( + DpmTestUtils.newRestrictions(defaultRestrictions), + dpms.getDeviceOwnerAdminLocked().ensureUserRestrictions() + ); + DpmTestUtils.assertRestrictions( + DpmTestUtils.newRestrictions(defaultRestrictions), + dpm.getUserRestrictions(admin1) + ); + + for (String restriction : defaultRestrictions) { + dpm.clearUserRestriction(admin1, restriction); + } + DpmTestUtils.assertRestrictions( DpmTestUtils.newRestrictions(), dpms.getDeviceOwnerAdminLocked().ensureUserRestrictions() @@ -2188,7 +2205,7 @@ public class DevicePolicyManagerTest extends DpmTestBase { assertCheckProvisioningPreCondition(DevicePolicyManager.ACTION_PROVISION_MANAGED_DEVICE, DevicePolicyManager.CODE_OK); assertCheckProvisioningPreCondition(DevicePolicyManager.ACTION_PROVISION_MANAGED_PROFILE, - DevicePolicyManager.CODE_CANNOT_ADD_MANAGED_PROFILE); + DevicePolicyManager.CODE_SPLIT_SYSTEM_USER_DEVICE_SYSTEM_USER); assertCheckProvisioningPreCondition( DevicePolicyManager.ACTION_PROVISION_MANAGED_SHAREABLE_DEVICE, DevicePolicyManager.CODE_OK); @@ -2226,7 +2243,7 @@ public class DevicePolicyManagerTest extends DpmTestBase { assertCheckProvisioningPreCondition(DevicePolicyManager.ACTION_PROVISION_MANAGED_DEVICE, DevicePolicyManager.CODE_OK); assertCheckProvisioningPreCondition(DevicePolicyManager.ACTION_PROVISION_MANAGED_PROFILE, - DevicePolicyManager.CODE_CANNOT_ADD_MANAGED_PROFILE); + DevicePolicyManager.CODE_SPLIT_SYSTEM_USER_DEVICE_SYSTEM_USER); assertCheckProvisioningPreCondition( DevicePolicyManager.ACTION_PROVISION_MANAGED_SHAREABLE_DEVICE, DevicePolicyManager.CODE_OK); @@ -2368,7 +2385,9 @@ public class DevicePolicyManagerTest extends DpmTestBase { when(mContext.ipackageManager.hasSystemFeature(PackageManager.FEATURE_MANAGED_USERS, 0)) .thenReturn(true); when(mContext.userManagerForMock.isSplitSystemUser()).thenReturn(true); - when(mContext.userManager.hasUserRestriction(UserManager.DISALLOW_REMOVE_USER)) + when(mContext.userManager.hasUserRestriction( + UserManager.DISALLOW_REMOVE_MANAGED_PROFILE, + UserHandle.of(DpmMockContext.CALLER_USER_HANDLE))) .thenReturn(true); when(mContext.userManager.canAddMoreManagedProfiles(DpmMockContext.CALLER_USER_HANDLE, false /* we can't remove a managed profile */)).thenReturn(false); diff --git a/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java b/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java index 40d8ac0434c80..9b2c94e0d63b4 100644 --- a/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java @@ -241,15 +241,14 @@ public class UserManagerTest extends AndroidTestCase { } } - // Make sure createProfile would fail if we have DISALLOW_ADD_USER. + // Make sure createUser would fail if we have DISALLOW_ADD_USER. @MediumTest - public void testCreateProfileForUser_disallowAddUser() throws Exception { + public void testCreateUser_disallowAddUser() throws Exception { final int primaryUserId = mUserManager.getPrimaryUser().id; final UserHandle primaryUserHandle = new UserHandle(primaryUserId); mUserManager.setUserRestriction(UserManager.DISALLOW_ADD_USER, true, primaryUserHandle); try { - UserInfo userInfo = createProfileForUser("Managed", - UserInfo.FLAG_MANAGED_PROFILE, primaryUserId); + UserInfo userInfo = createUser("SecondaryUser", /*flags=*/ 0); assertNull(userInfo); } finally { mUserManager.setUserRestriction(UserManager.DISALLOW_ADD_USER, false, @@ -257,16 +256,50 @@ public class UserManagerTest extends AndroidTestCase { } } - // Make sure createProfileEvenWhenDisallowedForUser bypass DISALLOW_ADD_USER. + // Make sure createProfile would fail if we have DISALLOW_ADD_MANAGED_PROFILE. + @MediumTest + public void testCreateProfileForUser_disallowAddManagedProfile() throws Exception { + final int primaryUserId = mUserManager.getPrimaryUser().id; + final UserHandle primaryUserHandle = new UserHandle(primaryUserId); + mUserManager.setUserRestriction(UserManager.DISALLOW_ADD_MANAGED_PROFILE, true, + primaryUserHandle); + try { + UserInfo userInfo = createProfileForUser("Managed", + UserInfo.FLAG_MANAGED_PROFILE, primaryUserId); + assertNull(userInfo); + } finally { + mUserManager.setUserRestriction(UserManager.DISALLOW_ADD_MANAGED_PROFILE, false, + primaryUserHandle); + } + } + + // Make sure createProfileEvenWhenDisallowedForUser bypass DISALLOW_ADD_MANAGED_PROFILE. @MediumTest public void testCreateProfileForUserEvenWhenDisallowed() throws Exception { final int primaryUserId = mUserManager.getPrimaryUser().id; final UserHandle primaryUserHandle = new UserHandle(primaryUserId); - mUserManager.setUserRestriction(UserManager.DISALLOW_ADD_USER, true, primaryUserHandle); + mUserManager.setUserRestriction(UserManager.DISALLOW_ADD_MANAGED_PROFILE, true, + primaryUserHandle); try { UserInfo userInfo = createProfileEvenWhenDisallowedForUser("Managed", UserInfo.FLAG_MANAGED_PROFILE, primaryUserId); assertNotNull(userInfo); + } finally { + mUserManager.setUserRestriction(UserManager.DISALLOW_ADD_MANAGED_PROFILE, false, + primaryUserHandle); + } + } + + // createProfile succeeds even if DISALLOW_ADD_USER is set + @MediumTest + public void testCreateProfileForUser_disallowAddUser() throws Exception { + final int primaryUserId = mUserManager.getPrimaryUser().id; + final UserHandle primaryUserHandle = new UserHandle(primaryUserId); + mUserManager.setUserRestriction(UserManager.DISALLOW_ADD_USER, true, primaryUserHandle); + try { + UserInfo userInfo = createProfileForUser("Managed", + UserInfo.FLAG_MANAGED_PROFILE, primaryUserId); + assertNotNull(userInfo); } finally { mUserManager.setUserRestriction(UserManager.DISALLOW_ADD_USER, false, primaryUserHandle);