diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerImpl.kt index 4ab94749395..2f3096ba22f 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerImpl.kt @@ -11,6 +11,7 @@ import com.x8bit.bitwarden.data.auth.repository.model.LogoutReason import com.x8bit.bitwarden.data.platform.datasource.disk.PushDiskSource import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource import com.x8bit.bitwarden.data.platform.manager.CredentialExchangeRegistryManager +import com.x8bit.bitwarden.data.platform.manager.policy.PasswordPolicyManager import com.x8bit.bitwarden.data.tools.generator.datasource.disk.GeneratorDiskSource import com.x8bit.bitwarden.data.tools.generator.datasource.disk.PasswordHistoryDiskSource import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource @@ -36,6 +37,7 @@ class UserLogoutManagerImpl( private val vaultDiskSource: VaultDiskSource, private val vaultSdkSource: VaultSdkSource, private val credentialExchangeRegistryManager: CredentialExchangeRegistryManager, + private val passwordPolicyManager: PasswordPolicyManager, dispatcherManager: DispatcherManager, ) : UserLogoutManager { private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined) @@ -116,6 +118,7 @@ class UserLogoutManagerImpl( } private fun clearData(userId: String) { + passwordPolicyManager.removePasswordToCheck(userId = userId) vaultSdkSource.clearCrypto(userId = userId) authDiskSource.clearData(userId = userId) generatorDiskSource.clearData(userId = userId) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/di/AuthManagerModule.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/di/AuthManagerModule.kt index 5f8a2f4f44a..dfdef119b08 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/di/AuthManagerModule.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/di/AuthManagerModule.kt @@ -30,6 +30,7 @@ import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource import com.x8bit.bitwarden.data.platform.manager.CredentialExchangeRegistryManager import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.PushManager +import com.x8bit.bitwarden.data.platform.manager.policy.PasswordPolicyManager import com.x8bit.bitwarden.data.tools.generator.datasource.disk.GeneratorDiskSource import com.x8bit.bitwarden.data.tools.generator.datasource.disk.PasswordHistoryDiskSource import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource @@ -124,6 +125,7 @@ object AuthManagerModule { vaultSdkSource: VaultSdkSource, dispatcherManager: DispatcherManager, credentialExchangeRegistryManager: CredentialExchangeRegistryManager, + passwordPolicyManager: PasswordPolicyManager, ): UserLogoutManager = UserLogoutManagerImpl( authDiskSource = authDiskSource, @@ -136,6 +138,7 @@ object AuthManagerModule { vaultSdkSource = vaultSdkSource, dispatcherManager = dispatcherManager, credentialExchangeRegistryManager = credentialExchangeRegistryManager, + passwordPolicyManager = passwordPolicyManager, ) @Provides diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt index 2a7f3f19a4c..0a59b18780a 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt @@ -2,7 +2,6 @@ package com.x8bit.bitwarden.data.auth.repository import com.bitwarden.network.model.GetTokenResponseJson import com.bitwarden.network.model.TwoFactorDataModel -import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetReason import com.x8bit.bitwarden.data.auth.datasource.disk.model.OnboardingStatus import com.x8bit.bitwarden.data.auth.manager.AuthRequestManager import com.x8bit.bitwarden.data.auth.manager.KdfManager @@ -19,8 +18,6 @@ import com.x8bit.bitwarden.data.auth.repository.model.LogoutReason import com.x8bit.bitwarden.data.auth.repository.model.NewSsoUserResult import com.x8bit.bitwarden.data.auth.repository.model.Organization import com.x8bit.bitwarden.data.auth.repository.model.PasswordHintResult -import com.x8bit.bitwarden.data.auth.repository.model.PasswordStrengthResult -import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation import com.x8bit.bitwarden.data.auth.repository.model.PrevalidateSsoResult import com.x8bit.bitwarden.data.auth.repository.model.RegisterResult import com.x8bit.bitwarden.data.auth.repository.model.RemovePasswordResult @@ -42,6 +39,7 @@ import com.x8bit.bitwarden.data.auth.repository.util.WebAuthResult import com.x8bit.bitwarden.data.auth.util.YubiKeyResult import com.x8bit.bitwarden.data.platform.datasource.network.authenticator.AuthenticatorProvider import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager +import com.x8bit.bitwarden.data.platform.manager.policy.PasswordPolicyManager import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.StateFlow @@ -54,6 +52,7 @@ interface AuthRepository : AuthRequestManager, BiometricsEncryptionManager, KdfManager, + PasswordPolicyManager, UserStateManager { /** * Models the current auth state. @@ -121,16 +120,6 @@ interface AuthRepository : */ var shouldTrustDevice: Boolean - /** - * Return the cached password policies for the current user. - */ - val passwordPolicies: List - - /** - * The reason for resetting the password. - */ - val passwordResetReason: ForcePasswordResetReason? - /** * The organization for the active user. */ @@ -373,13 +362,6 @@ interface AuthRepository : */ suspend fun getPasswordBreachCount(password: String): BreachCountResult - /** - * Get the password strength for the given [email] and [password] combo. - * If no value is passed for the [email] will use the active email of the current active - * account via the [userStateFlow]. - */ - suspend fun getPasswordStrength(email: String? = null, password: String): PasswordStrengthResult - /** * Validates the master password for the current logged-in user. */ @@ -390,12 +372,6 @@ interface AuthRepository : */ suspend fun validatePinUserKey(pin: String): ValidatePinResult - /** - * Validates the given [password] against the master password - * policies for the current user. - */ - suspend fun validatePasswordAgainstPolicies(password: String): Boolean - /** * Send a verification email. */ diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt index 5300aeaec16..88ef6c23482 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt @@ -51,8 +51,6 @@ import com.bitwarden.network.service.HaveIBeenPwnedService import com.bitwarden.network.service.IdentityService import com.bitwarden.network.service.OrganizationService import com.bitwarden.network.util.isSslHandShakeError -import com.bitwarden.policies.PolicyType -import com.bitwarden.policies.PolicyView import com.bitwarden.ui.platform.resource.BitwardenString import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource import com.x8bit.bitwarden.data.auth.datasource.disk.model.AccountJson @@ -61,7 +59,6 @@ import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetRea import com.x8bit.bitwarden.data.auth.datasource.disk.model.OnboardingStatus import com.x8bit.bitwarden.data.auth.datasource.network.model.DeviceDataModel import com.x8bit.bitwarden.data.auth.datasource.sdk.AuthSdkSource -import com.x8bit.bitwarden.data.auth.datasource.sdk.util.toInt import com.x8bit.bitwarden.data.auth.datasource.sdk.util.toKdfTypeJson import com.x8bit.bitwarden.data.auth.manager.AuthRequestManager import com.x8bit.bitwarden.data.auth.manager.KdfManager @@ -83,7 +80,6 @@ import com.x8bit.bitwarden.data.auth.repository.model.NewSsoUserResult import com.x8bit.bitwarden.data.auth.repository.model.Organization import com.x8bit.bitwarden.data.auth.repository.model.PasswordHintResult import com.x8bit.bitwarden.data.auth.repository.model.PasswordStrengthResult -import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation import com.x8bit.bitwarden.data.auth.repository.model.PrevalidateSsoResult import com.x8bit.bitwarden.data.auth.repository.model.RegisterResult import com.x8bit.bitwarden.data.auth.repository.model.RemovePasswordResult @@ -105,7 +101,6 @@ import com.x8bit.bitwarden.data.auth.repository.util.DuoCallbackTokenResult import com.x8bit.bitwarden.data.auth.repository.util.SsoCallbackResult import com.x8bit.bitwarden.data.auth.repository.util.WebAuthResult import com.x8bit.bitwarden.data.auth.repository.util.activeUserIdChangesFlow -import com.x8bit.bitwarden.data.auth.repository.util.policyInformation import com.x8bit.bitwarden.data.auth.repository.util.toAccountCryptographicState import com.x8bit.bitwarden.data.auth.repository.util.toDeviceInfo import com.x8bit.bitwarden.data.auth.repository.util.toOrganizations @@ -113,7 +108,6 @@ import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams import com.x8bit.bitwarden.data.auth.repository.util.toUserState import com.x8bit.bitwarden.data.auth.repository.util.updateForcePasswordReset import com.x8bit.bitwarden.data.auth.repository.util.updateMasterPasswordUnlock -import com.x8bit.bitwarden.data.auth.repository.util.userSwitchingChangesFlow import com.x8bit.bitwarden.data.auth.util.KdfParamsConstants.DEFAULT_PBKDF2_ITERATIONS import com.x8bit.bitwarden.data.auth.util.YubiKeyResult import com.x8bit.bitwarden.data.auth.util.toSdkParams @@ -122,9 +116,8 @@ import com.x8bit.bitwarden.data.platform.error.NoActiveUserException import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.LogsManager -import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.PushManager -import com.x8bit.bitwarden.data.platform.manager.util.getActivePolicies +import com.x8bit.bitwarden.data.platform.manager.policy.PasswordPolicyManager import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.platform.util.appLinksScheme @@ -143,13 +136,10 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.combine -import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map -import kotlinx.coroutines.flow.mapNotNull -import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn @@ -182,18 +172,19 @@ class AuthRepositoryImpl( private val keyConnectorManager: KeyConnectorManager, private val trustedDeviceManager: TrustedDeviceManager, private val userLogoutManager: UserLogoutManager, - private val policyManager: PolicyManager, private val userStateManager: UserStateManager, private val kdfManager: KdfManager, private val toastManager: ToastManager, private val featureFlagManager: FeatureFlagManager, logsManager: LogsManager, pushManager: PushManager, + passwordPolicyManager: PasswordPolicyManager, dispatcherManager: DispatcherManager, ) : AuthRepository, AuthRequestManager by authRequestManager, BiometricsEncryptionManager by biometricsEncryptionManager, KdfManager by kdfManager, + PasswordPolicyManager by passwordPolicyManager, UserStateManager by userStateManager { /** * A scope intended for use when simply collecting multiple flows in order to combine them. The @@ -231,12 +222,6 @@ class AuthRepositoryImpl( private var organizationIdentifier: String? = null - /** - * The password that needs to be checked against any organization policies before - * the user can complete the login flow. This value is stored using the user ID. - */ - private var passwordsToCheckMap = mutableMapOf() - private var keyConnectorResponse: GetTokenResponseJson.Success? = null override var twoFactorResponse: GetTokenResponseJson.TwoFactorRequired? = null @@ -299,16 +284,6 @@ class AuthRepositoryImpl( } } - override val passwordPolicies: List - get() = policyManager.getActivePolicies() - - override val passwordResetReason: ForcePasswordResetReason? - get() = authDiskSource - .userState - ?.activeAccount - ?.profile - ?.forcePasswordResetReason - override val organizations: List get() = activeUserId ?.let { authDiskSource.getOrganizations(it) } @@ -364,52 +339,6 @@ class AuthRepositoryImpl( .logoutFlow .onEach { logout(userId = it.userId, reason = LogoutReason.Notification) } .launchIn(unconfinedScope) - - // When the policies for the user have been set, complete the login process. - policyManager - .getActivePoliciesFlow(type = PolicyType.MASTER_PASSWORD) - .onEach { policies -> - val userId = activeUserId ?: return@onEach - - // If the user is logging on without a password, the check should complete. - val passwordToCheck = passwordsToCheckMap.remove(key = userId) ?: return@onEach - - // If the password already has to be reset for some other reason, there's no - // need to check the password policies. - if (passwordResetReason != null) return@onEach - - // Otherwise check the user's password against the policies and set or - // clear the force reset reason accordingly. - authDiskSource.userState = authDiskSource.userState?.updateForcePasswordReset( - userId = userId, - reason = ForcePasswordResetReason - .WEAK_MASTER_PASSWORD_ON_LOGIN - .takeIf { - !passwordPassesPolicies( - password = passwordToCheck, - policies = policies, - ) - }, - ) - } - .launchIn(unconfinedScope) - - // Clear the cached password whenever the user is no longer active - // or the vault is locked for that user. - merge( - authDiskSource - .userSwitchingChangesFlow - .mapNotNull { it.previousActiveUserId }, - vaultRepository - .vaultUnlockDataStateFlow - .filter { vaultUnlockDataList -> - // Clear if the active user is not currently unlocking or unlocked - vaultUnlockDataList.none { it.userId == activeUserId } - } - .mapNotNull { activeUserId }, - ) - .onEach { userId -> passwordsToCheckMap.remove(key = userId) } - .launchIn(unconfinedScope) } override suspend fun deleteAccountWithMasterPassword( @@ -1537,11 +1466,6 @@ class AuthRepositoryImpl( ) } - override suspend fun validatePasswordAgainstPolicies( - password: String, - ): Boolean = passwordPolicies - .all { validatePasswordAgainstPolicy(password, it) } - override suspend fun sendVerificationEmail( email: String, name: String, @@ -1619,62 +1543,6 @@ class AuthRepositoryImpl( onFailure = { RevokeFromOrganizationResult.Error(error = it) }, ) - @Suppress("CyclomaticComplexMethod") - private suspend fun validatePasswordAgainstPolicy( - password: String, - policy: PolicyInformation.MasterPassword, - ): Boolean { - // Check the password against all the enforced rules in the policy. - policy.minLength?.let { minLength -> - if (minLength > 0 && password.length < minLength) return false - } - policy.minComplexity?.let { minComplexity -> - // If there was a problem checking the complexity of the password, ignore - // the complexity checks and continue checking the other aspects of the policy. - val profile = authDiskSource.userState?.activeAccount?.profile ?: return@let - val passwordStrengthResult = getPasswordStrength(profile.email, password) - val passwordStrength = (passwordStrengthResult as? PasswordStrengthResult.Success) - ?.passwordStrength - ?.toInt() - ?: return@let - if (minComplexity > 0 && passwordStrength < minComplexity) return false - } - policy.requireUpper?.let { requiresUpper -> - if (requiresUpper && !password.any { it.isUpperCase() }) return false - } - policy.requireLower?.let { requiresLower -> - if (requiresLower && !password.any { it.isLowerCase() }) return false - } - policy.requireNumbers?.let { requiresNumbers -> - if (requiresNumbers && !password.any { it.isDigit() }) return false - } - policy.requireSpecial?.let { requiresSpecial -> - if (requiresSpecial && !password.contains("^.*[!@#$%\\^&*].*$".toRegex())) return false - } - - return true - } - - /** - * Return true if there are any [PolicyInformation.MasterPassword] policies that the user's - * master password has failed to pass. - */ - private suspend fun passwordPassesPolicies( - password: String, - policies: List, - ): Boolean { - // If there are no master password policies that are enabled and should be - // enforced on login, the check should complete. - val passwordPolicies = policies - .mapNotNull { it.policyInformation as? PolicyInformation.MasterPassword } - .filter { it.enforceOnLogin == true } - - // Check the password against all the policies. - return passwordPolicies.all { policy -> - validatePasswordAgainstPolicy(password, policy) - } - } - /** * Enrolls the active user in password reset if their organization requires it. */ @@ -1895,7 +1763,7 @@ class AuthRepositoryImpl( } // Cache the password to verify against any password policies after the sync completes. - passwordsToCheckMap.put(userId, it) + storePasswordToCheck(userId = userId, password = it) } settingsRepository.hasUserLoggedInOrCreatedAccount = true diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/di/AuthRepositoryModule.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/di/AuthRepositoryModule.kt index 8713b0fc263..b3abf86d81d 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/di/AuthRepositoryModule.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/di/AuthRepositoryModule.kt @@ -26,6 +26,7 @@ import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager import com.x8bit.bitwarden.data.platform.manager.LogsManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.PushManager +import com.x8bit.bitwarden.data.platform.manager.policy.PasswordPolicyManager import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource @@ -69,7 +70,7 @@ object AuthRepositoryModule { trustedDeviceManager: TrustedDeviceManager, userLogoutManager: UserLogoutManager, pushManager: PushManager, - policyManager: PolicyManager, + passwordPolicyManager: PasswordPolicyManager, logsManager: LogsManager, userStateManager: UserStateManager, kdfManager: KdfManager, @@ -97,7 +98,7 @@ object AuthRepositoryModule { trustedDeviceManager = trustedDeviceManager, userLogoutManager = userLogoutManager, pushManager = pushManager, - policyManager = policyManager, + passwordPolicyManager = passwordPolicyManager, logsManager = logsManager, userStateManager = userStateManager, kdfManager = kdfManager, diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt index d66908241ab..73b140d6565 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt @@ -5,6 +5,7 @@ import android.content.Context import androidx.core.content.getSystemService import com.bitwarden.core.data.manager.dispatcher.DispatcherManager import com.bitwarden.core.data.manager.dispatcher.DispatcherManagerImpl +import com.bitwarden.core.data.manager.encryption.EncryptionManager import com.bitwarden.core.data.manager.realtime.RealtimeManager import com.bitwarden.core.data.manager.realtime.RealtimeManagerImpl import com.bitwarden.core.data.manager.toast.ToastManager @@ -77,6 +78,8 @@ import com.x8bit.bitwarden.data.platform.manager.network.NetworkCookieManager import com.x8bit.bitwarden.data.platform.manager.network.NetworkCookieManagerImpl import com.x8bit.bitwarden.data.platform.manager.network.NetworkPermissionManager import com.x8bit.bitwarden.data.platform.manager.network.NetworkPermissionManagerImpl +import com.x8bit.bitwarden.data.platform.manager.policy.PasswordPolicyManager +import com.x8bit.bitwarden.data.platform.manager.policy.PasswordPolicyManagerImpl import com.x8bit.bitwarden.data.platform.manager.restriction.RestrictionManager import com.x8bit.bitwarden.data.platform.manager.restriction.RestrictionManagerImpl import com.x8bit.bitwarden.data.platform.manager.sdk.SdkPlatformApiFactory @@ -273,6 +276,22 @@ object PlatformManagerModule { featureFlagManager = featureFlagManager, ) + @Provides + @Singleton + fun providePasswordPolicyManager( + authDiskSource: AuthDiskSource, + authSdkSource: AuthSdkSource, + policyManager: PolicyManager, + encryptionManager: EncryptionManager, + dispatcherManager: DispatcherManager, + ): PasswordPolicyManager = PasswordPolicyManagerImpl( + authDiskSource = authDiskSource, + authSdkSource = authSdkSource, + policyManager = policyManager, + encryptionManager = encryptionManager, + dispatcherManager = dispatcherManager, + ) + @Provides @Singleton fun providePushManager( diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/policy/PasswordPolicyManager.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/policy/PasswordPolicyManager.kt new file mode 100644 index 00000000000..408b0869be0 --- /dev/null +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/policy/PasswordPolicyManager.kt @@ -0,0 +1,44 @@ +package com.x8bit.bitwarden.data.platform.manager.policy + +import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetReason +import com.x8bit.bitwarden.data.auth.repository.model.PasswordStrengthResult +import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation + +/** + * A manager for password policy requirements. + */ +interface PasswordPolicyManager { + /** + * Return the cached password policies for the current user. + */ + val passwordPolicies: List + + /** + * The reason for resetting the password. + */ + val passwordResetReason: ForcePasswordResetReason? + + /** + * Get the password strength for the given [email] and [password] combo. If no value is + * passed for the [email] will use the active email of the current active account. + */ + suspend fun getPasswordStrength( + email: String? = null, + password: String, + ): PasswordStrengthResult + + /** + * Remove the password to be validated against the Master Password Policy. + */ + fun removePasswordToCheck(userId: String) + + /** + * Store the password to be validated against the Master Password Policy. + */ + fun storePasswordToCheck(userId: String, password: String) + + /** + * Validates the given [password] against the master password policies for the current user. + */ + suspend fun validatePasswordAgainstPolicies(password: String): Boolean +} diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/policy/PasswordPolicyManagerImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/policy/PasswordPolicyManagerImpl.kt new file mode 100644 index 00000000000..e89a4fbb4bc --- /dev/null +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/platform/manager/policy/PasswordPolicyManagerImpl.kt @@ -0,0 +1,196 @@ +package com.x8bit.bitwarden.data.platform.manager.policy + +import com.bitwarden.core.data.manager.dispatcher.DispatcherManager +import com.bitwarden.core.data.manager.encryption.EncryptionManager +import com.bitwarden.policies.PolicyType +import com.bitwarden.policies.PolicyView +import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource +import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetReason +import com.x8bit.bitwarden.data.auth.datasource.disk.model.UserStateJson +import com.x8bit.bitwarden.data.auth.datasource.sdk.AuthSdkSource +import com.x8bit.bitwarden.data.auth.datasource.sdk.util.toInt +import com.x8bit.bitwarden.data.auth.repository.model.PasswordStrengthResult +import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation +import com.x8bit.bitwarden.data.auth.repository.util.policyInformation +import com.x8bit.bitwarden.data.auth.repository.util.updateForcePasswordReset +import com.x8bit.bitwarden.data.auth.repository.util.userSwitchingChangesFlow +import com.x8bit.bitwarden.data.platform.manager.PolicyManager +import com.x8bit.bitwarden.data.platform.manager.util.getActivePolicies +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.mapNotNull +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.update + +private const val ALIAS: String = "PasswordPolicyManager" + +/** + * The default [PasswordPolicyManager] implementation. This class is responsible for validating + * that password adhere to password policies. + */ +internal class PasswordPolicyManagerImpl( + private val authDiskSource: AuthDiskSource, + private val authSdkSource: AuthSdkSource, + private val policyManager: PolicyManager, + private val encryptionManager: EncryptionManager, + dispatcherManager: DispatcherManager, +) : PasswordPolicyManager { + + private val unconfinedScope: CoroutineScope = CoroutineScope(dispatcherManager.unconfined) + + private val userState: UserStateJson? get() = authDiskSource.userState + private val activeUserId: String? get() = userState?.activeUserId + + /** + * The encrypted password that needs to be checked against any organization policies before + * the user can complete the login flow. This value is stored using the user ID. + */ + private val mutablePasswordsToCheckFlow = MutableStateFlow(value = mapOf()) + + init { + // When the policies for the user have been set, complete the login process. + combine( + mutablePasswordsToCheckFlow, + policyManager.getActivePoliciesFlow(type = PolicyType.MASTER_PASSWORD), + ) { map, policies -> + val userId = activeUserId ?: return@combine null + val encryptedPasswordToCheck = map[userId] ?: return@combine null + removePasswordToCheck(userId = userId) + if (passwordResetReason != null) return@combine null + Triple(userId, encryptedPasswordToCheck, policies) + } + .filterNotNull() + .onEach { (userId, encryptedPasswordToCheck, policies) -> + encryptionManager + .decrypt(alias = ALIAS, bytes = encryptedPasswordToCheck) + .getOrNull() + ?.decodeToString() + ?.let { passwordToCheck -> + // Otherwise check the user's password against the policies and set or + // clear the force reset reason accordingly. + storeUserResetPasswordReason( + userId = userId, + reason = ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN.takeIf { + !passwordPassesPolicies( + password = passwordToCheck, + policies = policies, + ) + }, + ) + } + } + .launchIn(unconfinedScope) + authDiskSource + .userSwitchingChangesFlow + .mapNotNull { it.previousActiveUserId } + .onEach { userId -> removePasswordToCheck(userId = userId) } + .launchIn(unconfinedScope) + } + + override val passwordPolicies: List + get() = policyManager.getActivePolicies() + + override val passwordResetReason: ForcePasswordResetReason? + get() = userState?.activeAccount?.profile?.forcePasswordResetReason + + override suspend fun getPasswordStrength( + email: String?, + password: String, + ): PasswordStrengthResult = + authSdkSource + .passwordStrength( + email = email ?: userState?.activeAccount?.profile?.email.orEmpty(), + password = password, + ) + .fold( + onSuccess = { PasswordStrengthResult.Success(passwordStrength = it) }, + onFailure = { PasswordStrengthResult.Error(error = it) }, + ) + + override fun removePasswordToCheck(userId: String) { + mutablePasswordsToCheckFlow.update { it.toMutableMap().apply { remove(key = userId) } } + } + + override fun storePasswordToCheck(userId: String, password: String) { + encryptionManager + .encrypt(alias = ALIAS, bytes = password.encodeToByteArray()) + .getOrNull() + ?.let { encryptedPassword -> + mutablePasswordsToCheckFlow.update { + it.toMutableMap().apply { put(key = userId, value = encryptedPassword) } + } + } + } + + override suspend fun validatePasswordAgainstPolicies( + password: String, + ): Boolean = passwordPolicies.all { + validatePasswordAgainstPolicy(password = password, policy = it) + } + + @Suppress("CyclomaticComplexMethod") + private suspend fun validatePasswordAgainstPolicy( + password: String, + policy: PolicyInformation.MasterPassword, + ): Boolean { + // Check the password against all the enforced rules in the policy. + policy.minLength?.let { minLength -> + if (minLength > 0 && password.length < minLength) return false + } + policy.minComplexity?.let { minComplexity -> + // If there was a problem checking the complexity of the password, ignore + // the complexity checks and continue checking the other aspects of the policy. + val profile = userState?.activeAccount?.profile ?: return@let + val passwordStrengthResult = getPasswordStrength(profile.email, password) + val passwordStrength = (passwordStrengthResult as? PasswordStrengthResult.Success) + ?.passwordStrength + ?.toInt() + ?: return@let + if (minComplexity > 0 && passwordStrength < minComplexity) return false + } + policy.requireUpper?.let { requiresUpper -> + if (requiresUpper && !password.any { it.isUpperCase() }) return false + } + policy.requireLower?.let { requiresLower -> + if (requiresLower && !password.any { it.isLowerCase() }) return false + } + policy.requireNumbers?.let { requiresNumbers -> + if (requiresNumbers && !password.any { it.isDigit() }) return false + } + policy.requireSpecial?.let { requiresSpecial -> + if (requiresSpecial && !password.contains("^.*[!@#$%\\^&*].*$".toRegex())) return false + } + return true + } + + /** + * Update the saved state with the force password reset reason. + */ + private fun storeUserResetPasswordReason(userId: String, reason: ForcePasswordResetReason?) { + authDiskSource.userState = authDiskSource.userState?.updateForcePasswordReset( + userId = userId, + reason = reason, + ) + } + + /** + * Return true if there are any [PolicyInformation.MasterPassword] policies that the user's + * master password has failed to pass. + */ + private suspend fun passwordPassesPolicies( + password: String, + policies: List, + ): Boolean { + // If there are no master password policies that are enabled and should be + // enforced on login, the check should complete. + val passwordPolicies = policies + .mapNotNull { it.policyInformation as? PolicyInformation.MasterPassword } + .filter { it.enforceOnLogin == true } + + // Check the password against all the policies. + return passwordPolicies.all { policy -> validatePasswordAgainstPolicy(password, policy) } + } +} diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt index 0ff6dc089d8..f105f1f674c 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt @@ -32,6 +32,7 @@ import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams import com.x8bit.bitwarden.data.auth.repository.util.userAccountTokens import com.x8bit.bitwarden.data.auth.repository.util.userSwitchingChangesFlow import com.x8bit.bitwarden.data.platform.error.NoActiveUserException +import com.x8bit.bitwarden.data.platform.manager.policy.PasswordPolicyManager import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.platform.repository.model.VaultTimeout import com.x8bit.bitwarden.data.platform.repository.model.VaultTimeoutAction @@ -41,6 +42,7 @@ import com.x8bit.bitwarden.data.vault.manager.model.VaultStateEvent import com.x8bit.bitwarden.data.vault.repository.model.VaultUnlockData import com.x8bit.bitwarden.data.vault.repository.model.VaultUnlockResult import com.x8bit.bitwarden.data.vault.repository.util.logTag +import com.x8bit.bitwarden.data.vault.repository.util.password import com.x8bit.bitwarden.data.vault.repository.util.statusFor import com.x8bit.bitwarden.data.vault.repository.util.toV2UpgradeToken import com.x8bit.bitwarden.data.vault.repository.util.toVaultUnlockResult @@ -94,6 +96,7 @@ class VaultLockManagerImpl( private val trustedDeviceManager: TrustedDeviceManager, private val kdfManager: KdfManager, private val pinProtectedUserKeyManager: PinProtectedUserKeyManager, + private val passwordPolicyManager: PasswordPolicyManager, dispatcherManager: DispatcherManager, context: Context, ) : VaultLockManager { @@ -222,7 +225,7 @@ class VaultLockManagerImpl( initializeCryptoResult .toVaultUnlockResult() .also { - hashAndStoreMasterPassword( + processMasterPassword( initUserCryptoMethod = initUserCryptoMethod, email = email, kdf = kdf, @@ -254,29 +257,27 @@ class VaultLockManagerImpl( /** * Hashes a password and stores it as the master password hash for a given user. + * The password is also temporarily stored for validation against the Master Password policy. */ - private suspend fun hashAndStoreMasterPassword( + private suspend fun processMasterPassword( initUserCryptoMethod: InitUserCryptoMethod, email: String, kdf: Kdf, userId: String, ) { - (initUserCryptoMethod as? InitUserCryptoMethod.MasterPasswordUnlock)?.let { - // Save the master password hash. - authSdkSource - .hashPassword( - email = email, - password = initUserCryptoMethod.password, - kdf = kdf, - purpose = HashPurpose.LOCAL_AUTHORIZATION, - ) - .onSuccess { - authDiskSource.storeMasterPasswordHash( - userId = userId, - passwordHash = it, - ) - } - } + val password = initUserCryptoMethod.password ?: return + // Save the master password hash and store the password for policy validation. + authSdkSource + .hashPassword( + email = email, + password = password, + kdf = kdf, + purpose = HashPurpose.LOCAL_AUTHORIZATION, + ) + .onSuccess { hash -> + authDiskSource.storeMasterPasswordHash(userId = userId, passwordHash = hash) + } + passwordPolicyManager.storePasswordToCheck(userId = userId, password = password) } override suspend fun waitUntilUnlocked(userId: String) { @@ -350,6 +351,7 @@ class VaultLockManagerImpl( userId = userId, userAutoUnlockKey = null, ) + passwordPolicyManager.removePasswordToCheck(userId = userId) if (!wasVaultLocked) { mutableVaultStateEventSharedFlow.tryEmit(VaultStateEvent.Locked(userId = userId)) authDiskSource.storeLastLockTimestamp( @@ -699,19 +701,7 @@ class VaultLockManagerImpl( } private suspend fun updateKdfIfNeeded(initUserCryptoMethod: InitUserCryptoMethod) { - val password = when (initUserCryptoMethod) { - is InitUserCryptoMethod.MasterPasswordUnlock -> initUserCryptoMethod.password - is InitUserCryptoMethod.AuthRequest, - is InitUserCryptoMethod.DecryptedKey, - is InitUserCryptoMethod.DeviceKey, - is InitUserCryptoMethod.KeyConnector, - is InitUserCryptoMethod.KeyConnectorUrl, - is InitUserCryptoMethod.Pin, - is InitUserCryptoMethod.PinEnvelope, - is InitUserCryptoMethod.PinState, - -> return - } - + val password = initUserCryptoMethod.password ?: return kdfManager .updateKdfToMinimumsIfNeeded( password = password, @@ -768,7 +758,7 @@ class VaultLockManagerImpl( * Indicates the app has entered a Created state. * * @param firstTimeCreation if this is the first time the process is being created. - * @param createdForAutofill if the the creation event is due to an activity being launched + * @param createdForAutofill if the creation event is due to an activity being launched * for autofill. */ data class AppCreated( diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/di/VaultManagerModule.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/di/VaultManagerModule.kt index 21452a4a680..f7778b21378 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/di/VaultManagerModule.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/di/VaultManagerModule.kt @@ -26,6 +26,7 @@ import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.PushManager import com.x8bit.bitwarden.data.platform.manager.ReviewPromptManager import com.x8bit.bitwarden.data.platform.manager.network.NetworkConnectionManager +import com.x8bit.bitwarden.data.platform.manager.policy.PasswordPolicyManager import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource @@ -181,6 +182,7 @@ object VaultManagerModule { trustedDeviceManager: TrustedDeviceManager, kdfManager: KdfManager, pinProtectedUserKeyManager: PinProtectedUserKeyManager, + passwordPolicyManager: PasswordPolicyManager, ): VaultLockManager = VaultLockManagerImpl( context = context, @@ -196,6 +198,7 @@ object VaultManagerModule { trustedDeviceManager = trustedDeviceManager, kdfManager = kdfManager, pinProtectedUserKeyManager = pinProtectedUserKeyManager, + passwordPolicyManager = passwordPolicyManager, ) @Provides diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/util/InitUserCryptoMethodExtensions.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/util/InitUserCryptoMethodExtensions.kt index e18723514c1..f0d7a9d30b9 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/util/InitUserCryptoMethodExtensions.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/util/InitUserCryptoMethodExtensions.kt @@ -18,3 +18,20 @@ val InitUserCryptoMethod.logTag: String is InitUserCryptoMethod.KeyConnectorUrl -> "Key Connector Url" is InitUserCryptoMethod.MasterPasswordUnlock -> "Master Password Unlock" } + +/** + * Returns the password for the given [InitUserCryptoMethod] if available. + */ +val InitUserCryptoMethod.password: String? + get() = when (this) { + is InitUserCryptoMethod.MasterPasswordUnlock -> this.password + is InitUserCryptoMethod.AuthRequest, + is InitUserCryptoMethod.DecryptedKey, + is InitUserCryptoMethod.DeviceKey, + is InitUserCryptoMethod.KeyConnector, + is InitUserCryptoMethod.KeyConnectorUrl, + is InitUserCryptoMethod.Pin, + is InitUserCryptoMethod.PinEnvelope, + is InitUserCryptoMethod.PinState, + -> null + } diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerTest.kt index 12050c61a97..1c83d76e00f 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/manager/UserLogoutManagerTest.kt @@ -14,6 +14,7 @@ import com.x8bit.bitwarden.data.platform.datasource.disk.PushDiskSource import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource import com.x8bit.bitwarden.data.platform.manager.CredentialExchangeRegistryManager import com.x8bit.bitwarden.data.platform.manager.model.UnregisterExportResult +import com.x8bit.bitwarden.data.platform.manager.policy.PasswordPolicyManager import com.x8bit.bitwarden.data.platform.repository.model.VaultTimeoutAction import com.x8bit.bitwarden.data.tools.generator.datasource.disk.GeneratorDiskSource import com.x8bit.bitwarden.data.tools.generator.datasource.disk.PasswordHistoryDiskSource @@ -63,6 +64,9 @@ class UserLogoutManagerTest { private val credentialExchangeRegistryManager: CredentialExchangeRegistryManager = mockk { coEvery { unregister() } returns UnregisterExportResult.Success } + private val passwordPolicyManager: PasswordPolicyManager = mockk { + every { removePasswordToCheck(userId = any()) } just runs + } private val userLogoutManager: UserLogoutManager = UserLogoutManagerImpl( @@ -76,6 +80,7 @@ class UserLogoutManagerTest { vaultSdkSource = vaultSdkSource, dispatcherManager = FakeDispatcherManager(), credentialExchangeRegistryManager = credentialExchangeRegistryManager, + passwordPolicyManager = passwordPolicyManager, ) @Suppress("MaxLineLength") @@ -291,13 +296,16 @@ class UserLogoutManagerTest { } private fun assertDataCleared(userId: String) { - verify { vaultSdkSource.clearCrypto(userId = userId) } - verify { authDiskSource.clearData(userId = userId) } - verify { generatorDiskSource.clearData(userId = userId) } - verify { pushDiskSource.clearData(userId = userId) } - verify { settingsDiskSource.clearData(userId = userId) } - coVerify { passwordHistoryDiskSource.clearPasswordHistories(userId = userId) } - coVerify { + verify(exactly = 1) { + passwordPolicyManager.removePasswordToCheck(userId = userId) + vaultSdkSource.clearCrypto(userId = userId) + authDiskSource.clearData(userId = userId) + generatorDiskSource.clearData(userId = userId) + pushDiskSource.clearData(userId = userId) + settingsDiskSource.clearData(userId = userId) + } + coVerify(exactly = 1) { + passwordHistoryDiskSource.clearPasswordHistories(userId = userId) vaultDiskSource.deleteVaultData(userId = userId) } } diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt index 53ad2bbc7b1..fc822e5599a 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt @@ -71,8 +71,6 @@ import com.bitwarden.network.service.DevicesService import com.bitwarden.network.service.HaveIBeenPwnedService import com.bitwarden.network.service.IdentityService import com.bitwarden.network.service.OrganizationService -import com.bitwarden.policies.PolicyType -import com.bitwarden.policies.PolicyView import com.bitwarden.ui.platform.resource.BitwardenString import com.x8bit.bitwarden.data.auth.datasource.disk.model.AccountJson import com.x8bit.bitwarden.data.auth.datasource.disk.model.AccountTokensJson @@ -82,11 +80,6 @@ import com.x8bit.bitwarden.data.auth.datasource.disk.model.PendingAuthRequestJso import com.x8bit.bitwarden.data.auth.datasource.disk.model.UserStateJson import com.x8bit.bitwarden.data.auth.datasource.disk.util.FakeAuthDiskSource import com.x8bit.bitwarden.data.auth.datasource.sdk.AuthSdkSource -import com.x8bit.bitwarden.data.auth.datasource.sdk.model.PasswordStrength.LEVEL_0 -import com.x8bit.bitwarden.data.auth.datasource.sdk.model.PasswordStrength.LEVEL_1 -import com.x8bit.bitwarden.data.auth.datasource.sdk.model.PasswordStrength.LEVEL_2 -import com.x8bit.bitwarden.data.auth.datasource.sdk.model.PasswordStrength.LEVEL_3 -import com.x8bit.bitwarden.data.auth.datasource.sdk.model.PasswordStrength.LEVEL_4 import com.x8bit.bitwarden.data.auth.datasource.sdk.util.toKdfRequestModel import com.x8bit.bitwarden.data.auth.manager.AuthRequestManager import com.x8bit.bitwarden.data.auth.manager.KdfManager @@ -110,7 +103,6 @@ import com.x8bit.bitwarden.data.auth.repository.model.LogoutReason import com.x8bit.bitwarden.data.auth.repository.model.NewSsoUserResult import com.x8bit.bitwarden.data.auth.repository.model.Organization import com.x8bit.bitwarden.data.auth.repository.model.PasswordHintResult -import com.x8bit.bitwarden.data.auth.repository.model.PasswordStrengthResult import com.x8bit.bitwarden.data.auth.repository.model.PrevalidateSsoResult import com.x8bit.bitwarden.data.auth.repository.model.RegisterResult import com.x8bit.bitwarden.data.auth.repository.model.RemovePasswordResult @@ -140,13 +132,12 @@ import com.x8bit.bitwarden.data.platform.datasource.disk.util.FakeSettingsDiskSo import com.x8bit.bitwarden.data.platform.error.NoActiveUserException import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.LogsManager -import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.PushManager import com.x8bit.bitwarden.data.platform.manager.model.NotificationLogoutData +import com.x8bit.bitwarden.data.platform.manager.policy.PasswordPolicyManager import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.platform.repository.util.FakeEnvironmentRepository import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource -import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockPolicyView import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.data.vault.repository.model.VaultUnlockData import com.x8bit.bitwarden.data.vault.repository.model.VaultUnlockResult @@ -255,15 +246,12 @@ class AuthRepositoryTest { private val mutableLogoutFlow = bufferedMutableSharedFlow() private val mutableSyncOrgKeysFlow = bufferedMutableSharedFlow() - private val mutableActivePolicyFlow = bufferedMutableSharedFlow>() private val pushManager: PushManager = mockk { every { logoutFlow } returns mutableLogoutFlow every { syncOrgKeysFlow } returns mutableSyncOrgKeysFlow } - private val policyManager: PolicyManager = mockk { - every { - getActivePoliciesFlow(type = PolicyType.MASTER_PASSWORD) - } returns mutableActivePolicyFlow + private val passwordPolicyManager: PasswordPolicyManager = mockk { + every { storePasswordToCheck(userId = any(), password = any()) } just runs } private val logsManager: LogsManager = mockk { every { setUserData(userId = any(), environmentType = any()) } just runs @@ -318,7 +306,7 @@ class AuthRepositoryTest { userLogoutManager = userLogoutManager, dispatcherManager = dispatcherManager, pushManager = pushManager, - policyManager = policyManager, + passwordPolicyManager = passwordPolicyManager, logsManager = logsManager, userStateManager = userStateManager, kdfManager = kdfManager, @@ -402,124 +390,6 @@ class AuthRepositoryTest { } } - @Test - @Suppress("MaxLineLength") - fun `loading the policies should emit masterPasswordPolicyFlow if the password fails any checks`() = - runTest { - val successResponse = GET_TOKEN_WITH_ACCOUNT_KEYS_RESPONSE_SUCCESS - coEvery { - identityService.preLogin(email = EMAIL) - } returns PRE_LOGIN_SUCCESS.asSuccess() - coEvery { - identityService.getToken( - email = EMAIL, - authModel = IdentityTokenAuthModel.MasterPassword( - username = EMAIL, - password = PASSWORD_HASH, - ), - uniqueAppId = UNIQUE_APP_ID, - deeplinkScheme = DEEPLINK_SCHEME, - ) - } returns successResponse.asSuccess() - coEvery { - vaultRepository.unlockVault( - accountCryptographicState = ACCOUNT_CRYPTOGRAPHIC_STATE_V2, - userId = USER_ID_1, - email = EMAIL, - kdf = ACCOUNT_1.profile.toSdkParams(), - initUserCryptoMethod = InitUserCryptoMethod.MasterPasswordUnlock( - password = PASSWORD, - masterPasswordUnlock = MOCK_MASTER_PASSWORD_UNLOCK, - ), - organizationKeys = null, - ) - } returns VaultUnlockResult.Success - coEvery { vaultRepository.syncIfNecessary() } just runs - every { - GET_TOKEN_WITH_ACCOUNT_KEYS_RESPONSE_SUCCESS.toUserState( - previousUserState = null, - environmentUrlData = EnvironmentUrlDataJson.DEFAULT_US, - ) - } returns SINGLE_USER_STATE_1 - - // Start the login flow so that all the necessary data is cached. - val result = repository.login(email = EMAIL, password = PASSWORD) - - // Set policies that will fail the password. - mutableActivePolicyFlow.emit( - listOf( - createMockPolicyView( - type = PolicyType.MASTER_PASSWORD, - enabled = true, - data = """ - { - "minLength":100, - "minComplexity":null, - "requireUpper":null, - "requireLower":null, - "requireNumbers":null, - "requireSpecial":null, - "enforceOnLogin":true - } - """, - ), - ), - ) - - // Verify the results. - assertEquals(LoginResult.Success, result) - assertEquals(AuthState.Authenticated(ACCESS_TOKEN), repository.authStateFlow.value) - coVerify { identityService.preLogin(email = EMAIL) } - fakeAuthDiskSource.assertAccountCryptographicState( - userId = USER_ID_1, - accountCryptographicState = ACCOUNT_CRYPTOGRAPHIC_STATE_V2, - ) - fakeAuthDiskSource.assertMasterPasswordHash( - userId = USER_ID_1, - passwordHash = PASSWORD_HASH, - ) - coVerify { - identityService.getToken( - email = EMAIL, - authModel = IdentityTokenAuthModel.MasterPassword( - username = EMAIL, - password = PASSWORD_HASH, - ), - uniqueAppId = UNIQUE_APP_ID, - deeplinkScheme = DEEPLINK_SCHEME, - ) - vaultRepository.unlockVault( - accountCryptographicState = ACCOUNT_CRYPTOGRAPHIC_STATE_V2, - userId = USER_ID_1, - email = EMAIL, - kdf = ACCOUNT_1.profile.toSdkParams(), - initUserCryptoMethod = InitUserCryptoMethod.MasterPasswordUnlock( - password = PASSWORD, - masterPasswordUnlock = MOCK_MASTER_PASSWORD_UNLOCK, - ), - organizationKeys = null, - ) - vaultRepository.syncIfNecessary() - } - assertEquals( - UserStateJson( - activeUserId = USER_ID_1, - accounts = mapOf( - USER_ID_1 to ACCOUNT_1.copy( - profile = ACCOUNT_1.profile.copy( - forcePasswordResetReason = ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN, - ), - ), - ), - ), - fakeAuthDiskSource.userState, - ) - verify(exactly = 1) { - userStateManager.hasPendingAccountAddition = false - settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1) - } - } - @Test fun `rememberedEmailAddress should pull from and update AuthDiskSource`() { // AuthDiskSource and the repository start with the same value. @@ -578,25 +448,6 @@ class AuthRepositoryTest { assertEquals(false, repository.shouldTrustDevice) } - @Test - fun `passwordResetReason should pull from the user's profile in AuthDiskSource`() = runTest { - val updatedProfile = ACCOUNT_1.profile.copy( - forcePasswordResetReason = ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN, - ) - fakeAuthDiskSource.userState = UserStateJson( - activeUserId = USER_ID_1, - accounts = mapOf( - USER_ID_1 to ACCOUNT_1.copy( - profile = updatedProfile, - ), - ), - ) - assertEquals( - ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN, - repository.passwordResetReason, - ) - } - @Test fun `organizations should return an empty list when there is no active user`() = runTest { assertEquals(emptyList(), repository.organizations) @@ -2096,6 +1947,7 @@ class AuthRepositoryTest { verify(exactly = 1) { userStateManager.hasPendingAccountAddition = false settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1) + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = PASSWORD) } } @@ -2185,6 +2037,7 @@ class AuthRepositoryTest { ) verify(exactly = 1) { userStateManager.hasPendingAccountAddition = false + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = PASSWORD) settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1) } } @@ -2349,6 +2202,7 @@ class AuthRepositoryTest { ) verify(exactly = 1) { userStateManager.hasPendingAccountAddition = false + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = PASSWORD) settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1) } coVerify(exactly = 0) { @@ -2461,6 +2315,7 @@ class AuthRepositoryTest { verify(exactly = 1) { userStateManager.hasPendingAccountAddition userStateManager.hasPendingAccountAddition = true + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = PASSWORD) settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1) } } @@ -2596,6 +2451,7 @@ class AuthRepositoryTest { twoFactorToken = "twoFactorTokenToStore", ) verify(exactly = 1) { + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = PASSWORD) userStateManager.hasPendingAccountAddition = false } } @@ -2780,6 +2636,7 @@ class AuthRepositoryTest { ) verify(exactly = 1) { userStateManager.hasPendingAccountAddition = false + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = PASSWORD) settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1) settingsRepository.storeUserHasLoggedInValue(userId = USER_ID_1) } @@ -6485,54 +6342,6 @@ class AuthRepositoryTest { assertEquals(BreachCountResult.Success(breachCount), result) } - @Test - fun `getPasswordStrength returns expected results for various strength levels`() = runTest { - coEvery { - authSdkSource.passwordStrength(any(), eq("level_0")) - } returns LEVEL_0.asSuccess() - - coEvery { - authSdkSource.passwordStrength(any(), eq("level_1")) - } returns LEVEL_1.asSuccess() - - coEvery { - authSdkSource.passwordStrength(any(), eq("level_2")) - } returns LEVEL_2.asSuccess() - - coEvery { - authSdkSource.passwordStrength(any(), eq("level_3")) - } returns LEVEL_3.asSuccess() - - coEvery { - authSdkSource.passwordStrength(any(), eq("level_4")) - } returns LEVEL_4.asSuccess() - - assertEquals( - PasswordStrengthResult.Success(LEVEL_0), - repository.getPasswordStrength(EMAIL, "level_0"), - ) - - assertEquals( - PasswordStrengthResult.Success(LEVEL_1), - repository.getPasswordStrength(EMAIL, "level_1"), - ) - - assertEquals( - PasswordStrengthResult.Success(LEVEL_2), - repository.getPasswordStrength(EMAIL, "level_2"), - ) - - assertEquals( - PasswordStrengthResult.Success(LEVEL_3), - repository.getPasswordStrength(EMAIL, "level_3"), - ) - - assertEquals( - PasswordStrengthResult.Success(LEVEL_4), - repository.getPasswordStrength(EMAIL, "level_4"), - ) - } - @Test fun `validatePassword with no current user returns ValidatePasswordResult Error`() = runTest { val password = "password" @@ -6853,66 +6662,6 @@ class AuthRepositoryTest { ) } - @Test - fun `validatePasswordAgainstPolicy validates password against policy requirements`() = runTest { - fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 - - // A helper method to set a policy with the given parameters. - fun setPolicy( - minLength: Int = 0, - minComplexity: Int? = null, - requireUpper: Boolean = false, - requireLower: Boolean = false, - requireNumbers: Boolean = false, - requireSpecial: Boolean = false, - ) { - every { - policyManager.getActivePolicies(type = PolicyType.MASTER_PASSWORD) - } returns listOf( - createMockPolicyView( - type = PolicyType.MASTER_PASSWORD, - enabled = true, - data = """ - { - "minLength":$minLength, - "minComplexity":$minComplexity, - "requireUpper":$requireUpper, - "requireLower":$requireLower, - "requireNumbers":$requireNumbers, - "requireSpecial":$requireSpecial, - "enforceOnLogin":true - } - """, - ), - ) - } - - setPolicy(minLength = 10) - assertFalse(repository.validatePasswordAgainstPolicies(password = "123")) - - val password = "simple" - coEvery { - authSdkSource.passwordStrength( - email = SINGLE_USER_STATE_1.activeAccount.profile.email, - password = password, - ) - } returns LEVEL_0.asSuccess() - setPolicy(minComplexity = 10) - assertFalse(repository.validatePasswordAgainstPolicies(password = password)) - - setPolicy(requireUpper = true) - assertFalse(repository.validatePasswordAgainstPolicies(password = "lower")) - - setPolicy(requireLower = true) - assertFalse(repository.validatePasswordAgainstPolicies(password = "UPPER")) - - setPolicy(requireNumbers = true) - assertFalse(repository.validatePasswordAgainstPolicies(password = "letters")) - - setPolicy(requireSpecial = true) - assertFalse(repository.validatePasswordAgainstPolicies(password = "letters")) - } - @Test fun `sendVerificationEmail success should return success`() = runTest { coEvery { @@ -7198,6 +6947,7 @@ class AuthRepositoryTest { assertNull(fakeAuthDiskSource.getOnboardingStatus(USER_ID_1)) verify(exactly = 1) { userStateManager.hasPendingAccountAddition = false + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = PASSWORD) } } @@ -7254,6 +7004,7 @@ class AuthRepositoryTest { ) verify(exactly = 1) { userStateManager.hasPendingAccountAddition = false + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = PASSWORD) settingsRepository.setDefaultsIfNecessary(userId = USER_ID_1) } assertNull(fakeAuthDiskSource.getOnboardingStatus(USER_ID_1)) diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/policy/PasswordPolicyManagerTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/policy/PasswordPolicyManagerTest.kt new file mode 100644 index 00000000000..ea134c22dbd --- /dev/null +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/policy/PasswordPolicyManagerTest.kt @@ -0,0 +1,419 @@ +package com.x8bit.bitwarden.data.platform.manager.policy + +import com.bitwarden.core.data.manager.dispatcher.FakeDispatcherManager +import com.bitwarden.core.data.manager.encryption.EncryptionManager +import com.bitwarden.core.data.repository.util.bufferedMutableSharedFlow +import com.bitwarden.core.data.util.asFailure +import com.bitwarden.core.data.util.asSuccess +import com.bitwarden.data.datasource.disk.model.EnvironmentUrlDataJson +import com.bitwarden.network.model.KdfTypeJson +import com.bitwarden.network.model.MasterPasswordUnlockDataJson +import com.bitwarden.network.model.UserDecryptionOptionsJson +import com.bitwarden.policies.PolicyType +import com.bitwarden.policies.PolicyView +import com.x8bit.bitwarden.data.auth.datasource.disk.model.AccountJson +import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetReason +import com.x8bit.bitwarden.data.auth.datasource.disk.model.UserStateJson +import com.x8bit.bitwarden.data.auth.datasource.disk.util.FakeAuthDiskSource +import com.x8bit.bitwarden.data.auth.datasource.sdk.AuthSdkSource +import com.x8bit.bitwarden.data.auth.datasource.sdk.model.PasswordStrength +import com.x8bit.bitwarden.data.auth.datasource.sdk.util.toKdfRequestModel +import com.x8bit.bitwarden.data.auth.repository.model.PasswordStrengthResult +import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams +import com.x8bit.bitwarden.data.platform.manager.PolicyManager +import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockPolicyView +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Test +import java.time.Instant + +class PasswordPolicyManagerTest { + + private val fakeAuthDiskSource = FakeAuthDiskSource() + private val authSdkSource: AuthSdkSource = mockk() + private val mutableActivePolicyFlow = bufferedMutableSharedFlow>() + private val policyManager: PolicyManager = mockk { + every { + getActivePoliciesFlow(type = PolicyType.MASTER_PASSWORD) + } returns mutableActivePolicyFlow + } + private val encryptionManager: EncryptionManager = mockk { + every { + encrypt(alias = "PasswordPolicyManager", bytes = any()) + } returns DEFAULT_ENCRYPTED_BYTE_ARRAY.asSuccess() + every { + decrypt(alias = "PasswordPolicyManager", bytes = any()) + } returns DEFAULT_DECRYPTED_BYTE_ARRAY.asSuccess() + } + + private val passwordPolicyManager: PasswordPolicyManager = PasswordPolicyManagerImpl( + authDiskSource = fakeAuthDiskSource, + authSdkSource = authSdkSource, + policyManager = policyManager, + encryptionManager = encryptionManager, + dispatcherManager = FakeDispatcherManager(), + ) + + @Test + fun `validatePasswordAgainstPolicy validates password against policy requirements`() = runTest { + fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 + + // A helper method to set a policy with the given parameters. + fun setPolicy( + minLength: Int = 0, + minComplexity: Int? = null, + requireUpper: Boolean = false, + requireLower: Boolean = false, + requireNumbers: Boolean = false, + requireSpecial: Boolean = false, + ) { + every { + policyManager.getActivePolicies(type = PolicyType.MASTER_PASSWORD) + } returns listOf( + createMockPolicyView( + type = PolicyType.MASTER_PASSWORD, + enabled = true, + data = """ + { + "minLength":$minLength, + "minComplexity":$minComplexity, + "requireUpper":$requireUpper, + "requireLower":$requireLower, + "requireNumbers":$requireNumbers, + "requireSpecial":$requireSpecial, + "enforceOnLogin":true + } + """, + ), + ) + } + + setPolicy(minLength = 10) + assertFalse(passwordPolicyManager.validatePasswordAgainstPolicies(password = "123")) + + val password = "simple" + coEvery { + authSdkSource.passwordStrength( + email = SINGLE_USER_STATE_1.activeAccount.profile.email, + password = password, + ) + } returns PasswordStrength.LEVEL_0.asSuccess() + setPolicy(minComplexity = 10) + assertFalse(passwordPolicyManager.validatePasswordAgainstPolicies(password = password)) + + setPolicy(requireUpper = true) + assertFalse(passwordPolicyManager.validatePasswordAgainstPolicies(password = "lower")) + + setPolicy(requireLower = true) + assertFalse(passwordPolicyManager.validatePasswordAgainstPolicies(password = "UPPER")) + + setPolicy(requireNumbers = true) + assertFalse(passwordPolicyManager.validatePasswordAgainstPolicies(password = "letters")) + + setPolicy(requireSpecial = true) + assertFalse(passwordPolicyManager.validatePasswordAgainstPolicies(password = "letters")) + } + + @Test + fun `passwordResetReason should pull from the user's profile in AuthDiskSource`() = runTest { + val updatedProfile = PROFILE_1.copy( + forcePasswordResetReason = ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN, + ) + fakeAuthDiskSource.userState = UserStateJson( + activeUserId = USER_ID_1, + accounts = mapOf( + USER_ID_1 to ACCOUNT_1.copy( + profile = updatedProfile, + ), + ), + ) + assertEquals( + ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN, + passwordPolicyManager.passwordResetReason, + ) + } + + @Test + fun `getPasswordStrength returns expected results for various strength levels`() = runTest { + coEvery { + authSdkSource.passwordStrength(email = any(), password = eq("level_0")) + } returns PasswordStrength.LEVEL_0.asSuccess() + + coEvery { + authSdkSource.passwordStrength(email = any(), password = eq("level_1")) + } returns PasswordStrength.LEVEL_1.asSuccess() + + coEvery { + authSdkSource.passwordStrength(email = any(), password = eq("level_2")) + } returns PasswordStrength.LEVEL_2.asSuccess() + + coEvery { + authSdkSource.passwordStrength(email = any(), password = eq("level_3")) + } returns PasswordStrength.LEVEL_3.asSuccess() + + coEvery { + authSdkSource.passwordStrength(email = any(), password = eq("level_4")) + } returns PasswordStrength.LEVEL_4.asSuccess() + + assertEquals( + PasswordStrengthResult.Success(PasswordStrength.LEVEL_0), + passwordPolicyManager.getPasswordStrength(email = EMAIL, password = "level_0"), + ) + + assertEquals( + PasswordStrengthResult.Success(PasswordStrength.LEVEL_1), + passwordPolicyManager.getPasswordStrength(email = EMAIL, password = "level_1"), + ) + + assertEquals( + PasswordStrengthResult.Success(PasswordStrength.LEVEL_2), + passwordPolicyManager.getPasswordStrength(email = EMAIL, password = "level_2"), + ) + + assertEquals( + PasswordStrengthResult.Success(PasswordStrength.LEVEL_3), + passwordPolicyManager.getPasswordStrength(email = EMAIL, password = "level_3"), + ) + + assertEquals( + PasswordStrengthResult.Success(PasswordStrength.LEVEL_4), + passwordPolicyManager.getPasswordStrength(email = EMAIL, password = "level_4"), + ) + } + + @Test + fun `storePasswordToCheck encrypts the password using the EncryptionManager`() = runTest { + val password = "somePassword" + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = password) + + verify(exactly = 1) { + encryptionManager.encrypt( + alias = "PasswordPolicyManager", + bytes = match { it.contentEquals(password.encodeToByteArray()) }, + ) + } + } + + @Test + fun `storePasswordToCheck does not store the password when encryption fails`() = runTest { + fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 + val password = "somePassword" + every { + encryptionManager.encrypt( + alias = "PasswordPolicyManager", + bytes = password.encodeToByteArray(), + ) + } returns Throwable("Fail").asFailure() + + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = password) + mutableActivePolicyFlow.tryEmit(listOf(createMasterPasswordPolicyView(minLength = 10))) + + // With nothing stored, there is no encrypted password to decrypt. + verify(exactly = 0) { + encryptionManager.decrypt(alias = "PasswordPolicyManager", bytes = any()) + } + assertNull(fakeAuthDiskSource.userState?.activeAccount?.profile?.forcePasswordResetReason) + } + + @Test + fun `policy emission decrypts the stored password and sets reset reason when it fails`() = + runTest { + fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 + val password = "123" + every { + encryptionManager.decrypt(alias = "PasswordPolicyManager", bytes = any()) + } returns password.encodeToByteArray().asSuccess() + + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = password) + mutableActivePolicyFlow.tryEmit(listOf(createMasterPasswordPolicyView(minLength = 10))) + + verify(exactly = 1) { + encryptionManager.decrypt( + alias = "PasswordPolicyManager", + bytes = match { it.contentEquals(DEFAULT_ENCRYPTED_BYTE_ARRAY) }, + ) + } + assertEquals( + ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN, + fakeAuthDiskSource.userState?.activeAccount?.profile?.forcePasswordResetReason, + ) + } + + @Test + fun `policy emission decrypts the stored password and clears reset reason when it passes`() = + runTest { + fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 + val password = "aValidPassword" + every { + encryptionManager.decrypt(alias = "PasswordPolicyManager", bytes = any()) + } returns password.encodeToByteArray().asSuccess() + + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = password) + mutableActivePolicyFlow.tryEmit(listOf(createMasterPasswordPolicyView(minLength = 5))) + + verify(exactly = 1) { + encryptionManager.decrypt( + alias = "PasswordPolicyManager", + bytes = match { it.contentEquals(DEFAULT_ENCRYPTED_BYTE_ARRAY) }, + ) + } + assertNull( + fakeAuthDiskSource.userState?.activeAccount?.profile?.forcePasswordResetReason, + ) + } + + @Test + fun `policy emission does not set a reset reason when decryption fails`() = runTest { + fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 + every { + encryptionManager.decrypt(alias = "PasswordPolicyManager", bytes = any()) + } returns Throwable("Fail").asFailure() + + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = "123") + mutableActivePolicyFlow.tryEmit(listOf(createMasterPasswordPolicyView(minLength = 10))) + + verify(exactly = 1) { + encryptionManager.decrypt( + alias = "PasswordPolicyManager", + bytes = match { it.contentEquals(DEFAULT_ENCRYPTED_BYTE_ARRAY) }, + ) + } + assertNull(fakeAuthDiskSource.userState?.activeAccount?.profile?.forcePasswordResetReason) + } + + @Test + fun `policy emission does not decrypt when a reset reason is already set`() = runTest { + fakeAuthDiskSource.userState = SINGLE_USER_STATE_1.copy( + accounts = mapOf( + USER_ID_1 to ACCOUNT_1.copy( + profile = PROFILE_1.copy( + forcePasswordResetReason = + ForcePasswordResetReason.WEAK_MASTER_PASSWORD_ON_LOGIN, + ), + ), + ), + ) + + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = "123") + mutableActivePolicyFlow.tryEmit(listOf(createMasterPasswordPolicyView(minLength = 10))) + + // The user is already being forced to reset, so no re-check is performed. + verify(exactly = 0) { + encryptionManager.decrypt(alias = "PasswordPolicyManager", bytes = any()) + } + } + + @Test + fun `removePasswordToCheck prevents the stored password from being decrypted`() = runTest { + fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 + + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = "123") + passwordPolicyManager.removePasswordToCheck(userId = USER_ID_1) + mutableActivePolicyFlow.tryEmit(listOf(createMasterPasswordPolicyView(minLength = 10))) + + verify(exactly = 0) { + encryptionManager.decrypt(alias = "PasswordPolicyManager", bytes = any()) + } + assertNull(fakeAuthDiskSource.userState?.activeAccount?.profile?.forcePasswordResetReason) + } + + @Test + fun `switching users removes the stored password before it can be decrypted`() = runTest { + fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 + passwordPolicyManager.storePasswordToCheck(userId = USER_ID_1, password = "123") + + // Switch away to a different active user and then back to the original user. + fakeAuthDiskSource.userState = UserStateJson( + activeUserId = USER_ID_2, + accounts = mapOf(USER_ID_2 to ACCOUNT_1), + ) + fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 + + mutableActivePolicyFlow.tryEmit(listOf(createMasterPasswordPolicyView(minLength = 10))) + + // The password was purged when the previous active user changed away. + verify(exactly = 0) { + encryptionManager.decrypt(alias = "PasswordPolicyManager", bytes = any()) + } + } +} + +private const val EMAIL = "test@bitwarden.com" +private const val USER_ID_1 = "2a135b23-e1fb-42c9-bec3-573857bc8181" +private const val USER_ID_2 = "b9d09b0a-4c3f-4b3d-9c2f-1b2d3e4f5a6b" +private const val ENCRYPTED_USER_KEY = "encryptedUserKey" + +/** + * Creates a [PolicyView] with a master password policy enforcing the given [minLength] on login. + */ +private fun createMasterPasswordPolicyView(minLength: Int): PolicyView = + createMockPolicyView( + type = PolicyType.MASTER_PASSWORD, + enabled = true, + data = """ + { + "minLength":$minLength, + "minComplexity":null, + "requireUpper":false, + "requireLower":false, + "requireNumbers":false, + "requireSpecial":false, + "enforceOnLogin":true + } + """, + ) + +private val BASE_PROFILE_1 = AccountJson.Profile( + userId = USER_ID_1, + email = EMAIL, + isEmailVerified = true, + name = "Bitwarden Tester", + hasPremiumPersonally = false, + hasPremiumFromOrganization = null, + stamp = null, + organizationId = null, + avatarColorHex = null, + forcePasswordResetReason = null, + kdfType = KdfTypeJson.ARGON2_ID, + kdfIterations = 600000, + kdfMemory = 16, + kdfParallelism = 4, + userDecryptionOptions = null, + isTwoFactorEnabled = false, + creationDate = Instant.parse("2024-09-13T01:00:00.00Z"), +) + +private val PROFILE_1 = BASE_PROFILE_1.copy( + userDecryptionOptions = UserDecryptionOptionsJson( + hasMasterPassword = true, + trustedDeviceUserDecryptionOptions = null, + keyConnectorUserDecryptionOptions = null, + masterPasswordUnlock = MasterPasswordUnlockDataJson( + kdf = BASE_PROFILE_1.toSdkParams().toKdfRequestModel(), + masterKeyWrappedUserKey = ENCRYPTED_USER_KEY, + salt = EMAIL, + ), + ), +) +private val ACCOUNT_1 = AccountJson( + profile = PROFILE_1, + settings = AccountJson.Settings( + environmentUrlData = EnvironmentUrlDataJson.DEFAULT_US, + ), +) + +private val SINGLE_USER_STATE_1 = UserStateJson( + activeUserId = USER_ID_1, + accounts = mapOf( + USER_ID_1 to ACCOUNT_1, + ), +) + +private val DEFAULT_ENCRYPTED_BYTE_ARRAY: ByteArray = ByteArray(4) { (0x80 + it).toByte() } +private val DEFAULT_DECRYPTED_BYTE_ARRAY: ByteArray = ByteArray(4) { (0xFF + it).toByte() } diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerTest.kt index bc3ef27d7b6..98a04cb5987 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerTest.kt @@ -31,6 +31,7 @@ import com.x8bit.bitwarden.data.auth.manager.model.LogoutEvent import com.x8bit.bitwarden.data.auth.repository.model.LogoutReason import com.x8bit.bitwarden.data.auth.repository.model.UpdateKdfMinimumsResult import com.x8bit.bitwarden.data.auth.repository.util.toSdkParams +import com.x8bit.bitwarden.data.platform.manager.policy.PasswordPolicyManager import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.platform.repository.model.VaultTimeout import com.x8bit.bitwarden.data.platform.repository.model.VaultTimeoutAction @@ -118,6 +119,10 @@ class VaultLockManagerTest { private val pinProtectedUserKeyManager: PinProtectedUserKeyManager = mockk { coEvery { migratePinProtectedUserKeyIfNeeded(userId = any()) } just runs } + private val passwordPolicyManager: PasswordPolicyManager = mockk { + every { storePasswordToCheck(userId = any(), password = any()) } just runs + every { removePasswordToCheck(userId = any()) } just runs + } private val vaultLockManager: VaultLockManager = VaultLockManagerImpl( context = context, @@ -133,6 +138,7 @@ class VaultLockManagerTest { dispatcherManager = fakeDispatcherManager, kdfManager = kdfManager, pinProtectedUserKeyManager = pinProtectedUserKeyManager, + passwordPolicyManager = passwordPolicyManager, ) @Test @@ -262,6 +268,7 @@ class VaultLockManagerTest { // Will be used within each loop to reset the test to a suitable initial state. fun resetTest(vaultTimeout: VaultTimeout) { clearVerifications(userLogoutManager) + clearVerifications(passwordPolicyManager) mutableVaultTimeoutStateFlow.value = vaultTimeout fakeAppStateManager.appForegroundState = AppForegroundState.FOREGROUNDED verifyUnlockedVaultBlocking(userId = USER_ID) @@ -435,6 +442,7 @@ class VaultLockManagerTest { // Will be used within each loop to reset the test to a suitable initial state. fun resetTest(vaultTimeout: VaultTimeout) { + clearVerifications(passwordPolicyManager) mutableVaultTimeoutStateFlow.value = vaultTimeout fakeAppStateManager.appCreationState = AppCreationState.Destroyed clearVerifications(userLogoutManager) @@ -517,6 +525,7 @@ class VaultLockManagerTest { // Will be used within each loop to reset the test to a suitable initial state. fun resetTest(vaultTimeout: VaultTimeout) { + clearVerifications(passwordPolicyManager) mutableVaultTimeoutStateFlow.value = vaultTimeout fakeAppStateManager.appCreationState = AppCreationState.Destroyed clearVerifications(userLogoutManager) @@ -570,6 +579,7 @@ class VaultLockManagerTest { // Will be used within each loop to reset the test to a suitable initial state. fun resetTest(vaultTimeout: VaultTimeout) { clearVerifications(userLogoutManager) + clearVerifications(passwordPolicyManager) mutableVaultTimeoutStateFlow.value = vaultTimeout verifyUnlockedVaultBlocking(userId = USER_ID) verifyUnlockedVaultBlocking(userId = userId2) @@ -876,7 +886,10 @@ class VaultLockManagerTest { emptyList(), vaultLockManager.vaultUnlockDataStateFlow.value, ) - verify { vaultSdkSource.clearCrypto(userId = USER_ID) } + verify(exactly = 1) { + passwordPolicyManager.removePasswordToCheck(userId = USER_ID) + vaultSdkSource.clearCrypto(userId = USER_ID) + } } @Suppress("MaxLineLength") @@ -998,6 +1011,10 @@ class VaultLockManagerTest { userId = USER_ID, request = InitOrgCryptoRequest(organizationKeys = organizationKeys), ) + passwordPolicyManager.storePasswordToCheck( + userId = USER_ID, + password = masterPassword, + ) trustedDeviceManager.trustThisDeviceIfNecessary(userId = USER_ID) kdfManager.updateKdfToMinimumsIfNeeded(masterPassword) } @@ -1628,6 +1645,7 @@ class VaultLockManagerTest { // Confirm the vault is still locked assertFalse(vaultLockManager.isVaultUnlocked(userId = USER_ID)) coVerify(exactly = 1) { + passwordPolicyManager.removePasswordToCheck(userId = USER_ID) vaultSdkSource.getUserEncryptionKey(userId = USER_ID) } } @@ -1971,6 +1989,9 @@ class VaultLockManagerTest { ), ) } + verify(exactly = 1) { + passwordPolicyManager.storePasswordToCheck(userId = USER_ID, password = masterPassword) + } } private fun verifyUnlockedVaultBlocking(userId: String) { diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/repository/util/InitUserCryptoMethodExtensionsTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/repository/util/InitUserCryptoMethodExtensionsTest.kt new file mode 100644 index 00000000000..7353c7fdf9d --- /dev/null +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/repository/util/InitUserCryptoMethodExtensionsTest.kt @@ -0,0 +1,88 @@ +package com.x8bit.bitwarden.data.vault.repository.util + +import com.bitwarden.core.AuthRequestMethod +import com.bitwarden.core.InitUserCryptoMethod +import com.bitwarden.core.MasterPasswordUnlockData +import com.bitwarden.crypto.Kdf +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Test + +class InitUserCryptoMethodExtensionsTest { + @Test + fun `password returns the password for MasterPasswordUnlock`() { + assertEquals( + MASTER_PASSWORD, + MASTER_PASSWORD_UNLOCK_METHOD.password, + ) + } + + @Test + fun `password returns null for all non-MasterPasswordUnlock methods`() { + LOG_TAG_MAP + .keys + .filterNot { it is InitUserCryptoMethod.MasterPasswordUnlock } + .forEach { method -> assertNull(method.password) } + } + + @Test + fun `logTag returns the correct label for each method`() { + LOG_TAG_MAP.forEach { (method, expectedLogTag) -> + assertEquals(expectedLogTag, method.logTag) + } + } +} + +private const val MASTER_PASSWORD: String = "mockMasterPassword" + +/** + * Must be declared as the [InitUserCryptoMethod] supertype. Typed as the concrete + * `MasterPasswordUnlock` subclass, `password` would resolve to that data class's own member + * property instead of the extension under test. + */ +private val MASTER_PASSWORD_UNLOCK_METHOD: InitUserCryptoMethod = + InitUserCryptoMethod.MasterPasswordUnlock( + password = MASTER_PASSWORD, + masterPasswordUnlock = MasterPasswordUnlockData( + kdf = Kdf.Pbkdf2(iterations = 1u), + masterKeyWrappedUserKey = "mockMasterKeyWrappedUserKey", + salt = "mockSalt", + ), + ) + +/** + * Every [InitUserCryptoMethod] subclass mapped to its expected log tag. Adding a subclass to the + * SDK breaks the `when` blocks in the production code; this map must be updated alongside them. + */ +private val LOG_TAG_MAP: Map = mapOf( + MASTER_PASSWORD_UNLOCK_METHOD to "Master Password Unlock", + InitUserCryptoMethod.AuthRequest( + requestPrivateKey = "mockRequestPrivateKey", + method = AuthRequestMethod.UserKey(protectedUserKey = "mockProtectedUserKey"), + ) to "Auth Request", + InitUserCryptoMethod.DecryptedKey( + decryptedUserKey = "mockDecryptedUserKey", + ) to "Decrypted Key (Never Lock/Biometrics)", + InitUserCryptoMethod.DeviceKey( + deviceKey = "mockDeviceKey", + protectedDevicePrivateKey = "mockProtectedDevicePrivateKey", + deviceProtectedUserKey = "mockDeviceProtectedUserKey", + ) to "Device Key", + InitUserCryptoMethod.KeyConnector( + masterKey = "mockMasterKey", + userKey = "mockUserKey", + ) to "Key Connector", + InitUserCryptoMethod.KeyConnectorUrl( + url = "mockUrl", + keyConnectorKeyWrappedUserKey = "mockKeyConnectorKeyWrappedUserKey", + ) to "Key Connector Url", + InitUserCryptoMethod.Pin( + pin = "mockPin", + pinProtectedUserKey = "mockPinProtectedUserKey", + ) to "Pin", + InitUserCryptoMethod.PinEnvelope( + pin = "mockPin", + pinProtectedUserKeyEnvelope = "mockPinProtectedUserKeyEnvelope", + ) to "Pin Envelope", + InitUserCryptoMethod.PinState(pin = "mockPin") to "Pin State", +)