PM-2035: PRF Unlock (web + extension)

[abergs]

🎟️ Tracking

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

📔 Objective

This PR introduces PRF powered unlock in our web + browser clients.

Here's a summary of the changes:

• PRF Decryption Options for all the users passkeys are included in the server sync response and stored. This allows both offline unlock, but also allows for unlock via any passkey, regardless of method of login.
• New KM owned webauthn-prf-unlock.service.ts. Internally it forwards a few calls to the existing WebAuthnLoginPrfKeyServiceAbstraction.

This PR is dependant on the server changes proposed in bitwarden/server#6401.

📸 Screenshots

Demo recording: https://share.cleanshot.com/tf9QTgBb

⏰ 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

[github-actions]

Checkmarx One – Scan Summary & Details – 7af9f86c-edee-4a2e-aec2-76e4830882f9

Fixed Issues (64)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	CVE-2024-40643	Npm-htmlparser2-3.10.1
	CVE-2025-12432	Npm-electron-37.7.0
	CVE-2025-12433	Npm-electron-37.7.0
	CVE-2025-12436	Npm-electron-37.7.0
	CVE-2025-7783	Npm-form-data-3.0.3
	CVE-2025-11205	Npm-electron-37.7.0
	CVE-2025-11206	Npm-electron-37.7.0
	CVE-2025-11209	Npm-electron-37.7.0
	CVE-2025-11458	Npm-electron-37.7.0
	CVE-2025-11460	Npm-electron-37.7.0
	CVE-2025-11756	Npm-electron-37.7.0
	CVE-2025-12036	Npm-electron-37.7.0
	CVE-2025-12428	Npm-electron-37.7.0
	CVE-2025-12429	Npm-electron-37.7.0
	CVE-2025-12430	Npm-electron-37.7.0
	CVE-2025-12437	Npm-electron-37.7.0
	CVE-2025-12438	Npm-electron-37.7.0
	CVE-2025-12727	Npm-electron-37.7.0
	CVE-2025-12907	Npm-electron-37.7.0
	CVE-2025-13226	Npm-electron-37.7.0
	CVE-2025-13227	Npm-electron-37.7.0
	CVE-2025-30360	Npm-webpack-dev-server-5.2.0
	CVE-2025-59343	Npm-tar-fs-2.1.3
	CVE-2025-64756	Npm-glob-10.4.5
	CVE-2025-64756	Npm-glob-11.0.3
	Cx39aef355-ca85	Npm-@eslint/plugin-kit-0.2.8
	Cxdca8e59f-8bfe	Npm-inflight-1.0.6
	CVE-2025-11207	Npm-electron-37.7.0
	CVE-2025-11208	Npm-electron-37.7.0
	CVE-2025-11210	Npm-electron-37.7.0
	CVE-2025-11211	Npm-electron-37.7.0
	CVE-2025-11212	Npm-electron-37.7.0
	CVE-2025-11213	Npm-electron-37.7.0
	CVE-2025-11215	Npm-electron-37.7.0
	CVE-2025-11216	Npm-electron-37.7.0
	CVE-2025-12431	Npm-electron-37.7.0
	CVE-2025-12435	Npm-electron-37.7.0
	CVE-2025-12439	Npm-electron-37.7.0
	CVE-2025-12440	Npm-electron-37.7.0
	CVE-2025-12443	Npm-electron-37.7.0
	CVE-2025-12444	Npm-electron-37.7.0
	CVE-2025-12445	Npm-electron-37.7.0
	CVE-2025-12446	Npm-electron-37.7.0
	CVE-2025-12447	Npm-electron-37.7.0
	CVE-2025-12728	Npm-electron-37.7.0
	CVE-2025-12729	Npm-electron-37.7.0
	CVE-2025-12905	Npm-electron-37.7.0
	CVE-2025-12906	Npm-electron-37.7.0
	CVE-2025-30359	Npm-webpack-dev-server-5.2.0
	CVE-2025-54798	Npm-tmp-0.2.3
	CVE-2025-54798	Npm-tmp-0.0.33
	CVE-2025-59288	Npm-playwright-1.53.1
	CVE-2025-62522	Npm-vite-6.3.5
	CVE-2025-62522	Npm-vite-6.2.7
	CVE-2025-62595	Npm-koa-2.16.1
	CVE-2025-8129	Npm-koa-2.16.1
	CVE-2025-11219	Npm-electron-37.7.0
	CVE-2025-58751	Npm-vite-6.3.5
	CVE-2025-58751	Npm-vite-6.2.7
	CVE-2025-58752	Npm-vite-6.2.7
	CVE-2025-7339	Npm-on-headers-1.0.2
	Cx8bc4df28-fcf5	Npm-debug-2.6.9
	Cx8bc4df28-fcf5	Npm-debug-3.2.7
	Cxda14f253-4e52	Npm-bluebird-3.7.2

[quexten]

While this is still in draft, I've put some early comments since it looks interesting.

A general question: Is the intended flow here that users set up their credentials as login credentials, but then they also happen to be able to unlock with it? (I.e the use case of "i want my credential to be only usable for unlock" is one we don't care about?)

[quexten]

Non blocking for this PR but this should be abstracted away to the "rotateableKeySet" concept we use here. KM does not have an API and I don't expect it to be built in this PR but this needs cleaning up. We should probably drop a comment.

[abergs]

question: There is a slight race condition here. We check the activeAccount on row 423 and try to act only for it. However, since the setUserDecryptionOptions acts on the current account instead of accepting a userId, there is a theoretical risk that the account is changed between the check and this operation?

[abergs]

block: Auth will change setUserDecryptionOptions to accept a UserId to eliminate this race.

[abergs]

@bitwarden/team-auth-dev Did you get around to refactor setUserDecryptionOptions to accept a userid?

[abergs]

Implemented here: #17069 (pull on main)

[abergs]

Also implemented here (Plan a is to have this merge and rebase): #16894

[abergs]

A general question: Is the intended flow here that users set up their credentials as login credentials, but then they also happen to be able to unlock with it?

Yes

(I.e the use case of "i want my credential to be only usable for unlock" is one we don't care about?)

I do care about that use case (quite a lot actually) -- but it will have to come next and is likely not impacting the unlock flow, just the login flow. (I'm imagining a checkbox that says 'Unlock only' and we just check that flag when the user tries to login).

[quexten]

Note for auth reviewers:

After brief discussion with @abergs the approach here seems reasonable and secure. It has yet to be discussed within KM, however it is essentially not that much different from our existing approach for masterpassword unlock which also locally caches the login decryption data for local unlock. In this case we are caching more data, but the data is only useful with the correct PRF key.

It has some implications on how some future KM work will play out, such as key rotation without logout, but nothing that is blocking.

[pamperer562580892423]

As only a user, I would like to throw in something from the user's perspective. Personally I really long for that feature BTW, to make that clear from the beginning!

But I do see a growing confusion to just using the term "Unlock with passkey" (on the button etc.).

For a longer time now, I think it is already confusing enough to both call "login-with-passkeys"-passkeys and "FIDO2-2FA-passkeys" both just "passkeys". (at one point I thought, it would already be better to call them "login-passkey" and "2FA-passkey" to have some differentiation here)

And I can already see users trying to unlock their vault with Bitwarden-2FA-"passkeys" (which BTW in the web vault, when you set them up, are also just called the "passkey" option), getting confused and desperate why it doesn't work. (as I understand this PR, only "login-with-passkey"-passkeys with encryption / PRF will work for unlocking?!)

I do get the general idea to make it easy for users to just call both (login and 2FA) passkeys and be done with it. But I really think it creates more confusion for users, when two things that work differently and in different places (login / 2FA / unlocking) are all called the same...

Sorry for the intrusion and "rant". 😉

(PS: I added "FIDO2" to the 2FA option as it was and still is also called the FIDO2-2FA option... of course "login-with-passkey" - or rather "passkeys" in general - are also FIDO2.)

[abergs]

@pamperer562580892423 I hear where you are coming from.

when you set them up, are also just called the "passkey" option

We'll be looking to improve the UI where users register their passkeys, to more easily understand and control what capabilities each key has.

The work will however take place in a separate PR.

[pamperer562580892423]

@pamperer562580892423 I hear where you are coming from.

👍

when you set them up, are also just called the "passkey" option

We'll be looking to improve the UI where users register their passkeys, to more easily understand and control what capabilities each key has.

Very good! Though, it's also difficult for users to remember those kind of things e.g. a few months later...

If I may, I would suggest to also include something like "Unlock with passkey doesn't work with "2FA-passkeys". It only works with "login-passkeys" with encryption." in that error message here:

 

That way, users automatically see why it doesn't work if they try to use a 2FA-passkey...

The work will however take place in a separate PR.

Thanks for your response! 🤗

[quexten]

Some more smaller things

[quexten]

KM code lgtm, however the active userid issue on setDecryptionOptions is concerning, so please fix / work with auth to fix before merging.

[Thomas-Avery]

The feedback addressed looks good. Two things, one open piece around missing things in setUserKeyAndContinue and an FYI of incoming eslint warnings from main.

[claude]

Claude finished @abergs's task —— View job

---

PR Review: PRF Unlock Implementation

I've completed a comprehensive review of the latest changes. Here's my analysis:

---

Summary of Changes Since Last Review

The PR has been significantly refined based on previous feedback. Key updates include:

• Moved WebAuthnPrfUnlockService implementation to key-management-ui package
• Refactored unlock flow to return UserKey instead of setting it directly
• Converted to signal-based inputs/outputs in Angular components
• Removed unnecessary try-catch blocks to allow proper error propagation
• Fixed import issues and relative imports
• Updated sync service to handle PRF options from server responses

---

Critical Issues Found

1. Missing Unit Tests ⚠️

Severity: High

Location: libs/key-management-ui/src/lock/services/default-webauthn-prf-unlock.service.ts

Issue: No test file exists for the core PRF unlock service. This is a security-critical service handling cryptographic operations and should have comprehensive test coverage.

Impact:

• Codecov reports 27.96% patch coverage with 152 lines missing coverage
• The default-webauthn-prf-unlock.service.ts has 0% coverage (91 lines missing)
• No validation of error handling paths
• No testing of edge cases in cryptographic operations

Recommendation:
Create default-webauthn-prf-unlock.service.spec.ts with tests covering:

• Browser capability detection (Chromium check, WebAuthn support)
• Credential retrieval and validation
• PRF key derivation and decryption flow
• Error handling for various failure scenarios
• Mock WebAuthn API responses

---

2. Race Condition in Sync Service ⚠️

Severity: Medium

Location: libs/common/src/platform/sync/default-sync.service.ts:446-478

Issue: The active account check creates a potential race condition. The service checks if activeAccount.id === userId before updating PRF options, but the check happens asynchronously and the account could change between the check and the update.

// Get current at line 450
const activeAccount = await firstValueFrom(this.accountService.activeAccount$);

if (activeAccount?.id !== userId) {
  return;  // Guard clause
}

// But account could change before this line executes at 473
await this.userDecryptionOptionsService.setUserDecryptionOptions(updatedOptions);

Additional concern: The code silently returns without updating if not the active user. This could lead to sync data being lost if users switch accounts during sync.

Recommendation:

• Verify with auth team if setUserDecryptionOptions now accepts a userId parameter (there's a comment suggesting this was planned)
• If so, update to pass userId directly
• If not, add a lock/mutex mechanism or re-verify active user just before setting options

---

3. Incomplete Error Messages 📝

Severity: Low-Medium

Location: libs/key-management-ui/src/lock/components/unlock-via-prf.component.ts:89-102

Issue: Error handling only checks for two specific cases but uses a generic "unexpectedError" message for all other failures. Users won't understand why unlock failed.

if (error.message.includes("No PRF credentials")) {
  errorMessage = this.i18nService.t("noPrfCredentialsAvailable");
} else if (error.message.includes("canceled")) {
  // User canceled - don't show error
  this.unlocking = false;
  return;
}
// Falls through to generic "unexpectedError"

Recommendation:
Add more specific error messages for:

• WebAuthn not supported in browser
• Authenticator doesn't support PRF
• Decryption failures
• Network/sync issues

Consider using error codes/types instead of string matching for more reliable error handling.

---

4. Missing Browser Compatibility Check in Component ℹ️

Severity: Low

Location: libs/key-management-ui/src/lock/components/unlock-via-prf.component.ts

Issue: The component calls isPrfUnlockAvailable() which handles browser compatibility, but there's no user-facing messaging if PRF is unavailable due to browser limitations.

Impact: Users on non-Chromium browsers (Firefox, Safari in extensions) will see no button and no explanation why.

Recommendation:
Consider adding a help text or tooltip explaining PRF requirements when it's unavailable, similar to how biometrics displays biometricUnavailabilityReason.

---

Security Concerns

✅ Encryption Handling (Looks Good)

• No new encryption logic added (as per repo rules)
• Properly uses existing encryptService methods
• Follows the established PRF → private key → user key decryption chain
• No vault data logged in error messages

✅ Credential Storage (Looks Good)

• PRF decryption options stored encrypted from server
• Credential IDs properly base64url decoded for WebAuthn API
• Uses same salt as login flow for consistency

⚠️ Input Validation

Location: default-webauthn-prf-unlock.service.ts:143-149

The code decodes credential IDs without validation:

const decodedId = Fido2Utils.stringToBuffer(credentialId);

Recommendation: Consider adding try-catch around the decode operation in case malformed data is stored.

---

Code Quality & Best Practices

Good Practices Observed 👍

• Signal-based inputs/outputs follow ADR-0027
• Proper use of Observable patterns with firstValueFrom
• Appropriate error logging with context
• Clean separation of concerns (service vs component)
• Follows DI patterns with safeProvider

Issues & Improvements

1. TypeScript any Usage

Location: default-webauthn-prf-unlock.service.ts:182

const extensionResults = response.getClientExtensionResults() as any;

There's a TODO comment acknowledging this. While acceptable temporarily, ensure the tech debt ticket (PM-27831) is tracked.

---

2. Inconsistent Change Detection Strategy ⚠️

Location: unlock-via-prf.component.ts:15

// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
@Component({
  selector: "bit-unlock-via-prf",
  ...
})

Issue: The component explicitly disables OnPush change detection but uses signals (which work best with OnPush). This creates inconsistency with Angular best practices.

Recommendation:

• Either refactor to support OnPush (preferred)
• Or document why default change detection is required

---

3. Mixed Template Syntax

Location: lock.component.html:7-217

The template mixes old *ngIf syntax with new @if control flow:

<ng-container *ngIf="unlockOptions && !loading; else spinner">
  ...
  @if (unlockWithMasterPasswordUnlockDataFlag$ | async) {

While both work, consistency is preferable. Per repo guidelines (ADR), new code should use control flow syntax (@if, @for).

Recommendation: Convert remaining *ngIf/*ngFor to @if/@for for consistency (can be a follow-up task).

---

4. Empty Challenge in WebAuthn Request

Location: default-webauthn-prf-unlock.service.ts:140

challenge: new Uint8Array(32),  // Empty array

Observation: The challenge is all zeros. While this works for unlock (since we're only using PRF, not validating the signature), it might confuse security auditors.

Recommendation: Add a comment explaining why an empty challenge is acceptable for PRF-only operations:

challenge: new Uint8Array(32), // Empty challenge is acceptable for PRF unlock (we're not validating the assertion signature)

---

5. Polling for Unlock Options

Location: lock.component.ts:205-218

interval(1000)
  .pipe(
    mergeMap(async () => {
      if (this.activeAccount?.id != null) {
        this.unlockOptions = await firstValueFrom(
          this.lockComponentService.getAvailableUnlockOptions$(this.activeAccount.id),
        );
      }
    }),

Issue: Polling every second seems excessive and could impact performance.

Question: What's the use case for this? Are unlock options expected to change while on the lock screen?

Recommendation: If options don't change dynamically, remove the polling. If they do, consider using a push-based approach or increase the interval.

---

Repository Standards Compliance

✅ Followed

• No TypeScript enums used (ADR-0025)
• Observable patterns used correctly (ADR-0003)
• Signals used appropriately in Angular components (ADR-0027)
• No code regions
• Proper eslint disable comments with context
• Relative imports used correctly
• DI configured properly via safeProvider

⚠️ Needs Attention

• Test coverage significantly below project standards
• Mixed template syntax (old/new control flow)
• Change detection strategy inconsistency

---

Previously Identified Issues - Status Check

Issues from Past Reviews (Click to expand)

✅ RESOLVED

1. Constructor DI vs property DI - ✅ Resolved (removed @Injectable decorator as suggested)
2. Opaque UserId types - ✅ Resolved (using UserId type correctly)
3. Naming consistency (passkey vs webauthn) - ✅ Resolved
4. Service location (key-management vs key-management-ui) - ✅ Resolved (moved to key-management-ui)
5. Return UserKey pattern - ✅ Resolved (now returns UserKey instead of setting directly)
6. Signal inputs/outputs - ✅ Resolved (converted to signal-based API)
7. Empty challenge (random vs null) - ✅ Resolved (using empty array)
8. Error handling in unlockVaultWithPrf - ✅ Resolved (removed extra try-catch)
9. Control flow syntax - ✅ Resolved (using @if/@for in new component)
10. PRF passkey icon - ✅ Resolved (icon added)
11. Relative imports in sync service - ✅ Resolved (line 19 now relative)

⏳ PENDING/IN PROGRESS

1. setUserDecryptionOptions race condition - ⏳ Auth team working on userId parameter (PR Refactor setUserDecryptionOptions to accept UserId #17069, [PM-26413] Remove ActiveUserState from UserDecryptionOptionsService #16894)
2. Tech debt for SDK PRF implementation - ⏳ Tracked in Jira (see comment by quexten)
3. webAuthnLoginPrfkeyService rename suggestion - ⏳ Non-blocking (suggested by quexten)

❌ NEW ISSUES IDENTIFIED

1. Missing unit tests (critical)
2. Incomplete error messages
3. Missing browser compatibility messaging for users
4. Input validation for credential IDs

---

Action Items for Author

Must Fix Before Merge

1. ❌ Add unit tests for DefaultWebAuthnPrfUnlockService to improve code coverage
2. ⚠️ Verify sync service race condition - Check with auth team if setUserDecryptionOptions now accepts userId

Should Fix

3. 📝 Enhance error messages - Add specific error types/codes for better user experience
4. 🛡️ Add input validation for credential ID decoding
5. 📖 Add comment explaining empty challenge in WebAuthn request

Nice to Have

6. 💭 Review polling interval - Assess if 1-second polling is necessary
7. 🎨 Consider OnPush change detection for unlock-via-prf component
8. 📝 Add user-facing messaging for browser compatibility (like biometrics does)

---

Overall Assessment

Status: 🟡 Nearly Ready - Needs test coverage and minor fixes

The implementation is well-structured and follows the repository's architectural patterns. The core cryptographic flow is sound and properly delegates to existing services. However, test coverage is critically low and must be addressed before merge.

The PR successfully addresses most previous review feedback, demonstrating good iteration and collaboration with reviewers.

Strengths

• Clean service architecture with proper separation of concerns
• Follows established cryptographic patterns
• Good error handling structure
• Proper Angular patterns (signals, observables, DI)
• Security-conscious implementation

Remaining Concerns

• Inadequate test coverage (27.96% patch coverage)
• Sync service race condition (pending auth team fix)
• User experience could be improved with better error messages

---

Branch: anders/unlock-prf-3
Files Changed: 28 (+782/-71)
Commits: 50

[Patrick-Pimentel-Bitwarden]

👍 Auth changes are great! Thank you. I see you have other comments pending so I'll watch to reapprove

[abergs]

@claude you probably need to update your review.

[Patrick-Pimentel-Bitwarden]

👍 Auth changes are great! Thank you

[Patrick-Pimentel-Bitwarden]

👍 Auth changes look good.