clients PM-8353 MacOS passkey provider

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-8353

📔 Objective

This PR serves as the long running feature branch for passkey provider support.

📸 Screenshots

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

Mar 24 '25 12:03 abergs

Codecov Report

:x: Patch coverage is 34.21053% with 300 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 41.70%. Comparing base (4a4ce83) to head (3bebe0a). :warning: Report is 3 commits behind head on main. :white_check_mark: All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
.../src/autofill/services/desktop-autofill.service.ts	2.24%	87 Missing :warning:
...l/services/desktop-fido2-user-interface.service.ts	0.00%	59 Missing :warning:
...s/desktop/desktop_native/macos_provider/src/lib.rs	0.00%	38 Missing :warning:
...src/platform/main/autofill/native-autofill.main.ts	0.00%	27 Missing :warning:
apps/desktop/desktop_native/napi/src/lib.rs	0.00%	19 Missing :warning:
...esktop/src/autofill/guards/reactive-vault-guard.ts	0.00%	19 Missing :warning:
...tofill/modal/credentials/fido2-create.component.ts	77.64%	18 Missing and 1 partial :warning:
...utofill/modal/credentials/fido2-vault.component.ts	86.66%	7 Missing and 1 partial :warning:
apps/desktop/src/platform/popup-modal-styles.ts	0.00%	6 Missing :warning:
apps/desktop/src/app/app-routing.module.ts	0.00%	4 Missing :warning:
... and 7 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13963      +/-   ##
==========================================
+ Coverage   41.66%   41.70%   +0.03%     
==========================================
  Files        3556     3559       +3     
  Lines      102355   102685     +330     
  Branches    15355    15399      +44     
==========================================
+ Hits        42646    42824     +178     
- Misses      57912    58061     +149     
- Partials     1797     1800       +3

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

:package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Mar 24 '25 12:03 codecov[bot]

Checkmarx One – Scan Summary & Details – 92b34311-f23f-47f2-99f7-195a0dc304c1

Great job! No new security vulnerabilities introduced in this pull request

Mar 24 '25 12:03 github-actions[bot]

⚠️ Files have been modified in this PR that impact the Autofill experience ⚠️

BIT was run to verify no regressions have been introduced to the core Autofill experience. The tests ran with the same feature flag configuration used by vault.bitwarden.com

❌ Unfortunately, one or more of these BIT tests failed. 😞

Please resolve the failure before merging; reach out to @bitwarden/team-autofill-dev if you'd like help.

You can view the detailed results of the tests here.

Aug 13 '25 17:08 bw-ghapp[bot]

⚠️ Files have been modified in this PR that impact the Autofill experience ⚠️

BIT was run to verify no regressions have been introduced to the core Autofill experience. The tests ran with all feature flags disabled.

❌ Unfortunately, one or more of these BIT tests failed. 😞

Please resolve the failure before merging; reach out to @bitwarden/team-autofill-dev if you'd like help.

You can view the detailed results of the tests here.

Aug 13 '25 17:08 bw-ghapp[bot]

@dani-garcia We can detect if we're running the MAS or not right? I'm wondering if we should disable the feature / IPC connection if we're not on MAS since we may get a permission prompt otherwise?

Aug 28 '25 12:08 abergs

@abergs: @dani-garcia We can detect if we're running the MAS or not right? I'm wondering if we should disable the feature / IPC connection if we're not on MAS since we may get a permission prompt otherwise?

That sounds reasonable yeah, we can check for MAS builds:

In renderer: you can use PlatformUtilsService.isMacAppStore()
In main: there is a utils file in src/main that also has a isMacAppStore()

Aug 28 '25 12:08 dani-garcia

@jholland-livefront Are the blocking reviews from @coroiu and @quexten resolved? If so, would you please re-request their review?

Aug 28 '25 12:08 abergs

@jholland-livefront There are some tests that fail, I think mainly in the fido2 create component. Could you make sure we're green on tests?

https://github.com/bitwarden/clients/actions/runs/17207515957/job/48811272770?pr=13963#step:8:376

Aug 28 '25 13:08 abergs

Claude finished @abergs's task —— View job

Code Review Complete

I've completed a comprehensive review of this large PR adding MacOS passkey provider support. Overall, the implementation is solid with good test coverage for the new components. Below are my findings organized by severity.

Findings Summary

Finding 1: PR description could be more detailed (💭)

The PR description provides good context with links to documentation, but could benefit from:

A brief description of the technical approach (IPC architecture, Rust/Swift integration)
Summary of key files changed
Known limitations or future work items

This would help future reviewers understand the implementation faster.

Finding 2: Type safety issue with any type in session (⚠️)

In apps/desktop/src/autofill/services/desktop-fido2-user-interface.service.ts:58, currentSession is typed as any. This should be DesktopFido2UserInterfaceSession | undefined for type safety. The any type bypasses TypeScript's type checking.

Finding 3: Missing null safety check in Swift (⚠️)

In apps/desktop/macos/autofill-extension/CredentialProviderViewController.swift:96 and line 120, force-unwrapping newClient! could crash if the connection fails after all retries. See inline comment for suggested guard statement pattern.

Finding 4: Inconsistent initialization pattern (💭)

In apps/desktop/src/autofill/modal/credentials/fido2-create.component.ts:56 and fido2-vault.component.ts:62, the optional type ? combined with explicit = null is redundant. See inline comment for consistent alternatives.

Finding 5: Magic number without full context (🎨)

In apps/desktop/src/autofill/services/desktop-autofill.service.ts:451, the 100-pixel offset has a comment but could explain why 100 specifically. Also contains a typo: xPostionOffset → xPositionOffset. See inline comment.

Finding 6: Error handling loses context (🎨)

In apps/desktop/src/autofill/services/desktop-fido2-user-interface.service.ts:310-312, catching and rethrowing a generic error loses the original error context. Consider logging and preserving the original error message.

Finding 7: Potential race condition in connection monitoring (💭)

In apps/desktop/macos/autofill-extension/CredentialProviderViewController.swift:108-134, the checkConnectionStatus() method accesses self.client which could be modified by the async getClient() method. Consider using proper synchronization.

Finding 8: Missing test coverage for service methods (⚠️)

Based on the Codecov report showing 34.21% patch coverage, several critical service methods lack tests:

desktop-autofill.service.ts (2.24% coverage)
desktop-fido2-user-interface.service.ts (0.00% coverage)
native-autofill.main.ts (0.00% coverage)

While complex integration code can be difficult to test, consider adding unit tests for error handling paths, message buffering logic, and IPC communication edge cases.

Finding 9: Rust unwrap usage needs addressing (❌)

In apps/desktop/desktop_native/macos_provider/src/lib.rs:96, there's a FIXME comment acknowledging unwrap usage that could panic. While some unwraps were converted to expect() with messages (good progress!), the FIXME should be resolved before merging. See inline comment for specific lines.

Finding 10: Subscription cleanup could use newer pattern (🎨)

In apps/desktop/src/autofill/modal/credentials/fido2-vault.component.ts:146, the subscription uses manual takeUntil(this.destroy$). Consider using takeUntilDestroyed() from Angular 16+ per ADR-0003 shown in CLAUDE.md. See inline comment for example.

Positive Observations

👍 Good use of standalone components following Angular best practices
👍 Comprehensive test suites for the new modal components
👍 Proper use of Tailwind tw-app-region-* classes throughout
👍 Clear separation of concerns between Rust IPC layer, TypeScript services, and Angular UI
👍 Good error messaging for users with i18n support
👍 Thoughtful async connection retry logic in Swift

Recommendations

Before merging: Address Finding 9 (Rust unwraps) as marked by the FIXME comment
High priority: Fix Finding 3 (Swift force unwrapping) - could cause production crashes
Medium priority: Improve test coverage for core service methods (Finding 8)
Nice to have: Address the code style improvements (Findings 4, 5, 6, 10)