PM-19233: Propagate auth request errors to the UI (#4868)

This commit is contained in:
David Perez
2025-03-14 13:33:39 -05:00
committed by GitHub
parent 6fe9eba620
commit 18ce45e7e5
12 changed files with 92 additions and 58 deletions

View File

@@ -16,6 +16,7 @@ import com.x8bit.bitwarden.data.auth.manager.model.AuthRequestsUpdatesResult
import com.x8bit.bitwarden.data.auth.manager.model.CreateAuthRequestResult import com.x8bit.bitwarden.data.auth.manager.model.CreateAuthRequestResult
import com.x8bit.bitwarden.data.auth.manager.util.isSso import com.x8bit.bitwarden.data.auth.manager.util.isSso
import com.x8bit.bitwarden.data.auth.manager.util.toAuthRequestTypeJson import com.x8bit.bitwarden.data.auth.manager.util.toAuthRequestTypeJson
import com.x8bit.bitwarden.data.platform.error.NoActiveUserException
import com.x8bit.bitwarden.data.platform.util.asFailure import com.x8bit.bitwarden.data.platform.util.asFailure
import com.x8bit.bitwarden.data.platform.util.asSuccess import com.x8bit.bitwarden.data.platform.util.asSuccess
import com.x8bit.bitwarden.data.platform.util.flatMap import com.x8bit.bitwarden.data.platform.util.flatMap
@@ -51,7 +52,9 @@ class AuthRequestManagerImpl(
override fun getAuthRequestsWithUpdates(): Flow<AuthRequestsUpdatesResult> = flow { override fun getAuthRequestsWithUpdates(): Flow<AuthRequestsUpdatesResult> = flow {
while (currentCoroutineContext().isActive) { while (currentCoroutineContext().isActive) {
when (val result = getAuthRequests()) { when (val result = getAuthRequests()) {
AuthRequestsResult.Error -> emit(AuthRequestsUpdatesResult.Error) is AuthRequestsResult.Error -> {
emit(AuthRequestsUpdatesResult.Error(error = result.error))
}
is AuthRequestsResult.Success -> { is AuthRequestsResult.Success -> {
emit(AuthRequestsUpdatesResult.Update(authRequests = result.authRequests)) emit(AuthRequestsUpdatesResult.Update(authRequests = result.authRequests))
@@ -181,7 +184,7 @@ class AuthRequestManagerImpl(
) )
} }
.fold( .fold(
onFailure = { emit(AuthRequestUpdatesResult.Error) }, onFailure = { emit(AuthRequestUpdatesResult.Error(error = it)) },
onSuccess = { updateAuthRequest -> onSuccess = { updateAuthRequest ->
when { when {
updateAuthRequest.requestApproved -> { updateAuthRequest.requestApproved -> {
@@ -217,13 +220,18 @@ class AuthRequestManagerImpl(
fingerprint: String, fingerprint: String,
): Flow<AuthRequestUpdatesResult> = getAuthRequest { ): Flow<AuthRequestUpdatesResult> = getAuthRequest {
when (val authRequestsResult = getAuthRequests()) { when (val authRequestsResult = getAuthRequests()) {
AuthRequestsResult.Error -> AuthRequestUpdatesResult.Error is AuthRequestsResult.Error -> {
AuthRequestUpdatesResult.Error(error = authRequestsResult.error)
}
is AuthRequestsResult.Success -> { is AuthRequestsResult.Success -> {
authRequestsResult authRequestsResult
.authRequests .authRequests
.firstOrNull { it.fingerprint == fingerprint } .firstOrNull { it.fingerprint == fingerprint }
?.let { AuthRequestUpdatesResult.Update(it) } ?.let { AuthRequestUpdatesResult.Update(it) }
?: AuthRequestUpdatesResult.Error ?: AuthRequestUpdatesResult.Error(
error = IllegalStateException("Could not find the auth request."),
)
} }
} }
} }
@@ -233,30 +241,28 @@ class AuthRequestManagerImpl(
): Flow<AuthRequestUpdatesResult> = getAuthRequest { ): Flow<AuthRequestUpdatesResult> = getAuthRequest {
authRequestsService authRequestsService
.getAuthRequest(requestId) .getAuthRequest(requestId)
.map { response -> .mapCatching { response ->
getFingerprintPhrase(response.publicKey).getOrNull()?.let { fingerprint -> getFingerprintPhrase(response.publicKey)
AuthRequest( .getOrThrow()
id = response.id, .let { fingerprint ->
publicKey = response.publicKey, AuthRequest(
platform = response.platform, id = response.id,
ipAddress = response.ipAddress, publicKey = response.publicKey,
key = response.key, platform = response.platform,
masterPasswordHash = response.masterPasswordHash, ipAddress = response.ipAddress,
creationDate = response.creationDate, key = response.key,
responseDate = response.responseDate, masterPasswordHash = response.masterPasswordHash,
requestApproved = response.requestApproved ?: false, creationDate = response.creationDate,
originUrl = response.originUrl, responseDate = response.responseDate,
fingerprint = fingerprint, requestApproved = response.requestApproved ?: false,
) originUrl = response.originUrl,
} fingerprint = fingerprint,
)
}
} }
.fold( .fold(
onFailure = { AuthRequestUpdatesResult.Error }, onFailure = { AuthRequestUpdatesResult.Error(error = it) },
onSuccess = { authRequest -> onSuccess = { AuthRequestUpdatesResult.Update(authRequest = it) },
authRequest
?.let { AuthRequestUpdatesResult.Update(it) }
?: AuthRequestUpdatesResult.Error
},
) )
} }
@@ -308,7 +314,7 @@ class AuthRequestManagerImpl(
} }
} }
.fold( .fold(
onFailure = { AuthRequestsResult.Error }, onFailure = { AuthRequestsResult.Error(error = it) },
onSuccess = { AuthRequestsResult.Success(authRequests = it) }, onSuccess = { AuthRequestsResult.Success(authRequests = it) },
) )
@@ -318,7 +324,7 @@ class AuthRequestManagerImpl(
publicKey: String, publicKey: String,
isApproved: Boolean, isApproved: Boolean,
): AuthRequestResult { ): AuthRequestResult {
val userId = activeUserId ?: return AuthRequestResult.Error val userId = activeUserId ?: return AuthRequestResult.Error(error = NoActiveUserException())
return vaultSdkSource return vaultSdkSource
.getAuthRequestKey( .getAuthRequestKey(
publicKey = publicKey, publicKey = publicKey,
@@ -349,7 +355,7 @@ class AuthRequestManagerImpl(
) )
} }
.fold( .fold(
onFailure = { AuthRequestResult.Error }, onFailure = { AuthRequestResult.Error(error = it) },
onSuccess = { AuthRequestResult.Success(authRequest = it) }, onSuccess = { AuthRequestResult.Success(authRequest = it) },
) )
} }
@@ -461,7 +467,7 @@ class AuthRequestManagerImpl(
publicKey: String, publicKey: String,
): Result<String> { ): Result<String> {
val profile = authDiskSource.userState?.activeAccount?.profile val profile = authDiskSource.userState?.activeAccount?.profile
?: return IllegalStateException("No active account").asFailure() ?: return NoActiveUserException().asFailure()
return authSdkSource.getUserFingerprint( return authSdkSource.getUserFingerprint(
email = profile.email, email = profile.email,
publicKey = publicKey, publicKey = publicKey,

View File

@@ -14,5 +14,7 @@ sealed class AuthRequestResult {
/** /**
* There was an error getting the user's auth requests. * There was an error getting the user's auth requests.
*/ */
data object Error : AuthRequestResult() data class Error(
val error: Throwable,
) : AuthRequestResult()
} }

View File

@@ -19,7 +19,9 @@ sealed class AuthRequestUpdatesResult {
/** /**
* There was an error getting the user's auth requests. * There was an error getting the user's auth requests.
*/ */
data object Error : AuthRequestUpdatesResult() data class Error(
val error: Throwable,
) : AuthRequestUpdatesResult()
/** /**
* The auth request has been declined. * The auth request has been declined.

View File

@@ -14,5 +14,7 @@ sealed class AuthRequestsResult {
/** /**
* There was an error getting the user's auth requests. * There was an error getting the user's auth requests.
*/ */
data object Error : AuthRequestsResult() data class Error(
val error: Throwable,
) : AuthRequestsResult()
} }

View File

@@ -14,5 +14,7 @@ sealed class AuthRequestsUpdatesResult {
/** /**
* There was an error getting the user's auth requests. * There was an error getting the user's auth requests.
*/ */
data object Error : AuthRequestsUpdatesResult() data class Error(
val error: Throwable,
) : AuthRequestsUpdatesResult()
} }

View File

@@ -0,0 +1,6 @@
package com.x8bit.bitwarden.data.platform.error
/**
* An exception indicating that there is currently no active user when one is required.
*/
class NoActiveUserException : IllegalStateException("No current active user!")

View File

@@ -296,6 +296,7 @@ private fun LoginApprovalDialogs(
is LoginApprovalState.DialogState.Error -> BitwardenBasicDialog( is LoginApprovalState.DialogState.Error -> BitwardenBasicDialog(
title = state.title?.invoke(), title = state.title?.invoke(),
message = state.message(), message = state.message(),
throwable = state.error,
onDismissRequest = onDismissError, onDismissRequest = onDismissError,
) )

View File

@@ -177,7 +177,7 @@ class LoginApprovalViewModel @Inject constructor(
private fun handleApproveRequestResultReceived( private fun handleApproveRequestResultReceived(
action: LoginApprovalAction.Internal.ApproveRequestResultReceive, action: LoginApprovalAction.Internal.ApproveRequestResultReceive,
) { ) {
when (action.result) { when (val result = action.result) {
is AuthRequestResult.Success -> { is AuthRequestResult.Success -> {
sendEvent(LoginApprovalEvent.ShowToast(R.string.login_approved.asText())) sendEvent(LoginApprovalEvent.ShowToast(R.string.login_approved.asText()))
sendClosingEvent() sendClosingEvent()
@@ -189,6 +189,7 @@ class LoginApprovalViewModel @Inject constructor(
dialogState = LoginApprovalState.DialogState.Error( dialogState = LoginApprovalState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(), title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(), message = R.string.generic_error_message.asText(),
error = result.error,
), ),
) )
} }
@@ -239,7 +240,7 @@ class LoginApprovalViewModel @Inject constructor(
private fun handleDeclineRequestResultReceived( private fun handleDeclineRequestResultReceived(
action: LoginApprovalAction.Internal.DeclineRequestResultReceive, action: LoginApprovalAction.Internal.DeclineRequestResultReceive,
) { ) {
when (action.result) { when (val result = action.result) {
is AuthRequestResult.Success -> { is AuthRequestResult.Success -> {
sendEvent(LoginApprovalEvent.ShowToast(R.string.log_in_denied.asText())) sendEvent(LoginApprovalEvent.ShowToast(R.string.log_in_denied.asText()))
sendClosingEvent() sendClosingEvent()
@@ -251,6 +252,7 @@ class LoginApprovalViewModel @Inject constructor(
dialogState = LoginApprovalState.DialogState.Error( dialogState = LoginApprovalState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(), title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(), message = R.string.generic_error_message.asText(),
error = result.error,
), ),
) )
} }
@@ -327,6 +329,7 @@ data class LoginApprovalState(
data class Error( data class Error(
val title: Text?, val title: Text?,
val message: Text, val message: Text,
val error: Throwable? = null,
) : DialogState() ) : DialogState()
/** /**

View File

@@ -169,7 +169,7 @@ class PendingRequestsViewModel @Inject constructor(
} }
} }
AuthRequestsUpdatesResult.Error -> { is AuthRequestsUpdatesResult.Error -> {
mutableStateFlow.update { mutableStateFlow.update {
it.copy( it.copy(
authRequests = emptyList(), authRequests = emptyList(),

View File

@@ -427,12 +427,13 @@ class AuthRequestManagerTest {
fun `getAuthRequestByFingerprintFlow should emit failure and cancel flow when getAuthRequests fails`() = fun `getAuthRequestByFingerprintFlow should emit failure and cancel flow when getAuthRequests fails`() =
runTest { runTest {
val fingerprint = "fingerprint" val fingerprint = "fingerprint"
coEvery { authRequestsService.getAuthRequests() } returns Throwable("Fail").asFailure() val error = Throwable("Fail")
coEvery { authRequestsService.getAuthRequests() } returns error.asFailure()
repository repository
.getAuthRequestByFingerprintFlow(fingerprint) .getAuthRequestByFingerprintFlow(fingerprint)
.test { .test {
assertEquals(AuthRequestUpdatesResult.Error, awaitItem()) assertEquals(AuthRequestUpdatesResult.Error(error = error), awaitItem())
awaitComplete() awaitComplete()
} }
@@ -449,8 +450,9 @@ class AuthRequestManagerTest {
authRequests = listOf(AUTH_REQUESTS_RESPONSE_JSON_AUTH_RESPONSE), authRequests = listOf(AUTH_REQUESTS_RESPONSE_JSON_AUTH_RESPONSE),
) )
val authRequest = AUTH_REQUEST val authRequest = AUTH_REQUEST
val error = Throwable("Fail")
val expectedOne = AuthRequestUpdatesResult.Update(authRequest = authRequest) val expectedOne = AuthRequestUpdatesResult.Update(authRequest = authRequest)
val expectedTwo = AuthRequestUpdatesResult.Error val expectedTwo = AuthRequestUpdatesResult.Error(error = error)
coEvery { coEvery {
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY) authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
} returns FINGER_PRINT.asSuccess() } returns FINGER_PRINT.asSuccess()
@@ -459,7 +461,7 @@ class AuthRequestManagerTest {
} returns authRequestsResponseJson.asSuccess() } returns authRequestsResponseJson.asSuccess()
coEvery { coEvery {
authRequestsService.getAuthRequest(requestId = REQUEST_ID) authRequestsService.getAuthRequest(requestId = REQUEST_ID)
} returns Throwable("Fail").asFailure() } returns error.asFailure()
fakeAuthDiskSource.userState = SINGLE_USER_STATE fakeAuthDiskSource.userState = SINGLE_USER_STATE
repository repository
@@ -639,14 +641,13 @@ class AuthRequestManagerTest {
@Test @Test
fun `getAuthRequestByIdFlow should emit failure and cancel flow when getAuthRequests fails`() = fun `getAuthRequestByIdFlow should emit failure and cancel flow when getAuthRequests fails`() =
runTest { runTest {
coEvery { val error = Throwable("Fail")
authRequestsService.getAuthRequest(REQUEST_ID) coEvery { authRequestsService.getAuthRequest(REQUEST_ID) } returns error.asFailure()
} returns Throwable("Fail").asFailure()
repository repository
.getAuthRequestByIdFlow(REQUEST_ID) .getAuthRequestByIdFlow(REQUEST_ID)
.test { .test {
assertEquals(AuthRequestUpdatesResult.Error, awaitItem()) assertEquals(AuthRequestUpdatesResult.Error(error = error), awaitItem())
awaitComplete() awaitComplete()
} }
@@ -660,10 +661,11 @@ class AuthRequestManagerTest {
fun `getAuthRequestByIdFlow should emit update then not cancel on failure when initial request succeeds and second fails`() = fun `getAuthRequestByIdFlow should emit update then not cancel on failure when initial request succeeds and second fails`() =
runTest { runTest {
val authRequestResponseOne = AUTH_REQUESTS_RESPONSE_JSON_AUTH_RESPONSE.asSuccess() val authRequestResponseOne = AUTH_REQUESTS_RESPONSE_JSON_AUTH_RESPONSE.asSuccess()
val authRequestResponseTwo = Throwable("Fail").asFailure() val error = Throwable("Fail")
val authRequestResponseTwo = error.asFailure()
val authRequest = AUTH_REQUEST.copy(id = REQUEST_ID) val authRequest = AUTH_REQUEST.copy(id = REQUEST_ID)
val expectedOne = AuthRequestUpdatesResult.Update(authRequest = authRequest) val expectedOne = AuthRequestUpdatesResult.Update(authRequest = authRequest)
val expectedTwo = AuthRequestUpdatesResult.Error val expectedTwo = AuthRequestUpdatesResult.Error(error = error)
coEvery { coEvery {
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY) authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
} returns FINGER_PRINT.asSuccess() } returns FINGER_PRINT.asSuccess()
@@ -850,11 +852,12 @@ class AuthRequestManagerTest {
val authRequestsResponseJson = AuthRequestsResponseJson( val authRequestsResponseJson = AuthRequestsResponseJson(
authRequests = listOf(AUTH_REQUESTS_RESPONSE_JSON_AUTH_RESPONSE), authRequests = listOf(AUTH_REQUESTS_RESPONSE_JSON_AUTH_RESPONSE),
) )
val expectedOne = AuthRequestsUpdatesResult.Error val error = Throwable("Fail")
val expectedOne = AuthRequestsUpdatesResult.Error(error = error)
val expectedTwo = AuthRequestsUpdatesResult.Update(authRequests = authRequests) val expectedTwo = AuthRequestsUpdatesResult.Update(authRequests = authRequests)
coEvery { coEvery {
authRequestsService.getAuthRequests() authRequestsService.getAuthRequests()
} returns Throwable("Fail").asFailure() andThen authRequestsResponseJson.asSuccess() } returns error.asFailure() andThen authRequestsResponseJson.asSuccess()
coEvery { coEvery {
authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY) authSdkSource.getUserFingerprint(email = EMAIL, publicKey = PUBLIC_KEY)
} returns FINGER_PRINT.asSuccess() } returns FINGER_PRINT.asSuccess()
@@ -935,14 +938,15 @@ class AuthRequestManagerTest {
@Test @Test
fun `getAuthRequests should return failure when service returns failure`() = runTest { fun `getAuthRequests should return failure when service returns failure`() = runTest {
coEvery { authRequestsService.getAuthRequests() } returns Throwable("Fail").asFailure() val error = Throwable("Fail")
coEvery { authRequestsService.getAuthRequests() } returns error.asFailure()
val result = repository.getAuthRequests() val result = repository.getAuthRequests()
coVerify(exactly = 1) { coVerify(exactly = 1) {
authRequestsService.getAuthRequests() authRequestsService.getAuthRequests()
} }
assertEquals(AuthRequestsResult.Error, result) assertEquals(AuthRequestsResult.Error(error = error), result)
} }
@Test @Test
@@ -1027,9 +1031,10 @@ class AuthRequestManagerTest {
@Test @Test
fun `updateAuthRequest should return failure when sdk returns failure`() = runTest { fun `updateAuthRequest should return failure when sdk returns failure`() = runTest {
val error = Throwable("Fail")
coEvery { coEvery {
vaultSdkSource.getAuthRequestKey(publicKey = PUBLIC_KEY, userId = USER_ID) vaultSdkSource.getAuthRequestKey(publicKey = PUBLIC_KEY, userId = USER_ID)
} returns Throwable("Fail").asFailure() } returns error.asFailure()
fakeAuthDiskSource.userState = SINGLE_USER_STATE fakeAuthDiskSource.userState = SINGLE_USER_STATE
val result = repository.updateAuthRequest( val result = repository.updateAuthRequest(
@@ -1042,7 +1047,7 @@ class AuthRequestManagerTest {
coVerify(exactly = 1) { coVerify(exactly = 1) {
vaultSdkSource.getAuthRequestKey(publicKey = PUBLIC_KEY, userId = USER_ID) vaultSdkSource.getAuthRequestKey(publicKey = PUBLIC_KEY, userId = USER_ID)
} }
assertEquals(AuthRequestResult.Error, result) assertEquals(AuthRequestResult.Error(error = error), result)
} }
@Test @Test
@@ -1050,6 +1055,7 @@ class AuthRequestManagerTest {
val requestId = "requestId" val requestId = "requestId"
val passwordHash = "masterPasswordHash" val passwordHash = "masterPasswordHash"
val encodedKey = "encodedKey" val encodedKey = "encodedKey"
val error = Throwable("Mission failed")
coEvery { coEvery {
vaultSdkSource.getAuthRequestKey(publicKey = PUBLIC_KEY, userId = USER_ID) vaultSdkSource.getAuthRequestKey(publicKey = PUBLIC_KEY, userId = USER_ID)
} returns encodedKey.asSuccess() } returns encodedKey.asSuccess()
@@ -1061,7 +1067,7 @@ class AuthRequestManagerTest {
deviceId = UNIQUE_APP_ID, deviceId = UNIQUE_APP_ID,
isApproved = false, isApproved = false,
) )
} returns Throwable("Mission failed").asFailure() } returns error.asFailure()
fakeAuthDiskSource.userState = SINGLE_USER_STATE fakeAuthDiskSource.userState = SINGLE_USER_STATE
val result = repository.updateAuthRequest( val result = repository.updateAuthRequest(
@@ -1074,7 +1080,7 @@ class AuthRequestManagerTest {
coVerify(exactly = 1) { coVerify(exactly = 1) {
vaultSdkSource.getAuthRequestKey(publicKey = PUBLIC_KEY, userId = USER_ID) vaultSdkSource.getAuthRequestKey(publicKey = PUBLIC_KEY, userId = USER_ID)
} }
assertEquals(AuthRequestResult.Error, result) assertEquals(AuthRequestResult.Error(error = error), result)
} }
@Test @Test

View File

@@ -210,7 +210,7 @@ class LoginApprovalViewModelTest : BaseViewModelTest() {
viewState = LoginApprovalState.ViewState.Error, viewState = LoginApprovalState.ViewState.Error,
) )
val viewModel = createViewModel() val viewModel = createViewModel()
mutableAuthRequestSharedFlow.tryEmit(AuthRequestUpdatesResult.Error) mutableAuthRequestSharedFlow.tryEmit(AuthRequestUpdatesResult.Error(error = Throwable()))
assertEquals(expected, viewModel.stateFlow.value) assertEquals(expected, viewModel.stateFlow.value)
} }
@@ -383,6 +383,7 @@ class LoginApprovalViewModelTest : BaseViewModelTest() {
@Test @Test
fun `on ErrorDialogDismiss should update state`() = runTest { fun `on ErrorDialogDismiss should update state`() = runTest {
val viewModel = createViewModel() val viewModel = createViewModel()
val error = Throwable("Fail!")
coEvery { coEvery {
mockAuthRepository.updateAuthRequest( mockAuthRepository.updateAuthRequest(
requestId = REQUEST_ID, requestId = REQUEST_ID,
@@ -390,7 +391,7 @@ class LoginApprovalViewModelTest : BaseViewModelTest() {
publicKey = PUBLIC_KEY, publicKey = PUBLIC_KEY,
isApproved = false, isApproved = false,
) )
} returns AuthRequestResult.Error } returns AuthRequestResult.Error(error = error)
viewModel.trySendAction(LoginApprovalAction.DeclineRequestClick) viewModel.trySendAction(LoginApprovalAction.DeclineRequestClick)
assertEquals( assertEquals(
@@ -399,6 +400,7 @@ class LoginApprovalViewModelTest : BaseViewModelTest() {
dialogState = LoginApprovalState.DialogState.Error( dialogState = LoginApprovalState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(), title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(), message = R.string.generic_error_message.asText(),
error = error,
), ),
), ),
) )

View File

@@ -152,7 +152,9 @@ class PendingRequestsViewModelTest : BaseViewModelTest() {
viewState = PendingRequestsState.ViewState.Error, viewState = PendingRequestsState.ViewState.Error,
) )
val viewModel = createViewModel() val viewModel = createViewModel()
mutableAuthRequestsWithUpdatesFlow.tryEmit(AuthRequestsUpdatesResult.Error) mutableAuthRequestsWithUpdatesFlow.tryEmit(
value = AuthRequestsUpdatesResult.Error(error = Throwable()),
)
assertEquals(expected, viewModel.stateFlow.value) assertEquals(expected, viewModel.stateFlow.value)
} }