Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit adc3add3 authored by Nikhil Kumar's avatar Nikhil Kumar
Browse files

Implement DISALLOW_GRANT_ADMIN restriction in UserManager APIs

Implement access restriction based on "DISALLOW_GRANT_ADMIN" restriction for preventing the use of setUserAdmin and revokeUserAdmin functions.
This change is particularly important for child unicorn users who have this restriction in place. It guarantees that they are never elevated to admin privileges or grant admin access to others.

Also implemented the same restriction in the createUser flow to restrict the child users having DISALLOW_GRANT_ADMIN restrictions in place to create new ADMIN users.

Bug: 357636075
Test: UserMangerTest -c
Flag: android.multiuser.unicorn_mode_refactoring_for_hsum_read_only

Change-Id: I69bd4bf34c8c3a21947637d3246c5b9e280d5833
parent cc25ceda
Loading
Loading
Loading
Loading
+2 −0
Original line number Diff line number Diff line
@@ -4824,6 +4824,7 @@ public class UserManager {
     * <p>Note that this does not alter the user's pre-existing user restrictions.
     *
     * @param userId the id of the user to become admin
     * @throws SecurityException if changing ADMIN status of the user is not allowed
     * @hide
     */
    @RequiresPermission(allOf = {
@@ -4844,6 +4845,7 @@ public class UserManager {
     * <p>Note that this does not alter the user's pre-existing user restrictions.
     *
     * @param userId the id of the user to revoke admin rights from
     * @throws SecurityException if changing ADMIN status of the user is not allowed
     * @hide
     */
    @RequiresPermission(allOf = {
+35 −0
Original line number Diff line number Diff line
@@ -2105,6 +2105,10 @@ public class UserManagerService extends IUserManager.Stub {
    @Override
    public void setUserAdmin(@UserIdInt int userId) {
        checkManageUserAndAcrossUsersFullPermission("set user admin");
        if (Flags.unicornModeRefactoringForHsumReadOnly()) {
            checkAdminStatusChangeAllowed(userId);
        }

        mUserJourneyLogger.logUserJourneyBegin(userId, USER_JOURNEY_GRANT_ADMIN);
        UserData user;
        synchronized (mPackagesLock) {
@@ -2133,6 +2137,10 @@ public class UserManagerService extends IUserManager.Stub {
    @Override
    public void revokeUserAdmin(@UserIdInt int userId) {
        checkManageUserAndAcrossUsersFullPermission("revoke admin privileges");
        if (Flags.unicornModeRefactoringForHsumReadOnly()) {
            checkAdminStatusChangeAllowed(userId);
        }

        mUserJourneyLogger.logUserJourneyBegin(userId, USER_JOURNEY_REVOKE_ADMIN);
        UserData user;
        synchronized (mPackagesLock) {
@@ -4065,6 +4073,26 @@ public class UserManagerService extends IUserManager.Stub {
        }
    }

    /**
     * Checks if changing the admin status of a target user is restricted
     * due to the DISALLOW_GRANT_ADMIN restriction. If either the calling
     * user or the target user has this restriction, a SecurityException
     * is thrown.
     *
     * @param targetUser The user ID of the user whose admin status is being
     * considered for change.
     * @throws SecurityException if the admin status change is restricted due
     * to the DISALLOW_GRANT_ADMIN restriction.
     */
    private void checkAdminStatusChangeAllowed(int targetUser) {
        if (hasUserRestriction(UserManager.DISALLOW_GRANT_ADMIN, UserHandle.getCallingUserId())
                || hasUserRestriction(UserManager.DISALLOW_GRANT_ADMIN, targetUser)) {
            throw new SecurityException(
                    "Admin status change is restricted. The DISALLOW_GRANT_ADMIN "
                            + "restriction is applied either on the current or the target user.");
        }
    }

    @GuardedBy({"mPackagesLock"})
    private void writeBitmapLP(UserInfo info, Bitmap bitmap) {
        try {
@@ -5443,6 +5471,13 @@ public class UserManagerService extends IUserManager.Stub {

        enforceUserRestriction(restriction, UserHandle.getCallingUserId(),
                "Cannot add user");
        if (Flags.unicornModeRefactoringForHsumReadOnly()) {
            if ((flags & UserInfo.FLAG_ADMIN) != 0) {
                enforceUserRestriction(UserManager.DISALLOW_GRANT_ADMIN,
                        UserHandle.getCallingUserId(), "Cannot create ADMIN user");
            }
        }

        return createUserInternalUnchecked(name, userType, flags, parentId,
                /* preCreate= */ false, disallowedPackages, /* token= */ null);
    }
+80 −0
Original line number Diff line number Diff line
@@ -121,6 +121,9 @@ public final class UserManagerTest {
        // Making a copy of mUsersToRemove to avoid ConcurrentModificationException
        mUsersToRemove.stream().toList().forEach(this::removeUser);
        mUserRemovalWaiter.close();

        mUserManager.setUserRestriction(UserManager.DISALLOW_GRANT_ADMIN, false,
                mContext.getUser());
    }

    private void removeExistingUsers() {
@@ -933,6 +936,35 @@ public final class UserManagerTest {
        assertThat(userInfo.isAdmin()).isTrue();
    }

    @MediumTest
    @Test
    @RequiresFlagsEnabled(android.multiuser.Flags.FLAG_UNICORN_MODE_REFACTORING_FOR_HSUM_READ_ONLY)
    public void testSetUserAdminThrowsSecurityException() throws Exception {
        UserInfo targetUser = createUser("SecondaryUser", /*flags=*/ 0);
        assertThat(targetUser.isAdmin()).isFalse();

        try {
            // 1. Target User Restriction
            mUserManager.setUserRestriction(UserManager.DISALLOW_GRANT_ADMIN, true,
                    targetUser.getUserHandle());
            assertThrows(SecurityException.class, () -> mUserManager.setUserAdmin(targetUser.id));

            // 2. Current User Restriction
            mUserManager.setUserRestriction(UserManager.DISALLOW_GRANT_ADMIN, false,
                    targetUser.getUserHandle());
            mUserManager.setUserRestriction(UserManager.DISALLOW_GRANT_ADMIN, true,
                    mContext.getUser());
            assertThrows(SecurityException.class, () -> mUserManager.setUserAdmin(targetUser.id));

        } finally {
            // Ensure restriction is removed even if test fails
            mUserManager.setUserRestriction(UserManager.DISALLOW_GRANT_ADMIN, false,
                    targetUser.getUserHandle());
            mUserManager.setUserRestriction(UserManager.DISALLOW_GRANT_ADMIN, false,
                    mContext.getUser());
        }
    }

    @MediumTest
    @Test
    public void testRevokeUserAdmin() throws Exception {
@@ -957,6 +989,37 @@ public final class UserManagerTest {
        assertThat(userInfo.isAdmin()).isFalse();
    }

    @MediumTest
    @Test
    @RequiresFlagsEnabled(android.multiuser.Flags.FLAG_UNICORN_MODE_REFACTORING_FOR_HSUM_READ_ONLY)
    public void testRevokeUserAdminThrowsSecurityException() throws Exception {
        UserInfo targetUser = createUser("SecondaryUser", /*flags=*/ 0);
        assertThat(targetUser.isAdmin()).isFalse();

        try {
            // 1. Target User Restriction
            mUserManager.setUserRestriction(UserManager.DISALLOW_GRANT_ADMIN, true,
                    targetUser.getUserHandle());
            assertThrows(SecurityException.class, () -> mUserManager
                    .revokeUserAdmin(targetUser.id));

            // 2. Current User Restriction
            mUserManager.setUserRestriction(UserManager.DISALLOW_GRANT_ADMIN, false,
                    targetUser.getUserHandle());
            mUserManager.setUserRestriction(UserManager.DISALLOW_GRANT_ADMIN, true,
                    mContext.getUser());
            assertThrows(SecurityException.class, () -> mUserManager
                    .revokeUserAdmin(targetUser.id));

        } finally {
            // Ensure restriction is removed even if test fails
            mUserManager.setUserRestriction(UserManager.DISALLOW_GRANT_ADMIN, false,
                    targetUser.getUserHandle());
            mUserManager.setUserRestriction(UserManager.DISALLOW_GRANT_ADMIN, false,
                    mContext.getUser());
        }
    }

    @MediumTest
    @Test
    public void testGetProfileParent() throws Exception {
@@ -1184,6 +1247,23 @@ public final class UserManagerTest {
        }
    }

    // Make sure createUser for ADMIN would fail if we have DISALLOW_GRANT_ADMIN.
    @MediumTest
    @Test
    @RequiresFlagsEnabled(android.multiuser.Flags.FLAG_UNICORN_MODE_REFACTORING_FOR_HSUM_READ_ONLY)
    public void testCreateAdminUser_disallowGrantAdmin() throws Exception {
        final int creatorId = ActivityManager.getCurrentUser();
        final UserHandle creatorHandle = asHandle(creatorId);
        mUserManager.setUserRestriction(UserManager.DISALLOW_GRANT_ADMIN, true, creatorHandle);
        try {
            UserInfo createdInfo = createUser("SecondaryUser", /*flags=*/ UserInfo.FLAG_ADMIN);
            assertThat(createdInfo).isNull();
        } finally {
            mUserManager.setUserRestriction(UserManager.DISALLOW_ADD_USER, false,
                    creatorHandle);
        }
    }

    // Make sure createProfile would fail if we have DISALLOW_ADD_CLONE_PROFILE.
    @MediumTest
    @Test