[PM-10671] update password generator policy (#3936)

This commit is contained in:
aj-rosado 2024-09-24 17:00:30 +02:00 committed by GitHub
parent 87e223bc59
commit 86bbf13175
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 155 additions and 225 deletions

View file

@ -47,7 +47,7 @@ sealed class PolicyInformation {
/**
* Represents a policy enforcing rules on the password generator.
*
* @property defaultType The default type of password to be generated.
* @property overridePasswordType The default type of password to be generated.
* @property minLength The minimum length of the password.
* @property useUpper Whether the password requires upper case letters.
* @property useLower Whether the password requires lower case letters.
@ -61,8 +61,8 @@ sealed class PolicyInformation {
*/
@Serializable
data class PasswordGenerator(
@SerialName("defaultType")
val defaultType: String?,
@SerialName("overridePasswordType")
val overridePasswordType: String?,
@SerialName("minLength")
val minLength: Int?,

View file

@ -6,7 +6,6 @@ import com.bitwarden.generators.PassphraseGeneratorRequest
import com.bitwarden.generators.PasswordGeneratorRequest
import com.bitwarden.generators.UsernameGeneratorRequest
import com.bitwarden.vault.PasswordHistoryView
import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation
import com.x8bit.bitwarden.data.platform.repository.model.LocalDataState
import com.x8bit.bitwarden.data.tools.generator.repository.model.GeneratedCatchAllUsernameResult
import com.x8bit.bitwarden.data.tools.generator.repository.model.GeneratedForwardedServiceUsernameResult
@ -85,11 +84,6 @@ interface GeneratorRepository {
forwardedServiceGeneratorRequest: UsernameGeneratorRequest.Forwarded,
): GeneratedForwardedServiceUsernameResult
/**
* Get the policy for password generation.
*/
fun getPasswordGeneratorPolicy(): PolicyInformation.PasswordGenerator?
/**
* Get the [PasscodeGenerationOptions] for the current user.
*/

View file

@ -7,10 +7,8 @@ import com.bitwarden.generators.PasswordGeneratorRequest
import com.bitwarden.generators.UsernameGeneratorRequest
import com.bitwarden.vault.PasswordHistoryView
import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource
import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
import com.x8bit.bitwarden.data.platform.manager.util.getActivePolicies
import com.x8bit.bitwarden.data.platform.repository.model.LocalDataState
import com.x8bit.bitwarden.data.platform.repository.util.observeWhenSubscribedAndLoggedIn
import com.x8bit.bitwarden.data.tools.generator.datasource.disk.GeneratorDiskSource
@ -42,7 +40,6 @@ import kotlinx.coroutines.flow.receiveAsFlow
import kotlinx.coroutines.launch
import java.time.Clock
import javax.inject.Singleton
import kotlin.math.max
/**
* Default implementation of [GeneratorRepository].
@ -204,73 +201,6 @@ class GeneratorRepositoryImpl(
},
)
@Suppress("LongMethod", "CyclomaticComplexMethod")
override fun getPasswordGeneratorPolicy(): PolicyInformation.PasswordGenerator? {
val policies: List<PolicyInformation.PasswordGenerator> = policyManager.getActivePolicies()
var minLength: Int? = null
var useUpper = false
var useLower = false
var useNumbers = false
var useSpecial = false
var minNumbers: Int? = null
var minSpecial: Int? = null
var minNumberWords: Int? = null
var capitalize = false
var includeNumber = false
var isPassphrasePresent = false
policies
.forEach { policy ->
if (policy.defaultType == PolicyInformation.PasswordGenerator.TYPE_PASSPHRASE) {
isPassphrasePresent = true
}
minLength = max(minLength ?: 0, policy.minLength ?: 0)
useUpper = useUpper || policy.useUpper == true
useLower = useLower || policy.useLower == true
useNumbers = useNumbers || policy.useNumbers == true
useSpecial = useSpecial || policy.useSpecial == true
minNumbers = max(minNumbers ?: 0, policy.minNumbers ?: 0)
minSpecial = max(minSpecial ?: 0, policy.minSpecial ?: 0)
minNumberWords = max(minNumberWords ?: 0, policy.minNumberWords ?: 0)
capitalize = capitalize || policy.capitalize == true
includeNumber = includeNumber || policy.includeNumber == true
}
// Only return a new policy if any policy settings were actually provided
return PolicyInformation.PasswordGenerator(
defaultType = if (isPassphrasePresent) {
PolicyInformation.PasswordGenerator.TYPE_PASSPHRASE
} else {
PolicyInformation.PasswordGenerator.TYPE_PASSWORD
},
minLength = minLength,
useUpper = useUpper,
useLower = useLower,
useNumbers = useNumbers,
useSpecial = useSpecial,
minNumbers = minNumbers,
minSpecial = minSpecial,
minNumberWords = minNumberWords,
capitalize = capitalize,
includeNumber = includeNumber,
).takeIf {
listOf(
minLength,
useUpper,
useLower,
useNumbers,
useSpecial,
minNumbers,
minSpecial,
minNumberWords,
capitalize,
includeNumber,
)
.any { it != null }
}
}
override fun getPasscodeGenerationOptions(): PasscodeGenerationOptions? {
val userId = authDiskSource.userState?.activeUserId
return userId?.let { generatorDiskSource.getPasscodeGenerationOptions(it) }

View file

@ -381,6 +381,7 @@ private fun ScrollContent(
onSubStateOptionClicked = onPasscodeSubStateOptionClicked,
passwordHandlers = passwordHandlers,
passphraseHandlers = passphraseHandlers,
overridePasswordPolicyRestriction = state.overridePassword,
)
}
@ -471,8 +472,9 @@ private fun ColumnScope.PasscodeTypeItems(
onSubStateOptionClicked: (GeneratorState.MainType.Passcode.PasscodeTypeOption) -> Unit,
passwordHandlers: PasswordHandlers,
passphraseHandlers: PassphraseHandlers,
overridePasswordPolicyRestriction: Boolean,
) {
PasscodeOptionsItem(passcodeState, onSubStateOptionClicked)
PasscodeOptionsItem(passcodeState, onSubStateOptionClicked, overridePasswordPolicyRestriction)
when (val selectedType = passcodeState.selectedType) {
is GeneratorState.MainType.Passcode.PasscodeType.Password -> {
@ -495,6 +497,7 @@ private fun ColumnScope.PasscodeTypeItems(
private fun PasscodeOptionsItem(
currentSubState: GeneratorState.MainType.Passcode,
onSubStateOptionClicked: (GeneratorState.MainType.Passcode.PasscodeTypeOption) -> Unit,
overridePasswordPolicyRestriction: Boolean,
) {
val possibleSubStates = GeneratorState.MainType.Passcode.PasscodeTypeOption.entries
val optionsWithStrings = possibleSubStates.associateWith { stringResource(id = it.labelRes) }
@ -508,6 +511,7 @@ private fun PasscodeOptionsItem(
optionsWithStrings.entries.first { it.value == selectedOption }.key
onSubStateOptionClicked(selectedOptionId)
},
isEnabled = !overridePasswordPolicyRestriction,
modifier = Modifier
.padding(horizontal = 16.dp)
.fillMaxWidth()

View file

@ -307,7 +307,7 @@ class GeneratorViewModel @Inject constructor(
}
}
@Suppress("CyclomaticComplexMethod")
@Suppress("CyclomaticComplexMethod", "LongMethod")
private fun loadPasscodeOptions(selectedType: Passcode) {
val options = generatorRepository.getPasscodeGenerationOptions()
?: generatePasscodeDefaultOptions()
@ -315,7 +315,22 @@ class GeneratorViewModel @Inject constructor(
val policy = policyManager
.getActivePolicies<PolicyInformation.PasswordGenerator>()
.toStrictestPolicy()
when (selectedType.selectedType) {
val passwordType = if (!policy.overridePasswordType.isNullOrBlank()) {
mutableStateFlow.update {
it.copy(overridePassword = true)
}
Passcode(
selectedType = policy.overridePasswordType.toSelectedType(),
)
} else {
mutableStateFlow.update {
it.copy(overridePassword = false)
}
selectedType
}
when (passwordType.selectedType) {
is Passphrase -> {
val minNumWords = policy.minNumberWords ?: Passphrase.PASSPHRASE_MIN_NUMBER_OF_WORDS
val passphrase = Passphrase(
@ -1713,6 +1728,7 @@ data class GeneratorState(
val currentEmailAddress: String,
val isUnderPolicy: Boolean = false,
val website: String? = null,
var overridePassword: Boolean = false,
) : Parcelable {
/**

View file

@ -1,6 +1,7 @@
package com.x8bit.bitwarden.ui.tools.feature.generator.util
import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation
import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation.PasswordGenerator.Companion.TYPE_PASSWORD
/**
* Creates the strictest set of rules based on the contents of the list of
@ -10,7 +11,14 @@ fun List<PolicyInformation.PasswordGenerator>.toStrictestPolicy():
PolicyInformation.PasswordGenerator {
return PolicyInformation.PasswordGenerator(
capitalize = mapNotNull { it.capitalize }.any { it },
defaultType = firstNotNullOfOrNull { it.defaultType },
overridePasswordType = mapNotNull { it.overridePasswordType }.reduceOrNull { acc, value ->
when {
// password should be prioritized over passphrase, else we keep the value
acc == TYPE_PASSWORD -> acc
value == TYPE_PASSWORD -> value
else -> value
}
},
includeNumber = mapNotNull { it.includeNumber }.any { it },
minLength = mapNotNull { it.minLength }.maxOrNull(),
minNumberWords = mapNotNull { it.minNumberWords }.maxOrNull(),

View file

@ -81,7 +81,7 @@ class SyncResponseJsonExtensionsTest {
@Test
fun `policyInformation converts the PasswordGenerator Json data to policy information`() {
val policyInformation = PolicyInformation.PasswordGenerator(
defaultType = "password",
overridePasswordType = "password",
minLength = null,
useUpper = true,
useLower = true,

View file

@ -18,7 +18,6 @@ import com.x8bit.bitwarden.data.auth.datasource.network.model.KdfTypeJson
import com.x8bit.bitwarden.data.auth.datasource.network.model.KeyConnectorUserDecryptionOptionsJson
import com.x8bit.bitwarden.data.auth.datasource.network.model.TrustedDeviceUserDecryptionOptionsJson
import com.x8bit.bitwarden.data.auth.datasource.network.model.UserDecryptionOptionsJson
import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation
import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
import com.x8bit.bitwarden.data.platform.repository.model.LocalDataState
@ -37,8 +36,6 @@ import com.x8bit.bitwarden.data.tools.generator.repository.model.GeneratedPlusAd
import com.x8bit.bitwarden.data.tools.generator.repository.model.GeneratedRandomWordUsernameResult
import com.x8bit.bitwarden.data.tools.generator.repository.model.PasscodeGenerationOptions
import com.x8bit.bitwarden.data.tools.generator.repository.model.UsernameGenerationOptions
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.datasource.sdk.VaultSdkSource
import io.mockk.coEvery
import io.mockk.coVerify
@ -49,11 +46,7 @@ import io.mockk.runs
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.test.runTest
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.encodeToJsonElement
import kotlinx.serialization.json.jsonObject
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertNotNull
import org.junit.jupiter.api.Assertions.assertNull
import org.junit.jupiter.api.Test
import java.time.Clock
@ -747,101 +740,6 @@ class GeneratorRepositoryTest {
generatorDiskSource.storeUsernameGenerationOptions(any(), any())
}
}
@Test
fun `getPasswordGeneratorPolicy returns default settings when no policies are present`() =
runTest {
every {
policyManager.getActivePolicies(type = PolicyTypeJson.PASSWORD_GENERATOR)
} returns emptyList()
val policy = repository.getPasswordGeneratorPolicy()
val expectedPolicy = PolicyInformation.PasswordGenerator(
defaultType = "password",
minLength = null,
useUpper = false,
useLower = false,
useNumbers = false,
useSpecial = false,
minNumbers = null,
minSpecial = null,
minNumberWords = null,
capitalize = false,
includeNumber = false,
)
assertNotNull(policy)
assertEquals(expectedPolicy, policy)
}
@Test
fun `getPasswordGeneratorPolicy applies strictest settings from multiple policies`() = runTest {
val policy1 = PolicyInformation.PasswordGenerator(
defaultType = "password",
minLength = 8,
useUpper = true,
useLower = true,
useNumbers = true,
useSpecial = false,
minNumbers = 1,
minSpecial = 1,
minNumberWords = 3,
capitalize = true,
includeNumber = true,
)
val policy2 = PolicyInformation.PasswordGenerator(
defaultType = "passphrase", // Different type, more specific in this context
minLength = 12, // More strict
useUpper = true,
useLower = true,
useNumbers = true,
useSpecial = true, // More strict
minNumbers = 2, // More strict
minSpecial = 2, // More strict
minNumberWords = 4, // More strict
capitalize = false, // Different, less strict, should not override
includeNumber = false, // Different, less strict, should not override
)
val policies = listOf(
SyncResponseJson.Policy(
id = "1",
type = PolicyTypeJson.PASSWORD_GENERATOR,
isEnabled = true,
data = Json.encodeToJsonElement(policy1).jsonObject,
organizationId = "id1",
),
SyncResponseJson.Policy(
id = "2",
type = PolicyTypeJson.PASSWORD_GENERATOR,
isEnabled = true,
data = Json.encodeToJsonElement(policy2).jsonObject,
organizationId = "id2",
),
)
every {
policyManager.getActivePolicies(type = PolicyTypeJson.PASSWORD_GENERATOR)
} returns policies
val resultPolicy = repository.getPasswordGeneratorPolicy()
// The expected combined policy
val expectedPolicy = PolicyInformation.PasswordGenerator(
defaultType = "passphrase",
minLength = 12,
useUpper = true,
useLower = true,
useNumbers = true,
useSpecial = true,
minNumbers = 2,
minSpecial = 2,
minNumberWords = 4,
capitalize = true,
includeNumber = true,
)
assertEquals(expectedPolicy, resultPolicy)
}
}
private val USER_STATE = UserStateJson(

View file

@ -142,10 +142,6 @@ class FakeGeneratorRepository : GeneratorRepository {
mutablePasswordHistoryStateFlow.value = LocalDataState.Loaded(emptyList())
}
override fun getPasswordGeneratorPolicy(): PolicyInformation.PasswordGenerator? {
return passwordGeneratorPolicy
}
/**
* Sets the mock result for the generatePassword function.
*/
@ -196,10 +192,6 @@ class FakeGeneratorRepository : GeneratorRepository {
generateRandomWordUsernameResult = result
}
fun setMockPasswordGeneratorPolicy(policy: PolicyInformation.PasswordGenerator?) {
this.passwordGeneratorPolicy = policy
}
/**
* Asserts that the passed in request matches the stored request.
*/

View file

@ -7,7 +7,6 @@ import com.bitwarden.generators.PasswordGeneratorRequest
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.auth.datasource.disk.model.OnboardingStatus
import com.x8bit.bitwarden.data.auth.repository.AuthRepository
import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation
import com.x8bit.bitwarden.data.auth.repository.model.UserState
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager
@ -183,7 +182,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
@Test
fun `activePolicyFlow changes should update state`() = runTest {
val payload = mapOf(
"defaultType" to JsonNull,
"overridePasswordType" to JsonNull,
"minLength" to JsonPrimitive(10),
"useUpper" to JsonPrimitive(true),
"useNumbers" to JsonPrimitive(true),
@ -392,8 +391,6 @@ class GeneratorViewModelTest : BaseViewModelTest() {
@Test
fun `RegenerateClick action for passphrase state updates generatedText and saves passphrase generation options on successful passphrase generation`() =
runTest {
setupMockPassphraseTypePolicy()
val updatedGeneratedPassphrase = "updatedPassphrase"
val viewModel = createViewModel(initialPassphraseState)
@ -436,8 +433,6 @@ class GeneratorViewModelTest : BaseViewModelTest() {
@Test
fun `RegenerateClick action for passphrase state sends ShowSnackbar event on passphrase generation failure`() =
runTest {
setupMockPassphraseTypePolicy()
val viewModel = createViewModel(initialPassphraseState)
fakeGeneratorRepository.setMockGeneratePassphraseResult(
@ -546,7 +541,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
isEnabled = true,
data = JsonObject(
mapOf(
"defaultType" to JsonNull,
"overridePasswordType" to JsonNull,
"minLength" to JsonPrimitive(10),
"useUpper" to JsonPrimitive(true),
"useNumbers" to JsonPrimitive(true),
@ -599,7 +594,7 @@ class GeneratorViewModelTest : BaseViewModelTest() {
isEnabled = true,
data = JsonObject(
mapOf(
"defaultType" to JsonNull,
"overridePasswordType" to JsonNull,
"minLength" to JsonPrimitive(10),
"useUpper" to JsonPrimitive(true),
"useNumbers" to JsonPrimitive(true),
@ -643,6 +638,78 @@ class GeneratorViewModelTest : BaseViewModelTest() {
)
}
@Test
fun `Policy should overwrite passwordType if has overridePasswordType`() {
val policy = createMockPolicy(
number = 1,
type = PolicyTypeJson.PASSWORD_GENERATOR,
isEnabled = true,
data = JsonObject(
mapOf(
"overridePasswordType" to JsonPrimitive("passphrase"),
),
),
)
every {
policyManager.getActivePolicies(any())
} returns listOf(policy)
val viewModel = createViewModel()
assertEquals(
initialPasscodeState.copy(
generatedText = "updatedPassphrase",
selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Passphrase(),
),
overridePassword = true,
),
viewModel.stateFlow.value,
)
}
@Test
fun `Policy should should prioritize password if multiple have OverridePasswordType`() {
val policies = listOf(
createMockPolicy(
number = 1,
type = PolicyTypeJson.PASSWORD_GENERATOR,
isEnabled = true,
data = JsonObject(
mapOf(
"overridePasswordType" to JsonPrimitive("passphrase"),
),
),
),
createMockPolicy(
number = 1,
type = PolicyTypeJson.PASSWORD_GENERATOR,
isEnabled = true,
data = JsonObject(
mapOf(
"overridePasswordType" to JsonPrimitive("password"),
),
),
),
)
every {
policyManager.getActivePolicies(any())
} returns policies
val viewModel = createViewModel()
assertEquals(
initialPasscodeState.copy(
generatedText = "defaultPassword",
selectedType = GeneratorState.MainType.Passcode(
GeneratorState.MainType.Passcode.PasscodeType.Password(),
),
overridePassword = true,
),
viewModel.stateFlow.value,
)
}
@Test
fun `MainTypeOptionSelect PASSWORD should switch to Passcode`() = runTest {
val viewModel = createViewModel()
@ -1529,7 +1596,6 @@ class GeneratorViewModelTest : BaseViewModelTest() {
@BeforeEach
fun setup() {
setupMockPassphraseTypePolicy()
fakeGeneratorRepository.setMockGeneratePasswordResult(
GeneratedPasswordResult.Success("defaultPassphrase"),
)
@ -2475,24 +2541,6 @@ class GeneratorViewModelTest : BaseViewModelTest() {
},
)
private fun setupMockPassphraseTypePolicy() {
fakeGeneratorRepository.setMockPasswordGeneratorPolicy(
PolicyInformation.PasswordGenerator(
defaultType = "passphrase",
minLength = null,
useUpper = false,
useLower = false,
useNumbers = false,
useSpecial = false,
minNumbers = null,
minSpecial = null,
minNumberWords = null,
capitalize = false,
includeNumber = false,
),
)
}
//endregion Helper Functions
}

View file

@ -9,7 +9,7 @@ class PolicyInformationPasswordGeneratorExtensionsTest {
fun `toStrictestPolicy should select the strictest version of each rule`() {
assertEquals(
PolicyInformation.PasswordGenerator(
defaultType = null,
overridePasswordType = "password",
minLength = 4,
capitalize = true,
includeNumber = true,
@ -24,10 +24,50 @@ class PolicyInformationPasswordGeneratorExtensionsTest {
listOf(POLICY_1, POLICY_2, POLICY_3).toStrictestPolicy(),
)
}
@Test
fun `toStrictestPolicy should select the strictest version of each rule and passphrase`() {
assertEquals(
PolicyInformation.PasswordGenerator(
overridePasswordType = "passphrase",
minLength = 4,
capitalize = true,
includeNumber = false,
minNumberWords = 2,
minNumbers = 9,
minSpecial = 3,
useLower = true,
useNumbers = false,
useSpecial = true,
useUpper = true,
),
listOf(POLICY_2, POLICY_3).toStrictestPolicy(),
)
}
@Test
fun `toStrictestPolicy returns default settings when no policies are present`() {
assertEquals(
PolicyInformation.PasswordGenerator(
overridePasswordType = null,
minLength = null,
capitalize = false,
includeNumber = false,
minNumberWords = null,
minNumbers = null,
minSpecial = null,
useLower = false,
useNumbers = false,
useSpecial = false,
useUpper = false,
),
listOf<PolicyInformation.PasswordGenerator>().toStrictestPolicy(),
)
}
}
private val POLICY_1 = PolicyInformation.PasswordGenerator(
defaultType = null,
overridePasswordType = "password",
minLength = 0,
capitalize = false,
includeNumber = true,
@ -41,7 +81,7 @@ private val POLICY_1 = PolicyInformation.PasswordGenerator(
)
private val POLICY_2 = PolicyInformation.PasswordGenerator(
defaultType = null,
overridePasswordType = "passphrase",
minLength = 0,
capitalize = false,
includeNumber = false,
@ -55,7 +95,7 @@ private val POLICY_2 = PolicyInformation.PasswordGenerator(
)
private val POLICY_3 = PolicyInformation.PasswordGenerator(
defaultType = null,
overridePasswordType = null,
minLength = 4,
capitalize = true,
includeNumber = false,