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

Commit e7580468 authored by Riley Campillo's avatar Riley Campillo
Browse files

Fixes bug where users could add other users when they should not have had that ability.

Consolidates logic in canCreateUser and canCreateGuest in a new method canAddMoreUsers that also checks UserManager.canAddUsers with specified  user type

Removes the check anyoneCanCreateUsers, since currentUserCanCreateUsers is a sufficient check. currentUserCanCreateUsers checks that both the current user and the system user do not have the DISALLOW_ADD_USER restriction.

Removes unnecessary isAddUsersFromLockScreenEnabled check in canManageUsers logic.

Uses (isDeviceLocked or isAddUsersFromLockScreenEnabled) condition rather than isAddUsersFromLockScreenEnabled

Flag: NONE
Fixes: 270370602
Test: UserSwitcherInteractorTest.kt, local hsum and non hsum testing
Bug:270370602

Change-Id: I85de669c9daee7b06825601f74b5193df0f31e5b
parent 9b5907be
Loading
Loading
Loading
Loading
+50 −47
Original line number Diff line number Diff line
@@ -24,50 +24,53 @@ import com.android.systemui.user.data.repository.UserRepository
/** Utilities related to user management actions. */
object UserActionsUtil {

    /** Returns `true` if it's possible to add a guest user to the device; `false` otherwise. */
    /**
     * Returns `true` if it's possible for the given user to add a guest user to the device; `false`
     * otherwise.
     */
    fun canCreateGuest(
        manager: UserManager,
        repository: UserRepository,
        isUserSwitcherEnabled: Boolean,
        isAddUsersFromLockScreenEnabled: Boolean,
        canAddUsersWhenLockedOrDeviceUnlocked: Boolean,
    ): Boolean {
        if (!isUserSwitcherEnabled) {
            return false
        }

        return currentUserCanCreateUsers(manager, repository) ||
            anyoneCanCreateUsers(manager, isAddUsersFromLockScreenEnabled)
        return canAddMoreUsers(
            manager,
            repository,
            isUserSwitcherEnabled,
            canAddUsersWhenLockedOrDeviceUnlocked,
            UserManager.USER_TYPE_FULL_GUEST
        )
    }

    /** Returns `true` if it's possible to add a user to the device; `false` otherwise. */
    /**
     * Returns `true` if it's possible for the given user to add a user to the device; `false`
     * otherwise.
     */
    fun canCreateUser(
        manager: UserManager,
        repository: UserRepository,
        isUserSwitcherEnabled: Boolean,
        isAddUsersFromLockScreenEnabled: Boolean,
        canAddUsersWhenLockedOrDeviceUnlocked: Boolean,
    ): Boolean {
        if (!isUserSwitcherEnabled) {
            return false
        }

        if (
            !currentUserCanCreateUsers(manager, repository) &&
                !anyoneCanCreateUsers(manager, isAddUsersFromLockScreenEnabled)
        ) {
            return false
        }

        return manager.canAddMoreUsers(UserManager.USER_TYPE_FULL_SECONDARY)
        return canAddMoreUsers(
            manager,
            repository,
            isUserSwitcherEnabled,
            canAddUsersWhenLockedOrDeviceUnlocked,
            UserManager.USER_TYPE_FULL_SECONDARY
        )
    }

    /**
     * Returns `true` if it's possible to add a supervised user to the device; `false` otherwise.
     * Returns `true` if it's possible to add a supervised user to the device given the current
     * user; false` otherwise.
     */
    fun canCreateSupervisedUser(
        manager: UserManager,
        repository: UserRepository,
        isUserSwitcherEnabled: Boolean,
        isAddUsersFromLockScreenEnabled: Boolean,
        canAddUsersWhenLockedOrDeviceUnlocked: Boolean,
        supervisedUserPackageName: String?
    ): Boolean {
        if (supervisedUserPackageName.isNullOrEmpty()) {
@@ -78,46 +81,46 @@ object UserActionsUtil {
            manager,
            repository,
            isUserSwitcherEnabled,
            isAddUsersFromLockScreenEnabled
            canAddUsersWhenLockedOrDeviceUnlocked
        )
    }

    fun canManageUsers(
        repository: UserRepository,
        isUserSwitcherEnabled: Boolean,
        isAddUsersFromLockScreenEnabled: Boolean,
    ): Boolean {
        return isUserSwitcherEnabled &&
            (repository.getSelectedUserInfo().isAdmin || isAddUsersFromLockScreenEnabled)
    fun canManageUsers(repository: UserRepository, isUserSwitcherEnabled: Boolean): Boolean {
        return isUserSwitcherEnabled && repository.getSelectedUserInfo().isAdmin
    }

    /**
     * Returns `true` if the current user is allowed to add users to the device; `false` otherwise.
     * Returns `true` if it's possible to add a user to the device for the given user type; false
     * otherwise.
     */
    private fun currentUserCanCreateUsers(
    private fun canAddMoreUsers(
        manager: UserManager,
        repository: UserRepository,
        isUserSwitcherEnabled: Boolean,
        canAddUsersWhenLockedOrDeviceUnlocked: Boolean,
        userType: String
    ): Boolean {
        val currentUser = repository.getSelectedUserInfo()
        if (!currentUser.isAdmin && currentUser.id != UserHandle.USER_SYSTEM) {
        if (!isUserSwitcherEnabled || !canAddUsersWhenLockedOrDeviceUnlocked) {
            return false
        }

        return systemCanCreateUsers(manager)
        return currentUserCanCreateUsers(manager, repository) && manager.canAddMoreUsers(userType)
    }

    /** Returns `true` if the system can add users to the device; `false` otherwise. */
    private fun systemCanCreateUsers(
    /**
     * Returns `true` if the current user is allowed to add users to the device; `false` otherwise.
     */
    private fun currentUserCanCreateUsers(
        manager: UserManager,
        repository: UserRepository
    ): Boolean {
        return !manager.hasBaseUserRestriction(UserManager.DISALLOW_ADD_USER, UserHandle.SYSTEM)
        val currentUser = repository.getSelectedUserInfo()
        if (!currentUser.isAdmin && currentUser.id != UserHandle.USER_SYSTEM) {
            return false
        }

    /** Returns `true` if it's allowed to add users to the device at all; `false` otherwise. */
    private fun anyoneCanCreateUsers(
        manager: UserManager,
        isAddUsersFromLockScreenEnabled: Boolean,
    ): Boolean {
        return systemCanCreateUsers(manager) && isAddUsersFromLockScreenEnabled
        return !manager.hasUserRestrictionForUser(
            UserManager.DISALLOW_ADD_USER,
            UserHandle.of(currentUser.id)
        ) && !manager.hasUserRestrictionForUser(UserManager.DISALLOW_ADD_USER, UserHandle.SYSTEM)
    }
}
+14 −11
Original line number Diff line number Diff line
@@ -170,7 +170,8 @@ constructor(
                keyguardInteractor.isKeyguardShowing,
            ) { _, userInfos, settings, isDeviceLocked ->
                buildList {
                    if (!isDeviceLocked || settings.isAddUsersFromLockscreen) {
                    val canAccessUserSwitcher = !isDeviceLocked || settings.isAddUsersFromLockscreen
                    if (canAccessUserSwitcher) {
                        // The device is locked and our setting to allow actions that add users
                        // from the lock-screen is not enabled. We can finish building the list
                        // here.
@@ -194,7 +195,10 @@ constructor(
                            when (it) {
                                UserActionModel.ENTER_GUEST_MODE -> {
                                    val hasGuestUser = userInfos.any { it.isGuest }
                                    if (!hasGuestUser && canCreateGuestUser(settings)) {
                                    if (
                                        !hasGuestUser &&
                                            canCreateGuestUser(settings, canAccessUserSwitcher)
                                    ) {
                                        add(UserActionModel.ENTER_GUEST_MODE)
                                    }
                                }
@@ -204,7 +208,7 @@ constructor(
                                            manager,
                                            repository,
                                            settings.isUserSwitcherEnabled,
                                            settings.isAddUsersFromLockscreen,
                                            canAccessUserSwitcher
                                        )

                                    if (canCreateUsers) {
@@ -217,7 +221,7 @@ constructor(
                                            manager,
                                            repository,
                                            settings.isUserSwitcherEnabled,
                                            settings.isAddUsersFromLockscreen,
                                            canAccessUserSwitcher,
                                            supervisedUserPackageName,
                                        )
                                    ) {
@@ -229,11 +233,7 @@ constructor(
                        }
                    }
                    if (
                        UserActionsUtil.canManageUsers(
                            repository,
                            settings.isUserSwitcherEnabled,
                            settings.isAddUsersFromLockscreen,
                        )
                        UserActionsUtil.canManageUsers(repository, settings.isUserSwitcherEnabled)
                    ) {
                        add(UserActionModel.NAVIGATE_TO_USER_MANAGEMENT)
                    }
@@ -820,13 +820,16 @@ constructor(
        )
    }

    private fun canCreateGuestUser(settings: UserSwitcherSettingsModel): Boolean {
    private fun canCreateGuestUser(
        settings: UserSwitcherSettingsModel,
        canAccessUserSwitcher: Boolean
    ): Boolean {
        return guestUserInteractor.isGuestUserAutoCreated ||
            UserActionsUtil.canCreateGuest(
                manager,
                repository,
                settings.isUserSwitcherEnabled,
                settings.isAddUsersFromLockscreen,
                canAccessUserSwitcher,
            )
    }

+130 −0
Original line number Diff line number Diff line
@@ -1014,6 +1014,136 @@ class UserSwitcherInteractorTest : SysuiTestCase() {
        verify(spyContext, never()).startServiceAsUser(any(), any())
    }

    @Test
    fun userIsAdminAndRestricted_addUserActionsNotAdded() {
        createUserInteractor()
        testScope.runTest {
            val id = 0
            val userInfo =
                UserInfo(
                    id,
                    "child",
                    /* iconPath= */ "",
                    /* flags= */ UserInfo.FLAG_ADMIN,
                    UserManager.USER_TYPE_FULL_RESTRICTED,
                )
            whenever(
                    manager.hasUserRestrictionForUser(
                        UserManager.DISALLOW_ADD_USER,
                        UserHandle.of(id)
                    )
                )
                .thenReturn(true)

            userRepository.setUserInfos(listOf(userInfo))
            userRepository.setSelectedUserInfo(userInfo)
            userRepository.setSettings(UserSwitcherSettingsModel(isUserSwitcherEnabled = true))

            val value = collectLastValue(underTest.actions)
            runCurrent()

            assertThat(value()).isEqualTo(listOf(UserActionModel.NAVIGATE_TO_USER_MANAGEMENT))
        }
    }

    @Test
    fun userIsNotRestrictedAndCannotAddGuests_actionsDoesNotIncludeAddGuest() {
        createUserInteractor()
        testScope.runTest {
            val userInfos = createUserInfos(count = 2, includeGuest = false)

            userRepository.setUserInfos(userInfos)
            userRepository.setSelectedUserInfo(userInfos[0])
            userRepository.setSettings(UserSwitcherSettingsModel(isUserSwitcherEnabled = true))
            keyguardRepository.setKeyguardShowing(false)

            whenever(manager.canAddMoreUsers(UserManager.USER_TYPE_FULL_GUEST)).thenReturn(false)

            val value = collectLastValue(underTest.actions)
            runCurrent()

            assertThat(value())
                .isEqualTo(
                    listOf(
                        UserActionModel.ADD_USER,
                        UserActionModel.ADD_SUPERVISED_USER,
                        UserActionModel.NAVIGATE_TO_USER_MANAGEMENT,
                    )
                )
        }
    }

    @Test
    fun userIsNotRestrictedAndCannotAddUsers_actionsDoesNotIncludeAddUsers() {
        createUserInteractor()
        testScope.runTest {
            val userInfos = createUserInfos(count = 2, includeGuest = false)

            userRepository.setUserInfos(userInfos)
            userRepository.setSelectedUserInfo(userInfos[0])
            userRepository.setSettings(UserSwitcherSettingsModel(isUserSwitcherEnabled = true))
            keyguardRepository.setKeyguardShowing(false)

            whenever(manager.canAddMoreUsers(UserManager.USER_TYPE_FULL_SECONDARY))
                .thenReturn(false)

            val value = collectLastValue(underTest.actions)
            runCurrent()

            assertThat(value())
                .isEqualTo(
                    listOf(
                        UserActionModel.ENTER_GUEST_MODE,
                        UserActionModel.NAVIGATE_TO_USER_MANAGEMENT,
                    )
                )
        }
    }

    @Test
    fun systemUserHasRestrictions_addUserActionsNotAdded() {
        createUserInteractor()
        testScope.runTest {
            val systemId = 0
            val systemUser =
                UserInfo(
                    systemId,
                    "system",
                    /* iconPath= */ "",
                    /* flags= */ UserInfo.FLAG_SYSTEM,
                    UserManager.USER_TYPE_SYSTEM_HEADLESS,
                )
            val adminId = 10
            val adminUser =
                UserInfo(
                    adminId,
                    "admin",
                    /* iconPath= */ "",
                    /* flags= */ UserInfo.FLAG_ADMIN,
                    UserManager.USER_TYPE_FULL_SYSTEM,
                )

            userRepository.setUserInfos(listOf(systemUser, adminUser))
            userRepository.setSelectedUserInfo(adminUser)
            userRepository.setSettings(UserSwitcherSettingsModel(isUserSwitcherEnabled = true))
            keyguardRepository.setKeyguardShowing(false)

            whenever(headlessSystemUserMode.isHeadlessSystemUserMode()).thenReturn(true)
            whenever(
                    manager.hasUserRestrictionForUser(
                        UserManager.DISALLOW_ADD_USER,
                        UserHandle.of(0)
                    )
                )
                .thenReturn(true)

            val value = collectLastValue(underTest.actions)
            runCurrent()

            assertThat(value()).isEqualTo(listOf(UserActionModel.NAVIGATE_TO_USER_MANAGEMENT))
        }
    }

    private fun assertUsers(
        models: List<UserModel>?,
        count: Int,