From 2e3200f53d5e81880023e163b85f554a00eb9854 Mon Sep 17 00:00:00 2001 From: Sean Weiser <125889608+sean-livefront@users.noreply.github.com> Date: Thu, 1 Feb 2024 01:25:08 -0600 Subject: [PATCH] BIT-779: Enforce policies on passcode generator screen (#927) --- .../feature/generator/GeneratorViewModel.kt | 45 +++++-- ...yInformationPasswordGeneratorExtensions.kt | 24 ++++ .../generator/GeneratorViewModelTest.kt | 117 ++++++++++++++++++ ...ormationPasswordGeneratorExtensionsTest.kt | 69 +++++++++++ 4 files changed, 245 insertions(+), 10 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/util/PolicyInformationPasswordGeneratorExtensions.kt create mode 100644 app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/util/PolicyInformationPasswordGeneratorExtensionsTest.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt index b451e6dd2..f5e85c85c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModel.kt @@ -11,7 +11,10 @@ import com.bitwarden.generators.PasswordGeneratorRequest import com.bitwarden.generators.UsernameGeneratorRequest import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.auth.repository.AuthRepository +import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation +import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager +import com.x8bit.bitwarden.data.platform.manager.util.getActivePolicies import com.x8bit.bitwarden.data.tools.generator.repository.GeneratorRepository import com.x8bit.bitwarden.data.tools.generator.repository.model.GeneratedCatchAllUsernameResult import com.x8bit.bitwarden.data.tools.generator.repository.model.GeneratedForwardedServiceUsernameResult @@ -41,6 +44,7 @@ import com.x8bit.bitwarden.ui.tools.feature.generator.GeneratorState.MainType.Us import com.x8bit.bitwarden.ui.tools.feature.generator.GeneratorState.MainType.Username.UsernameType.PlusAddressedEmail import com.x8bit.bitwarden.ui.tools.feature.generator.GeneratorState.MainType.Username.UsernameType.RandomWord import com.x8bit.bitwarden.ui.tools.feature.generator.model.GeneratorMode +import com.x8bit.bitwarden.ui.tools.feature.generator.util.toStrictestPolicy import com.x8bit.bitwarden.ui.tools.feature.generator.util.toUsernameGeneratorRequest import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.Job @@ -50,6 +54,7 @@ import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import kotlinx.parcelize.Parcelize import javax.inject.Inject +import kotlin.math.max private const val KEY_STATE = "state" private const val KEY_GENERATOR_MODE = "key_generator_mode" @@ -70,6 +75,7 @@ class GeneratorViewModel @Inject constructor( private val clipboardManager: BitwardenClipboardManager, private val generatorRepository: GeneratorRepository, private val authRepository: AuthRepository, + private val policyManager: PolicyManager, ) : BaseViewModel( initialState = savedStateHandle[KEY_STATE] ?: GeneratorState( generatedText = "", @@ -81,6 +87,9 @@ class GeneratorViewModel @Inject constructor( generatorMode = GeneratorArgs(savedStateHandle).type, currentEmailAddress = requireNotNull(authRepository.userStateFlow.value?.activeAccount?.email), + isUnderPolicy = policyManager + .getActivePolicies() + .any(), ), ) { @@ -238,17 +247,25 @@ class GeneratorViewModel @Inject constructor( //region Generation Handlers + @Suppress("CyclomaticComplexMethod") private fun loadPasscodeOptions(selectedType: Passcode) { val options = generatorRepository.getPasscodeGenerationOptions() ?: generatePasscodeDefaultOptions() + val policy = policyManager + .getActivePolicies() + .toStrictestPolicy() when (selectedType.selectedType) { is Passphrase -> { + val minNumWords = policy.minNumberWords ?: Passphrase.PASSPHRASE_MIN_NUMBER_OF_WORDS val passphrase = Passphrase( - numWords = options.numWords, + numWords = max(options.numWords, minNumWords), + minNumWords = minNumWords, wordSeparator = options.wordSeparator.toCharArray().first(), - capitalize = options.allowCapitalize, - includeNumber = options.allowIncludeNumber, + capitalize = options.allowCapitalize || policy.capitalize == true, + capitalizeEnabled = policy.capitalize != true, + includeNumber = options.allowIncludeNumber || policy.includeNumber == true, + includeNumberEnabled = policy.includeNumber != true, ) updateGeneratorMainType { Passcode(selectedType = passphrase) @@ -256,14 +273,22 @@ class GeneratorViewModel @Inject constructor( } is Password -> { + val minLength = policy.minLength ?: Password.PASSWORD_LENGTH_SLIDER_MIN val password = Password( - length = options.length, - useCapitals = options.hasUppercase, - useLowercase = options.hasLowercase, - useNumbers = options.hasNumbers, - useSpecialChars = options.allowSpecial, - minNumbers = options.minNumber, - minSpecial = options.minSpecial, + length = max(options.length, minLength), + minLength = minLength, + useCapitals = options.hasUppercase || policy.useUpper == true, + capitalsEnabled = policy.useUpper != true, + useLowercase = options.hasLowercase || policy.useLower == true, + lowercaseEnabled = policy.useLower != true, + useNumbers = options.hasNumbers || policy.useNumbers == true, + numbersEnabled = policy.useNumbers != true, + useSpecialChars = options.allowSpecial || policy.useSpecial == true, + specialCharsEnabled = policy.useSpecial != true, + minNumbers = max(options.minNumber, policy.minNumbers ?: 0), + minNumbersAllowed = policy.minNumbers ?: 0, + minSpecial = max(options.minSpecial, policy.minSpecial ?: 0), + minSpecialAllowed = policy.minSpecial ?: 0, avoidAmbiguousChars = options.allowAmbiguousChar, ) updateGeneratorMainType { diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/util/PolicyInformationPasswordGeneratorExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/util/PolicyInformationPasswordGeneratorExtensions.kt new file mode 100644 index 000000000..475504fd5 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/generator/util/PolicyInformationPasswordGeneratorExtensions.kt @@ -0,0 +1,24 @@ +package com.x8bit.bitwarden.ui.tools.feature.generator.util + +import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation + +/** + * Creates the strictest set of rules based on the contents of the list of + * [PolicyInformation.PasswordGenerator]. + */ +fun List.toStrictestPolicy(): + PolicyInformation.PasswordGenerator { + return PolicyInformation.PasswordGenerator( + capitalize = mapNotNull { it.capitalize }.any { it }, + defaultType = firstNotNullOfOrNull { it.defaultType }, + includeNumber = mapNotNull { it.includeNumber }.any { it }, + minLength = mapNotNull { it.minLength }.maxOrNull(), + minNumberWords = mapNotNull { it.minNumberWords }.maxOrNull(), + minNumbers = mapNotNull { it.minNumbers }.maxOrNull(), + minSpecial = mapNotNull { it.minSpecial }.maxOrNull(), + useLower = mapNotNull { it.useLower }.any { it }, + useNumbers = mapNotNull { it.useNumbers }.any { it }, + useSpecial = mapNotNull { it.useSpecial }.any { it }, + useUpper = mapNotNull { it.useUpper }.any { it }, + ) +} diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt index b5f512a24..b07d71daa 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/GeneratorViewModelTest.kt @@ -6,7 +6,9 @@ import app.cash.turbine.turbineScope import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.auth.repository.AuthRepository 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 +import com.x8bit.bitwarden.data.platform.manager.util.getActivePolicies import com.x8bit.bitwarden.data.platform.repository.model.Environment import com.x8bit.bitwarden.data.tools.generator.repository.model.GeneratedCatchAllUsernameResult import com.x8bit.bitwarden.data.tools.generator.repository.model.GeneratedForwardedServiceUsernameResult @@ -16,6 +18,8 @@ import com.x8bit.bitwarden.data.tools.generator.repository.model.GeneratedRandom import com.x8bit.bitwarden.data.tools.generator.repository.model.GeneratorResult import com.x8bit.bitwarden.data.tools.generator.repository.model.PasscodeGenerationOptions import com.x8bit.bitwarden.data.tools.generator.repository.util.FakeGeneratorRepository +import com.x8bit.bitwarden.data.vault.datasource.network.model.PolicyTypeJson +import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockPolicy import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest import com.x8bit.bitwarden.ui.platform.base.util.asText import com.x8bit.bitwarden.ui.tools.feature.generator.model.GeneratorMode @@ -26,6 +30,9 @@ import io.mockk.runs import io.mockk.verify import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runTest +import kotlinx.serialization.json.JsonNull +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.JsonPrimitive import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Nested @@ -84,6 +91,10 @@ class GeneratorViewModelTest : BaseViewModelTest() { ) } + private val policyManager: PolicyManager = mockk { + every { getActivePolicies(PolicyTypeJson.PASSWORD_GENERATOR) } returns emptyList() + } + @Test fun `initial state should be correct when there is no saved state`() { val viewModel = createViewModel(state = null) @@ -331,6 +342,111 @@ class GeneratorViewModelTest : BaseViewModelTest() { } } + @Test + fun `Policy should overwrite password options if stricter`() { + val policy = createMockPolicy( + number = 1, + type = PolicyTypeJson.PASSWORD_GENERATOR, + isEnabled = true, + data = JsonObject( + mapOf( + "defaultType" to JsonNull, + "minLength" to JsonPrimitive(10), + "useUpper" to JsonPrimitive(true), + "useNumbers" to JsonPrimitive(true), + "useSpecial" to JsonPrimitive(true), + "minNumbers" to JsonPrimitive(3), + "minSpecial" to JsonPrimitive(3), + "minNumberWords" to JsonPrimitive(5), + "capitalize" to JsonPrimitive(true), + "includeNumber" to JsonPrimitive(true), + "useLower" to JsonPrimitive(true), + ), + ), + ) + every { + policyManager.getActivePolicies(any()) + } returns listOf(policy) + + val viewModel = createViewModel() + assertEquals( + initialPasscodeState.copy( + selectedType = GeneratorState.MainType.Passcode( + GeneratorState.MainType.Passcode.PasscodeType.Password( + length = 14, + minLength = 10, + useCapitals = true, + capitalsEnabled = false, + useLowercase = true, + lowercaseEnabled = false, + useNumbers = true, + numbersEnabled = false, + useSpecialChars = true, + specialCharsEnabled = false, + minNumbers = 3, + minNumbersAllowed = 3, + minSpecial = 3, + minSpecialAllowed = 3, + avoidAmbiguousChars = false, + ), + ), + ), + viewModel.stateFlow.value, + ) + } + + @Test + fun `Policy should overwrite passphrase options if stricter`() { + val policy = createMockPolicy( + number = 1, + type = PolicyTypeJson.PASSWORD_GENERATOR, + isEnabled = true, + data = JsonObject( + mapOf( + "defaultType" to JsonNull, + "minLength" to JsonPrimitive(10), + "useUpper" to JsonPrimitive(true), + "useNumbers" to JsonPrimitive(true), + "useSpecial" to JsonPrimitive(true), + "minNumbers" to JsonPrimitive(3), + "minSpecial" to JsonPrimitive(3), + "minNumberWords" to JsonPrimitive(5), + "capitalize" to JsonPrimitive(true), + "includeNumber" to JsonPrimitive(true), + "useLower" to JsonPrimitive(true), + ), + ), + ) + every { + policyManager.getActivePolicies(any()) + } returns listOf(policy) + + val viewModel = createViewModel() + val action = GeneratorAction.MainType.Passcode.PasscodeTypeOptionSelect( + passcodeTypeOption = GeneratorState.MainType.Passcode.PasscodeTypeOption.PASSPHRASE, + ) + + viewModel.actionChannel.trySend(action) + + assertEquals( + initialPasscodeState.copy( + generatedText = "updatedPassphrase", + selectedType = GeneratorState.MainType.Passcode( + GeneratorState.MainType.Passcode.PasscodeType.Passphrase( + numWords = 5, + minNumWords = 5, + maxNumWords = 20, + capitalize = true, + capitalizeEnabled = false, + includeNumber = true, + includeNumberEnabled = false, + ), + ), + ), + viewModel.stateFlow.value, + ) + } + @Test fun `MainTypeOptionSelect PASSWORD should switch to Passcode`() = runTest { val viewModel = createViewModel() @@ -1775,6 +1891,7 @@ class GeneratorViewModelTest : BaseViewModelTest() { clipboardManager = clipboardManager, generatorRepository = fakeGeneratorRepository, authRepository = authRepository, + policyManager = policyManager, ) private fun createViewModel( diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/util/PolicyInformationPasswordGeneratorExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/util/PolicyInformationPasswordGeneratorExtensionsTest.kt new file mode 100644 index 000000000..729a035b9 --- /dev/null +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/generator/util/PolicyInformationPasswordGeneratorExtensionsTest.kt @@ -0,0 +1,69 @@ +package com.x8bit.bitwarden.ui.tools.feature.generator.util + +import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test + +class PolicyInformationPasswordGeneratorExtensionsTest { + @Test + fun `toStrictestPolicy should select the strictest version of each rule`() { + assertEquals( + PolicyInformation.PasswordGenerator( + defaultType = null, + minLength = 4, + capitalize = true, + includeNumber = true, + minNumberWords = 2, + minNumbers = 9, + minSpecial = 3, + useLower = true, + useNumbers = false, + useSpecial = true, + useUpper = true, + ), + listOf(POLICY_1, POLICY_2, POLICY_3).toStrictestPolicy(), + ) + } +} + +private val POLICY_1 = PolicyInformation.PasswordGenerator( + defaultType = null, + minLength = 0, + capitalize = false, + includeNumber = true, + minNumberWords = 2, + minNumbers = 3, + minSpecial = null, + useLower = false, + useNumbers = false, + useSpecial = true, + useUpper = false, +) + +private val POLICY_2 = PolicyInformation.PasswordGenerator( + defaultType = null, + minLength = 0, + capitalize = false, + includeNumber = false, + minNumberWords = 0, + minNumbers = 0, + minSpecial = 0, + useLower = false, + useNumbers = false, + useSpecial = false, + useUpper = false, +) + +private val POLICY_3 = PolicyInformation.PasswordGenerator( + defaultType = null, + minLength = 4, + capitalize = true, + includeNumber = false, + minNumberWords = 2, + minNumbers = 9, + minSpecial = 3, + useLower = true, + useNumbers = false, + useSpecial = true, + useUpper = true, +)