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

Commit 01b7062b authored by Nikhil Kumar's avatar Nikhil Kumar
Browse files

Restrict admin status change when DISALLOW_GRANT_ADMIN is present 

This change addresses a security gap where users with the "DISALLOW_GRANT_ADMIN" restriction could still grant admin privileges to others, potentially undermining supervised user models.  We've extended the existing logic to ensure that restricted users cannot grant or receive admin privileges when the DISALLOW_GRANT_ADMIN restriction is in place. This prevents scenarios where supervised users could create new admins to bypass restrictions and factory reset the device.

In addition to the core functionality improvement, significant code refactoring has been done to untangle complex multiple if conditions. The logic for determining when admin status changes are allowed is now clearer and more readable, taking into account both the "current user" (initiating the change) and the "target user" (whose privileges are being modified).

Bug: 357056776
Test: atest UserDetailsSettingsTest and manually verified the user having DISALLOW_GRANT_ADMIN restriction is not allowed to change admin status of any other user.
Flag: android.multiuser.unicorn_mode_refactoring_for_hsum_read_only
Change-Id: If02c9355ee7ce285b5b7bcddae630d716d5bf333
parent b377b1c0

src/com/android/settings/users/UserDetailsSettings.java

+40 −4
Original line number Diff line number Diff line
@@ -370,11 +370,18 @@ public class UserDetailsSettings extends SettingsPreferenceFragment 
            }

            mSwitchUserPref.setOnPreferenceClickListener(this);

        }

        if (android.multiuser.Flags.unicornModeRefactoringForHsumReadOnly()) {

            if (isChangingAdminStatusRestricted()) {

                removePreference(KEY_GRANT_ADMIN);

            }

        } else {

            if (mUserInfo.isMain() || mUserInfo.isGuest() || !UserManager.isMultipleAdminEnabled()

                    || mUserManager.hasUserRestrictionForUser(UserManager.DISALLOW_GRANT_ADMIN,

                    mUserInfo.getUserHandle()) || !mUserManager.isAdminUser()) {

                removePreference(KEY_GRANT_ADMIN);

            }

        }



        if (!mUserManager.isAdminUser()) { // non admin users can't remove users and allow calls

            removePreference(KEY_ENABLE_TELEPHONY);

            removePreference(KEY_REMOVE_USER);
@@ -552,4 +559,33 @@ public class UserDetailsSettings extends SettingsPreferenceFragment 
        //  return true so there will be no setup prompt dialog shown to the user anymore.

        return isSecondaryUser(mUserInfo) && !mUserInfo.isInitialized();

    }



    /**

     * Determines if changing admin status is restricted.

     *

     * <p>Admin status change is restricted under the following conditions of current & target user.

     *

     * <ul>

     *   <li>The <b>current</b> user is NOT an admin user.</li>

     *   <li>OR multiple admin support is NOT enabled.</li>

     *   <li>OR the <b>current</b> user has DISALLOW_GRANT_ADMIN restriction applied</li>

     *

     *   <li>OR the <b>target</b> user ('mUserInfo') is a main user OR a guest user.</li>

     *   <li>OR the <b>target</b> user ('mUserInfo') has DISALLOW_GRANT_ADMIN restriction.</li>

     * </ul>

     *

     * @return true if changing admin status is restricted, false otherwise

     */

    private boolean isChangingAdminStatusRestricted() {

        boolean currentUserRestricted = !mUserManager.isAdminUser()

                || !UserManager.isMultipleAdminEnabled()

                || mUserManager.hasUserRestriction(UserManager.DISALLOW_GRANT_ADMIN);



        boolean targetUserRestricted = mUserInfo.isMain()

                || mUserInfo.isGuest()

                || mUserManager.hasUserRestrictionForUser(UserManager.DISALLOW_GRANT_ADMIN,

                mUserInfo.getUserHandle());



        return currentUserRestricted || targetUserRestricted;

    }

}

tests/robotests/src/com/android/settings/users/UserDetailsSettingsTest.java

+13 −0
Original line number Diff line number Diff line
@@ -729,12 +729,25 @@ public class UserDetailsSettingsTest { 
    public void initialize_restrictUserSelected_shouldNotShowGrantAdminPref_MultipleAdminEnabled() {

        setupSelectedUser();

        ShadowUserManager.setIsMultipleAdminEnabled(true);

        // target user has DISALLOW_GRANT_ADMIN restriction

        mUserManager.setUserRestriction(mUserInfo.getUserHandle(),

                UserManager.DISALLOW_GRANT_ADMIN, true);

        mFragment.initialize(mActivity, mArguments);

        verify(mFragment).removePreference(KEY_GRANT_ADMIN);

    }



    @Test

    @RequiresFlagsEnabled(Flags.FLAG_UNICORN_MODE_REFACTORING_FOR_HSUM_READ_ONLY)

    public void initialize_currentUserRestrict_shouldNotShowGrantAdminPref_MultipleAdminEnabled() {

        setupSelectedUser();

        ShadowUserManager.setIsMultipleAdminEnabled(true);

        // current user has DISALLOW_GRANT_ADMIN restriction

        mUserManager.setUserRestriction(mContext.getUser(),

                UserManager.DISALLOW_GRANT_ADMIN, true);

        mFragment.initialize(mActivity, mArguments);

        verify(mFragment).removePreference(KEY_GRANT_ADMIN);

    }



    @Test

    public void initialize_mainUserSelected_shouldShowGrantAdminPref_MultipleAdminEnabled() {

        setupSelectedMainUser();