From ad9fe6df52baf59cb501abbef0b12bb0679c81cd Mon Sep 17 00:00:00 2001 From: Mayank Garg Date: Sun, 27 Feb 2022 10:48:48 -0800 Subject: [PATCH] Improved error code for Remove Result Bug: 215555912 Test: atest UserManagerTest Change-Id: If92e9a3fcc042236c89ec13e691412dbb6a18a66 --- core/api/system-current.txt | 6 +- core/java/android/os/UserManager.java | 56 +++++++++++++++++-- .../android/server/MasterClearReceiver.java | 2 +- .../android/server/pm/UserManagerService.java | 6 +- .../android/server/pm/UserManagerTest.java | 9 ++- 5 files changed, 66 insertions(+), 13 deletions(-) diff --git a/core/api/system-current.txt b/core/api/system-current.txt index a3e455032b9dc..db9a83207abe1 100644 --- a/core/api/system-current.txt +++ b/core/api/system-current.txt @@ -9821,6 +9821,7 @@ package android.os { method @RequiresPermission(anyOf={android.Manifest.permission.MANAGE_USERS, android.Manifest.permission.QUERY_USERS, android.Manifest.permission.INTERACT_ACROSS_USERS}, conditional=true) public boolean isManagedProfile(int); method @RequiresPermission(anyOf={android.Manifest.permission.MANAGE_USERS, android.Manifest.permission.INTERACT_ACROSS_USERS}, conditional=true) public boolean isMediaSharedWithParent(); method @RequiresPermission(anyOf={android.Manifest.permission.MANAGE_USERS, android.Manifest.permission.CREATE_USERS, android.Manifest.permission.QUERY_USERS}) public boolean isPrimaryUser(); + method public static boolean isRemoveResultSuccessful(int); method public boolean isRestrictedProfile(); method @RequiresPermission(anyOf={android.Manifest.permission.MANAGE_USERS, android.Manifest.permission.CREATE_USERS}, conditional=true) public boolean isRestrictedProfile(@NonNull android.os.UserHandle); method @RequiresPermission(anyOf={android.Manifest.permission.MANAGE_USERS, android.Manifest.permission.QUERY_USERS}) public boolean isSameProfileGroup(@NonNull android.os.UserHandle, @NonNull android.os.UserHandle); @@ -9838,7 +9839,10 @@ package android.os { field public static final String DISALLOW_RUN_IN_BACKGROUND = "no_run_in_background"; field public static final int REMOVE_RESULT_ALREADY_BEING_REMOVED = 2; // 0x2 field public static final int REMOVE_RESULT_DEFERRED = 1; // 0x1 - field public static final int REMOVE_RESULT_ERROR = 3; // 0x3 + field public static final int REMOVE_RESULT_ERROR_SYSTEM_USER = -4; // 0xfffffffc + field public static final int REMOVE_RESULT_ERROR_UNKNOWN = -1; // 0xffffffff + field public static final int REMOVE_RESULT_ERROR_USER_NOT_FOUND = -3; // 0xfffffffd + field public static final int REMOVE_RESULT_ERROR_USER_RESTRICTION = -2; // 0xfffffffe field public static final int REMOVE_RESULT_REMOVED = 0; // 0x0 field public static final int RESTRICTION_NOT_SET = 0; // 0x0 field public static final int RESTRICTION_SOURCE_DEVICE_OWNER = 2; // 0x2 diff --git a/core/java/android/os/UserManager.java b/core/java/android/os/UserManager.java index e56f214e30d9a..1b5f476346d3e 100644 --- a/core/java/android/os/UserManager.java +++ b/core/java/android/os/UserManager.java @@ -1703,12 +1703,40 @@ public class UserManager { /** * A response code from {@link #removeUserWhenPossible(UserHandle, boolean)} indicating that - * an error occurred that prevented the user from being removed or set as ephemeral. + * an unknown error occurred that prevented the user from being removed or set as ephemeral. * * @hide */ @SystemApi - public static final int REMOVE_RESULT_ERROR = 3; + public static final int REMOVE_RESULT_ERROR_UNKNOWN = -1; + + /** + * A response code from {@link #removeUserWhenPossible(UserHandle, boolean)} indicating that + * the user could not be removed due to a {@link #DISALLOW_REMOVE_MANAGED_PROFILE} or + * {@link #DISALLOW_REMOVE_USER} user restriction. + * + * @hide + */ + @SystemApi + public static final int REMOVE_RESULT_ERROR_USER_RESTRICTION = -2; + + /** + * A response code from {@link #removeUserWhenPossible(UserHandle, boolean)} indicating that + * user being removed does not exist. + * + * @hide + */ + @SystemApi + public static final int REMOVE_RESULT_ERROR_USER_NOT_FOUND = -3; + + /** + * A response code from {@link #removeUserWhenPossible(UserHandle, boolean)} indicating that + * user being removed is a {@link UserHandle#SYSTEM} user which can't be removed. + * + * @hide + */ + @SystemApi + public static final int REMOVE_RESULT_ERROR_SYSTEM_USER = -4; /** * Possible response codes from {@link #removeUserWhenPossible(UserHandle, boolean)}. @@ -1719,7 +1747,10 @@ public class UserManager { REMOVE_RESULT_REMOVED, REMOVE_RESULT_DEFERRED, REMOVE_RESULT_ALREADY_BEING_REMOVED, - REMOVE_RESULT_ERROR, + REMOVE_RESULT_ERROR_USER_RESTRICTION, + REMOVE_RESULT_ERROR_USER_NOT_FOUND, + REMOVE_RESULT_ERROR_SYSTEM_USER, + REMOVE_RESULT_ERROR_UNKNOWN, }) @Retention(RetentionPolicy.SOURCE) public @interface RemoveResult {} @@ -4842,8 +4873,10 @@ public class UserManager { * the {@link #DISALLOW_REMOVE_USER} or {@link #DISALLOW_REMOVE_MANAGED_PROFILE} restriction * * @return the {@link RemoveResult} code: {@link #REMOVE_RESULT_REMOVED}, - * {@link #REMOVE_RESULT_DEFERRED}, {@link #REMOVE_RESULT_ALREADY_BEING_REMOVED}, or - * {@link #REMOVE_RESULT_ERROR}. + * {@link #REMOVE_RESULT_DEFERRED}, {@link #REMOVE_RESULT_ALREADY_BEING_REMOVED}, + * {@link #REMOVE_RESULT_ERROR_USER_RESTRICTION}, {@link #REMOVE_RESULT_ERROR_USER_NOT_FOUND}, + * {@link #REMOVE_RESULT_ERROR_SYSTEM_USER}, or {@link #REMOVE_RESULT_ERROR_UNKNOWN}. All error + * codes have negative values. * * @hide */ @@ -4859,6 +4892,19 @@ public class UserManager { } } + /** + * Check if {@link #removeUserWhenPossible} returned a success code which means that the user + * has been removed or is slated for removal. + * + * @param result is {@link #RemoveResult} code return by {@link #removeUserWhenPossible}. + * @return {@code true} if it is a success code. + * @hide + */ + @SystemApi + public static boolean isRemoveResultSuccessful(@RemoveResult int result) { + return result >= 0; + } + /** * Updates the user's name. * diff --git a/services/core/java/com/android/server/MasterClearReceiver.java b/services/core/java/com/android/server/MasterClearReceiver.java index be2b7f72bd4fd..f1fa982d0234b 100644 --- a/services/core/java/com/android/server/MasterClearReceiver.java +++ b/services/core/java/com/android/server/MasterClearReceiver.java @@ -131,7 +131,7 @@ public class MasterClearReceiver extends BroadcastReceiver { final UserManager userManager = context.getSystemService(UserManager.class); final int result = userManager.removeUserWhenPossible( UserHandle.of(userId), /* overrideDevicePolicy= */ false); - if (result == UserManager.REMOVE_RESULT_ERROR) { + if (!UserManager.isRemoveResultSuccessful(result)) { Slogf.e(TAG, "Can't remove user %d", userId); return false; } diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index fed214fd5ab00..f2ec983b0d2e8 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -4585,12 +4585,12 @@ public class UserManagerService extends IUserManager.Stub { final String restriction = getUserRemovalRestriction(userId); if (getUserRestrictions(UserHandle.getCallingUserId()).getBoolean(restriction, false)) { Slog.w(LOG_TAG, "Cannot remove user. " + restriction + " is enabled."); - return UserManager.REMOVE_RESULT_ERROR; + return UserManager.REMOVE_RESULT_ERROR_USER_RESTRICTION; } } if (userId == UserHandle.USER_SYSTEM) { Slog.e(LOG_TAG, "System user cannot be removed."); - return UserManager.REMOVE_RESULT_ERROR; + return UserManager.REMOVE_RESULT_ERROR_SYSTEM_USER; } final long ident = Binder.clearCallingIdentity(); @@ -4602,7 +4602,7 @@ public class UserManagerService extends IUserManager.Stub { if (userData == null) { Slog.e(LOG_TAG, "Cannot remove user " + userId + ", invalid user id provided."); - return UserManager.REMOVE_RESULT_ERROR; + return UserManager.REMOVE_RESULT_ERROR_USER_NOT_FOUND; } if (mRemovingUserIds.get(userId)) { 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 401cd7f0b67e5..7d8c8ee1f2a94 100644 --- a/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java @@ -316,7 +316,8 @@ public final class UserManagerTest { asHandle(currentUser)); try { assertThat(mUserManager.removeUserWhenPossible(user1.getUserHandle(), - /* overrideDevicePolicy= */ false)).isEqualTo(UserManager.REMOVE_RESULT_ERROR); + /* overrideDevicePolicy= */ false)) + .isEqualTo(UserManager.REMOVE_RESULT_ERROR_USER_RESTRICTION); } finally { mUserManager.setUserRestriction(UserManager.DISALLOW_REMOVE_USER, /* value= */ false, asHandle(currentUser)); @@ -353,7 +354,8 @@ public final class UserManagerTest { @Test public void testRemoveUserWhenPossible_systemUserReturnsError() throws Exception { assertThat(mUserManager.removeUserWhenPossible(UserHandle.SYSTEM, - /* overrideDevicePolicy= */ false)).isEqualTo(UserManager.REMOVE_RESULT_ERROR); + /* overrideDevicePolicy= */ false)) + .isEqualTo(UserManager.REMOVE_RESULT_ERROR_SYSTEM_USER); assertThat(hasUser(UserHandle.USER_SYSTEM)).isTrue(); } @@ -363,7 +365,8 @@ public final class UserManagerTest { public void testRemoveUserWhenPossible_invalidUserReturnsError() throws Exception { assertThat(hasUser(Integer.MAX_VALUE)).isFalse(); assertThat(mUserManager.removeUserWhenPossible(UserHandle.of(Integer.MAX_VALUE), - /* overrideDevicePolicy= */ false)).isEqualTo(UserManager.REMOVE_RESULT_ERROR); + /* overrideDevicePolicy= */ false)) + .isEqualTo(UserManager.REMOVE_RESULT_ERROR_USER_NOT_FOUND); } @MediumTest