Skip to content

[PM-27134] feat: Add Register Passkey screen to TestHarness - #2946

Open
morganzellers-bw wants to merge 4 commits into
pm-40519-testharness-passkeys-sdk-foundationfrom
pm-27134-testharness-register-passkeys-ui
Open

[PM-27134] feat: Add Register Passkey screen to TestHarness#2946
morganzellers-bw wants to merge 4 commits into
pm-40519-testharness-passkeys-sdk-foundationfrom
pm-27134-testharness-register-passkeys-ui

Conversation

@morganzellers-bw

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

Copy link
Copy Markdown
Contributor

🎟️ Tracking

PM-27134

📔 Objective

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

  • Adds the "Register Passkey (SDK)" scenario: a form for relying party ID, username, and display name that calls SDKPasskeyService.registerPasskey directly, showing the resulting credential ID and attestation object.
  • Wires the new screen into the scenario picker and root navigation.
  • Also drops the dead "Passkey Autofill" scenario stub (route: nil, already unused) and adds a missing accessibility identifier case for the TOTP scenario button, since this PR was already touching those files.

📸 Screenshots

register1 register2

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

Reviewed the new "Register Passkey" TestHarness screen (Action/Effect/State/Processor/View plus processor tests), its wiring into RootRoute/RootCoordinator/ScenarioPickerState/ScenarioPickerView, the MakeCredentialResult fixture, the localization key changes, and the doc-string edits across TestHarnessShared/Core/Autofill/Passkey. The screen follows the same unidirectional Store/Processor shape as the sibling TestHarness scenarios, passkeyService is injected from ServiceContainer through HasPasskeyService (typealias Services = HasErrorReportBuilder & HasPasskeyService), and the test file's setup/teardown ordering matches CardAutofillFormProcessorTests/CreateAccountFormProcessorTests. Every localization key the new view references resolves in TestHarnessShared/.../en.lproj/Localizable.strings (CredentialId, DisplayName, PasskeyRegisteredSuccessfully, RegisterPasskey, RegisterPasskeyFormDescriptionLong, RegistrationResult, RelyingPartyId added; Credentials, Username, XColonY already present), the orphaned PasskeyAutofill string is now removed with no remaining references, and Fixtures/MakeCredentialResult+Fixtures.swift lands only in TestHarnessSharedTests per the **/Fixtures/* include/exclude rules in project-bwth.yml. No security, correctness, or breaking-change findings — the retired ScenarioButton_Passkey identifier and the reassigned ScenarioButton_TOTPForm identifier have no consumers anywhere in the repo, and Lint/Test are green.

Code Review Details
  • 🎨 : Editing a field mid-registration resets status to .idle, re-enabling the register button and allowing a concurrent duplicate registration (existing unresolved thread; no new comment posted)
    • TestHarnessShared/UI/Autofill/Passkey/RegisterPasskeyProcessor.swift:47-59

PR Metadata Assessment

  • QUESTION: The Screenshots section is still a placeholder comment for a brand-new screen; please attach captures of the idle/success/failure states before merge.
  • QUESTION: The description says the screen shows "the resulting credential ID and attestation object", but RegisterPasskeyView.statusSection renders only the credential ID.

@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from a989b44 to 290508f 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.53%. Comparing base (c4f80a6) to head (2bd6942).

Additional details and impacted files
@@                               Coverage Diff                                @@
##           pm-40519-testharness-passkeys-sdk-foundation    #2946      +/-   ##
================================================================================
- Coverage                                         79.54%   79.53%   -0.01%     
================================================================================
  Files                                              1169     1169              
  Lines                                             75095    75095              
================================================================================
- Hits                                              59732    59730       -2     
- Misses                                            15363    15365       +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-27134-testharness-register-passkeys-ui branch from 290508f to fbbcc0c Compare August 7, 2026 22:32
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from fbbcc0c to 06ae2bb Compare August 7, 2026 22:38
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from 06ae2bb to 63a2611 Compare August 10, 2026 14:14
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from 63a2611 to 3835795 Compare August 10, 2026 14:26
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from 3835795 to c0911cd Compare August 10, 2026 14:55
@morganzellers-bw morganzellers-bw changed the title [PM-27134] feat: Add SDK-backed Register Passkey (SDK) screen [PM-27134] feat: Add Register Passkey screen Aug 10, 2026
@morganzellers-bw morganzellers-bw changed the title [PM-27134] feat: Add Register Passkey screen [PM-27134] feat: Add Register Passkey screen to TestHarness Aug 10, 2026
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from c0911cd to bcaf088 Compare August 10, 2026 16:59
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from bcaf088 to 3d55e64 Compare August 10, 2026 18:46
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from 548c7e3 to eca9f12 Compare August 12, 2026 17:35
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from dd97b1d to 9ef86d8 Compare August 13, 2026 21:47
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from 9ef86d8 to 0f5816e Compare August 17, 2026 19:46
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from 0f5816e to 63f2af4 Compare August 17, 2026 20:16
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from 63f2af4 to d271a67 Compare August 18, 2026 16:26
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from d271a67 to 3863d3b Compare August 18, 2026 16:43
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from 3863d3b to 494041a Compare August 18, 2026 17:21
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from 494041a to ecc8ae3 Compare August 18, 2026 17:30
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from ecc8ae3 to e29a4aa Compare August 19, 2026 14:12
Comment on lines +47 to +59
override func receive(_ action: RegisterPasskeyAction) {
switch action {
case let .displayNameChanged(newValue):
state.displayName = newValue
state.status = .idle
case let .rpIdChanged(newValue):
state.rpId = newValue
state.status = .idle
case let .userNameChanged(newValue):
state.userName = newValue
state.status = .idle
}
}

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.

🎨 SUGGESTED: Editing a field while a registration is in flight resets status to .idle, re-enabling the register button.

Details and fix

The text fields stay editable during registration (only the button is .disabled), so typing mid-flight sets status back to .idle. That hides the ProgressView and re-enables the Register button (RegisterPasskeyView.swift:83), which lets a second concurrent registerPasskey run and register a duplicate credential for the same RP — later ambiguous for assertPasskey(credentialId: nil, rpId:).

Guarding the reset keeps the in-progress state authoritative:

Suggested change
override func receive(_ action: RegisterPasskeyAction) {
switch action {
case let .displayNameChanged(newValue):
state.displayName = newValue
state.status = .idle
case let .rpIdChanged(newValue):
state.rpId = newValue
state.status = .idle
case let .userNameChanged(newValue):
state.userName = newValue
state.status = .idle
}
}
override func receive(_ action: RegisterPasskeyAction) {
switch action {
case let .displayNameChanged(newValue):
state.displayName = newValue
case let .rpIdChanged(newValue):
state.rpId = newValue
case let .userNameChanged(newValue):
state.userName = newValue
}
if state.status != .inProgress {
state.status = .idle
}
}

@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from e29a4aa to c27001f Compare August 20, 2026 16:47
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from c27001f to ab70204 Compare August 25, 2026 20:41
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from ab70204 to 612b7f9 Compare August 26, 2026 19:08
Adds the "Register Passkey" scenario: a form for relying party
ID, username, and display name that calls PasskeyService.registerPasskey
directly, showing the resulting credential ID and attestation object.
Wires it into the scenario picker and root navigation. Also drops the
dead "Passkey Autofill" scenario stub (route: nil, already unused) and
adds a missing accessibility identifier case for the TOTP scenario
button while touching this file.
@morganzellers-bw
morganzellers-bw force-pushed the pm-27134-testharness-register-passkeys-ui branch from 612b7f9 to 2bd6942 Compare August 27, 2026 16:38
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