Add passkey and password history to a subset of seeded credentials#7635
Add passkey and password history to a subset of seeded credentials#7635harr1424 wants to merge 1 commit into
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7635 +/- ##
==========================================
+ Coverage 59.82% 59.83% +0.01%
==========================================
Files 2119 2119
Lines 93354 93354
Branches 8282 8282
==========================================
+ Hits 55849 55860 +11
+ Misses 35522 35506 -16
- Partials 1983 1988 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
MGibson1
left a comment
There was a problem hiding this comment.
Nice add! Appreciate the additional test and usage coverage as well
theMickster
left a comment
There was a problem hiding this comment.
@harr1424 One minor thing to double-check and then I am happy to stamp for approval.
In a follow-up PR, will you take a peek with Claude Code into adding similar functionality into the preset fixtures here util/Seeder/Seeds/fixtures? I think we could make some minor adjustments to some of those deterministic ciphers using the same percentage outlined in this PR to kick it up a notch. I recommend pointing Claude to read util/Seeder/Seeds/README.md and the docs in util/Seeder/Seeds/docs + then (while in plan mode) create a plan to add those for you. 😎
| var username = generator.Username.GenerateByIndex(index, totalHint: generator.CipherCount, domain: company.Domain); | ||
|
|
||
| // ~20% of logins get a FIDO2 passkey; ~40% get a 1-3 entry password history. Deterministic by index. | ||
| var fido2Credentials = index % 5 == 0 |
There was a problem hiding this comment.
♻️ A local multi-agent Claude Code review brought up the following finding. I have not dug into the exact specification (might be overkill to do that), but I think Claude may be correct in it's code suggestion. Given that this is a Seeder and not real credentials I'm not sure this is a huge deal, but worth a quick double-check IMO.
FIDO2 credential created with display name as rpId instead of domain
util/Seeder/Factories/CipherComposer.cs:55
Caught by: Bug analysis agent
Details
LoginCipherSeeder.CreateFido2Credential(company.Name, username) passes the company display name (e.g., "Sterling Cooper") as the first argument. CreateFido2Credential sets both RpId = rpName and RpName = rpName, so every seeded FIDO2 credential will have an rpId like "Sterling Cooper" instead of an eTLD+1 domain like "sterling-cooper.com".
The WebAuthn/FIDO2 spec requires rpId to be a valid, registrable domain suffix. Any client-side or test code that performs a WebAuthn assertion against these seeded credentials will fail the rpId check — making every seeded passkey structurally non-functional for passkey testing.
Note the inconsistency with the new integration tests in this same PR, which correctly pass "example.com" (a domain) as the first argument to CreateFido2Credential — those tests pass precisely because they use a domain.
company.Domain is already in scope at the call site:
// Before (line 55):
? new List<Fido2CredentialViewDto> { LoginCipherSeeder.CreateFido2Credential(company.Name, username) }
// After:
? new List<Fido2CredentialViewDto> { LoginCipherSeeder.CreateFido2Credential(company.Domain, username) }
🤖 Bitwarden Claude Code ReviewOverall Assessment: REQUEST CHANGES This PR adds Code Review Details
|



🎟️ Tracking
Inspired by work performed as part of PM-18786.
📔 Objective
This PR adds
fido2CredentialsandpasswordHistoryobjects to a subset of seeded login ciphers in order to better resemble a real-world vault. A single passkey is added to ~20% of logins and password history (1-3 iterations) is added to ~40% of logins.