Skip to content

[PM-17241] Incorporate Bitwarden FIDO SDK/passkey-rs changes #2190

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

Closed
iinuwa wants to merge 2 commits into from
Closed

[PM-17241] Incorporate Bitwarden FIDO SDK/passkey-rs changes #2190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16f235f
Incorporate Bitwarden FIDO SDK/passkey-rs changes
iinuwa Dec 10, 2025
57f3f0e
Remove remaining references to userVerification preference
iinuwa Dec 31, 2025
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
11 changes: 1 addition & 10 deletions Bitwarden.xcworkspace/xcshareddata/swiftpm/Package.resolved
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion BitwardenShared/Core/Auth/Services/TestHelpers/MockClientFido2Authenticator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import BitwardenSdk
import Foundation
import TestHelpers

@testable import BitwardenShared
Expand All @@ -23,7 +24,7 @@ class MockClientFido2Authenticator: ClientFido2AuthenticatorProtocol {
try makeCredentialMocker.invoke(param: request)
}

func silentlyDiscoverCredentials(rpId: String) async throws -> [Fido2CredentialAutofillView] {
func silentlyDiscoverCredentials(rpId: String, userHandle: Data?) async throws -> [Fido2CredentialAutofillView] {
try silentlyDiscoverCredentialsResult.get()
}
}
3 changes: 0 additions & 3 deletions BitwardenShared/Core/Autofill/Services/AutofillCredentialService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,9 +553,6 @@ extension DefaultAutofillCredentialService: AutofillCredentialService {
await fido2UserInterfaceHelper.setupDelegate(
fido2UserInterfaceHelperDelegate: fido2UserInterfaceHelperDelegate,
)
await fido2UserInterfaceHelper.setupCurrentUserVerificationPreference(
userVerificationPreference: request.options.uv,
)

#if DEBUG
Fido2DebuggingReportBuilder.builder.withGetAssertionRequest(request)
Expand Down
5 changes: 0 additions & 5 deletions BitwardenShared/Core/Autofill/Services/AutofillCredentialServiceTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,6 @@ class AutofillCredentialServiceTests: BitwardenTestCase { // swiftlint:disable:t
)

XCTAssertFalse(autofillCredentialServiceDelegate.unlockVaultWithNeverlockKeyCalled)
XCTAssertEqual(fido2UserInterfaceHelper.userVerificationPreferenceSetup, .discouraged)

XCTAssertTrue(totpService.copyTotpIfPossibleCalled)
XCTAssertTrue(errorReporter.errors.isEmpty)
Expand Down Expand Up @@ -402,7 +401,6 @@ class AutofillCredentialServiceTests: BitwardenTestCase { // swiftlint:disable:t
)

XCTAssertFalse(autofillCredentialServiceDelegate.unlockVaultWithNeverlockKeyCalled)
XCTAssertEqual(fido2UserInterfaceHelper.userVerificationPreferenceSetup, .discouraged)

XCTAssertEqual(result.userHandle, expectedAssertionResult.userHandle)
XCTAssertEqual(result.relyingParty, passkeyIdentity.relyingPartyIdentifier)
Expand Down Expand Up @@ -453,7 +451,6 @@ class AutofillCredentialServiceTests: BitwardenTestCase { // swiftlint:disable:t
XCTAssertTrue(autofillCredentialServiceDelegate.unlockVaultWithNeverlockKeyCalled)

XCTAssertNotNil(fido2UserInterfaceHelper.fido2UserInterfaceHelperDelegate)
XCTAssertEqual(fido2UserInterfaceHelper.userVerificationPreferenceSetup, .discouraged)

XCTAssertEqual(result.userHandle, expectedAssertionResult.userHandle)
XCTAssertEqual(result.relyingParty, passkeyIdentity.relyingPartyIdentifier)
Expand Down Expand Up @@ -523,7 +520,6 @@ class AutofillCredentialServiceTests: BitwardenTestCase { // swiftlint:disable:t
XCTAssertFalse(autofillCredentialServiceDelegate.unlockVaultWithNeverlockKeyCalled)

XCTAssertNotNil(fido2UserInterfaceHelper.fido2UserInterfaceHelperDelegate)
XCTAssertEqual(fido2UserInterfaceHelper.userVerificationPreferenceSetup, .discouraged)

XCTAssertEqual(result.userHandle, expectedAssertionResult.userHandle)
XCTAssertEqual(result.relyingParty, passkeyIdentity.relyingPartyIdentifier)
Expand Down Expand Up @@ -640,7 +636,6 @@ class AutofillCredentialServiceTests: BitwardenTestCase { // swiftlint:disable:t
for: passkeyParameters,
fido2UserInterfaceHelperDelegate: fido2UserInterfaceHelperDelegate,
)
XCTAssertEqual(fido2UserInterfaceHelper.userVerificationPreferenceSetup, .preferred)

XCTAssertEqual(result.userHandle, expectedAssertionResult.userHandle)
XCTAssertEqual(result.relyingParty, passkeyParameters.relyingPartyIdentifier)
Expand Down
10 changes: 7 additions & 3 deletions BitwardenShared/Core/Vault/Services/Fido2CredentialStoreService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ final class Fido2CredentialStoreService: Fido2CredentialStore {
/// When `nil` the `credentialId` filter is not applied.
/// - ripId: The `ripId` to match the Fido2 credential `rpId`.
/// - Returns: All the ciphers that matches the filter.
func findCredentials(ids: [Data]?, ripId: String) async throws -> [BitwardenSdk.CipherView] {
func findCredentials(ids: [Data]?, ripId: String, userHandle: Data?) async throws -> [BitwardenSdk.CipherView] {
do {
try await syncService.fetchSync(forceSync: false)
} catch {
Expand Down Expand Up @@ -82,6 +82,10 @@ final class Fido2CredentialStoreService: Fido2CredentialStore {
!ids.contains(fido2CredentialAutofillView.credentialId) {
continue
}

if let userHandle, fido2CredentialAutofillView.userHandle != userHandle {
continue
}

result.append(cipherView)
}
Expand Down Expand Up @@ -118,9 +122,9 @@ class DebuggingFido2CredentialStoreService: Fido2CredentialStore {
self.fido2CredentialStore = fido2CredentialStore
}

func findCredentials(ids: [Data]?, ripId: String) async throws -> [BitwardenSdk.CipherView] {
func findCredentials(ids: [Data]?, ripId: String, userHandle: Data?) async throws -> [BitwardenSdk.CipherView] {
do {
let result = try await fido2CredentialStore.findCredentials(ids: ids, ripId: ripId)
let result = try await fido2CredentialStore.findCredentials(ids: ids, ripId: ripId, userHandle: userHandle)
Fido2DebuggingReportBuilder.builder.withFindCredentialsResult(.success(result))
return result
} catch {
Expand Down
18 changes: 9 additions & 9 deletions BitwardenShared/Core/Vault/Services/Fido2CredentialStoreServiceTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ class Fido2CredentialStoreServiceTests: BitwardenTestCase { // swiftlint:disable
]
}

let result = try await subject.findCredentials(ids: credentialIds, ripId: expectedRpId)
let result = try await subject.findCredentials(ids: credentialIds, ripId: expectedRpId, userHandle: nil)

XCTAssertTrue(syncService.didFetchSync)
XCTAssertTrue(result.count == 1)
Expand Down Expand Up @@ -171,7 +171,7 @@ class Fido2CredentialStoreServiceTests: BitwardenTestCase { // swiftlint:disable
]
}

let result = try await subject.findCredentials(ids: nil, ripId: expectedRpId)
let result = try await subject.findCredentials(ids: nil, ripId: expectedRpId, userHandle: nil)

XCTAssertTrue(result.count == 2)
XCTAssertTrue(result[0].id == expectedCipherIds[0])
Expand All @@ -182,7 +182,7 @@ class Fido2CredentialStoreServiceTests: BitwardenTestCase { // swiftlint:disable
func test_findCredentials_empty() async throws {
cipherService.fetchAllCiphersResult = .success([])

let result = try await subject.findCredentials(ids: nil, ripId: "something")
let result = try await subject.findCredentials(ids: nil, ripId: "something", userHandle: nil)

XCTAssertTrue(result.isEmpty)
}
Expand All @@ -191,7 +191,7 @@ class Fido2CredentialStoreServiceTests: BitwardenTestCase { // swiftlint:disable
func test_findCredentials_throwsSync() async throws {
syncService.fetchSyncResult = .failure(BitwardenTestError.example)

_ = try await subject.findCredentials(ids: nil, ripId: "something")
_ = try await subject.findCredentials(ids: nil, ripId: "something", userHandle: nil)

XCTAssertFalse(errorReporter.errors.isEmpty)
XCTAssertTrue(cipherService.fetchAllCiphersCalled)
Expand All @@ -202,7 +202,7 @@ class Fido2CredentialStoreServiceTests: BitwardenTestCase { // swiftlint:disable
cipherService.fetchAllCiphersResult = .failure(BitwardenTestError.example)

await assertAsyncThrows(error: BitwardenTestError.example) {
_ = try await subject.findCredentials(ids: nil, ripId: "something")
_ = try await subject.findCredentials(ids: nil, ripId: "something", userHandle: nil)
}
}

Expand All @@ -224,7 +224,7 @@ class Fido2CredentialStoreServiceTests: BitwardenTestCase { // swiftlint:disable
}

await assertAsyncThrows(error: BitwardenTestError.example) {
_ = try await subject.findCredentials(ids: nil, ripId: "something")
_ = try await subject.findCredentials(ids: nil, ripId: "something", userHandle: nil)
}
}

Expand All @@ -245,7 +245,7 @@ class Fido2CredentialStoreServiceTests: BitwardenTestCase { // swiftlint:disable
.throwing(BitwardenTestError.example)

await assertAsyncThrows(error: BitwardenTestError.example) {
_ = try await subject.findCredentials(ids: nil, ripId: "something")
_ = try await subject.findCredentials(ids: nil, ripId: "something", userHandle: nil)
}
}

Expand Down Expand Up @@ -385,7 +385,7 @@ class DebuggingFido2CredentialStoreServiceTests: BitwardenTestCase {
/// `.findCredentials(ids:ripId:)` returns found credentials and reports it.
func test_findCredentials() async throws {
fido2CredentialStore.findCredentialsResult = .success([.fixture()])
let result = try await subject.findCredentials(ids: nil, ripId: "something")
let result = try await subject.findCredentials(ids: nil, ripId: "something", userHandle: nil)
XCTAssert(result.count == 1)
XCTAssertFalse(
(try? Fido2DebuggingReportBuilder.builder
Expand All @@ -397,7 +397,7 @@ class DebuggingFido2CredentialStoreServiceTests: BitwardenTestCase {
func test_findCredentialsthrows() async throws {
fido2CredentialStore.findCredentialsResult = .failure(BitwardenTestError.example)
await assertAsyncThrows(error: BitwardenTestError.example) {
_ = try await subject.findCredentials(ids: nil, ripId: "something")
_ = try await subject.findCredentials(ids: nil, ripId: "something", userHandle: nil)
}
XCTAssertNil(try? Fido2DebuggingReportBuilder.builder.getReport()?
.findCredentialsResult?.get())
Expand Down
2 changes: 1 addition & 1 deletion BitwardenShared/Core/Vault/Services/TestHelpers/MockFido2CredentialStore.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class MockFido2CredentialStore: Fido2CredentialStore {
var saveCredentialCalled = false
var saveCredentialError: (any Error)?

func findCredentials(ids: [Data]?, ripId: String) async throws -> [BitwardenSdk.CipherView] {
func findCredentials(ids: [Data]?, ripId: String, userHandle: Data?) async throws -> [BitwardenSdk.CipherView] {
try findCredentialsResult.get()
}

Expand Down
31 changes: 4 additions & 27 deletions BitwardenShared/UI/Autofill/Utilities/Fido2UserInterfaceHelper.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,6 @@ protocol Fido2UserInterfaceHelper: Fido2UserInterface {
/// Sets up the delegate to use on Fido2 user verification flows and to get information of the FIdo2 flow.
/// - Parameter fido2UserInterfaceHelperDelegate: The delegate to use
func setupDelegate(fido2UserInterfaceHelperDelegate: Fido2UserInterfaceHelperDelegate)

/// Sets up the user verification preference of the current Fido2 flow.
/// - Parameter userVerificationPreference: User verification preference to set up.
func setupCurrentUserVerificationPreference(userVerificationPreference: Uv)
}

// MARK: - DefaultFido2UserInterfaceHelper
Expand All @@ -94,11 +90,6 @@ class DefaultFido2UserInterfaceHelper: Fido2UserInterfaceHelper {
private(set) var fido2CreationOptions: BitwardenSdk.CheckUserOptions?
private(set) var fido2CredentialNewView: BitwardenSdk.Fido2CredentialNewView?

/// The user verification preference of the current Fido2 flow.
/// This is needed as a workaround to be used in `isVerificationEnabled`
/// as currently we don't have the UV value provided by the SDK.
private var userVerificationPreference: Uv?

/// Initializes a `DefaultFido2UserInterfaceHelper`.
/// - Parameter fido2UserVerificationMediator: Mediator which manages user verification on Fido2 flows
init(fido2UserVerificationMediator: Fido2UserVerificationMediator) {
Expand Down Expand Up @@ -186,19 +177,9 @@ class DefaultFido2UserInterfaceHelper: Fido2UserInterfaceHelper {
}
}

func isVerificationEnabled() async -> Bool {
guard let userVerificationPreference else {
return false
}

switch userVerificationPreference {
case .discouraged:
return false
case .preferred:
return await fido2UserVerificationMediator.isPreferredVerificationEnabled()
case .required:
return true
}
func isVerificationEnabled() -> Bool {
Copy link
Copy Markdown
Contributor

@KatherineInCode KatherineInCode Dec 31, 2025

Choose a reason for hiding this comment

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

🤔 Is this function likely to change in the future, to have more logic in it? If not, it would make sense to me to just delete it, and cascade up the true.

Copy link
Copy Markdown
Contributor Author

@iinuwa iinuwa Jan 5, 2026

Choose a reason for hiding this comment

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

No, not on iOS, where using passkeys requires setting up at least one local OS auth method. After thinking about this, I agree, I think it would be good to push it into the SDK.

This method exists because the SDK is passing through a required method to fulfill a passkey-rs trait, so we can't remove it without changing it for all consumers of the SDK. I think there are two reasons we should push this into the SDK:

  • I believe that all of our clients should have a way to do user verification, even it's just redoing whatever auth/unlock ceremony they have enabled for their vault. So this would just become a static value inside the SDK.
  • We are only using this method to:
    • set the options.uv value in authenticatorGetInfo, which is merely a hint for platforms, and to
    • set the options.uv value in checkUser on authenticatorMakeCredential and authenticatorGetAssertion—which is always ignored in our implementation, since we pass the WebAuthn UserVerificationRequirement enum instead.1

For those reasons, and because of the confusion having this method introduces, I think removing this method from the SDK trait and letting the SDK abstract over it would be beneficial.

But the current implementation, where isVerificationEnabled() always returns true produces the same behavior as pushing this SDK trait method, so I'd recommend following that up in a separate PR in the SDK.

I think I should clarify two other things that I am not talking about changing:

  • changing checkUser(options: BitwardenSdk.CheckUserOptions, ...) to checkUser(shouldVerify: bool, ...). Even though we could decide whether we should prompt the user for verification from the SDK, if we change this to a simple bool, it still loses information on how hard we should try to verify the user. For example, the current iOS implementation will prompt for UV on both preferred and required, but if set to required and no verification methods are available, it will start the PIN setup flow, but it will just return userVerified: false if only preferred.

  • removing shouldThrowEnforcingRequiredVerification parameter from checkUser(). I'm a little fuzzy on this one. It seems that we don't set this flag when calling this from the SDK, since the SDK will check this and return an appropriate CTAP2 error code. But we call this method from other places that don't go through the SDK FIDO methods (VaultAutofillListProcessor, AddEditItemProcessor), so this method is used there. I don't know enough about these two call sites to know whether this is useful or not.

Footnotes

  1. To expand on this point, passkey-rs only sends the CTAP2 options.uv boolean, which loses context on whether the UV request can be ignored or not (e.g. preferred is not represented.) So instead, we send the original WebAuthn UV preference, discouraged, required, or preferred (found in options.authenticatorSelection.userVerification in navigator.credential.create() and userVerification in navigator.credential.get()) which gives more context. ↩

Copy link
Copy Markdown
Member

@abergs abergs Jan 5, 2026

Choose a reason for hiding this comment

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

A ticket has been created to clean up / refine ´isVerificationEnabled´ so that we can unblock this PR https://bitwarden.atlassian.net/browse/PM-30454

// TODO: PM-30454 We should consider subsuming this into the SDK, since this value is constant across all our current clients
true
}

func pickedCredentialForAuthentication(result: Result<CipherView, Error>) {
Expand All @@ -216,10 +197,6 @@ class DefaultFido2UserInterfaceHelper: Fido2UserInterfaceHelper {
)
}

func setupCurrentUserVerificationPreference(userVerificationPreference: Uv) {
self.userVerificationPreference = userVerificationPreference
}

// MARK: Private

/// Checks if we should enforce `required` behavior on the passed `userVerificationPreference`
Expand All @@ -231,7 +208,7 @@ class DefaultFido2UserInterfaceHelper: Fido2UserInterfaceHelper {
case .discouraged:
false
case .preferred:
await isVerificationEnabled()
await fido2UserVerificationMediator.isPreferredVerificationEnabled()
case .required:
true
}
Expand Down
18 changes: 1 addition & 17 deletions BitwardenShared/UI/Autofill/Utilities/Fido2UserInterfaceHelperTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ class Fido2UserInterfaceHelperTests: BitwardenTestCase { // swiftlint:disable:th
fido2UserVerificationMediator.checkUserResult = .success(
CheckUserResult(userPresent: true, userVerified: false),
)
subject.setupCurrentUserVerificationPreference(userVerificationPreference: .preferred)
fido2UserVerificationMediator.isPreferredVerificationEnabledResult = true
await assertAsyncThrows(error: Fido2UserVerificationError.requiredEnforcementFailed) {
_ = try await subject.checkUser(
Expand Down Expand Up @@ -359,22 +358,7 @@ class Fido2UserInterfaceHelperTests: BitwardenTestCase { // swiftlint:disable:th

/// `isVerificationEnabled()` returns what the mediator returns.
func test_isVerificationEnabled() async throws {
subject.setupCurrentUserVerificationPreference(userVerificationPreference: .discouraged)
let resultDiscouraged = await subject.isVerificationEnabled()
XCTAssertFalse(resultDiscouraged)

subject.setupCurrentUserVerificationPreference(userVerificationPreference: .required)
let resultRequired = await subject.isVerificationEnabled()
XCTAssertTrue(resultRequired)

subject.setupCurrentUserVerificationPreference(userVerificationPreference: .preferred)
fido2UserVerificationMediator.isPreferredVerificationEnabledResult = true
let resultPreferredTrue = await subject.isVerificationEnabled()
XCTAssertTrue(resultPreferredTrue)

fido2UserVerificationMediator.isPreferredVerificationEnabledResult = false
let resultPreferredFalse = await subject.isVerificationEnabled()
XCTAssertFalse(resultPreferredFalse)
XCTAssertTrue(subject.isVerificationEnabled())
}

/// `setupDelegate(fido2UserVerificationMediatorDelegate:)` sets up delegate in inner mediator.
Expand Down
7 changes: 1 addition & 6 deletions BitwardenShared/UI/Autofill/Utilities/TestHelpers/MockFido2UserInterfaceHelper.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ class MockFido2UserInterfaceHelper: Fido2UserInterfaceHelper {
)
var isVerificationEnabledResult = false
var fido2UserInterfaceHelperDelegate: Fido2UserInterfaceHelperDelegate?
var userVerificationPreferenceSetup: Uv?

nonisolated init() {}

Expand Down Expand Up @@ -83,15 +82,11 @@ class MockFido2UserInterfaceHelper: Fido2UserInterfaceHelper {
try checkAndPickCredentialForCreationResult.get()
}

func isVerificationEnabled() async -> Bool {
func isVerificationEnabled() -> Bool {
isVerificationEnabledResult
}

func setupDelegate(fido2UserInterfaceHelperDelegate: any BitwardenShared.Fido2UserInterfaceHelperDelegate) {
self.fido2UserInterfaceHelperDelegate = fido2UserInterfaceHelperDelegate
}

func setupCurrentUserVerificationPreference(userVerificationPreference: BitwardenSdk.Uv) {
userVerificationPreferenceSetup = userVerificationPreference
}
}
1 change: 0 additions & 1 deletion BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor+Fido2Tests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,6 @@ class VaultAutofillListProcessorFido2Tests: BitwardenTestCase { // swiftlint:dis
XCTAssertEqual(subject.state.emptyViewButtonText, Localizations.savePasskeyAsNewLogin)

XCTAssertTrue(fido2UserInterfaceHelper.fido2UserInterfaceHelperDelegate != nil)
XCTAssertEqual(fido2UserInterfaceHelper.userVerificationPreferenceSetup, .discouraged)
appExtensionDelegate.completeRegistrationRequestMocker.assertUnwrapping { credential in
credential.relyingParty == expectedCredentialIdentity.relyingPartyIdentifier
&& credential.clientDataHash == expectedRequest.clientDataHash
Expand Down
3 changes: 0 additions & 3 deletions BitwardenShared/UI/Vault/Vault/AutofillList/VaultAutofillListProcessor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,9 +618,6 @@ extension VaultAutofillListProcessor {
),
extensions: nil,
)
services.fido2UserInterfaceHelper.setupCurrentUserVerificationPreference(
userVerificationPreference: userVerificationPreference,
)
let createdCredential = try await services.clientService.platform().fido2()
.authenticator(
userInterface: services.fido2UserInterfaceHelper,
Expand Down
Loading