Seeder/secrets manager scenes - #8255
Conversation
Add four SM seeder scenes (project, secret, service account, access policy) that persist real rows via the commercial EF repositories, and wire AddSecretsManagerEfRepositories into SeederApi Startup so the scenes override the OSS Noop registrations. Scenes mirror OrganizationCollectionScene: inject repositories and IManglerService, load and validate the org, encrypt fields under the org key, and return the mangle map. Adds an end-to-end integration test exercising all four scenes through POST /seed and asserting real DB rows with round-tripped encryption.
The Secrets Manager seeder scenes claimed to target an SM-enabled org but never checked Organization.UseSecretsManager, so they could seed impossible fixtures (SM data on an SM-off org), and bad refs surfaced as raw FK 500s. Add a UseSecretsManager guard to OrganizationProjectScene, OrganizationSecretScene, OrganizationServiceAccountScene, and OrganizationAccessPolicyScene. The guard throws InvalidOperationException, which SceneExecutor/SeedController surface as a 400 with a clean message. Add an integration test seeding a non-SM Enterprise org (via OrganizationOverrides.UseSecretsManager=false) and asserting the project scene returns 400 and writes no row.
Extract the duplicated org-load + not-found + Secrets Manager-enabled guard into a GetSecretsManagerOrganizationOrThrowAsync extension on IOrganizationRepository, and call it from the four SM scenes. Keeps the exception messages byte-identical so the BadRequest integration test and HTTP 400 mapping are unaffected.
nthompson-bitwarden
added
the
t:misc
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #8255 +/- ##
==========================================
- Coverage 68.88% 63.19% -5.70%
==========================================
Files 2410 2409 -1
Lines 104442 104393 -49
Branches 9457 9453 -4
==========================================
- Hits 71946 65971 -5975
- Misses 30115 36164 +6049
+ Partials 2381 2258 -123 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the four new Secrets Manager seeder scenes ( Code Review DetailsNo blocking findings. The previously raised ♻️ finding on the unused Dependency Changes
Internal repo project reference, not a third-party dependency — no AppSec review required. Consistent with |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
SeederApi only calls AddSecretsManagerEfRepositories(), which lives in Commercial.Infrastructure.EntityFramework. Nothing in util/SeederApi or test/SeederApi.IntegrationTest uses Bit.Commercial.Core, so the reference only pulled Commercial.Core (and CsvHelper transitively) into the build and container image. Regenerated both packages.lock.json files.
theMickster
left a comment
theMickster
left a comment
There was a problem hiding this comment.
I think we have refactoring work to do in the scenes and a couple things for thought/discussion. Thanks!
| mangleMap: manglerService.GetMangleMap()); | ||
| } | ||
|
|
||
| private static BaseAccessPolicy BuildPolicy(Grant grant) => |
There was a problem hiding this comment.
util/Seeder/Factories folder. Doing this keeps the scene's responsibility to only being the orchestrator and not the creator of seeds, and moving this allows for code reuse by the CLI (or future scenes).
| throw new InvalidOperationException($"Organization {request.OrganizationId} not found."); | ||
| } | ||
|
|
||
| var project = new Project |
There was a problem hiding this comment.
c5966eb#r3860957845
| throw new InvalidOperationException($"Organization {request.OrganizationId} not found."); | ||
| } | ||
|
|
||
| var secret = new Secret |
There was a problem hiding this comment.
c5966eb#r3860957845
| throw new InvalidOperationException($"Organization {request.OrganizationId} not found."); | ||
| } | ||
|
|
||
| var serviceAccount = new ServiceAccount |
There was a problem hiding this comment.
c5966eb#r3860957845
There was a problem hiding this comment.
🎨 I don't love that we have added an extensions helper class to the namespace that contains scenes. I also really don't want to have this in an base class an inheritance coupling 🤮
Where else is a better home for this (and future) extensions?
Secondarily, I am also not convinced at first glance that this must be public; wouldn't marking this as internal be more accurate/applicable? 🧐
| /// Creates a Secrets Manager secret (key/value/note encrypted with the organization's symmetric key) | ||
| /// for an existing Secrets Manager-enabled organization, optionally associating it with projects. | ||
| /// </summary> | ||
| public class OrganizationSecretScene( |
There was a problem hiding this comment.
❓Should we also consider injecting IProjectRepository and perhaps call await projectRepository.ProjectsAreInOrganization(request.ProjectIds.ToList(), organization.Id) to guard that the project is properly bound to the correct organization?
There was a problem hiding this comment.
♻️ A local multi-agent Claude Code review brought to light the following finding. It's verbose, but I think it's correct that we slow down to analyze if/how we properly introduce this project reference into the Seeder and thus into the SeederApi.IntegrationTest test library.
Don't take Claude's word for word on the work to do, but let's be wise and double-check the recommendations.
Commercial project reference and SM repo registration are not gated behind the OSS build guard
`util/SeederApi/SeederApi.csproj:15`
**Caught by:** Code quality agent
This PR adds <ProjectReference Include="..\..\bitwarden_license\src\Commercial.Infrastructure.EntityFramework\Commercial.Infrastructure.EntityFramework.csproj" /> unconditionally, and util/SeederApi/Startup.cs:39 calls services.AddSecretsManagerEfRepositories() unconditionally.
Both established sibling sites gate exactly this dependency:
src/Api/Api.csproj:31-38andsrc/Admin/Admin.csproj:21-28wrap the sameCommercial.Infrastructure.EntityFrameworkreference in<Choose><When Condition="!$(DefineConstants.Contains('OSS'))">.src/Api/Startup.cs:204-212andsrc/Admin/Startup.cs:98-103select betweenservices.AddOosServices()(Noop SM repositories) andservices.AddSecretsManagerEfRepositories()via#if OSS/#else.
Two consequences:
util/SeederApisits in the AGPL portion of the tree and now has a hard, ungated compile-time dependency onbitwarden_licensesource. An OSS-defined build of SeederApi cannot restore or compile.test/SeederApi.IntegrationTestbecomes the first project under./testto transitively referencebitwarden_license(grep -rn bitwarden_license test --include='*.csproj'currently returns nothing), which blurs thedotnet test ./test("OSS solution") vsdotnet test ./bitwarden_license/test("Bitwarden solution") split in.github/workflows/test.yml:52-56.
Suggested fix: mirror the sibling pattern — move the ProjectReference into a <Choose><When Condition="!$(DefineConstants.Contains('OSS'))"> block and wrap the AddSecretsManagerEfRepositories() call in #if OSS / #else, taking services.AddOosServices() on the OSS side.
Note the fix is not purely mechanical: AddOosServices() (src/SharedWeb/Utilities/ServiceCollectionExtensions.cs:386-394) registers Noops for IProjectRepository, ISecretRepository, IServiceAccountRepository and ISecretVersionRepository, but not IAccessPolicyRepository. Since SeederApi's default host builder validates the container on build and AddScenes() registers every scene's concrete type, OrganizationAccessPolicyScene would fail startup validation on the OSS side unless a Noop IAccessPolicyRepository is added too. That gap should be resolved as part of the gating rather than sidestepped by leaving the dependency ungated.
2 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/QA-2400
📔 Objective
Add Secrets Manager seeder scenes to the SeederApi so tests and local setups can persist real SM fixtures (projects, secrets, service accounts, access policies) against an SM-enabled org.
What changed:
OrganizationProjectScene,OrganizationSecretScene,OrganizationServiceAccountScene, andOrganizationAccessPolicyScene— that persist real rows via the commercial EF repositories. Each mirrorsOrganizationCollectionScene: inject repositories andIManglerService, load and validate the org, encrypt fields under the org key, and return the mangle map.AddSecretsManagerEfRepositoriesinto SeederApiStartupso the scenes override the OSS Noop registrations.Organization.UseSecretsManager, so they could seed impossible fixtures (SM data on an SM-off org) and bad refs surfaced as raw FK 500s. The guard throwsInvalidOperationException, whichSceneExecutor/SeedControllersurface as a clean 400.GetSecretsManagerOrganizationOrThrowAsyncextension onIOrganizationRepository, called by all four scenes. Exception messages are kept byte-identical so the BadRequest integration test and HTTP 400 mapping are unaffected.Tests:
POST /seed, asserting real DB rows with round-tripped encryption.OrganizationOverrides.UseSecretsManager=false) asserting the project scene returns 400 and writes no row.