Skip to content

[PM-41929] fix: Update the manage devices screen when a passwordless request push is received #7280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aj-rosado wants to merge 9 commits into
base: main
Choose a base branch
from
Open

[PM-41929] fix: Update the manage devices screen when a passwordless request push is received #7280

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
25ab96b
Added flow so push notifications update the manage devices screen
aj-rosado Aug 18, 2026
5b943e1
Exported isActionable to an utils to avoid duplicate code
aj-rosado Aug 18, 2026
35b427f
Address PR comments
aj-rosado Aug 19, 2026
8071273
add repeated functions into extension
aj-rosado Aug 21, 2026
85ed1ce
Merge push notifications into the auth request updates flow
aj-rosado Aug 24, 2026
f1f45da
Showing error when fetch fails
aj-rosado Aug 25, 2026
e8042cf
reverted to display error when loading devices
aj-rosado Aug 25, 2026
c378660
addressed ManageDevices screen test error
aj-rosado Aug 26, 2026
2cdc315
removed unnecessary suppresion
aj-rosado Aug 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/AuthRequestManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ interface AuthRequestManager {
*/
fun getAuthRequestsWithUpdates(): Flow<AuthRequestsUpdatesResult>

/**
* Get the [AuthRequest] for each incoming passwordless request for the active user, hydrated
* with the fingerprint required to approve it.
*
* Only requests that can still be acted upon are emitted; those already approved, declined, or
* expired are not. Requests that cannot be retrieved are omitted rather than emitted as errors.
*/
fun getPasswordlessAuthRequestFlow(): Flow<AuthRequest>

/**
* Get an [AuthRequest] by its request ID.
*/
Expand Down
164 changes: 55 additions & 109 deletions app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/AuthRequestManagerImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.bitwarden.core.AuthRequestResponse
import com.bitwarden.core.data.util.asFailure
import com.bitwarden.core.data.util.asSuccess
import com.bitwarden.core.data.util.flatMap
import com.bitwarden.core.util.isOverFiveMinutesOld
import com.bitwarden.network.model.AuthRequestTypeJson
import com.bitwarden.network.service.AuthRequestsService
import com.bitwarden.network.service.NewAuthRequestService
Expand All @@ -18,14 +19,19 @@ import com.x8bit.bitwarden.data.auth.manager.model.AuthRequestsResult
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.util.isSso
import com.x8bit.bitwarden.data.auth.manager.util.toAuthRequest
import com.x8bit.bitwarden.data.auth.manager.util.toAuthRequestTypeJson
import com.x8bit.bitwarden.data.platform.error.NoActiveUserException
import com.x8bit.bitwarden.data.platform.manager.PushManager
import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.mapNotNull
import kotlinx.coroutines.isActive
import timber.log.Timber
import java.time.Clock
import java.time.Instant
import javax.inject.Singleton
Expand All @@ -38,7 +44,7 @@ private const val PASSWORDLESS_APPROVER_INTERVAL_MILLIS: Long = 5L * 60L * 1_000
/**
* Default implementation of [AuthRequestManager].
*/
@Suppress("TooManyFunctions")
@Suppress("LongParameterList", "TooManyFunctions")
@Singleton
class AuthRequestManagerImpl(
private val clock: Clock,
Expand All @@ -47,6 +53,7 @@ class AuthRequestManagerImpl(
private val authDiskSource: AuthDiskSource,
private val authSdkSource: AuthSdkSource,
private val vaultSdkSource: VaultSdkSource,
private val pushManager: PushManager,
) : AuthRequestManager {
private val activeUserId: String? get() = authDiskSource.userState?.activeUserId

Expand Down Expand Up @@ -91,19 +98,7 @@ class AuthRequestManagerImpl(
isSso = authRequestType.isSso,
)
.map { request ->
AuthRequest(
id = request.id,
publicKey = request.publicKey,
platform = request.platform,
ipAddress = request.ipAddress,
key = request.key,
masterPasswordHash = request.masterPasswordHash,
creationDate = request.creationDate,
responseDate = request.responseDate,
requestApproved = request.requestApproved ?: false,
originUrl = request.originUrl,
fingerprint = authRequest.fingerprint,
)
request.toAuthRequest(fingerprint = authRequest.fingerprint)
}
.fold(
onFailure = { emit(CreateAuthRequestResult.Error(error = it)) },
Expand Down Expand Up @@ -183,21 +178,15 @@ class AuthRequestManagerImpl(
isRequestApproved = false
responseDate = clock.instant()
}
AuthRequest(
id = request.id,
platform = request.platform,
ipAddress = request.ipAddress,
key = request.key,
masterPasswordHash = request.masterPasswordHash,
creationDate = request.creationDate,
originUrl = request.originUrl,
responseDate = responseDate,
requestApproved = isRequestApproved,
request
// The PublicKey and Fingerprint should be frozen in place to
// ensure no funny-business happens between multiple requests.
publicKey = initialAuthRequest.publicKey,
fingerprint = initialAuthRequest.fingerprint,
)
.toAuthRequest(fingerprint = initialAuthRequest.fingerprint)

@david-livefront david-livefront Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just pass these values into the extension method instead of copying it.

fun AuthRequestsResponseJson.AuthRequest.toAuthRequest(
    fingerprint: String,
    publicKey: String = this.publicKey,
    responseDate: Instant = this.responseDate,
    isRequestApproved: Boolean = this.requestApproved,
): AuthRequest = AuthRequest(

.copy(
publicKey = initialAuthRequest.publicKey,
responseDate = responseDate,
requestApproved = isRequestApproved,
)
}
}
.fold(
Expand Down Expand Up @@ -258,49 +247,44 @@ class AuthRequestManagerImpl(
authRequestsService
.getAuthRequest(requestId)
.mapCatching { response ->
getFingerprintPhrase(response.publicKey)
.getOrThrow()
.let { fingerprint ->
AuthRequest(
id = response.id,
publicKey = response.publicKey,
platform = response.platform,
ipAddress = response.ipAddress,
key = response.key,
masterPasswordHash = response.masterPasswordHash,
creationDate = response.creationDate,
responseDate = response.responseDate,
requestApproved = response.requestApproved ?: false,
originUrl = response.originUrl,
fingerprint = fingerprint,
)
}
response.toAuthRequest(
fingerprint = getFingerprintPhrase(response.publicKey).getOrThrow(),
)
}
.fold(
onFailure = { AuthRequestUpdatesResult.Error(error = it) },
onSuccess = { AuthRequestUpdatesResult.Update(authRequest = it) },
)
}

override fun getPasswordlessAuthRequestFlow(): Flow<AuthRequest> = pushManager
.passwordlessRequestFlow
// A push for a non-active user would otherwise be hydrated with the active user's token.
.filter { it.userId == activeUserId }
.mapNotNull { data ->
authRequestsService
.getAuthRequest(data.loginRequestId)
.mapCatching { response ->
response.toAuthRequest(
fingerprint = getFingerprintPhrase(response.publicKey).getOrThrow(),
)
}
.fold(
onFailure = {
Timber.d(it, "Unable to hydrate the requested auth request.")
null
},
onSuccess = { authRequest -> authRequest.takeIf { it.isActionable } },
)
}

override suspend fun getAuthRequestIfApproved(requestId: String): Result<AuthRequest> =
authRequestsService
.getAuthRequest(requestId)
.flatMap { request ->
if (request.requestApproved == true) {
getFingerprintPhrase(request.publicKey).map { fingerprint ->
AuthRequest(
id = request.id,
publicKey = request.publicKey,
platform = request.platform,
ipAddress = request.ipAddress,
key = request.key,
masterPasswordHash = request.masterPasswordHash,
creationDate = request.creationDate,
responseDate = request.responseDate,
requestApproved = true,
originUrl = request.originUrl,
fingerprint = fingerprint,
)
request.toAuthRequest(fingerprint = fingerprint)
}
} else {
IllegalStateException("Request not approved.").asFailure()
Expand All @@ -313,19 +297,7 @@ class AuthRequestManagerImpl(
.map { response ->
response.authRequests.mapNotNull { request ->
getFingerprintPhrase(request.publicKey).getOrNull()?.let { fingerprint ->
AuthRequest(
id = request.id,
publicKey = request.publicKey,
platform = request.platform,
ipAddress = request.ipAddress,
key = request.key,
masterPasswordHash = request.masterPasswordHash,
creationDate = request.creationDate,
responseDate = request.responseDate,
requestApproved = request.requestApproved ?: false,
originUrl = request.originUrl,
fingerprint = fingerprint,
)
request.toAuthRequest(fingerprint = fingerprint)
}
}
}
Expand Down Expand Up @@ -355,21 +327,7 @@ class AuthRequestManagerImpl(
isApproved = isApproved,
)
}
.map { request ->
AuthRequest(
id = request.id,
publicKey = request.publicKey,
platform = request.platform,
ipAddress = request.ipAddress,
key = request.key,
masterPasswordHash = request.masterPasswordHash,
creationDate = request.creationDate,
responseDate = request.responseDate,
requestApproved = request.requestApproved ?: false,
originUrl = request.originUrl,
fingerprint = "",
)
}
.map { request -> request.toAuthRequest(fingerprint = "") }
.fold(
onFailure = { AuthRequestResult.Error(error = it) },
onSuccess = { AuthRequestResult.Success(authRequest = it) },
Expand All @@ -393,17 +351,7 @@ class AuthRequestManagerImpl(
.getAuthRequest(pendingAuthRequest.requestId)
.map {
NewAuthRequestData(
authRequest = AuthRequest(
id = it.id,
publicKey = it.publicKey,
platform = it.platform,
ipAddress = it.ipAddress,
key = it.key,
masterPasswordHash = it.masterPasswordHash,
creationDate = it.creationDate,
responseDate = it.responseDate,
requestApproved = it.requestApproved ?: false,
originUrl = it.originUrl,
authRequest = it.toAuthRequest(
fingerprint = pendingAuthRequest.requestFingerprint,
),
privateKey = pendingAuthRequest.requestPrivateKey,
Expand Down Expand Up @@ -456,19 +404,7 @@ class AuthRequestManagerImpl(
}
}
.map { request ->
AuthRequest(
id = request.id,
publicKey = request.publicKey,
platform = request.platform,
ipAddress = request.ipAddress,
key = request.key,
masterPasswordHash = request.masterPasswordHash,
creationDate = request.creationDate,
responseDate = request.responseDate,
requestApproved = request.requestApproved ?: false,
originUrl = request.originUrl,
fingerprint = authRequestResponse.fingerprint,
)
request.toAuthRequest(fingerprint = authRequestResponse.fingerprint)
}
.map {
NewAuthRequestData(
Expand All @@ -489,6 +425,16 @@ class AuthRequestManagerImpl(
publicKey = publicKey,
)
}

/**
* Whether this request may still be approved or declined, meaning it has not already been
* approved, not been declined (indicated by it not being approved & having a responseDate),
* and has not expired (it is under 5 minutes old).
*/
private val AuthRequest.isActionable: Boolean
get() = !requestApproved &&
responseDate == null &&
!creationDate.isOverFiveMinutesOld(clock)

@claude claude Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ DEBT: isActionable is now a third copy of the approved/declined/expired predicate.

Details and fix

The same three-clause rule already exists in two places:

  • ManageDevicesViewModel.kt:537 — List<AuthRequest>.filterRespondedAndExpired(clock)
  • PendingRequestsViewModel.kt:401 — identical filterRespondedAndExpired(clock)

Adding a third copy here means the five-minute window and the decline detection now have to be kept in sync across three files.

Since this PR already extracts toAuthRequest into data/auth/manager/util/, consider putting the predicate there too and having both view models' filterRespondedAndExpired delegate to it:

// data/auth/manager/util/AuthRequestExtensions.kt
val AuthRequest.isActionable: Boolean
    get() = !requestApproved &&
        responseDate == null &&
        !creationDate.isOverFiveMinutesOld(clock)

(The clock would need to become a parameter, matching how filterRespondedAndExpired already takes one.)

Non-blocking — the current behavior is correct and matches the existing copies.

}

/**
Expand Down
2 changes: 2 additions & 0 deletions app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/di/AuthManagerModule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ object AuthManagerModule {
authSdkSource: AuthSdkSource,
vaultSdkSource: VaultSdkSource,
authDiskSource: AuthDiskSource,
pushManager: PushManager,
): AuthRequestManager =
AuthRequestManagerImpl(
clock = clock,
Expand All @@ -81,6 +82,7 @@ object AuthManagerModule {
authSdkSource = authSdkSource,
vaultSdkSource = vaultSdkSource,
authDiskSource = authDiskSource,
pushManager = pushManager,
)

@Provides
Expand Down
24 changes: 24 additions & 0 deletions ...n/kotlin/com/x8bit/bitwarden/data/auth/manager/util/AuthRequestsResponseJsonExtensions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.x8bit.bitwarden.data.auth.manager.util

import com.bitwarden.network.model.AuthRequestsResponseJson
import com.x8bit.bitwarden.data.auth.manager.model.AuthRequest

/**
* Converts the given [AuthRequestsResponseJson.AuthRequest] to an [AuthRequest], given the
* [fingerprint] that the response itself does not carry.
*/
fun AuthRequestsResponseJson.AuthRequest.toAuthRequest(
fingerprint: String,
): AuthRequest = AuthRequest(
id = id,

@david-livefront david-livefront Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity, can you dd the explicit this to all of these.

publicKey = publicKey,
platform = platform,
ipAddress = ipAddress,
key = key,
masterPasswordHash = masterPasswordHash,
creationDate = creationDate,
responseDate = responseDate,
requestApproved = requestApproved ?: false,
originUrl = originUrl,
fingerprint = fingerprint,
)
Loading
Loading