[PM-18451] Elevated privileges do not exempt from remove pin unlock policy (#4791)

This commit is contained in:
André Bispo
2025-02-26 16:48:56 +00:00
committed by GitHub
parent 55e03565a6
commit d04ac5e672
17 changed files with 46 additions and 114 deletions
@@ -9,7 +9,6 @@ import com.x8bit.bitwarden.data.vault.datasource.network.model.OrganizationType
* @property name The name of the organization (if applicable).
* @property shouldManageResetPassword Indicates that this user has the permission to manage their
* own password.
* @property shouldManagePolicies Indicates that this user has the permission to manage policies.
* @property shouldUseKeyConnector Indicates that the organization uses a key connector.
* @property role The user's role in the organization.
*/
@@ -17,7 +16,6 @@ data class Organization(
val id: String,
val name: String?,
val shouldManageResetPassword: Boolean,
val shouldManagePolicies: Boolean,
val shouldUseKeyConnector: Boolean,
val role: OrganizationType,
)
@@ -22,7 +22,6 @@ fun SyncResponseJson.Profile.Organization.toOrganization(): Organization =
shouldUseKeyConnector = this.shouldUseKeyConnector,
role = this.type,
shouldManageResetPassword = this.permissions.shouldManageResetPassword,
shouldManagePolicies = this.permissions.shouldManagePolicies,
)
/**
@@ -93,13 +93,21 @@ class PolicyManagerImpl(
organization: SyncResponseJson.Profile.Organization,
policyType: PolicyTypeJson,
): Boolean =
if (policyType == PolicyTypeJson.MAXIMUM_VAULT_TIMEOUT) {
organization.type == OrganizationType.OWNER
} else if (policyType == PolicyTypeJson.PASSWORD_GENERATOR) {
false
} else {
(organization.type == OrganizationType.OWNER ||
organization.type == OrganizationType.ADMIN) ||
organization.permissions.shouldManagePolicies
when (policyType) {
PolicyTypeJson.MAXIMUM_VAULT_TIMEOUT -> {
organization.type == OrganizationType.OWNER
}
PolicyTypeJson.PASSWORD_GENERATOR,
PolicyTypeJson.REMOVE_UNLOCK_WITH_PIN,
-> {
false
}
else -> {
(organization.type == OrganizationType.OWNER ||
organization.type == OrganizationType.ADMIN) ||
organization.permissions.shouldManagePolicies
}
}
}
@@ -6,7 +6,6 @@ import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.viewModelScope
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.Organization
import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation
import com.x8bit.bitwarden.data.auth.repository.model.UserFingerprintResult
import com.x8bit.bitwarden.data.auth.repository.util.policyInformation
@@ -22,9 +21,7 @@ import com.x8bit.bitwarden.data.platform.repository.model.VaultTimeout
import com.x8bit.bitwarden.data.platform.repository.model.VaultTimeoutAction
import com.x8bit.bitwarden.data.platform.repository.util.baseWebVaultUrlOrDefault
import com.x8bit.bitwarden.data.platform.util.isBuildVersionBelow
import com.x8bit.bitwarden.data.vault.datasource.network.model.OrganizationType
import com.x8bit.bitwarden.data.vault.datasource.network.model.PolicyTypeJson
import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
import com.x8bit.bitwarden.ui.platform.base.BaseViewModel
import com.x8bit.bitwarden.ui.platform.base.util.Text
@@ -116,10 +113,10 @@ class AccountSecurityViewModel @Inject constructor(
.launchIn(viewModelScope)
policyManager
.getActivePoliciesFlow(type = (PolicyTypeJson.REMOVE_UNLOCK_WITH_PIN))
.getActivePoliciesFlow(type = PolicyTypeJson.REMOVE_UNLOCK_WITH_PIN)
.map { policies ->
AccountSecurityAction.Internal.RemovePinPolicyUpdateReceive(
removeUnlockWithPinPolicyEnabled = arePoliciesValid(policies),
removeUnlockWithPinPolicyEnabled = policies.isNotEmpty(),
)
}
.onEach(::sendAction)
@@ -169,20 +166,6 @@ class AccountSecurityViewModel @Inject constructor(
}
}
private fun arePoliciesValid(policies: List<SyncResponseJson.Policy>) =
policies.any { policy ->
authRepository.userStateFlow.value?.activeAccount?.organizations?.any { org ->
(policy.organizationId == org.id) && !isExemptFromPolicy(org)
} == true
}
private fun isExemptFromPolicy(
userOrg: Organization,
) = userOrg.role == OrganizationType.OWNER ||
userOrg.role == OrganizationType.ADMIN ||
(userOrg.role == OrganizationType.CUSTOM &&
userOrg.shouldManagePolicies)
override fun handleAction(action: AccountSecurityAction): Unit = when (action) {
AccountSecurityAction.AccountFingerprintPhraseClick -> handleAccountFingerprintPhraseClick()
is AccountSecurityAction.AuthenticatorSyncToggle -> handleAuthenticatorSyncToggle(action)
@@ -4450,7 +4450,6 @@ class AuthRepositoryTest {
every { name } returns "orgName"
every { permissions } returns mockk {
every { shouldManageResetPassword } returns false
every { shouldManagePolicies } returns false
}
every { shouldUseKeyConnector } returns true
every { type } returns OrganizationType.USER
@@ -4476,7 +4475,6 @@ class AuthRepositoryTest {
every { name } returns "orgName"
every { permissions } returns mockk {
every { shouldManageResetPassword } returns false
every { shouldManagePolicies } returns false
}
every { shouldUseKeyConnector } returns true
every { type } returns OrganizationType.USER
@@ -4513,7 +4511,6 @@ class AuthRepositoryTest {
every { name } returns "orgName"
every { permissions } returns mockk {
every { shouldManageResetPassword } returns false
every { shouldManagePolicies } returns false
}
every { shouldUseKeyConnector } returns true
every { type } returns OrganizationType.USER
@@ -194,7 +194,6 @@ class AuthDiskSourceExtensionsTest {
shouldManageResetPassword = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
shouldManagePolicies = false,
),
),
),
@@ -205,7 +204,6 @@ class AuthDiskSourceExtensionsTest {
id = "mockId-2",
name = "mockName-2",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -218,7 +216,6 @@ class AuthDiskSourceExtensionsTest {
id = "mockId-3",
name = "mockName-3",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -366,7 +363,6 @@ class AuthDiskSourceExtensionsTest {
id = "mockId-1",
name = "mockName-1",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -398,7 +394,6 @@ class AuthDiskSourceExtensionsTest {
id = "mockId-1",
name = "mockName-1",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -411,7 +406,6 @@ class AuthDiskSourceExtensionsTest {
id = "mockId-2",
name = "mockName-2",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -21,7 +21,6 @@ class SyncResponseJsonExtensionsTest {
id = "mockId-1",
name = "mockName-1",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -37,7 +36,6 @@ class SyncResponseJsonExtensionsTest {
id = "mockId-1",
name = "mockName-1",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = true,
role = OrganizationType.ADMIN,
),
@@ -45,7 +43,6 @@ class SyncResponseJsonExtensionsTest {
id = "mockId-2",
name = "mockName-2",
shouldManageResetPassword = true,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.USER,
),
@@ -364,7 +364,6 @@ class UserStateJsonExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -429,7 +428,6 @@ class UserStateJsonExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -474,7 +472,6 @@ class UserStateJsonExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -535,7 +532,6 @@ class UserStateJsonExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -581,7 +577,6 @@ class UserStateJsonExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -650,7 +645,6 @@ class UserStateJsonExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -696,7 +690,6 @@ class UserStateJsonExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -765,7 +758,6 @@ class UserStateJsonExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -811,7 +803,6 @@ class UserStateJsonExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -880,7 +871,6 @@ class UserStateJsonExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -927,7 +917,6 @@ class UserStateJsonExtensionsTest {
// Key part of the result #1, this is true or the role is owner or
// admin
shouldManageResetPassword = true,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.USER,
),
@@ -999,7 +988,6 @@ class UserStateJsonExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = true,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.USER,
),
@@ -1204,7 +1192,6 @@ class UserStateJsonExtensionsTest {
// Key part of the result #1, this is true or the role is owner or
// admin
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.USER,
),
@@ -1275,7 +1262,6 @@ class UserStateJsonExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.USER,
),
@@ -1321,7 +1307,6 @@ class UserStateJsonExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -1392,7 +1377,6 @@ class UserStateJsonExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -159,7 +159,6 @@ private val DEFAULT_ACCOUNT = UserState.Account(
id = "orgId",
name = ORGANIZATION_NAME,
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = true,
role = OrganizationType.USER,
),
@@ -385,7 +385,6 @@ class RootNavViewModelTest : BaseViewModelTest() {
id = "orgId",
name = "orgName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = true,
role = OrganizationType.USER,
),
@@ -177,7 +177,31 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
mutableRemovePinPolicyFlow.emit(
listOf(
createMockPolicy(
isEnabled = true,
type = PolicyTypeJson.REMOVE_UNLOCK_WITH_PIN,
organizationId = "organizationUser",
),
),
)
viewModel.stateFlow.test {
assertEquals(
DEFAULT_STATE.copy(
removeUnlockWithPinPolicyEnabled = true,
),
awaitItem(),
)
}
}
@Test
fun `remove pin policy is true when user role is ADMIN`() = runTest {
val viewModel = createViewModel()
mutableRemovePinPolicyFlow.emit(
listOf(
createMockPolicy(
organizationId = "organizationAdmin",
isEnabled = true,
type = PolicyTypeJson.REMOVE_UNLOCK_WITH_PIN,
),
@@ -195,31 +219,7 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
}
@Test
fun `remove pin policy is false when user role is ADMIN`() = runTest {
val viewModel = createViewModel()
mutableRemovePinPolicyFlow.emit(
listOf(
createMockPolicy(
organizationId = "organizationAdmin",
isEnabled = true,
type = PolicyTypeJson.REMOVE_UNLOCK_WITH_PIN,
),
),
)
viewModel.stateFlow.test {
assertEquals(
DEFAULT_STATE.copy(
removeUnlockWithPinPolicyEnabled = false,
),
awaitItem(),
)
}
}
@Test
fun `remove pin policy is false when user role is OWNER`() = runTest {
fun `remove pin policy is true when user role is OWNER`() = runTest {
val viewModel = createViewModel()
mutableRemovePinPolicyFlow.emit(
@@ -235,7 +235,7 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.test {
assertEquals(
DEFAULT_STATE.copy(
removeUnlockWithPinPolicyEnabled = false,
removeUnlockWithPinPolicyEnabled = true,
),
awaitItem(),
)
@@ -243,7 +243,7 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
}
@Test
fun `remove pin policy is false when user role is CUSTOM with manage policies`() = runTest {
fun `remove pin policy is true when user role is CUSTOM with manage policies`() = runTest {
val viewModel = createViewModel()
mutableRemovePinPolicyFlow.emit(
@@ -259,7 +259,7 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.test {
assertEquals(
DEFAULT_STATE.copy(
removeUnlockWithPinPolicyEnabled = false,
removeUnlockWithPinPolicyEnabled = true,
),
awaitItem(),
)
@@ -988,7 +988,6 @@ private val DEFAULT_USER_STATE = UserState(
Organization(
id = "organizationUser",
name = "Organization User",
shouldManagePolicies = false,
shouldUseKeyConnector = false,
shouldManageResetPassword = false,
role = OrganizationType.USER,
@@ -996,7 +995,6 @@ private val DEFAULT_USER_STATE = UserState(
Organization(
id = "organizationAdmin",
name = "Organization Admin",
shouldManagePolicies = false,
shouldUseKeyConnector = false,
shouldManageResetPassword = false,
role = OrganizationType.ADMIN,
@@ -1004,7 +1002,6 @@ private val DEFAULT_USER_STATE = UserState(
Organization(
id = "organizationOwner",
name = "Organization Owner",
shouldManagePolicies = false,
shouldUseKeyConnector = false,
shouldManageResetPassword = false,
role = OrganizationType.OWNER,
@@ -1012,7 +1009,6 @@ private val DEFAULT_USER_STATE = UserState(
Organization(
id = "organizationCustom",
name = "Organization Owner",
shouldManagePolicies = true,
shouldUseKeyConnector = false,
shouldManageResetPassword = false,
role = OrganizationType.CUSTOM,
@@ -4598,7 +4598,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -562,7 +562,6 @@ class CipherViewExtensionsTest {
id = "mockOrganizationId-1",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -494,7 +494,6 @@ private val DEFAULT_USER_STATE = UserState(
id = "mockOrganizationId-1",
name = "mockOrganizationName-1",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -502,7 +501,6 @@ private val DEFAULT_USER_STATE = UserState(
id = "mockOrganizationId-2",
name = "mockOrganizationName-2",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -510,7 +508,6 @@ private val DEFAULT_USER_STATE = UserState(
id = "mockOrganizationId-3",
name = "mockOrganizationName-3",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -106,7 +106,6 @@ private fun createMockUserState(hasOrganizations: Boolean = true): UserState =
id = "mockOrganizationId-1",
name = "mockOrganizationName-1",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -114,7 +113,6 @@ private fun createMockUserState(hasOrganizations: Boolean = true): UserState =
id = "mockOrganizationId-2",
name = "mockOrganizationName-2",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -122,7 +120,6 @@ private fun createMockUserState(hasOrganizations: Boolean = true): UserState =
id = "mockOrganizationId-3",
name = "mockOrganizationName-3",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -240,7 +240,6 @@ class VaultViewModelTest : BaseViewModelTest() {
id = "organiationId",
name = "Test Organization",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -327,7 +326,6 @@ class VaultViewModelTest : BaseViewModelTest() {
id = "organizationId",
name = "Test Organization",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -559,7 +557,6 @@ class VaultViewModelTest : BaseViewModelTest() {
id = "testOrganizationId",
name = "Test Organization",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -80,7 +80,6 @@ class UserStateExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -108,7 +107,6 @@ class UserStateExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -140,7 +138,6 @@ class UserStateExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -172,7 +169,6 @@ class UserStateExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -219,7 +215,6 @@ class UserStateExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -264,7 +259,6 @@ class UserStateExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -313,7 +307,6 @@ class UserStateExtensionsTest {
id = "organizationId",
name = "organizationName",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -393,14 +386,12 @@ class UserStateExtensionsTest {
name = "Organization B",
shouldUseKeyConnector = false,
shouldManageResetPassword = false,
shouldManagePolicies = false,
role = OrganizationType.ADMIN,
),
Organization(
id = "organizationId-A",
name = "Organization A",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -452,7 +443,6 @@ class UserStateExtensionsTest {
id = "organizationId-B",
name = "Organization B",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),
@@ -460,7 +450,6 @@ class UserStateExtensionsTest {
id = "organizationId-A",
name = "Organization A",
shouldManageResetPassword = false,
shouldManagePolicies = false,
shouldUseKeyConnector = false,
role = OrganizationType.ADMIN,
),