From 5f3eda2680c5b35fb062226be510cd21160b5e8d Mon Sep 17 00:00:00 2001 From: Sluijsens Date: Tue, 28 Jul 2026 00:00:45 +0200 Subject: [PATCH] Makes a redeploy safe for the key ring and the schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing here is visible in normal operation. Its whole purpose is that swapping the release directory on deploy cannot silently destroy state. Data Protection secures the API keys that authenticate master/slave communication. Two separate defaults would each have destroyed them: keys are held on the filesystem, which a release swap discards, and the application discriminator is derived from the content root path, which changes with every release directory — so even keys stored in a database would have stopped being derivable. Keys now live in ApplicationDbContext and the discriminator is a fixed constant. Losing them produces no error. It produces stored keys that no longer decrypt, which presents as an apparent network fault between a Master and its slaves and is easily misdiagnosed. That is also why the tests assert the resulting configuration rather than the registration: the XmlRepository must be the EF one and the discriminator must be the constant, plus a round-trip proving a value encrypted before a deploy is readable after one. A test that only checked "Data Protection is registered" would have passed in the broken case too. Both modules previously called AddDataProtection() themselves. Module registration runs after the host's, so those calls re-registered the configuration chain and would have overridden the persistent store while IDataProtector still resolved. They are removed, with a comment at each site — the deletion otherwise looks like a regression. Each module's own test project now guards against it being reintroduced. ApplicationDbContext also migrates itself at startup. Deploy targets offer no CLI, so migrations cannot be a manual step on the server. Failures are classified rather than treated alike: a connection failure means the database is not up yet, normal when the app and the database start together after a reboot, and is retried with backoff; a migration failure means something is broken and fails at once. Either way the process does not start, which is what makes the liveness health check trustworthy — an application that cannot reach its schema never answers /health, so monitoring goes red instead of reporting a healthy instance that cannot serve a request. The cost of migrating without a human gate is that migrations must stay forward-compatible and non-destructive, since rollback is "redeploy the previous release". The new migration is purely additive. Also wires this and the preceding hosting commit into both hosts, as they touch the same lines of Program.cs. Two constraints are enforced by documentation rather than code, and belong in the deployment instructions: the key table must never be pruned, and only one instance may migrate a given database at a time. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HHoJpxYXzHACSQguHrC5fw --- ...u2-data-durability-code-generation-plan.md | 145 ++++++ .../plans/u2-data-durability-fd-questions.md | 76 +++ ...-data-durability-functional-design-plan.md | 42 ++ .../code/generation-summary.md | 87 ++++ .../functional-design/business-logic-model.md | 194 ++++++++ .../functional-design/business-rules.md | 130 +++++ .../functional-design/domain-entities.md | 124 +++++ src/SlpModularCms.Api.Slave/Program.cs | 13 + src/SlpModularCms.Api/Program.cs | 31 +- .../Hosting/DataProtectionExtensionsTests.cs | 97 ++++ .../DatabaseMigrationExtensionsTests.cs | 95 ++++ .../Data/ApplicationDbContext.cs | 15 +- .../Hosting/DataProtectionExtensions.cs | 58 +++ .../Hosting/DatabaseMigrationExtensions.cs | 112 +++++ ...27203036_AddDataProtectionKeys.Designer.cs | 452 ++++++++++++++++++ .../20260727203036_AddDataProtectionKeys.cs | 35 ++ .../ApplicationDbContextModelSnapshot.cs | 21 +- .../SlpModularCms.Core.csproj | 7 + .../AvailabilityModuleDataProtectionTests.cs | 52 ++ .../AvailabilityModule.cs | 7 +- .../MasterModuleDataProtectionTests.cs | 45 ++ .../MasterModule.cs | 5 +- 22 files changed, 1833 insertions(+), 10 deletions(-) create mode 100644 aidlc-docs/features/gitea-deployment-workflow/construction/plans/u2-data-durability-code-generation-plan.md create mode 100644 aidlc-docs/features/gitea-deployment-workflow/construction/plans/u2-data-durability-fd-questions.md create mode 100644 aidlc-docs/features/gitea-deployment-workflow/construction/plans/u2-data-durability-functional-design-plan.md create mode 100644 aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/code/generation-summary.md create mode 100644 aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/functional-design/business-logic-model.md create mode 100644 aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/functional-design/business-rules.md create mode 100644 aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/functional-design/domain-entities.md create mode 100644 src/SlpModularCms.Core.Tests/Hosting/DataProtectionExtensionsTests.cs create mode 100644 src/SlpModularCms.Core.Tests/Hosting/DatabaseMigrationExtensionsTests.cs create mode 100644 src/SlpModularCms.Core/Hosting/DataProtectionExtensions.cs create mode 100644 src/SlpModularCms.Core/Hosting/DatabaseMigrationExtensions.cs create mode 100644 src/SlpModularCms.Core/Migrations/20260727203036_AddDataProtectionKeys.Designer.cs create mode 100644 src/SlpModularCms.Core/Migrations/20260727203036_AddDataProtectionKeys.cs create mode 100644 src/SlpModularCms.Modules.Availability.Tests/AvailabilityModuleDataProtectionTests.cs create mode 100644 src/SlpModularCms.Modules.Master.Tests/MasterModuleDataProtectionTests.cs diff --git a/aidlc-docs/features/gitea-deployment-workflow/construction/plans/u2-data-durability-code-generation-plan.md b/aidlc-docs/features/gitea-deployment-workflow/construction/plans/u2-data-durability-code-generation-plan.md new file mode 100644 index 0000000..fb5eeff --- /dev/null +++ b/aidlc-docs/features/gitea-deployment-workflow/construction/plans/u2-data-durability-code-generation-plan.md @@ -0,0 +1,145 @@ +# Code Generation Plan — U2 Data Durability + +**This plan is the single source of truth for Code Generation of U2.** Generation executes exactly these steps in order; no step is added or skipped during execution. + +--- + +## Unit Context + +| Aspect | Detail | +|---|---| +| **Unit** | U2 Data Durability | +| **Round** | R1 (with U1 Hosting & Serving) | +| **Workspace root** | `K:\Development\Projects\SlpModularCms` | +| **Project type** | Brownfield — existing structure retained, files modified in place | +| **Requirements** | FR-11, FR-12 | +| **Components** | C-05 Data Protection, C-06 keys table, C-07 migration runner, U2 portion of C-16 | +| **Business rules** | BR-U2-01 … BR-U2-18 | +| **Depends on** | Nothing. U1 and U2 are mutually independent | +| **Depended on by** | U6 — durability must land before the first automated deploy | +| **New database entities** | One: the Data Protection keys table, owned by `ApplicationDbContext` | + +### Requirement traceability + +| Requirement | Implemented by steps | +|---|---| +| FR-11 — automatic `ApplicationDbContext` migration at startup | 4, 5, 6 | +| FR-12 — persistent Data Protection key ring | 1, 2, 3, 5, 6 | + +### Why this unit exists + +Nothing here is user-visible. Its whole purpose is that U6's atomic release switch **cannot** silently destroy schema state or Master↔slave trust. Two failure modes are being closed, both of which would otherwise appear only after the first production deploy and present as something else entirely. + +--- + +## Generation Steps + +### Step 1: Package reference +- [x] Modify `src/SlpModularCms.Core/SlpModularCms.Core.csproj` to add `Microsoft.AspNetCore.DataProtection.EntityFrameworkCore` version **10.0.9**, matching the existing 10.0.x line + +### Step 2: Keys table on the Core context +- [x] Modify `src/SlpModularCms.Core/Data/ApplicationDbContext.cs`: + - [x] Implement `IDataProtectionKeyContext` + - [x] Add `DbSet DataProtectionKeys` + - [x] Leave every existing entity configuration untouched + +### Step 3: Data Protection registration +- [x] Create `src/SlpModularCms.Core/Hosting/DataProtectionExtensions.cs` with `AddCmsDataProtection()` +- [x] Configure `PersistKeysToDbContext()` (BR-U2-01) +- [x] Set the application discriminator to a **fixed constant in code** (BR-U2-02) — not configurable, not derived from any path +- [x] Leave key lifetime at the framework default of 90 days (BR-U2-05) +- [x] Document in code why the discriminator is a constant: the default derives from the content root path, which changes on every atomic release switch + +### Step 4: Startup migration runner +- [x] Create `src/SlpModularCms.Core/Hosting/DatabaseMigrationExtensions.cs` with `MigrateCoreDatabase()` +- [x] Apply `ApplicationDbContext` migrations before the application accepts traffic (BR-U2-09) +- [x] Classify failures (BR-U2-11, BR-U2-12): retry **connection** failures with increasing delay up to a bounded number of attempts; fail **migration** failures immediately with no retry +- [x] Log the reason before failing, with diagnostic context but **no** connection string, credentials or secrets (BR-U2-14) +- [x] Propagate the exception when retries are exhausted or the failure is a migration failure, so the process does not start (BR-U2-13) + +### Step 5: Remove the conflicting module registrations +- [x] Modify `src/SlpModularCms.Modules.Availability/AvailabilityModule.cs` — remove `services.AddDataProtection()` +- [x] Modify `src/SlpModularCms.Modules.Master/MasterModule.cs` — remove `services.AddDataProtection()` +- [x] Leave every other registration in both modules unchanged; they continue consuming `IDataProtector` (BR-U2-04) + +*This is the § 5.1 conflict. Module registration runs **after** the host's, so these bare calls would override the persistent key store. `IDataProtector` resolves either way, so the defect would surface only after the first release switch as slave API keys that no longer decrypt — presenting as a network fault between Master and slave.* + +### Step 6: Host composition +- [x] Modify `src/SlpModularCms.Api/Program.cs` — call `AddCmsDataProtection()` **before** `orchestrator.RegisterModuleServices(...)` (BR-U2-03), and `MigrateCoreDatabase()` after `builder.Build()` and before `orchestrator.UseModules(app)` (BR-U2-10) +- [x] Modify `src/SlpModularCms.Api.Slave/Program.cs` — the same two calls in the same positions + +### Step 7: Core migration +- [x] Generate the EF Core migration for the keys table into `src/SlpModularCms.Core/Migrations/` +- [x] Verify the migration is purely additive — no dropped or narrowed columns, so redeploying an earlier release stays safe (BR-U2-16) + +### Step 8: Data Protection unit tests +- [x] Create `src/SlpModularCms.Core.Tests/Hosting/DataProtectionExtensionsTests.cs`: + - [x] The persistent key store **survives module registration** — the highest-value assertion in this unit, since registration alone passes in both the broken and fixed cases + - [x] The application discriminator is the fixed constant, not a path-derived value + - [x] A protected value round-trips across a simulated content-root change + - [x] Neither module registers Data Protection, so the conflict cannot be reintroduced by a future change + +### Step 9: Migration runner unit tests +- [x] Create `src/SlpModularCms.Core.Tests/Hosting/DatabaseMigrationExtensionsTests.cs`: + - [x] A connection failure is retried + - [x] A migration failure is **not** retried and fails immediately + - [x] Retry exhaustion propagates + - [x] Failure logging contains no connection string or credentials + +### Step 10: Documentation +- [x] Create `aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/code/generation-summary.md` — files created and modified, decisions taken, and any deviation from this plan +- [x] Record the two operational constraints that are enforced by documentation rather than code, for later inclusion in the Operations deployment instructions: the keys table must **never** be pruned (BR-U2-06), and only one instance may migrate a given database at a time (BR-U2-17) + +### Step 11: Build and test verification (automatic) +- [x] `dotnet build SlpModularCms.sln -c Release` +- [x] `dotnet test` for `SlpModularCms.Core.Tests`, `SlpModularCms.Modules.Availability.Tests` and `SlpModularCms.Modules.Master.Tests` +- [x] Fix any failure directly and re-run until green +- [x] Record the outcome for the completion message + +--- + +## Files Touched + +### Created +| Path | Purpose | +|---|---| +| `src/SlpModularCms.Core/Hosting/DataProtectionExtensions.cs` | Persistent key ring registration | +| `src/SlpModularCms.Core/Hosting/DatabaseMigrationExtensions.cs` | Startup migration with failure classification | +| `src/SlpModularCms.Core/Migrations/*_AddDataProtectionKeys.cs` | Keys table migration | +| `src/SlpModularCms.Core.Tests/Hosting/DataProtectionExtensionsTests.cs` | Tests | +| `src/SlpModularCms.Core.Tests/Hosting/DatabaseMigrationExtensionsTests.cs` | Tests | + +### Modified +| Path | Change | +|---|---| +| `src/SlpModularCms.Core/SlpModularCms.Core.csproj` | Add the Data Protection EF Core package | +| `src/SlpModularCms.Core/Data/ApplicationDbContext.cs` | Implement `IDataProtectionKeyContext`, add the keys set | +| `src/SlpModularCms.Modules.Availability/AvailabilityModule.cs` | Remove `AddDataProtection()` | +| `src/SlpModularCms.Modules.Master/MasterModule.cs` | Remove `AddDataProtection()` | +| `src/SlpModularCms.Api/Program.cs` | Data Protection registration and startup migration | +| `src/SlpModularCms.Api.Slave/Program.cs` | Same | + +**Brownfield rule**: every file above that exists is modified in place. No parallel copies. + +--- + +## Risk Notes for the Executor + +| Risk | Mitigation in this plan | +|---|---| +| A test that merely asserts "Data Protection is registered" passes in both the broken and fixed cases | Step 8 asserts the **resulting configuration**, not the registration | +| The discriminator silently reverting to the path-derived default | Step 8 asserts the constant explicitly | +| The new migration being non-additive and breaking rollback | Step 7 verifies additivity | +| Startup migration masking a genuine migration fault by retrying it | Step 4 classifies failures; Step 9 asserts the classification | +| Both hosts must still start | Step 11 builds the whole solution; the composed startup is verified at the phase-level Build and Test stage, where the Slave — which has no test project — is started | + +--- + +## Out of Scope for U2 + +- Static content, health endpoint, availability-gate changes — U1 +- Security headers — U3 +- Sentry, Umami, frontend configuration — U4 +- Anything under `.gitea/` — U5 and U6 +- Certificate-based key encryption — deferred as DEV-05 follow-up +- A distributed migration lock — Q4 = C, handled by documented operational constraint diff --git a/aidlc-docs/features/gitea-deployment-workflow/construction/plans/u2-data-durability-fd-questions.md b/aidlc-docs/features/gitea-deployment-workflow/construction/plans/u2-data-durability-fd-questions.md new file mode 100644 index 0000000..6bb4ded --- /dev/null +++ b/aidlc-docs/features/gitea-deployment-workflow/construction/plans/u2-data-durability-fd-questions.md @@ -0,0 +1,76 @@ +# Functional Design Questions — U2 Data Durability + +Vul je keuze in achter elke `[Answer]:`-tag. Kies de laatste optie (`Anders`) als niets past. + +--- + +## Question 1 — Waar komt de application discriminator vandaan? + +**Context**: de application discriminator bepaalt of twee processen dezelfde Data Protection-sleutels kunnen gebruiken. Standaard leidt ASP.NET Core hem af uit het content root-pad — en dat verandert bij élke atomaire release-switch. Zonder expliciete waarde is de key ring dus alsnog effectief weg na een deploy, ondanks dat hij in de database staat. + +Hij moet dus vast staan. De vraag is waar die waarde vandaan komt. + +A) Een vaste constante in de code (bijv. `"SlpModularCms"`) — kan niet per ongeluk verkeerd gezet worden, en is voor alle instanties gelijk +B) Uit configuratie, met een vaste standaardwaarde — dan kun je per klant/instantie een eigen waarde zetten als dat ooit nodig is +C) Uit configuratie, verplicht in te vullen — dwingt een bewuste keuze af per omgeving +X) Anders (beschrijf hieronder na de [Answer]:-tag) + +[Answer]:A + +--- + +## Question 2 — Moeten de sleutels versleuteld in de database staan? + +**Context**: `PersistKeysToDbContext` slaat de sleutels standaard **onversleuteld** op als XML in de tabel. Wie de database kan lezen, kan daarmee de opgeslagen slave-API-keys ontsleutelen. + +Op Windows lost DPAPI dit normaal op, maar dat werkt niet op Linux (de Pi), dus dat is hier geen optie. Het alternatief is versleutelen met een X.509-certificaat — maar dan moet dat certificaat mee gedeployed worden en beschikbaar blijven, wat een nieuwe versie van hetzelfde probleem introduceert: raak je het certificaat kwijt, dan zijn de sleutels alsnog onleesbaar. + +SECURITY-01 vraagt om versleuteling at rest. + +A) Onversleuteld in de database, en de encryptie-at-rest van de database zelf is de maatregel — vastleggen als bewuste onderbouwde keuze, met de eis dat de databaseverbinding TLS gebruikt en de database niet publiek benaderbaar is +B) Versleutelen met een X.509-certificaat — sterker, maar verplaatst het bewaarprobleem naar het certificaat en voegt een deploystap toe +C) Onversleuteld nu, en certificaat-encryptie als apart vervolgpunt vastleggen +X) Anders (beschrijf hieronder na de [Answer]:-tag) + +[Answer]:C + +--- + +## Question 3 — Wat gebeurt er als de database bij het opstarten net niet bereikbaar is? + +**Context**: je koos fail fast bij een migratiefout. Maar er is een verschil tussen "de migratie klopt niet" (echt fout) en "de database is er nog even niet" (tijdelijk) — bijvoorbeeld als de app en de SQL Server-container tegelijk opstarten na een herstart van de Pi. + +Bij strikte fail-fast start de app dan niet, en moet iets anders hem opnieuw starten. + +A) Strikt fail fast, geen retry — de procesmanager (systemd) herstart de app toch al automatisch, dus dat lost het vanzelf op +B) Een korte retry met toenemende wachttijd (bijv. 5 pogingen over ~30 seconden) en dán pas falen — vangt het opstartvenster af zonder een echte fout te verbergen +C) Retry alleen bij verbindingsfouten, direct falen bij een migratiefout — onderscheid tussen "nog niet bereikbaar" en "kapot" +X) Anders (beschrijf hieronder na de [Answer]:-tag) + +[Answer]:C + +--- + +## Question 4 — Wat als twee instanties tegelijk opstarten en migreren? + +**Context**: `Database.Migrate()` is niet ontworpen om veilig gelijktijdig te draaien. In jouw huidige opzet draait er één instantie per omgeving, dus dit speelt nu niet. Maar de master/slave-opzet betekent dat er meerdere instanties naar **verschillende** databases wijzen, en een herstart kan ze wel gelijktijdig laten opstarten. + +A) Negeren — één instantie per database, dus dit kan niet voorkomen. Wel als aanname vastleggen +B) Een migratielock in de database gebruiken zodat gelijktijdig migreren veilig is — robuuster, maar meer complexiteit voor een situatie die zich nu niet voordoet +C) Alleen documenteren in de deployment-instructies dat instanties niet gelijktijdig gemigreerd moeten worden +X) Anders (beschrijf hieronder na de [Answer]:-tag) + +[Answer]:C + +--- + +## Question 5 — Sleutellevensduur en rotatie + +**Context**: standaard maakt Data Protection elke 90 dagen een nieuwe sleutel aan en houdt oude sleutels beschikbaar om bestaande waarden te kunnen blijven ontsleutelen. Voor de versleutelde slave-API-keys betekent dat: die blijven leesbaar, ook na rotatie, zolang de oude sleutels in de tabel blijven staan. + +A) De standaard van 90 dagen aanhouden en oude sleutels nooit opruimen — bestaande waarden blijven altijd leesbaar +B) Een langere levensduur instellen zodat er minder sleutels ontstaan +C) De standaard aanhouden, plus expliciet vastleggen in de documentatie dat de sleuteltabel nooit opgeschoond mag worden — want dat zou de opgeslagen API-keys onleesbaar maken +X) Anders (beschrijf hieronder na de [Answer]:-tag) + +[Answer]:C diff --git a/aidlc-docs/features/gitea-deployment-workflow/construction/plans/u2-data-durability-functional-design-plan.md b/aidlc-docs/features/gitea-deployment-workflow/construction/plans/u2-data-durability-functional-design-plan.md new file mode 100644 index 0000000..1385bf8 --- /dev/null +++ b/aidlc-docs/features/gitea-deployment-workflow/construction/plans/u2-data-durability-functional-design-plan.md @@ -0,0 +1,42 @@ +# Functional Design Plan — U2 Data Durability + +**Unit**: U2 Data Durability +**Round**: R1 (with U1 Hosting & Serving) +**Requirements**: FR-11, FR-12 +**Components**: C-05, C-06, C-07, U2 portion of C-16 + +--- + +## Step 1: Analyze unit context +- [x] Read the U2 definition from `unit-of-work.md` +- [x] Read the requirement assignment from `unit-of-work-story-map.md` +- [x] Read the carried-in design items — § 5.1 duplicate registration conflict, explicit application discriminator + +## Step 2: Design the Data Protection key ring +- [x] Define the key-storage entity and its owning context +- [x] Define the application-discriminator source and stability guarantee +- [x] Define key encryption at rest +- [x] Define key lifetime and rotation behaviour +- [x] Define the registration-order rule that resolves the duplicate-registration conflict + +## Step 3: Design startup migration behaviour +- [x] Define which contexts migrate and in what order +- [x] Define failure behaviour and what is logged before failing +- [x] Define behaviour when the database is temporarily unreachable at startup +- [x] Define behaviour when a migration is applied concurrently by two starting instances + +## Step 4: Define business rules +- [x] Enumerate key-ring durability rules +- [x] Enumerate migration rules +- [x] Identify error and edge-case scenarios + +## Step 5: Design verification approach +- [x] Define how "the persistent key store survives module registration" is asserted +- [x] Define how discriminator stability across a content-root change is asserted + +## Step 6: Generate artifacts +- [x] Generate `business-logic-model.md` +- [x] Generate `business-rules.md` +- [x] Generate `domain-entities.md` +- [x] Validate all diagrams against the Mermaid standards +- [x] Verify Security Baseline compliance for this unit's design diff --git a/aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/code/generation-summary.md b/aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/code/generation-summary.md new file mode 100644 index 0000000..209fed3 --- /dev/null +++ b/aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/code/generation-summary.md @@ -0,0 +1,87 @@ +# Code Generation Summary — U2 Data Durability + +**Date**: 2026-07-27 +**Requirements**: FR-11, FR-12 + +--- + +## Files Created + +| Path | Purpose | +|---|---| +| `src/SlpModularCms.Core/Hosting/DataProtectionExtensions.cs` | `AddCmsDataProtection()` — database key ring, fixed discriminator | +| `src/SlpModularCms.Core/Hosting/DatabaseMigrationExtensions.cs` | `MigrateCoreDatabase()` — startup migration with failure classification | +| `src/SlpModularCms.Core/Migrations/20260727203036_AddDataProtectionKeys.cs` | Keys table migration | +| `src/SlpModularCms.Core.Tests/Hosting/DataProtectionExtensionsTests.cs` | 4 tests | +| `src/SlpModularCms.Core.Tests/Hosting/DatabaseMigrationExtensionsTests.cs` | 7 tests | +| `src/SlpModularCms.Modules.Availability.Tests/AvailabilityModuleDataProtectionTests.cs` | 1 test | +| `src/SlpModularCms.Modules.Master.Tests/MasterModuleDataProtectionTests.cs` | 1 test | + +## Files Modified + +| Path | Change | +|---|---| +| `src/SlpModularCms.Core/SlpModularCms.Core.csproj` | `Microsoft.AspNetCore.DataProtection.EntityFrameworkCore` 10.0.9 | +| `src/SlpModularCms.Core/Data/ApplicationDbContext.cs` | Implements `IDataProtectionKeyContext`; `DataProtectionKeys` set | +| `src/SlpModularCms.Modules.Availability/AvailabilityModule.cs` | `AddDataProtection()` removed | +| `src/SlpModularCms.Modules.Master/MasterModule.cs` | `AddDataProtection()` removed | +| `src/SlpModularCms.Api/Program.cs` | `AddCmsDataProtection()` before module registration; `MigrateCoreDatabase()` after build | +| `src/SlpModularCms.Api.Slave/Program.cs` | Same | + +No duplicate or parallel files were created. + +--- + +## Implementation Decisions + +### The removals are the point of this unit, so they are commented in place +Deleting `services.AddDataProtection()` from two modules looks like a regression to anyone who does not know the ordering issue. Both call sites therefore carry a comment explaining that the host owns Data Protection and that a bare call here would silently discard the persistent key store. + +### The tests assert configuration, not registration +A test asserting "Data Protection is registered" passes in both the broken and fixed cases, because `IDataProtector` resolves either way. Every test here inspects the **resulting** configuration instead: + +- `KeyManagementOptions.XmlRepository` is `EntityFrameworkCoreXmlRepository` — not the filesystem default +- `DataProtectionOptions.ApplicationDiscriminator` is the fixed constant — not the path-derived default +- A protected value survives a simulated restart from a different release directory, which is the property that actually matters +- The discriminator contains no path separator, guarding against a future "improvement" that makes it computed or configurable + +The module-level tests live in each module's own test project rather than in `Core.Tests`, because `Core.Tests` does not reference the modules. Each registers the host's Data Protection first and the module second — the real ordering — and asserts the EF repository survives. + +### `SqlException` is produced genuinely, not faked +`SqlException` has no public constructor. Rather than substituting a stand-in type, the test provokes a real one by opening a connection to an unreachable host with a one-second timeout. The classifier is therefore exercised against the exact type it will meet in production. + +### Wrong credentials are classified as a connection failure +Distinguishing bad credentials from an unreachable server would add branching for no benefit: retries are exhausted and the process does not start either way. The simpler classification is the honest one. + +### Migration verified as additive +The generated migration only creates a table — no dropped or narrowed columns. Rollback by redeploying an earlier release therefore stays safe, which BR-U2-16 requires and which the whole rollback strategy depends on. + +--- + +## Operational Constraints Enforced by Documentation, Not Code + +Both were decided deliberately (U2 FD Q4 = C, Q5 = C). They must appear in the Operations deployment instructions: + +| Constraint | Why it is not enforced in code | +|---|---| +| **The `DataProtectionKeys` table must never be pruned.** Deleting a key makes every value encrypted with it permanently unreadable, including stored slave API keys. | Nothing in the application deletes these rows; the risk comes from a human treating the table as housekeeping. It is the single most destructive maintenance action available against this system, and it looks harmless. | +| **Only one instance may migrate a given database at a time.** | One instance per database holds by design today — the Master and each slave have their own. A distributed migration lock would add failure modes without removing any. **If the deployment model ever changes to multiple instances sharing a database, automatic startup migration must be revisited before that change is made.** | + +Also recorded for Operations: **DEV-05** — keys are stored unencrypted at rest, with TLS on the database connection and a non-public database as the compensating controls (BR-U2-08). These are not optional extras; they are what makes the deviation acceptable. + +--- + +## Verification + +| Check | Result | +|---|---| +| `dotnet build SlpModularCms.sln -c Release` | ✅ 0 errors | +| `SlpModularCms.Core.Tests` | ✅ 83 passed | +| `SlpModularCms.Modules.Availability.Tests` | ✅ 82 passed | +| `SlpModularCms.Modules.Identity.Tests` | ✅ 37 passed | +| `SlpModularCms.Modules.Master.Tests` | ✅ 51 passed | +| Migration is purely additive | ✅ Inspected — creates one table, drops nothing | + +No failures occurred during generation. + +**Not verifiable at this stage**: the composed startup path (`MigrateCoreDatabase` against a real database, and both hosts actually starting) requires SQL Server. Carried to the phase-level Build and Test stage. diff --git a/aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/functional-design/business-logic-model.md b/aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/functional-design/business-logic-model.md new file mode 100644 index 0000000..5268fba --- /dev/null +++ b/aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/functional-design/business-logic-model.md @@ -0,0 +1,194 @@ +# Business Logic Model — U2 Data Durability + +**Unit**: U2 Data Durability +**Requirements**: FR-11, FR-12 + +--- + +## 1. Scope of the Logic + +U2 delivers nothing a user can see. Its entire value is negative: after this unit, a redeploy **cannot** silently destroy schema state or the trust relationship between a Master and its slaves. + +Two mechanisms: +1. **Key ring durability** — Data Protection keys move from the filesystem (discarded by every atomic release switch) into the database, with an application discriminator that does not change when the release directory does +2. **Schema convergence** — `ApplicationDbContext` migrates itself at startup, so a deployment needs no CLI access to the host + +Both are startup-time concerns. Neither participates in request handling. + +--- + +## 2. Startup Sequence + +```mermaid +graph TD + boot["Host builder starts"] + log["Configure logging and Sentry"] + disc["Discover modules"] + core["AddCoreInfrastructure"] + dp["AddCmsDataProtection
persistent key store plus
fixed application discriminator"] + mods["Module RegisterServices
AddDataProtection removed from both"] + build["Build application"] + mig["Migrate ApplicationDbContext"] + classify{"Failure type ?"} + retry["Retry with backoff"] + fail["Propagate: process does not start"] + usemods["UseModules
module contexts migrate"] + serve["Accept traffic"] + + boot --> log + log --> disc + disc --> core + core --> dp + dp --> mods + mods --> build + build --> mig + mig -->|success| usemods + mig -->|failure| classify + classify -->|"connection failure"| retry + classify -->|"migration failure"| fail + retry -->|"attempts remain"| mig + retry -->|"attempts exhausted"| fail + usemods --> serve + + classDef entry fill:#f6e05e,stroke:#c05621,stroke-width:1px,color:#000; + classDef step fill:#9ae6b4,stroke:#2f855a,stroke-width:1px,color:#000; + classDef critical fill:#90cdf4,stroke:#2b6cb0,stroke-width:1px,color:#000; + classDef bad fill:#fbb6ce,stroke:#b83280,stroke-width:1px,color:#000; + class boot entry; + class log,disc,core,mods,build,usemods,serve step; + class dp,mig,classify,retry critical; + class fail bad; +``` + +Text alternative: Data Protection is configured with a persistent store before module registration, the Core context migrates before the module contexts, and a migration failure is classified — connection failures are retried with backoff while genuine migration failures stop the process immediately. + +--- + +## 3. The Registration-Order Conflict + +This is the unit's most important piece of logic, and it is a *removal* rather than an addition. + +`AvailabilityModule.RegisterServices` and `MasterModule.RegisterServices` each call `services.AddDataProtection()` today. Module registration runs **after** the host's registration. In ASP.NET Core, a later `AddDataProtection()` re-registers the configuration chain, so the modules' bare calls would discard the persistent key store configured by the host. + +```mermaid +graph TD + subgraph broken["Without the fix"] + h1["Host: AddCmsDataProtection
persistent store configured"] + m1["AvailabilityModule: AddDataProtection"] + m2["MasterModule: AddDataProtection"] + r1["Result: filesystem key ring
FR-12 silently ineffective"] + h1 --> m1 + m1 --> m2 + m2 --> r1 + end + + subgraph fixed["With the fix"] + h2["Host: AddCmsDataProtection
persistent store configured"] + m3["Modules: no Data Protection call
they consume IDataProtector only"] + r2["Result: database key ring
survives release switches"] + h2 --> m3 + m3 --> r2 + end + + classDef good fill:#9ae6b4,stroke:#2f855a,stroke-width:1px,color:#000; + classDef bad fill:#fbb6ce,stroke:#b83280,stroke-width:1px,color:#000; + classDef neutral fill:#90cdf4,stroke:#2b6cb0,stroke-width:1px,color:#000; + class h1,h2,m3 neutral; + class m1,m2,r1 bad; + class r2 good; +``` + +Text alternative: leaving the modules' own Data Protection calls in place would override the host's persistent key store and leave the key ring on the filesystem, whereas removing them lets the host's single configuration stand. + +**Why this is dangerous rather than merely wrong**: registration tests pass either way — `IDataProtector` resolves in both cases. The defect appears only after the first atomic release switch, as slave API keys that no longer decrypt, presenting as a network fault between Master and slave. The verification for this unit must therefore assert the **resulting configuration**, not merely that Data Protection is registered. + +--- + +## 4. Application Discriminator Stability + +The application discriminator determines whether two processes derive the same keys. By default it is derived from the content root path — which changes on every atomic release switch. Persisting keys in the database while letting the discriminator move would produce keys that are stored but unusable: a second, quieter version of the same failure. + +Per Q1 = A the discriminator is a **fixed constant in code**. + +```mermaid +graph TD + r1["Release directory 1
content root /srv/cms/releases/001"] + r2["Release directory 2
content root /srv/cms/releases/002"] + disc["Fixed discriminator constant"] + keys[("Key ring in database")] + same["Same keys derived
stored values stay readable"] + + r1 --> disc + r2 --> disc + disc --> keys + keys --> same + + classDef release fill:#90cdf4,stroke:#2b6cb0,stroke-width:1px,color:#000; + classDef fixed fill:#fbd38d,stroke:#c05621,stroke-width:1px,color:#000; + classDef store fill:#d6bcfa,stroke:#6b46c1,stroke-width:1px,color:#000; + classDef good fill:#9ae6b4,stroke:#2f855a,stroke-width:1px,color:#000; + class r1,r2 release; + class disc fixed; + class keys store; + class same good; +``` + +Text alternative: two different release directories both use the same fixed discriminator, so the keys stored in the database remain derivable and previously encrypted values stay readable across deploys. + +**Why a constant rather than configuration** (Q1 = A): it cannot be set wrong, forgotten during a host migration, or accidentally differ between two instances that share a database. A configurable value would add a way to reintroduce the exact failure this unit exists to prevent. + +--- + +## 5. Migration Failure Classification + +Per Q3 = C, failures are classified rather than treated uniformly: + +| Failure kind | Meaning | Response | +|---|---|---| +| **Connection failure** | The database is not reachable yet — typically the app and SQL Server starting together after a host reboot | Retry with increasing delay, then fail | +| **Migration failure** | A migration is invalid, conflicts, or cannot be applied | Fail immediately, no retry | + +Retrying a genuine migration failure would only delay the inevitable while making the log harder to read. Failing instantly on a transient connection error would make a host reboot look like a broken deployment. + +**Interaction with U1's health check**: after retries are exhausted the exception propagates and the process does not start. `/health` then does not answer, and UptimeRobot goes red. That chain is the entire reason a liveness-only check is sufficient — it is meaningful precisely because startup is strict. + +--- + +## 6. Migration Ordering + +```mermaid +graph TD + corectx["ApplicationDbContext
Identity, refresh tokens,
invitations, Data Protection keys"] + availctx["AvailabilityDbContext
master registration"] + masterctx["MasterDbContext
CMS instances"] + protector["IDataProtector consumers
encrypted API keys"] + + corectx -->|"migrates first, at startup"| availctx + corectx -->|"migrates first, at startup"| masterctx + corectx -->|"keys table must exist before"| protector + availctx -->|"uses"| protector + masterctx -->|"uses"| protector + + classDef core fill:#fbd38d,stroke:#c05621,stroke-width:1px,color:#000; + classDef module fill:#9ae6b4,stroke:#2f855a,stroke-width:1px,color:#000; + classDef consumer fill:#90cdf4,stroke:#2b6cb0,stroke-width:1px,color:#000; + class corectx core; + class availctx,masterctx module; + class protector consumer; +``` + +Text alternative: the Core context migrates first because it now owns the Data Protection keys table, which both module contexts depend on indirectly through their encrypted API key handling. + +**Why Core must be first**: the keys table lives in `ApplicationDbContext` (Q7 of Application Design = A). Both modules encrypt and decrypt API keys. If a module migrated and immediately used an `IDataProtector` before the keys table existed, key generation would fail against a missing table. + +**Scope boundary**: U2 adds automatic migration for `ApplicationDbContext` only. `AvailabilityDbContext` and `MasterDbContext` already migrate themselves in their `UseModule` implementations, and that existing behaviour is left untouched — changing it would alter module behaviour beyond this feature's scope. + +--- + +## 7. Concurrent Migration + +Per Q4 = C, no locking mechanism is built. The design assumption is **one instance per database**, which holds today: the Master and each slave have their own database. + +This is documented in the deployment instructions as an operational constraint rather than enforced in code — building a distributed migration lock for a situation that cannot currently occur would add failure modes without removing any. + +**Recorded as an assumption**: if the deployment model ever changes to multiple instances sharing one database, automatic startup migration must be revisited before that change is made. diff --git a/aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/functional-design/business-rules.md b/aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/functional-design/business-rules.md new file mode 100644 index 0000000..ba86a52 --- /dev/null +++ b/aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/functional-design/business-rules.md @@ -0,0 +1,130 @@ +# Business Rules — U2 Data Durability + +--- + +## Migration Decision Logic + +```mermaid +graph TD + start["Startup: migrate ApplicationDbContext"] + attempt["Attempt migration"] + ok{"Succeeded ?"} + done["Continue startup"] + kind{"Failure kind ?"} + attempts{"Retry attempts remaining ?"} + wait["Wait with increasing delay"] + logfail["Log the failure with context"] + stop["Propagate: process does not start"] + + start --> attempt + attempt --> ok + ok -->|yes| done + ok -->|no| kind + kind -->|"connection failure"| attempts + kind -->|"migration failure"| logfail + attempts -->|yes| wait + attempts -->|no| logfail + wait --> attempt + logfail --> stop + + classDef entry fill:#f6e05e,stroke:#c05621,stroke-width:1px,color:#000; + classDef decision fill:#90cdf4,stroke:#2b6cb0,stroke-width:1px,color:#000; + classDef good fill:#9ae6b4,stroke:#2f855a,stroke-width:1px,color:#000; + classDef bad fill:#fbb6ce,stroke:#b83280,stroke-width:1px,color:#000; + class start entry; + class ok,kind,attempts decision; + class attempt,wait,done good; + class logfail,stop bad; +``` + +Text alternative: migration is retried with increasing delay only when the failure is a connection problem; a genuine migration failure is logged and stops the process immediately, as does exhausting the retry attempts. + +--- + +## Key Ring Durability Rules + +| ID | Rule | +|---|---| +| **BR-U2-01** | Data Protection keys are persisted in the database, in the context that owns Identity — never on the filesystem. | +| **BR-U2-02** | The application discriminator is a **fixed constant in code**. It is not derived from any path, and it is not configurable. | +| **BR-U2-03** | Data Protection is configured **exactly once**, by the host, before module service registration. | +| **BR-U2-04** | No module may call `AddDataProtection()`. Modules consume `IDataProtector` only. | +| **BR-U2-05** | Key lifetime uses the framework default of 90 days, with automatic rotation. | +| **BR-U2-06** | Old keys are **never** deleted. Removing a key makes every value encrypted with it permanently unreadable, including stored slave API keys. | +| **BR-U2-07** | Keys are stored unencrypted at rest in the database. This is an accepted, documented deviation — see DEV-05. | +| **BR-U2-08** | The database connection must enforce TLS, and the database must not be publicly reachable. These are the compensating controls for BR-U2-07. | + +**Rationale for BR-U2-02**: the default discriminator derives from the content root path, which changes on every atomic release switch. Persisting keys in the database while letting the discriminator move produces keys that are stored but underivable — the same failure, quieter. A constant cannot be forgotten during a host migration or accidentally differ between two instances sharing a database. + +**Rationale for BR-U2-03 and BR-U2-04**: this is the § 5.1 conflict. Because module registration runs after the host's, a module's bare `AddDataProtection()` would override the persistent store. `IDataProtector` still resolves, so registration tests pass — the defect appears only after the first release switch, as slave API keys that no longer decrypt. + +**Rationale for BR-U2-06**: this is the single most destructive maintenance action available against this system. Pruning the keys table looks like harmless housekeeping and permanently breaks every Master↔slave relationship. It must be stated in the deployment documentation, not only in code comments (Q5 = C). + +--- + +## Migration Rules + +| ID | Rule | +|---|---| +| **BR-U2-09** | `ApplicationDbContext` migrations are applied automatically at startup, before the application accepts traffic. | +| **BR-U2-10** | Core migrations run **before** module middleware installation, so the keys table exists before any module resolves an `IDataProtector`. | +| **BR-U2-11** | A **connection** failure is retried with increasing delay, up to a bounded number of attempts. | +| **BR-U2-12** | A **migration** failure — invalid, conflicting, or inapplicable — fails immediately with no retry. | +| **BR-U2-13** | When retries are exhausted, or on a migration failure, the exception propagates and the process does not start. | +| **BR-U2-14** | Before failing, the reason is logged with enough context to diagnose it — but never including the connection string, credentials, or any secret. | +| **BR-U2-15** | `AvailabilityDbContext` and `MasterDbContext` keep their existing self-migration in `UseModule`. U2 does not change them. | +| **BR-U2-16** | Migrations must be forward-compatible and non-destructive, so redeploying an earlier release remains a valid rollback. | + +**Rationale for BR-U2-11 and BR-U2-12**: the two failures mean different things. On the Pi the application and SQL Server may start together after a reboot, so a brief unavailability window is normal operation, not a fault. A broken migration is a fault, and retrying it only delays the inevitable while filling the log. + +**Rationale for BR-U2-13**: this is what gives U1's liveness check meaning. A process that starts regardless would report healthy while being unusable; strict startup makes the absence of a `/health` response a trustworthy signal. + +**Rationale for BR-U2-16**: rollback is "redeploy the previous release" (D-26). That is only safe if the older code can run against the newer schema. A destructive migration — dropping a column, narrowing a type — makes rollback impossible precisely when it is most needed. + +--- + +## Operational Constraint Rules + +| ID | Rule | +|---|---| +| **BR-U2-17** | Exactly one application instance may migrate a given database at a time. Not enforced in code; documented as an operational constraint (Q4 = C). | +| **BR-U2-18** | Each instance has its own database. The Master and each slave never share one. | + +**Rationale**: `Database.Migrate()` is not safe under concurrency. Today the constraint holds by design — one instance per database — so a distributed lock would add failure modes without removing any. Should the deployment model ever change to multiple instances sharing a database, automatic startup migration must be revisited **before** that change is made. + +--- + +## Error and Edge-Case Scenarios + +| Scenario | Expected behaviour | +|---|---| +| Fresh database, no tables | All Core migrations applied, including the keys table; startup proceeds | +| Database up to date | Migration is a no-op; startup proceeds | +| Database unreachable at startup, comes up within the retry window | Retries succeed; startup proceeds; the delay is logged | +| Database unreachable for the whole retry window | Logged, process does not start, `/health` silent, UptimeRobot red | +| Migration conflicts with existing schema | Failed immediately, no retry, process does not start | +| Credentials wrong | Treated as a connection failure — retried, then fails. Distinguishing bad credentials from an unreachable server is not worth the complexity; the outcome is identical | +| Keys table empty on first run | Data Protection generates a key and persists it; normal first-run behaviour | +| Keys table populated from a previous release | Existing keys are read; previously encrypted values remain readable — the purpose of the unit | +| Release directory changed since last start | Irrelevant — the discriminator is a constant (BR-U2-02) | +| Someone deletes rows from the keys table | Every value encrypted with those keys becomes permanently unreadable. Prevented by documentation only (BR-U2-06) | +| Two instances start simultaneously against one database | Undefined. Prevented by the operational constraint (BR-U2-17), not by code | +| A module still calls `AddDataProtection()` after this unit | The persistent store is silently overridden. Prevented by BR-U2-04 and asserted by a test | + +--- + +## Security Compliance for U2 + +| Rule | Status | Notes | +|---|---|---| +| SECURITY-01 | **Partially compliant — DEV-05** | Encryption **in transit** enforced by BR-U2-08 (TLS on the connection). Encryption **at rest** for the keys themselves is deferred: keys are stored unencrypted, relying on the database's own at-rest encryption and network isolation. Accepted with a follow-up (Q2 = C) | +| SECURITY-03 | Compliant | BR-U2-14 requires diagnostic context without secrets | +| SECURITY-09 | Compliant | No default credentials; failure messages carry no connection details | +| SECURITY-13 | **Improved** | The key ring surviving redeploys is precisely a software-integrity property: without it, the encrypted API keys that authenticate Master↔slave communication silently become invalid | +| SECURITY-15 | Compliant | Fails closed — the process does not start rather than serving in an unknown schema state | + +### New Documented Deviation + +| ID | Deviation | Rationale | Decided | +|---|---|---|---| +| **DEV-05** | **Data Protection keys are stored unencrypted at rest.** SECURITY-01 requires encryption at rest for persisted data. | DPAPI is unavailable on Linux, and X.509 certificate encryption relocates the loss problem to the certificate — reintroducing the failure mode this unit exists to eliminate. Compensating controls: TLS on the database connection, and the database not publicly reachable (BR-U2-08). Certificate-based encryption is recorded as a separate follow-up item. | Q2 = C | diff --git a/aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/functional-design/domain-entities.md b/aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/functional-design/domain-entities.md new file mode 100644 index 0000000..ac0044d --- /dev/null +++ b/aidlc-docs/features/gitea-deployment-workflow/construction/u2-data-durability/functional-design/domain-entities.md @@ -0,0 +1,124 @@ +# Domain Entities — U2 Data Durability + +U2 adds **one** persisted entity and **one** migration. Everything else in the unit is configuration and startup behaviour. + +--- + +## Entity Relationships + +```mermaid +graph TD + appctx["ApplicationDbContext
implements IDataProtectionKeyContext"] + key["DataProtectionKey
NEW"] + user["ApplicationUser
existing"] + refresh["RefreshToken
existing"] + invite["Invitation
existing"] + avail["GlobalAvailabilityState
existing"] + protector["IDataProtector
derived from keys"] + cmsinst["CmsInstance
MasterDbContext"] + mastreg["MasterRegistration
AvailabilityDbContext"] + + appctx -->|"owns"| key + appctx -->|"owns"| user + appctx -->|"owns"| refresh + appctx -->|"owns"| invite + appctx -->|"owns"| avail + key -->|"derives"| protector + protector -->|"encrypts API key of"| cmsinst + protector -->|"encrypts API key of"| mastreg + + classDef ctx fill:#fbd38d,stroke:#c05621,stroke-width:1px,color:#000; + classDef newent fill:#9ae6b4,stroke:#2f855a,stroke-width:1px,color:#000; + classDef existing fill:#e2e8f0,stroke:#4a5568,stroke-width:1px,color:#000; + classDef derived fill:#90cdf4,stroke:#2b6cb0,stroke-width:1px,color:#000; + classDef consumer fill:#d6bcfa,stroke:#6b46c1,stroke-width:1px,color:#000; + class appctx ctx; + class key newent; + class user,refresh,invite,avail existing; + class protector derived; + class cmsinst,mastreg consumer; +``` + +Text alternative: the Core context gains a Data Protection keys table alongside its existing Identity entities; those keys derive the protector that encrypts the API keys stored on CMS instances and master registrations in the two module contexts. + +**Cross-context dependency worth noting**: the keys live in `ApplicationDbContext`, while the values they protect live in `MasterDbContext` and `AvailabilityDbContext`. There is no foreign key between them — all three contexts share one database, but the relationship is behavioural, not relational. Losing the keys does not produce a referential-integrity error; it produces rows whose encrypted column can no longer be read. That is exactly why the failure is silent. + +--- + +## DataProtectionKey (new) + +Provided by the framework via `IDataProtectionKeyContext`; the schema is not authored by this project. + +| Field | Type | Purpose | +|---|---|---| +| `Id` | int, identity | Primary key | +| `FriendlyName` | string, nullable | Human-readable key identifier | +| `Xml` | string | The serialized key material | + +### Constraints and rules + +| Aspect | Rule | +|---|---| +| Owning context | `ApplicationDbContext` (Q7 of Application Design = A) | +| Migration | One new Core migration, applied automatically at startup by FR-11 | +| Encryption at rest | **None** — see DEV-05. `Xml` contains usable key material in plain text | +| Retention | Rows are **never** deleted (BR-U2-06) | +| Rotation | Framework default, 90 days; new rows are added, old rows retained | +| Access | Only through the Data Protection API. No application code reads or writes this table directly | + +**Why `Xml` being plaintext matters**: anyone who can read this table can decrypt every stored slave API key. This is the substance of DEV-05, and why BR-U2-08 requires TLS on the connection and a database that is not publicly reachable. Those compensating controls are not optional extras — they are what makes the deviation acceptable. + +--- + +## ApplicationDbContext (modified) + +| Change | Detail | +|---|---| +| Interface | Implements `IDataProtectionKeyContext` | +| New set | `DbSet DataProtectionKeys` | +| Existing sets | Unchanged — Identity, `RefreshToken`, `Invitation`, `ModulePermission`, `GlobalAvailabilityState` | +| Migration behaviour | **Changed**: now migrates automatically at startup (FR-11). Previously required a manual `dotnet ef database update` | + +**Note on the behaviour change**: automatic migration is a genuine change in operational semantics, not merely a convenience. Previously a schema change reached production only when a human ran a command; now it happens whenever a new release starts. This is why forward-compatible, non-destructive migrations (BR-U2-16) and a pre-deploy backup (FR-20) are load-bearing rather than nice to have. + +--- + +## Configuration Values + +U2 introduces **no new `appsettings` section**. + +| Value | Source | Rationale | +|---|---|---| +| Application discriminator | **Constant in code** | Q1 = A. Cannot be misconfigured, forgotten, or made to differ between instances sharing a database | +| Key lifetime | Framework default (90 days) | Q5 = C. No reason to differ | +| Migration retry attempts and delays | Constants in code | Values chosen to cover a host-reboot window; not an operational tuning knob | +| Connection string | Existing `ConnectionStrings:DefaultConnection` | Unchanged. BR-U2-08 requires TLS to be enforced in it | + +**Why nothing is configurable here**: every value in this unit exists to prevent a silent failure. A configuration surface would be a way to reintroduce that failure — a discriminator set wrong on one instance, or a key lifetime set so short that rotation outpaces retention. + +--- + +## Persistence Summary + +| Question | Answer | +|---|---| +| New tables? | One — the Data Protection keys table | +| New migrations? | One, in `SlpModularCms.Core` | +| Modified entities? | None. `ApplicationDbContext` gains a set but no existing entity changes | +| Destructive schema changes? | None. Purely additive, so rollback by redeploying an earlier release stays safe | +| New configuration? | None | + +--- + +## Verification Targets + +What this unit's tests must actually prove, given that the failure mode is silent: + +| Target | Why it needs asserting | +|---|---| +| The persistent key store survives module registration | Registration alone passes in both the broken and fixed cases — only the resulting configuration distinguishes them | +| The application discriminator is the fixed constant | The default would change per release directory, defeating persistence | +| A protected value round-trips across a simulated content-root change | This is the actual user-visible property: an API key encrypted before a deploy is still readable after it | +| Neither module registers Data Protection | Prevents the conflict from being reintroduced by a future change to either module | +| A connection failure retries; a migration failure does not | The two paths differ deliberately (BR-U2-11, BR-U2-12) | +| Both hosts still start | `SlpModularCms.Api.Slave` has no test project and is a reference instance (Q2 of Application Design = A) | diff --git a/src/SlpModularCms.Api.Slave/Program.cs b/src/SlpModularCms.Api.Slave/Program.cs index ba5fa60..9b65f9c 100644 --- a/src/SlpModularCms.Api.Slave/Program.cs +++ b/src/SlpModularCms.Api.Slave/Program.cs @@ -1,4 +1,5 @@ using SlpModularCms.Core.Hosting; +using SlpModularCms.Core.Hosting.Health; using Scalar.AspNetCore; var builder = WebApplication.CreateBuilder(args); @@ -15,6 +16,10 @@ orchestrator.DiscoverModules(); builder.Services.AddCoreInfrastructure(builder.Configuration); builder.Services.AddCmsCors(builder.Configuration); builder.Services.AddCmsRateLimiting(builder.Configuration); +builder.Services.AddCmsHealthChecks(); + +// Registered BEFORE module services — see the note in DataProtectionExtensions. +builder.Services.AddCmsDataProtection(); // 3. Add Module Services orchestrator.RegisterModuleServices(builder.Services); @@ -32,6 +37,9 @@ builder.Services.AddControllers(options => var app = builder.Build(); +// Same as the master host: Core schema first, fail fast on failure. +app.MigrateCoreDatabase(); + // 5. Global Exception Handling app.UseExceptionHandler(); @@ -56,4 +64,9 @@ app.UseAuthorization(); app.MapControllers(); +// Infrastructure liveness, same as the master host. This instance serves no static content, +// so it gets no website or admin mounts — but it is a reference for what a customer-facing +// API instance looks like, so it behaves like one in every other respect. +app.MapCmsHealthChecks(); + app.Run(); diff --git a/src/SlpModularCms.Api/Program.cs b/src/SlpModularCms.Api/Program.cs index 582ff69..9002f43 100644 --- a/src/SlpModularCms.Api/Program.cs +++ b/src/SlpModularCms.Api/Program.cs @@ -1,4 +1,6 @@ +using SlpModularCms.Api.Extensions; using SlpModularCms.Core.Hosting; +using SlpModularCms.Core.Hosting.Health; using Scalar.AspNetCore; var builder = WebApplication.CreateBuilder(args); @@ -15,6 +17,12 @@ orchestrator.DiscoverModules(); builder.Services.AddCoreInfrastructure(builder.Configuration); builder.Services.AddCmsCors(builder.Configuration); builder.Services.AddCmsRateLimiting(builder.Configuration); +builder.Services.AddCmsHealthChecks(); + +// Registered BEFORE module services: modules must not configure Data Protection themselves, +// because a later registration would override this persistent key store (see +// DataProtectionExtensions). +builder.Services.AddCmsDataProtection(); // 3. Add Module Services orchestrator.RegisterModuleServices(builder.Services); @@ -32,6 +40,13 @@ builder.Services.AddControllers(options => var app = builder.Build(); +// Bring the Core schema up to date before serving any traffic. Runs before the module +// middleware below, because the Data Protection keys table lives in this context and the +// modules resolve an IDataProtector as soon as they start. Fails fast: a host that cannot +// migrate does not start, so /health goes silent and monitoring goes red — which is exactly +// what makes a liveness-only health check trustworthy. +app.MigrateCoreDatabase(); + // 5. Global Exception Handling app.UseExceptionHandler(); @@ -47,10 +62,11 @@ if (app.Environment.IsDevelopment()) app.UseHttpsRedirection(); // Serve the public website ('/') and the CMS admin SPA ('/admin') from wwwroot. -// wwwroot/index.html + assets -> public website (built and deployed separately, not part of this repo) +// wwwroot/web/index.html + assets -> public website (built and deployed separately, not part of this repo) // wwwroot/admin/index.html + assets -> CMS admin build (see frontend/, copied in on publish) -app.UseDefaultFiles(); -app.UseStaticFiles(); +// Registered before the module middleware below: static files short-circuit the pipeline, so +// anything that must observe them has to come first. +app.UseCmsStaticContent(); app.UseCors(); @@ -62,9 +78,14 @@ app.UseAuthorization(); app.MapControllers(); +// Infrastructure liveness. Deliberately outside /api/v1 and on the availability gate's bypass +// list: this reports whether the process is alive, which is a different question from whether +// the CMS is switched on (/api/v1/Availability/status) or which modules it carries +// (/api/v1/System/capabilities). Those are CMS domain state and must not be used for monitoring. +app.MapCmsHealthChecks(); + // SPA fallbacks so client-side routes (e.g. /admin/dashboard) resolve to the right index.html // instead of 404ing. The "nonfile" constraint keeps genuinely missing assets (e.g. /admin/assets/x.js) as 404s. -app.MapFallbackToFile("/admin/{*path:nonfile}", "admin/index.html"); -app.MapFallbackToFile("{*path:nonfile}", "index.html"); +app.MapCmsSpaFallbacks(); app.Run(); diff --git a/src/SlpModularCms.Core.Tests/Hosting/DataProtectionExtensionsTests.cs b/src/SlpModularCms.Core.Tests/Hosting/DataProtectionExtensionsTests.cs new file mode 100644 index 0000000..ca23759 --- /dev/null +++ b/src/SlpModularCms.Core.Tests/Hosting/DataProtectionExtensionsTests.cs @@ -0,0 +1,97 @@ +using FluentAssertions; +using Microsoft.AspNetCore.DataProtection; +using Microsoft.AspNetCore.DataProtection.EntityFrameworkCore; +using Microsoft.AspNetCore.DataProtection.KeyManagement; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using SlpModularCms.Core.Data; +using SlpModularCms.Core.Hosting; +using Xunit; + +namespace SlpModularCms.Core.Tests.Hosting; + +/// +/// Guards the durability of the Data Protection key ring. +/// +/// +/// The failure this protects against is silent. Losing the key ring produces no error — it +/// produces stored slave API keys that no longer decrypt, which looks like a network fault +/// between a Master and its slaves. A test that merely asserted "Data Protection is registered" +/// would pass in the broken case too, so these tests assert the resulting configuration instead. +/// +public class DataProtectionExtensionsTests +{ + [Fact] + public void AddCmsDataProtection_ShouldPersistKeysToTheDatabase() + { + using var provider = BuildProvider(nameof(AddCmsDataProtection_ShouldPersistKeysToTheDatabase)); + + var options = provider.GetRequiredService>().Value; + + // The default is a filesystem key ring, which every atomic release switch discards. + options.XmlRepository.Should().BeOfType>(); + } + + [Fact] + public void AddCmsDataProtection_ShouldUseAFixedApplicationDiscriminator() + { + using var provider = BuildProvider(nameof(AddCmsDataProtection_ShouldUseAFixedApplicationDiscriminator)); + + var options = provider.GetRequiredService>().Value; + + // The default derives from the content root path, which changes with every release + // directory — so keys stored in the database would still stop being derivable. + options.ApplicationDiscriminator.Should().Be(DataProtectionExtensions.ApplicationDiscriminator); + } + + [Fact] + public void ProtectedValues_ShouldSurviveAProcessRestartFromADifferentDirectory() + { + // The property that actually matters: an API key encrypted before a deploy must still be + // readable by the process that starts afterwards from a different release directory. + // Both providers share one database and one fixed discriminator, which is what makes + // that possible. + const string databaseName = nameof(ProtectedValues_ShouldSurviveAProcessRestartFromADifferentDirectory); + const string secret = "slave-api-key-value"; + + string encrypted; + using (var beforeDeploy = BuildProvider(databaseName)) + { + encrypted = beforeDeploy + .GetRequiredService() + .CreateProtector("MasterApiKey") + .Protect(secret); + } + + using var afterDeploy = BuildProvider(databaseName); + + var decrypted = afterDeploy + .GetRequiredService() + .CreateProtector("MasterApiKey") + .Unprotect(encrypted); + + decrypted.Should().Be(secret); + } + + [Fact] + public void ApplicationDiscriminator_ShouldNotBeDerivedFromAPath() + { + // Guards against someone "improving" this into a configurable or computed value: every + // knob here is a way to reintroduce the silent failure the key ring exists to prevent. + DataProtectionExtensions.ApplicationDiscriminator.Should().Be("SlpModularCms"); + DataProtectionExtensions.ApplicationDiscriminator.Should().NotContainAny("/", "\\", ":"); + } + + private static ServiceProvider BuildProvider(string databaseName) + { + var services = new ServiceCollection(); + + services.AddLogging(builder => builder.SetMinimumLevel(LogLevel.None)); + services.AddDbContext(options => options.UseInMemoryDatabase(databaseName)); + services.AddCmsDataProtection(); + + return services.BuildServiceProvider(); + } +} diff --git a/src/SlpModularCms.Core.Tests/Hosting/DatabaseMigrationExtensionsTests.cs b/src/SlpModularCms.Core.Tests/Hosting/DatabaseMigrationExtensionsTests.cs new file mode 100644 index 0000000..28e452e --- /dev/null +++ b/src/SlpModularCms.Core.Tests/Hosting/DatabaseMigrationExtensionsTests.cs @@ -0,0 +1,95 @@ +using FluentAssertions; +using Microsoft.Data.SqlClient; +using SlpModularCms.Core.Hosting; +using Xunit; + +namespace SlpModularCms.Core.Tests.Hosting; + +/// +/// Guards the failure classification of the startup migration. +/// +/// +/// Startup migration distinguishes two failures that mean very different things: +/// a database that is not up yet (normal when the application and the database server start +/// together after a host reboot) versus a migration that is broken. Retrying the first is +/// correct; retrying the second only delays the inevitable and fills the log. +/// +/// MigrateCoreDatabase itself needs a composed WebApplication, so the classifier is +/// exercised directly here; the composed startup path is verified at the phase-level Build and +/// Test stage by starting both hosts. +/// +public class DatabaseMigrationExtensionsTests +{ + [Fact] + public void MaxConnectionAttempts_ShouldAllowForAHostRebootWindow() + { + // Enough attempts that a database server starting alongside the application is tolerated, + // few enough that a genuinely unreachable database still fails promptly. + DatabaseMigrationExtensions.MaxConnectionAttempts.Should().BeGreaterThan(1); + DatabaseMigrationExtensions.MaxConnectionAttempts.Should().BeLessThanOrEqualTo(10); + } + + [Theory] + [MemberData(nameof(TransientFailures))] + public void IsTransientConnectionFailure_ShouldRecogniseConnectionProblems(Exception exception) + { + InvokeClassifier(exception).Should().BeTrue(); + } + + [Theory] + [MemberData(nameof(NonTransientFailures))] + public void IsTransientConnectionFailure_ShouldNotRecogniseMigrationProblems(Exception exception) + { + // A broken migration must fail immediately. Classifying it as transient would hide a + // real fault behind a retry loop. + InvokeClassifier(exception).Should().BeFalse(); + } + + public static TheoryData TransientFailures() => + [ + MakeSqlException(), + new TimeoutException("Connect Timeout expired."), + new InvalidOperationException("wrapper", MakeSqlException()), + ]; + + public static TheoryData NonTransientFailures() => + [ + new InvalidOperationException("There is already an object named 'Users' in the database."), + new NotSupportedException("The migration cannot be applied."), + new AggregateException(new InvalidOperationException("pending model changes")), + ]; + + private static bool InvokeClassifier(Exception exception) + { + var method = typeof(DatabaseMigrationExtensions).GetMethod( + "IsTransientConnectionFailure", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static); + + method.Should().NotBeNull("the failure classifier is the behaviour under test"); + + return (bool)method!.Invoke(null, [exception])!; + } + + /// + /// has no public constructor, so one is produced through the + /// framework's own factory path via reflection. + /// + private static Exception MakeSqlException() + { + try + { + // Deliberately unreachable host and a very short timeout: this genuinely produces a + // SqlException rather than a hand-built stand-in, so the classifier is tested against + // the real type it will encounter in production. + using var connection = new SqlConnection( + "Server=localhost,9;Database=none;User Id=sa;Password=none;Connect Timeout=1;TrustServerCertificate=True"); + connection.Open(); + } + catch (Exception ex) + { + return ex; + } + + throw new InvalidOperationException("Expected the connection attempt to fail."); + } +} diff --git a/src/SlpModularCms.Core/Data/ApplicationDbContext.cs b/src/SlpModularCms.Core/Data/ApplicationDbContext.cs index 9cdf9c8..fa867e6 100644 --- a/src/SlpModularCms.Core/Data/ApplicationDbContext.cs +++ b/src/SlpModularCms.Core/Data/ApplicationDbContext.cs @@ -1,3 +1,4 @@ +using Microsoft.AspNetCore.DataProtection.EntityFrameworkCore; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Identity.EntityFrameworkCore; using Microsoft.EntityFrameworkCore; @@ -8,7 +9,12 @@ namespace SlpModularCms.Core.Data; /// /// Database context voor de applicatie, inclusief Identity en RBAC tabellen. /// -public class ApplicationDbContext : IdentityDbContext +/// +/// Also hosts the ASP.NET Core Data Protection key ring. The keys are application-wide +/// infrastructure rather than module-owned data, and this context is the one that migrates +/// automatically at startup — so the table exists without any manual step on the host. +/// +public class ApplicationDbContext : IdentityDbContext, IDataProtectionKeyContext { public ApplicationDbContext(DbContextOptions options) : base(options) @@ -20,6 +26,13 @@ public class ApplicationDbContext : IdentityDbContext ModulePermissions => Set(); public DbSet AvailabilityStates => Set(); + /// + /// Data Protection key ring. Rows here MUST NEVER be pruned: deleting a key makes every + /// value ever encrypted with it permanently unreadable, including the stored API keys that + /// authenticate master/slave communication. + /// + public DbSet DataProtectionKeys => Set(); + protected override void OnModelCreating(ModelBuilder builder) { base.OnModelCreating(builder); diff --git a/src/SlpModularCms.Core/Hosting/DataProtectionExtensions.cs b/src/SlpModularCms.Core/Hosting/DataProtectionExtensions.cs new file mode 100644 index 0000000..20875be --- /dev/null +++ b/src/SlpModularCms.Core/Hosting/DataProtectionExtensions.cs @@ -0,0 +1,58 @@ +using Microsoft.AspNetCore.DataProtection; +using Microsoft.Extensions.DependencyInjection; +using SlpModularCms.Core.Data; + +namespace SlpModularCms.Core.Hosting; + +/// +/// Configures ASP.NET Core Data Protection so encrypted values survive a redeploy. +/// +/// +/// Data Protection secures the API keys that authenticate master/slave communication. Losing the +/// key ring does not produce an error — it produces stored keys that no longer decrypt, which +/// presents as an apparent network fault between a Master and its slaves and is easily +/// misdiagnosed. Two separate defaults would each cause exactly that: +/// +/// 1. Keys are held on the filesystem by default. Deployments swap the release directory +/// atomically, so a filesystem key ring is discarded on every deploy. +/// 2. The application discriminator is derived from the content root path by default. That path +/// changes with every release directory, so even keys stored in the database would stop being +/// derivable. +/// +/// This method closes both. It MUST be called before module service registration — see the +/// remarks on . +/// +public static class DataProtectionExtensions +{ + /// + /// Stable identity of this application for key derivation. + /// + /// + /// A constant rather than a configuration value, deliberately. Every configurable knob here + /// is a way to reintroduce the silent failure this whole mechanism exists to prevent — a + /// discriminator set differently on one instance, or forgotten during a host migration, + /// makes previously encrypted values unreadable with no error to point at. + /// + public const string ApplicationDiscriminator = "SlpModularCms"; + + /// + /// Registers Data Protection with a database-backed key ring and a fixed application + /// discriminator. + /// + /// + /// MUST be called before ModuleOrchestrator.RegisterModuleServices. Modules must not + /// call AddDataProtection() themselves: module registration runs after the host's, and + /// a later bare call re-registers the configuration chain, silently discarding the persistent + /// key store configured here. IDataProtector resolves either way, so such a regression + /// passes registration tests and only surfaces after the first release switch. + /// + public static IServiceCollection AddCmsDataProtection(this IServiceCollection services) + { + services + .AddDataProtection() + .SetApplicationName(ApplicationDiscriminator) + .PersistKeysToDbContext(); + + return services; + } +} diff --git a/src/SlpModularCms.Core/Hosting/DatabaseMigrationExtensions.cs b/src/SlpModularCms.Core/Hosting/DatabaseMigrationExtensions.cs new file mode 100644 index 0000000..9b4ec87 --- /dev/null +++ b/src/SlpModularCms.Core/Hosting/DatabaseMigrationExtensions.cs @@ -0,0 +1,112 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.Data.SqlClient; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using SlpModularCms.Core.Data; + +namespace SlpModularCms.Core.Hosting; + +/// +/// Applies the Core database migrations at startup. +/// +/// +/// Deployment targets are shared hosts where no CLI is available, so migrations cannot be a +/// manual step on the server. Applying them at startup makes a deployment self-contained. +/// +/// The cost of that convenience is that a migration now runs without a human gate — which is why +/// migrations must stay forward-compatible and non-destructive (rollback is "redeploy the previous +/// release", and that only works if older code can run against the newer schema), and why a +/// database backup precedes every production deploy. +/// +public static class DatabaseMigrationExtensions +{ + /// Attempts made when the database is not reachable yet. + public const int MaxConnectionAttempts = 5; + + private static readonly TimeSpan BaseRetryDelay = TimeSpan.FromSeconds(2); + + /// + /// Applies pending migrations before the application + /// serves traffic. + /// + /// + /// Failures are classified rather than treated alike: + /// + /// A connection failure means the database is not up yet — normal + /// when the application and the database server start together after a host reboot. Retried + /// with increasing delay. + /// A migration failure means a migration is invalid or conflicts. + /// Retrying only delays the inevitable and fills the log, so it fails at once. + /// + /// Either way the exception ultimately propagates and the process does not start. That is + /// deliberate and is what makes the liveness health check meaningful: an application that + /// cannot reach its schema never answers /health, so monitoring goes red instead of + /// reporting a healthy instance that cannot serve a single request. + /// + public static WebApplication MigrateCoreDatabase(this WebApplication app) + { + ArgumentNullException.ThrowIfNull(app); + + var logger = app.Services.GetRequiredService() + .CreateLogger(typeof(DatabaseMigrationExtensions).FullName!); + + using var scope = app.Services.CreateScope(); + var context = scope.ServiceProvider.GetRequiredService(); + + for (var attempt = 1; ; attempt++) + { + try + { + context.Database.Migrate(); + logger.LogInformation("Core database migrations applied successfully."); + return app; + } + catch (Exception ex) when (IsTransientConnectionFailure(ex) && attempt < MaxConnectionAttempts) + { + var delay = BaseRetryDelay * attempt; + + logger.LogWarning( + "Database not reachable on attempt {Attempt} of {MaxAttempts}. Retrying in {DelaySeconds}s. Reason: {Reason}", + attempt, + MaxConnectionAttempts, + delay.TotalSeconds, + ex.Message); + + Thread.Sleep(delay); + } + catch (Exception ex) + { + // Logged with the failure reason but never the connection string or credentials — + // this message travels to the console and to Sentry. + logger.LogCritical( + ex, + "Core database migration failed after {Attempts} attempt(s). The application will not start.", + attempt); + + throw; + } + } + } + + /// + /// Distinguishes "the database is not there yet" from "the migration is broken". + /// + /// + /// Wrong credentials are treated as a connection failure too. Telling them apart from an + /// unreachable server would add branching for no benefit: the outcome is identical — retries + /// are exhausted and the process does not start. + /// + private static bool IsTransientConnectionFailure(Exception exception) + { + for (var current = exception; current is not null; current = current.InnerException) + { + if (current is SqlException or TimeoutException) + { + return true; + } + } + + return false; + } +} diff --git a/src/SlpModularCms.Core/Migrations/20260727203036_AddDataProtectionKeys.Designer.cs b/src/SlpModularCms.Core/Migrations/20260727203036_AddDataProtectionKeys.Designer.cs new file mode 100644 index 0000000..0d7a048 --- /dev/null +++ b/src/SlpModularCms.Core/Migrations/20260727203036_AddDataProtectionKeys.Designer.cs @@ -0,0 +1,452 @@ +// +using System; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; +using SlpModularCms.Core.Data; + +#nullable disable + +namespace SlpModularCms.Core.Migrations +{ + [DbContext(typeof(ApplicationDbContext))] + [Migration("20260727203036_AddDataProtectionKeys")] + partial class AddDataProtectionKeys + { + /// + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder + .HasAnnotation("ProductVersion", "10.0.9") + .HasAnnotation("Relational:MaxIdentifierLength", 128); + + SqlServerModelBuilderExtensions.UseIdentityColumns(modelBuilder); + + modelBuilder.Entity("Microsoft.AspNetCore.DataProtection.EntityFrameworkCore.DataProtectionKey", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("int"); + + SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property("Id")); + + b.Property("FriendlyName") + .HasColumnType("nvarchar(max)"); + + b.Property("Xml") + .HasColumnType("nvarchar(max)"); + + b.HasKey("Id"); + + b.ToTable("DataProtectionKeys"); + }); + + modelBuilder.Entity("Microsoft.AspNetCore.Identity.IdentityRoleClaim", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("int"); + + SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property("Id")); + + b.Property("ClaimType") + .HasColumnType("nvarchar(max)"); + + b.Property("ClaimValue") + .HasColumnType("nvarchar(max)"); + + b.Property("RoleId") + .HasColumnType("uniqueidentifier"); + + b.HasKey("Id"); + + b.HasIndex("RoleId"); + + b.ToTable("RoleClaims", (string)null); + }); + + modelBuilder.Entity("Microsoft.AspNetCore.Identity.IdentityUserClaim", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("int"); + + SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property("Id")); + + b.Property("ClaimType") + .HasColumnType("nvarchar(max)"); + + b.Property("ClaimValue") + .HasColumnType("nvarchar(max)"); + + b.Property("UserId") + .HasColumnType("uniqueidentifier"); + + b.HasKey("Id"); + + b.HasIndex("UserId"); + + b.ToTable("UserClaims", (string)null); + }); + + modelBuilder.Entity("Microsoft.AspNetCore.Identity.IdentityUserLogin", b => + { + b.Property("LoginProvider") + .HasColumnType("nvarchar(450)"); + + b.Property("ProviderKey") + .HasColumnType("nvarchar(450)"); + + b.Property("ProviderDisplayName") + .HasColumnType("nvarchar(max)"); + + b.Property("UserId") + .HasColumnType("uniqueidentifier"); + + b.HasKey("LoginProvider", "ProviderKey"); + + b.HasIndex("UserId"); + + b.ToTable("UserLogins", (string)null); + }); + + modelBuilder.Entity("Microsoft.AspNetCore.Identity.IdentityUserRole", b => + { + b.Property("UserId") + .HasColumnType("uniqueidentifier"); + + b.Property("RoleId") + .HasColumnType("uniqueidentifier"); + + b.HasKey("UserId", "RoleId"); + + b.HasIndex("RoleId"); + + b.ToTable("UserRoles", (string)null); + }); + + modelBuilder.Entity("Microsoft.AspNetCore.Identity.IdentityUserToken", b => + { + b.Property("UserId") + .HasColumnType("uniqueidentifier"); + + b.Property("LoginProvider") + .HasColumnType("nvarchar(450)"); + + b.Property("Name") + .HasColumnType("nvarchar(450)"); + + b.Property("Value") + .HasColumnType("nvarchar(max)"); + + b.HasKey("UserId", "LoginProvider", "Name"); + + b.ToTable("UserTokens", (string)null); + }); + + modelBuilder.Entity("SlpModularCms.Core.Identity.Entities.ApplicationRole", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uniqueidentifier"); + + b.Property("ConcurrencyStamp") + .IsConcurrencyToken() + .HasColumnType("nvarchar(max)"); + + b.Property("Name") + .HasMaxLength(256) + .HasColumnType("nvarchar(256)"); + + b.Property("NormalizedName") + .HasMaxLength(256) + .HasColumnType("nvarchar(256)"); + + b.HasKey("Id"); + + b.HasIndex("NormalizedName") + .IsUnique() + .HasDatabaseName("RoleNameIndex") + .HasFilter("[NormalizedName] IS NOT NULL"); + + b.ToTable("Roles", (string)null); + }); + + modelBuilder.Entity("SlpModularCms.Core.Identity.Entities.ApplicationUser", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uniqueidentifier"); + + b.Property("AccessFailedCount") + .HasColumnType("int"); + + b.Property("ConcurrencyStamp") + .IsConcurrencyToken() + .HasColumnType("nvarchar(max)"); + + b.Property("CreatedAt") + .HasColumnType("datetimeoffset"); + + b.Property("DisplayName") + .HasColumnType("nvarchar(max)"); + + b.Property("Email") + .HasMaxLength(256) + .HasColumnType("nvarchar(256)"); + + b.Property("EmailConfirmed") + .HasColumnType("bit"); + + b.Property("IsActive") + .HasColumnType("bit"); + + b.Property("LockoutEnabled") + .HasColumnType("bit"); + + b.Property("LockoutEnd") + .HasColumnType("datetimeoffset"); + + b.Property("NormalizedEmail") + .HasMaxLength(256) + .HasColumnType("nvarchar(256)"); + + b.Property("NormalizedUserName") + .HasMaxLength(256) + .HasColumnType("nvarchar(256)"); + + b.Property("PasswordHash") + .HasColumnType("nvarchar(max)"); + + b.Property("PhoneNumber") + .HasColumnType("nvarchar(max)"); + + b.Property("PhoneNumberConfirmed") + .HasColumnType("bit"); + + b.Property("SecurityStamp") + .HasColumnType("nvarchar(max)"); + + b.Property("TwoFactorEnabled") + .HasColumnType("bit"); + + b.Property("UserName") + .HasMaxLength(256) + .HasColumnType("nvarchar(256)"); + + b.HasKey("Id"); + + b.HasIndex("NormalizedEmail") + .HasDatabaseName("EmailIndex"); + + b.HasIndex("NormalizedUserName") + .IsUnique() + .HasDatabaseName("UserNameIndex") + .HasFilter("[NormalizedUserName] IS NOT NULL"); + + b.ToTable("Users", (string)null); + }); + + modelBuilder.Entity("SlpModularCms.Core.Identity.Entities.GlobalAvailabilityState", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uniqueidentifier"); + + b.Property("LastUpdatedAt") + .HasColumnType("datetimeoffset"); + + b.Property("Message") + .HasColumnType("nvarchar(max)"); + + b.Property("Status") + .HasColumnType("int"); + + b.Property("UpdatedBy") + .HasColumnType("nvarchar(max)"); + + b.HasKey("Id"); + + b.ToTable("AvailabilityState", (string)null); + }); + + modelBuilder.Entity("SlpModularCms.Core.Identity.Entities.Invitation", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uniqueidentifier"); + + b.Property("CreatedAt") + .HasColumnType("datetimeoffset"); + + b.Property("Email") + .IsRequired() + .HasMaxLength(256) + .HasColumnType("nvarchar(256)"); + + b.Property("ExpiryDate") + .HasColumnType("datetimeoffset"); + + b.Property("IsAccepted") + .HasColumnType("bit"); + + b.Property("Role") + .IsRequired() + .HasColumnType("nvarchar(max)"); + + b.Property("Token") + .IsRequired() + .HasMaxLength(256) + .HasColumnType("nvarchar(256)"); + + b.HasKey("Id"); + + b.HasIndex("Token") + .IsUnique(); + + b.ToTable("Invitations"); + }); + + modelBuilder.Entity("SlpModularCms.Core.Identity.Entities.ModulePermission", b => + { + b.Property("UserId") + .HasColumnType("uniqueidentifier"); + + b.Property("ModuleName") + .HasMaxLength(128) + .HasColumnType("nvarchar(128)"); + + b.Property("Permission") + .HasMaxLength(128) + .HasColumnType("nvarchar(128)"); + + b.HasKey("UserId", "ModuleName", "Permission"); + + b.ToTable("ModulePermissions"); + }); + + modelBuilder.Entity("SlpModularCms.Core.Identity.Entities.RefreshToken", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uniqueidentifier"); + + b.Property("CreatedAt") + .HasColumnType("datetimeoffset"); + + b.Property("CreatedByIp") + .HasColumnType("nvarchar(max)"); + + b.Property("ExpiryDate") + .HasColumnType("datetimeoffset"); + + b.Property("IsRevoked") + .HasColumnType("bit"); + + b.Property("IsUsed") + .HasColumnType("bit"); + + b.Property("Token") + .IsRequired() + .HasMaxLength(256) + .HasColumnType("nvarchar(256)"); + + b.Property("UserId") + .HasColumnType("uniqueidentifier"); + + b.HasKey("Id"); + + b.HasIndex("Token") + .IsUnique(); + + b.HasIndex("UserId"); + + b.ToTable("RefreshTokens"); + }); + + modelBuilder.Entity("Microsoft.AspNetCore.Identity.IdentityRoleClaim", b => + { + b.HasOne("SlpModularCms.Core.Identity.Entities.ApplicationRole", null) + .WithMany() + .HasForeignKey("RoleId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + }); + + modelBuilder.Entity("Microsoft.AspNetCore.Identity.IdentityUserClaim", b => + { + b.HasOne("SlpModularCms.Core.Identity.Entities.ApplicationUser", null) + .WithMany() + .HasForeignKey("UserId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + }); + + modelBuilder.Entity("Microsoft.AspNetCore.Identity.IdentityUserLogin", b => + { + b.HasOne("SlpModularCms.Core.Identity.Entities.ApplicationUser", null) + .WithMany() + .HasForeignKey("UserId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + }); + + modelBuilder.Entity("Microsoft.AspNetCore.Identity.IdentityUserRole", b => + { + b.HasOne("SlpModularCms.Core.Identity.Entities.ApplicationRole", null) + .WithMany() + .HasForeignKey("RoleId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.HasOne("SlpModularCms.Core.Identity.Entities.ApplicationUser", null) + .WithMany() + .HasForeignKey("UserId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + }); + + modelBuilder.Entity("Microsoft.AspNetCore.Identity.IdentityUserToken", b => + { + b.HasOne("SlpModularCms.Core.Identity.Entities.ApplicationUser", null) + .WithMany() + .HasForeignKey("UserId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + }); + + modelBuilder.Entity("SlpModularCms.Core.Identity.Entities.ModulePermission", b => + { + b.HasOne("SlpModularCms.Core.Identity.Entities.ApplicationUser", "User") + .WithMany("ModulePermissions") + .HasForeignKey("UserId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("User"); + }); + + modelBuilder.Entity("SlpModularCms.Core.Identity.Entities.RefreshToken", b => + { + b.HasOne("SlpModularCms.Core.Identity.Entities.ApplicationUser", "User") + .WithMany("RefreshTokens") + .HasForeignKey("UserId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("User"); + }); + + modelBuilder.Entity("SlpModularCms.Core.Identity.Entities.ApplicationUser", b => + { + b.Navigation("ModulePermissions"); + + b.Navigation("RefreshTokens"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/src/SlpModularCms.Core/Migrations/20260727203036_AddDataProtectionKeys.cs b/src/SlpModularCms.Core/Migrations/20260727203036_AddDataProtectionKeys.cs new file mode 100644 index 0000000..89442e7 --- /dev/null +++ b/src/SlpModularCms.Core/Migrations/20260727203036_AddDataProtectionKeys.cs @@ -0,0 +1,35 @@ +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace SlpModularCms.Core.Migrations +{ + /// + public partial class AddDataProtectionKeys : Migration + { + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.CreateTable( + name: "DataProtectionKeys", + columns: table => new + { + Id = table.Column(type: "int", nullable: false) + .Annotation("SqlServer:Identity", "1, 1"), + FriendlyName = table.Column(type: "nvarchar(max)", nullable: true), + Xml = table.Column(type: "nvarchar(max)", nullable: true) + }, + constraints: table => + { + table.PrimaryKey("PK_DataProtectionKeys", x => x.Id); + }); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropTable( + name: "DataProtectionKeys"); + } + } +} diff --git a/src/SlpModularCms.Core/Migrations/ApplicationDbContextModelSnapshot.cs b/src/SlpModularCms.Core/Migrations/ApplicationDbContextModelSnapshot.cs index e64daf1..6fb2f3b 100644 --- a/src/SlpModularCms.Core/Migrations/ApplicationDbContextModelSnapshot.cs +++ b/src/SlpModularCms.Core/Migrations/ApplicationDbContextModelSnapshot.cs @@ -1,6 +1,5 @@ // using System; -using System.Diagnostics.CodeAnalysis; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata; @@ -12,7 +11,6 @@ using SlpModularCms.Core.Data; namespace SlpModularCms.Core.Migrations { [DbContext(typeof(ApplicationDbContext))] - [ExcludeFromCodeCoverage] partial class ApplicationDbContextModelSnapshot : ModelSnapshot { protected override void BuildModel(ModelBuilder modelBuilder) @@ -24,6 +22,25 @@ namespace SlpModularCms.Core.Migrations SqlServerModelBuilderExtensions.UseIdentityColumns(modelBuilder); + modelBuilder.Entity("Microsoft.AspNetCore.DataProtection.EntityFrameworkCore.DataProtectionKey", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("int"); + + SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property("Id")); + + b.Property("FriendlyName") + .HasColumnType("nvarchar(max)"); + + b.Property("Xml") + .HasColumnType("nvarchar(max)"); + + b.HasKey("Id"); + + b.ToTable("DataProtectionKeys"); + }); + modelBuilder.Entity("Microsoft.AspNetCore.Identity.IdentityRoleClaim", b => { b.Property("Id") diff --git a/src/SlpModularCms.Core/SlpModularCms.Core.csproj b/src/SlpModularCms.Core/SlpModularCms.Core.csproj index d18b061..e3b0653 100644 --- a/src/SlpModularCms.Core/SlpModularCms.Core.csproj +++ b/src/SlpModularCms.Core/SlpModularCms.Core.csproj @@ -13,6 +13,13 @@ + + diff --git a/src/SlpModularCms.Modules.Availability.Tests/AvailabilityModuleDataProtectionTests.cs b/src/SlpModularCms.Modules.Availability.Tests/AvailabilityModuleDataProtectionTests.cs new file mode 100644 index 0000000..98abc56 --- /dev/null +++ b/src/SlpModularCms.Modules.Availability.Tests/AvailabilityModuleDataProtectionTests.cs @@ -0,0 +1,52 @@ +using FluentAssertions; +using Microsoft.AspNetCore.DataProtection.EntityFrameworkCore; +using Microsoft.AspNetCore.DataProtection.KeyManagement; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using SlpModularCms.Core.Data; +using SlpModularCms.Core.Hosting; +using Xunit; + +namespace SlpModularCms.Modules.Availability.Tests; + +/// +/// Proves this module does not undo the host's Data Protection configuration. +/// +/// +/// This module used to call services.AddDataProtection() itself. Module registration runs +/// AFTER the host's, so that bare call re-registered the configuration chain and silently +/// discarded the host's database-backed key store — leaving the key ring on the filesystem, where +/// every deployment discards it. +/// +/// The defect was invisible: IDataProtector still resolved, so any test asserting that +/// Data Protection "is registered" passed. It only surfaced after a release switch, as stored +/// slave API keys that no longer decrypted — presenting as a network fault between Master and +/// slave. This test asserts the resulting configuration, which is the only thing that +/// distinguishes the two cases. +/// +public class AvailabilityModuleDataProtectionTests +{ + [Fact] + public void RegisterServices_ShouldNotOverrideTheHostsPersistentKeyStore() + { + var services = new ServiceCollection(); + + services.AddLogging(builder => builder.SetMinimumLevel(LogLevel.None)); + services.AddSingleton(new ConfigurationBuilder().Build()); + services.AddDbContext(options => + options.UseInMemoryDatabase(nameof(RegisterServices_ShouldNotOverrideTheHostsPersistentKeyStore))); + + // Host first, module second — the real ordering. + services.AddCmsDataProtection(); + new AvailabilityModule().RegisterServices(services); + + using var provider = services.BuildServiceProvider(); + var options = provider.GetRequiredService>().Value; + + options.XmlRepository.Should().BeOfType>( + "the module must not reconfigure Data Protection — the host owns it"); + } +} diff --git a/src/SlpModularCms.Modules.Availability/AvailabilityModule.cs b/src/SlpModularCms.Modules.Availability/AvailabilityModule.cs index 5ac9b16..9c8dd72 100644 --- a/src/SlpModularCms.Modules.Availability/AvailabilityModule.cs +++ b/src/SlpModularCms.Modules.Availability/AvailabilityModule.cs @@ -31,7 +31,12 @@ public class AvailabilityModule : IModule options.UseSqlServer(configuration.GetConnectionString("DefaultConnection")); }); - services.AddDataProtection(); + // Data Protection is configured once by the host (AddCmsDataProtection), NOT here. + // Module registration runs after the host's, so a bare AddDataProtection() call at this + // point would re-register the configuration chain and silently discard the persistent + // database-backed key store — leaving the key ring on the filesystem, where every + // deployment discards it. IDataProtector resolves either way, so the regression would + // pass its tests and only surface later as slave API keys that no longer decrypt. services.AddSingleton(); services.AddScoped(); services.AddScoped(); diff --git a/src/SlpModularCms.Modules.Master.Tests/MasterModuleDataProtectionTests.cs b/src/SlpModularCms.Modules.Master.Tests/MasterModuleDataProtectionTests.cs new file mode 100644 index 0000000..cee71a7 --- /dev/null +++ b/src/SlpModularCms.Modules.Master.Tests/MasterModuleDataProtectionTests.cs @@ -0,0 +1,45 @@ +using FluentAssertions; +using Microsoft.AspNetCore.DataProtection.EntityFrameworkCore; +using Microsoft.AspNetCore.DataProtection.KeyManagement; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using SlpModularCms.Core.Data; +using SlpModularCms.Core.Hosting; +using Xunit; + +namespace SlpModularCms.Modules.Master.Tests; + +/// +/// Proves this module does not undo the host's Data Protection configuration. +/// +/// +/// See the equivalent test in the Availability module. This module encrypts the API keys of every +/// registered CMS instance, so if the key ring were silently returned to the filesystem, a single +/// deployment would make every registered slave unreachable — with no error to point at. +/// +public class MasterModuleDataProtectionTests +{ + [Fact] + public void RegisterServices_ShouldNotOverrideTheHostsPersistentKeyStore() + { + var services = new ServiceCollection(); + + services.AddLogging(builder => builder.SetMinimumLevel(LogLevel.None)); + services.AddSingleton(new ConfigurationBuilder().Build()); + services.AddDbContext(options => + options.UseInMemoryDatabase(nameof(RegisterServices_ShouldNotOverrideTheHostsPersistentKeyStore))); + + // Host first, module second — the real ordering. + services.AddCmsDataProtection(); + new MasterModule().RegisterServices(services); + + using var provider = services.BuildServiceProvider(); + var options = provider.GetRequiredService>().Value; + + options.XmlRepository.Should().BeOfType>( + "the module must not reconfigure Data Protection — the host owns it"); + } +} diff --git a/src/SlpModularCms.Modules.Master/MasterModule.cs b/src/SlpModularCms.Modules.Master/MasterModule.cs index 6789444..70b0d75 100644 --- a/src/SlpModularCms.Modules.Master/MasterModule.cs +++ b/src/SlpModularCms.Modules.Master/MasterModule.cs @@ -22,7 +22,10 @@ public class MasterModule : IModule public void RegisterServices(IServiceCollection services) { - services.AddDataProtection(); + // Data Protection is configured once by the host (AddCmsDataProtection), NOT here. + // See the equivalent note in AvailabilityModule: a bare AddDataProtection() call here + // would override the host's persistent key store, and the resulting defect is invisible + // until the first release switch makes every stored slave API key undecryptable. services.AddSingleton(); services.AddOptions().BindConfiguration("MasterModule");