From 17c579bfc2fba755a97f5ed1f4d08ac7f20c13c6 Mon Sep 17 00:00:00 2001 From: Dave Severns <149429124+dseverns-livefront@users.noreply.github.com> Date: Thu, 29 Aug 2024 14:07:42 -0400 Subject: [PATCH] =?UTF-8?q?PM-11387=20on=20new=20create=20account=20with?= =?UTF-8?q?=20email=20verification,=20attempt=20login=E2=80=A6=20(#3842)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../CompleteRegistrationNavigation.kt | 2 +- .../CompleteRegistrationScreen.kt | 2 +- .../CompleteRegistrationViewModel.kt | 78 ++++++-- app/src/main/res/values/strings.xml | 1 + .../CompleteRegistrationViewModelTest.kt | 185 ++++++++++-------- 5 files changed, 171 insertions(+), 97 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationNavigation.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationNavigation.kt index ff8a22b3d3..24d07b8ddf 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationNavigation.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationNavigation.kt @@ -54,7 +54,7 @@ fun NavGraphBuilder.completeRegistrationDestination( onNavigateBack: () -> Unit, onNavigateToPasswordGuidance: () -> Unit, onNavigateToPreventAccountLockout: () -> Unit, - onNavigateToLogin: (email: String, token: String) -> Unit, + onNavigateToLogin: (email: String, token: String?) -> Unit, ) { composableWithSlideTransitions( route = COMPLETE_REGISTRATION_ROUTE, diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationScreen.kt index a824e5ace9..b281b7a7b3 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationScreen.kt @@ -73,7 +73,7 @@ fun CompleteRegistrationScreen( onNavigateBack: () -> Unit, onNavigateToPasswordGuidance: () -> Unit, onNavigateToPreventAccountLockout: () -> Unit, - onNavigateToLogin: (email: String, token: String) -> Unit, + onNavigateToLogin: (email: String, token: String?) -> Unit, viewModel: CompleteRegistrationViewModel = hiltViewModel(), ) { val state by viewModel.stateFlow.collectAsStateWithLifecycle() diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModel.kt index bd04ac3992..93c0ebc71d 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModel.kt @@ -6,6 +6,7 @@ import androidx.lifecycle.viewModelScope import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.auth.datasource.sdk.model.PasswordStrength import com.x8bit.bitwarden.data.auth.repository.AuthRepository +import com.x8bit.bitwarden.data.auth.repository.model.LoginResult import com.x8bit.bitwarden.data.auth.repository.model.PasswordStrengthResult import com.x8bit.bitwarden.data.auth.repository.model.RegisterResult import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager @@ -106,18 +107,14 @@ class CompleteRegistrationViewModel @Inject constructor( override fun handleAction(action: CompleteRegistrationAction) { when (action) { + is Internal -> handleInternalAction(action) is ConfirmPasswordInputChange -> handleConfirmPasswordInputChanged(action) is PasswordHintChange -> handlePasswordHintChanged(action) is PasswordInputChange -> handlePasswordInputChanged(action) is BackClick -> handleBackClicked() is ErrorDialogDismiss -> handleDialogDismiss() is CheckDataBreachesToggle -> handleCheckDataBreachesToggle(action) - is Internal.ReceiveRegisterResult -> { - handleReceiveRegisterAccountResult(action) - } - ContinueWithBreachedPasswordClick -> handleContinueWithBreachedPasswordClick() - is ReceivePasswordStrengthResult -> handlePasswordStrengthResult(action) CompleteRegistrationAction.LearnToPreventLockoutClick -> { handlePreventAccountLockoutClickAction() } @@ -127,10 +124,16 @@ class CompleteRegistrationViewModel @Inject constructor( } CompleteRegistrationAction.CallToActionClick -> handleCallToActionClick() + } + } + + private fun handleInternalAction(action: Internal) { + when (action) { + is Internal.GeneratedPasswordResult -> handleGeneratedPasswordResult(action) + is ReceivePasswordStrengthResult -> handlePasswordStrengthResult(action) + is Internal.ReceiveRegisterResult -> handleReceiveRegisterAccountResult(action) is Internal.UpdateOnboardingFeatureState -> handleUpdateOnboardingFeatureState(action) - is Internal.GeneratedPasswordResult -> handleGeneratedPasswordResult( - action, - ) + is Internal.ReceiveLoginResult -> handleLoginResult(action) } } @@ -215,13 +218,19 @@ class CompleteRegistrationViewModel @Inject constructor( } is RegisterResult.Success -> { - mutableStateFlow.update { it.copy(dialog = null) } - sendEvent( - CompleteRegistrationEvent.NavigateToLogin( + viewModelScope.launch { + val loginResult = authRepository.login( email = state.userEmail, + password = state.passwordInput, captchaToken = registerAccountResult.captchaToken, - ), - ) + ) + sendAction( + Internal.ReceiveLoginResult( + loginResult = loginResult, + captchaToken = registerAccountResult.captchaToken, + ), + ) + } } RegisterResult.DataBreachFound -> { @@ -259,6 +268,25 @@ class CompleteRegistrationViewModel @Inject constructor( } } + private fun handleLoginResult(action: Internal.ReceiveLoginResult) { + clearDialogState() + sendEvent( + CompleteRegistrationEvent.ShowToast( + message = R.string.account_created_success.asText(), + ), + ) + // If the login result is Success the state based navigation will take care of it. + // otherwise we need to navigate to the login screen. + if (action.loginResult !is LoginResult.Success) { + sendEvent( + CompleteRegistrationEvent.NavigateToLogin( + email = state.userEmail, + captchaToken = action.captchaToken, + ), + ) + } + } + private fun handleCheckDataBreachesToggle(action: CheckDataBreachesToggle) { mutableStateFlow.update { it.copy(isCheckDataBreachesToggled = action.newState) @@ -266,9 +294,7 @@ class CompleteRegistrationViewModel @Inject constructor( } private fun handleDialogDismiss() { - mutableStateFlow.update { - it.copy(dialog = null) - } + clearDialogState() } private fun handleBackClicked() { @@ -393,6 +419,12 @@ class CompleteRegistrationViewModel @Inject constructor( } } } + + private fun clearDialogState() { + mutableStateFlow.update { + it.copy(dialog = null) + } + } } /** @@ -509,7 +541,7 @@ sealed class CompleteRegistrationEvent { */ data class NavigateToLogin( val email: String, - val captchaToken: String, + val captchaToken: String?, ) : CompleteRegistrationEvent() } @@ -595,5 +627,17 @@ sealed class CompleteRegistrationAction { * Indicates a generated password has been received. */ data class GeneratedPasswordResult(val generatedPassword: String) : Internal() + + /** + * Indicates registration was successful and will now attempt to login and unlock the vault. + * @property captchaToken The captcha token to use for login. With the login function this + * is possible to be negative. + * + * @see [AuthRepository.login] + */ + data class ReceiveLoginResult( + val loginResult: LoginResult, + val captchaToken: String?, + ) : Internal() } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 36a1ec9a6b..bee1042bfa 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -82,6 +82,7 @@ Yes Account Your new account has been created! You may now log in. + Your new account has been created! Add an Item App extension Use the Bitwarden accessibility service to auto-fill your logins across apps and the web. diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModelTest.kt index b35cc707de..708ecf4b36 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/completeregistration/CompleteRegistrationViewModelTest.kt @@ -10,6 +10,7 @@ import com.x8bit.bitwarden.data.auth.datasource.sdk.model.PasswordStrength.LEVEL 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.repository.AuthRepository +import com.x8bit.bitwarden.data.auth.repository.model.LoginResult import com.x8bit.bitwarden.data.auth.repository.model.PasswordStrengthResult import com.x8bit.bitwarden.data.auth.repository.model.RegisterResult import com.x8bit.bitwarden.data.auth.repository.model.UserState @@ -58,6 +59,25 @@ class CompleteRegistrationViewModelTest : BaseViewModelTest() { private val mutableUserStateFlow = MutableStateFlow(null) private val mockAuthRepository = mockk() { every { userStateFlow } returns mutableUserStateFlow + coEvery { + login( + email = any(), + password = any(), + captchaToken = any(), + ) + } returns LoginResult.Success + + coEvery { + register( + email = any(), + masterPassword = any(), + masterPasswordHint = any(), + emailVerificationToken = any(), + captchaToken = any(), + shouldCheckDataBreaches = any(), + isMasterPasswordStrong = any(), + ) + } returns RegisterResult.Success(captchaToken = CAPTCHA_BYPASS_TOKEN) } private val fakeEnvironmentRepository = FakeEnvironmentRepository() @@ -119,10 +139,10 @@ class CompleteRegistrationViewModelTest : BaseViewModelTest() { } @Test - fun `CallToActionClick with all inputs valid should show and hide loading dialog`() = runTest { - val repo = mockk { + fun `CallToActionClick with all inputs valid should account created toast and hide dialog`() = + runTest { coEvery { - register( + mockAuthRepository.register( email = EMAIL, masterPassword = PASSWORD, masterPasswordHint = null, @@ -132,45 +152,39 @@ class CompleteRegistrationViewModelTest : BaseViewModelTest() { isMasterPasswordStrong = true, ) } returns RegisterResult.Success(captchaToken = CAPTCHA_BYPASS_TOKEN) + val viewModel = createCompleteRegistrationViewModel(VALID_INPUT_STATE) + turbineScope { + val stateFlow = viewModel.stateFlow.testIn(backgroundScope) + val eventFlow = viewModel.eventFlow.testIn(backgroundScope) + assertEquals(VALID_INPUT_STATE, stateFlow.awaitItem()) + viewModel.trySendAction(CompleteRegistrationAction.CallToActionClick) + assertEquals( + VALID_INPUT_STATE.copy(dialog = CompleteRegistrationDialog.Loading), + stateFlow.awaitItem(), + ) + assertEquals( + CompleteRegistrationEvent.ShowToast(R.string.account_created_success.asText()), + eventFlow.awaitItem(), + ) + // Make sure loading dialog is hidden: + assertEquals(VALID_INPUT_STATE, stateFlow.awaitItem()) + } } - val viewModel = createCompleteRegistrationViewModel(VALID_INPUT_STATE, repo) - turbineScope { - val stateFlow = viewModel.stateFlow.testIn(backgroundScope) - val eventFlow = viewModel.eventFlow.testIn(backgroundScope) - assertEquals(VALID_INPUT_STATE, stateFlow.awaitItem()) - viewModel.trySendAction(CompleteRegistrationAction.CallToActionClick) - assertEquals( - VALID_INPUT_STATE.copy(dialog = CompleteRegistrationDialog.Loading), - stateFlow.awaitItem(), - ) - assertEquals( - CompleteRegistrationEvent.NavigateToLogin( - EMAIL, - CAPTCHA_BYPASS_TOKEN, - ), - eventFlow.awaitItem(), - ) - // Make sure loading dialog is hidden: - assertEquals(VALID_INPUT_STATE, stateFlow.awaitItem()) - } - } @Test fun `CallToActionClick register returns error should update errorDialogState`() = runTest { - val repo = mockk { - coEvery { - register( - email = EMAIL, - masterPassword = PASSWORD, - masterPasswordHint = null, - emailVerificationToken = TOKEN, - captchaToken = null, - shouldCheckDataBreaches = false, - isMasterPasswordStrong = true, - ) - } returns RegisterResult.Error(errorMessage = "mock_error") - } - val viewModel = createCompleteRegistrationViewModel(VALID_INPUT_STATE, repo) + coEvery { + mockAuthRepository.register( + email = EMAIL, + masterPassword = PASSWORD, + masterPasswordHint = null, + emailVerificationToken = TOKEN, + captchaToken = null, + shouldCheckDataBreaches = false, + isMasterPasswordStrong = true, + ) + } returns RegisterResult.Error(errorMessage = "mock_error") + val viewModel = createCompleteRegistrationViewModel(VALID_INPUT_STATE) viewModel.stateFlow.test { assertEquals(VALID_INPUT_STATE, awaitItem()) viewModel.trySendAction(CompleteRegistrationAction.CallToActionClick) @@ -193,52 +207,68 @@ class CompleteRegistrationViewModelTest : BaseViewModelTest() { } @Test - fun `CallToActionClick register returns Success should emit NavigateToLogin`() = runTest { - val repo = mockk { - coEvery { - register( - email = EMAIL, - masterPassword = PASSWORD, - masterPasswordHint = null, - emailVerificationToken = TOKEN, - captchaToken = null, - shouldCheckDataBreaches = false, - isMasterPasswordStrong = true, - ) - } returns RegisterResult.Success(captchaToken = CAPTCHA_BYPASS_TOKEN) - } - val viewModel = createCompleteRegistrationViewModel(VALID_INPUT_STATE, repo) - viewModel.eventFlow.test { - viewModel.trySendAction(CompleteRegistrationAction.CallToActionClick) - assertEquals( - CompleteRegistrationEvent.NavigateToLogin( - EMAIL, - CAPTCHA_BYPASS_TOKEN, - ), - awaitItem(), + fun `CallToActionClick register returns Success should attempt login`() = runTest { + val viewModel = createCompleteRegistrationViewModel(VALID_INPUT_STATE) + viewModel.trySendAction(CompleteRegistrationAction.CallToActionClick) + coVerify { + mockAuthRepository.login( + email = EMAIL, + password = PASSWORD, + captchaToken = CAPTCHA_BYPASS_TOKEN, ) } } @Test - fun `ContinueWithBreachedPasswordClick should call repository with checkDataBreaches false`() { - val repo = mockk { - coEvery { - register( - email = EMAIL, - masterPassword = PASSWORD, - masterPasswordHint = null, - emailVerificationToken = TOKEN, - captchaToken = null, - shouldCheckDataBreaches = false, - isMasterPasswordStrong = true, - ) - } returns RegisterResult.Error(null) + fun `when login attempt returns success should wait for state based navigation`() = runTest { + val viewModel = createCompleteRegistrationViewModel(VALID_INPUT_STATE) + viewModel.trySendAction(CompleteRegistrationAction.CallToActionClick) + viewModel.eventFlow.test { + assertTrue(awaitItem() is CompleteRegistrationEvent.ShowToast) + expectNoEvents() } - val viewModel = createCompleteRegistrationViewModel(VALID_INPUT_STATE, repo) + } + + @Suppress("MaxLineLength") + @Test + fun `when login attempt returns anything other than success should send navigate to login event`() = + runTest { + coEvery { + mockAuthRepository.login( + email = EMAIL, + password = PASSWORD, + captchaToken = CAPTCHA_BYPASS_TOKEN, + ) + } returns LoginResult.TwoFactorRequired + + val viewModel = createCompleteRegistrationViewModel(VALID_INPUT_STATE) + viewModel.trySendAction(CompleteRegistrationAction.CallToActionClick) + viewModel.eventFlow.test { + assertTrue(awaitItem() is CompleteRegistrationEvent.ShowToast) + assertEquals( + CompleteRegistrationEvent.NavigateToLogin(EMAIL, CAPTCHA_BYPASS_TOKEN), + awaitItem(), + ) + } + } + + @Test + fun `ContinueWithBreachedPasswordClick should call repository with checkDataBreaches false`() { + coEvery { + mockAuthRepository.register( + email = EMAIL, + masterPassword = PASSWORD, + masterPasswordHint = null, + emailVerificationToken = TOKEN, + captchaToken = null, + shouldCheckDataBreaches = false, + isMasterPasswordStrong = true, + ) + } returns RegisterResult.Error(null) + val viewModel = createCompleteRegistrationViewModel(VALID_INPUT_STATE) viewModel.trySendAction(CompleteRegistrationAction.ContinueWithBreachedPasswordClick) coVerify { - repo.register( + mockAuthRepository.register( email = EMAIL, masterPassword = PASSWORD, masterPasswordHint = null, @@ -632,14 +662,13 @@ class CompleteRegistrationViewModelTest : BaseViewModelTest() { private fun createCompleteRegistrationViewModel( completeRegistrationState: CompleteRegistrationState? = DEFAULT_STATE, - authRepository: AuthRepository = mockAuthRepository, ): CompleteRegistrationViewModel = CompleteRegistrationViewModel( savedStateHandle = SavedStateHandle( mapOf( "state" to completeRegistrationState, ), ), - authRepository = authRepository, + authRepository = mockAuthRepository, environmentRepository = fakeEnvironmentRepository, specialCircumstanceManager = specialCircumstanceManager, featureFlagManager = featureFlagManager,