Skip to content

[PM-27135] feat: Add Use Passkey screen - #2947

Draft
morganzellers-bw wants to merge 1 commit into
pm-27134-testharness-register-passkeys-uifrom
pm-27135-testharness-use-passkeys-ui
Draft

[PM-27135] feat: Add Use Passkey screen#2947
morganzellers-bw wants to merge 1 commit into
pm-27134-testharness-register-passkeys-uifrom
pm-27135-testharness-use-passkeys-ui

Conversation

@morganzellers-bw

@morganzellers-bw morganzellers-bw commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

PM-27135

📔 Objective

Third PR in a 4-PR stack adding SDK-backed passkey test scenarios to TestHarness. Stacked on #2946.

  • Adds the "Use Passkey (SDK)" scenario: lists credentials registered via SDKPasskeyService.registeredCredentials, and asserts against the tapped credential's specific credential ID and relying party ID via SDKPasskeyService.assertPasskey, showing the resulting relying party ID and username.
  • Wires the new screen into the scenario picker and root navigation.

📸 Screenshots


Stack: #2945#2946#2947#2948

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Re-reviewed the third PR in the TestHarness passkey stack after the SDK* type prefixes were dropped. The change adds the "Use Passkey" screen (Action/Effect/State/Processor/View plus processor tests and a GetAssertionResult fixture) and wires it into RootRoute, RootCoordinator, and the scenario picker. Structure, Localizations keys (all present in Localizable.strings for SwiftGen), fixture placement under Fixtures/ (excluded from the app target and included in the test target by project-bwth.yml), preview #if DEBUG gating, and test setup/teardown all mirror the sibling RegisterPasskey screen; Data.asHexString() resolves to the public BitwardenKit extension. No security concerns: this is a separate TestHarness target, no production code is touched, crypto stays behind PasskeyService/BitwardenSdk, and the synthetic identity remains keychain-backed.

Code Review Details
  • ⚠️ : Empty state renders while credentials are still loading, showing "No credentials registered yet" before the list arrives (existing unresolved thread — not re-posted)
    • TestHarnessShared/UI/Autofill/Passkey/UsePasskeyView.swift:40

PR Metadata Assessment

  • QUESTION: This adds a new user-facing screen but the Screenshots section is still an HTML-comment placeholder, so it renders empty. Please add screenshots before merge as the description asks.

Comment on lines +40 to +42
if store.state.registeredCredentials.isEmpty {
Text(Localizations.sdkNoRegisteredCredentials)
.foregroundStyle(.secondary)

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.

⚠️ IMPORTANT: The empty state renders while credentials are still loading, so testers see "No credentials registered yet" before the real list arrives.

Details and fix

registeredCredentials starts [] and status starts .idle, so SwiftUI evaluates body and shows the empty-state message before .task { await store.perform(.loadRegisteredCredentials) } completes. That load is not cheap on the first call — DefaultSDKPasskeyService.session() hits the keychain and then runs initializeUserCrypto with 600,000 PBKDF2 iterations — so the misleading "Use Register Passkey (SDK) to create one" message is visible for a noticeable moment even when credentials do exist. For a harness screen whose whole job is to list registered credentials, that can read as a registration failure.

Distinguishing "not loaded yet" from "loaded and empty" fixes it, e.g. add a loading flag to the state:

// SDKUsePasskeyState
var isLoadingCredentials = true
// SDKUsePasskeyProcessor.loadRegisteredCredentials()
defer { state.isLoadingCredentials = false }
// SDKUsePasskeyView.registeredCredentialsSection
if store.state.isLoadingCredentials {
    ProgressView()
} else if store.state.registeredCredentials.isEmpty {
    Text(Localizations.sdkNoRegisteredCredentials)
        .foregroundStyle(.secondary)
} else {
    // ...
}

This also covers the tap path: .disabled(store.state.status == .inProgress) silently disables the rows during an assertion with no visible indicator, unlike SDKRegisterPasskeyView, which shows a ProgressView next to its button.

@morganzellers-bw
morganzellers-bw force-pushed the pm-27135-testharness-use-passkeys-ui branch from 544b847 to 5065ad1 Compare August 7, 2026 21:56
@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.47%. Comparing base (3d55e64) to head (9f46c09).

Additional details and impacted files
@@                              Coverage Diff                              @@
##           pm-27134-testharness-register-passkeys-ui    #2947      +/-   ##
=============================================================================
- Coverage                                      79.48%   79.47%   -0.01%     
=============================================================================
  Files                                           1172     1172              
  Lines                                          75144    75144              
=============================================================================
- Hits                                           59726    59724       -2     
- Misses                                         15418    15420       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@morganzellers-bw
morganzellers-bw force-pushed the pm-27135-testharness-use-passkeys-ui branch from 5065ad1 to ad91844 Compare August 7, 2026 22:32
@morganzellers-bw
morganzellers-bw force-pushed the pm-27135-testharness-use-passkeys-ui branch from ad91844 to 2daf425 Compare August 7, 2026 22:38
@morganzellers-bw
morganzellers-bw force-pushed the pm-27135-testharness-use-passkeys-ui branch from 2daf425 to 5669016 Compare August 10, 2026 14:14
@morganzellers-bw
morganzellers-bw force-pushed the pm-27135-testharness-use-passkeys-ui branch from 5669016 to c2a8b7a Compare August 10, 2026 14:26
@morganzellers-bw
morganzellers-bw force-pushed the pm-27135-testharness-use-passkeys-ui branch from c2a8b7a to 9d219bb Compare August 10, 2026 14:55
@morganzellers-bw morganzellers-bw changed the title [PM-27135] feat: Add SDK-backed Use Passkey (SDK) screen [PM-27135] feat: Add Use Passkey screen Aug 10, 2026
@morganzellers-bw
morganzellers-bw force-pushed the pm-27135-testharness-use-passkeys-ui branch from 9d219bb to eedacf1 Compare August 10, 2026 17:00
Adds the "Use Passkey" scenario: lists credentials registered via
PasskeyService.registeredCredentials, and asserts against the
tapped credential's specific credential ID and relying party ID via
PasskeyService.assertPasskey, showing the resulting relying party
ID and username. Wires it into the scenario picker and root
navigation.
@morganzellers-bw
morganzellers-bw force-pushed the pm-27135-testharness-use-passkeys-ui branch from eedacf1 to 9f46c09 Compare August 10, 2026 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review app:password-manager Bitwarden Password Manager app context t:feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant