Skip to content

feat: first-class protected-secret support (DataProtectorExtensions) - #17

Merged
andregoepel merged 2 commits into
mainfrom
feature/protected-secret-settings
Jul 26, 2026
Merged

feat: first-class protected-secret support (DataProtectorExtensions)#17
andregoepel merged 2 commits into
mainfrom
feature/protected-secret-settings

Conversation

@andregoepel

Copy link
Copy Markdown
Owner

Summary

Closes #13

Adds DataProtectorExtensions.ProtectOrKeepExisting, extracting the "protect a new value, or keep the existing ciphertext when the caller left the field blank" round trip that is currently hand-rolled near-identically in three places:

  • app-foundation's MartenEmailSettingsStore (SMTP password)
  • customer-portal's MartenInvoiceNinjaSettingsStore (API token)
  • finance-app's MartenCredentialStore (provider credential)

Design decisions

Shape: an extension method on IDataProtector, not a wrapper value type. I looked at what's actually identical across the three call sites versus what's app-specific:

  • Identical: the branching — !string.IsNullOrEmpty(newPlaintext) ? protector.Protect(newPlaintext) : existingCiphertext.
  • App-specific: what to do when there's neither a new value nor an existing one (MartenEmailSettingsStore fails the save; MartenInvoiceNinjaSettingsStore treats a blank token as a valid state and protects string.Empty; MartenCredentialStore doesn't have this case at all — a secret is always required). So ProtectOrKeepExisting returns null in that case and leaves the decision to the caller.
  • The "unprotect on read" half of the round trip is not wrapped — IDataProtector.Unprotect(string) is already a one-line framework extension method (DataProtectionCommonExtensions). Wrapping a one-liner that already exists would only add indirection, not reduce duplication.

No wrapper value type for the ciphertext. All three call sites store the ciphertext as a plain string field on their document (ProtectedPassword, ProtectedApiToken, ProtectedPayload). Introducing a ProtectedValue struct would mean either changing those field types (out of scope — migration is tracked separately) or the primitive would go unused. Keeping it as a plain string in/out means it can be adopted with a one-line change at each call site with no document/serialization migration.

No new extension point on ISettingsStore/MartenSettingsStore for resolving an IDataProtector. I considered adding one (per-document purpose string → IDataProtector), but IDataProtectionProvider.CreateProtector(string purpose) already is that extension point — it's supplied by the framework and every call site already uses it directly. Adding another abstraction on top would be pure indirection with no reduction in duplication, so I left it out per "don't over-engineer."

Package choice: Microsoft.AspNetCore.DataProtection.Abstractions, not Microsoft.AspNetCore.DataProtection. The extension method only operates on the IDataProtector interface — it never constructs a provider itself (callers get theirs from DI, as they already do). The Abstractions package is a standalone NuGet package (no ASP.NET Core shared-framework dependency), consistent with this repo's "no ASP.NET Core dependency" rule and with how finance-app's AndreGoepel.FinanceApp.Domain project already depends on it. I verified concretely: EphemeralDataProtectionProvider / DataProtectionProvider.Create(...) are not resolvable from Abstractions alone — they require FrameworkReference Include="Microsoft.AspNetCore.App". That's fine, because this library never needs a concrete provider, only the interface.

New test project. Added AndreGoepel.Marten.Configuration.Tests (pure unit tests, no I/O) alongside the existing AndreGoepel.Marten.Configuration.IntegrationTests (Docker/Testcontainers), following the same split used in marten-identity. It's the only project in the solution with FrameworkReference Include="Microsoft.AspNetCore.App" — test-only, to get EphemeralDataProtectionProvider for in-memory round trips — so the packable src project stays framework-free.

Out of scope (tracked separately): migrating app-foundation (app-foundation#155), customer-portal, and finance-app (finance-app#68) onto this primitive. This PR ships the primitive only.

Test plan

  • dotnet csharpier format .
  • dotnet build — 0 warnings, 0 errors
  • dotnet test — all unit tests (6) and integration tests (4) pass
  • dotnet list package --vulnerable --include-transitive — clean

…ttings fields

Extracts the "protect a new value, or keep the existing ciphertext when
the caller left the field blank" round trip that was hand-rolled
near-identically in app-foundation's MartenEmailSettingsStore,
customer-portal's MartenInvoiceNinjaSettingsStore, and finance-app's
MartenCredentialStore. The extension takes the caller's own
IDataProtector, so this library only depends on
Microsoft.AspNetCore.DataProtection.Abstractions (interfaces only), not
the full ASP.NET Core shared framework.

Adds a new AndreGoepel.Marten.Configuration.Tests unit test project
(no I/O, no Docker) alongside the existing IntegrationTests project.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

First-class protected-secret support in settings documents (plan C3)

1 participant