Skip to content

PM-33964 - Unify CipherSeeder factories behind CipherSeed domain model.#7330

Merged
theMickster merged 2 commits intomainfrom
seeder/PM-33968-cipher-seed-options
Mar 28, 2026
Merged

PM-33964 - Unify CipherSeeder factories behind CipherSeed domain model.#7330
theMickster merged 2 commits intomainfrom
seeder/PM-33968-cipher-seed-options

Conversation

@theMickster
Copy link
Copy Markdown
Contributor

@theMickster theMickster commented Mar 27, 2026

🎟️ Tracking

PM-33968
PM-33964

📔 Objective

This is a fairly pure code refactoring and reorganization PR.
No new functionality was introduced; all tech debt cleanup to implement a stronger domain model and less duplicative code

Details

  • Introduce CipherSeed domain model — strongly-typed, self-contained plaintext representation of a cipher before encryption, with FromSeedItem() factory for fixture JSON ingestion
  • Unify all 5 CipherSeeder factories behind a single Create(CipherSeed) method, eliminating the dual Create/CreateFromSeed pattern
  • Migrate all 16 call sites (CipherComposer, 5 Scenes, 6 tests, CreateCiphersStep) to construct CipherSeed directly
  • Delete SeedItemMapping.cs — mapping logic now lives on CipherSeed itself where it belongs
  • Add CipherSeed.Validate() for centralized pre-factory validation instead of duplicated null guards across 5 factories
  • Promote Reprompt to a first-class CipherSeed property, closing a silent omission gap where FromSeedItem() dropped the field
  • Fix new Random() → RandomNumberGenerator.Fill() in CreateFido2Credential for cryptographically secure user handle
    generation

@theMickster theMickster added the ai-review-vnext Request a Claude code review using the vNext workflow label Mar 27, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR introduces a CipherSeed domain model that unifies all five cipher seeder factories behind a single Create(CipherSeed) method, replacing the previous dual Create/CreateFromSeed pattern. The refactoring eliminates SeedItemMapping.cs by moving mapping logic into CipherSeed.FromSeedItem(), migrates all 16 call sites consistently, and includes a security improvement replacing new Random() with RandomNumberGenerator.Fill() for FIDO2 credential user handle generation. Behavioral equivalence was verified by tracing the Reprompt and DeletedDate flows through the encryption pipeline.

Code Review Details

No findings. The refactoring preserves existing behavior across all code paths (CipherComposer, Scenes, CreateCiphersStep, and tests). The security fix for cryptographically secure random bytes in CreateFido2Credential is a welcome improvement.

@theMickster theMickster marked this pull request as ready for review March 27, 2026 18:30
@sonarqubecloud
Copy link
Copy Markdown

@github-actions
Copy link
Copy Markdown
Contributor

Logo
Checkmarx One – Scan Summary & Details65404737-e3f5-4da2-b59f-ff1d8ad392c5


Fixed Issues (2) Great job! The following issues were fixed in this Pull Request
Severity Issue Source File / Package
MEDIUM CSRF /src/Api/Billing/Controllers/VNext/AccountBillingVNextController.cs: 55
MEDIUM Use_of_Cryptographically_Weak_PRNG /util/Seeder/Factories/LoginCipherSeeder.cs: 94

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.01%. Comparing base (c5b46cb) to head (955bb9f).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7330      +/-   ##
==========================================
+ Coverage   57.91%   58.01%   +0.09%     
==========================================
  Files        2045     2049       +4     
  Lines       90236    90363     +127     
  Branches     8024     8030       +6     
==========================================
+ Hits        52264    52422     +158     
+ Misses      36105    36077      -28     
+ Partials     1867     1864       -3     

☔ View full report in Codecov by Sentry.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@theMickster theMickster merged commit 6547361 into main Mar 28, 2026
69 of 70 checks passed
@theMickster theMickster deleted the seeder/PM-33968-cipher-seed-options branch March 28, 2026 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review-vnext Request a Claude code review using the vNext workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants