diff --git a/.claude/skills/add-architecture-test/SKILL.md b/.claude/skills/add-architecture-test/SKILL.md index a7146b1..19414d4 100644 --- a/.claude/skills/add-architecture-test/SKILL.md +++ b/.claude/skills/add-architecture-test/SKILL.md @@ -120,7 +120,7 @@ The current set lives across these files; add yours to the right one: | `AuditTests.cs` | `AuditEntry` inheritance, no direct `audit_log` writes. | | `EntitlementTests.cs` | Plan-projected vs tenant-flag separation, FeatureKey registry. | | `PermissionTests.cs` | Closed action set, scope correctness, denied-test presence. | -| `HubContractTests.cs` | No direct Hub-URL references, four-endpoint surface enforcement. | +| `HubContractTests.cs` | No direct Hub-URL references; Hub clients only inside the named adapters (ADR-0034). | | `DomainGenericTests.cs` | `Core_Modules_HaveNo_DomainSpecific_Names`, no `Verticals/`. | | `DaprDirectInjectionTests.cs` | No `IConnectionMultiplexer` / `KafkaProducer` / `VaultClient` in modules. | | `ConventionTests.cs` | Strongly-typed ids in commands, validator pairing, etc. | @@ -154,7 +154,15 @@ public void Every_TenantOwned_Table_HasRls_With_AppTenantId() if (content.Contains("CREATE TABLE") && content.Contains("tenant_id")) { Assert.Contains("ENABLE ROW LEVEL SECURITY", content); - Assert.Contains("current_setting('app.tenant_id')", content); + // FORCE is the half that matters: without it the table owner bypasses + // its own policy and the whole layer is inert while ENABLE stays green. + // Matched as a regex because the canonical template writes two spaces. + Assert.Matches(@"FORCE\s+ROW LEVEL SECURITY", content); + // Must match the canonical template's exact shape. A bare + // current_setting('app.tenant_id') assertion FAILS against every + // correct migration and PASSES against the superseded one-argument + // form — see ADR-0003 Amendment 3 and 05-database.md. + Assert.Contains("NULLIF(current_setting('app.tenant_id', true), '')", content); } } } diff --git a/.claude/skills/add-audit-coverage/SKILL.md b/.claude/skills/add-audit-coverage/SKILL.md index 7c235da..3881c4f 100644 --- a/.claude/skills/add-audit-coverage/SKILL.md +++ b/.claude/skills/add-audit-coverage/SKILL.md @@ -16,11 +16,19 @@ description: > ## Purpose Wire a new operation into LearnStack's central audit pipeline -([ADR-0016](../../../docs/decisions/0016-audit-log-subsystem.md), +([ADR-0033](../../../docs/decisions/0033-audit-durability-model.md) — the binding +durability contract; [ADR-0016](../../../docs/decisions/0016-audit-log-subsystem.md) — +superseded, read only for subsystem context; [31-audit-subsystem.md](../../../docs/architecture/31-audit-subsystem.md), -[18-audit-coverage.md](../../../docs/standards/18-audit-coverage.md)) by extending -the module's matrix and the audit catalogue. Modules never write `audit_log` -directly; the catalogue + the `AuditLogBehavior` MediatR behaviour do. +[18-audit-coverage.md](../../../docs/standards/18-audit-coverage.md)) by extending the +module's matrix and the audit catalogue. Modules never write `audit_log` directly; the +catalogue plus the pipeline do. + +The pipeline is **decide → write → reconcile**: `AuditLogBehavior` classifies at step 3 +and parks an intent, `TransactionBehavior` writes the row on the business transaction +immediately before `COMMIT`, and `AuditLogBehavior` re-writes it standalone on the way out +if that transaction did not commit. You do not touch any of it — but the classification +you pick decides which of those paths a given operation takes. ## When to use @@ -91,6 +99,30 @@ Use the rules from If your operation falls outside this list, it probably belongs as a sub-resource (see [add-permission § Closed Action Set](../add-permission/SKILL.md)). +### Step 2b: What a MUST classification now costs + +Under [ADR-0033](../../../docs/decisions/0033-audit-durability-model.md) the class is +**load-bearing, not documentary**. Before you write MUST, know what you are buying: + +- The row is inserted on the **same transaction** as the business write, immediately + before `COMMIT`, while `app.tenant_id` is set — so it commits with the state change or + not at all, and Row Level Security accepts it. +- If the transaction rolls back, the row is **re-written standalone** with outcome + `failed`. A MUST-class operation is never left with no row, including on the ordinary + path where a handler saves and then returns `Result.Fail(...)`. +- The operation **fails closed**. If the audit row cannot be written at all, the command + is rejected — the caller gets `503 audit_unavailable`, never a partial success. +- MUST-class events with no business transaction — `denied` outcomes, read-sensitive + queries, non-mutating security events — get a standalone row in a short transaction + that sets its own tenant GUC. Classifying a *query* MUST is legitimate and costs a + synchronous write before the result is returned. +- A tenant `AuditConfig` override can narrow SHOULD/MAY. It can never remove baseline + MUST coverage; the catalogue re-applies the MUST floor after the override. +- SHOULD/MAY stays best-effort. Choosing it is choosing a **documented accepted loss** — + write that loss into the module's matrix rather than leaving it implied. + +So MUST is an availability trade as well as a compliance one. Classify deliberately. + ### Step 3: Register in the catalogue In the module's `RegisterAuditCoverage`: @@ -216,8 +248,17 @@ public async Task User_NationalId_isRedacted_In_AuditSnapshot() ## Common pitfalls -- **Calling `IAuditStore` directly from the handler.** Forbidden. The - `AuditLogBehavior` does this once; a second write produces a duplicate row. +- **Calling `IAuditStore` directly from the handler.** Forbidden. The pipeline does this + once per operation; a second write produces a duplicate row. `IAuditStore` is + infrastructure, not a handler collaborator — + `Modules_Do_Not_Write_AuditLog_Directly` enforces it. +- **Adding `AuditEntry` to a module's `DbContext`** so a handler can "enrol the row in + its own `SaveChanges`". Forbidden and unnecessary: atomicity comes from the + transaction, not from sharing a `SaveChanges` call, and mapping the Audit module's + aggregate into another module's context inverts the dependency direction. +- **`UPDATE`ing an audit row to add detail after the fact.** There is no second phase. + The row is composed complete at the commit boundary, `IAuditStore` has no update + method, and `learnstack_app` holds no `UPDATE` privilege on `audit_log`. - **Truncating snapshots silently.** If a `before/after` JSON is too large, store an external pointer (`audit_blob_id`); never an empty object. - **Skipping the matrix update.** `Module__HasAuditMatrix` will fail; CI diff --git a/.claude/skills/add-ef-migration/SKILL.md b/.claude/skills/add-ef-migration/SKILL.md index 13a1ce5..2794859 100644 --- a/.claude/skills/add-ef-migration/SKILL.md +++ b/.claude/skills/add-ef-migration/SKILL.md @@ -87,49 +87,106 @@ migrationBuilder.Sql(""" created_by uuid NOT NULL, updated_at timestamptz NOT NULL DEFAULT now(), updated_by uuid NOT NULL, - row_version bigint NOT NULL DEFAULT 0 + row_version bigint NOT NULL DEFAULT 0, + -- Exists solely so child tables can carry a composite FK into this one. + CONSTRAINT ux__tenant_id_id UNIQUE (tenant_id, id) ); - CREATE INDEX ix__tenant_id ON (tenant_id); - CREATE INDEX ix__organization_id ON (organization_id) - WHERE organization_id IS NOT NULL; - - ALTER TABLE ENABLE ROW LEVEL SECURITY; - - CREATE POLICY _tenant_isolation ON - USING (tenant_id = current_setting('app.tenant_id')::uuid); - - CREATE POLICY _organization_isolation ON - USING ( - organization_id IS NULL - OR organization_id = current_setting('app.organization_id', true)::uuid - ); + -- Every foreign key from this table to another tenant-owned table is + -- COMPOSITE on tenant_id: + -- + -- CONSTRAINT fk__ + -- FOREIGN KEY (tenant_id, _id) + -- REFERENCES (tenant_id, id) + -- + -- Referential-integrity checks run on behalf of the table owner and are NOT + -- subject to Row Level Security, so a single-column FK lets one tenant + -- reference another tenant's rows and no policy ever observes it. + + -- One composite index, deliberately NOT partial: the policy's + -- `organization_id IS NULL` branch matches every tenant-wide row and a b-tree + -- indexes NULLs, so the non-partial form serves both branches. No standalone + -- index on tenant_id — the UNIQUE constraints above already lead with it. + -- Drop organization_id from the index when the table is not org-scoped. + CREATE INDEX ix__tenant_id_organization_id + ON (tenant_id, organization_id); + + -- ───────────────────────────────────────────────────────────────────────── + -- RLS: DO NOT WRITE THE POLICY FROM MEMORY, AND DO NOT COPY IT HERE. + -- + -- Open docs/standards/05-database.md § Tenant-Owned and Organization-Scoped + -- Tables and copy the canonical block into this migration now, substituting + -- . That file is the only place the template exists. + -- + -- The pre-2026-08-08 template lived in four documents and was wrong in all + -- four — two PERMISSIVE policies, which PostgreSQL combines with OR, so every + -- tenant-wide row was visible across tenants (ADR-0003 Amendment 3). It was + -- corrected once, in one file. A second copy is how that recurs. + -- + -- Checklist for what you paste: + -- * ENABLE *and* FORCE ROW LEVEL SECURITY (both table kinds) + -- * explicit WITH CHECK (both table kinds) + -- * NULLIF(current_setting(...), '') on every GUC read (both table kinds) + -- + -- TENANT-ONLY table: exactly ONE permissive policy carrying the tenant + -- predicate alone. No organization term, no restrictive guards — there is + -- no second scope to widen or narrow. + -- + -- [OrganizationScoped] table: still exactly ONE permissive policy, but its + -- predicate ANDs the tenant term with the organization term; PLUS the two + -- AS RESTRICTIVE guards (FOR UPDATE, FOR DELETE), because the + -- app.scope='tenant' read hatch must not widen writes and DELETE has no + -- WITH CHECK. + -- ───────────────────────────────────────────────────────────────────────── """); ``` -**Session variable names** are canonical: `app.tenant_id` / `app.organization_id`. -Architecture test `Every_TenantOwned_Table_HasRls_With_AppTenantId` enforces. +> The canonical template is +> [05-database.md § Tenant-Owned and Organization-Scoped Tables](../../../docs/standards/05-database.md), +> and this skill deliberately does **not** mirror it — the block above tells you to open +> that file and copy from it. See +> [ADR-0003 Amendment 3](../../../docs/decisions/0003-tenant-isolation-defense-in-depth.md) +> for why the two-policy shape was withdrawn. + +**Session variable names** are canonical: `app.tenant_id` / `app.organization_id` / +`app.scope`. Always pass the second `true` argument so an unset context filters the row +out instead of raising on a pooled connection. -### Step 4: Append-only / partitioned table +**Roles.** Migrations run as `learnstack_migration` (the table owner); +the application connects as `learnstack_app` (`NOBYPASSRLS`, not the owner). Grant the +new table to `learnstack_app` in the same migration, or the application cannot read it. -If the table is append-only at scale (audit, large event log): +### Step 4: Append-only table + +An append-only table ships **unpartitioned**, with a composite primary key that a +future partition conversion can reuse. Do **not** write `PARTITION BY` in the first +migration: partitioning is demand-gated to +[Phase 11](../../../docs/roadmap/phase-11-production-hardening.md) on measured growth +([ADR-0035](../../../docs/decisions/0035-demand-gated-infrastructure.md)), and shipping +it early buys partition maintenance before there is anything to maintain. ```csharp migrationBuilder.Sql(""" CREATE TABLE audit_log ( id uuid NOT NULL, - occurred_at timestamptz NOT NULL, + timestamp timestamptz NOT NULL DEFAULT now(), tenant_id uuid NOT NULL, -- ... rest ... - PRIMARY KEY (id, occurred_at) - ) PARTITION BY RANGE (occurred_at); - - -- First partition (others created by retention job per ADR-0028 reservation) - CREATE TABLE audit_log_2026_06 PARTITION OF audit_log - FOR VALUES FROM ('2026-06-01') TO ('2026-07-01'); + -- The partition key must be IN the primary key for the Phase 11 conversion, + -- so declare the composite now even though nothing is partitioned yet. + -- Column name is `timestamp`, matching the canonical DDL in ADR-0033 and + -- 31-audit-subsystem — not `occurred_at`, which is outbox_messages' column. + CONSTRAINT audit_log_pkey PRIMARY KEY (id, timestamp) + ); """); ``` +PostgreSQL has no `ALTER TABLE … PARTITION BY`, so Phase 11 does not convert this table +in place: it creates a partitioned parent, attaches this table to it, and recreates the +indexes and the policy on the parent. The composite key above is what keeps that a data +operation rather than a key migration +([ADR-0033 § Corrected `audit_log` DDL](../../../docs/decisions/0033-audit-durability-model.md)). + The Postgres trigger that rejects `UPDATE` / `DELETE` on `audit_log` lives in the audit module's setup migration; reproduce it for any new append-only table. @@ -183,8 +240,32 @@ before mutating data: ```csharp foreach (var tenantId in tenantIds) { - await connection.ExecuteAsync($"SET LOCAL app.tenant_id = '{tenantId}'"); - await connection.ExecuteAsync("UPDATE ... WHERE tenant_id = current_setting('app.tenant_id')::uuid"); + await using var tx = await connection.BeginTransactionAsync(ct); + + // set_config(key, value, is_local: true) is the parameterised equivalent of + // SET LOCAL, and it MUST run inside an explicit transaction: PostgreSQL + // discards a SET LOCAL issued outside one (with a warning), so the UPDATE + // below would otherwise run with no tenant context at all — which, under a + // NULLIF-wrapped policy, means it silently updates zero rows. + // + // Parameterised, not interpolated. String-interpolated SQL is banned by + // 05-database.md § Forbidden. + await connection.ExecuteAsync( + "SELECT set_config('app.tenant_id', @tenantId, true)", + new { tenantId = tenantId.ToString() }, + transaction: tx); + + // No `WHERE tenant_id = current_setting(...)` clause. The connection runs as + // learnstack_migration, which is NOBYPASSRLS against a table that is FORCE ROW + // LEVEL SECURITY, so the policy scopes the statement to this tenant on its own; + // restating the predicate in application SQL is a second copy of the policy that + // can drift from the first. Note the consequence: skip the set_config above and + // the UPDATE matches ZERO rows and still reports success. + await connection.ExecuteAsync( + "UPDATE enrollments SET source = 'legacy' WHERE source IS NULL", + transaction: tx); + + await tx.CommitAsync(ct); } ``` diff --git a/.claude/skills/add-feature-key/SKILL.md b/.claude/skills/add-feature-key/SKILL.md index 3da4af8..62bea5c 100644 --- a/.claude/skills/add-feature-key/SKILL.md +++ b/.claude/skills/add-feature-key/SKILL.md @@ -187,7 +187,7 @@ See [add-feature-gated-ui](../add-feature-gated-ui/SKILL.md) for hook usage. ### Step 4: Hub-side plan editor (if PlanProjected) -For plan-projected keys, the Hub operator portal (`learnstack-hub-web` in the +For plan-projected keys, the Hub operator portal (`operator-portal` in the separate repo) lists every key declared in the `FeatureKeys` catalogue. The Hub plan editor surfaces them as toggle checkboxes. The Hub publishes the resulting JSON entitlement projection to LearnStack via diff --git a/.claude/skills/add-frontend-route/SKILL.md b/.claude/skills/add-frontend-route/SKILL.md index 724e524..e34f2e1 100644 --- a/.claude/skills/add-frontend-route/SKILL.md +++ b/.claude/skills/add-frontend-route/SKILL.md @@ -7,7 +7,7 @@ description: > portal), and SDK-based data fetching. USE FOR: a new public page, Studio screen, or learner / instructor portal screen. DO NOT USE FOR: thin BFF proxy endpoints (those live in `app/api/`), routes for the operator portal (that's the separate - `learnstack-hub-web` app), or hand-rolled `fetch` to the backend (use the typed + `operator-portal` app), or hand-rolled `fetch` to the backend (use the typed SDK). --- @@ -30,7 +30,7 @@ contract per ## When not to use -- Operator portal pages — they live in `learnstack-hub-web`, a separate repo. +- Operator portal pages — they live in `operator-portal`, a separate repo. - Calling the API directly from a Client Component without the SDK — forbidden by ESLint (`no-restricted-imports`). - Routes that bypass tenant resolution — every authenticated route requires a diff --git a/.claude/skills/add-mediatr-handler/SKILL.md b/.claude/skills/add-mediatr-handler/SKILL.md index 7dae527..d743353 100644 --- a/.claude/skills/add-mediatr-handler/SKILL.md +++ b/.claude/skills/add-mediatr-handler/SKILL.md @@ -15,7 +15,7 @@ description: > ## Purpose Write a command/query handler that participates correctly in the LearnStack MediatR -pipeline: `Validation → Logging → Audit → TenantContext → Authorization → +pipeline: `Validation → Logging → AuditLog → TenantContext → Authorization → Transaction → OutboxFlush → Handler`. The pipeline is shared (per [ADR-0032 § Sub-decision 2](../../../docs/decisions/0032-exception-handling-logging-and-observability.md) and [Standards 02 § Pipeline Behaviors](../../../docs/standards/02-backend-coding.md)), diff --git a/.claude/skills/add-permission/SKILL.md b/.claude/skills/add-permission/SKILL.md index 59298ba..44d091c 100644 --- a/.claude/skills/add-permission/SKILL.md +++ b/.claude/skills/add-permission/SKILL.md @@ -58,7 +58,7 @@ education.course.write education.course.delete education.course_publication.write ← "publish a course" enrollment.enrollment.write -enrollment.entitlement.read +enrollment.course_access.read tenancy.organization.admin identity.impersonation.write audit.export.read @@ -83,12 +83,12 @@ In `.Application/Module.cs`: public void RegisterPermissions(IPermissionRegistry registry) { registry.Tenant( - key: "enrollment.entitlement.read", + key: "enrollment.course_access.read", description: "View entitlements", defaultGrants: [Roles.TenantAdmin, Roles.OrgAdmin, Roles.Instructor]); registry.Tenant( - key: "enrollment.entitlement.write", + key: "enrollment.course_access.write", description: "Grant or revoke entitlements", defaultGrants: [Roles.TenantAdmin]); diff --git a/.claude/skills/add-provider-adapter/SKILL.md b/.claude/skills/add-provider-adapter/SKILL.md index 54c794f..6376748 100644 --- a/.claude/skills/add-provider-adapter/SKILL.md +++ b/.claude/skills/add-provider-adapter/SKILL.md @@ -59,7 +59,7 @@ the canonical wiring per building blocks, handled by [wire-cross-cutting-foundation](../wire-cross-cutting-foundation/SKILL.md). Dapr's runtime provides retry + DLQ + circuit-breaker semantics already. -- The four Hub HTTPS endpoints (`IEntitlementProvider`, `IUsageReporter`, +- The Hub HTTPS contract surface (`IEntitlementProvider`, `IUsageReporter`, `IHubTenantSync`) — those use the dedicated mTLS + signed JWT + HMAC wrapper per [ADR-0019](../../../docs/decisions/0019-learnstack-hub.md). - Pure in-process integrations (a JSON converter, a hash function) — no @@ -313,7 +313,7 @@ In `docs/modules//providers.md`, add the adapter: - Adapter: `LearnStack.Infrastructure.LiveClassroom.LiveKit.LiveKitClient` - Resilience section: `Resilience:liveclass:` - Exception subclass: `LiveClassProviderException` -- ADR: [ADR-0005](../../decisions/0005-live-classroom-media-stack.md) +- ADR: [ADR-0005](../../../docs/decisions/0005-live-classroom-media-stack.md) ``` ## Validation diff --git a/.claude/skills/add-tenant-owned-entity/SKILL.md b/.claude/skills/add-tenant-owned-entity/SKILL.md index 3efa4da..eefb3aa 100644 --- a/.claude/skills/add-tenant-owned-entity/SKILL.md +++ b/.claude/skills/add-tenant-owned-entity/SKILL.md @@ -135,7 +135,16 @@ dotnet ef migrations add Add_ \ --startup-project backend/src/LearnStack.Api ``` -Edit the generated migration to add **both** RLS policies: +Edit the generated migration to add the table and **one** RLS policy. + +> **The canonical template lives in +> [05-database.md § Tenant-Owned and Organization-Scoped Tables](../../../docs/standards/05-database.md), +> and this skill does not mirror it.** Open that file and copy the block from there. +> Mirroring it here would be the same mistake that produced four divergent copies before +> 2026-08-08, one of which leaked every tenant-wide row across tenants +> ([ADR-0003 Amendment 3](../../../docs/decisions/0003-tenant-isolation-defense-in-depth.md)) — +> and a disclaimer saying "the standard wins if they disagree" does not stop the drift, +> it only predicts it. ```csharp migrationBuilder.Sql(""" @@ -148,29 +157,90 @@ migrationBuilder.Sql(""" created_by uuid NOT NULL, updated_at timestamptz NOT NULL DEFAULT now(), updated_by uuid NOT NULL, - row_version bigint NOT NULL DEFAULT 0 + row_version bigint NOT NULL DEFAULT 0, + -- Exists solely so child tables can carry a composite FK into this one. + -- Looks redundant next to the primary key; it is not. See the note below. + CONSTRAINT ux__tenant_id_id UNIQUE (tenant_id, id) ); - CREATE INDEX ix__tenant_id ON (tenant_id); - CREATE INDEX ix__organization_id ON (organization_id) - WHERE organization_id IS NOT NULL; -- omit if not org-scoped - - ALTER TABLE ENABLE ROW LEVEL SECURITY; - - CREATE POLICY _tenant_isolation ON - USING (tenant_id = current_setting('app.tenant_id')::uuid); - - -- org policy: omit if not org-scoped - CREATE POLICY _organization_isolation ON - USING ( - organization_id IS NULL - OR organization_id = current_setting('app.organization_id', true)::uuid - ); + -- Every foreign key from this table to another tenant-owned table is + -- COMPOSITE on tenant_id: + -- + -- CONSTRAINT fk__ + -- FOREIGN KEY (tenant_id, _id) + -- REFERENCES (tenant_id, id) + -- + -- PostgreSQL evaluates referential integrity as a security-restricted + -- operation on behalf of the table owner, and RI checks are NOT subject to + -- Row Level Security. A single-column FK therefore lets a row in tenant A + -- reference a row in tenant B: the child's WITH CHECK passes because its + -- tenant_id is A's, and the FK check passes because it can see B's row. The + -- result is a permanent cross-tenant reference that no policy ever sees, + -- because no policy ran. + + -- One composite index, deliberately NOT partial: the policy's + -- `organization_id IS NULL` branch matches every tenant-wide row and a b-tree + -- indexes NULLs, so the non-partial form serves both branches. No standalone + -- index on tenant_id — the UNIQUE constraints above already lead with it. + -- Drop organization_id from the index if the table is not org-scoped. + CREATE INDEX ix__tenant_id_organization_id + ON (tenant_id, organization_id); + + -- ───────────────────────────────────────────────────────────────────────── + -- RLS: DO NOT WRITE THE POLICY FROM MEMORY, AND DO NOT COPY IT HERE. + -- + -- Open docs/standards/05-database.md § Tenant-Owned and Organization-Scoped + -- Tables and copy the canonical block into this migration NOW, substituting + -- . That file is the only place the template exists; this skill + -- deliberately does not carry a second instance of it. + -- + -- The template that preceded 2026-08-08 lived in four documents and was wrong + -- in all four — two PERMISSIVE policies, which PostgreSQL combines with OR, so + -- every tenant-wide row was visible across tenants. It was corrected once, in + -- one file. A copy here is how that recurs (ADR-0003 Amendment 3). + -- + -- What you are copying, so you can tell if you got it wrong: + -- * ENABLE *and* FORCE ROW LEVEL SECURITY (without FORCE the owner bypasses) + -- * exactly ONE permissive policy, tenant AND organization in one predicate + -- * an explicit WITH CHECK, without the app.scope='tenant' read hatch + -- * two AS RESTRICTIVE guards, FOR UPDATE and FOR DELETE, when org-scoped + -- * NULLIF(current_setting(...), '') on every GUC read + -- Drop the organization terms entirely if the table is not org-scoped. + -- ───────────────────────────────────────────────────────────────────────── """); ``` -The session-variable names are **canonical**: `app.tenant_id`, `app.organization_id` -([05-database.md](../../../docs/standards/05-database.md)). Other names break RLS. +Five properties of the block you just copied are load-bearing. Check each one against +what you pasted — a reviewer will: + +- **`FORCE ROW LEVEL SECURITY`** — without it the owner bypasses the policy and the + whole layer is inert while every structural test stays green. +- **One policy with an `AND`-ed predicate** — splitting the tenant and organization + terms into two policies inverts the meaning from AND to OR. +- **`WITH CHECK`** — `USING` governs reads; without `WITH CHECK` a write can place a + row in another tenant. Note the `app.scope = 'tenant'` term is deliberately absent + from `WITH CHECK`: tenant-scope reporting may *read* across organizations, but + nothing may *write* outside its own. `WITH CHECK` is not sufficient on its own for + that guarantee — PostgreSQL has no `WITH CHECK` for `DELETE`, and `USING` is also + what selects the rows an `UPDATE` may target — which is why the two `AS RESTRICTIVE` + guards above are part of the template and not an optional extra. +- **The composite `UNIQUE (tenant_id, id)` and the composite foreign keys** — referential + integrity is checked on behalf of the table owner and bypasses RLS entirely, so a + single-column FK is a cross-tenant reference waiting to happen, invisible to every + policy. See + [05-database.md § Foreign keys between tenant-owned tables](../../../docs/standards/05-database.md). + +Always call `current_setting` with the second argument `true`. Without it an unset +context raises inside a pooled connection instead of simply filtering the row out. + +The session-variable names are **canonical**: `app.tenant_id`, `app.organization_id`, +`app.scope` ([05-database.md](../../../docs/standards/05-database.md)). Other names +break RLS silently. + +The runtime connects as **`learnstack_app`** (`NOBYPASSRLS`, not the table owner); +migrations run as `learnstack_migration`, which owns the table. Integration tests for +this entity must connect as `learnstack_app` — a test that connects as the owner passes +against an inert policy and proves nothing. ### Step 4: Architecture test (already covered by convention) diff --git a/.claude/skills/code-review/SKILL.md b/.claude/skills/code-review/SKILL.md index ff0d95d..4557684 100644 --- a/.claude/skills/code-review/SKILL.md +++ b/.claude/skills/code-review/SKILL.md @@ -262,7 +262,8 @@ Author intent: `Asana`, `Kyu`, `CodeChallenge` — all forbidden; live as `TenantContentType` / `TenantLevelTaxonomy` / `TenantScoringRule` data). - **No `Verticals/` folder.** ADR-0018 superseded ADR-0011. -- **Hub HTTPS contract surface closed at four endpoints.** Adding a fifth +- **Hub HTTPS contract surface governed by two invariants** (the Hub stores no tenant + content; every crossing goes through a named adapter — ADR-0034). Adding an endpoint requires a new ADR. - **No direct `IConnectionMultiplexer` / `IDistributedCache` / `KafkaProducer` / `VaultClient` injection** (per [CLAUDE.md hard rules](../../../CLAUDE.md) and diff --git a/.claude/skills/commit-and-pr/SKILL.md b/.claude/skills/commit-and-pr/SKILL.md index a06e841..c9414e9 100644 --- a/.claude/skills/commit-and-pr/SKILL.md +++ b/.claude/skills/commit-and-pr/SKILL.md @@ -91,7 +91,7 @@ Trailers go at the **end** of the body (after a blank line). The supported set: Pick the trailer that matches the agent that did material work: - Claude Code session: - `Co-Authored-By: Claude Opus 4.7 (1M context) ` + `Co-Authored-By: Claude Opus 5 (1M context) ` - OpenAI Codex session: `Co-Authored-By: Codex Opus 4.7 (1M context) ` @@ -110,7 +110,7 @@ access shares the per-learner entitlement read path with free access. ADR: 0010 Module: Enrollment, Billing -Co-Authored-By: Claude Opus 4.7 (1M context) +Co-Authored-By: Claude Opus 5 (1M context) EOF )" ``` diff --git a/.claude/skills/local-dev-setup/SKILL.md b/.claude/skills/local-dev-setup/SKILL.md index 31c69e2..6cd0f9b 100644 --- a/.claude/skills/local-dev-setup/SKILL.md +++ b/.claude/skills/local-dev-setup/SKILL.md @@ -184,7 +184,7 @@ make restart-api |---------|-----| | `Bind for 0.0.0.0:5432 failed: port is already allocated` | Stop your local Postgres (or change `POSTGRES_PORT` in `.env`). | | `relation "tenants" does not exist` | Migrations didn't run; `make migrate`. | -| `unable to read app.tenant_id` | Connection pool checkout interceptor not wired; check `DeploymentMode`. | +| `unable to read app.tenant_id` | The `DbCommandInterceptor` tenant-context guard is unwired, or `TransactionBehavior` did not issue the `SET LOCAL` pair. It is deliberately **not** a connection-checkout interceptor — checkout precedes `BEGIN`. | | Keycloak realm not found | First-run seed failed; `make seed-reset` rebuilds. | | Web app shows raw i18n keys | i18n bundle build skipped; `pnpm build:i18n`. | | Hub-backed mode hangs | The `learnstack-hub` repo's stack isn't up; start it or switch to `Development`. | diff --git a/.claude/skills/seed-tenant/SKILL.md b/.claude/skills/seed-tenant/SKILL.md index 2ad7bac..a467086 100644 --- a/.claude/skills/seed-tenant/SKILL.md +++ b/.claude/skills/seed-tenant/SKILL.md @@ -34,7 +34,7 @@ course tree, all as **data**, so: ## When not to use - Production tenant create. That's an operator action from the Hub portal - (`learnstack-hub-web`) via `POST /api/internal/tenants`. + (`operator-portal`) via `POST /api/internal/tenants`. - Self-Hosted license issuance. Hub-side (Phase 02c / 09b). - Reseeding production data. Never. diff --git a/.claude/skills/standards-check/SKILL.md b/.claude/skills/standards-check/SKILL.md index 1a76c18..e2ca21d 100644 --- a/.claude/skills/standards-check/SKILL.md +++ b/.claude/skills/standards-check/SKILL.md @@ -95,12 +95,20 @@ Walk every item in [CLAUDE.md § Hard rules](../../../CLAUDE.md) and LiveKit OSS, SeaweedFS, Meilisearch, Kafka, Vault). - [ ] **No domain-specific code in any module.** Domain shape = tenant data per ADR-0018. -- [ ] **Foundation building blocks are Day-1.** Dapr / APISIX / audit - infrastructure / org scope / entitlement projection / host resolver / arch - tests all in Phase 02a, not later. +- [ ] **Irreversible now, additive on demand — the one-way-door test** + (ADR-0035). Tenant + org isolation, the outbox table, typed IDs, the + localization schema, audit *correctness*, and the foundation **ports** are + Phase 02a. The Dapr / Kafka / APISIX / Vault **adapters**, the Hub + integration, signed licence keys, custom-domain TLS automation and + `audit_log` partitioning are demand-gated to their owning phase against a + written trigger — shipping them early is a finding, not a bonus. - [ ] **Provider adapters everywhere.** No SaaS lock-in in Domain / Application. -- [ ] **Hub HTTPS surface closed at 4 endpoints.** Adding a 5th = new ADR. +- [ ] **Hub contract surface honours its two invariants** (ADR-0034): the Hub + stores no tenant content, and every crossing goes through + `IEntitlementProvider` / `IUsageReporter` / `IHubTenantSync`. Nothing else + holds a Hub client; nothing resolves a host by calling the Hub. Adding an + endpoint still needs an ADR — it is a cross-repository contract. - [ ] **Three deployment modes, one binary.** Module code never branches on `DeploymentMode`. @@ -180,17 +188,41 @@ domain the diff doesn't touch. #### `05-database.md` - [ ] Naming conventions (snake_case, plural tables, `ix_` / `ux_` / `ck_` / `tg_` / `fn_` prefixes). -- [ ] Every `[TenantOwned]` table has `tenant_id` + index, RLS enabled, - policy keyed on `current_setting('app.tenant_id')`. +- [ ] Every `[TenantOwned]` table has `tenant_id` + index, `ENABLE` **and** + `FORCE ROW LEVEL SECURITY`, and **exactly one** policy whose `USING` predicate + `AND`-s the tenant term with the organization term, plus an explicit `WITH CHECK`. + **Two policies is a defect, not a style choice** — both are `PERMISSIVE`, PostgreSQL + `OR`-s them, and the intended `AND` becomes an `OR` that exposes every tenant-wide + row across tenants + ([ADR-0003 Amendment 3](../../../docs/decisions/0003-tenant-isolation-defense-in-depth.md)). - [ ] Every `[OrganizationScoped]` table additionally has nullable - `organization_id` + index + RLS policy on `app.organization_id`. + `organization_id` + index, and its organization term is **inside** that same policy — + not in a second one. +- [ ] `current_setting` is always called with the missing-OK second argument **and** + wrapped in `NULLIF(…, '')` — `NULLIF(current_setting('app.tenant_id', true), '')`. + The two cover different failure paths and both are required: missing-OK handles a + variable never set in this session, `NULLIF` handles one that *was* set and has since + reset. A customized (dotted) GUC's reset value is the empty string, and `''::uuid` + raises `22P02` instead of filtering. Text comparisons such as + `current_setting('app.scope', true) = 'tenant'` are the exception — `'' = 'tenant'` + is already false, which is the correct fail-closed result. +- [ ] Org-scoped tables carry both `AS RESTRICTIVE` write guards, `FOR UPDATE` and + `FOR DELETE`. The `app.scope = 'tenant'` hatch is read-only, and `USING` is the only + gate a `DELETE` has. +- [ ] Every foreign key between two tenant-owned tables is composite on `tenant_id`, + and the parent carries `UNIQUE (tenant_id, id)`. RI checks bypass RLS. +- [ ] Isolation tests for the table connect as **`learnstack_app`**, not as the owner + or a `BYPASSRLS` role. A test that connects as the owner passes against an inert + policy and proves nothing. - [ ] Mutable aggregates carry the audit columns (`created_at` / `created_by` / `updated_at` / `updated_by` / `row_version`). - [ ] Migrations forward-only by default; destructive change has a two-step plan documented. - [ ] PgBouncer transaction-pooling assumption respected (no statement-mode patterns). -- [ ] `correlation_id` columns are `text NULL` (canonical type). +- [ ] `correlation_id` columns are `text` (holding the full W3C traceparent), never + `uuid`. On `outbox_messages` it is **`NOT NULL`**; on `audit_log` it stays nullable + ([31-audit-subsystem.md](../../../docs/architecture/31-audit-subsystem.md)). #### `06-testing.md` - [ ] Test pyramid respected (unit > integration > E2E in volume). @@ -300,7 +332,8 @@ domain the diff doesn't touch. - [ ] Topic naming `learnstack.{module}.{aggregate}`. - [ ] APISIX in standalone YAML-reload mode; routes under `infra/apisix/` per [30-api-gateway.md § 2](../../../docs/architecture/30-api-gateway.md). -- [ ] Hub HTTPS contract surface untouched (still 4 endpoints). +- [ ] Hub contract surface still satisfies ADR-0034's two invariants; any new + endpoint carries its ADR and lands in both repositories. - [ ] Outbox + inbox usage correct (atomic with aggregate write; inbox guard in every consumer). @@ -332,7 +365,7 @@ checklist: `docs/modules//permissions.md` updated. - [ ] Frontend i18n key changed → `I18n:` commit trailer present. - [ ] Hub-side change in this repo → flagged for coordination with - `learnstack-hub` repo (the four-endpoint contract is shared). + `learnstack-hub` repo (the contract surface is shared; see ADR-0034). ### Step 6 — Output diff --git a/.claude/skills/update-glossary/SKILL.md b/.claude/skills/update-glossary/SKILL.md index 864bcf0..ea168c9 100644 --- a/.claude/skills/update-glossary/SKILL.md +++ b/.claude/skills/update-glossary/SKILL.md @@ -99,7 +99,7 @@ Rules: doc, not a long glossary entry. - Lead with what it **is**, not what it's *for*. - Cite the authoritative source inline (e.g. - "[ADR-0017](decisions/0017-tenant-organization-hierarchy.md)"). + "[ADR-0017](../../../docs/decisions/0017-tenant-organization-hierarchy.md)"). - Avoid restating things the canonical doc already says — link. ### Step 4: Update downstream references diff --git a/.claude/skills/wire-cross-cutting-foundation/SKILL.md b/.claude/skills/wire-cross-cutting-foundation/SKILL.md index f3da7cf..c22160b 100644 --- a/.claude/skills/wire-cross-cutting-foundation/SKILL.md +++ b/.claude/skills/wire-cross-cutting-foundation/SKILL.md @@ -70,7 +70,7 @@ and You should be able to recite, before you write a line of code: - The eight pipeline behaviors and their order - (`Validation → Logging → Audit → TenantContext → Authorization → Transaction → OutboxFlush → Handler`). + (`Validation → Logging → AuditLog → TenantContext → Authorization → Transaction → OutboxFlush → Handler`). - The Sentry-vs-OTel boundary (`ShouldCapture(ex)` table). - The Serilog + OTLP wiring rule (no `AddOpenTelemetry().WithLogging()` alongside). - The composition-root branching for `IErrorTrackingProvider`. diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index fc38172..6f38b47 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -20,11 +20,24 @@ Configure these in **GitHub → Settings → Branches → Branch protection rule - `frontend (typecheck + lint + build + test)` - `meta (commit hygiene + link audit)` - `secret scan (leakwatch)` - - Deferred checks — flip the `if: false` guards in `ci.yml` AND add the - job name here when the owning phase lands: - - `backend integration (Testcontainers — deferred)` — Phase 02a. - - `openapi diff (deferred to Phase 03)` — Phase 03. - - `lighthouse budget (deferred to Phase 04)` — Phase 04. + - Deferred checks. Each is gated on a repository variable (`vars.ENABLE_*`, + unset by default — a constant `if: false` is rejected by actionlint). + Activating one is **four edits, in the same pull request wherever possible**: + set the variable in **Settings → Secrets and variables → Actions → Variables**, + replace the placeholder step with the real one, **rename the job** to drop the + `(deferred …)` suffix, and add the new name both to this list and to the live + branch-protection setting. Setting the variable alone leaves a job that runs + but gates nothing. + - `backend integration (Testcontainers — deferred)` — Phase 02a **Packet 7**, + with the first cross-tenant isolation test. + - `openapi diff (deferred to Phase 02d)` — **Phase 02d**, with the first real + `/api/v1/*` read endpoints. + - `lighthouse budget (deferred to Phase 02d)` — **Phase 02d**, with the first + content-bearing public pages. + + GitHub matches required checks **by name**, so the rename is the dangerous + half: a renamed check that nobody re-required is a check that no longer blocks + anything, and the PR still shows green. - **Require conversation resolution before merging**: on. - **Require signed commits**: optional (off until the team rolls out signing keys). - **Require linear history**: on (we use squash-merge or rebase-merge, never bubble). @@ -44,7 +57,7 @@ Per CLAUDE.md § Commit conventions: - **Conventional Commits**: `type(scope): subject` with subject in imperative mood, ≤ 72 chars. - AI-assisted commits carry the trailer - `Co-Authored-By: Claude Opus 4.7 (1M context) ` + `Co-Authored-By: Claude Opus 5 (1M context) ` (replace the model name when authoring with a different assistant). - `docs(scope)` for doc-only commits; scope ∈ `architecture | decisions | standards | roadmap` or omitted for cross-cutting changes. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 68b7696..79000ef 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,17 +10,20 @@ # - frontend : pnpm install + typecheck + lint + build + Vitest # - meta : `make lint`-style format verification # -# Deferred to later phases (jobs are scaffolded as `if: false` placeholders -# so the activation is a one-line flip): +# Deferred to later phases. Each is scaffolded behind a repository variable +# (`vars.ENABLE_*`), unset by default, so activation is one variable plus the +# real steps. A constant `if: false` would be simpler but actionlint rejects it +# ([if-cond] constant expression). Activation is never *only* the variable — +# see .github/CONTRIBUTING.md § Branch protection for the three edits: # - backend-integration : Testcontainers needs a real Docker socket inside # the runner — works on `ubuntu-latest` natively. Activates when the # first integration test lands (Phase 02a) so we have something to run. # - openapi-diff : oasdiff against the prior `main` spec. Activates -# in Phase 03 when the first real endpoint replaces `/healthz` as the -# only documented surface (until then there is nothing to diff). +# in Phase 02d, which ships the first real `/api/v1/*` read endpoints and +# retires `/healthz` as the only documented surface. # - lighthouse-budget : LHCI against the built Next.js app. Activates -# when Phase 04 ships the first content-bearing public page (the -# current placeholder routes are not worth scoring). +# in Phase 02d, which ships the first content-bearing public pages (the +# two tenant catalog / lesson pages a visitor actually loads). name: ci @@ -109,7 +112,10 @@ jobs: backend-integration: name: backend integration (Testcontainers — deferred) runs-on: ubuntu-latest - if: false # activate when LearnStack.Tests.Integration has its first test + # Disabled by default: unset vars are the empty string, so this is false until + # the repository variable is set to 'true'. Not `if: false` — actionlint rejects + # a constant condition ([if-cond]). + if: vars.ENABLE_BACKEND_INTEGRATION == 'true' steps: - run: echo "Placeholder — Phase 02a wires the first Testcontainers integration test." @@ -161,21 +167,21 @@ jobs: - name: Test (Vitest) run: pnpm -r test - # ─── OpenAPI breaking-change check (deferred — Phase 03) ────────────── + # ─── OpenAPI breaking-change check (deferred — Phase 02d) ───────────── openapi-diff: - name: openapi diff (deferred to Phase 03) + name: openapi diff (deferred to Phase 02d) runs-on: ubuntu-latest - if: false # activate when /api/v1/* endpoints replace the `/healthz` placeholder + if: vars.ENABLE_OPENAPI_DIFF == 'true' steps: - - run: echo "Placeholder — Phase 03 wires oasdiff against the prior main spec." + - run: echo "Placeholder — Phase 02d wires oasdiff against the prior main spec." - # ─── Lighthouse budget (deferred — Phase 04) ────────────────────────── + # ─── Lighthouse budget (deferred — Phase 02d) ───────────────────────── lighthouse-budget: - name: lighthouse budget (deferred to Phase 04) + name: lighthouse budget (deferred to Phase 02d) runs-on: ubuntu-latest - if: false # activate when the first content-bearing public page ships + if: vars.ENABLE_LIGHTHOUSE_BUDGET == 'true' steps: - - run: echo "Placeholder — Phase 04 wires LHCI against the built Next.js app." + - run: echo "Placeholder — Phase 02d wires LHCI against the built Next.js app." # ─── Meta (commit-message format, link audit) ───────────────────────── meta: diff --git a/AGENTS.md b/AGENTS.md index c19a110..c3b1f64 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,7 +16,7 @@ There are no rule differences. The only thing that varies between agent runtimes the **`Co-Authored-By` commit trailer**, which names the assistant that contributed: - Claude Code sessions: - `Co-Authored-By: Claude Opus 4.7 (1M context) ` + `Co-Authored-By: Claude Opus 5 (1M context) ` - OpenAI Codex sessions: `Co-Authored-By: Codex Opus 4.7 (1M context) ` diff --git a/CLAUDE.md b/CLAUDE.md index 423134e..ba0fd5f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,47 +6,91 @@ the conventions you must follow when contributing. ## What this is -LearnStack is a **multi-tenant core platform for building education -products** — not a single LMS. It powers arbitrarily-domained education -products (English-learning, yoga, coding bootcamps, music schools, -driving schools, …) on the **same code paths**; the difference between -them is **tenant customization data** loaded at provisioning, not code. -The first showcase tenant is an online English-learning platform -(Phase 10); the substrate-genericity proof is a second non-English -tenant running the same code paths. +LearnStack is a **white-label platform for multi-branch education +businesses that teach live** — not a single LMS, and not an education +product of its own. One binary, one schema, and one set of container +images serve a language school, a yoga studio, a music school, or a +coding bootcamp. What differs between them is **tenant customization +data** loaded at provisioning, not code +([ADR-0018](docs/decisions/0018-tenant-driven-customization-model.md)). + +That claim has a stated edge, and the edge lives in exactly one place: +[Platform Vision § Genericity boundary](docs/architecture/01-platform-vision.md). +Content shape, presentation, and pure rule evaluation are tenant data. +Stateful entitlement (credit packs, session quotas) and external +capability invocation (running submitted code, scoring speech) are +platform features gated by plan — they need a release, not a +customization row. Link to that section; do not restate it. LearnStack ships in three production deployment modes — SaaS, Dedicated, -Self-Hosted — backed by the companion **`learnstack-hub`** repository -(separate repo, see [ADR-0019](docs/decisions/0019-learnstack-hub.md)). -The Hub repo lives at sibling path `../learnstack-hub` on developer -workstations; GitHub: https://github.com/cemililik/LearnStack-Hub. -Phase 02c P02c-0 (Hub repo bootstrap) shipped 2026-05-21 — see the -Phase 02c roadmap doc for the per-packet status. +Self-Hosted — backed by the companion **LearnStack Hub** control plane +(separate repository, see +[ADR-0019](docs/decisions/0019-learnstack-hub.md)). On developer +workstations the Hub repo is the sibling directory `../LearnStack-Hub`; +GitHub: https://github.com/HodeTech/LearnStack-Hub. The Hub repository +**owns its own roadmap** at `../LearnStack-Hub/docs/roadmap/`; this +repository holds only LearnStack's side of the boundary, in +[Phase 02c](docs/roadmap/phase-02c-hub-foundation.md). ## What state this is in -**Phase 01 complete — repository scaffolding, local infrastructure, DX, -and CI baseline. No domain code yet — Phase 02a starts that.** - -What shipped: the .NET 10 solution scaffold under `backend/` (core + 7 -modules × 4 projects + 4 test projects including the non-skippable -`LearnStack.Tests.Architecture`), the `pnpm` frontend monorepo under -`frontend/` (`apps/web` Next.js App Router + `packages/{config,ui,sdk}`), -the full local-dev compose stack at `infra/compose/dev.yml` — PostgreSQL -18, Valkey, SeaweedFS, Mailpit, Meilisearch, Keycloak (two realms), -LiveKit OSS + Coturn, Kafka + kafka-ui, Vault, Dapr sidecar + placement, -APISIX in file-driven standalone mode — and the DX + CI surround -(repo-root `Makefile`, `.env.example` single source of truth, -`.githooks/pre-commit` formatter + Leakwatch, `infra/compose/e2e.yml` -ephemeral overlay, `.github/workflows/ci.yml` with backend + frontend + -meta + secret-scan required checks, `scripts/seed.sh`). - -Every module assembly is empty of domain code today. Module-level +**Phase 01 complete. +[Phase 02a](docs/roadmap/phase-02a-kernel-tenancy.md) in progress — +packets 0–3 shipped; packets 3b–10 were re-scoped on 2026-08-08 after a +four-report audit of the corpus.** + +**Phase 01** shipped the .NET 10 solution scaffold under `backend/` +(core + 7 modules × 4 projects + 4 test projects including the +non-skippable `LearnStack.Tests.Architecture`), the `pnpm` frontend +monorepo under `frontend/` (`apps/web` Next.js App Router + +`packages/{config,ui,sdk}`), the local-dev compose stack at +`infra/compose/dev.yml`, and the DX + CI surround (repo-root `Makefile`, +`.env.example` single source of truth, `.githooks/pre-commit` formatter + +Leakwatch, `infra/compose/e2e.yml` ephemeral overlay, +`.github/workflows/ci.yml` with backend + frontend + meta + secret-scan +required checks, `scripts/seed.sh`). + +**Phase 02a packets 0–3** shipped the decision set (Vogen, API +versioning, audit partition management), the shared kernel core +(`Result` + `LocalizedMessage`, `Entity` / `AuditableEntity`, +domain events, cursor pagination, `IClock` / `IRandom` / `IGuidFactory`), +and the [ADR-0032](docs/decisions/0032-exception-handling-logging-and-observability.md) +cross-cutting foundation (L1 `IExceptionHandler`, the eight-step MediatR +pipeline, Serilog → OTLP, `TenantContextSpanProcessor`, +`IErrorTrackingProvider`, `IProviderResilience`, the `LS0001` +analyzer). Those records are frozen delivery history. + +**The 2026-08-08 restructure** re-scoped packets 3b–10 along three lines, +all recorded in the Phase 02a Status block: + +- *Correctness moved earlier.* The Row Level Security template that four + documents carried produced two **permissive** policies, which + PostgreSQL combines with `OR` — leaking every tenant-wide row across + tenants. [ADR-0003 Amendment 3](docs/decisions/0003-tenant-isolation-defense-in-depth.md) + corrects it, and [ADR-0033](docs/decisions/0033-audit-durability-model.md) + makes MUST-class audit a durable intent inside the business + transaction — which is also what stops the corrected policy from + rejecting every audit insert. +- *Additive infrastructure moved later.* Per + [ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md), Packet 5 + ships the foundation **ports and their default implementations**; the + Dapr, Kafka, APISIX and Vault adapters land in + [Phase 11](docs/roadmap/phase-11-production-hardening.md) against + written triggers. +- *Proof moved earlier.* Two seed tenants in unrelated domains land in + Packet 7, and [Phase 02d](docs/roadmap/phase-02d-walking-skeleton.md) + renders both of them in a browser. + +**[Phase 02d: Two-Tenant Walking Skeleton](docs/roadmap/phase-02d-walking-skeleton.md) +is the next user-visible milestone** — the first phase whose output +someone who does not read C# can evaluate: two hosts, two tenants, two +education sites, one binary and one database. + +Every module assembly is still empty of domain code. Module-level references in the docs (e.g. `LearnStack.Modules.Education.Application`, `ILiveClassProvider`, `ITenantSearch`) describe **intended** shape that -the corpus anchors against — Phase 02a (Platform Kernel + -Multi-Tenancy) is where those types actually land. Phase 02c (Hub -Foundation, separate `learnstack-hub` repo) runs in parallel. +the corpus anchors against; Phase 02a packets 6–9 and Phase 02d are +where the first of those types actually land. ## Where to start @@ -55,10 +99,19 @@ For any task, read in this order: 1. [README.md](README.md) — direction at a glance. 2. [docs/architecture/01-platform-vision.md](docs/architecture/01-platform-vision.md) — what we build and why. 3. [docs/architecture/05-mvp-scope.md](docs/architecture/05-mvp-scope.md) — what is in / out / deferred. -4. [docs/roadmap/README.md](docs/roadmap/README.md) — phased plan with explicit dependencies. +4. [docs/roadmap/README.md](docs/roadmap/README.md) — phased plan with explicit dependencies and the one-way-door sequencing principle. 5. [docs/standards/00-principles.md](docs/standards/00-principles.md) — the beliefs every other standard descends from. 6. [docs/glossary.md](docs/glossary.md) — terminology; the single source of truth for project-specific terms. +Then read the two phases that are live: + +- [docs/roadmap/phase-02a-kernel-tenancy.md](docs/roadmap/phase-02a-kernel-tenancy.md) + — the current phase, with a dated Status block listing every packet. +- [docs/roadmap/phase-02d-walking-skeleton.md](docs/roadmap/phase-02d-walking-skeleton.md) + — what Phase 02a is building toward. `02d` sorts after `02b`/`02c` but + runs **before** them; the roadmap dependency map is authoritative for + order, filename order is not. + Once the high-level reading is done, pick **exactly one** skill entry point based on the user's intent. The entry point dispatches the rest internally; do not chain entry points yourself. @@ -82,8 +135,8 @@ let the entry point pick it. |-----------|---------|------------| | `docs/architecture/` | Conceptual descriptions of what we are building. Numbered `NN-topic.md` linearly. | Editable as the system evolves. | | `docs/decisions/` | ADRs — one-time decisions with status, context, decision, consequences. Redirect / superseded ADRs live under `_redirects/`. | Accepted ADRs are immutable except for dated Amendments. | -| `docs/standards/` | Engineering rules (`NN-topic.md`, 00 – 20). Each anchored standard carries a `**Derives from:** ADR-NNNN` header. | Editable as the team learns; standard changes cite an ADR. | -| `docs/roadmap/` | Phased plan (`phase-NN-topic.md`, 00 – 12 with 02a/02b/02c, 08a/08b/08c, and 09/09b splits). | Editable per phase. | +| `docs/standards/` | Engineering rules (`NN-topic.md`, 00 – 21). Each anchored standard carries a `**Derives from:** ADR-NNNN` header. | Editable as the team learns; standard changes cite an ADR. | +| `docs/roadmap/` | Phased plan (`phase-NN-topic.md`, 00 – 12 with 02a/02b/02c/**02d**, 08a/08b/08c, and 09/09b splits). Every phase doc carries the same six sections — Goal, Scope, Deliverables, Completion Criteria, Risks, Phase Exit Decision — with three declared exceptions listed in [the roadmap index](docs/roadmap/README.md): Phase 09b and Phase 12 are pointer documents into the Hub repository, and Phase 01 predates the convention. | Editable per phase; the Status block of a shipped packet is a dated delivery record and is not rewritten. | | `docs/glossary.md` | Terminology source of truth. | Editable; new term goes here first, then used. | > `docs/analysis/` exists locally but is **gitignored** — it is a private scratchpad @@ -98,14 +151,17 @@ let the entry point pick it. - **Single source of truth.** Each piece of knowledge lives in exactly one place. The glossary holds terms. ADRs hold decisions. Standards hold ongoing rules. Architecture docs hold conceptual descriptions. Roadmap holds phases. Do not duplicate. - **ADR numbers are sequential and never reused.** Superseded ADRs become redirect stubs under `decisions/_redirects/`. Adding a new ADR uses the next free number. - **Standards changes cite an ADR.** A new standard rule or a change to an existing one is paired with an ADR when the rule is non-trivial. -- **Modular monolith with four cross-module mechanisms** ([ADR-0010](docs/decisions/0010-cross-module-communication.md)): application contract, intra-module domain event, integration event via outbox (dispatched through Dapr pub/sub per Amendment 1), read-model projection. No fifth. +- **Modular monolith with four cross-module mechanisms** ([ADR-0010](docs/decisions/0010-cross-module-communication.md)): application contract, intra-module domain event, integration event via outbox (dispatched through `IEventBus` — `InProcessEventBus` today, the Dapr/Kafka adapter on its trigger), read-model projection. No fifth. - **Tenant + organization isolation is defense-in-depth from day one** ([ADR-0003 Amendment 1](docs/decisions/0003-tenant-isolation-defense-in-depth.md), [ADR-0017](docs/decisions/0017-tenant-organization-hierarchy.md)): tenant + organization context + EF query filters + PostgreSQL RLS + architecture tests. -- **Self-hosted infrastructure preferred** for Keycloak (auth, with two realms — `learnstack` + `learnstack-hub`), LiveKit OSS (live classroom), SeaweedFS (object storage), Meilisearch (search), Kafka (pub/sub backend), Vault (secrets). See ADRs 0004, 0005, 0014. -- **The core platform stays domain-generic.** Domain-specific shapes (CEFR levels, English placement-test scoring, kyu/dan ranks, yoga asana catalogs, …) live as **tenant customization data** ([ADR-0018](docs/decisions/0018-tenant-driven-customization-model.md)), never as code in any module. There is no `Verticals/` folder. ADR-0011 is superseded. -- **Foundation building blocks are Day-1, not Phase-11.** Dapr (`IEventBus`/`ICacheService`/`ISecretProvider`), APISIX gateway, audit infrastructure, organization scope, entitlement projection socket, host-to-tenant resolver, and architecture tests all ship in Phase 02a — not as later hardening. -- **Provider adapters everywhere.** Payments, auth, storage, search, live classroom, notifications, **event bus, cache, secrets, Hub HTTPS contract, entitlement source, host resolver** — all sit behind interfaces. No SaaS lock-in in `Domain` or `Application`. See [20-infrastructure-stack.md](docs/standards/20-infrastructure-stack.md). -- **Hub HTTPS contract surface is closed at four endpoints.** Adding a fifth requires a new ADR. See [ADR-0019](docs/decisions/0019-learnstack-hub.md). -- **Three deployment modes, one binary.** `SaaS` / `Dedicated` / `SelfHosted` selection happens at composition root via `DeploymentMode`; module code never branches on the mode. See [ADR-0020](docs/decisions/0020-triple-deployment-hybrid-license.md). +- **One canonical RLS template, in one file.** The corrected policy shape — one `AND`-ed policy per table, `ENABLE` **and** `FORCE ROW LEVEL SECURITY`, an explicit `WITH CHECK`, and the four-role model (`learnstack_migration` owns, `learnstack_app` connects with `NOBYPASSRLS`, `learnstack_platform` and `learnstack_outbox_admin` hold audited bypasses) — is decided in [ADR-0003 Amendment 3](docs/decisions/0003-tenant-isolation-defense-in-depth.md) and written as SQL in exactly one document: [Database Standards](docs/standards/05-database.md). Every other document links there. The superseded template lived in four documents and was wrong in all four — two *permissive* policies, which PostgreSQL combines with `OR`, so every tenant-wide row was visible across tenants. +- **Self-hosted infrastructure preferred** for Keycloak (auth, with two realms — `learnstack` + `learnstack-hub`), LiveKit OSS (live classroom), SeaweedFS (object storage), Meilisearch (search), Kafka (pub/sub backend), Vault (secrets). See ADRs 0004, 0005, 0014. **What** LearnStack uses is settled; **when** each arrives is [ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md)'s trigger table. +- **The core platform stays domain-generic.** Domain-specific shapes (CEFR levels, English placement-test scoring, kyu/dan ranks, yoga asana catalogs, …) live as **tenant customization data** ([ADR-0018](docs/decisions/0018-tenant-driven-customization-model.md)), never as code in any module. There is no `Verticals/` folder. ADR-0011 is superseded. The boundary of that claim is in [Platform Vision § Genericity boundary](docs/architecture/01-platform-vision.md). +- **Irreversible now, additive on demand — the one-way-door test** ([ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md)): *if I add this six months from now, will I have to touch code that is already written?* + - **Yes → ship it now.** Tenant + organization isolation, the corrected RLS policies, the `outbox_messages` table and its ownership, strongly-typed identifiers, the localization schema, MUST-class audit durability, module boundaries and their architecture tests. These touch every query, every migration, and every job payload. + - **No → ship the port now, the adapter on a named trigger.** Dapr pub/sub, Kafka, Valkey-backed cache, Vault, APISIX, the Hub entitlement source, signed licence keys, custom-domain TLS automation, `audit_log` partitioning. Each has a port in `LearnStack.SharedKernel` (shipped, or landing in Phase 02a Packet 5), a working default implementation (`InProcessEventBus`, `InMemoryCacheService`, `ConfigurationSecretProvider`, `NullEntitlementProvider`), an owning phase, and a written trigger condition. A building block missing any of those four is not demand-gated — it is missing. +- **Provider adapters everywhere.** Payments, auth, storage, search, live classroom, notifications, **event bus, cache, secrets, Hub contract, entitlement source, host resolver** — all sit behind interfaces. No SaaS lock-in in `Domain` or `Application`. See [20-infrastructure-stack.md](docs/standards/20-infrastructure-stack.md). +- **The Hub contract is governed by two invariants, not by a count** ([ADR-0034](docs/decisions/0034-hub-contract-surface-invariant.md)): (1) the Hub stores **no tenant content** — courses, lessons, learners, enrollments, sessions and media live only in LearnStack, and the Hub holds tenant *metadata* only; (2) **every LearnStack↔Hub crossing goes through a named adapter** — `IEntitlementProvider`, `IUsageReporter`, `IHubTenantSync`, and nothing else may hold a Hub client. Adding an endpoint still requires an ADR, because the surface is a cross-repository contract both repositories have to agree on. +- **One binary, five `DeploymentMode` values, two of them wired.** Selection happens at the composition root; module code never branches on the mode ([ADR-0020](docs/decisions/0020-triple-deployment-hybrid-license.md), enforced by `Modules_Do_Not_Reference_DeploymentMode`). `Development` and `SaaS` are wired end to end; `Dedicated`, `SelfHostedOnline` and `SelfHostedAirGapped` are **prepared seams, not supported deployments**, until [Phase 11](docs/roadmap/phase-11-production-hardening.md) builds their adapters and integration suites. ## Conventions when editing docs @@ -128,7 +184,8 @@ rules: `frontend/apps/web` with route segments ([03](docs/standards/03-frontend-coding.md), [07](docs/standards/07-frontend-architecture.md)). The operator portal - (`learnstack-hub-web`) is a separate app in the `learnstack-hub` repo. + is a separate app, `frontend/apps/operator-portal`, in the + `LearnStack-Hub` repository. - REST + RFC 7807 Problem Details + cursor pagination + idempotency keys + ETag concurrency ([04](docs/standards/04-api-design.md)). - OpenTelemetry + correlation id end to end ([10](docs/standards/10-observability.md)). @@ -136,9 +193,12 @@ rules: - Audit-coverage matrix required per module ([18](docs/standards/18-audit-coverage.md)). - Permission keys `{module}.{resource}.{action}` with closed action set + scope (Platform / Tenant / Organization) ([19](docs/standards/19-permissions.md)). -- Infrastructure-stack rules (Dapr building blocks, APISIX, Hub HTTPS contract, - outbox + inbox, entitlement projection) in - [20](docs/standards/20-infrastructure-stack.md). +- Infrastructure-stack rules (foundation ports and their default + implementations, the Hub contract surface, outbox + inbox, entitlement + projection) in [20](docs/standards/20-infrastructure-stack.md). +- The architecture-test catalogue in + [21](docs/standards/21-architecture-tests-catalogue.md) — canonical rule + names live there; do not invent a second spelling. - Zero-tolerance review blockers enumerated in [17](docs/standards/17-code-review.md). ## Commit conventions @@ -149,7 +209,7 @@ rules: `architecture`, `decisions`, `standards`, `roadmap`, or omitted for cross-cutting changes. - Commits made with AI assistance carry the trailer - `Co-Authored-By: Claude Opus 4.7 (1M context) `. + `Co-Authored-By: Claude Opus 5 (1M context) `. ## Things to never do @@ -162,9 +222,45 @@ rules: [ADR-0018](docs/decisions/0018-tenant-driven-customization-model.md). There is no `Verticals/` folder; the architecture test `No_Source_Folder_Named_Verticals` enforces it. -- Add a fifth endpoint to the Hub HTTPS contract surface without an ADR. +- Add an endpoint to the Hub contract surface without an ADR. The count + is not the rule — [ADR-0034](docs/decisions/0034-hub-contract-surface-invariant.md)'s + two invariants are — but the surface is a cross-repository contract, so + it changes by decision record, in both repositories, or not at all. - Call Hub endpoints from anywhere except the dedicated `IEntitlementProvider` / `IUsageReporter` / `IHubTenantSync` adapters. +- Resolve a host by calling the Hub. `IHostToTenantResolver` reads + `platform_host_to_tenant` and nothing else + ([ADR-0034](docs/decisions/0034-hub-contract-surface-invariant.md)); an + anonymous page load must never depend on a control plane being + reachable. +- Carry TLS certificates or private keys in the entitlement payload. Cert + material moves by secret-store replication and is referenced by path + from `PUT /api/internal/tenants/{id}/host-mappings`, never by value + through a payload LearnStack caches, logs, audits and mirrors. +- Copy the RLS template into a second document. It lives only in + [Database Standards](docs/standards/05-database.md); everywhere else + links to it. The last duplication shipped a broken policy into four + files at once. +- Run a tenant- or organization-isolation test as the table owner or as a + `BYPASSRLS` role. Isolation tests connect as **`learnstack_app`** — a + test that runs as `learnstack_migration`, `learnstack_platform` or + `learnstack_outbox_admin` passes even when every policy is inert, and + therefore proves nothing. +- Write a MUST-class audit row outside the business transaction. MUST-class + audit is written on the **same transaction** as the state change it describes + ([ADR-0033](docs/decisions/0033-audit-durability-model.md)) — `AuditLogBehavior` + classifies and parks the intent, `TransactionBehavior` writes it immediately + before `COMMIT` — so it commits with that change or not at all, and so it + executes while `app.tenant_id` is set and RLS accepts it. "The same + `SaveChanges` as the business write" was the earlier formulation and ADR-0033 + **withdraws** it: the guarantee is the transaction, which is what a reader of + `audit_log` observes and which needs no cross-`DbContext` machinery. A tenant `AuditConfig` may + narrow SHOULD/MAY coverage but never removes baseline MUST coverage. Exactly + two failures reject the operation: an operation the catalogue does not + classify at all, and a MUST-class row that cannot be written durably. A + tenant-override **read** failure does not — it falls back to the in-process + catalogue, which carries the same MUST floor, so nothing proceeds unaudited + and a cache outage does not deny every request platform-wide. - Inject `IConnectionMultiplexer` / `IDistributedCache` / `KafkaProducer` / `VaultClient` directly — use `IEventBus` / `ICacheService` / `ISecretProvider`. @@ -180,7 +276,11 @@ rules: the existing documents ambiguous about ownership; if a topic needs more space, expand the existing doc rather than splintering. - Mention a feature as "deferred to a later phase" without naming the - phase that owns it. + phase that owns it. For an infrastructure building block the bar is + higher: name the **port**, the **default implementation**, the **owning + phase**, and the **trigger condition** + ([ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md)). Three + out of four is not demand-gating. - Throw `DomainException` for expected business-rule violations — use `Result.Fail(business_rule_violation, ...)`. `DomainException` is reserved for programmer errors / aggregate invariant diff --git a/README.md b/README.md index dd8b498..ee356fe 100644 --- a/README.md +++ b/README.md @@ -1,46 +1,70 @@ # LearnStack -LearnStack is a **multi-tenant core platform for building education products** — not a -single LMS. It is an education-aware CMS and platform engine that powers different -learning brands, landing pages, catalogs, portals, and **domain-agnostic** education -products. The same code paths serve an online English-learning brand, a yoga studio, -a coding bootcamp, a music school, or a driving school — the difference between them -is **tenant customization data** loaded at provisioning, not compiled code. - -The first showcase tenant happens to be an online English-learning platform (Phase 10); -the substrate-genericity proof is a second, non-English tenant running the same code -paths against its own customization data. +LearnStack is a **white-label platform for multi-branch education businesses that teach +live** — not a single LMS, and not an education product of its own. It is an +education-aware CMS, learning engine, and platform foundation that powers different +brands, landing pages, catalogs, and learner portals. The same code paths serve a +language school, a yoga studio, a music school, or a coding bootcamp — the difference +between them is **tenant customization data** loaded at provisioning, not compiled code +([ADR-0018](docs/decisions/0018-tenant-driven-customization-model.md)). + +That claim has a stated edge: content shape, presentation and pure rule evaluation are +tenant data; stateful entitlement and external capability invocation are platform +features gated by plan. See +[Platform Vision § Genericity boundary](docs/architecture/01-platform-vision.md). + +Two tenants in unrelated domains exist from +[Phase 02a Packet 7](docs/roadmap/phase-02a-kernel-tenancy.md) onward and are both +rendered in a browser in [Phase 02d](docs/roadmap/phase-02d-walking-skeleton.md), so +genericity is tested continuously rather than asserted and checked once at the end. LearnStack ships in three production deployment modes — SaaS, Dedicated, Self-Hosted — -backed by the companion **`learnstack-hub`** repository which provides the SaaS / +backed by the companion **LearnStack Hub** repository, which provides the SaaS / Dedicated control plane, plan editor, custom-domain admin, and license-key issuance. -The Hub repo is expected to live at `../learnstack-hub` (sibling to this repo on the -developer's workstation) so the cross-repo doc cross-links resolve. See -[learnstack-hub on GitHub](https://github.com/cemililik/LearnStack-Hub) and -[docs/roadmap/phase-02c-hub-foundation.md](docs/roadmap/phase-02c-hub-foundation.md). +Today only `Development` and `SaaS` are wired and tested end to end. `Dedicated` and +the two Self-Hosted `DeploymentMode` values (`SelfHostedOnline`, +`SelfHostedAirGapped`) are **prepared seams, not supported deployments** — the +composition root branches on them, but their adapters and integration suites land in +[Phase 11](docs/roadmap/phase-11-production-hardening.md) per +[ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md). See +[25-deployment-models.md](docs/architecture/25-deployment-models.md) for what a +prepared seam means concretely. + +The Hub repo is expected to live at `../LearnStack-Hub` (sibling to this repo on the +developer's workstation) so the cross-repo doc links resolve. The Hub repository owns +its own roadmap at `../LearnStack-Hub/docs/roadmap/`. See +[LearnStack-Hub on GitHub](https://github.com/HodeTech/LearnStack-Hub) and +[docs/roadmap/phase-02c-hub-foundation.md](docs/roadmap/phase-02c-hub-foundation.md) +for LearnStack's side of the boundary. ## Status -Phase 01 complete. The repository now holds the .NET 10 solution scaffold (7 -modules × 4 projects + 4 test projects with `No_Source_Folder_Named_Verticals` -architecture test), the `pnpm` frontend monorepo (`apps/web` + -`packages/{config,ui,sdk}`), the full local-dev `docker-compose` stack — -PostgreSQL 18, Valkey, SeaweedFS, Mailpit, Meilisearch, Keycloak (two realms), -LiveKit OSS + Coturn, Kafka + kafka-ui, Vault, Dapr sidecar + placement, APISIX -(file-driven standalone) — and the DX + CI surround: repo-root `Makefile`, -`.env.example` single source of truth, `.githooks/pre-commit` formatter, -`infra/compose/e2e.yml` ephemeral overlay, `.github/workflows/ci.yml`, and -`scripts/seed.sh`. See [docs/roadmap/phase-01-repository-tooling.md](docs/roadmap/phase-01-repository-tooling.md) -for the per-packet history. Phase 02a (Platform Kernel + Multi-Tenancy) is -underway; see [Phase 02a Status & Packets](docs/roadmap/phase-02a-kernel-tenancy.md) -for the 11-packet breakdown. Packets 0 (Kickoff) and 1 (Foundation decisions -— [ADR-0023](docs/decisions/0023-strongly-typed-id-source-generator.md) Vogen, +**Phase 01 complete. [Phase 02a](docs/roadmap/phase-02a-kernel-tenancy.md) in progress — +packets 0–3 shipped; packets 3b–10 re-scoped on 2026-08-08.** + +Phase 01 shipped the .NET 10 solution scaffold, the `pnpm` frontend monorepo +(`apps/web` + `packages/{config,ui,sdk}`), the local-dev `docker-compose` stack, and the +DX + CI surround. See +[phase-01-repository-tooling.md](docs/roadmap/phase-01-repository-tooling.md) for the +per-packet history. + +Phase 02a packets 0–3 shipped the foundation decisions +([ADR-0023](docs/decisions/0023-strongly-typed-id-source-generator.md) Vogen, [ADR-0024](docs/decisions/0024-api-versioning-policy.md) API versioning, -[ADR-0028](docs/decisions/0028-audit-log-partition-management.md) audit -partition mgmt) have shipped; Packet 2 — Shared Kernel core is next. -Phase 02c (Hub Foundation, parallel, separate repo) starts once the 02a -sockets it depends on are in place. +[ADR-0028](docs/decisions/0028-audit-log-partition-management.md) audit partition +management — whose *timing* later moved to Phase 11), the shared kernel core, and the +[ADR-0032](docs/decisions/0032-exception-handling-logging-and-observability.md) +cross-cutting foundation. + +The 2026-08-08 restructure moved correctness earlier (the corrected RLS template in +[ADR-0003 Amendment 3](docs/decisions/0003-tenant-isolation-defense-in-depth.md), durable +MUST-class audit in [ADR-0033](docs/decisions/0033-audit-durability-model.md)), moved +additive infrastructure later behind its ports +([ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md)), and moved the +genericity proof earlier — two seed tenants in Packet 7, rendered in a browser in +[Phase 02d](docs/roadmap/phase-02d-walking-skeleton.md), the next user-visible +milestone. Module assemblies hold no domain code yet. ```bash make install # one-time: deps + git hooks @@ -48,38 +72,56 @@ make dev # bring local stack up make seed # verify health + print demo credentials ``` +> `make seed`'s health gate currently times out, because three compose services declare +> no healthcheck. [Phase 02a Packet 3b](docs/roadmap/phase-02a-kernel-tenancy.md) fixes +> it along with the rest of the Phase 01 development-loop debt. + ## Direction At A Glance - **Backend:** .NET 10, ASP.NET Core, Entity Framework Core, MediatR. -- **Database:** PostgreSQL 18, with Row-Level Security from day one. Tenant + **Organization** - defense in depth ([ADR-0003 Amendment 1](docs/decisions/0003-tenant-isolation-defense-in-depth.md), - [ADR-0017](docs/decisions/0017-tenant-organization-hierarchy.md)). -- **Cache / Pub-Sub / Secrets:** Valkey 8 (RESP-protocol Linux-Foundation BSD fork - per [ADR-0030](docs/decisions/0030-redis-compatible-store-valkey.md)), Kafka, - HashiCorp Vault — all accessed via **Dapr** building blocks (`IEventBus`, - `ICacheService`, `ISecretProvider`) per - [ADR-0014](docs/decisions/0014-adopt-dapr.md). -- **API Gateway:** **APISIX** in file-driven standalone (`data_plane`) mode per - [ADR-0015](docs/decisions/0015-api-gateway-apisix.md). +- **Database:** PostgreSQL 18, with Row-Level Security from day one. Tenant + + **Organization** defense in depth + ([ADR-0003](docs/decisions/0003-tenant-isolation-defense-in-depth.md) Amendment 1 for + organization scope, **Amendment 3** for the corrected policy template and the + four-role database model, + [ADR-0017](docs/decisions/0017-tenant-organization-hierarchy.md)). The canonical SQL + lives in exactly one file: + [Database Standards](docs/standards/05-database.md). +- **Foundation ports:** `IEventBus`, `ICacheService`, `ISecretProvider`, + `IEntitlementProvider`, `IHostToTenantResolver` in `LearnStack.SharedKernel`, each + with a working default implementation. Vendor adapters — Dapr + ([ADR-0014](docs/decisions/0014-adopt-dapr.md)), Kafka, Valkey + ([ADR-0030](docs/decisions/0030-redis-compatible-store-valkey.md)), Vault, APISIX + ([ADR-0015](docs/decisions/0015-api-gateway-apisix.md)) — are **demand-gated**: each + has an owning phase and a written trigger condition in + [ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md). - **Object storage:** SeaweedFS locally, S3-compatible storage in production. -- **Search:** Meilisearch initially. +- **Search:** PostgreSQL full-text search first; Meilisearch behind `ITenantSearch` when + quality or scale requires it. - **Frontend:** Next.js 15 (App Router), TypeScript, React. **One** application - (`apps/web`) with route segments for public, studio, and portal — multi-app split - inside this repo deferred. The operator portal (`learnstack-hub-web`) lives in the - separate `learnstack-hub` repository. + (`apps/web`) with route segments for public, studio, and portal; a multi-app split + inside this repo is not planned before + [Phase 11](docs/roadmap/phase-11-production-hardening.md). The operator portal + (`frontend/apps/operator-portal`) lives in the separate `LearnStack-Hub` repository. - **Identity:** Self-hosted Keycloak with **two realms** — `learnstack` for tenant users, `learnstack-hub` for operators. - **Architecture:** Modular monolith with explicit module contracts. - **Tenant customization:** Per [ADR-0018](docs/decisions/0018-tenant-driven-customization-model.md), content types, page blocks, lesson item types, level taxonomies, scoring rules, completion rules, custom fields, and notification templates are **data** authored - by tenants, not code. The core stays generic. + by tenants, not code. The core stays generic, within the boundary the ADR's 2026-08-08 + amendment draws. - **Audit:** Append-only `LearnStack.Modules.Audit` with EF interceptor + MediatR - behavior + partitioned `audit_log` table per - [ADR-0016](docs/decisions/0016-audit-log-subsystem.md). + behavior. MUST-class audit is a durable intent written inside the business + transaction per [ADR-0033](docs/decisions/0033-audit-durability-model.md), which + supersedes ADR-0016; `audit_log` partitioning and retention land in + [Phase 11](docs/roadmap/phase-11-production-hardening.md). - **Entitlements:** Feature-based projection mirrored from the Hub per [ADR-0021](docs/decisions/0021-feature-based-entitlement.md); typed - `FeatureKeys` / `LimitKeys` registries. + `FeatureKeys` / `LimitKeys` registries. The Hub contract is governed by two + invariants — the Hub stores no tenant content, and every crossing goes through a + named adapter — per + [ADR-0034](docs/decisions/0034-hub-contract-surface-invariant.md). - **Live classroom:** In-app WebRTC; **self-hosted LiveKit OSS** is the default; LiveKit Cloud available behind the same `ILiveClassProvider` interface. A custom WebRTC SFU is explicitly out of scope. @@ -131,6 +173,7 @@ Platform substrate deep dives: - [30 — API Gateway (APISIX)](docs/architecture/30-api-gateway.md) - [31 — Audit Subsystem](docs/architecture/31-audit-subsystem.md) - [32 — Tenant Customization Model](docs/architecture/32-tenant-customization-model.md) +- [33 — Cross-Cutting Concerns](docs/architecture/33-cross-cutting-concerns.md) Decision context: - [18 — WebRTC Build vs Adopt](docs/architecture/18-webrtc-build-vs-adopt.md) @@ -138,16 +181,25 @@ Decision context: ### Decisions (`docs/decisions/`) - [ADR index](docs/decisions/README.md) — accepted decisions with their reasoning and - consequences. The 2026-05-18 redesign added ADRs 0014–0022. + consequences. The 2026-05-18 redesign added ADRs 0014–0022; the 2026-08-08 + restructure added [ADR-0033](docs/decisions/0033-audit-durability-model.md) (audit + durability, supersedes ADR-0016), + [ADR-0034](docs/decisions/0034-hub-contract-surface-invariant.md) (Hub contract + invariants) and + [ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md) (demand-gated + infrastructure). ### Engineering Standards (`docs/standards/`) - [Standards index](docs/standards/README.md) — rules that apply to every PR: coding, testing, security, observability, accessibility, performance, - [infrastructure-stack rules](docs/standards/20-infrastructure-stack.md), and more. + [infrastructure-stack rules](docs/standards/20-infrastructure-stack.md), and the + [architecture-test catalogue](docs/standards/21-architecture-tests-catalogue.md). ### Roadmap (`docs/roadmap/`) -- [Phased roadmap](docs/roadmap/README.md) — phases 00 through 12, including the - Phase 02c (Hub Foundation) and Phase 09b (Hub Billing) parallel tracks. +- [Phased roadmap](docs/roadmap/README.md) — phases 00 through 12, including + [Phase 02d](docs/roadmap/phase-02d-walking-skeleton.md) (Two-Tenant Walking Skeleton) + and the Phase 02c (Hub Integration) and Phase 09b (Hub Billing) parallel tracks. The + dependency map there is authoritative for order; filename order is not. ### Reference - [Glossary](docs/glossary.md) — canonical definitions. @@ -168,7 +220,11 @@ For the strategy and the headline decisions: 1. [Platform Vision](docs/architecture/01-platform-vision.md) 2. [MVP Scope](docs/architecture/05-mvp-scope.md) 3. [Roadmap overview](docs/roadmap/README.md) -4. [ADR index](docs/decisions/README.md) +4. [Phase 02a: Kernel + Tenancy](docs/roadmap/phase-02a-kernel-tenancy.md) — where the + work is now +5. [Phase 02d: Two-Tenant Walking Skeleton](docs/roadmap/phase-02d-walking-skeleton.md) + — where it is going next +6. [ADR index](docs/decisions/README.md) For the technical foundations: 1. [Technical Architecture](docs/architecture/04-technical-architecture.md) diff --git a/backend/analyzers/LearnStack.Analyzers/DomainExceptionThrowAnalyzer.cs b/backend/analyzers/LearnStack.Analyzers/DomainExceptionThrowAnalyzer.cs index 86fedd3..fb7b67d 100644 --- a/backend/analyzers/LearnStack.Analyzers/DomainExceptionThrowAnalyzer.cs +++ b/backend/analyzers/LearnStack.Analyzers/DomainExceptionThrowAnalyzer.cs @@ -61,7 +61,7 @@ public sealed class DomainExceptionThrowAnalyzer : DiagnosticAnalyzer defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true, description: Description, - helpLinkUri: "https://github.com/cemililik/LearnStack/blob/main/docs/decisions/0032-exception-handling-logging-and-observability.md"); + helpLinkUri: "https://github.com/HodeTech/LearnStack/blob/main/docs/decisions/0032-exception-handling-logging-and-observability.md"); public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); diff --git a/backend/src/LearnStack.SharedKernel/Secrets/ConfigurationSecretProvider.cs b/backend/src/LearnStack.SharedKernel/Secrets/ConfigurationSecretProvider.cs index 9fae778..7d1ff1d 100644 --- a/backend/src/LearnStack.SharedKernel/Secrets/ConfigurationSecretProvider.cs +++ b/backend/src/LearnStack.SharedKernel/Secrets/ConfigurationSecretProvider.cs @@ -5,9 +5,9 @@ namespace LearnStack.SharedKernel.Secrets; /// /// Default that delegates to /// . Phase 02a Packet 3 ships this as the -/// composition-root default for every DeploymentMode; Packet 5 -/// swaps it for the Dapr-backed implementation when running against a -/// Vault-equipped environment. +/// composition-root default for every DeploymentMode. Per ADR-0035 the +/// Dapr/Vault-backed implementation is demand-gated to Phase 11, triggered by +/// secrets needing rotation without a redeploy or a non-development deployment. /// /// /// The configuration layer already merges environment variables, user diff --git a/docs/architecture/01-platform-vision.md b/docs/architecture/01-platform-vision.md index 8a82cce..8bcbc49 100644 --- a/docs/architecture/01-platform-vision.md +++ b/docs/architecture/01-platform-vision.md @@ -1,23 +1,42 @@ # Platform Vision -LearnStack is a **PaaS for education** — a platform on which customers build their own -education platforms in arbitrary domains and disciplines. It is not itself an education -product. Customers might use LearnStack to build: +LearnStack is a **white-label platform for multi-branch education businesses that teach +live**. Its customer is an education business — a language school with three branches, a +yoga studio with four, a coding bootcamp, a music school — that sells courses, teaches +them partly or wholly in scheduled live sessions, and wants its own brand on its own +domain rather than a listing inside someone else's marketplace. LearnStack is not itself +an education product. + +Three properties define the fit. A prospect that has none of them is not the target +customer: + +- **Multi-branch.** Campuses, studios, departments, or cohorts that share one brand and + one catalog but keep their own members, staff, and schedules. This is the + `Organization` level ([ADR-0017](../decisions/0017-tenant-organization-hierarchy.md)), + and it is the reason a single-teacher course seller is a poor fit. +- **Teaches live.** Scheduling, booking, attendance, and an in-app live classroom are + first-class subsystems, not an add-on bolted to a video library. +- **Owns its brand.** Own domain, own design tokens, own locales, own notification + senders, own catalog — no LearnStack logo in the learner's browser. + +What is **not** fixed is the subject those businesses teach. The same code paths serve: - An online English-learning platform with CEFR levels and placement tests. - A yoga studio platform with asana taxonomy, sequence-based lessons, and teacher scheduling. -- A coding bootcamp with code-challenge lesson items and automatic test-runner grading. +- A coding bootcamp with programming-exercise lesson items and automatic grading. - A music school with score-reading lesson items, MIDI playback, and audio submissions. - A meditation app with timed practice sessions and habit-streak tracking. - A driving school with vehicle scheduling and progress-checkpoint workflows. - An art workshop with portfolio uploads and peer-review assessments. -- A meditation, certification, exam-prep, or any domain not anticipated above. +- A certification body, an exam-prep provider, or a domain not anticipated above. LearnStack ships **one codebase, one set of container images, one Helm chart** that serves all of these customers. The differentiator across customers is their **data** (content, content type definitions, page block schemas, scoring rules, level taxonomies, custom fields) — not their code. LearnStack engineers never write per-vertical code. +That claim holds inside a stated edge — see +[Genericity boundary](#genericity-boundary) below. ## Product thesis @@ -37,6 +56,9 @@ Education businesses need infrastructure that is: ([24-learnstack-hub.md](24-learnstack-hub.md)). - **Deployment-flexible.** Same codebase deploys as SaaS, Dedicated (LearnStack-managed single-tenant), or Self-Hosted ([25-deployment-models.md](25-deployment-models.md)). + `Development` and `SaaS` are wired end to end today; the other three `DeploymentMode` + values are prepared seams until Phase 11 builds their adapters and integration suites + ([ADR-0035](../decisions/0035-demand-gated-infrastructure.md)). ## The three layers @@ -85,11 +107,56 @@ flowchart TB See [28-platform-tenant-organization.md](28-platform-tenant-organization.md) for the full conceptual model. +## Genericity boundary + +"The domain is data" is true inside a boundary, and stating the boundary is what keeps +the claim credible. The boundary is drawn in +[ADR-0018 Amendment (2026-08-08)](../decisions/0018-tenant-driven-customization-model.md); +this section is its product-facing statement. + +**Inside the boundary — tenant customization data, no LearnStack release required:** + +| Dimension | What the tenant declares | Example | +|---|---|---| +| **Content shape** | A `TenantContentType` JSON Schema | A vocabulary card, an asana pose, a grammar topic, a repertoire piece | +| **Presentation** | Which blocks compose a page, which fields a card renders, what the level taxonomy is called | CEFR A1–C2 versus Foundation–Advanced versus kyu/dan | +| **Pure rule evaluation** | A `TenantScoringRule` or `TenantCompletionRule` expression | Map a placement-test answer set to a recommended level; decide a lesson is complete from facts already recorded | + +The discriminating property is that all three are **pure functions of tenant data and +already-recorded state**. Nothing in this column holds new state or reaches outside the +process, so a schema plus an evaluator is a complete answer. + +**Outside the boundary — platform features, written by LearnStack, gated by plan:** + +| Dimension | Example | Why it cannot be a customization row | +|---|---|---| +| **Stateful entitlement** | A ten-session credit pack; "three make-up classes per term"; a per-learner session quota | The tenant needs a balance that is decremented on booking, refunded on cancellation, expired on a schedule, and reconstructible in a dispute. **A JSON Schema declares a shape; it cannot declare a ledger.** | +| **External capability invocation** | Running a learner's submitted program; scoring pronunciation from an audio clip; automated proctoring | The tenant needs a sandbox, a runtime, a resource budget, and a security boundary that survives hostile input. **A rule DSL evaluates; it does not execute arbitrary programs.** | + +Said plainly: **a tenant that needs something in the second column needs a LearnStack +release, or an integration with an external provider through an adapter. It does not +need — and cannot have — a customization row.** Sales conversations, plan design, and +roadmap intake all depend on that sentence being said out loud rather than discovered +during implementation. + +Two things this boundary does **not** change: + +- **The core stays domain-neutral either way.** A credit-pack ledger is not a yoga + feature and an execution sandbox is not a coding-bootcamp feature. Both are named for + the capability, never for the domain that first asked for it — the rule + `Core_Modules_HaveNo_DomainSpecific_Names` enforces the naming mechanically from + [Phase 02a Packet 10](../roadmap/phase-02a-kernel-tenancy.md). +- **There is still no `Verticals/` folder.** A platform feature is a generic capability + offered to every tenant and switched on by a `FeatureKey`; it is not a per-vertical + code package. [ADR-0011](../decisions/0011-extension-points.md) stays superseded. + ## Design principles -- **Generic-only core.** No domain-specific code in LearnStack modules. CEFR, asanas, - code challenges live as **tenant data** (ADR-0018), never as `LearnStack.Verticals.*` - source code. +- **Generic-only core.** No domain-specific code in LearnStack modules. CEFR levels, + asana taxonomies, and programming-exercise shapes live as **tenant data** (ADR-0018), + never as `LearnStack.Verticals.*` source code. Where a tenant need falls outside the + customization model, it becomes a **generically named platform capability** — see + [Genericity boundary](#genericity-boundary) — not a vertical package. - **Multi-tenant from day one.** Tenant isolation is non-negotiable; defense-in-depth = tenant context + organization filter + EF query filter + RLS + architecture tests (ADR-0003 Amendment 1). @@ -100,9 +167,16 @@ conceptual model. The renderer is a client of the core, not the other way around. - **Provider adapters everywhere.** Payments, auth, storage, search, live classroom, notifications, recording — all behind interfaces. No SaaS lock-in baked into core code. -- **Auditability and event tracking as platform primitives.** Domain events, integration - events (outbox → Dapr pub/sub → Kafka — ADR-0014), and audit log (ADR-0016) are designed - in, not bolted on. +- **Auditability and event tracking as platform primitives.** Domain events, the + LearnStack-owned outbox, and the audit log are designed in, not bolted on. MUST-class + audit is a durable intent written inside the business transaction + ([ADR-0033](../decisions/0033-audit-durability-model.md), superseding ADR-0016). +- **Ports on day one, adapters on demand.** The one-way-door test + ([ADR-0035](../decisions/0035-demand-gated-infrastructure.md)) separates decisions that + get more expensive every week — isolation, schema ownership, typed identifiers — from + decisions a port makes reversible. `IEventBus` ships in Phase 02a; its Dapr/Kafka + adapter ships in Phase 11 when a second process needs to consume an integration event + (ADR-0014 decides *what*, ADR-0035 decides *when*). - **Versioned publish workflows** for content and courses that affect learners. - **Hub-separated control plane.** Tenant lifecycle, billing, licensing, custom domains, compliance run in a separate codebase (`learnstack-hub`, ADR-0019). LearnStack core @@ -134,9 +208,12 @@ When the foundation is in place, LearnStack should be able to: sales-assisted for Enterprise. - **Map a custom domain end-to-end** — DNS verification → Let's Encrypt cert → APISIX hot-reload → tenant resolver mapping. All Hub-orchestrated, ADR-0022. -- **Run three radically different tenant platforms on the same binary** — English - learning + yoga studio + coding bootcamp + driving school — without LearnStack writing - any domain code. +- **Run radically different tenant platforms on the same binary** — English learning + + yoga studio + coding bootcamp + driving school — without LearnStack writing any domain + code. Two such tenants exist from + [Phase 02a Packet 7](../roadmap/phase-02a-kernel-tenancy.md) and render side by side in + a browser from [Phase 02d](../roadmap/phase-02d-walking-skeleton.md), so the claim is + tested continuously rather than asserted until the showcase phase. - **Tenant admin defines a custom content type in Admin Studio** (e.g. "BreathTechnique" for a yoga tenant) — JSON Schema authored in a visual editor — and immediately uses it in a course lesson. @@ -148,8 +225,10 @@ When the foundation is in place, LearnStack should be able to: - **Air-gapped customer renews a license key offline** — signed `.lic` file delivered via SFTP, placed on disk, SIGHUP triggers re-read, entitlement refreshed — no outbound network required. -- **Regulator inquires about a specific tenant's history** — audit log (ADR-0016) plus - Hub audit stream answer "who did what when" with full correlation. +- **Regulator inquires about a specific tenant's history** — audit log + ([ADR-0033](../decisions/0033-audit-durability-model.md)) plus Hub audit stream answer + "who did what when" with full correlation, and the MUST-class rows are there because + the operation would have been rejected if they could not be written. - **Customer migrates from SaaS to Self-Hosted** — same binary, same data export tools, no rewrite. @@ -170,13 +249,20 @@ When the foundation is in place, LearnStack should be able to: ## References to formative decisions -- ADR-0003 Amendment 1 — Tenant Isolation + Organization scope. -- ADR-0014 — Adopt Dapr. -- ADR-0015 — APISIX gateway. -- ADR-0016 — Audit log subsystem. +- ADR-0003 Amendment 1 (Organization scope) + Amendment 3 (corrected RLS template and + database role model) — Tenant Isolation. +- ADR-0014 — Adopt Dapr (what), with ADR-0035 deciding when. +- ADR-0015 — APISIX gateway (what), with ADR-0035 deciding when. - ADR-0017 — Tenant + Organization hierarchy. -- ADR-0018 — Tenant-driven customization (supersedes ADR-0011 vertical packs). +- ADR-0018 — Tenant-driven customization (supersedes ADR-0011 vertical packs); the + 2026-08-08 Amendment draws the genericity boundary above. - ADR-0019 — LearnStack Hub. - ADR-0020 — Triple deployment + hybrid license. - ADR-0021 — Feature-based entitlement. - ADR-0022 — Custom domain & TLS. +- [ADR-0033](../decisions/0033-audit-durability-model.md) — Audit durability model + (supersedes ADR-0016). +- [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md) — Hub contract surface + invariant. +- [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) — Demand-gated + infrastructure. diff --git a/docs/architecture/02-domain-model.md b/docs/architecture/02-domain-model.md index 0edb2ba..9d38047 100644 --- a/docs/architecture/02-domain-model.md +++ b/docs/architecture/02-domain-model.md @@ -130,7 +130,7 @@ flowchart LR subgraph enrollment["Enrollment"] Enrollment - Entitlement + CourseAccess Cohort Progress end @@ -298,7 +298,7 @@ per ADR-0018, not on `Membership` extension tables. | Entity | Aggregate root? | Notes | |--------|-----------------|-------| | `Enrollment` | Yes | Grant of access to a `CourseVersion` (optionally bound to a `Cohort`). Has status (active, suspended, completed, cancelled) and source (manual, billing, invitation, integration). | -| `Entitlement` | Yes | Right to access a paid or assigned capability. Enrollment is one source. | +| `CourseAccess` | Yes | A learner's right to open a specific course. Sources: manual grant, invitation, cohort membership, billing. Not an `Entitlement` — that term is reserved for the Hub-owned, tenant-subject projection (see [glossary](../glossary.md)). | | `Cohort` | Yes | Group of learners progressing on a shared timeline through a `CourseVersion`. Has its own lifecycle (open → in-progress → completed → archived). | | `Progress` | Inside Enrollment | Learner advancement record. | @@ -365,7 +365,7 @@ See [In-App Live Classroom](07-in-app-live-classroom.md) for the provider abstra | `InvoiceReference` | Inside Order | Pointer to external invoice/payment record. | | `PaymentProviderAccount` | Yes | Per-tenant provider configuration. | -Billing produces `Entitlement`s; the Enrollment module consumes them through an integration event. +Billing produces `CourseAccess` grants; the Enrollment module creates them on consuming `OrderPaidV1`. ## Analytics @@ -406,12 +406,14 @@ examples live in [32-tenant-customization-model.md](32-tenant-customization-mode ## Audit -Per [ADR-0016](../decisions/0016-audit-log-subsystem.md), the Audit module owns the -append-only platform-level audit trail. +Per [ADR-0033](../decisions/0033-audit-durability-model.md), which supersedes +[ADR-0016](../decisions/0016-audit-log-subsystem.md), the Audit module owns the +append-only platform-level audit trail and MUST-class rows commit with the state change +they describe. | Entity | Aggregate root? | Notes | |--------|-----------------|-------| -| `AuditEntry` | Yes (append-only — inherits `Entity` NOT `AuditableEntity`) | One row per command/sensitive query/security event. Fields: `tenant_id`, `organization_id?`, `actor_user_id?`, `module`, `operation`, `operation_type`, `operation_class`, `entity_type?`, `entity_id?`, `is_success`, `error_key?`, `before_state` (jsonb), `after_state` (jsonb), `changes` (jsonb), `correlation_id?`, `ip_address?`, `user_agent?`, `timestamp`, `metadata?`. Stored in `audit_log` table partitioned by month. | +| `AuditEntry` | Yes (append-only — inherits `Entity` NOT `AuditableEntity`) | One row per command/sensitive query/security event. Fields: `tenant_id`, `organization_id?`, `actor_user_id?`, `module`, `operation`, `operation_type`, `operation_class`, `entity_type?`, `entity_id?`, `outcome` (`success` \| `denied` \| `failed`), `error_key?`, `before_state` (jsonb), `after_state` (jsonb), `changes` (jsonb), `correlation_id?`, `ip_address?`, `user_agent?`, `reason?`, `timestamp`, `metadata?`. Stored in the `audit_log` table — a single plain table with the composite key `(id, timestamp)` in Phase 02a; partitioned by month from [Phase 11](../roadmap/phase-11-production-hardening.md) per [ADR-0035](../decisions/0035-demand-gated-infrastructure.md). | | `AuditConfig` | Yes | Per-tenant override of per-(module, operation) audit enablement. Tenant-overridable within MUST/SHOULD/MAY classification. | Capture pipeline (`AuditChangeTrackerInterceptor` → `IAuditStateCapture` → diff --git a/docs/architecture/03-module-boundaries.md b/docs/architecture/03-module-boundaries.md index 4b05c37..430efcc 100644 --- a/docs/architecture/03-module-boundaries.md +++ b/docs/architecture/03-module-boundaries.md @@ -121,7 +121,7 @@ flowchart TB audit -. integration event consumer .-> classroom hubapi -. "mTLS + signed JWT + HMAC
POST /api/internal/*" .-> tenancy - identity -. "API key
POST /api/v1/internal/license/verify" .-> hubapi + identity -. "mTLS + JWT + HMAC
POST /api/v1/internal/license/verify" .-> hubapi ``` The dashed arrows are **integration events** (via Dapr pub/sub → Kafka, ADR-0014), **read-model projections**, or **Hub HTTPS contracts** — not direct calls or shared tables. @@ -237,7 +237,7 @@ Owns external provider credentials, webhooks, API keys, LTI/xAPI readiness, inte | Read another module's **public read model** (projection table). | Joining across module-owned tables in SQL. | | Use the **shared kernel** (ids, audit fields, errors, pagination, base types, `IEventBus`, `ICacheService`, `ISecretProvider`, `IEntitlementProvider`). | Importing another module's `Domain` namespace. | | Provide an **adapter implementation** at the composition root. | Domain-specific names (`CEFR`, `Asana`, `Kyu`, …) anywhere in a core module — those belong to tenant customization data, not code. | -| Read a **Hub-mirrored projection** (`platform_entitlement_cache`, `platform_host_to_tenant`) for read-only entitlement / host resolution. | Direct HTTPS calls to Hub from anywhere except `IEntitlementProvider` / `IHostToTenantResolver` adapter implementations. | +| Read a **Hub-mirrored projection** (`platform_entitlement_cache`, `platform_host_to_tenant`) for read-only entitlement / host resolution. | Direct HTTPS calls to Hub from anywhere except the `IEntitlementProvider` / `IUsageReporter` / `IHubTenantSync` adapter implementations; resolving a host by calling the Hub at all — `IHostToTenantResolver` reads `platform_host_to_tenant` and nothing else ([ADR-0034](../decisions/0034-hub-contract-surface-invariant.md)). | | Reference the `LearnStack.Modules.Audit` module via integration events (Audit subscribes to events from other modules). | Writing to `audit_log` directly from outside the Audit infrastructure pipeline. | Architecture tests enforce these rules — see [Testing Standards](../standards/06-testing.md). diff --git a/docs/architecture/04-technical-architecture.md b/docs/architecture/04-technical-architecture.md index 0c082fb..0b9447d 100644 --- a/docs/architecture/04-technical-architecture.md +++ b/docs/architecture/04-technical-architecture.md @@ -17,7 +17,7 @@ | Search | Meilisearch (initial), OpenSearch (later, if needed). See [ADR 0012](../decisions/0012-search-strategy.md) | | Auth | Keycloak (self-hosted OIDC) — two realms: `learnstack` (tenant users) + `learnstack-hub` (operators); ADR-0004 Amendment 1 | | Live classroom | LiveKit OSS (self-hosted) + coturn — see [07-in-app-live-classroom.md](07-in-app-live-classroom.md) | -| Frontend | Next.js (App Router), React, TypeScript — one app for tenant users (`apps/web`); separate Next.js app for Hub operators (`learnstack-hub-web`, in `learnstack-hub` repo) | +| Frontend | Next.js (App Router), React, TypeScript — one app for tenant users (`apps/web`); separate Next.js app for Hub operators (`operator-portal`, in `learnstack-hub` repo) | | API gateway | **Apache APISIX** (standalone mode) — JWT validation, rate limit, CORS, correlation ID; [30-api-gateway.md](30-api-gateway.md), [ADR-0015](../decisions/0015-api-gateway-apisix.md) | | API contract | REST + OpenAPI + RFC 7807 Problem Details; GraphQL only if a clear frontend requirement appears | | Observability | OpenTelemetry (traces + metrics + logs), Sentry, Grafana + Tempo + Loki + Prometheus | @@ -31,7 +31,7 @@ flowchart LR subgraph clients["Clients"] web[Public Site / Studio / Portal
Next.js apps/web] - hubweb[Hub Operator Portal
Next.js learnstack-hub-web] + hubweb[Hub Operator Portal
Next.js operator-portal] end subgraph edge["Edge"] @@ -97,7 +97,7 @@ flowchart LR lk --> egress hubapi -- "mTLS + signed JWT + HMAC
POST /api/internal/*" --> api - api -- "API key
POST /api/v1/internal/license/verify" --> hubapi + api -- "mTLS + JWT + HMAC
POST /api/v1/internal/license/verify" --> hubapi hubapi --> kc_hub hubapi --> pg ``` @@ -147,7 +147,7 @@ Future options (deferred): schema-per-tenant for enterprise tenants, read replic - **Cursor pagination** for list endpoints. Offset pagination is allowed only for admin-bounded lists. - **Idempotency keys** for `POST` operations that have external side effects (payments, webhooks, send-notification). - **Optimistic concurrency** for any mutable entity using `xmin` or `row_version` column. -- **API versioning** via URL prefix: `/v1/...`. Breaking changes bump to `/v2/...`; non-breaking additions stay on the existing version. ADR-pending. +- **API versioning** via URL prefix: `/api/v1/...`. Breaking changes bump to `/api/v2/...`; non-breaking additions stay on the existing version. See [ADR-0024](../decisions/0024-api-versioning-policy.md), which fixed exactly this `/v1/` vs `/api/v1/` inconsistency. - **Authentication** via OIDC bearer tokens issued by Keycloak. Frontends use Auth.js to bridge. - **Authorization** layered: tenant scope → role/permission → resource ownership where applicable. diff --git a/docs/architecture/05-mvp-scope.md b/docs/architecture/05-mvp-scope.md index 7757e2c..5f56fe8 100644 --- a/docs/architecture/05-mvp-scope.md +++ b/docs/architecture/05-mvp-scope.md @@ -24,8 +24,12 @@ Run an end-to-end online English-learning tenant on LearnStack, where: scoring rule, speaking-practice lesson item, lesson-package custom fields) is loaded from `TenantContentType`, `TenantLevelTaxonomy`, `TenantScoringRule`, etc. **No English-specific code lives in any module.** -- A second tenant exists in parallel with a **different** domain shape (e.g. coding or - yoga) loaded from its own customization data, proving the substrate is generic. +- A second tenant — a **yoga studio** — has existed since + [Phase 02a Packet 7](../roadmap/phase-02a-kernel-tenancy.md), with its own + taxonomy, content types and branding loaded from its own customization data. The + substrate-genericity proof is therefore continuous from + [Phase 02d](../roadmap/phase-02d-walking-skeleton.md) onward, not a checkbox at MVP + exit. ## Vertical Slice First @@ -200,9 +204,11 @@ contracts and the option to point at a Hub instance are MVP-complete: - Separate `learnstack-hub` repository ([ADR-0019](../decisions/0019-learnstack-hub.md)). - Separate Keycloak realm (`learnstack-hub`). - `Plan` / `HubSubscription` / `Entitlement` aggregates on the Hub side. -- mTLS + signed JWT + HMAC internal API: - `PUT /api/internal/tenants/{id}/entitlements`, `POST /api/internal/tenants`, - `POST /api/v1/internal/license/verify`, `POST /api/v1/usage/report`. +- mTLS + RS256 JWT + HMAC internal API, governed by the two invariants in + [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md) rather than by an + endpoint count: the Hub stores no tenant content, and every crossing goes through + `IEntitlementProvider` / `IUsageReporter` / `IHubTenantSync`. ADR-0034 § The + endpoint set enumerates the current paths. - Feature-based entitlement projection per [ADR-0021](../decisions/0021-feature-based-entitlement.md). @@ -217,8 +223,12 @@ Delivered as **tenant customization data** loaded at provisioning, **not** as co etc. - `TenantTemplateLibrary` populated with English-locale email templates. -The same provisioning pipeline loads a **second tenant** with a non-English customization -data set (the second tenant is the substrate-genericity proof; see Exit Criteria). +The **second tenant already exists** — the yoga studio seeded in +[Phase 02a Packet 7](../roadmap/phase-02a-kernel-tenancy.md) and rendered in a browser +since [Phase 02d](../roadmap/phase-02d-walking-skeleton.md). Phase 10 is therefore a +**depth** showcase rather than a breadth one: its job is to exercise *every* +customization aggregate against one real tenant, proving the customization surface is +complete. Genericity is already proven, and re-proven on every CI run. ## Deferred @@ -270,9 +280,19 @@ extend a customization aggregate?" — not "should we add a core module?". instructor and tenant admin. Recording execution itself is **tenant-configurable and off by default** ([16-media-pipeline.md](16-media-pipeline.md)); whether any session actually records during MVP exit depends on the tenant's policy. -- A **second tenant** exists in parallel with a different customization data set (e.g. - a coding bootcamp with `Track` levels and `CodeChallenge` lesson items); cross-tenant - isolation tests pass and the same code paths serve both tenants without modification. +- The **second tenant** (the yoga studio, seeded in + [Phase 02a Packet 7](../roadmap/phase-02a-kernel-tenancy.md)) still runs on the same + code paths with a different customization data set; cross-tenant isolation tests pass + under the `learnstack_app` role. + + > A coding bootcamp was the other candidate and was **dropped**. Its distinguishing + > feature — running a learner's submitted code — falls outside the genericity + > boundary drawn in + > [ADR-0018's amendment](../decisions/0018-tenant-driven-customization-model.md): + > external capability invocation needs a sandbox, a runtime and a resource budget, + > so it is a plan-gated platform feature written by LearnStack, not a customization + > row. Choosing it as the genericity proof would have proven the opposite of the + > intended point. - Every MUST-class audit event in [Audit Coverage Standard](../standards/18-audit-coverage.md) is captured for both tenants and queryable via the Audit admin API. - LearnStack runs against either `NullEntitlementProvider` (no Hub) or against a real diff --git a/docs/architecture/06-extension-model.md b/docs/architecture/06-extension-model.md index f5583a0..8e7312b 100644 --- a/docs/architecture/06-extension-model.md +++ b/docs/architecture/06-extension-model.md @@ -74,7 +74,9 @@ The original Extension Model document's invariants are preserved by ADR-0018: - **Core stays generic.** No `Cefr`, `Asana`, `CodeChallenge`, `EnglishPlacement`, `YogaSequence` etc. in any LearnStack module. Architecture test - `No_DomainSpecific_Names_In_Modules` enforces this. + `Core_Modules_HaveNo_DomainSpecific_Names` enforces this + ([canonical spelling](../standards/21-architecture-tests-catalogue.md)); the folder + rule is the separate `No_Source_Folder_Named_Verticals`. - **Anti-patterns to reject** (still apply): - Adding domain-specific columns to core entities. - Importing live-classroom SDK types in Domain or Application. @@ -151,10 +153,18 @@ implementations: | `ISmsProvider` | `TwilioSmsProvider`, `NetGsmSmsProvider` | | `ILiveClassProvider` | `LiveKitSelfHostedProvider`, `LiveKitCloudProvider` | | `IFileStorageService` | `MinioFileStorageService`, `S3FileStorageService` | -| `IEventBus` | `DaprEventBus` (default), `InProcessEventBus` (dev fallback) | -| `ICacheService` | `DaprCacheService` (default), `InMemoryCacheService` (dev fallback) | -| `ISecretProvider` | `DaprSecretProvider` (default), `EnvironmentSecretProvider` (dev fallback) | -| `IEntitlementProvider` | `NullEntitlementProvider` (dev), `HubEntitlementProvider` (online), `SignedLicenseKeyEntitlementProvider` (air-gapped) | +| `IEventBus` | `InProcessEventBus` (lands with the port in Packet 5), `DaprEventBus` (demand-gated to Phase 11) | +| `ICacheService` | `InMemoryCacheService` (lands with the port in Packet 5), `DaprCacheService` (demand-gated to Phase 11) | +| `ISecretProvider` | `ConfigurationSecretProvider` (**registered today**, shipped in Packet 3), `DaprSecretProvider` (demand-gated to Phase 11) | +| `IEntitlementProvider` | `NullEntitlementProvider` (Packet 9), `HubEntitlementProvider` (Phase 02c), `SignedLicenseKeyEntitlementProvider` (skeleton from Hub `P02c-6`, hardened in Phase 11) | + +The last three rows are the demand-gated set from +[ADR-0035](../decisions/0035-demand-gated-infrastructure.md): the port and its default +ship together, and the vendor adapter ships in the phase named against its written +trigger. Only `ISecretProvider` has shipped so far — the other two ports and their +defaults land in [Phase 02a Packet 5](../roadmap/phase-02a-kernel-tenancy.md). The +in-process implementations are not a development convenience: once registered they are +the only implementations in **every** deployment mode until Phase 11. Adding a new provider is a code change in core (new adapter implementation in `LearnStack.Infrastructure`) — not a tenant action. diff --git a/docs/architecture/09-tenant-isolation.md b/docs/architecture/09-tenant-isolation.md index 35b003c..7bc6ae9 100644 --- a/docs/architecture/09-tenant-isolation.md +++ b/docs/architecture/09-tenant-isolation.md @@ -15,8 +15,14 @@ two scopes (tenant + organization): - `tenant_id` on every tenant-owned table (mandatory). - `organization_id` on every org-scoped tenant-owned table (nullable; null = tenant-wide). - EF Core global query filters for both `tenant_id` and `organization_id`. -- PostgreSQL Row Level Security policies on every tenant-owned table; org-scoped tables - carry an additional policy. +- PostgreSQL Row Level Security on every tenant-owned table: **one** permissive policy + whose predicate `AND`s the tenant term with the organization term, plus the two + `AS RESTRICTIVE` write guards when the table is org-scoped. Never a second permissive + policy — PostgreSQL combines those with `OR`, which is the defect + [ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) + corrects. Platform-scoped tables — the ones read *before* the tenant is known — follow + a different rule; see + [Database Standards § Table classes](../standards/05-database.md). - Architecture tests that detect unprotected tenant-owned / org-scoped entities. - Explicit platform-admin paths for cross-tenant operations (Hub-driven, audited). - Tenant- and org-aware storage, cache, search, analytics, audit, and logs. @@ -27,11 +33,11 @@ two scopes (tenant + organization): |-------|------------------|------------------------| | Application context | `ITenantContextAccessor.Current.TenantId` (AsyncLocal) | `ITenantContextAccessor.Current.OrganizationId` (AsyncLocal; nullable) | | EF Core | Global query filter `e.TenantId == currentTenantId` | Global query filter `e.OrganizationId == null OR e.OrganizationId == currentOrgId` | -| PostgreSQL | RLS policy `tenant_id = current_setting('app.tenant_id', true)::uuid` | RLS policy `organization_id IS NULL OR organization_id = current_setting('app.organization_id', true)::uuid` | +| PostgreSQL | The tenant term of the single policy: `tenant_id = NULLIF(current_setting('app.tenant_id', true), '')::uuid` | The organization term `AND`-ed into that **same** policy, plus the restrictive `UPDATE` / `DELETE` write guards. Canonical SQL in [Database Standards](../standards/05-database.md) | | Identity | Single-realm `learnstack` with `tenant_id` JWT claim (default per [ADR-0004](../decisions/0004-authentication-strategy.md); realm-per-tenant is an opt-in for enterprise isolation only) | `organization_id` JWT claim populated from active org membership | | Cache | Cache key auto-prefixed `{tenant_id}:{key}` | `{tenant_id}:{org_id}:{key}` when org context set | | Files (SeaweedFS) | Object key prefix `tenants/{tenant_id}/...` | `tenants/{tenant_id}/organizations/{org_id}/...` for org-scoped assets | -| Search (Meilisearch) | `tenant_id` as mandatory filter | `organization_id = X OR organization_id IS NULL` clause when org context | +| Search | `tenant_id` as a mandatory filter composed **inside** `ITenantSearch` — callers pass criteria, never filter strings. Until Meilisearch's demand gate fires ([ADR-0035](../decisions/0035-demand-gated-infrastructure.md)), search runs on PostgreSQL full-text over tenant-owned tables and inherits Row Level Security; the engine-enforced per-request tenant token arrives with the Meilisearch adapter in [Phase 09](../roadmap/phase-09-billing-integrations-analytics.md) | `organization_id = X OR organization_id IS NULL` clause when org context | | Jobs (Hangfire) | `JobParams.TenantId` mandatory | `JobParams.OrganizationId` nullable | | Audit (ADR-0016) | `audit_log.tenant_id` mandatory | `audit_log.organization_id` nullable | | Logs (Serilog) | Every log scope carries `TenantId` | `OrganizationId` when context set | @@ -55,8 +61,8 @@ sequenceDiagram API->>MW: HTTP pipeline MW->>MW: Read tenant_id, organization_id from JWT claims MW->>Accessor: SetTenant(tenantId, organizationId, userId) - Accessor->>PG: SET LOCAL app.tenant_id = '...'
SET LOCAL app.organization_id = '...' MW->>API: continue + API->>EF: BeginTransaction, then SET LOCAL app.tenant_id /
app.organization_id as the first statement (TransactionBehavior, step 6) API->>EF: Query tenant-owned aggregate EF->>EF: Apply global filter (tenant + org) EF->>PG: Query with WHERE tenant_id = X AND (organization_id = Y OR IS NULL) @@ -66,38 +72,31 @@ sequenceDiagram ## RLS policy templates -### Tenant-only entity - -```sql -ALTER TABLE ADD COLUMN tenant_id uuid NOT NULL; -CREATE INDEX ix_
_tenant_id ON
(tenant_id); - -ALTER TABLE
ENABLE ROW LEVEL SECURITY; -CREATE POLICY
_tenant_isolation ON
- USING (tenant_id = current_setting('app.tenant_id', true)::uuid); -``` - -### Tenant + Organization entity - -```sql -ALTER TABLE
ADD COLUMN tenant_id uuid NOT NULL; -ALTER TABLE
ADD COLUMN organization_id uuid NULL; -CREATE INDEX ix_
_tenant_org ON
(tenant_id, organization_id); - -ALTER TABLE
ENABLE ROW LEVEL SECURITY; -CREATE POLICY
_tenant_isolation ON
- USING (tenant_id = current_setting('app.tenant_id', true)::uuid); -CREATE POLICY
_organization_isolation ON
- USING ( - organization_id IS NULL -- tenant-wide row, visible to all orgs in tenant - OR organization_id = current_setting('app.organization_id', true)::uuid -- org-scoped row, only matching org - OR current_setting('app.scope', true) = 'tenant' -- tenant-scope operation (admin / reporting) sees all orgs - ); -``` +The **canonical SQL template** — for both the tenant-only and the tenant + organization +shape — lives in exactly one place: +[Database Standards § Tenant-Owned and Organization-Scoped Tables](../standards/05-database.md). +It is not repeated here. Copying it into a second document is how the four divergent +copies that preceded [ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) +came about. + +Three properties of that template matter to the isolation model described on this page: + +- **One policy, one `AND`-ed predicate.** Tenant and organization scope are evaluated + together. Two separate policies would both be *permissive*, and PostgreSQL combines + permissive policies with `OR` — under which a tenant-wide row (`organization_id IS + NULL`) satisfies the organization half on its own and becomes visible to every + tenant. A second policy may only ever be added `AS RESTRICTIVE`. +- **`ENABLE` and `FORCE ROW LEVEL SECURITY`.** Without `FORCE`, the table owner + bypasses its own policies — and the default Entity Framework Core arrangement makes + the application that owner. +- **An explicit `WITH CHECK`.** `USING` decides what is readable; `WITH CHECK` decides + what is writable. The `app.scope = 'tenant'` setting is set by middleware when the request comes from a -tenant-admin role with a tenant-wide operation flag (e.g. cross-org reporting). The -default scope (`null` or `'organization'`) honours both filters strictly. +tenant-admin role with a tenant-wide operation flag (e.g. cross-org reporting). It +widens **reads** across organizations within the caller's tenant; it never widens +writes, and it never crosses a tenant boundary. The default scope (`null` or +`'organization'`) restricts reads to the caller's organization plus tenant-wide rows. ## Default org semantics @@ -118,9 +117,14 @@ Platform admin (LearnStack operator) access must be explicit: not `learnstack` realm. - Cross-tenant queries from Hub go through `/api/internal/*` endpoints with mTLS + signed JWT + HMAC; never proxied via APISIX (ADR-0019). -- LearnStack-side endpoint receives request with no tenant context; sets a special - `learnstack_audit_admin` Postgres role for the query, which **bypasses RLS** (and - emits a `read-sensitive` audit row for every cross-tenant access). +- The LearnStack-side endpoint receives a request with no tenant context and runs the + query through `EnterPlatformAdminScope(reason)`, which opens a **second connection** + authenticated as `learnstack_platform` — the `BYPASSRLS` role of the four-role model. + There is no `learnstack_audit_admin` role, and `learnstack_app` is not a member of + `learnstack_platform`, so the application role cannot reach the bypass by `SET ROLE`. + Every cross-tenant access emits a `read-sensitive` audit row, written inside the scope + under the sentinel platform tenant id. See + [Database Standards § Database roles](../standards/05-database.md). - No hidden arbitrary `IgnoreQueryFilters()` usage; architecture test `IgnoreQueryFilters_OnlyInPlatformAdminScope` forbids it outside the `LearnStack.Modules.Identity.Application.Platform` namespace. @@ -181,8 +185,8 @@ public abstract class PlatformJob : LearnStackJob |------|---------| | `Every_TenantOwned_Entity_HasTenantId` | Every aggregate marked `[TenantOwned]` (or inheriting `AuditableEntity<>`) has a `TenantId` property and an EF query filter referencing it. | | `Every_OrgScoped_Entity_HasOrgIdAndFilter` | Every aggregate marked `[OrganizationScoped]` has `OrganizationId` nullable + EF query filter. | -| `Every_TenantOwned_Table_HasRlsPolicy` | Migration scan: every tenant-owned table has at least one RLS policy. | -| `Every_OrgScoped_Table_HasOrgRlsPolicy` | Migration scan: every org-scoped table has the org isolation policy. | +| `Every_TenantOwned_Table_HasRlsPolicy` | Migration scan: every tenant-owned table has `ENABLE` **and** `FORCE ROW LEVEL SECURITY` and **exactly one** permissive policy with an explicit `WITH CHECK`. Two permissive policies fail the test. | +| `Every_OrgScoped_Table_HasOrgRlsPolicy` | Migration scan: the organization term is `AND`-ed inside that single policy — not in a second permissive one — and both `AS RESTRICTIVE` write guards are present. | | `IgnoreQueryFilters_OnlyInPlatformAdminScope` | Roslyn source scan: `IgnoreQueryFilters()` appears only in `LearnStack.Modules.Identity.Application.Platform` or behind an `architecture-allow: ignore-query-filters ADR-NNNN` marker. | | `Hangfire_JobPayloads_IncludeTenantId` | Reflection: every `LearnStackJob` subclass's `TParams` has `TenantId`. | | `LearnStackJob_RunAsync_SetsTenantBeforeExecute` | Source-grep + reflection: `RunAsync` is non-virtual; `SetTenant(...)` precedes `ExecuteAsync(...)`. | diff --git a/docs/architecture/12-localization.md b/docs/architecture/12-localization.md index 9227260..2e7a57d 100644 --- a/docs/architecture/12-localization.md +++ b/docs/architecture/12-localization.md @@ -51,35 +51,70 @@ For entities like `Course`, `Lesson`, `Page`, `ContentEntry`: ```sql CREATE TABLE courses ( - id UUID PRIMARY KEY, - tenant_id UUID NOT NULL, - slug_key TEXT NOT NULL, -- locale-independent stable identifier - visibility TEXT NOT NULL, - -- non-translatable fields only - created_at TIMESTAMPTZ NOT NULL, + id uuid PRIMARY KEY, + tenant_id uuid NOT NULL, + organization_id uuid NULL, -- null = tenant-wide, per ADR-0017 + slug_key text NOT NULL, -- stable authoring handle; NOT routable + visibility text NOT NULL, + -- non-translatable columns only: no title, no description, no slug + created_at timestamptz NOT NULL, -- ... - UNIQUE (tenant_id, slug_key) + CONSTRAINT ux_courses_tenant_id_slug_key UNIQUE (tenant_id, slug_key), + CONSTRAINT ux_courses_tenant_id_id UNIQUE (tenant_id, id) ); CREATE TABLE course_translations ( - course_id UUID NOT NULL REFERENCES courses(id) ON DELETE CASCADE, - locale TEXT NOT NULL, - title TEXT NOT NULL, - description TEXT, - slug TEXT NOT NULL, -- locale-specific URL slug - seo_title TEXT, - seo_description TEXT, - PRIMARY KEY (course_id, locale) + course_id uuid NOT NULL, + tenant_id uuid NOT NULL, -- a real column: RLS is per table, never inherited + organization_id uuid NULL, -- mirrors the parent; for RLS, never for uniqueness + locale text NOT NULL, + title text NOT NULL, + description text NULL, + slug text NOT NULL, -- locale-specific URL slug + seo_title text NULL, + seo_description text NULL, + PRIMARY KEY (course_id, locale), + -- The routing constraint. Every column in it is NOT NULL, so PostgreSQL's + -- nulls-are-distinct rule cannot apply and it rejects every duplicate. + CONSTRAINT ux_course_translations_tenant_id_locale_slug + UNIQUE (tenant_id, locale, slug), + CONSTRAINT fk_course_translations_course + FOREIGN KEY (tenant_id, course_id) REFERENCES courses (tenant_id, id) + ON DELETE CASCADE ); - -CREATE UNIQUE INDEX course_translations_slug_unique - ON course_translations (course_id, locale, slug); ``` -- A row in the parent table represents the entity. -- A row in the translation table represents the entity *in a specific locale*. -- Slug is per-locale; the same course has `/tr/kurslar/baslangic-ingilizce` and `/en/courses/beginner-english`. -- Both tables include or join via `tenant_id` for RLS purposes (translation tables inherit tenant ownership through the parent via a check constraint). +> **Corrected 2026-08-09.** The block above previously declared +> `CREATE UNIQUE INDEX course_translations_slug_unique ON +> course_translations (course_id, locale, slug)`. Those index columns are a proper +> superset of the primary key `(course_id, locale)`, so the primary key already +> guaranteed it and the index could reject no row the table would otherwise accept — two +> different courses in one tenant could both hold `/en/courses/beginner`. The block also +> claimed translation tables inherit tenant ownership through a parent check constraint, +> which PostgreSQL does not do. Both are fixed here; the rule is published in +> [Localization Standards § Pattern A](../standards/08-localization.md). + +- A row in the parent table represents the entity. A row in the translation table + represents the entity *in a specific locale*. +- Slug is per-locale; the same course has `/tr/kurslar/baslangic-ingilizce` and + `/en/courses/beginner-english`. +- `slug_key` is an authoring convenience — a stable handle for translators and for + import / export. **Nothing routes on it.** The routable identifier is + `course_translations.slug`. +- Translation tables carry `tenant_id` as a real column and declare their own + `ENABLE` + `FORCE ROW LEVEL SECURITY` and the full policy set from the canonical + template in [Database Standards](../standards/05-database.md). Row Level Security is + per table; it is not inherited from a parent through a check constraint, and a + satellite carrying `title` and `slug` carries the content. +- `organization_id` mirrors the parent and exists **only** so the satellite can carry the + same isolation predicate. It is deliberately absent from the slug constraint — see + [§ Slugs and URLs](#slugs-and-urls). Denormalizing it is safe because + `organization_id` on a tenant-owned row is immutable after insert; see + [Database Standards § Translation satellite tables](../standards/05-database.md). +- The foreign key is composite on `tenant_id` for the reason in + [Database Standards § Foreign keys between tenant-owned tables](../standards/05-database.md): + referential-integrity checks run with Row Level Security bypassed, so a single-column + key would let one tenant's translation row reference another tenant's course. ### Pattern B: JSONB Field (for compact, optional translations) @@ -143,7 +178,35 @@ Slugs are **per locale**. Two patterns: The Next.js renderer reads tenant locale config at the edge and produces locale-aware routes. -Slug uniqueness is `(tenant_id, locale, slug)`. The same entity can have completely different slugs per locale. +Slug uniqueness is `UNIQUE (tenant_id, locale, slug)`, declared on the translation table, +and **flat across organizations**. The same entity can have completely different slugs +per locale; two different entities in one tenant cannot share one slug in one locale. + +`organization_id` is deliberately not part of that key. A host resolves to +`(tenant_id, organization_id?)`, and the canonical isolation policy admits +`organization_id IS NULL OR organization_id = ` — so an organization's host +serves tenant-wide rows *and* its own, both tiers compete for one URL, and a key that +partitioned them would force the renderer to pick a winner. Preferring the +organization-scoped row means publishing a branch course silently changes what an +already-published tenant URL serves, with no redirect and no signal to the author who +owns the tenant-wide row — the "URL changes are breaking" risk below, arrived at without +anyone editing a slug. Preferring the tenant-wide row is worse: it lets an organization +author create a row no URL can reach. One flat namespace per `(tenant_id, locale)` +removes the question. An organization that wants its own variant of a shared course gives +it its own slug, and the publish command rejects the collision with a business-rule +failure rather than resolving it at render time. + +The routing consequences follow directly, and are behaviour rather than defects: + +- A host resolving to `(tenant_id, NULL)` serves tenant-wide rows only. Organization-scoped + rows are filtered out by the isolation policy, so their slugs 404 there even though the + slug is reserved tenant-wide. +- A host resolving to `(tenant_id, organization_id)` serves both tiers, and the flat key + guarantees at most one match. +- Slug lookup is **exact**. The fallback chain below resolves display fields after the + entity is found; it never resolves a slug. An entity with no translation in the + requested locale has no URL in that locale, and a link to it is omitted rather than + rendered dead. ## UI String Catalogue @@ -189,15 +252,37 @@ CREATE TABLE tenant_template_library ( created_by uuid NOT NULL, updated_at timestamptz NOT NULL DEFAULT now(), updated_by uuid NOT NULL, + -- NULLS NOT DISTINCT (PostgreSQL 15+; LearnStack pins 18 per ADR-0031) is + -- load-bearing here. organization_id is null on every tenant-wide template, and a + -- standard UNIQUE treats nulls as distinct, so without it a tenant could hold + -- unlimited duplicate tenant-wide rows for one (key, channel, locale) and dispatch + -- would pick one arbitrarily. organization_id genuinely belongs in this key: an org + -- override is a distinct row that dispatch resolves through the org -> tenant + -- fallback chain described below. Contrast a routable slug, where the two tiers + -- compete for one URL and the column is dropped from the key instead -- see + -- Database Standards section Constraints. CONSTRAINT ux_tenant_template_library - UNIQUE (tenant_id, organization_id, key, channel, locale) + UNIQUE NULLS NOT DISTINCT (tenant_id, organization_id, key, channel, locale) ); -ALTER TABLE tenant_template_library ENABLE ROW LEVEL SECURITY; -CREATE POLICY tenant_template_library_tenant_isolation ON tenant_template_library - USING (tenant_id = current_setting('app.tenant_id')::uuid); + +-- Row Level Security: apply the canonical template from +-- docs/standards/05-database.md § Tenant-Owned and Organization-Scoped Tables +-- to this table verbatim, substituting tenant_template_library. It is org-scoped, +-- so it takes the full set: ENABLE + FORCE, one permissive policy with the +-- organization term AND-ed in, and both AS RESTRICTIVE write guards. The SQL is +-- deliberately not repeated here — it lives in exactly one file, because the +-- version that lived in four was wrong in all four (ADR-0003 Amendment 3). ``` +This table is org-scoped (`organization_id` is nullable and participates in the +uniqueness constraint), so its organization term lives **inside** the single policy — +per the canonical template in +[Database Standards](../standards/05-database.md) and +[ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md). A +separate organization policy would be `PERMISSIVE` and `OR`-ed with this one, which +would expose every tenant-wide template to every tenant. + Dispatch (in the Notifications module) resolves the recipient's preferred locale, applies the organization → tenant → tenant-default fallback chain, then renders the chosen row with the dispatch context. The two-table `notification_templates` + diff --git a/docs/architecture/14-frontend-architecture.md b/docs/architecture/14-frontend-architecture.md index 2a46b41..dcf69e9 100644 --- a/docs/architecture/14-frontend-architecture.md +++ b/docs/architecture/14-frontend-architecture.md @@ -7,7 +7,7 @@ LearnStack ships **two independent Next.js applications**: marketing/CMS rendering, admin Studio, and learner/instructor portals. The multi-app split is deferred until concrete need ([ADR 0009 — Frontend Single App First](../decisions/0009-frontend-single-app-first.md)). -- **`learnstack-hub-web`** in the **separate `learnstack-hub` repository** — the +- **`operator-portal`** in the **separate `learnstack-hub` repository** — the **operator portal** at `hub.learnstack.dev`. Operators authenticate against the `learnstack-hub` Keycloak realm (ADR-0004 Amendment 1); different realm, different user pool, different domain. The two apps **do not share code at runtime**. If a @@ -61,7 +61,7 @@ frontend/ config/ # eslint, tsconfig, tailwind shared bits ``` -The operator portal (`learnstack-hub-web`) is a **separate Next.js application in the +The operator portal (`operator-portal`) is a **separate Next.js application in the separate `learnstack-hub` repository**; nothing about it lives under this `frontend/` tree. @@ -264,7 +264,7 @@ actions**. The tenant-facing surface in `apps/web` is read-only: - Studio renders a banner with the DNS records the tenant must add and a "Recheck now" button that **proxies to a Hub admin endpoint** through the internal API. - Registering a *new* custom domain happens in the **operator portal** - (`learnstack-hub-web`), not in `apps/web`. Tenant admins request a domain via a form + (`operator-portal`), not in `apps/web`. Tenant admins request a domain via a form in Studio that creates a support ticket / Hub-side request — the actual create is an operator action. @@ -304,14 +304,14 @@ CI runs Lighthouse on representative public pages on every PR; budgets failing t ## Splitting into Multiple Apps Later -The operator portal split has already happened: `learnstack-hub-web` is a *separate +The operator portal split has already happened: `operator-portal` is a *separate repository*, not a separate app within this repo. Within `apps/web`, if and when the single-app model breaks down (rebuild times, deploy cadence conflicts, separate teams owning different surfaces), the split path is: 1. Extract `packages/ui` first — duplicated primitives become a shared package. (This is the same `packages/ui` candidate that, post-extraction, could be a build-time - dependency for `learnstack-hub-web` as well.) + dependency for `operator-portal` as well.) 2. Extract `packages/sdk` — already generated, easy lift. 3. Move `(studio)` into `apps/studio`. Keep `(public)` and `(portal)` together initially. diff --git a/docs/architecture/15-event-and-outbox.md b/docs/architecture/15-event-and-outbox.md index 4c4cda6..5087195 100644 --- a/docs/architecture/15-event-and-outbox.md +++ b/docs/architecture/15-event-and-outbox.md @@ -1,25 +1,52 @@ # Events and Outbox Events allow modules to collaborate without cross-module database coupling. This document -defines the event types, the outbox pattern, and how dispatch is delivered via Dapr -pub/sub to Kafka (ADR-0010 Amendment 1 + ADR-0014). +defines the event types, the outbox pattern, the claim protocol that makes concurrent +dispatch safe, and how dispatch reaches subscribers — in process today, through Dapr +pub/sub to Kafka when the trigger for that adapter fires (ADR-0010 Amendment 1 + +ADR-0014 + ADR-0035). ## Decision Use **domain events** within a module (in-process MediatR notifications, same transaction as the aggregate change) and **integration events** across modules (written to an outbox -in the same transaction; dispatched at-least-once to subscribers via Dapr pub/sub to -Kafka with retry / backoff / dead-lettering). +in the same transaction; dispatched at-least-once to subscribers with retry / backoff / +dead-lettering on both the producer and the subscriber side). -The outbox table is **LearnStack-owned** (per-module in PostgreSQL); Dapr is the -**dispatch transport**, not the durable buffer. +The outbox table is **LearnStack-owned** (a single shared table in PostgreSQL); the event +bus is the **dispatch transport**, not the durable buffer. That ownership split is what +makes the transport swappable without touching a single producer or consumer. + +## What ships when + +Three separable things are often collapsed into one; they are not the same and they do +not land together. + +| Piece | Owning phase | State | +|---|---|---| +| `outbox_messages` table, its schema, and its LearnStack ownership | [Phase 02a Packet 6](../roadmap/phase-02a-kernel-tenancy.md) | Ships even though nothing dispatches from it yet — the schema and its ownership are a one-way door | +| `IEventBus` port + `InProcessEventBus` | [Phase 02a Packet 5](../roadmap/phase-02a-kernel-tenancy.md) | The only registered implementation | +| `OutboxProcessor`, `IInboxGuard`, the claim protocol, the first real integration event | [Phase 02b](../roadmap/phase-02b-events-auth.md) | The durable dispatcher | +| `DaprEventBus` → Dapr pub/sub → Kafka | [Phase 11](../roadmap/phase-11-production-hardening.md) | Demand-gated; trigger: a second process needs to consume an integration event, or event volume / replay / cross-process ordering is required ([ADR-0035](../decisions/0035-demand-gated-infrastructure.md)) | + +**`InProcessEventBus` is a first-class transport, not a stub.** It uses the same +`IIntegrationEventHandler` interface, the same `IInboxGuard` deduplication, and the +same tenant-context restoration as the durable path. A development transport that skips +those is a development transport that never exercises the isolation code — and it forces +every consumer to carry two implementations, one of which is never tested against the +other. Everything in this document about consumer obligations applies identically to both +transports; the only difference is what carries the bytes between publish and handle. + +[ADR-0014](../decisions/0014-adopt-dapr.md) stands as the decision that Dapr is the +cross-process transport LearnStack uses. [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) +decides when it arrives, and the answer is "when a second process exists". ## Event types | Type | Scope | Transport | Example | |------|-------|-----------|---------| | **Domain event** | Inside one module | MediatR `INotification` in-process | `CourseVersionPublished` | -| **Integration event** | Cross-module | Outbox → `IEventBus.PublishAsync` → Dapr pub/sub → Kafka | `OrderPaidV1`, `EnrollmentCreated`, `TenantSuspended` | +| **Integration event** | Cross-module | Outbox → `IEventBus.PublishAsync` → transport (in-process dispatcher today; Dapr pub/sub → Kafka from Phase 11) | `OrderPaidV1`, `EnrollmentCreated`, `TenantSuspended` | | **Analytics event** | Reporting stream | Same channel as integration events | `LessonCompleted` | | **Provider event** | External callback (webhook) | Provider → API endpoint → outbox → ... | `LiveKitParticipantJoined`, `StripeInvoicePaid` | @@ -28,12 +55,11 @@ The outbox table is **LearnStack-owned** (per-module in PostgreSQL); Dapr is the ```mermaid sequenceDiagram participant Module as Owning Module - participant DB as PostgreSQL (module schema) + participant DB as PostgreSQL participant Outbox as outbox_messages participant Processor as OutboxProcessor (BackgroundService) - participant Dapr as Dapr sidecar - participant Kafka - participant ConsumerDapr as Consumer Dapr sidecar + participant Bus as IEventBus + participant Transport as Transport (in-process dispatcher, or Dapr to Kafka) participant Consumer as Consuming Module participant Inbox as inbox_messages @@ -42,60 +68,56 @@ sequenceDiagram DB->>DB: COMMIT (atomic: aggregate + outbox row) loop Polling (every 200ms; configurable) - Processor->>Outbox: SELECT WHERE processed_at IS NULL
FOR UPDATE SKIP LOCKED LIMIT @batch - Processor->>Dapr: IEventBus.PublishAsync → DaprClient.PublishEventAsync
(topic: learnstack.{module}.{aggregate}) - Dapr->>Kafka: produce - Processor->>Outbox: UPDATE processed_at = now() + Processor->>Outbox: CLAIM — UPDATE SET locked_by, locked_until
WHERE id IN (SELECT ... FOR UPDATE SKIP LOCKED)
RETURNING *; the claim survives the COMMIT + Processor->>Bus: PublishAsync(event, partitionKey)
topic learnstack.{module}.{aggregate} + Bus->>Transport: deliver + Processor->>Outbox: UPDATE processed_at = now()
WHERE id = @id AND locked_by = @me end - Kafka->>ConsumerDapr: deliver to subscribed sidecar - ConsumerDapr->>Consumer: HTTP POST /events/{topic} - Consumer->>Inbox: IsAlreadyProcessedAsync(@event.EventId) + Transport->>Consumer: deliver to subscribed handler + Consumer->>Consumer: Restore tenant context from event.TenantId + Consumer->>Inbox: IsAlreadyProcessedAsync(event.EventId) alt Already processed Inbox-->>Consumer: skip else New event Consumer->>Consumer: Handle business logic - Consumer->>Inbox: MarkAsProcessed(@event.EventId, @event.GetType().Name) + Consumer->>Inbox: MarkAsProcessed(event.EventId, event type name) Consumer->>DB: SaveChangesAsync (business write + inbox marker, atomic) end ``` +Read as text: the producer writes the aggregate change and the outbox row in one +transaction. A processor **claims** a batch of unprocessed rows by stamping a lease on +them, and the claim outlives the claiming transaction. It publishes each claimed row and +marks it processed only if it still holds the lease. The transport delivers to the +consumer, which restores tenant context, deduplicates through its inbox, and commits the +business write and the inbox marker together. + ## Outbox table -Each module owns its own outbox table inside the module's schema (or in a shared -namespace with module-prefixed key). LearnStack uses a single shared `outbox_messages` -table with `tenant_id` for RLS isolation: +LearnStack uses a **single shared `outbox_messages` table** — not one per module — with +`tenant_id` for Row Level Security isolation. The canonical DDL, its indexes, and its RLS +policy live in exactly one place: +[Database Standards § Outbox](../standards/05-database.md). Do not copy the DDL into a +third document; the last time this table's policy was duplicated, the copy carried the +superseded two-permissive-policy shape that +[ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) corrects. -```sql -CREATE TABLE outbox_messages ( - id uuid PRIMARY KEY, - occurred_at timestamptz NOT NULL DEFAULT now(), - tenant_id uuid NOT NULL, - correlation_id text NULL, - causation_id uuid NULL, - actor_user_id uuid NULL, - type text NOT NULL, -- assembly-qualified event type name - topic text NOT NULL, -- "learnstack.identity.user" - payload jsonb NOT NULL, - metadata jsonb NULL, - processed_at timestamptz NULL, - attempts int NOT NULL DEFAULT 0, - last_error text NULL, - available_after timestamptz NOT NULL DEFAULT now() -); +Three columns exist for reasons this document owns rather than the schema, and the two +guarantees below (ordering, single-claimant dispatch) cannot be honoured without them: -CREATE INDEX ix_outbox_pending - ON outbox_messages (available_after) - WHERE processed_at IS NULL; -CREATE INDEX ix_outbox_tenant_pending - ON outbox_messages (tenant_id, available_after) - WHERE processed_at IS NULL; - -ALTER TABLE outbox_messages ENABLE ROW LEVEL SECURITY; -CREATE POLICY outbox_messages_tenant_isolation ON outbox_messages - USING (tenant_id = current_setting('app.tenant_id', true)::uuid); --- OutboxProcessor uses learnstack_outbox_admin role that bypasses RLS to read all tenants. -``` +| Column | Why it exists | Lands with | +|---|---|---| +| `partition_key text NOT NULL` | The ordering guarantee below is expressed entirely through this value. Defaults to the aggregate id; falls back to `tenant_id` when the event names no aggregate. Set at enqueue time by `IOutbox.EnqueueAsync`, never by the transport. | The table itself, in [Phase 02a Packet 6](../roadmap/phase-02a-kernel-tenancy.md) — a producer-side column is cheaper to ship with the table than to backfill | +| `locked_by text NULL` + `locked_until timestamptz NULL` | The dispatch **lease**. A processor stamps them to claim a row, and the claim survives the claiming transaction's commit. See the claim protocol below. | The dispatcher, in [Phase 02b](../roadmap/phase-02b-events-auth.md) | +| `available_after timestamptz NOT NULL` | Retry backoff. A failed dispatch pushes this forward instead of blocking the batch. | Already in the canonical DDL | + +Both additions go into the canonical DDL in Standards 05 when their phase ships; this +document does not carry a second copy of the table. + +The pending index therefore covers `(available_after)` filtered on +`processed_at IS NULL`, and the claim predicate additionally reads +`locked_until IS NULL OR locked_until < now()`. ### Why the `learnstack_outbox_admin` role bypasses RLS — and what bounds the bypass @@ -104,13 +126,25 @@ BackgroundService instance; setting `app.tenant_id` per row would defeat the `FOR UPDATE SKIP LOCKED` batching pattern. The role bypasses RLS by design, and the bypass is bounded by: -- **Scope by grant**. The role has `SELECT`, `UPDATE` only on `outbox_messages` - (status column transitions: `processed_at`, `attempts`, `last_error`, - `available_after`). It has **no** access to any other tenant-owned table; the - bypass cannot be used as a generic backdoor. -- **No connection sharing**. The role is used only by the `OutboxProcessor` - BackgroundService DI scope; no MediatR handler or API endpoint ever runs - under this role (architecture test +- **Scope by grant, and grant is the only bound.** `BYPASSRLS` bypasses *policies*, not + `GRANT`s: a role holding the attribute with no privilege on a table gets + `permission denied for table`. The role holds `SELECT` on `outbox_messages` plus a + **column-level** `UPDATE (processed_at, attempts, last_error, available_after)` — and + nothing anywhere else. It has no `DELETE` (purging processed rows is a + `learnstack_platform` operation) and cannot touch `payload`, `tenant_id`, `topic` or + `type`. `SELECT … FOR UPDATE SKIP LOCKED` works with a column-level `UPDATE` grant, so + the claim protocol needs no table-wide `UPDATE`. When `locked_by` and `locked_until` + land in [Phase 02b](../roadmap/phase-02b-events-auth.md), that migration extends the + column list; a column added without extending it fails at runtime with + `permission denied for table`, which is the intended loud failure. Because grant scope + is the *only* thing bounding the bypass, no privilege is ever granted to `PUBLIC` and + no `ALTER DEFAULT PRIVILEGES` grant exists — either would silently un-bound this role + the next time a table is created. +- **No connection sharing.** The role has its own login credential and its own + connection string (`ConnectionStrings:OutboxDispatcher`), present only in the worker + host that runs `OutboxProcessor`. It is not reached by `SET ROLE` from + `learnstack_app`, which is not a member of it. No MediatR handler or API endpoint ever + runs under this role (architecture test `LearnStack_OutboxAdmin_Role_OnlyUsedBy_OutboxProcessor` enforces this). - **Audited on use**. Every dispatch attempt produces a structured log entry with `event_id`, `tenant_id`, `event_type`, and outcome. Bypass-as-such is @@ -121,10 +155,14 @@ the bypass is bounded by: the RLS bypass is the minimum privilege escalation that makes that pattern work. It is not an exception to ADR-0003 — it is the only legitimate path. -The same shape applies to `learnstack_audit_admin` (read-side cross-tenant -audit queries, see [31-audit-subsystem.md](31-audit-subsystem.md)) and any -future `learnstack_*_admin` role: bounded by grant, isolated by DI scope, -discoverable by log. +`learnstack_outbox_admin` is one of the **four** roles in the database model fixed by +[ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) — +`learnstack_migration` (owner, `NOBYPASSRLS`), `learnstack_app` (runtime, +`NOBYPASSRLS`), `learnstack_platform` (audited cross-tenant admin, `BYPASSRLS`), and +this one. Cross-tenant audit reads use `learnstack_platform` through the audited +`EnterPlatformAdminScope(reason)` path; there is no separate audit role. Adding a fifth +role requires an ADR — every `BYPASSRLS` role is a hole in the isolation model that has +to earn its existence by grant scope, DI-scope isolation, and log discoverability. ## Inbox table (per consumer module) @@ -166,7 +204,9 @@ public async Task> Handle(CreateEnrollmentCommand cmd, Can ``` `IOutbox.EnqueueAsync` writes to the same `DbContext` (no separate transaction); commit -is atomic with the aggregate write. +is atomic with the aggregate write. It also resolves and stores the row's +`partition_key` — here `EnrollmentId`, the aggregate this event is about. See +[Ordering](#ordering). ## Consumer pattern @@ -196,13 +236,34 @@ public sealed class CreateAuditTrailOnEnrollmentCreated( ## OutboxProcessor (BackgroundService) -Continuously polls `outbox_messages` and dispatches: +### The claim problem + +`SELECT ... FOR UPDATE SKIP LOCKED` holds its row locks **only until the transaction that +took them ends**. A processor that selects a batch, commits to release the locks, and +*then* starts publishing has released every row it is about to work on. A second +processor polling one millisecond later sees those rows with `processed_at IS NULL`, is +not blocked by any lock, and dispatches the same events. The first processor then marks +them processed, and nothing in the system records that they went out twice. + +That is why this document no longer claims "no double dispatch". The delivery contract is +**at-least-once**, and consumer-side idempotency through `IInboxGuard` is what makes it +safe — not the row lock. What the claim protocol buys is that duplicates come from crash +recovery and lease expiry, which are rare and bounded, rather than from every concurrent +poll, which is neither. + +### The claim protocol + +The row lock has to survive the transaction that takes it, which means the claim must be +**written to the row**, not held in the lock table: ```csharp public sealed class OutboxProcessor : BackgroundService { private const int BatchSize = 100; private static readonly TimeSpan PollInterval = TimeSpan.FromMilliseconds(200); + private static readonly TimeSpan LeaseDuration = TimeSpan.FromSeconds(60); + + private readonly string _processorId = $"{Environment.MachineName}:{Guid.NewGuid():N}"; protected override async Task ExecuteAsync(CancellationToken ct) { @@ -220,35 +281,40 @@ public sealed class OutboxProcessor : BackgroundService { var db = services.GetRequiredService(); var eventBus = services.GetRequiredService(); - // Connection set to learnstack_outbox_admin role; RLS bypassed. + // Connection runs as learnstack_outbox_admin: BYPASSRLS skips the policy, + // and the GRANT is what bounds what it can reach (see § the bypass bounds). - await using var tx = await db.Database.BeginTransactionAsync(ct); + // 1. CLAIM. One statement, its own short transaction. The lease is written to the + // row, so it outlives the commit that releases the physical locks. var batch = await db.Set() .FromSqlInterpolated($@" - SELECT * FROM outbox_messages - WHERE processed_at IS NULL AND available_after <= now() - ORDER BY occurred_at - LIMIT {BatchSize} - FOR UPDATE SKIP LOCKED") + UPDATE outbox_messages + SET locked_by = {_processorId}, + locked_until = now() + {LeaseDuration}, + attempts = attempts + 1 + WHERE id IN ( + SELECT id FROM outbox_messages + WHERE processed_at IS NULL + AND available_after <= now() + AND (locked_until IS NULL OR locked_until < now()) + ORDER BY occurred_at + LIMIT {BatchSize} + FOR UPDATE SKIP LOCKED) + RETURNING *") .ToListAsync(ct); - if (batch.Count == 0) - { - await tx.CommitAsync(ct); - return 0; - } - - // Commit the SELECT FOR UPDATE transaction immediately to release row locks. - await tx.CommitAsync(ct); + if (batch.Count == 0) return 0; + // 2. PUBLISH. One short transaction per message. A single poisoned message does + // not roll back the batch. foreach (var msg in batch) { await using var perMessageTx = await db.Database.BeginTransactionAsync(ct); try { var eventInstance = JsonSerializer.Deserialize(msg.Payload, Type.GetType(msg.Type)!); - await eventBus.PublishAsync((IIntegrationEvent)eventInstance!, ct); - msg.MarkProcessed(DateTimeOffset.UtcNow); + await eventBus.PublishAsync((IIntegrationEvent)eventInstance!, msg.PartitionKey, ct); + msg.MarkProcessed(_clock.UtcNow, _processorId); // no-op if the lease was lost } catch (Exception ex) { @@ -264,32 +330,84 @@ public sealed class OutboxProcessor : BackgroundService } ``` -Key invariants: +`MarkProcessed` writes `processed_at` **and** clears the lease under the predicate +`locked_by = @me`, so a processor whose lease expired mid-batch — because the transport +hung past `LeaseDuration` — cannot overwrite the state of whichever processor took the +row next. A lost lease is logged at warning level and surfaces on the +`learnstack_outbox_lease_lost_total` counter; a non-zero rate means `LeaseDuration` is +shorter than the transport's tail latency and must be raised. + +### The simpler alternative, and its cost + +Holding one transaction open across the whole batch — `BEGIN`, `SELECT ... FOR UPDATE +SKIP LOCKED`, publish each, `UPDATE processed_at`, `COMMIT` — is also correct, and is an +acceptable implementation of the same contract. Its costs are real and must be accepted +knowingly: + +- The transaction stays open for the duration of every network publish in the batch, + pinning the `VACUUM` horizon and the oldest transaction id for that long. +- One poisoned message rolls back the batch unless every publish is wrapped in a + savepoint, which reintroduces most of the complexity the approach was meant to avoid. +- A crash mid-batch republishes everything already published in it. + +The lease is the specified design. The batch-held transaction is the acceptable fallback +if a deployment cannot add the two columns. Whichever ships, the choice is made once, in +[Phase 02b](../roadmap/phase-02b-events-auth.md), and written into the dispatcher — not +left to each reader of this document. + +### What the processor guarantees + +- **At-least-once delivery.** Duplicates are expected; consumers deduplicate through + `IInboxGuard`. This is the contract, not a defect. +- **At most one active claimant per row.** Two processors cannot both hold a live lease + on the same row, so concurrent polling does not multiply dispatches. Duplicates arise + only from lease expiry and crash recovery. +- **Batch independence.** Each message gets its own transaction; one failure does not + roll back the batch. +- **Bounded retry.** Failed messages retry with exponential backoff (1s, 5s, 30s, 5min, + 1h). On reaching `MaxAttempts` the row moves to the producer-side dead-letter state + described below. +- **Horizontal scalability.** `SKIP LOCKED` plus the lease predicate lets N processors + across N pods drain the same table without coordination. + +## `IEventBus` and the partition key + +The port takes the partition key explicitly. It is not derived inside the transport, +because the transport is the one component that does not know what the event's ordering +domain is: -- `FOR UPDATE SKIP LOCKED` allows horizontal scaling (multiple OutboxProcessor instances - across pods can run concurrently without double-dispatch). -- The `SELECT FOR UPDATE` transaction commits immediately to release row locks. -- Each message gets its own transaction; one failure doesn't roll back the batch. -- Failed messages are retried with exponential backoff (1s, 5s, 30s, 5min, 1h); - permanent failures (max retries reached) move to dead-letter (manual intervention). +```csharp +public interface IEventBus +{ + Task PublishAsync(TEvent @event, string partitionKey, CancellationToken ct = default) + where TEvent : IIntegrationEvent; +} +``` -## `IEventBus` → Dapr pub/sub → Kafka +The durable implementation forwards it as Dapr's `partitionKey` metadata, which the Kafka +pub/sub component maps onto the Kafka message key: ```csharp public sealed class DaprEventBus(DaprClient daprClient) : IEventBus { - public Task PublishAsync(TEvent @event, CancellationToken ct = default) + public Task PublishAsync(TEvent @event, string partitionKey, CancellationToken ct = default) where TEvent : IIntegrationEvent - { - var topic = ConventionTopicName(@event); // "learnstack.{module}.{aggregate}" - return daprClient.PublishEventAsync("pubsub", topic, @event, ct); - } + => daprClient.PublishEventAsync( + "pubsub", + ConventionTopicName(@event), // "learnstack.{module}.{aggregate}" + @event, + new Dictionary { ["partitionKey"] = partitionKey }, + ct); private static string ConventionTopicName(IIntegrationEvent @event) => $"learnstack.{ExtractModule(@event.GetType())}.{ExtractAggregate(@event.GetType())}"; } ``` +`InProcessEventBus` honours the same key by serialising dispatch per partition key — +concurrent across keys, sequential within one — so a consumer cannot observe an ordering +in development that the durable transport would not also produce. + Topic naming convention: `learnstack.{module}.{aggregate}`. Examples: - `learnstack.identity.user` @@ -300,22 +418,46 @@ Topic naming convention: `learnstack.{module}.{aggregate}`. Examples: - `learnstack.hub.entitlement` (Hub-side) - `learnstack.cache.invalidation` (cross-instance L1 cache) -## Development mode +## `InProcessEventBus` + +Until the Dapr adapter's trigger fires, `InProcessEventBus` is the only registered +`IEventBus`. It is a **transport**, not a shortcut, and it carries the same four +obligations as the durable path: -When `DeploymentMode.Development` and Dapr sidecar isn't running, `InProcessEventBus` -replaces `DaprEventBus`: +| Obligation | Why it cannot be skipped in development | +|---|---| +| Same handler interface — `IIntegrationEventHandler`, never a bare `INotificationHandler` | Two interfaces means two implementations per consumer, and the one that runs in CI is not the one that runs in production | +| Same `IInboxGuard` deduplication | Deduplication bugs are the most common integration-event defect; a transport that never delivers a duplicate never surfaces them | +| Same tenant-context restoration from `@event.TenantId` before the handler runs | A dev path that skips it is a dev path where Row Level Security and the query filters are never exercised on the consumer side | +| Same per-partition-key ordering | Ordering assumptions that hold only in process are discovered in production | ```csharp -public sealed class InProcessEventBus(IPublisher publisher) : IEventBus +public sealed class InProcessEventBus( + IServiceScopeFactory scopeFactory, + ITenantContextAccessor tenantAccessor, + IPartitionSerializer partitions) : IEventBus { - public Task PublishAsync(TEvent @event, CancellationToken ct = default) + public Task PublishAsync(TEvent @event, string partitionKey, CancellationToken ct = default) where TEvent : IIntegrationEvent - => publisher.Publish(@event, ct); + => partitions.RunSequentiallyFor(partitionKey, async () => + { + await using var scope = scopeFactory.CreateAsyncScope(); + tenantAccessor.Set(TenantContext.FromEvent(@event)); // same restore as the durable path + foreach (var handler in scope.ServiceProvider.GetServices>()) + await handler.HandleAsync(@event, ct); // handler calls IInboxGuard itself + }); } ``` -Module subscribers (`INotificationHandler` for dev / `IIntegrationEventHandler` -for prod) handle the event. The composition root chooses based on `DeploymentMode`. +What the in-process transport genuinely does **not** provide, and what therefore +constitutes the trigger for the Dapr adapter: delivery to a second process, broker-side +replay, and durability of the in-flight message if the process dies between publish and +handle. The outbox row covers the third — an unhandled event has not been marked +processed and is redelivered — which is precisely why the outbox is LearnStack-owned and +ships before any transport. + +The composition root selects the implementation by `DeploymentMode`; modules never see +the choice. ## Rules @@ -325,25 +467,103 @@ for prod) handle the event. The composition root chooses based on `DeploymentMod - Consumers are **idempotent** via `IInboxGuard` (per-module inbox table). - Mandatory metadata on every integration event: `EventId`, `TenantId`, `OccurredAt`, `CorrelationId`. Optional but recommended: `CausationId`, `ActorUserId`. -- **Tenant context** restored on the consumer side before any business logic runs - (Dapr passes the event payload; consumer middleware sets `accessor.SetTenant(...)` - from `@event.TenantId` before invoking the handler). -- Ordering is **not** guaranteed across topics; **per-partition** ordering within a Kafka - topic is preserved by partition-keying on the aggregate id when ordering is required - (rare). -- Failed dispatches **retry with exponential backoff**; permanent failures move to a - dead-letter state (manual review via the `OutboxStatusEndpoints` admin API). +- **Tenant context** restored on the consumer side before any business logic runs. The + transport delivers the event payload; the consumer scope sets the ambient context from + `@event.TenantId` before invoking the handler, so the handler's queries carry the same + filters and the same `app.tenant_id` as an HTTP-originated command. + +## Ordering + +Ordering is **not** guaranteed across topics, and never has been. Within one topic, +ordering is guaranteed **per partition key** — and a partition key that nobody sets is a +guarantee nobody has. + +The rule, therefore, is that **every outbox row carries a non-null `partition_key`**: + +| Event shape | Partition key | +|---|---| +| Event about one aggregate instance (the normal case) | The aggregate id — `EnrollmentCreatedIntegrationEvent` keys on `EnrollmentId` | +| Event about the tenant as a whole (`TenantSuspended`, `EntitlementUpdated`) | `TenantId` | +| Event with an explicit ordering domain that is neither (rare) | Declared by the event type through `IPartitionedIntegrationEvent.PartitionKey` | + +`IOutbox.EnqueueAsync` resolves the key at enqueue time and writes it to the row; the +processor reads it from the row and passes it to `IEventBus.PublishAsync`. Nothing +downstream re-derives it, so the ordering domain is decided once, by the producer that +knows it. + +Consequences worth stating plainly: + +- Two events about the **same** aggregate arrive in publish order at every consumer. +- Two events about **different** aggregates may arrive in any relative order, even inside + one topic. A consumer that needs a cross-aggregate order needs a different design — + usually a read model, not an ordering assumption. +- Keying on `TenantId` for high-volume events serialises that tenant's whole stream onto + one partition. That is a deliberate throughput cost, paid only where the ordering is + genuinely tenant-wide. +- The column is `NOT NULL`, so the database asserts it. The catalogued + `Outbox_Row_Carries_Correlation_Context` integration test extends its assertion to + `partition_key` when the column lands, so a new enqueue path that forgets it fails in + CI rather than in a consumer. + +## Dead-letter: two sides, two failure domains + +The distinction the earlier draft of this document missed: a producer-side dispatch +success means "the transport accepted the bytes". It says nothing about whether any +consumer processed them. The two sides fail independently and need separate handling. + +### Producer side — dispatch never succeeded + +The outbox row reaches `MaxAttempts` without a successful `PublishAsync`. + +- The row stays in the table with `processed_at IS NULL`, `attempts >= MaxAttempts`, and + the final `last_error`. +- `available_after` is pushed beyond the retry horizon so the poller stops picking it up. +- It surfaces on the `OutboxStatusEndpoints` admin API and on the + `learnstack_outbox_deadletter_total{event_type}` counter; the alert is on the counter's + rate, not on the table's size. +- Recovery is a manual replay: an operator clears the lease, resets `attempts`, and + resets `available_after`. + +### Subscriber side — dispatch succeeded, handling did not + +A consumer's handler throws on every attempt: a malformed payload, a schema version it +cannot read, a permanently failing downstream dependency. The producer's row is long +since `processed_at`-stamped, and no amount of producer-side retry will help. + +- The transport retries the handler with backoff. On exhausting its retry policy it + routes the message to a **dead-letter topic**, `learnstack.dlq.{module}` — one per + subscribing module, three segments, satisfying + `Dapr_PubSub_TopicNames_FollowConvention`. Under Dapr this is the subscription's + `deadLetterTopic`; under `InProcessEventBus` the equivalent is a row in + `dead_letter_messages` carrying the same envelope. +- **The inbox row is not marked processed.** A dead-lettered message has not been + handled, and recording it as handled would make a later replay a silent no-op. +- Every module subscribes to its own dead-letter topic with a handler that does nothing + but persist the envelope, emit `learnstack_inbox_deadletter_total{module, event_type}`, + and raise an operational alert. A dead-letter topic nobody consumes is a queue that + grows until the broker's retention silently deletes the evidence. +- Recovery is an operator-initiated replay from the dead-letter store back onto the + original topic. Because the inbox never marked the event processed, the replayed + message is handled normally; because `IInboxGuard` is still in the path, a replay that + races a late success is still idempotent. +- Poison-message containment is per subscription. One module's dead-lettered event does + not block another module's consumption of the same topic — each subscription has its + own offset and its own dead-letter path. ## Service extraction readiness The outbox + `IEventBus` is the future service boundary. If a module is promoted to a separate service later: -- The module's integration events already cross a real broker (Kafka). -- The consumer-side subscription pattern (Dapr `[Topic]` attribute or programmatic - subscription) works identically across in-process and cross-service. -- The producer-side outbox is durable; service extraction doesn't require dual-write - semantics. +- The producer-side outbox is durable and LearnStack-owned; service extraction does not + require dual-write semantics. +- The consumer-side contract — `IIntegrationEventHandler`, `IInboxGuard`, tenant-context + restoration, partition key — is identical under both transports, so extraction changes + which process runs the handler, not how the handler is written. +- Swapping `InProcessEventBus` for `DaprEventBus` is a composition-root change. That is + the whole reason the transport is demand-gated: extraction gets no cheaper by having + built the adapter early, and the platform pays the operational cost of a broker every + day until then. ## Architecture tests @@ -360,23 +580,46 @@ separate service later: LearnStack-core topics remain 3-segment (`learnstack.{module}.{aggregate}`). - `OutboxProcessor_NeverBlocks_OnSingleMessageFailure` — integration test asserts one poisoned message doesn't prevent others in the batch from processing. +- `Integration_Event_Handler_Restores_Tenant_Context` — catalogued in + [Architecture Tests Catalogue](../standards/21-architecture-tests-catalogue.md); runs + against whichever transport is registered, so `InProcessEventBus` is held to the same + restoration contract as the durable path. +- `Outbox_Row_Carries_Correlation_Context` — extended to assert a non-null + `partition_key`. +- Concurrent-claim integration test: two `OutboxProcessor` instances draining one table + produce **one** claim per row while both leases are live. This is the test that would + have caught the released-lock defect, and it needs two processors — a single-processor + test passes against the broken protocol. ## Observability -- Outbox lag metric: `learnstack_outbox_pending_count{tenant_id}` (gauge). +- Outbox lag: `learnstack_outbox_pending_count{tenant_id}` (gauge). - Dispatch duration: `learnstack_outbox_dispatch_duration_seconds{event_type}` (histogram). - Dispatch failures: `learnstack_outbox_dispatch_failed_total{event_type}` (counter). -- Inbox dedup count: `learnstack_inbox_dedup_total{module, event_type}` (counter). - -Grafana dashboard surfaces these alongside Dapr's own pub/sub metrics -(`dapr_component_pubsub_*`). +- Producer-side dead letter: `learnstack_outbox_deadletter_total{event_type}` (counter). +- Lost leases: `learnstack_outbox_lease_lost_total` (counter) — non-zero means the lease + is shorter than the transport's tail latency. +- Inbox dedup: `learnstack_inbox_dedup_total{module, event_type}` (counter). +- Subscriber-side dead letter: `learnstack_inbox_deadletter_total{module, event_type}` + (counter). + +The two dead-letter counters are **separate signals on separate dashboards**. A +producer-side spike means the transport is down; a subscriber-side spike means a handler +or a payload is broken. Collapsing them into one "events failed" number destroys the only +information that distinguishes the two incidents. Once the Dapr adapter lands, both sit +alongside Dapr's own `dapr_component_pubsub_*` metrics. ## References -- ADR-0006 (Amendment 1) — Events and Outbox; Dapr pub/sub dispatch transport. +- ADR-0006 (Amendment 1) — Events and Outbox; dispatch transport. - ADR-0010 (Amendment 1) — Cross-Module Communication; outbox dispatch target. -- ADR-0014 — Adopt Dapr. -- ADR-0016 — Audit Log Subsystem. +- ADR-0014 — Adopt Dapr (what LearnStack uses for cross-process pub/sub). +- [ADR-0033](../decisions/0033-audit-durability-model.md) — Audit durability model; + audit fan-out to external sinks rides this outbox, MUST-class audit does not. +- [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) — Demand-gated + infrastructure; when the Dapr and Kafka adapters arrive. +- [Database Standards § Outbox](../standards/05-database.md) — the canonical + `outbox_messages` DDL and RLS policy. - [29-dapr-integration.md](29-dapr-integration.md) — Dapr deep dive. - [10-cross-module-contracts.md](10-cross-module-contracts.md) — the four sanctioned cross-module mechanisms. diff --git a/docs/architecture/20-search.md b/docs/architecture/20-search.md index e5c963e..e053d0b 100644 --- a/docs/architecture/20-search.md +++ b/docs/architecture/20-search.md @@ -1,5 +1,16 @@ # Search +> **Read this first.** Meilisearch is **demand-gated** +> ([ADR-0035](../decisions/0035-demand-gated-infrastructure.md)). The `ITenantSearch` +> port and a **PostgreSQL full-text** implementation ship in +> [Phase 04](../roadmap/phase-04-cms-media-pages.md), which is the first phase with +> content to index; the Meilisearch adapter lands in +> [Phase 09](../roadmap/phase-09-billing-integrations-analytics.md) when its trigger +> fires — search quality or scale exceeds PostgreSQL full-text. Everything below +> describes the Meilisearch target, not the running system. The tenant-filter rule is +> the part that applies to **both** implementations: the predicate is composed inside +> the port, never by a caller. + LearnStack uses **Meilisearch** for tenant-scoped full-text search across courses, content entries, media metadata, and lesson item titles. This document defines index layout, tenant + locale isolation, indexing pipeline, query path, and operational rules. The strategic decision lives in [ADR 0012: Search Strategy](../decisions/0012-search-strategy.md). This document is the implementation reference. diff --git a/docs/architecture/21-feature-flags.md b/docs/architecture/21-feature-flags.md index 8392919..7da7e4f 100644 --- a/docs/architecture/21-feature-flags.md +++ b/docs/architecture/21-feature-flags.md @@ -117,26 +117,47 @@ CREATE TABLE tenant_feature_flags ( ); -- Hub-projected entitlement cache (plan-level features + limits + compliance caps). --- One row per tenant; replaced on `learnstack.hub.entitlement` Dapr pub/sub event. +-- One row per tenant. Despite the name this is a DURABLE projection store, not a +-- cache: it is the layer that makes the grace window real. Written only by +-- `IEntitlementProvider.RefreshAsync`, from PUT /api/internal/tenants/{id}/entitlements +-- or from a signed licence key. The `learnstack.hub.entitlement` event is the eager +-- INVALIDATION signal for the L1/L2 caches in front of it, not the write path. CREATE TABLE platform_entitlement_cache ( tenant_id uuid PRIMARY KEY, - plan_code text NOT NULL, - features jsonb NOT NULL, -- Dictionary - limits jsonb NOT NULL, -- Dictionary - compliance jsonb NOT NULL, -- caps, regions, retention overrides - valid_until timestamptz NOT NULL, + plan_code text NOT NULL, -- wire field `tier` + features jsonb NOT NULL, -- Dictionary + limits jsonb NOT NULL, -- Dictionary + compliance jsonb NOT NULL, -- caps, regions, retention overrides + valid_until timestamptz NOT NULL, -- wire field `expires_at` + grace_until timestamptz NULL, -- null unless in grace; bounds the window + generation bigint NOT NULL DEFAULT 1, -- monotonic; a push is accepted only + -- when received.generation >= stored refreshed_at timestamptz NOT NULL DEFAULT now(), - source text NOT NULL -- 'hub' | 'signed-license-key' | 'null-provider' + source text NOT NULL -- 'hub' | 'signed-license-key' | 'null-provider' ); ``` +The wire shape and the column names differ in two places, and the mapping is normative: +the projection field `tier` persists to `plan_code`, and `expires_at` persists to +`valid_until`. `grace_until` and `generation` keep their wire names. `grace_until` is +nullable — a tenant not in grace has none, and the Hub sends null. `generation` defaults +to 1 so the tenant-provisioning insert at `POST /api/internal/tenants` needs no special +case; every later push must carry a value greater than or equal to the stored one, which +is what makes an out-of-order or replayed push a no-op rather than a downgrade. The wire +shape is pinned by `entitlement-v1.schema.json` in both repositories; the Hub-side +rendering is in the `learnstack-hub` repository's +`docs/architecture/entitlement-projection.md`. + Rules: - `tenant_id = NULL` is **not** allowed. Platform-wide flags use a sentinel "platform" tenant id, never `NULL`. - The Hub is the **owner** of `platform_entitlement_cache`; the LearnStack core only - reads + invalidates. Writes happen through `IEntitlementProvider.RefreshAsync` on - the inbound Dapr event from Hub. See + reads + invalidates. Writes happen through `IEntitlementProvider.RefreshAsync` + only, driven by the HTTP push (`PUT /api/internal/tenants/{id}/entitlements`) or by a + signed licence key. The inbound `learnstack.hub.entitlement` event invalidates the + L1 / L2 layers; it is not the write path + ([ADR-0034](../decisions/0034-hub-contract-surface-invariant.md)). See [ADR-0021](../decisions/0021-feature-based-entitlement.md) and [29-dapr-integration.md](29-dapr-integration.md). - A short-TTL Valkey cache (60 s) fronts both tables for hot-path reads. Eager @@ -278,7 +299,7 @@ Both surfaces are MUST-audit security-events (see - **Phase 06** — Admin Studio surface for editing per-tenant flag overrides and viewing the entitlement projection. The Studio screen for `platform_entitlement_cache` is **read-only** — actual plan edits happen in the operator portal - (`learnstack-hub-web`). + (`operator-portal`). - **Phase 09** — Audit + observability hooks for both flag writes and entitlement refreshes plug into the audit + analytics pipeline. - **Phase 11** — Quarterly hygiene review and CI surfacing of stale flags become diff --git a/docs/architecture/23-data-protection.md b/docs/architecture/23-data-protection.md index e9036d3..fc3a66a 100644 --- a/docs/architecture/23-data-protection.md +++ b/docs/architecture/23-data-protection.md @@ -56,13 +56,32 @@ Per category: - Payment → retained for legal period; user notified of the exception. - Audit → actor field anonymised; action record retained. -Deletion is a workflow, not a single SQL statement: - -1. Tenant admin (or user via self-service post-MVP) initiates the request. -2. Platform validates eligibility (no active payment dispute, no legal hold). -3. `UserAnonymisationRequestedV1` event published. -4. Each module consumes the event and performs its part: anonymise rows, delete storage objects, invalidate Keycloak user. -5. A final reconciliation job confirms every module reported completion within 30 days. Failures escalate to platform admin. +Deletion is a workflow, not a single SQL statement — and there are **two** of them, +separated by authority. Conflating them lets a tenant admin close a person's account +([Phase 03 § Tenant Data Ownership](../roadmap/phase-03-identity-admin.md)). + +**Tenant-scoped erasure.** Initiated by a tenant admin, or by the person acting inside +that tenant. + +1. Eligibility is validated (no active payment dispute, no legal hold). +2. The `Membership` and its `MembershipProfile` are removed. +3. That tenant's behaviour rows are anonymised; that tenant's audit entries keep the + action record with the actor field anonymised. +4. The `users` row survives for as long as any membership remains anywhere. Nothing in + this flow touches Keycloak. + +**Global account closure.** A **platform-scoped** operation. There is no tenant-admin +endpoint for it. + +1. The platform admin enters scope through the audited `EnterPlatformAdminScope(reason)` + — the only path that reads across tenants. +2. Eligibility is validated (no active payment dispute, no legal hold). +3. `UserAnonymisationRequestedV1` fans out to every tenant the person belongs to. +4. Each module consumes the event and performs its part: anonymise rows, delete storage + objects. **The Keycloak user is invalidated once, at the end, by the platform-scoped + step — never by a module reacting to a tenant-scoped request.** +5. A final reconciliation job confirms every module reported completion within 30 days. + Failures escalate to platform admin. ### Right to Restriction of Processing @@ -145,9 +164,10 @@ not residency-bound. Two flows legitimately cross regions: - **LearnStack ↔ Hub** internal API. Hub may live in a single region (e.g. eu-west) - while serving tenants in multiple regions. The four-endpoint contract is small, - audit-friendly, and carries no tenant content — only plan / entitlement / license - metadata. Tenants whose plan forbids cross-region operator access are served by a + while serving tenants in multiple regions. The contract surface is small, + audit-friendly, and — by the first invariant of + [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md) — carries no tenant + content, only plan / entitlement / licence metadata. Tenants whose plan forbids cross-region operator access are served by a region-local Hub instance (Hub federation is a Phase-11+ topic, not MVP). - **Telemetry → centralised OpenTelemetry collector**. Trace and metric data can be shipped cross-region for unified observability; PII redaction in the pipeline keeps diff --git a/docs/architecture/24-learnstack-hub.md b/docs/architecture/24-learnstack-hub.md index 16f90aa..922b38a 100644 --- a/docs/architecture/24-learnstack-hub.md +++ b/docs/architecture/24-learnstack-hub.md @@ -27,7 +27,7 @@ flowchart TB subgraph Hub["learnstack-hub (separate repo)"] HubApi["LearnStack.Hub.Api"] HubDb[("Hub Postgres
(tenant metadata only,
no tenant content)")] - HubWeb["learnstack-hub-web
(operator portal Next.js)"] + HubWeb["operator-portal
(operator portal Next.js)"] end subgraph External @@ -45,7 +45,7 @@ flowchart TB HubApi --> Vault HubApi -- "mTLS + signed JWT + HMAC
POST /api/internal/*
(tenant lifecycle, entitlement push)" --> LSApi - LSApi -- "API key
(license verify, usage report)
POST /api/v1/internal/*" --> HubApi + LSApi -- "mTLS + signed JWT + HMAC
(license verify, usage report)
POST /api/v1/internal/*" --> HubApi LSApi --> LSDb LSDb -. "RLS isolated; Hub NEVER queries" .- HubApi @@ -182,43 +182,59 @@ erDiagram ## 3. Communication contracts -### Contract surface — "closed at four" - -The **closed Hub HTTPS contract surface** is the set of endpoints that the -`LearnStack_Modules_DoNotReference_Hub` architecture test and the "no fifth without -ADR" rule (per [ADR-0019](../decisions/0019-learnstack-hub.md) and -[20-infrastructure-stack.md § Hub HTTPS Contract Surface](../standards/20-infrastructure-stack.md)) -guard. Those four endpoints are the **load-bearing entitlement + lifecycle + telemetry -contract**: - -| # | Direction | Method | Path | Purpose | -|---|-----------|--------|------|---------| -| 1 | Hub → LS | POST | `/api/internal/tenants` | Create tenant + default organization | -| 2 | Hub → LS | PUT | `/api/internal/tenants/{id}/entitlements` | Push updated entitlement projection (status, features, limits, compliance.caps) | -| 3 | LS → Hub | POST | `/api/v1/internal/license/verify` | Pull / verify entitlement (called by `HubEntitlementProvider`) | -| 4 | LS → Hub | POST | `/api/v1/usage/report` | Report usage metric (idempotent; Hub aggregates per period) | - -Adding a fifth endpoint to this closed set requires a new ADR. - -### Auxiliary lifecycle endpoints (within the closed surface) - -The operational endpoints below are **specializations of endpoint #2** in the closed -set — they share the same `/api/internal/tenants/{id}` resource, the same -mTLS + signed JWT + HMAC chain, the same `Hub → LearnStack` direction, and the same -authorization scope. They are listed separately for clarity but do not count as new -contract entries: - -| Method | Path | Purpose | Belongs to closed-set # | -|--------|------|---------|--------------------------| -| PUT | `/api/internal/tenants/{id}/status` | Suspend / activate / archive (status field of entitlement projection) | #2 | -| GET | `/api/internal/tenants/{id}/usage` | Pull aggregated usage metrics (Hub-side mirror of #4 reports) | #4 | -| DELETE | `/api/internal/tenants/{id}` | Terminate (hard delete with confirmation) | #1 (inverse) | -| POST | `/api/v1/internal/license/refresh` | Phone-home refresh used by `SelfHostedOnline` mode — a scheduled call shape of #3 | #3 | - -If any of these auxiliary endpoints needs a contract or auth model that **differs** -from its parent (different role, different rate limit, different payload shape that -isn't a subset of the parent), it must be promoted to a new ADR-registered closed-set -entry. +### Contract surface — two invariants, not a count + +Per [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md), the Hub contract +surface is governed by two properties rather than by an endpoint count: + +1. **The Hub stores no tenant content.** Courses, lessons, learners, enrollments, + classroom sessions, media and content entries live exclusively in LearnStack. The + Hub holds tenant *metadata*: plan, subscription, licence, custom domain, compliance + caps, aggregated usage. Enforced by `Hub_NeverStores_TenantData`. +2. **Every crossing goes through a named adapter** — `IEntitlementProvider`, + `IUsageReporter`, `IHubTenantSync`. No other type holds a Hub client, and nothing + resolves a host by calling the Hub. Enforced by + `LearnStack_Modules_DoNotReference_Hub` and + `Hub_Client_Referenced_Only_By_Named_Adapters`. + +This section previously declared the surface "closed at four" and then listed four +further endpoints as "specializations" that did not count. An HTTP endpoint is a path +plus a method; `DELETE /api/internal/tenants/{id}` is not `POST /api/internal/tenants` +because one is the inverse of the other. Worse, the pressure to keep the count at four +is what drove [ADR-0022](../decisions/0022-custom-domain-tls.md) Amendment 1 to tunnel +TLS private keys through the entitlement payload. The invariants above are what anyone +actually needed the count to stand for. + +**Hub → LearnStack** (`/api/internal/*`, internal listener only): + +| Method | Path | Purpose | +|--------|------|---------| +| `POST` | `/api/internal/tenants` | Create tenant + default organization | +| `PUT` | `/api/internal/tenants/{id}/entitlements` | Push the entitlement projection | +| `PUT` | `/api/internal/tenants/{id}/status` | Suspend / activate / archive | +| `DELETE` | `/api/internal/tenants/{id}` | Terminate | +| `GET` | `/api/internal/tenants/{id}/usage` | Pull aggregated usage | +| `PUT` | `/api/internal/tenants/{id}/host-mappings` | Push host → `(tenant_id, organization_id?)` mappings. Carries the tuple only — certificate material moves by secret-store replication and is referenced by path | + +**LearnStack → Hub:** + +| Method | Path | Purpose | +|--------|------|---------| +| `POST` | `/api/v1/internal/license/verify` | Verify / pull the entitlement projection | +| `POST` | `/api/v1/internal/license/refresh` | Scheduled phone-home refresh | +| `POST` | `/api/v1/usage/report` | Report a usage metric (idempotent) | +| `POST` | `/api/v1/internal/tenants/{id}/custom-domains` | Submit a custom domain on behalf of a tenant admin (proxied through `IHubTenantSync`; the Admin Studio never calls the Hub directly, because a `learnstack` realm token is rejected there) | + +Every one of these carries the same auth chain: mTLS with LearnStack-internal CA-signed +client certificates, an RS256 JWT with `aud=learnstack-internal` and `exp ≤ 5min` +replay-protected on `jti`, and an HMAC-SHA256 body signature in `X-Signature`. + +Adding an endpoint still requires an ADR — not because the count is sacred, but because +the surface is a cross-repository contract and both repositories have to agree. + +The Hub's own tenant-facing and operator-facing APIs (`/api/v1/tenants/*`, +`/api/v1/subscriptions/*`, `/api/v1/webhooks/*`) are **not** part of this surface; they +are the Hub's public API, governed by the Hub repository. ### Authentication chain (applies to every endpoint above) @@ -229,9 +245,15 @@ entry. - Bound to internal listener; **never** proxied through APISIX **LearnStack → Hub `/api/v1/internal/*` and `/api/v1/usage/*`:** -- API key per LearnStack instance, stored in Vault (`learnstack/hub/api-key`) -- Rate limit: 100 req/min per key -- Scope strictly limited to license verification + usage reporting +- The same three layers: mTLS client cert, RS256 JWT (`aud=learnstack-internal`, + `exp ≤ 5 min`, `jti` replay-protected), HMAC-SHA256 body signature +- Scope strictly limited to licence verification, phone-home refresh, usage reporting, and + proxied custom-domain submission — the set enumerated in + [ADR-0034 § The endpoint set](../decisions/0034-hub-contract-surface-invariant.md) +- The per-instance API key from [ADR-0019](../decisions/0019-learnstack-hub.md) is + superseded by [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md); rate + limiting for this direction is enforced by the Hub's own gateway, not by the + credential ### 3.3. Hub-internal: Stripe / Iyzico webhooks @@ -335,7 +357,7 @@ sequenceDiagram HubAPI->>HubAPI: Create LearnStackTenant + HubSubscription + Entitlement (gen=1) HubAPI->>Keycloak: Provision tenant in `learnstack` realm
(create tenant_admin user, set tenant_id claim) HubAPI->>LSApi: POST /api/internal/tenants {tenant_id, slug, default_org}
(mTLS + signed JWT + HMAC) - LSApi->>LSDB: INSERT tenants, organizations (default), platform_entitlement_cache + LSApi->>LSDB: INSERT tenants, organizations (default),
platform_host_to_tenant ({slug}.learnstack.app), platform_entitlement_cache LSApi-->>HubAPI: 201 Created HubAPI->>HubAPI: Schedule welcome email HubAPI-->>HubUI: Tenant ready, redirect to {slug}.learnstack.app @@ -379,7 +401,7 @@ sequenceDiagram alt Cache fresh (<15m) Cache-->>LSApi: Entitlement (gen=42) else Cache stale or miss - LSApi->>HubAPI: POST /api/v1/internal/license/verify
(API key, tenant_id, feature_key) + LSApi->>HubAPI: POST /api/v1/internal/license/verify
(mTLS + JWT + HMAC; tenant_id, feature_key) HubAPI-->>LSApi: Entitlement (gen=42) LSApi->>Cache: UPSERT end @@ -433,7 +455,7 @@ sequenceDiagram ## 6. Operator portal (frontend) -`learnstack-hub-web` is a separate Next.js 16 app deployed at `hub.learnstack.dev`. +`operator-portal` is a separate Next.js 16 app deployed at `hub.learnstack.dev`. Authenticates against `learnstack-hub` Keycloak realm. Operators see: ``` @@ -544,7 +566,7 @@ In Self-Hosted (especially air-gapped) deployments, Hub may not exist at all; th Hub's `LearnStack.Hub.Architecture.Tests` runs: -1. `Hub_NeverStores_TenantContent` — Hub DbContext does not contain `Course`, `Lesson`, +1. `Hub_NeverStores_TenantData` — Hub DbContext does not contain `Course`, `Lesson`, `User`, `Enrollment`, `LiveSession`, `LessonItem` entities (or any LearnStack core aggregate). Migration scan asserts the absence. 2. `Hub_Modules_DoNotReference_LearnStack_Internals` — Hub modules reference only @@ -556,7 +578,15 @@ Hub's `LearnStack.Hub.Architecture.Tests` runs: `LearnStack.Hub.Modules.Subscriptions.Infrastructure.Stripe`. 5. `Iyzico_SDK_Types_NotImportedOutsideInfrastructure` — same for `Iyzipay.*`. 6. `Hub_Operator_JWT_NeverAccepted_On_LearnStack_Routes` — integration test asserts a - `learnstack-hub` realm JWT is rejected by any LearnStack tenant-facing endpoint. + `learnstack-hub` realm JWT is rejected by any LearnStack tenant-facing endpoint, and a + `learnstack` realm token by `/api/internal/*`. +7. `Hub_Client_Referenced_Only_By_Named_Adapters` — the second invariant of + [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md): a Hub client type is + referenced only from `IEntitlementProvider` / `IUsageReporter` / `IHubTenantSync` + implementations, never from a module assembly. + +`LearnStack_Modules_DoNotReference_Hub` is the mirror of test 2 and is owned and run by +the **LearnStack** repository, which is why it does not appear in this list. ## 11. Phasing diff --git a/docs/architecture/25-deployment-models.md b/docs/architecture/25-deployment-models.md index 7a9c7c8..d689025 100644 --- a/docs/architecture/25-deployment-models.md +++ b/docs/architecture/25-deployment-models.md @@ -4,8 +4,19 @@ [ADR-0019](../decisions/0019-learnstack-hub.md). LearnStack supports three deployment models from **one codebase, one Helm chart, one set -of container images**. The differentiator across modes is configuration + Dapr component -YAML + `IEntitlementProvider` implementation choice — never application code. +of container images**. The differentiator across modes is configuration + component +wiring + `IEntitlementProvider` implementation choice — never application code. + +> **Support state (2026-08-08).** The `DeploymentMode` enum has five values and the +> composition root branches on all five. Only **`Development`** and **`SaaS`** are wired +> and tested end to end today. `Dedicated`, `SelfHostedOnline`, and +> `SelfHostedAirGapped` are **prepared seams, not supported deployments**, until +> [Phase 11](../roadmap/phase-11-production-hardening.md) builds their adapters and +> integration suites, per +> [ADR-0035](../decisions/0035-demand-gated-infrastructure.md). See +> [§ 5](#5-deploymentmode-configuration) for what "prepared seam" means concretely. The +> rest of this document describes the target topologies; it is a design document for all +> five, and a description of running systems for two. ## 1. The three modes @@ -83,8 +94,29 @@ Internet time: < 1 minute. - **Tenant URL**: Starts on `{slug}.learnstack.app`; upgrades to custom domain on Growth+ tier. -- **Failure isolation**: RLS protects tenant rows; one tenant's burst doesn't starve - others (rate limits per remote_addr; Phase 11+ per-tenant consumer keys). +- **Failure isolation**: two different problems, and only one of them is solved today. + **Correctness** isolation — no tenant can read or write another tenant's rows — is + covered by tenant context + EF query filters + Row Level Security + architecture tests + ([ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md)). + **Resource** isolation — no tenant can starve another of connections, CPU, or job + workers — is **not** covered by any of those. Row Level Security is a visibility + predicate: it filters rows, and a filtered query consumes exactly as much of the + database as an unfiltered one. A single tenant running an unbounded report can hold a + connection, saturate the pool, and degrade every other tenant on the instance while + every isolation test stays green. + + What exists today: APISIX `limit-req` keyed on `remote_addr`, which throttles a noisy + client but not a noisy tenant — one tenant behind many IPs is unaffected, and many + tenants behind one NAT are punished together. + + What is required, and where it lives: **resource fairness is + [Phase 11](../roadmap/phase-11-production-hardening.md)** — `statement_timeout` per + connection class, per-tenant connection-pool partitioning, query cost ceilings on + report and export paths, Hangfire queue fairness so one tenant's bulk import cannot + monopolise the workers, and per-tenant rate limiting keyed on the resolved tenant + rather than on the client address. No earlier phase owns this work, and until Phase 11 + ships it, SaaS multi-tenancy is protected against **leakage** but not against + **contention**. ## 3. Dedicated mode @@ -188,17 +220,46 @@ public enum DeploymentMode } ``` -`Program.cs` wires the entitlement provider based on this value (see ADR-0020). +`Program.cs` wires the entitlement provider based on this value (see ADR-0020). Modules +never read the enum — `Modules_Do_Not_Reference_DeploymentMode` enforces that the +composition root branches once. -Other config settings switched by mode: +### Supported today versus prepared seam + +| Value | State | What that means concretely | +|---|---|---| +| `Development` | **Supported** | Wired, run daily, covered by the integration suite | +| `SaaS` | **Supported** | Wired and covered end to end from [Phase 02c](../roadmap/phase-02c-hub-foundation.md) | +| `Dedicated` | Prepared seam | The branch exists and resolves to the default implementations; no dedicated-topology integration suite, no operational runbook | +| `SelfHostedOnline` | Prepared seam | Same; the phone-home path needs the Hub adapter and its failure-mode tests | +| `SelfHostedAirGapped` | Prepared seam | Same, plus a signed-licence provider and a no-egress telemetry target that do not exist yet | + +A prepared seam is a **branch point with no adapter behind it**: the value is accepted, +the composition root routes it, and the implementations it selects are the same defaults +`Development` gets. It is honest to design for it; it is not honest to sell it. Each seam +becomes a supported mode in [Phase 11](../roadmap/phase-11-production-hardening.md) when +its trigger fires — for `SelfHostedAirGapped`, that trigger is a signed Self-Hosted +contract ([ADR-0035](../decisions/0035-demand-gated-infrastructure.md)). + +The same ADR adds a rule that this document must obey: **a deployment mode without a +signed contract cannot be the deciding factor in a technical choice.** It may break a tie +between otherwise-equal options. It may not, on its own, reject a dependency or justify +an abstraction. + +### Other config settings switched by mode - **Hub URL** — pointed at LearnStack-hosted (SaaS / Dedicated / SelfHostedOnline) OR customer-hosted (rare) OR not set (SelfHostedAirGapped). -- **Default secret store** — Dapr Vault (most modes) OR file-based (air-gapped optional). +- **Secret store** — `ConfigurationSecretProvider` today in every mode; the Vault-backed + provider is demand-gated to Phase 11 behind `ISecretProvider`, triggered by secrets + needing rotation without a redeploy or by a non-development deployment existing. - **Telemetry sink** — LearnStack OTel collector (SaaS / Dedicated) OR customer OTel - (Self-Hosted). -- **Dapr pub/sub component** — Kafka backend differs by deployment (managed Kafka vs - bundled Kafka). + (Self-Hosted). `SelfHostedAirGapped` wires no network exporter at all; its file target + lands in Phase 11. +- **Event transport** — `InProcessEventBus` today in every mode; the Dapr pub/sub + component and its Kafka backend land in Phase 11, triggered by a second process needing + to consume an integration event. See + [15-event-and-outbox.md](15-event-and-outbox.md). ## 6. Same Helm chart @@ -210,7 +271,8 @@ deployment: mode: SaaS hub: url: https://hub.learnstack.dev - apiKeyRef: { name: hub-api-key, key: token } + # mTLS client cert + HMAC secret + JWT signing key; no API key (ADR-0034) + credentialsRef: { name: hub-internal-api, key: bundle } postgres: managed: true connectionRef: { name: postgres-creds, key: connection-string } @@ -254,12 +316,14 @@ Same chart. Same templates. Different values. ## 7. Deployment-mode-conditional behaviour -A small list of behaviours change across modes: +A small list of behaviours change across modes. This is the **target** table; the two +supported modes reach it today and the three seams reach it in Phase 11. | Behaviour | SaaS | Dedicated | Self-Hosted Online | Self-Hosted Air-Gapped | |-----------|------|-----------|--------------------|------------------------| | `IEntitlementProvider` | Hub | Hub | Hub | SignedLicenseKey | -| `ISecretProvider` | Dapr → Vault | Dapr → Vault | Dapr → Vault | Dapr → Vault OR file | +| `ISecretProvider` (target) | Vault | Vault | Vault | Vault OR file | +| `ISecretProvider` (today) | `ConfigurationSecretProvider` in every mode — the Vault adapter is demand-gated to Phase 11 | ← | ← | ← | | Phone-home enabled | Yes | Yes | Yes | No | | Outbound HTTP to Hub | Yes | Yes | Yes | No | | OTel collector endpoint | LearnStack-hosted | LearnStack-hosted | Customer-hosted | Customer-hosted | @@ -295,7 +359,7 @@ ghcr.io/learnstack/learnstack-api:v1.0.1 ghcr.io/learnstack/learnstack-hub-api:v1.0.0 ghcr.io/learnstack/learnstack-hub-api:v1.0.1 ghcr.io/learnstack/learnstack-web:v1.0.0 -ghcr.io/learnstack/learnstack-hub-web:v1.0.0 +ghcr.io/learnstack/learnstack-hub-operator-portal:v1.0.0 ``` For Self-Hosted Air-Gapped, customers pull images into their own registry (script in @@ -324,6 +388,9 @@ To be authored: - `docs/operations/migration-orchestration.md` — Schema migration coordination. - `docs/operations/security-incident.md` — Security incident response. - `docs/operations/data-export-and-portability.md` — GDPR data export per tenant. +- `docs/operations/resource-fairness.md` — the SaaS contention controls named in + [§ 2](#2-saas-mode): `statement_timeout` classes, pool partitioning, query cost + ceilings, Hangfire queue fairness, per-tenant rate limits. ## 12. Non-goals @@ -338,6 +405,12 @@ To be authored: - ADR-0020 — Triple Deployment + Hybrid License. - ADR-0019 — LearnStack Hub. +- [ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) — what + Row Level Security does and does not guarantee. +- [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) — which modes are + supported, which are seams, and what promotes a seam. - [26-hybrid-license-model.md](26-hybrid-license-model.md) — license payload + lifecycle. - [24-learnstack-hub.md](24-learnstack-hub.md) — Hub architecture. - [04-technical-architecture.md](04-technical-architecture.md) — overall stack. +- [Phase 11: Production Hardening, Operations, and Scale](../roadmap/phase-11-production-hardening.md) + — owns resource fairness and the three prepared seams. diff --git a/docs/architecture/26-hybrid-license-model.md b/docs/architecture/26-hybrid-license-model.md index 987d57a..9d22865 100644 --- a/docs/architecture/26-hybrid-license-model.md +++ b/docs/architecture/26-hybrid-license-model.md @@ -15,14 +15,47 @@ The model handles all three deployment modes (SaaS / Dedicated / Self-Hosted) wi common implementation. This document describes the license payload, lifecycle, signing procedure, refresh cadence, revocation flow, and operational runbook. +## 0. Canonical key vocabulary + +The corpus has carried two incompatible spellings for the same cross-repository +projection: `classroom.recording` versus `classroom.recording.enabled`, and +`tenancy.max_learners` versus `limits.max_users` versus a bare `max_users`. Since the +payload is parsed by two repositories and cached in a third place, "close enough" means a +feature silently reads `false`. + +**One form is canonical, and it is the one fixed by +[ADR-0021 Amendment 1 (2026-05-18)](../decisions/0021-feature-based-entitlement.md) and +[21-feature-flags.md](21-feature-flags.md):** + +| Rule | Canonical | Not canonical | +|---|---|---| +| Every key is `{area}.{name}`, lowercase, dot-separated, `snake_case` within a segment | `classroom.recording` | `recording`, `Classroom.Recording` | +| Feature keys carry **no** `.enabled` suffix — a `FeatureKey` is boolean by construction | `classroom.recording` | `classroom.recording.enabled` | +| Limit keys are prefixed by their **subject area**, never by the word `limits` | `tenancy.max_learners` | `limits.max_users`, `max_users` | +| The area is the capability's owner, not the payload section it appears in | `media.storage_gb` | `limits.media_storage_gb` | + +Two further rules make the vocabulary enforceable rather than aspirational: + +- **A key that is not in `FeatureKeys` / `LimitKeys` may not appear in a payload.** The + registries in [21-feature-flags.md](21-feature-flags.md) are the closed set; adding a + key is a code change in both repositories plus a plan-editor update, not a Hub-side + string. +- **`FeatureKey_AllReferences_AreInRegistry`** (catalogued in + [Architecture Tests Catalogue](../standards/21-architecture-tests-catalogue.md)) fails + the build on a free-form string, and the schema snapshot test in + [§ 5](#5-entitlement-read-path-and-grace-enforcement) fails on a payload that carries + an unregistered key. + +Every older spelling elsewhere in the corpus is superseded by this table. Where a +document still shows `limits.max_users`, it is stale, not an alternative. + ## 1. License key payload The key is a JWT-style RS256-signed token with a custom header `typ: "LSL"` -("LearnStack License"). The embedded `entitlement.features` key strings follow -the typed `FeatureKey` catalog in -[21-feature-flags.md](21-feature-flags.md) and -[ADR-0021 Amendment 1](../decisions/0021-feature-based-entitlement.md) — the -trailing `.enabled` suffix used in earlier drafts has been dropped. +("LearnStack License"). Its `entitlement` object is the **same shape** the Hub pushes +over `PUT /api/internal/tenants/{id}/entitlements` and the same shape +`platform_entitlement_cache` stores — one wire contract, three carriers — and it is +pinned by `entitlement-v1.schema.json` (see [§ 5](#5-entitlement-read-path-and-grace-enforcement)). ```json { @@ -51,13 +84,13 @@ trailing `.enabled` suffix used in earlier drafts has been dropped. "compliance.data_residency": true }, "limits": { - "limits.max_users": 50000, - "limits.max_organizations": 100, - "limits.classroom_minutes_per_month": -1, - "limits.recording_storage_gb": 10000, - "limits.media_storage_gb": 50000, - "limits.media_bandwidth_gb_per_month": -1, - "limits.api_rate_per_minute": 60000 + "tenancy.max_learners": 50000, + "tenancy.max_organizations": 100, + "classroom.minutes_per_month": -1, + "classroom.recording_storage_gb": 10000, + "media.storage_gb": 50000, + "media.bandwidth_gb_per_month": -1, + "integrations.api_rate": 60000 }, "compliance": { "caps": { @@ -71,7 +104,7 @@ trailing `.enabled` suffix used in earlier drafts has been dropped. "expires_at": "2028-05-18T00:00:00Z", "grace_until": "2028-06-17T00:00:00Z", "phone_home_url": "https://hub.learnstack.dev/api/v1/internal/license/refresh", - "revocation_list_url": "https://hub.learnstack.dev/api/v1/internal/license/revocations" + "revocation_list_url": "https://hub.learnstack.dev/.well-known/learnstack/revocation-list.signed.json" }, "signature": "base64url-encoded-RS256-signature-over-base64url(header).base64url(payload)" } @@ -186,46 +219,126 @@ public sealed class LicenseRefreshJob : LearnStackJob hit Hub at the same time. The job adds a random 0-119 minute offset per tenant when first enqueued. -## 5. Grace period enforcement +## 5. Entitlement read path and grace enforcement + +### What was wrong + +The earlier `HubEntitlementProvider.GetAsync` had two defects that cancelled out the +guarantee this whole document exists to provide: + +1. **No cold-start fallback.** On a cache miss it called the Hub unguarded. A Hub outage + therefore threw a transport exception **out of a feature-flag check** — so a pod + restart during a Hub incident turned "is recording enabled?" into a 500 on an + unrelated request path. +2. **The durable table was never read.** `platform_entitlement_cache` — the table that + carries `valid_until` and `grace_until` — did not appear anywhere in the read path. + Grace was evaluated against whatever happened to be in the distributed cache, whose + TTL is ≤ 15 minutes. The advertised **30-day grace period was, in practice, a 15-minute + cache TTL**: on eviction, the only source left was the Hub, which was the thing that + was down. -`HubEntitlementProvider.GetAsync(tenantId)`: +Those are the same defect seen twice: a *freshness* mechanism (cache TTL) was being used +as an *authorisation* mechanism (grace window). They are different clocks and they need +different storage. + +### The normative order + +[ADR-0034 § The entitlement read path](../decisions/0034-hub-contract-surface-invariant.md) +fixes the order, and the order is normative — an implementation may not skip a layer: + +```text +L1 in-process cache (per pod; seconds) + → L2 distributed cache (ICacheService; ≤ 15 min TTL — freshness only) + → platform_entitlement_cache (durable table; carries valid_until + grace_until) + → Hub POST /api/v1/internal/license/verify (last resort, always guarded) +``` ```csharp -public async Task GetAsync(Guid tenantId, CancellationToken ct) +public async Task GetAsync(TenantId tenantId, CancellationToken ct) { - var cached = await _cache.GetAsync(CacheKey(tenantId), ct); - if (cached is null) - { - // First access; fetch from Hub - cached = await _hubClient.VerifyAsync(tenantId, ct); - await _cache.SetAsync(CacheKey(tenantId), cached, ct); - return cached; - } + // 1-2. L1 → L2, both pure freshness layers. + if (await _cache.GetAsync(CacheKey(tenantId), ct) is { } cached) + return Evaluate(cached, source: EntitlementSource.Cache); - var now = _clock.UtcNow; - if (now < cached.ExpiresAt) - { - return cached; // Fresh; serve as-is - } + // 3. Durable projection. This is the layer that makes grace real: it survives pod + // restarts, cache flushes and Hub outages, and it is the only place valid_until / + // grace_until are authoritative. + var durable = await _entitlementCacheStore.FindAsync(tenantId, ct); - if (cached.GraceUntil is not null && now < cached.GraceUntil) + // 4. Hub — only when the durable row is missing or stale, and never unguarded. + if (durable is null || _clock.UtcNow >= durable.ValidUntil) { - _logger.LogWarning("Entitlement for {TenantId} in grace period (expired {Expiry}, grace until {Grace})", - tenantId, cached.ExpiresAt, cached.GraceUntil); - return cached with { InGracePeriod = true }; // Serve cached; flag for UI banner + try + { + var refreshed = await _hubClient.VerifyAsync(tenantId, ct); + await _entitlementCacheStore.UpsertAsync(refreshed, ct); // durable first + await _cache.SetAsync(CacheKey(tenantId), refreshed, ct); // then fast layers + return Evaluate(refreshed, source: EntitlementSource.Hub); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + // A control-plane outage must not throw out of a feature-flag check. + _logger.LogWarning(ex, "Hub verify failed for {TenantId}; falling back", tenantId); + _metrics.HubVerifyFailed(); + if (durable is null) + return EntitlementLookup.Unresolved(tenantId); // policy table below decides + } } - // Past grace; read-only mode - return Entitlement.ReadOnly(tenantId, cached.Generation); + await _cache.SetAsync(CacheKey(tenantId), durable!, ct); + return Evaluate(durable!, source: EntitlementSource.Durable); } ``` -When in grace, the Admin Studio surfaces a banner: "Your license is in a 30-day grace -period. Contact support / renew via Hub." +`Evaluate` applies the lifecycle from [§ 3](#3-lifecycle): fresh → serve; past +`valid_until` but within `grace_until` → serve with `InGracePeriod = true`; past +`grace_until` → read-only. + +The provider **never throws** out of a feature-flag or limit check. Every path returns an +`EntitlementLookup` carrying the values, the source, and whether the answer is degraded. + +### Failure policy by key class + +`EntitlementLookup.Unresolved` — no cache, no durable row, no Hub — is the only genuinely +hard case, and one blanket answer is wrong for it. Each key class declares its posture +**explicitly in the registry**, so the behaviour is a property of the key rather than of +the call site: + +| Key class | Posture when unresolved | Why | +|---|---|---| +| Compliance caps (`compliance.*`, `audit.retention.days`, `data.residency.region`) | **Fail closed** — reject the operation that depends on the cap | An unknown residency or retention cap must never be read as permissive; a wrong answer here is a regulatory finding | +| Security-surface features (`identity.sso.saml`, `identity.scim`, `audit.export`, `integrations.api_access`) | **Fail closed** — treat as disabled | An unknown answer must not open an export or an API surface | +| Product capability features (`classroom.recording`, `classroom.breakout_rooms`, `tenancy.custom_domain`, `analytics.advanced_reporting`) | **Fail closed on a cold start, fail open to the last known value otherwise** | A paying tenant mid-class should not lose recording because the Hub is down — but the platform must not invent an entitlement it has never seen | +| Numeric limits (`tenancy.*`, `classroom.*`, `media.*`, `integrations.api_rate`) | **Fall back to the built-in floor** — the Starter-tier defaults compiled into the binary. Never `-1`, never `0` | Unlimited is a gift; zero is an outage. The floor keeps a tenant working at the smallest plan's ceiling until the answer arrives | + +Two consequences worth naming: + +- Cold-start-unresolved is **rare and loud**: it requires an empty L1, an empty L2, no + durable row, and an unreachable Hub. It is alerted on + `learnstack_entitlement_unresolved_total{tenant_id}`, not silently absorbed. +- The source of every answer is observable — + `learnstack_entitlement_source_total{source}` over `cache | durable | hub | floor`. A + rising `durable` share means the Hub is degraded; a non-zero `floor` share means + tenants are being served the fallback and someone must know. + +### The wire contract is pinned -When past grace, every feature flag returns `false`; every limit returns `0`. Writes -return `Result.Failure("license.expired_read_only")`. Reads continue (so customers don't -lose access to their own data). +The projection's shape is checked into **both** repositories as +`entitlement-v1.schema.json`, and a snapshot test in each fails the build when the +serialised payload drifts from it. This is what makes the read path above safe to +evolve: a Hub-side field rename that LearnStack cannot parse breaks a test in the Hub +repository, not a tenant's feature flag in production. + +The schema also encodes the vocabulary from [§ 0](#0-canonical-key-vocabulary) — +`features` and `limits` keys are validated against the registry — so an unregistered key +cannot reach a payload that LearnStack will cache. + +### UI surface + +When in grace, Admin Studio surfaces a banner: "Your license is in a 30-day grace period. +Contact support / renew via Hub." When past grace, every feature flag returns `false` and +every limit returns `0`; writes return `Result.Fail("license.expired_read_only")` and +reads continue, so customers never lose access to their own data. ## 6. Signed-key delivery (air-gapped) @@ -249,11 +362,23 @@ Same pattern as Nexora's license hot-reload ## 7. Revocation +The revocation list is a **signed static artefact at a fixed URL**, not an internal-API +endpoint: + ``` -GET https://hub.learnstack.dev/api/v1/internal/license/revocations +https://hub.learnstack.dev/.well-known/learnstack/revocation-list.signed.json ``` -Returns a signed bundle: +It is deliberately **not** part of the Hub contract surface +([ADR-0034](../decisions/0034-hub-contract-surface-invariant.md)), and it is +unauthenticated: the RS256 signature is its authentication, and its contents are opaque +licence ids — no tenant name, no slug, no plan. An unauthenticated reader learns how many +licences were revoked, not whose. This is also the only form an air-gapped customer can +consume: a file they can carry in on media, verify offline, and place next to their +licence. [ADR-0020 § Revocation](../decisions/0020-triple-deployment-hybrid-license.md) +names the same artefact. Moving it onto an endpoint would require an ADR. + +The bundle: ```json { @@ -269,8 +394,12 @@ Returns a signed bundle: } ``` -LearnStack runtime (any deployment mode) fetches this bundle daily via a Hangfire job. -The signed bundle is cached locally; signature verified against the same key set. +The fetch is performed by `SignedLicenseKeyEntitlementProvider` on a daily Hangfire job +— the same named adapter that reads the `.lic` file, so no unnamed type holds a Hub +reference and [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md)'s second +invariant holds. Because the artefact is static, signed and unauthenticated, the fetch is +not a contract-surface crossing. The bundle is cached locally; the signature is verified +against the same key set. License verification: @@ -305,8 +434,9 @@ Hub operator portal exposes: | Air-gapped customer over-running expiry | 30-day grace + LearnStack-operator visibility for proactive renewal contact | | Customer extending license offline | License is RSA-signed; customer cannot forge a new signature | | Customer tampering with cached entitlement | Cache table protected by RLS + DB-level read-only role for non-admin paths | -| Stolen Hub API key | Per-tenant API key; rotatable; scope limited to license verify + usage report | -| Revoked license still cached | Cache TTL ≤ 15 min; eager invalidation via Dapr event when revoked online; revocation list pulled daily | +| Stolen phone-home credential | Per-instance client certificate + short-lived RS256 JWT + HMAC body signature; all three rotatable; scope limited to license verify, refresh and usage report ([ADR-0034](../decisions/0034-hub-contract-surface-invariant.md)) | +| Revoked license still cached | Revocation invalidates **all four** layers, in the order that closes the window: `platform_entitlement_cache` row first, then L2, then L1 via the invalidation event. Invalidating only the fast layers leaves the durable row to re-serve the revoked entitlement on the next miss. Cache TTL ≤ 15 min and the daily revocation-list pull are the backstops, not the mechanism | +| Grace window silently collapsing to a cache TTL | `valid_until` / `grace_until` are read from `platform_entitlement_cache`, never from a cache entry. An integration test flushes both cache layers, stops the Hub, and asserts the tenant still resolves through the durable row for the full grace window | ## 10. Architecture tests @@ -317,17 +447,32 @@ Hub operator portal exposes: 3. `NullEntitlementProvider_RejectedInProduction` — runtime startup check: in any non- Development environment, `IEntitlementProvider` is `HubEntitlementProvider` or `SignedLicenseKeyEntitlementProvider`; never `NullEntitlementProvider`. -4. `LicenseKey_Payload_MatchesSchema` — `entitlement-v1.schema.json` snapshot test; any - breaking change requires schema-version bump. +4. `LicenseKey_Payload_MatchesSchema` — `entitlement-v1.schema.json` snapshot test, run + in **both** repositories against the same checked-in schema; any breaking change + requires a schema-version bump and a coordinated change in both. A snapshot test in + only one repository proves only that that repository is self-consistent. +5. `Entitlement_Read_Path_Falls_Through_To_Durable_Row` — integration test: flush L1 and + L2, make the Hub unreachable, assert the tenant resolves from + `platform_entitlement_cache` and that no exception escapes the feature-flag call. +6. `FeatureKey_AllReferences_AreInRegistry` — catalogued in + [Architecture Tests Catalogue](../standards/21-architecture-tests-catalogue.md); + backs the vocabulary rule in [§ 0](#0-canonical-key-vocabulary). ## 11. Phasing -| Phase | Deliverable | -|-------|-------------| -| 02 | `IEntitlementProvider` interface in SharedKernel. `NullEntitlementProvider` default. `platform_entitlement_cache` table. `DeploymentMode` config enum. | -| 02c | `HubEntitlementProvider` (calls Hub `/internal/license/verify`); Hub-side `Entitlement` recompute on subscription change. | -| 09b | License-key issuance UI in Hub operator portal. | -| 11 | `SignedLicenseKeyEntitlementProvider` (air-gapped). Revocation list signing + distribution. Phone-home retry / backoff tuning. Grace period enforcement integration tests. SIGHUP hot-reload. Key rotation procedure documented. | +Per [ADR-0035](../decisions/0035-demand-gated-infrastructure.md), the port ships early +and each implementation ships against a written trigger. + +| Phase | Deliverable | Trigger | +|-------|-------------|---------| +| [02a Packet 6](../roadmap/phase-02a-kernel-tenancy.md) | `platform_entitlement_cache` table; `DeploymentMode` config enum | One-way door — the durable projection's schema and ownership | +| [02a Packet 9](../roadmap/phase-02a-kernel-tenancy.md) | `IEntitlementProvider` socket; `NullEntitlementProvider` (all features enabled, no limits) as the **only** implementation | — | +| [02c](../roadmap/phase-02c-hub-foundation.md) | `HubEntitlementProvider` with the four-layer read path above; `entitlement-v1.schema.json` in both repositories; Hub-side `Entitlement` recompute on subscription change | A tenant must be billed or plan-gated | +| [09b](../roadmap/phase-09b-hub-billing.md) | License-key issuance UI in the Hub operator portal | Commercial billing needed | +| [11](../roadmap/phase-11-production-hardening.md) | `SignedLicenseKeyEntitlementProvider` (air-gapped); revocation-list signing + distribution; phone-home retry / backoff tuning; grace-period integration tests; SIGHUP hot-reload; key-rotation procedure | A Self-Hosted contract is signed | + +`NullEntitlementProvider` must not be registered outside `Development` once Phase 02c +lands (`NullEntitlementProvider_NotRegistered_OutsideDevelopment`). ## 12. Operational runbook (Phase 11) @@ -339,8 +484,13 @@ Hub operator portal exposes: ## References - ADR-0020 — Triple Deployment + Hybrid License. -- ADR-0021 — Feature-Based Entitlement. +- ADR-0021 (Amendment 1) — Feature-Based Entitlement; the canonical key vocabulary. - ADR-0019 — LearnStack Hub. +- [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md) — the normative + entitlement read path and the Hub contract surface invariants. +- [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) — which entitlement + implementation ships when. +- [21-feature-flags.md](21-feature-flags.md) — the `FeatureKeys` / `LimitKeys` registries. - [25-deployment-models.md](25-deployment-models.md) — three-mode topology. - [24-learnstack-hub.md](24-learnstack-hub.md) — Hub architecture. - Nexora reference: `Nexora/docs/decisions/0030-license-hot-reload-mechanism.md`, diff --git a/docs/architecture/27-custom-domain-tls.md b/docs/architecture/27-custom-domain-tls.md index bfa6070..10d1710 100644 --- a/docs/architecture/27-custom-domain-tls.md +++ b/docs/architecture/27-custom-domain-tls.md @@ -27,7 +27,7 @@ sequenceDiagram Tenant->>Studio: Enter domain "anatoliayoga.com" Studio->>LSApi: POST /api/v1/tenant/custom-domains (acts as proxy) - LSApi->>HubAPI: POST /api/v1/tenants/{id}/custom-domains
(internal API, mTLS) + LSApi->>HubAPI: POST /api/v1/internal/tenants/{id}/custom-domains
(via IHubTenantSync; mTLS + JWT + HMAC) HubAPI->>HubAPI: Insert CustomDomain row, status=Pending HubAPI->>Studio: 201 + verification instructions
(CNAME instructions) @@ -40,10 +40,11 @@ sequenceDiagram HubJob->>HubJob: Status=Verifying HubJob->>LE: ACME order via DNS-01 (or HTTP-01 fallback) LE-->>HubJob: Cert issued - HubJob->>HubAPI: Store cert in Vault; update CustomDomain
Status=Active, cert_expires_at=2026-08-16 - HubAPI->>HubAPI: Publish learnstack.hub.custom-domain.activated event
via Dapr pub/sub - HubAPI->>APISIX: Hot-reload route table partial
(add SNI cert + route entry) - HubAPI->>LSApi: Cache invalidation for host→tenant lookup + HubJob->>HubAPI: Store cert + key in the Hub secret store;
update CustomDomain Status=Active, cert_expires_at + HubAPI->>HubAPI: Replicate cert + key to the LearnStack-side secret store
(secret-store replication — never over HTTP payload) + HubAPI->>HubAPI: Publish learnstack.hub.custom-domain.activated event + HubAPI->>APISIX: Hot-reload route table partial
(SNI entry referencing the secret BY PATH) + HubAPI->>LSApi: PUT /api/internal/tenants/{id}/host-mappings
(host to tenant/org mapping only — no key material) else CNAME mismatch HubJob->>HubJob: Increment attempt; retry in 60s end @@ -165,11 +166,21 @@ routes: ssl: - id: ssl-anatoliayoga sni: anatoliayoga.com - # cert/key materialized via Vault Agent sidecar in production - cert_ref: vault://learnstack-hub/certs/anatoliayoga.com/cert - key_ref: vault://learnstack-hub/certs/anatoliayoga.com/key + # Materialised from the LEARNSTACK-side secret store by the secret-agent sidecar. + # The path is the replication target of the Hub-side secret; the value never + # travels in an HTTP payload. See § 6. + cert_ref: secret://learnstack/certs/anatoliayoga.com/cert + key_ref: secret://learnstack/certs/anatoliayoga.com/key ``` +> **Phasing note.** APISIX itself is demand-gated to +> [Phase 11](../roadmap/phase-11-production-hardening.md) per +> [ADR-0035](../decisions/0035-demand-gated-infrastructure.md); until it arrives, host +> routing and TLS termination are the deployment's ingress concern and +> `platform_host_to_tenant` remains the sole authority for host → tenant resolution +> inside the application. The route-partial mechanism described here is the target +> design, not a running system. + APISIX watches the file (standalone mode); changes are picked up within seconds. In Kubernetes, the file is in a ConfigMap; rolling update of the ConfigMap propagates; @@ -179,35 +190,112 @@ Etcd-backed APISIX (Phase 11+) replaces file-write with direct Admin API call. ## 6. Tenant resolver mapping -LearnStack runtime needs to map `Host: anatoliayoga.com` → `tenant_id`. Implementation: +LearnStack runtime needs to map `Host: anatoliayoga.com` → `(tenant_id, +organization_id?)`. + +### Host resolution never calls the Hub + +An earlier version of this document showed `CachedHostToTenantResolver` taking an +`IHubClient` and calling `LookupHostAsync` on every cache miss. That broke three rules at +once, and the third is the one that matters operationally: + +1. It was an **unrecorded endpoint** — no ADR, no entry in the contract surface, and no + counterpart in the Hub's own API documentation. +2. It was a **Hub call from outside the sanctioned adapters**. Only + `IEntitlementProvider`, `IUsageReporter` and `IHubTenantSync` may hold a Hub client + ([ADR-0034](../decisions/0034-hub-contract-surface-invariant.md)). +3. It put the **Hub on the hot path of anonymous public page loads**. Every cache miss on + every marketing page of every tenant became a synchronous dependency on the control + plane. A Hub outage — or a cold cache after a deploy during one — would have taken + every tenant's public site down, for a lookup whose answer LearnStack already stores. + +**`IHubClient.LookupHostAsync` is deleted.** `IHostToTenantResolver` reads +`platform_host_to_tenant` and nothing else: ```csharp namespace LearnStack.Infrastructure.MultiTenancy; public interface IHostToTenantResolver { - Task ResolveAsync(string host, CancellationToken ct = default); + Task ResolveAsync(string host, CancellationToken ct = default); } +public sealed record HostResolution(TenantId TenantId, OrganizationId? OrganizationId); + public sealed class CachedHostToTenantResolver( ICacheService cache, - IHubClient hubClient) : IHostToTenantResolver + TenancyDbContext db) : IHostToTenantResolver { - public async Task ResolveAsync(string host, CancellationToken ct = default) - { - return await cache.GetOrSetAsync( - $"hub:host:{host}", - async _ => await hubClient.LookupHostAsync(host, _), + public Task ResolveAsync(string host, CancellationToken ct = default) + => cache.GetOrSetAsync( + $"host:{host}", + async token => await db.HostMappings + .AsNoTracking() + .Where(m => m.Host == host && m.IsActive) + .Select(m => new HostResolution(m.TenantId, m.OrganizationId)) + .SingleOrDefaultAsync(token), new CacheOptions(L1Ttl: TimeSpan.FromMinutes(2), L2Ttl: TimeSpan.FromMinutes(15)), ct); - } } ``` -Cache invalidated on `learnstack.hub.custom-domain.activated` and -`learnstack.hub.custom-domain.revoked` Dapr pub/sub events. `TenantMiddleware` calls this -resolver first (before JWT validation, since the tenant context is needed for some -public anonymous routes too). +`platform_host_to_tenant` is a **platform-level table**, not tenant-owned — the row is +what *determines* the tenant, so it cannot be filtered by a tenant context that does not +exist yet. It is read before `ITenantContext` is populated, and the read is the only +database access in the request that legitimately runs unscoped. + +Consequences of the change: + +- A Hub outage degrades **billing and provisioning**. It does not touch page loads. +- The cache in front of the table is a latency optimisation, not an availability + mechanism. Even a total cache failure leaves a single indexed primary-key lookup. +- `TenantResolverMiddleware` calls this resolver first, before JWT validation, because + anonymous public routes need a tenant context too. + +### How mappings arrive + +Hub pushes host mappings over a dedicated endpoint, **not** through the entitlement +payload: + +```text +PUT /api/internal/tenants/{id}/host-mappings +``` + +The endpoint is part of the enumerated Hub → LearnStack surface in +[ADR-0034](../decisions/0034-hub-contract-surface-invariant.md), carries the same auth +chain as every other internal call (mTLS + RS256 JWT with `aud=learnstack-internal` + +HMAC body signature + `jti` replay protection), and is handled by `IHubTenantSync`. The +handler upserts `platform_host_to_tenant` and invalidates the resolver cache for the +affected hosts on `learnstack.hub.custom-domain.activated` / +`.revoked`. + +### Certificate material never rides the mapping payload + +The host-mapping payload carries **hosts and identifiers only**. TLS certificates and +private keys are never carried in an HTTP payload that LearnStack caches, logs, audits or +mirrors — which is precisely what the superseded design did when it tunnelled cert +material through `PUT /api/internal/tenants/{id}/entitlements`, a payload that lands in +`platform_entitlement_cache`. + +Key material moves by **secret-store replication** between the Hub-owned and +LearnStack-owned secret stores, and the mapping payload references it **by path**: + +```json +{ + "host": "anatoliayoga.com", + "tenant_id": "…", + "organization_id": null, + "certificate_ref": "learnstack/certs/anatoliayoga.com", + "is_active": true +} +``` + +`certificate_ref` is a path, resolvable only by a principal that already holds read +access to that secret store. Possession of the payload — in a log line, an audit row, a +cache entry, or a support ticket screenshot — grants nothing. +[ADR-0022 Amendment 1](../decisions/0022-custom-domain-tls.md)'s step 3 is superseded +accordingly; its central guarantee, that the Hub never holds Kubernetes credentials on +the LearnStack cluster, is unchanged. ## 7. Cert renewal @@ -301,16 +389,29 @@ instead of just `tenant_id`. always derived from authenticated session, never from request body / query. - `CustomDomain_Revocation_RemovesTenantResolverMapping` — integration test ends with the resolver returning null for the revoked host. +- `Hub_Client_Referenced_Only_By_Named_Adapters` — from + [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md); a resolver, a + middleware or a controller holding an `IHubClient` fails the build. This is the + mechanical guard against the deleted `LookupHostAsync` pattern reappearing. +- `Host_Resolution_Makes_No_Outbound_Calls` — integration test resolves a host with the + Hub client registered as a throwing stub and asserts the resolution still succeeds. ## 11. Phasing -| Phase | Deliverable | -|-------|-------------| -| 02a | `IHostToTenantResolver` interface + `CachedHostToTenantResolver` implementation in LearnStack. | -| 02c | `CustomDomain` aggregate in Hub. Submission endpoint scaffolded; verification logic stubbed (always returns success in dev). | -| 04 | Admin Studio custom-domain settings page: submission UI, DNS instructions, real-time status. | -| 09b | Hub operator portal: Pending Queue, Active List, Renewal Watch dashboards. Compliance caps editor includes domain-gating policy. | -| 11 | Production hardening: cert-manager + Let's Encrypt automation finalised; DNS provider API integrations (Cloudflare, Route 53, GCP DNS, Azure DNS); HTTP-01 fallback; renewal job tested at scale; APISIX hot-reload validated. | +| Phase | Deliverable | Trigger | +|-------|-------------|---------| +| [02a Packet 6](../roadmap/phase-02a-kernel-tenancy.md) | `platform_host_to_tenant` table | One-way door — the table that determines the tenant | +| [02a Packet 7](../roadmap/phase-02a-kernel-tenancy.md) | `IHostToTenantResolver` + `CachedHostToTenantResolver` reading `platform_host_to_tenant` and nothing else; two hosts wired to two seed tenants | — | +| [02d](../roadmap/phase-02d-walking-skeleton.md) | Host-based resolution exercised end to end: two hosts, two tenants, one binary | — | +| [02c](../roadmap/phase-02c-hub-foundation.md) | LearnStack-side `PUT /api/internal/tenants/{id}/host-mappings` handler behind `IHubTenantSync`; Hub-side `CustomDomain` aggregate and submission endpoint live in the Hub repository | A tenant is provisioned through the Hub | +| [04](../roadmap/phase-04-cms-media-pages.md) | Admin Studio custom-domain settings page: submission UI, DNS instructions, real-time status | — | +| [09b](../roadmap/phase-09b-hub-billing.md) | Hub operator portal: Pending Queue, Active List, Renewal Watch dashboards; compliance-caps editor includes domain gating | Commercial billing needed | +| [11](../roadmap/phase-11-production-hardening.md) | **The TLS automation itself**: ACME client, Let's Encrypt integration, DNS provider APIs (Cloudflare, Route 53, GCP DNS, Azure DNS), HTTP-01 fallback, renewal job at scale, secret-store replication, APISIX hot-reload validation | A tenant needs its own domain in production ([ADR-0035](../decisions/0035-demand-gated-infrastructure.md)) | + +The split is deliberate: **the mapping is a one-way door and ships early; the automation +that populates it is additive and ships on demand.** A tenant can run on a custom domain +before Phase 11 by having an operator insert the mapping row and place the certificate in +the secret store by hand. What Phase 11 removes is the operator, not the capability. ## 12. Operational runbook (Phase 11) @@ -333,7 +434,13 @@ instead of just `tenant_id`. ## References -- ADR-0022 — Custom Domain & TLS. +- ADR-0022 — Custom Domain & TLS. Amendment 1's step 3 (certificate material inside the + entitlement payload) is superseded by ADR-0034. +- [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md) — Hub contract surface + invariant: the `host-mappings` endpoint, key material leaving the entitlement payload, + and the rule that host resolution never calls the Hub. +- [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) — when the TLS automation + ships. - ADR-0019 — LearnStack Hub. - ADR-0021 — Feature-Based Entitlement (gate). - ADR-0015 — APISIX Gateway (hot-reload of route table + SSL config). diff --git a/docs/architecture/28-platform-tenant-organization.md b/docs/architecture/28-platform-tenant-organization.md index c6dd405..587c964 100644 --- a/docs/architecture/28-platform-tenant-organization.md +++ b/docs/architecture/28-platform-tenant-organization.md @@ -252,19 +252,21 @@ operational discipline: Both repositories carry a row for a tenant. Each field belongs to **exactly one** of the two as the source of truth; the other side mirrors read-only and is invalidated -by integration events. The closed Hub HTTPS contract surface (four endpoints, -ADR-0019) means the only legitimate cross-system writes flow through one of those -endpoints. +by integration events. The Hub HTTPS contract surface +([ADR-0034](../decisions/0034-hub-contract-surface-invariant.md)) means the only +legitimate cross-system writes flow through one of its enumerated endpoints, and every +one of them goes through a named adapter — `IEntitlementProvider`, `IUsageReporter` or +`IHubTenantSync`. | Field | Authoritative side | Mirrored side | Sync direction & event | |-------|--------------------|---------------|------------------------| | `tenant_id` (UUID) | Hub (issued on tenant create) | LearnStack | One-time push at provisioning via `POST /api/internal/tenants` | | `slug` (handle) | Hub | LearnStack | `POST /api/internal/tenants` at create; renames require an ADR (none today) | | `display_name` | **LearnStack** | Hub | LearnStack publishes `learnstack.tenancy.tenant.renamed`; Hub consumer updates its mirror | -| `status` (Trial/Active/Suspended/Archived) | Hub | LearnStack | `PUT /api/internal/tenants/{id}/entitlements` carries the status; or a dedicated lifecycle endpoint to be added by future ADR | +| `status` (Trial/Active/Suspended/Archived) | Hub | LearnStack | `PUT /api/internal/tenants/{id}/status` | | `plan_code` | Hub | LearnStack | Carried inside entitlement projection (`PUT /api/internal/tenants/{id}/entitlements`) | | `features` / `limits` / `compliance` | Hub | LearnStack (`platform_entitlement_cache`) | `PUT /api/internal/tenants/{id}/entitlements`; eagerly invalidated by Dapr event `learnstack.hub.entitlement` | -| Custom-domain `host → tenant` mapping | Hub | LearnStack (`platform_host_to_tenant`) | Hub pushes after DNS / TLS verification; Dapr event `learnstack.hub.custom-domain.activated` / `.deactivated` | +| Custom-domain `host → tenant` mapping | Hub | LearnStack (`platform_host_to_tenant`) | Hub pushes after DNS / TLS verification via `PUT /api/internal/tenants/{id}/host-mappings`, which carries the tuple only — certificate material moves by secret-store replication and is referenced by path. LearnStack resolves hosts from `platform_host_to_tenant` and **never** calls the Hub | | Branding (logo, colours, typography) | **LearnStack** | Hub (denormalised summary only, optional) | LearnStack-side write only; no sync needed today | | Organizations + memberships | **LearnStack** | — | Hub does not mirror; tenant-internal shape | | Course / content / lesson / enrollment / progress data | **LearnStack** | — | Hub never stores tenant content (invariant #3 above) | @@ -274,9 +276,9 @@ endpoints. **Rename rule.** The two fields that can change after provisioning (`display_name`, branding) are LearnStack-authoritative — there is no Hub endpoint to -write them, only a LearnStack-published event the Hub consumer mirrors. This keeps the -four-endpoint contract closed; adding a Hub-side write for `display_name` would -require a new ADR. +write them, only a LearnStack-published event the Hub consumer mirrors. Adding a +Hub-side write for `display_name` would require a new ADR, because the contract surface +is a cross-repository agreement. **Mirror staleness budget.** The entitlement projection is **eagerly** invalidated via Dapr and otherwise has a 15-min TTL. The host → tenant mapping is invalidated by Dapr diff --git a/docs/architecture/29-dapr-integration.md b/docs/architecture/29-dapr-integration.md index 8de0750..8e21f75 100644 --- a/docs/architecture/29-dapr-integration.md +++ b/docs/architecture/29-dapr-integration.md @@ -4,6 +4,19 @@ [ADR-0006](../decisions/0006-events-and-outbox.md), [ADR-0010](../decisions/0010-cross-module-communication.md). +> **Read this first.** This document describes Dapr in the present tense as the **target +> design**. Per [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) no Dapr +> component is wired today. Of the three ports, only `ISecretProvider` has shipped — +> `ConfigurationSecretProvider`, in Packet 3. `IEventBus` and `ICacheService` land with +> their in-process defaults (`InProcessEventBus`, `InMemoryCacheService`) in +> [Phase 02a Packet 5](../roadmap/phase-02a-kernel-tenancy.md); from then on those +> defaults are the only registrations in every deployment mode. The three Dapr adapters +> land in +> [Phase 11](../roadmap/phase-11-production-hardening.md) against written triggers — a +> second process consuming an integration event, a second application instance, and +> secrets needing rotation without a redeploy. Nothing below is wrong; none of it is +> running. + LearnStack uses **Dapr** (Distributed Application Runtime) for three building blocks: **pub/sub** (Kafka), **state store** (Valkey), **secret store** (Vault). Application code interacts with Dapr only through SharedKernel abstractions (`IEventBus`, `ICacheService`, @@ -150,12 +163,13 @@ secret/learnstack/seaweedfs endpoint, access-key, secret-key secret/learnstack/meilisearch master-key, public-key secret/learnstack/livekit api-key, api-secret, ws-url secret/learnstack/coturn shared-secret -secret/learnstack/hub api-key, internal-api-hmac-key, internal-api-mtls-cert +secret/learnstack/hub internal-api-hmac-key, internal-api-mtls-cert, + internal-api-mtls-key, internal-api-jwt-signing-key ``` In `Development` the **primary** `ISecretProvider` implementation is -`EnvironmentSecretProvider` (reads from process env vars; configured via -`.env`; matches the composition-root table in +`ConfigurationSecretProvider` (reads `IConfiguration`, which already merges environment +variables, user secrets and `appsettings.{env}.json`; matches the composition-root table in [20-infrastructure-stack.md § Composition Root and Deployment Mode](../standards/20-infrastructure-stack.md)). For dev workflows that prefer Dapr-shaped secrets (e.g. exercising the `DaprSecretProvider` code path locally), an optional @@ -207,6 +221,8 @@ public interface ICacheService CacheOptions? options = null, CancellationToken ct = default); Task SetAsync(string key, T value, CacheOptions? options = null, CancellationToken ct = default); Task RemoveAsync(string key, CancellationToken ct = default); + // Removed, or redesigned to a generation-key pattern, before Phase 02a Packet 5 + // ships (ADR-0035). See the note under the reference implementation below. Task RemoveByPrefixAsync(string prefix, CancellationToken ct = default); } @@ -291,6 +307,11 @@ internal sealed class DaprCacheService : ICacheService return _dapr.SaveStateAsync(StateStoreName, prefixed, value, metadata: metadata, cancellationToken: ct); } + // NOTE: superseded. `_trackedKeys` is instance-local, so keys written by another + // pod are never evicted and this method silently under-invalidates the moment a + // second instance runs. Per ADR-0035 it is removed from `ICacheService` or + // redesigned to a generation-key pattern before Phase 02a Packet 5 ships; see + // 32-tenant-customization-model.md § 8.2 for the generation-counter shape. public async Task RemoveByPrefixAsync(string prefix, CancellationToken ct = default) { var prefixed = PrefixKey(prefix); @@ -312,10 +333,14 @@ internal sealed class DaprCacheService : ICacheService } ``` -Cross-instance L1 invalidation: when one pod calls `RemoveByPrefixAsync`, it publishes a -`learnstack.cache.invalidation` event. A `CacheInvalidationSubscriber` (background -service) on every pod consumes the event and clears matching L1 entries — except those -published by its own instance (`InstanceId` skip-self). +Cross-instance L1 invalidation was originally specified against `RemoveByPrefixAsync`: +one pod publishes a `learnstack.cache.invalidation` event, and a +`CacheInvalidationSubscriber` on every pod clears matching L1 entries except those it +published itself. That contract is superseded. A tenant-scoped **generation counter** +embedded in the cache key makes every stale key unreachable at once without enumerating +keys, and needs no invalidation topic on the write path. See +[32-tenant-customization-model.md § 8.2](32-tenant-customization-model.md) and +[ADR-0035](../decisions/0035-demand-gated-infrastructure.md). ## 5. Sidecar deployment diff --git a/docs/architecture/30-api-gateway.md b/docs/architecture/30-api-gateway.md index 3933652..4e67593 100644 --- a/docs/architecture/30-api-gateway.md +++ b/docs/architecture/30-api-gateway.md @@ -7,6 +7,33 @@ LearnStack uses **Apache APISIX** in standalone mode as the API gateway in front of both the LearnStack core API and the LearnStack Hub API. +> **Status (2026-08-08).** APISIX is **demand-gated to +> [Phase 11](../roadmap/phase-11-production-hardening.md)** per +> [ADR-0035](../decisions/0035-demand-gated-infrastructure.md); its trigger is a non-development +> deployment that needs edge rate limiting, host routing, or JWT pre-validation. Until +> then the same responsibilities are carried by ASP.NET middleware inside the API +> process: correlation-id assignment, CORS, JWT bearer validation, rate limiting, and +> header normalisation. [ADR-0015](../decisions/0015-api-gateway-apisix.md) stands as the +> decision that APISIX is the gateway LearnStack uses; ADR-0035 decides when. +> +> **A gap worth naming rather than discovering.** In the shipped +> `infra/apisix/apisix.yaml`, route 100's `openid-connect` block is **commented out**, so +> today every `/api/v*` request reaches the backend unauthenticated **at the gateway**. +> **Neither layer authenticates today.** The backend does not re-validate, because there +> is nothing yet to re-validate against: `grep AddAuthentication\|UseAuthentication` over +> `backend/src` returns no registration, and `AuthorizationBehavior.Handle` is +> `return next()` with a Phase 03 TODO. Authentication lands in +> [Phase 02b](../roadmap/phase-02b-events-auth.md) and authorization in +> [Phase 03](../roadmap/phase-03-identity-admin.md); until then every `/api/v*` route is +> open, which is survivable only because no tenant-owned table and no protected endpoint +> exists yet. The "validated twice" property in § 5 describes the Phase 11 target, not +> the running system — and the first half of it arrives well before the gateway half. +> The block is uncommented in +> [Phase 11](../roadmap/phase-11-production-hardening.md), with the rest of APISIX: +> [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) demand-gates the gateway, +> so [Phase 03](../roadmap/phase-03-identity-admin.md) delivers authorization in ASP.NET +> middleware and corrects this route table, but does not stand APISIX up in front of it. + ## 1. Topology ```mermaid @@ -36,7 +63,7 @@ flowchart LR APISIX -- "host: app.learnstack.dev OR
{tenant-custom-domain}" --> LSApi APISIX -- "host: hub.learnstack.dev" --> HubApi HubApi -- "mTLS + signed JWT + HMAC
NEVER via APISIX" --> LSApi - LSApi -- "metering / verify (API key)" --> HubInternal + LSApi -- "metering / verify (mTLS + JWT + HMAC)" --> HubInternal LSApi <-.-> LSDapr HubApi <-.-> HubDapr ``` @@ -96,6 +123,10 @@ Client request 6. limit-count ─── per-window count limit for write endpoints (60/min/token) │ ▼ +6b. basic-auth ─── consumer credentials from the secret store; used only by the + │ `/admin/hangfire*` infrastructure route, never on `/api/v*` + │ + ▼ 7. proxy-rewrite ─── header normalisation; strip Authorization-Internal if leaked │ ▼ @@ -129,6 +160,7 @@ plugins: - openid-connect - limit-req - limit-count + - basic-auth # /admin/hangfire* only; a route may not use an undeclared plugin - proxy-rewrite - response-rewrite - prometheus @@ -137,72 +169,166 @@ plugins: ## 4. Route table -`apisix.yaml` declares routes with priority bands: +Three rules govern this table, and each exists because its violation has already been +found in the corpus. + +### Rule 1 — the URI wildcard is a single trailing `*` + +`lua-resty-radixtree`, the matcher APISIX uses, supports **one** wildcard and only at the +**end** of the path. There is no `**` form, and a `*` in the middle of a path is not a +wildcard — it is a literal asterisk. A route declared as `/api/v*/**` therefore does not +mean "any version, any sub-path"; it means something no client will ever send. + +Because [ADR-0024](../decisions/0024-api-versioning-policy.md) puts the version in the +URL and the version set is small and enumerable, the correct spelling is one route per +live version: + +| Wrong | Right | +|---|---| +| `/api/v*/**` | `/api/v1/*` (and `/api/v2/*` when v2 ships) | +| `/api/v*/localization/*` | `/api/v1/localization/*` | + +Adding a version adds routes. That is the intended cost of URL versioning, and it keeps +the deprecation window in ADR-0024 visible in the gateway config rather than hidden +behind a wildcard. + +### Rule 2 — every route declares an explicit `priority` + +APISIX resolves overlapping routes by `priority`, **higher wins**, and an omitted +`priority` defaults to `0`. The `id` field is an identifier, not an ordering — a fact the +earlier version of this table obscured by using `id` values that looked like priority +bands. + +The consequence of that confusion was concrete: the public `GET /api/v*/localization/*` +route carried no `priority` (so `0`) while the authenticated catch-all carried +`priority: 100`. Both matched a localization request, the catch-all won, and **a route +documented as public anonymous would have demanded a bearer token** — an anonymous +learner loading a public page would have received 401 from the gateway. + +The banding is therefore **specific-beats-general, written down**: + +| Band | Priority | Contents | +|---|---|---| +| Infrastructure | 400 | `/healthz`, `/openapi/*` (environment-gated), `/admin/hangfire*` | +| Public anonymous | 300 | Named public paths: localization, public catalog reads, auth endpoints with their own strict limits | +| CORS preflight | 200 | `OPTIONS` on the versioned prefix — must beat the authenticated band so preflight never reaches `openid-connect` | +| Authenticated catch-all | 100 | Everything else under the versioned prefix | + +**A public route always outranks the catch-all.** The invariant is **not** asserted in CI +today — there is no route-table lint, and calling a rule enforced when the check does not +exist is precisely the failure +[Standards 21](../standards/21-architecture-tests-catalogue.md) is written against. The +lint ships with APISIX itself in +[Phase 11](../roadmap/phase-11-production-hardening.md). It will fail when any route omits +`priority`, and when any route whose plugin +set lacks `openid-connect` shares a path prefix with a higher-or-equal-priority route +that has it. + +### Rule 3 — one trust domain per credential + +Route 100 previously carried +`client_secret_ref: vault://learnstack/hub/internal-api-hmac-key`. That path is the +**Hub internal API HMAC body-signing key** — a credential from an entirely different +trust domain, whose possession lets the holder forge a signed entitlement push for **any +tenant**. Handing it to the edge gateway as an OIDC client secret placed the platform's +highest-value secret on its most exposed component. + +The line is **deleted**, and no substitute is needed: the route is `bearer_only: true`, +which means APISIX validates a presented bearer token against the realm's public JWKS and +never performs a code exchange. A `bearer_only` OIDC route has no use for a client +secret at all. ```yaml routes: - # Public anonymous — priority 1 + # ---- Infrastructure band — priority 400 ------------------------------- - id: 1 - uri: /health + uri: /healthz + methods: [GET] + priority: 400 plugins: - limit-req: - rate: 10 - burst: 5 - rejected_code: 429 - key: remote_addr + limit-req: { rate: 10, burst: 5, key: remote_addr, rejected_code: 429 } upstream: { type: roundrobin, nodes: { "learnstack-api:5000": 1 } } - id: 2 - uri: /api/v*/localization/* + uri: /openapi/* + priority: 400 + upstream: { type: roundrobin, nodes: { "learnstack-api:5000": 1 } } + # Dev-only; environment-gated to 404 in production. + + # Hangfire dashboard — defense-in-depth: gateway BasicAuth + backend role check + - id: 3 + uri: /admin/hangfire* + priority: 400 + plugins: + basic-auth: {} # consumer credentials from the secret store + upstream: { type: roundrobin, nodes: { "learnstack-api:5000": 1 } } + + # ---- Public anonymous band — priority 300 ----------------------------- + # MUST outrank the authenticated catch-all, or these become 401s. + - id: 10 + uri: /api/v1/localization/* methods: [GET] + priority: 300 plugins: - cors: - allow_origins: "https://app.learnstack.dev,https://hub.learnstack.dev" - limit-req: { rate: 50, burst: 20, key: remote_addr } + cors: { allow_origins: "https://app.learnstack.dev,https://hub.learnstack.dev" } + limit-req: { rate: 50, burst: 20, key: remote_addr, rejected_code: 429 } upstream: { type: roundrobin, nodes: { "learnstack-api:5000": 1 } } - - id: 3 - uri: /openapi/* + - id: 11 + uri: /api/v1/auth/* + methods: [POST] + priority: 300 + plugins: + # allow_origins takes EXACT origins or `**`. It has no `*.domain` wildcard — + # `*.learnstack.app` matches nothing and silently blocks every tenant origin. + # Subdomain patterns go through allow_origins_by_regex, anchored at both ends. + cors: + allow_origins: "https://app.learnstack.dev" + allow_origins_by_regex: ["^https://[a-z0-9-]+\\.learnstack\\.app$"] + limit-count: { count: 5, time_window: 60, key: remote_addr, rejected_code: 429 } upstream: { type: roundrobin, nodes: { "learnstack-api:5000": 1 } } - # Dev-only; rejected by environment-gated NGINX rule in production. - # CORS preflight separation — priority 99 (just above main authenticated band) - # OPTIONS bypasses openid-connect (which would reject preflight without Authorization header). - - id: 99 - uri: /api/v*/** + # ---- CORS preflight band — priority 200 ------------------------------- + # OPTIONS carries no Authorization header; openid-connect would reject it. + - id: 20 + uri: /api/v1/* methods: [OPTIONS] + priority: 200 plugins: cors: - allow_origins: "https://app.learnstack.dev,*.learnstack.app" + allow_origins: "https://app.learnstack.dev" + allow_origins_by_regex: ["^https://[a-z0-9-]+\\.learnstack\\.app$"] allow_methods: "GET,POST,PUT,PATCH,DELETE,OPTIONS" - allow_headers: "Authorization,Content-Type,X-Tenant-Id,X-Correlation-Id" + allow_headers: "Authorization,Content-Type,X-Tenant-Id,X-Organization-Id,X-Correlation-Id" max_age: 3600 - # Authenticated band — priority 100 + # ---- Authenticated catch-all — priority 100 --------------------------- - id: 100 - uri: /api/v*/** + uri: /api/v1/* methods: [GET, POST, PUT, PATCH, DELETE] + priority: 100 plugins: openid-connect: client_id: learnstack-gateway - client_secret_ref: vault://learnstack/hub/internal-api-hmac-key # via Vault Agent sidecar discovery: http://keycloak:8080/realms/learnstack/.well-known/openid-configuration - bearer_only: true - realm: learnstack + bearer_only: true # validates a presented token; no code exchange, + realm: learnstack # therefore no client secret — see Rule 3 scope: openid set_access_token_header: true access_token_in_authorization_header: true cors: - allow_origins: "https://app.learnstack.dev,*.learnstack.app" - limit-req: { rate: 100, burst: 50, key: remote_addr } + allow_origins: "https://app.learnstack.dev" + allow_origins_by_regex: ["^https://[a-z0-9-]+\\.learnstack\\.app$"] + limit-req: { rate: 100, burst: 50, key: remote_addr, rejected_code: 429 } request-id: { include_in_response: true } upstream: { type: roundrobin, nodes: { "learnstack-api:5000": 1 } } - # Hub host band — priority 200 + # ---- Hub host — same banding, scoped by host -------------------------- - id: 200 host: hub.learnstack.dev - uri: /api/v*/** + uri: /api/v1/* methods: [OPTIONS] + priority: 200 plugins: cors: allow_origins: "https://hub.learnstack.dev" @@ -211,28 +337,27 @@ routes: - id: 201 host: hub.learnstack.dev - uri: /api/v*/** + uri: /api/v1/* methods: [GET, POST, PUT, PATCH, DELETE] + priority: 100 plugins: openid-connect: client_id: learnstack-hub-gateway discovery: http://keycloak:8080/realms/learnstack-hub/.well-known/openid-configuration bearer_only: true realm: learnstack-hub - cors: - allow_origins: "https://hub.learnstack.dev" - limit-req: { rate: 60, burst: 30, key: remote_addr } # stricter on Hub + cors: { allow_origins: "https://hub.learnstack.dev" } + limit-req: { rate: 60, burst: 30, key: remote_addr, rejected_code: 429 } # stricter on Hub request-id: { include_in_response: true } upstream: { type: roundrobin, nodes: { "learnstack-hub:5000": 1 } } - - # Hangfire dashboard — defense-in-depth: gateway BasicAuth + backend role check - - id: 300 - uri: /admin/hangfire* - plugins: - basic-auth: {} # consumer credentials in Vault - upstream: { type: roundrobin, nodes: { "learnstack-api:5000": 1 } } ``` +The corrections in Rules 1–3 land in the shipped `infra/apisix/apisix.yaml` in +[Phase 03](../roadmap/phase-03-identity-admin.md), alongside uncommenting the +`openid-connect` block against the realm that exists by then. APISIX becomes a real +gateway — TLS termination, consumer-keyed limits, WAF — in +[Phase 11](../roadmap/phase-11-production-hardening.md). + ## 5. Defense-in-depth: JWT validated twice **Why validate JWT both at APISIX and at the backend?** @@ -247,6 +372,13 @@ If APISIX is misconfigured, bypassed (someone hits the backend directly on the i network), or compromised, the backend still rejects unauthenticated requests. This is non-negotiable. +That property is what makes today's state a gap rather than a breach: with route 100's +`openid-connect` block commented out, layer 1 is absent and layer 2 carries the whole +load. The system is not open — it is running on one line of defense where the design +calls for two, and the cheap fast rejection at the edge is not happening. Restoring +layer 1 is [Phase 03](../roadmap/phase-03-identity-admin.md) work; nothing about the +backend's obligations changes when it lands. + ```csharp // LearnStack.Host/Program.cs builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme) @@ -274,15 +406,19 @@ builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme) | Surface | Rate | Burst | Key | |---------|------|-------|-----| -| `/health` | 10/s | 5 | remote_addr | -| `/api/v*/localization/*` | 50/s | 20 | remote_addr | +| `/healthz` | 10/s | 5 | remote_addr | +| `/api/v1/localization/*` | 50/s | 20 | remote_addr | +| `/api/v1/auth/*` (login, password reset) | 5/min | 0 | remote_addr | | Authenticated GET/POST/PUT/PATCH/DELETE (main API) | 100/s | 50 | remote_addr | | Hub authenticated API | 60/s | 30 | remote_addr | -| `/api/v*/auth/*` (login, password reset) | 5/min | 0 | remote_addr | -| Write endpoints (POST/PUT/PATCH/DELETE) on authenticated routes | 60/min/token | — | consumer (when consumer-key authn enabled, Phase 11) | +| Write endpoints (POST/PUT/PATCH/DELETE) on authenticated routes | 60/min/token | — | consumer (when consumer-key authn is wired, Phase 11) | -Phase 11+ moves from `remote_addr`-keyed limits to consumer-key-keyed limits when APISIX -consumer support is wired (etcd-backed mode required for dynamic consumer issuance). +`remote_addr` keying limits a noisy **client**, not a noisy **tenant**: one tenant behind +many addresses is unaffected, and many tenants behind one NAT are throttled together. +Per-tenant fairness — at the gateway and inside the database — is +[Phase 11](../roadmap/phase-11-production-hardening.md) work, described in +[25-deployment-models.md § 2](25-deployment-models.md). Consumer-key-keyed limits require +etcd-backed mode for dynamic consumer issuance and land in the same phase. ## 7. CORS preflight separation @@ -290,9 +426,11 @@ The `openid-connect` plugin **rejects OPTIONS requests** because they don't carr `Authorization` header. This breaks browser CORS preflight unless OPTIONS is handled separately. -Solution: a higher-priority OPTIONS-only route (id 99 / 200) carries the `cors` plugin -without `openid-connect`. The browser's preflight succeeds; the subsequent actual -request (with the `Authorization` header) hits the authenticated route. +Solution: an OPTIONS-only route in the **preflight band (priority 200)** carries the +`cors` plugin without `openid-connect`. Because 200 outranks the authenticated +catch-all's 100, the browser's preflight succeeds; the subsequent actual request, with +its `Authorization` header and a non-OPTIONS method, falls through to the authenticated +route. Nexora pattern verbatim (see `Nexora/docs/decisions/0003-deployment-strategy.md` and `Nexora/docs/operations/HELM_INSTALLATION.md`). @@ -351,10 +489,18 @@ Phase 11 deliverables: ## 11. Architecture tests - `Apisix_RouteYaml_IsValid` — CI step running `apisix test` against `apisix.yaml`. +- `Apisix_Routes_Declare_Explicit_Priority` — route-table lint: every route sets + `priority`; a default-`0` route is a latent ordering bug (Rule 2). +- `Apisix_Public_Routes_Outrank_Authenticated_Catchall` — route-table lint: no route + lacking `openid-connect` shares a path prefix with a higher-or-equal-priority route + that has it (Rule 2). +- `Apisix_Uri_Patterns_Are_RadixtreeValid` — route-table lint: at most one `*`, and only + as the final path segment (Rule 1). - `Apisix_NeverFronts_InternalApi` — integration test asserts requests to `app.learnstack.dev/api/internal/*` return 404 from APISIX (route not defined). - `Backend_RequiresJwt_OnAllAuthenticatedRoutes` — `WebApplicationFactory` integration test: every endpoint outside the public allow-list returns 401 without a bearer token. + This is the test that holds the line while the gateway's OIDC block is commented out. ## 12. Non-goals @@ -367,7 +513,11 @@ Phase 11 deliverables: ## References -- ADR-0015 — API Gateway with APISIX. +- ADR-0015 — API Gateway with APISIX (what). +- [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) — Demand-gated + infrastructure (when), and what carries the gateway's responsibilities until then. +- [ADR-0024](../decisions/0024-api-versioning-policy.md) — URL versioning; why the route + table enumerates versions instead of wildcarding them. - ADR-0004 Amendment 1 — `learnstack-hub` realm. - [11-security.md](../standards/11-security.md) — defense-in-depth. - [13-identity-and-auth.md](13-identity-and-auth.md) — Keycloak realm diff --git a/docs/architecture/31-audit-subsystem.md b/docs/architecture/31-audit-subsystem.md index 37ac7f6..999125e 100644 --- a/docs/architecture/31-audit-subsystem.md +++ b/docs/architecture/31-audit-subsystem.md @@ -1,42 +1,110 @@ # Audit Subsystem -**Derives from:** [ADR-0016](../decisions/0016-audit-log-subsystem.md), +**Derives from:** [ADR-0033](../decisions/0033-audit-durability-model.md) +(supersedes [ADR-0016](../decisions/0016-audit-log-subsystem.md)), [ADR-0017 (Tenant + Organization)](../decisions/0017-tenant-organization-hierarchy.md), [18-audit-coverage.md](../standards/18-audit-coverage.md). The audit subsystem captures and persists an append-only history of security- and compliance-relevant operations across every LearnStack module. This document describes -the three-piece pipeline, data model, retention, redaction, and operational concerns. - -## 1. Pipeline overview +the pipeline, the durability model, the data model, retention, redaction, and operational +concerns. + +## 1. Two durability classes + +The single most important thing to understand about this subsystem is that **audit is not +one mechanism**. ADR-0016 treated it as one and inherited a contradiction: Standards 18 +required MUST-class rows to be written in the same transaction as the change they record, +while ADR-0016 required that audit never block business logic. Under the shipped MediatR +order — `Validation → Logging → AuditLog → TenantContext → Authorization → Transaction → +OutboxFlush → Handler` — `AuditLogBehavior` wraps `TransactionBehavior` from the outside, +so its write lands **after** the business transaction has already committed or rolled +back. Both requirements could not hold. + +Worse, once the corrected Row Level Security template from +[ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) lands, an +audit insert executed outside that transaction runs with no `app.tenant_id` set. The +policy's `WITH CHECK` rejects the row, and a catch-and-log posture swallows the +rejection: the audit log would record nothing while reporting success. A silent, complete +audit failure is strictly worse than a loud one. + +[ADR-0033](../decisions/0033-audit-durability-model.md) resolves it by splitting the +classes rather than reordering the pipeline: + +| Class | Where the row is written | On failure | Rationale | +|---|---|---|---| +| **MUST**, with a business transaction — security, compliance, privileged access | **Inside the business transaction**, as one parameterised `INSERT` issued by `IAuditStore.WritePendingAsync` immediately before `COMMIT` | **Fail closed** — the transaction rolls back and the caller receives `503 audit_unavailable` | For these events the audit row *is* part of the operation's contract. "Platform admin read tenant B's learner records" with no audit row is an audit finding | +| **MUST**, with no committed business transaction — `denied` outcomes, read-sensitive queries, non-mutating security events, **and any request whose transaction rolled back or whose commit outcome is unknown** | **Standalone**, through `IAuditStore.WriteStandaloneAsync`, on a connection outside the business transaction: `BEGIN; SET LOCAL app.tenant_id; INSERT; COMMIT` | **Fail closed** — the caller receives `503 audit_unavailable` instead of the original result, never a propagated exception | There is no business transaction to ride, or the one that existed is gone. The row must still satisfy `audit_log`'s `WITH CHECK`, so it sets the GUC on its own terms. Reusing the *business* connection here would be a defect: a row written inside a transaction that is about to roll back rolls back with it | +| **SHOULD / MAY** — operational, diagnostic | Outside any business transaction, best-effort, same standalone shape | Logged and dropped; the accepted loss is written down in the module's matrix, not assumed | Losing "course renamed" costs a support conversation | + +The single most important consequence: **"written" is not "committed".** The +in-transaction `INSERT` at step 6 becomes durable only when `COMMIT` returns. Between the +two, a constraint violation, a lost connection, or — far more commonly — a handler that +calls `SaveChanges` and then returns `Result.Fail(...)` takes the audit row away with the +business row. A per-request "consumed" flag cannot observe that: the flag lives in a DI +scope and a database rollback does not touch it. `TransactionBehavior` therefore reports +the commit boundary explicitly — `Committed`, `RolledBack` or `Indeterminate` — and +`AuditLogBehavior` re-writes the row standalone for anything that is not `Committed`. + +Redaction, projection and external fan-out happen after the commit, reading the committed +row; none of them updates it. ADR-0016's "audit never blocks business logic" is preserved +for that second stage and withdrawn for the first. + +## 2. Pipeline overview ```mermaid -flowchart LR - Cmd["Command / Query / Action"] --> Behavior["AuditLogBehavior
(MediatR pipeline)"] - Behavior --> Handler["Module handler
(business logic)"] - Handler --> SaveChanges["DbContext.SaveChangesAsync"] - SaveChanges --> Interceptor["AuditChangeTrackerInterceptor
(EF SaveChangesInterceptor)"] - Interceptor --> Buffer["IAuditStateCapture
(scoped buffer)"] - Handler --> ReturnsResult["Handler returns Result"] - ReturnsResult --> Behavior - Behavior --> Buffer - Behavior --> Build["Build AuditEntry"] - Build --> Store["IAuditStore.SaveAsync"] - Store --> Table[("audit_log
(partitioned by month)")] +flowchart TB + Cmd["Command / Query / Action"] --> Behavior["AuditLogBehavior (step 3)
DECIDE"] + Behavior --> Config["IAuditConfigService.ClassifyAsync
in-process catalogue + cached audit_config
(no request-path DB read)"] + Config -->|"not in the catalogue"| Closed["REJECT
audit_unclassified_operation"] + Config --> Intent["MUST-class: park a pending intent
in IAuditStateCapture (no DbContext)"] + Intent --> Tx["TransactionBehavior (step 6)
BEGIN; SET LOCAL app.tenant_id"] + Tx --> Handler["Handler + OutboxFlush
DbContext.SaveChangesAsync (1..n)"] + Handler --> Capture["AuditChangeTrackerInterceptor
snapshots the ChangeTracker into
IAuditStateCapture (writes nothing)"] + Capture --> Write["WRITE — IAuditStore.WritePendingAsync
one INSERT on the ambient transaction"] + Write --> Commit[("COMMIT — business rows + MUST audit row,
atomically, with app.tenant_id set")] + Commit -->|"CommitAsync returned"| Ok["state := Committed"] + Commit -->|"rolled back / commit faulted"| NotOk["state := RolledBack | Indeterminate"] + Ok --> Recon["RECONCILE — AuditLogBehavior, on the way out"] + NotOk --> Recon + Recon -->|"state = Committed"| Done["nothing to do — the row is durable"] + Recon -->|"anything else, MUST"| Standalone["IAuditStore.WriteStandaloneAsync
own transaction, real outcome"] + Recon -->|"SHOULD / MAY"| Best["IAuditStore.WriteBestEffortAsync"] ``` -Three pieces, separated concerns: - -1. **`AuditChangeTrackerInterceptor`** — runs inside `DbContext.SaveChangesAsync`, - walks the ChangeTracker, snapshots state for every entity inheriting `AuditableEntity`. -2. **`IAuditStateCapture`** — a scoped (per-request) buffer that holds entity snapshots - until the MediatR behavior reads them. -3. **`AuditLogBehavior`** — wraps the handler, awaits it (catching - exceptions to still audit failed operations), reads the buffer, builds one `AuditEntry` - per request, writes via `IAuditStore.SaveAsync`. Failure to write never blocks the - business response. - -## 2. The interceptor +Read as text — **decide → write → reconcile**. At step 3 the behavior classifies the +operation from in-process state and, for MUST, parks a pending intent in the scoped +`IAuditStateCapture`; it opens no transaction and touches no `DbContext`. The handler +runs inside the transaction `TransactionBehavior` opened, which issued +`SET LOCAL app.tenant_id` as its first statement; the interceptor snapshots each flush's +ChangeTracker into the same buffer and writes nothing. Immediately before `COMMIT`, +`TransactionBehavior` calls `IAuditStore.WritePendingAsync`, which composes the complete +row and inserts it on that transaction — so it commits with the business write or not at +all, and Row Level Security accepts it. `TransactionBehavior` then records the commit +boundary. On the way out, the behavior reconciles: `Committed` means there is nothing to +do, and anything else means the row is re-written standalone with the real outcome. +SHOULD/MAY rows and all fan-out are written on that same outbound pass, best-effort. + +Four components, separated concerns: + +1. **`AuditChangeTrackerInterceptor`** — runs inside `DbContext.SaveChangesAsync`, walks + the ChangeTracker, snapshots state for every entity inheriting `AuditableEntity` + into `IAuditStateCapture`. It **never** constructs an `AuditEntry` and never inserts + one. Making it the writer would work in EF Core terms but would leave two questions + unanswerable: which of several flushes in one transaction owns the row, and how the + audit type gets mapped into every module's `DbContext` without inverting the + dependency direction (see § 7 and [ADR-0033 § Implementation Notes](../decisions/0033-audit-durability-model.md)). +2. **`IAuditStateCapture`** — the scoped (per-request) audit state: the entity snapshots, + the pending MUST-class intent, and the intent's lifecycle state. +3. **`TransactionBehavior`** — owns the commit boundary, and therefore owns both the + durable audit write (immediately before `COMMIT`) and the `Committed` / `RolledBack` / + `Indeterminate` signal the reconcile step reads. +4. **`AuditLogBehavior`** — keeps its shipped position and its + shipped exception responsibility: catch handler exceptions, record the outcome, + rethrow via `ExceptionDispatchInfo`. It decides on the way in and reconciles on the + way out; it no longer writes the MUST-class row itself except in the standalone case. + +## 3. The interceptor ```csharp namespace LearnStack.Infrastructure.Audit; @@ -86,24 +154,56 @@ public sealed class AuditChangeTrackerInterceptor : ISaveChangesInterceptor } ``` +The interceptor's job is **capture only**. It returns the unmodified +`InterceptionResult`, adds nothing to the context, and issues no SQL. It runs once per +flush, and a MUST-class command may flush more than once inside one transaction — which +is precisely why the audit row is not built here. `TransactionBehavior` composes it once, +after the last flush and before `COMMIT`, so the snapshots are complete regardless of how +many times the handler saved. + Pattern explicitly mirrors Nexora's `AuditChangeTrackerInterceptor` (see `Nexora/docs/modules/tier-1-core/audit/SPEC.md` and `Nexora/docs/decisions/0009-audit-repository-pattern.md`) — verbatim port to LearnStack naming. -## 3. The scoped buffer +## 4. The scoped buffer ```csharp namespace LearnStack.SharedKernel.Abstractions.Audit; +public enum AuditIntentState +{ + None, // no MUST-class intent for this request + Pending, // declared at step 3; nothing written yet + WrittenInTransaction, // INSERTed on the ambient transaction — NOT yet durable + Committed, // the ambient transaction committed; the row is durable + RolledBack, // the ambient transaction rolled back; the row is gone + Indeterminate, // COMMIT faulted; the server-side outcome is unknown +} + public interface IAuditStateCapture { IReadOnlyList Changes { get; } void Add(CapturedEntityChange change); + + // The MUST-class intent and its lifecycle. Exactly one intent per request. + AuditIntent? Intent { get; } + AuditIntentState State { get; } + + void Declare(AuditIntent intent); // AuditLogBehavior, step 3 + void MarkWrittenInTransaction(); // IAuditStore.WritePendingAsync + void MarkCommitted(); // TransactionBehavior, after CommitAsync + void MarkRolledBack(); // TransactionBehavior, after RollbackAsync + void MarkIndeterminate(Exception cause);// TransactionBehavior, CommitAsync faulted + void Clear(); } ``` +`State` is the only durability signal in the system, and it is deliberately **not** +a "consumed" flag. `WrittenInTransaction` is not durable; only `Committed` is. This +interface is a `SharedKernel` abstraction and names no EF Core type. + ```csharp namespace LearnStack.Infrastructure.Audit; @@ -117,9 +217,13 @@ public sealed class AuditStateCapture : IAuditStateCapture ``` Registered as **scoped** in DI (per-request lifetime). Cleared at the end of every -request to prevent cross-request bleed (architecture test enforces this). +request to prevent cross-request bleed +([`AuditStateCapture_ClearedPerRequest`](../standards/21-architecture-tests-catalogue.md) +enforces this). Because the lifetime is the DI scope and not the database transaction, a +rollback leaves every field of this object intact — which is exactly why `State` must be +set by the component that owns the commit, and never inferred. -## 4. The MediatR behavior +## 5. The MediatR behavior ```csharp namespace LearnStack.Infrastructure.Behaviors; @@ -130,9 +234,11 @@ public sealed class AuditLogBehavior( IAuditStore auditStore, IAuditStateCapture stateCapture, ITenantContextAccessor tenantAccessor, + IClock clock, ILogger> logger) : IPipelineBehavior - where TRequest : IRequest + where TRequest : notnull + where TResponse : IResultBase { public async Task Handle(TRequest request, RequestHandlerDelegate next, CancellationToken ct) @@ -141,101 +247,146 @@ public sealed class AuditLogBehavior( if (requestKind == RequestKind.Other) return await next(); var (module, operation) = ExtractModuleAndOperation(request, requestKind); - var defaultEnabled = requestKind == RequestKind.Command; - - bool auditEnabled; - try - { - auditEnabled = await configService.IsEnabledAsync(module, operation, ct, defaultEnabled); - } - catch (Exception ex) - { - // Audit config check failed → skip audit; never block business write. - logger.LogError(ex, "Audit config check failed for {Module}.{Operation}", - module, operation); - return await next(); - } - if (!auditEnabled) return await next(); + // DECIDE. ClassifyAsync reads the in-process catalogue plus the tenant's cached + // audit_config overrides. It issues NO query on the request path, and that is a + // correctness requirement, not an optimisation: at step 3 no transaction is open, + // app.tenant_id is unset, and audit_config carries ENABLE + FORCE row level + // security (§ 7). A read here would return ZERO ROWS SILENTLY — indistinguishable + // from "this tenant has no overrides" — so no catch could ever fire. On a cache + // miss the loader opens its OWN short transaction and sets app.tenant_id itself. + var classification = await configService.ClassifyAsync(module, operation, ct); + + // The catalogue is in-process and cannot be unavailable, so "proceeding + // unaudited" is impossible by construction. What can happen is an operation + // nobody classified — that is rejected, loudly. + if (classification == AuditClassification.Unclassified) + return Result.FailFor(AuditErrors.UnclassifiedOperation); + + if (classification == AuditClassification.Off) return await next(); + + // MUST-class: declare the intent. The id is minted here so the in-transaction row + // and any standalone replacement carry the same identity. Nothing is written yet + // and no DbContext is touched. + if (classification == AuditClassification.Must) + stateCapture.Declare(new AuditIntent( + AuditEntryId: AuditEntryId.New(), + Module: module, + Operation: operation, + OperationType: DeriveOperationType(operation), + OperationClass: OperationClass.Must, + DeclaredAt: clock.UtcNow)); TResponse response; - bool handlerFailed = false; Exception? handlerException = null; try { response = await next(); } - catch (Exception ex) + catch (Exception ex) when (ex is not OperationCanceledException) { - handlerFailed = true; handlerException = ex; response = default!; } + // RECONCILE. TransactionBehavior already wrote the MUST-class row on the ambient + // transaction and already reported the commit boundary. The only question left is + // whether that transaction COMMITTED — "written" is not "committed", and a + // per-request flag cannot observe a rollback. try { - var (isSuccess, errorKey) = handlerFailed - ? (false, (string?)"audit.handler_exception") - : DetermineOutcome(response); - - var entry = new AuditEntry( - Id: AuditEntryId.New(), - TenantId: tenantAccessor.Current?.TenantId ?? Guid.Empty, - OrganizationId: tenantAccessor.Current?.OrganizationId, - Module: module, - Operation: operation, - OperationType: DeriveOperationType(operation), - OperationClass: DeriveOperationClass(module, operation), - ActorUserId: auditContext.UserId, - ActorEmail: auditContext.UserEmail, - CorrelationId: auditContext.CorrelationId, - IpAddress: auditContext.IpAddress, - UserAgent: auditContext.UserAgent, - IsSuccess: isSuccess, - ErrorKey: errorKey, - EntityType: stateCapture.Changes.Count == 1 ? stateCapture.Changes[0].EntityType : null, - EntityId: stateCapture.Changes.Count == 1 ? stateCapture.Changes[0].EntityId : null, - BeforeState: SerializeBefore(stateCapture.Changes), - AfterState: SerializeAfter(stateCapture.Changes), - Changes: SerializeChanges(stateCapture.Changes), - Timestamp: DateTimeOffset.UtcNow); - - await auditStore.SaveAsync(entry, ct); + if (stateCapture.Intent is { } intent) + { + if (stateCapture.State != AuditIntentState.Committed) + await auditStore.WriteStandaloneAsync( + BuildDraft(intent, response, handlerException, stateCapture), ct); + } + else + { + await auditStore.WriteBestEffortAsync( + BuildDraft(module, operation, classification, response, + handlerException, stateCapture), ct); + } + } + catch (Exception ex) when (classification != AuditClassification.Must) + { + // SHOULD/MAY only: log and drop. The accepted loss is written down in the + // module's audit-coverage matrix, not assumed. + logger.LogError(ex, "Best-effort audit save failed for {Module}.{Operation}", + module, operation); } catch (Exception ex) { - // Audit save failed → log, never block business write. - logger.LogError(ex, "Audit save failed for {Module}.{Operation}", module, operation); + // MUST-class, and even the standalone write failed. End of the line: the + // platform cannot reach PostgreSQL at all. Report loudly and return + // audit_unavailable — never a silent success. + logger.LogCritical(ex, + "MUST-class audit could not be written for {Module}.{Operation}; rejecting", + module, operation); + return Result.FailFor(AuditErrors.Unavailable); } finally { stateCapture.Clear(); } - if (handlerFailed) - ExceptionDispatchInfo.Capture(handlerException!).Throw(); + if (handlerException is not null) + ExceptionDispatchInfo.Capture(handlerException).Throw(); return response; } - // ClassifyRequest, ExtractModuleAndOperation, DetermineOutcome, DeriveOperationType, - // DeriveOperationClass, SerializeBefore/After/Changes are private static helpers. + // ClassifyRequest, ExtractModuleAndOperation, DeriveOperationType and the two + // BuildDraft overloads are private helpers. BuildDraft resolves the outcome: + // Denied — the Result carries `forbidden` + // Failed — any other failure Result, or a handler exception, or + // stateCapture.State == RolledBack + // Indeterminate — stateCapture.State == Indeterminate + // Success — otherwise + // and fills tenant / organization from ITenantContextAccessor, actor + correlation + // from IAuditContext, and the snapshots from stateCapture.Changes. } ``` Key invariants enforced by this behavior: -- **Audit failure never blocks business write.** The `try { auditStore.SaveAsync(...) } - catch { log }` and the surrounding `finally { stateCapture.Clear(); }` guarantee that - even a totally broken audit store doesn't reject business commands. +- **Only `Committed` counts.** The reconcile step branches on + `IAuditStateCapture.State`, never on a "consumed" flag. `WrittenInTransaction`, + `RolledBack` and `Indeterminate` all produce a standalone row. A MUST-class row that + was inserted and then rolled back is re-written with outcome `failed`, so a rolled-back + privileged operation is still on the record. +- **An unclassified operation is rejected.** The in-process catalogue cannot be + unavailable, so "proceeding unaudited" is not a reachable state; what is reachable is an + operation nobody classified, and that fails with `audit_unclassified_operation`. +- **A tenant-override read failure does not reject.** Classification falls back to the + in-process catalogue, which carries the MUST floor, and the failure is logged at `Error` + and surfaced on the audit health check. Rejecting every request platform-wide because a + cache is unavailable is a worse compliance outcome than losing one tenant's voluntary + SHOULD→MUST elevation; the property ADR-0016 lost — silently switching auditing *off* — + is impossible here either way. +- **MUST-class audit failure fails the operation.** The durable write throws and + `TransactionBehavior` rolls back; if even the standalone write fails, the unfiltered + `catch` returns `audit_unavailable` (HTTP 503). This is a real availability trade-off, + stated in [ADR-0033 § Consequences](../decisions/0033-audit-durability-model.md) and + required to be visible in the operational runbooks. +- **SHOULD/MAY audit failure never blocks the business write.** The cheap path stays + cheap; the platform does not pay compliance-grade cost for "a course was renamed". - **Failed handlers still get audited.** The behavior catches the handler exception, - writes an audit entry with `IsSuccess=false`, then re-throws via `ExceptionDispatchInfo` - to preserve the original stack trace. -- **Per-(module, operation) toggling.** `IAuditConfigService.IsEnabledAsync` reads from - per-tenant config (`audit_config` table) with module-declared defaults. - -## 5. Pipeline order + writes the `failed` outcome through the standalone path — the business transaction has + rolled back, so there is no transaction left to ride — then rethrows via + `ExceptionDispatchInfo` so the original stack trace survives. +- **`Indeterminate` prefers a duplicate to a loss.** When `CommitAsync` faults, the row's + fate is genuinely unknown, so the standalone row is written anyway, carrying the same + `AuditEntryId` as the in-transaction attempt and outcome `indeterminate`. Two rows with + one id is the recorded signature of a commit-in-doubt event; the audit read model groups + by id and flags it. Losing the audit for a possibly-committed privileged operation is + the failure this whole ADR exists to prevent. +- **A tenant override cannot remove MUST coverage.** `IAuditConfigService.ClassifyAsync` + applies the per-tenant `audit_config` override and then re-applies the catalogue's MUST + floor. A tenant may audit *more* than the baseline, never less. + +## 6. Pipeline order MediatR pipeline behaviors are registered in this order in `LearnStack.Infrastructure.DependencyInjection`: @@ -244,7 +395,7 @@ MediatR pipeline behaviors are registered in this order in services.AddTransient(typeof(IPipelineBehavior<,>), typeof(ValidationBehavior<,>)); services.AddTransient(typeof(IPipelineBehavior<,>), typeof(LoggingBehavior<,>)); services.AddTransient(typeof(IPipelineBehavior<,>), typeof(AuditLogBehavior<,>)); -services.AddTransient(typeof(IPipelineBehavior<,>), typeof(TenantContextCheckBehavior<,>)); +services.AddTransient(typeof(IPipelineBehavior<,>), typeof(TenantContextBehavior<,>)); services.AddTransient(typeof(IPipelineBehavior<,>), typeof(AuthorizationBehavior<,>)); services.AddTransient(typeof(IPipelineBehavior<,>), typeof(TransactionBehavior<,>)); services.AddTransient(typeof(IPipelineBehavior<,>), typeof(OutboxFlushBehavior<,>)); @@ -256,15 +407,88 @@ Effective execution order (outer → inner): Request → Validation (reject before any work; FluentValidation) → Logging (request scope; correlation id) - → AuditLog (wraps the rest; audit failure never blocks) - → TenantContextCheck (assert tenant_id resolved) + → AuditLog (classifies; rejects an unclassified operation) + → TenantContext (assert tenant_id resolved) → Authorization (resource-scoped checks beyond endpoint-level [Authorize]) - → Transaction (begin transaction; commit/rollback on outcome) + → Transaction (begin transaction; SET LOCAL app.tenant_id; commit/rollback) → OutboxFlush (flush outbox writes to DbContext before commit) → Handler (business logic) ``` -## 6. Data model +**The order did not change, and does not need to.** `AuditLogBehavior` still sits outside +`TransactionBehavior`, which is exactly why its *own* write cannot be the durable one. The +durable MUST-class row is written by `TransactionBehavior` — the behavior that owns the +commit boundary — from the intent the outer behavior declared. The decision travels inward +through the pipeline; the commit outcome travels back out. + +```csharp +// LearnStack.Application.Pipeline.TransactionBehavior — step 6. The commit boundary owns +// the durable audit write AND the durability signal, because they are the same fact. +public async Task Handle(TRequest request, + RequestHandlerDelegate next, CancellationToken ct) +{ + if (!RequiresTransaction(request)) return await next(); + + await unitOfWork.BeginTransactionAsync(ct); + // First statement inside the transaction, per ADR-0003 Amendment 3. + await unitOfWork.SetTenantContextAsync(tenantContext, ct); + + try + { + var response = await next(); + + if (!response.IsSuccess) + { + await unitOfWork.RollbackAsync(CancellationToken.None); + stateCapture.MarkRolledBack(); + return response; // audited standalone, outcome = failed + } + + // WRITE. No-op unless a MUST-class intent is pending. Throws on failure, which + // reaches the catch below and rolls the business write back — fail closed. + await auditStore.WritePendingAsync(unitOfWork, ct); + + try + { + await unitOfWork.CommitAsync(ct); + stateCapture.MarkCommitted(); // the ONLY place durability is claimed + } + catch (Exception ex) + { + // A faulted COMMIT leaves the server-side outcome genuinely unknown. + stateCapture.MarkIndeterminate(ex); + throw; + } + + return response; + } + catch (Exception ex) when (stateCapture.State != AuditIntentState.Indeterminate) + { + await unitOfWork.RollbackAsync(CancellationToken.None); + stateCapture.MarkRolledBack(); + + if (ex is AuditWriteFailedException) + return Result.FailFor(AuditErrors.Unavailable); + + throw; + } +} +``` + +`IUnitOfWork` is the seam that lets this generic behavior open and commit a transaction +without naming any module's `DbContext`, and through which `IAuditStore` reaches the +ambient connection. It is a [Phase 02a Packet 6](../roadmap/phase-02a-kernel-tenancy.md) +deliverable that the shipped `TransactionBehavior` shell already presumes. + +The alternative — moving `TransactionBehavior` outward so it wraps `AuditLogBehavior` — +was considered and rejected in +[ADR-0033 § Considered Options](../decisions/0033-audit-durability-model.md): it would +also drag `TenantContext` and `Authorization` inside the transaction and open the +transaction before validation has finished, changing a shipped, test-asserted global +ordering (`MediatR_Pipeline_Order_Matches_Canonical_Sequence`) to solve a problem +belonging to one behavior. + +## 7. Data model ### `AuditEntry` aggregate @@ -325,6 +549,29 @@ public enum OperationClass { Must, Should, May } ### `audit_log` table +`audit_log` ships in [Phase 02a Packet 9](../roadmap/phase-02a-kernel-tenancy.md) as a +**single, plain, correct table**. Monthly partitioning, the partition-management job, and +the retention purge from [ADR-0028](../decisions/0028-audit-log-partition-management.md) +move to [Phase 11](../roadmap/phase-11-production-hardening.md) per +[ADR-0035](../decisions/0035-demand-gated-infrastructure.md), against the trigger +"measured `audit_log` growth justifies partition maintenance". Audit **correctness** +cannot be added later; audit **scale** can, and the platform has no rows yet to scale. + +Two details that a partition-ready design gets wrong if it is copied carelessly: + +- The primary key is the **composite** `(id, timestamp)`. ADR-0016's DDL declared a + primary key twice — inline on `id` and again as a table constraint on `(id, timestamp)` + — and PostgreSQL rejects that table outright. When partitioning arrives, a partitioned + table must include every partition-key column in its primary key, so the composite is + the correct one and the inline declaration was the error. Shipping the composite now + means Phase 11 adds `PARTITION BY RANGE (timestamp)` without a key migration. +- Cross-tenant reads use **`learnstack_platform`**, entered through the audited + `EnterPlatformAdminScope(reason)` path. There is no separate `learnstack_audit_admin` + role: the database role model fixed by + [ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) has + exactly four roles, and every additional `BYPASSRLS` role is a hole in the isolation + model that would need its own ADR. + ```sql CREATE TABLE audit_log ( id uuid NOT NULL, @@ -338,8 +585,9 @@ CREATE TABLE audit_log ( operation_class text NOT NULL, entity_type text NULL, entity_id text NULL, - is_success boolean NOT NULL, + outcome text NOT NULL, -- 'success' | 'denied' | 'failed' error_key text NULL, + reason text NULL, -- EnterPlatformAdminScope(reason), denial cause before_state jsonb NULL, after_state jsonb NULL, changes jsonb NULL, @@ -348,14 +596,15 @@ CREATE TABLE audit_log ( user_agent text NULL, timestamp timestamptz NOT NULL DEFAULT now(), metadata jsonb NULL, - PRIMARY KEY (id, timestamp) -) PARTITION BY RANGE (timestamp); - -CREATE TABLE audit_log_2026_05 PARTITION OF audit_log - FOR VALUES FROM ('2026-05-01') TO ('2026-06-01'); -CREATE TABLE audit_log_2026_06 PARTITION OF audit_log - FOR VALUES FROM ('2026-06-01') TO ('2026-07-01'); --- ... auto-created monthly by a Hangfire recurring job (LearnStackJob) + CONSTRAINT audit_log_pkey PRIMARY KEY (id, timestamp) +); +-- Phase 11 does NOT alter this table in place: PostgreSQL has no +-- ALTER TABLE ... PARTITION BY. It creates a partitioned parent, attaches this +-- table to it, and recreates the indexes and the policy on the parent, under a +-- lock. The composite key above is what keeps that a data operation rather than +-- a key migration (ADR-0033 § Corrected audit_log DDL). +-- The composite key above is already partition-compatible, so that change is +-- additive rather than a key migration. CREATE INDEX ix_audit_log_tenant_timestamp ON audit_log (tenant_id, timestamp DESC); @@ -368,13 +617,102 @@ CREATE INDEX ix_audit_log_correlation CREATE INDEX ix_audit_log_module_operation_timestamp ON audit_log (module, operation, timestamp DESC); --- RLS +-- RLS: built from the canonical template in Database Standards § Tenant-Owned and +-- Organization-Scoped Tables — one AND-ed policy, ENABLE *and* FORCE, explicit +-- WITH CHECK. Do not hand-write it here; the template is the single source of truth. ALTER TABLE audit_log ENABLE ROW LEVEL SECURITY; -CREATE POLICY audit_log_tenant_isolation ON audit_log - USING (tenant_id = current_setting('app.tenant_id', true)::uuid); --- Platform admin role bypasses RLS via SET role learnstack_audit_admin (audited). +ALTER TABLE audit_log FORCE ROW LEVEL SECURITY; +CREATE POLICY audit_log_isolation ON audit_log + USING (tenant_id = NULLIF(current_setting('app.tenant_id', true), '')::uuid) + WITH CHECK (tenant_id = NULLIF(current_setting('app.tenant_id', true), '')::uuid); +-- Cross-tenant reads run as learnstack_platform, entered through the audited +-- EnterPlatformAdminScope(reason) path. +``` + +The `WITH CHECK` clause is the reason a MUST-class audit row must be written either inside +the business transaction or inside a short transaction that sets the GUC itself. With +neither, `app.tenant_id` is unset or reset to `''`, `NULLIF(current_setting(…), '')` +yields `NULL`, the predicate is false, and the insert is rejected — which the old +catch-and-log posture would have swallowed. Note honestly what this clause does and does +not buy: because the standalone writer derives both the GUC and the row's `tenant_id` from +the same `ITenantContext`, `WITH CHECK` is vacuous for that write. The guard that matters +is that `tenant_id` on an audit row comes from `ITenantContext` and **never** from the +request payload. See [Database Standards](../standards/05-database.md) for the template +and [ADR-0033](../decisions/0033-audit-durability-model.md) for the durability rule. + +Platform-scope events with no resolved tenant (provisioning, Hub-operator actions) are +written with the reserved nil UUID `00000000-0000-0000-0000-000000000000` as `tenant_id`, +and the standalone writer sets `app.tenant_id` to the same value. No tenant may ever be +provisioned with the nil UUID, so those rows are invisible to every tenant policy and +readable only through `learnstack_platform`. + +### Append-only enforcement + +Append-only is enforced by **privilege first, trigger second** — not by convention and +not by an architecture test alone. + +```sql +-- The runtime role may only add rows and read them back. +REVOKE UPDATE, DELETE ON audit_log FROM learnstack_app; + +-- Exactly two mutating paths exist. Both are owned by the Audit module and both run as +-- learnstack_platform through the audited EnterPlatformAdminScope(reason) path: +-- 1. GDPR redaction — UPDATE, restricted to the redactable columns (§ 10). +-- 2. Retention purge — DELETE of rows past retention (§ 9). After Phase 11 +-- partitioning this becomes DETACH + DROP PARTITION and issues no DELETE at all. +CREATE OR REPLACE FUNCTION audit_log_append_only() +RETURNS trigger +LANGUAGE plpgsql +AS $$ +BEGIN + IF current_user <> 'learnstack_platform' THEN + RAISE EXCEPTION 'audit_log is append-only (attempted % as %)', TG_OP, current_user + USING ERRCODE = 'insufficient_privilege'; + END IF; + + IF TG_OP = 'DELETE' THEN + RETURN OLD; -- allow the purge; returning NULL here would cancel it + END IF; + + -- UPDATE: every column except the six redactable ones must be unchanged. Expressed + -- as a jsonb difference rather than a column list so the guard survives every future + -- column addition — including is_success -> outcome — without an edit here. + IF (to_jsonb(NEW) - 'actor_email' - 'ip_address' - 'user_agent' + - 'before_state' - 'after_state' - 'changes') + IS DISTINCT FROM + (to_jsonb(OLD) - 'actor_email' - 'ip_address' - 'user_agent' + - 'before_state' - 'after_state' - 'changes') + THEN + RAISE EXCEPTION 'audit_log UPDATE may only redact actor_email, ip_address, user_agent, before_state, after_state, changes' + USING ERRCODE = 'insufficient_privilege'; + END IF; + + RETURN NEW; +END; +$$; + +CREATE TRIGGER audit_log_append_only_guard + BEFORE UPDATE OR DELETE ON audit_log + FOR EACH ROW EXECUTE FUNCTION audit_log_append_only(); ``` +Three properties worth stating, because a careless copy loses each of them: + +- **`actor_user_id` is deliberately immutable.** Once the `users` row is erased it is an + orphan surrogate key with no path back to a natural person, which is what keeps the + audit row's existence auditable after erasure. Redacting it would collapse every erased + user's history into one indistinguishable bucket and make the probe-detection queries + [Audit Coverage Standards](../standards/18-audit-coverage.md) justifies the whole + `denied` class with unanswerable. +- **`BEFORE` row triggers on partitioned tables are supported from PostgreSQL 13**, and + LearnStack runs 18+ ([ADR-0031](../decisions/0031-postgresql-major-version.md)). The + trigger is inherited by partitions created later, so Phase 11 partitioning remains + additive — no re-creation, no gap. +- **The trigger is the second layer, not the first.** `learnstack_app` holds no `UPDATE` + or `DELETE` privilege at all, so the ordinary path fails with `42501` before the trigger + is reached. The trigger's job is to constrain `learnstack_platform`, the one role that + can mutate. + ### `audit_config` table ```sql @@ -389,14 +727,24 @@ CREATE TABLE audit_config ( UNIQUE (tenant_id, module, operation) ); ALTER TABLE audit_config ENABLE ROW LEVEL SECURITY; -CREATE POLICY audit_config_tenant_isolation ON audit_config - USING (tenant_id = current_setting('app.tenant_id', true)::uuid); +ALTER TABLE audit_config FORCE ROW LEVEL SECURITY; +CREATE POLICY audit_config_isolation ON audit_config + USING (tenant_id = NULLIF(current_setting('app.tenant_id', true), '')::uuid) + WITH CHECK (tenant_id = NULLIF(current_setting('app.tenant_id', true), '')::uuid); ``` Defaults declared in each module via `IModule.RegisterAuditDefaults()`; the table holds per-tenant overrides only. -## 7. Per-module coverage matrix (baseline) +`is_enabled` is deliberately **not** the whole story. A row here can narrow SHOULD/MAY +coverage; it cannot switch off an operation the catalogue classifies MUST. +`ClassifyAsync` applies the override and then re-applies the MUST floor, and a read +failure against this table falls back to the in-process catalogue — which carries that +same MUST floor — logged at `Error` and surfaced on the audit health check, rather than +rejecting the operation. Rejecting would turn a cache outage into a platform-wide denial +of service; see [§ 5](#5-the-mediatr-behavior), which is the authority. + +## 8. Per-module coverage matrix (baseline) Standard 18 defines the MUST / SHOULD / MAY matrix per module. Excerpt for Tier-1 core modules: @@ -417,7 +765,7 @@ modules: Hub-side modules audit to **Hub's own audit stream**, separate from LearnStack's; same shape, different table. -## 8. Retention +## 9. Retention Default retention by operation class (per-tenant overridable within plan limits): @@ -429,66 +777,102 @@ Default retention by operation class (per-tenant overridable within plan limits) | ReadSensitive | **2 years** | 6 months – 5 years | | Other Action | **1 year** | 3 months – 3 years | -Retention purge runs as a Hangfire recurring job (`LearnStackJob` analog): +Both retention jobs run **daily**. Three documents previously disagreed — daily in +[Audit Coverage Standards](../standards/18-audit-coverage.md) and +[ADR-0028](../decisions/0028-audit-log-partition-management.md), weekly here — and this +document was the outlier. Daily is correct: a weekly purge means a tenant's stated +retention can be exceeded by up to six days, which is a compliance answer nobody wants to +give. -- `learnstack:audit:partition-management` — runs daily; creates next month's partition - (if not exists); drops partitions older than max retention window across all tenants - (10y for safety; tenant-specific retention enforced by row-level deletes within - partition). -- `learnstack:audit:retention-purge` — runs weekly; deletes individual rows per tenant's - configured retention. Uses tenant-config retention values; bypasses RLS via - `learnstack_audit_admin` role; the purge itself emits a `SecurityEvent` audit row - summarising what was deleted. +Both jobs land in [Phase 11](../roadmap/phase-11-production-hardening.md) alongside +partitioning; until then `audit_log` is a single table and nothing purges it. -## 9. GDPR / PII redaction +| Hangfire recurring job | Cadence | What it does | +|---|---|---| +| `learnstack:audit:partition-management` | Daily | Creates next month's partition if absent; drops partitions older than the maximum retention window across all tenants (10y for safety) | +| `learnstack:audit:retention-purge` | **Daily** | Deletes individual rows per the tenant's configured retention, in batches. Runs as `learnstack_platform` through the audited platform-admin scope. The purge itself emits a `SecurityEvent` audit row summarising what was deleted | + +## 10. GDPR / PII redaction When a user is GDPR-erased (`UserGdprDeletedIntegrationEvent` published), audit rows containing that user's PII are **redacted in place** (not deleted — the audit row's existence must remain auditable): +Redaction is one of exactly **two** sanctioned mutations of `audit_log` (the other is the +retention purge, § 9). It is not an exception carved out of the append-only rule by +convention — it is the shape the `audit_log_append_only_guard` trigger in § 7 was written +to permit, and nothing else. + ```csharp // LearnStack.Modules.Audit.Infrastructure.IntegrationEvents public sealed class UserGdprDeletedIntegrationEventHandler( - AuditDbContext db, ILogger logger) + AuditDbContext db, + IPlatformAdminScope platformScope, + IAuditStore auditStore, + IInboxGuard inboxGuard, + IEnumerable userReferenceLocators) : IIntegrationEventHandler { public async Task HandleAsync(UserGdprDeletedIntegrationEvent @event, CancellationToken ct) { - // 1. Idempotent: inbox guard - if (await _inboxGuard.IsAlreadyProcessedAsync(@event.EventId, ct)) return; - - // 2. Redact rows where actor_user_id == @event.UserId - await db.Database.ExecuteSqlInterpolatedAsync($@" - UPDATE audit_log - SET actor_email = '[REDACTED]', - ip_address = NULL, - user_agent = '[REDACTED]', - before_state = jsonb_set(before_state, '{{redacted}}', 'true'), - after_state = jsonb_set(after_state, '{{redacted}}', 'true'), - changes = jsonb_set(changes, '{{redacted}}', 'true') - WHERE actor_user_id = {@event.UserId} - AND tenant_id = {@event.TenantId}", ct); - - // 3. Redact rows where entity references this user (via per-module IUserReferenceLocator) - foreach (var locator in _userReferenceLocators) - await locator.RedactReferencesAsync(@event.UserId, ct); - - // 4. Inbox: mark processed; SaveChanges - _inboxGuard.MarkAsProcessed(@event.EventId, @event.GetType().Name); - await db.SaveChangesAsync(ct); - - // 5. Audit the redaction itself as a SecurityEvent (meta-audit) - logger.LogInformation("GDPR redaction applied for User {UserId} in Tenant {TenantId}", - @event.UserId, @event.TenantId); + // 1. Idempotent: inbox guard. + if (await inboxGuard.IsAlreadyProcessedAsync(@event.EventId, ct)) return; + + // 2. learnstack_app holds no UPDATE privilege on audit_log, so the redaction runs + // as learnstack_platform. Entering the scope is itself a MUST-class audit event. + await using (await platformScope.EnterAsync( + reason: $"gdpr-redaction:{@event.UserId}", ct)) + { + // 3. Actor PII only. The payload columns are NOT touched here. A blanket + // jsonb_set('{redacted}', 'true') would (a) not redact anything — it adds a + // flag and leaves the PII in place — and (b) raise + // 'cannot set path in scalar' on any snapshot that is a JSON scalar or + // array. Payload redaction belongs to the per-module locator below, which + // knows which JSON paths in its own snapshots reference a user. + await db.Database.ExecuteSqlInterpolatedAsync($@" + UPDATE audit_log + SET actor_email = '[REDACTED]', + ip_address = NULL, + user_agent = '[REDACTED]' + WHERE actor_user_id = {@event.UserId} + AND tenant_id = {@event.TenantId}", ct); + + // 4. Payload references, per module. Each locator issues column-restricted + // UPDATEs against before_state / after_state / changes only. + foreach (var locator in userReferenceLocators) + await locator.RedactReferencesAsync(@event.UserId, @event.TenantId, ct); + + // 5. Inbox: mark processed; SaveChanges. + inboxGuard.MarkAsProcessed(@event.EventId, @event.GetType().Name); + await db.SaveChangesAsync(ct); + } + + // 6. Meta-audit. The redaction is itself a MUST-class security event, and a log + // line is not an audit row — the previous version of this handler logged and + // called it audited. + await auditStore.WriteStandaloneAsync( + AuditEntryDraft.SecurityEvent( + tenantId: @event.TenantId, + module: "audit", + operation: "audit.redaction.apply", + outcome: AuditOutcome.Success, + metadata: new { subjectUserId = @event.UserId }), + ct); } } ``` +`actor_user_id`, `module`, `operation`, `operation_type`, `operation_class`, `outcome`, +`correlation_id` and `timestamp` are **never** redacted; the trigger rejects an `UPDATE` +that changes any of them. What survives erasure is a pseudonymous record that a regulator +can still reconstruct "who did what, when, with what outcome" from — which is the point of +redacting in place rather than deleting. + Every module that stores user references in audit payloads must register an `IUserReferenceLocator` implementation (architecture test enforces this — same shape as Nexora's `IContactReferenceLocator`). -## 10. Querying audit log +## 11. Querying audit log The Audit module's admin API exposes: @@ -508,7 +892,7 @@ Required permission: `audit.events.read` (tenant scope). Required for export: `audit.events.export`. Required for cross-tenant query (platform admin only): `platform.audit.events.read`. -## 11. Hub-side audit stream +## 12. Hub-side audit stream Hub maintains a parallel audit stream in its own database (`hub_audit_log`), capturing operator actions: @@ -523,43 +907,84 @@ operator actions: Cross-stream correlation by `correlation_id`. A regulatory inquiry covering "what happened to tenant X on date Y" pulls from both streams and joins by correlation. -## 12. Architecture tests - -Five blocker-level tests added in Phase 02: - -1. `Every_Command_HasAuditCoverage` — auto-discovers commands by interface - (`ICommand`); for each, asserts the (module, operation) appears in - `IAuditConfigService.GetCoverageMatrix()` with at least SHOULD classification. -2. `AuditEntry_Is_AppendOnly` — `AuditEntry` does not implement `ISoftDeletable`; has no - `Update*` public methods; reflection assertion. -3. `AuditLogBehavior_NeverBlocks_BusinessWrites` — integration test asserts that when - `IAuditStore.SaveAsync` throws, the command result is still returned (handler success - path). -4. `AuditStateCapture_ClearedPerRequest` — integration test asserts that after a request - completes (success or failure), the scoped `IAuditStateCapture.Changes` collection is - empty for the next request. -5. `Every_PII_Module_RegistersUserReferenceLocator` — modules storing user references in - audit payloads (declared via `[StoresUserReference]` attribute or - `IModule.RegisterUserReferences()`) must register an `IUserReferenceLocator` impl. - -## 13. Phasing +## 13. Architecture tests + +Blocker-level tests, registered in +[Architecture Tests Catalogue](../standards/21-architecture-tests-catalogue.md) by +[Phase 02a Packet 9](../roadmap/phase-02a-kernel-tenancy.md): + +1. `Every_TenantOwned_Command_HasAuditCoverage` — auto-discovers commands by interface; + for each, asserts the (module, operation) appears in the coverage matrix with at least + a SHOULD classification. +2. `AuditEntry_Inherits_Entity_Not_AuditableEntity` — append-only by construction: an + audit row that carries `UpdatedAt` / `DeletedAt` is a contradiction. +3. `MustClass_Audit_Writes_Share_The_Business_Transaction` — the binding test for + [ADR-0033](../decisions/0033-audit-durability-model.md). One MUST-class command + produces exactly one `audit_log` row, inserted on the same transaction as the business + write; a command whose durable audit write is forced to fail produces **zero** business + rows and returns `503 audit_unavailable`. Runs as `learnstack_app` (`NOBYPASSRLS`). +4. `Audit_Survives_Transaction_Rollback` — the test that closes the gap a "consumed" flag + would have left open. A MUST-class command whose transaction is forced to roll back at + `COMMIT` produces zero business rows and **exactly one** `audit_log` row with outcome + `failed`. A companion case covers the ordinary path: a handler that calls `SaveChanges` + and then returns `Result.Fail(...)` produces the same pair. +5. `Audit_Classification_Does_Not_Read_The_Database_On_The_Request_Path` — with the + `audit_config` table made unreadable, a MUST-class command still completes and still + writes its row at the catalogue classification; an operation absent from the catalogue + is rejected with `audit_unclassified_operation`. Without this, a silent RLS-filtered + empty read is indistinguishable from "this tenant has no overrides". +6. `AuditLog_Update_Is_Column_Restricted` — as `learnstack_app`, any `UPDATE` or `DELETE` + on `audit_log` raises `42501`. As `learnstack_platform`, an `UPDATE` touching only the + six redactable columns succeeds, an `UPDATE` touching any other column raises, and a + `DELETE` succeeds (the retention purge). +7. `AuditStateCapture_ClearedPerRequest` — after a request completes, success or failure, + the scoped `IAuditStateCapture` holds no changes, no intent, and `State == None` for + the next request. +8. `Every_Module_Has_An_AuditCoverage_Matrix` — a module without a matrix cannot classify + its operations, and under ADR-0033 classification is functional, not documentary. +9. `Every_PII_Module_RegistersUserReferenceLocator` — modules storing user references in + audit payloads must register an `IUserReferenceLocator`. + +`Audit_Config_Failure_Rejects_Operation` from the earlier draft is **withdrawn**, not +renamed: under ADR-0033 as settled, a tenant-override read failure falls back to the +in-process catalogue rather than rejecting, so the assertion would have locked in a +platform-wide denial of service triggered by a cache outage. Entry 5 above asserts the +property that actually matters. + +`AuditLogBehavior_NeverBlocks_BusinessWrites` from the ADR-0016 era is **replaced**: the +property it asserted is now true only for SHOULD/MAY-class operations, and asserting it +for MUST-class would lock in exactly the defect ADR-0033 removes. + +## 14. Phasing | Phase | Deliverable | |-------|-------------| -| 02 | Infrastructure: `AuditChangeTrackerInterceptor`, `IAuditStateCapture` + impl, `AuditLogBehavior`, `IAuditStore` + `PostgresAuditStore`, `audit_log` table + first month partition, `LearnStackJob` for partition management. | -| 03 | `LearnStack.Modules.Audit`: `AuditEntry` aggregate, repository, admin API endpoints. `UserGdprDeletedIntegrationEventHandler` + per-module `IUserReferenceLocator`. | -| 06+ | Admin Studio audit UI: timeline view, filter by user/module/operation/operation_class/timestamp, diff viewer, CSV / JSON export. | -| 09 | Hub-side `hub_audit_log` + cross-stream correlation query. | -| 11 | Production: long-term partition lifecycle automation, off-archive policy for partitions > 1 year (e.g. move to cheaper storage tier), per-tenant retention enforcement under plan limits. | +| [02a Packet 9](../roadmap/phase-02a-kernel-tenancy.md) | `AuditChangeTrackerInterceptor`, `IAuditStateCapture` + impl, `AuditLogBehavior` lit up per ADR-0033, `IAuditStore` + `PostgresAuditStore`, `AuditEntry` aggregate, `AuditConfig` with the fail-closed MUST floor, `audit_log` as a **single plain table** with the composite primary key. | +| [03](../roadmap/phase-03-identity-admin.md) | Admin API endpoints over the audit stream; `UserGdprDeletedIntegrationEventHandler` + per-module `IUserReferenceLocator`. | +| [06](../roadmap/phase-06-renderer-admin-studio.md) | Admin Studio audit UI: timeline view, filters, diff viewer, CSV / JSON export. | +| [09](../roadmap/phase-09-billing-integrations-analytics.md) | Hub-side `hub_audit_log` + cross-stream correlation query. | +| [11](../roadmap/phase-11-production-hardening.md) | **Scale, not correctness**: `PARTITION BY RANGE (timestamp)` plus monthly partitions, the daily partition-management job, the daily retention purge, off-archive policy for partitions older than a year, per-tenant retention enforcement under plan limits. Trigger: measured `audit_log` growth ([ADR-0035](../decisions/0035-demand-gated-infrastructure.md)). | ## References -- ADR-0016 — Audit Log Subsystem. -- ADR-0003 Amendment 1 — Organization scope (audit row carries organization_id). +- [ADR-0033](../decisions/0033-audit-durability-model.md) — Audit Durability Model + (supersedes ADR-0016); the two durability classes, the fail-closed rules, and the + corrected `audit_log` primary key. +- [ADR-0016](../decisions/0016-audit-log-subsystem.md) — Audit Log Subsystem + (**superseded**; retained for its data model and coverage rationale). +- [ADR-0003 Amendment 1 + Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) + — organization scope on the audit row; the corrected RLS template and four-role model. +- [ADR-0028](../decisions/0028-audit-log-partition-management.md) — partition management + via Hangfire; lands in Phase 11. +- [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) — why partitioning is + demand-gated and correctness is not. +- [Database Standards](../standards/05-database.md) — the canonical RLS template. - [18-audit-coverage.md](../standards/18-audit-coverage.md) — MUST/SHOULD/MAY matrix (standard). +- [15-event-and-outbox.md](15-event-and-outbox.md) — audit fan-out to external sinks + rides the outbox; MUST-class audit does not. - [29-dapr-integration.md](29-dapr-integration.md) — `UserGdprDeletedIntegrationEvent` - arrives via Dapr pub/sub. + transport. - Nexora reference: `Nexora/docs/modules/tier-1-core/audit/SPEC.md`, `Nexora/docs/decisions/0009-audit-repository-pattern.md`, `Nexora/docs/standards/audit-coverage.md`. diff --git a/docs/architecture/32-tenant-customization-model.md b/docs/architecture/32-tenant-customization-model.md index 099e7d4..4dd0f48 100644 --- a/docs/architecture/32-tenant-customization-model.md +++ b/docs/architecture/32-tenant-customization-model.md @@ -89,14 +89,15 @@ These map 1:1 to JSON Schema `type` + `format` combinations: ```typescript const COMPOSITE_RENDERERS = { - 'default-card': DefaultCardRenderer, // image + title + description - 'content-list': ContentListRenderer, // grid / carousel / list of items - 'media-gallery': MediaGalleryRenderer, - 'rich-page': RichPageRenderer, - 'lesson-shell': LessonShellRenderer, // standard lesson play UI - 'quiz-shell': QuizShellRenderer, - 'placement-shell': PlacementShellRenderer, - 'live-shell': LiveShellRenderer, + 'default-card': DefaultCardRenderer, // image + title + description + 'content-list': ContentListRenderer, // grid / carousel / list of items + 'media-gallery': MediaGalleryRenderer, + 'rich-page': RichPageRenderer, + 'lesson-shell': LessonShellRenderer, // standard lesson play UI + 'quiz-shell': QuizShellRenderer, + 'placement-shell': PlacementShellRenderer, + 'live-shell': LiveShellRenderer, + 'submission-shell': SubmissionShellRenderer, // prompt + authoring surface + submit + result // ... small, fixed list } as const; ``` @@ -104,6 +105,12 @@ const COMPOSITE_RENDERERS = { Adding a new primitive or composite renderer is a LearnStack release (CODEOWNERS rule on this folder). Tenants compose existing primitives — they cannot bring custom JSX. +**Every key in both registries is named for a capability, never for a domain.** A +`cefr-level-badge` or `asana-card` key would fail +`Core_Modules_HaveNo_DomainSpecific_Names`. When a tenant needs a domain-flavoured +presentation, it declares a `TenantPageBlock` row pointing at a generic composite — the +domain lives in the row's display name and its schema, not in the registry. + ## 3. Worked example: three tenants, same modules ### Example A — English learning platform @@ -253,14 +260,39 @@ Same modules. Different data. "difficulty": { "type": "string", "x-taxonomy": "coding-difficulty" } } }, - "renderer_key": "code-challenge-shell" + "renderer_key": "submission-shell" } ``` -Note: `code-challenge-shell` is a **composite renderer** that LearnStack ships (since -running test suites server-side is a paid LearnStack feature, gated by -`FeatureKeys.CodeChallengeRunner`). The composite is generic — it works for any language -declared in the schema; the tenant doesn't bring custom JSX. +**Two naming rules meet in this example, and they point in opposite directions.** + +The content type's `key` is `code-challenge`. That is fine: it is a **row in the tenant's +database**, authored by the tenant, and it may name its domain as specifically as the +tenant likes. `vocabulary-card`, `asana-pose`, `code-challenge` and `breath-technique` +are all legitimate tenant data. + +The renderer key and the feature key are **code**, and code may not name a domain. Both +were previously named for the first tenant that asked for them: + +| Was | Is | Why | +|---|---|---| +| `code-challenge-shell` (composite renderer) | **`submission-shell`** | The capability is "prompt + authoring surface + submit + evaluated result panel". It serves a code exercise, a pronunciation recording, an essay, and a portfolio upload equally | +| `FeatureKeys.CodeChallengeRunner` | **`FeatureKeys.SandboxedEvaluation`** (`assessment.sandboxed_evaluation`) | The capability is "evaluate a learner's submitted artefact in a sandbox with a resource budget". Nothing about it is specific to code | + +The rule is not stylistic. `Core_Modules_HaveNo_DomainSpecific_Names` enforces it +mechanically from +[Phase 02a Packet 10](../roadmap/phase-02a-kernel-tenancy.md) — a module type, namespace +or registry key matching `Cefr`, `Asana`, `CodeChallenge`, `English*`, `Yoga*` fails the +build. The old names would have failed it. + +`submission-shell` is also where this document meets the **genericity boundary**. The +renderer is presentation and therefore inside the boundary; the *evaluation* it triggers +is external capability invocation and therefore outside it. Running a learner's submitted +program needs a sandbox, a runtime, a resource budget and a security boundary that +survives hostile input — none of which a JSON Schema or a rule DSL can declare. It is a +**platform feature gated by plan**, written by LearnStack, not a customization row. See +[ADR-0018 Amendment (2026-08-08)](../decisions/0018-tenant-driven-customization-model.md) +and [Platform Vision § Genericity boundary](01-platform-vision.md). ## 4. Schema versioning @@ -284,14 +316,19 @@ sets the rule for page blocks; this document extends it to all customization sur ## 5. Custom fields on built-in entities -A subset of built-in entities (`User`, `Course`, `Enrollment`, `LiveSession`, `Lesson`) -accept tenant-defined custom fields: +Tenant-defined custom fields attach to **tenant-owned** entities: `Membership`, +`Course`, `Enrollment`, `LiveSession`, `Lesson`. `target_entity = "User"` resolves to +`membership_profiles` — never to `users`, which is global, carries no `tenant_id`, and +therefore has no query filter and no Row Level Security policy. A tenant-authored +column on a global table is a cross-tenant read by construction +([Phase 03 § Tenant Data Ownership](../roadmap/phase-03-identity-admin.md)). ```sql CREATE TABLE tenant_custom_field_defs ( id uuid PRIMARY KEY, tenant_id uuid NOT NULL, - target_entity text NOT NULL, -- "User", "Course", "Enrollment", etc. + target_entity text NOT NULL, -- "Membership", "Course", "Enrollment", …; never "User" + pii_category text NOT NULL, -- PII-Identity | PII-Behaviour | PII-Sensitive | None; no default, see Phase 03 key text NOT NULL, -- e.g. "preferred_practice_time" display_name text NOT NULL, json_schema jsonb NOT NULL, -- field definition @@ -305,8 +342,11 @@ CREATE TABLE tenant_custom_field_defs ( Values stored on the entity as a JSONB column: ```sql -ALTER TABLE users ADD COLUMN custom_fields jsonb NOT NULL DEFAULT '{}'; -ALTER TABLE courses ADD COLUMN custom_fields jsonb NOT NULL DEFAULT '{}'; +-- Values live on the tenant-owned row, never on a global one. +ALTER TABLE membership_profiles ADD COLUMN custom_fields jsonb NOT NULL DEFAULT '{}'; +ALTER TABLE courses ADD COLUMN custom_fields jsonb NOT NULL DEFAULT '{}'; +-- Every table above is [TenantOwned] and carries the canonical RLS policy from +-- Database Standards; `users` is not in this list and never will be. -- etc. ``` @@ -360,7 +400,158 @@ expression: | Same sandbox engine as scoring rules (decision pending). -## 8. What cannot be customized +## 8. Runtime cost model + +Everything above describes what a tenant *can* declare. This section describes what it +*costs* to serve, because a customization model without a cost model is an invitation to +declare something the runtime cannot render inside a request budget. Every limit here is +enforced at write time, so a tenant learns about it while authoring rather than a learner +discovering it on a page load. + +### 8.1 Validation timing — write time, not read time + +| Validation | When | Failure mode | +|---|---|---| +| The `json_schema` is itself a valid JSON Schema (draft 2020-12), and every `x-renderer` / `x-taxonomy` / `x-language` extension resolves to a registry entry | **On saving the content type / block / lesson item type** | 400 with Problem Details naming the offending JSON pointer | +| A content entry conforms to its content type's schema at its pinned `schema_version` | **On saving the entry** | 400; the entry is not persisted | +| The declared limits in § 8.3 | **On saving the schema** | 400 | +| Renderer resolution: does `renderer_key` exist in `COMPOSITE_RENDERERS`? | **On saving**, again on **server render** as a cheap dictionary lookup | Save is rejected; a stale reference renders a fallback block plus a logged warning, never an exception | + +**Nothing is schema-validated on the read path.** A published content entry has already +been validated against its pinned schema version, and re-validating it on every page load +would put a JSON Schema evaluation in the hot path of anonymous traffic for a result that +cannot have changed. The consequence is stated plainly: **the schema is a write-time +contract, and the read path trusts the database.** Anything that can write a content-entry +row without going through the validating command path — a manual `INSERT`, a bad +migration, a future bulk importer — breaks that trust for every subsequent read. Bulk +import therefore goes through the same validator, at +[Phase 04](../roadmap/phase-04-cms-media-pages.md). + +The one read-time check is **structural, not semantic**: the renderer walks the entry's +JSON against the schema's field list and skips unknown fields. That is an O(fields) pass, +not a validation. + +### 8.2 Cache strategy + +Customization definitions are read on nearly every request and written a handful of times +per tenant per month. That ratio is the whole design. + +| What | Layer | Key | TTL | Invalidated by | +|---|---|---|---|---| +| `TenantContentType` set for a tenant | L1 + L2 | `cust:{tenant_id}:content-types:{generation}` | L1 60s, L2 15 min | Generation bump | +| `TenantLevelTaxonomy` by key | L1 + L2 | `cust:{tenant_id}:taxonomy:{key}:{generation}` | same | Generation bump | +| `TenantPageBlock` set | L1 + L2 | `cust:{tenant_id}:blocks:{generation}` | same | Generation bump | +| Compiled JSON Schema validator | L1 only, per pod | `(tenant_id, content_type_key, schema_version)` | Process lifetime, bounded LRU | Immutable — a schema version never changes | + +Two rules make this safe: + +- **A generation counter, not prefix eviction.** Each tenant carries a + `customization_generation` integer, bumped in the same transaction as any customization + write. Cache keys embed it, so a write makes every stale key unreachable at once, + across every pod, without enumerating keys. This is deliberate: the published + `ICacheService.RemoveByPrefixAsync` contract cannot be honoured across instances by any + candidate backend, and it is removed or redesigned to exactly this pattern before + [Phase 02a Packet 5](../roadmap/phase-02a-kernel-tenancy.md) ships. +- **Compiled validators are cached separately from definitions**, keyed by an immutable + `(key, schema_version)` tuple. Compiling a JSON Schema is the expensive part; because a + published schema version is immutable ([§ 4](#4-schema-versioning)), the compiled form + never needs invalidating — it only needs bounding, hence the LRU. + +Cache misses cost one indexed query per tenant per definition set. A cold pod serving its +first request for a tenant performs at most four such queries, not one per entry. + +### 8.3 The N+1 problem, and the limits that bound it + +The guided-sequence example in [§ 3](#3-worked-example-three-tenants-same-modules) is a +textbook N+1 and it is worth being explicit about, because it is the shape tenants will +naturally author: + +```json +"poses": { "type": "array", "items": { "$ref": "#/$defs/pose-ref" } } +``` + +Each `pose-ref` carries `{ content_type: "asana-pose", content_id: "" }`. A naive +renderer resolves each reference with its own query, so a 40-pose sequence issues 40 +round trips — and a lesson page containing three such sequences issues 120. At 2 ms per +round trip that is a quarter-second of pure latency for a page that reads four rows' +worth of actual information. + +**The resolution contract:** + +1. The renderer **collects every reference in the whole payload first**, walking the + entry's JSON once and gathering `(content_type, content_id)` pairs into a set. +2. It issues **one batched query per content type** — `WHERE tenant_id = @t AND + content_type = @ct AND id = ANY(@ids)` — not one per reference. Duplicated references + to the same entry cost nothing extra. +3. It hydrates the payload from the resulting dictionary. +4. References that do not resolve render a placeholder and emit a warning. A dangling + reference is a data problem, never an exception on a public page. + +Total cost for any entry: **one query per distinct referenced content type**, regardless +of reference count. Reference resolution is capped at **depth 2** — an entry may +reference entries, and those may not reference further. Deeper composition is a +[Phase 05](../roadmap/phase-05-education-learning-content.md) decision with its own +batching design, not something a tenant can reach by nesting `$ref`s. + +The `Customization_Reference_Resolution_Is_Batched` integration test asserts the query +count for a 40-reference entry is a small constant, not 40. Without a test that counts +queries, this contract silently regresses the first time someone writes a convenient +`foreach`. + +### 8.4 Declared limits + +Every limit is checked at write time and returns Problem Details naming the limit that +was exceeded. They are deliberately generous — they exist to stop pathological documents, +not to constrain reasonable authoring. + +| Limit | Value | Why this one | +|---|---|---| +| `$ref` / `$defs` nesting depth inside one schema | 5 | JSON Schema validators are recursive; unbounded depth is a stack-exhaustion vector on a tenant-authored document | +| Reference resolution depth (entry → entry) | 2 | Bounds the batched-query fan-out at § 8.3 to a fixed small number of round trips | +| Properties in one content type | 100 | Beyond this the authoring UI is unusable and the row is a schema smell | +| Array `maxItems` where the schema omits it | 200 (applied as a default, not a rejection) | An unbounded array in a JSONB column is an unbounded render loop | +| References in one content entry | 500 | Caps the `ANY(@ids)` parameter list and the hydration dictionary | +| Block instances on one page | 100 | Each instance is a render subtree; the page is a document, not an application | +| Serialised size of one content entry | 1 MB | PostgreSQL will happily TOAST more; the browser will not happily render it | +| Content types per tenant | Plan-gated via `FeatureKeys` / `LimitKeys` | Unlimited content types is a Growth+ feature ([ADR-0021](../decisions/0021-feature-based-entitlement.md)) | + +A schema that violates a structural limit is rejected on save. An **existing** entry that +would violate a newly tightened limit still renders — limits are enforced forward, and a +tightening ships with the migration that reports which existing rows exceed it. + +### 8.5 The `embed-html` sanitisation contract + +`embed-html` is the one primitive that takes an **HTML sink authored by a lower-trust +actor** and puts it in the tenant's page. Everything else in the primitive set renders +structured data through React text nodes and is safe by construction. This one is not +safe by construction; it is safe by contract, and the contract has to be written down — +calling the primitive set "closed and safe" without stating it is precisely the gap that +lets a reviewer approve an unsanitised path. + +**The threat model.** A tenant's content editor is not the tenant's security team. In a +multi-branch business, an organization-level editor at one studio can author an +`embed-html` block that renders on a page a learner from another organization loads. The +actor is semi-trusted, the audience is not the actor, and the two may sit in different +organizations of the same tenant. + +**The contract:** + +| Rule | Detail | +|---|---| +| Sanitise **on write and on render** | Write-time sanitisation gives the author immediate feedback and keeps the stored value clean. Render-time sanitisation is the one that actually protects the learner, because it is the only one that also covers rows written before the current allow-list | +| Allow-list, never block-list | Elements: structural and text markup plus `iframe` (see below). Attributes: `class`, `id`, `href`, `src`, `alt`, `title`, `width`, `height`, `colspan`, `rowspan`. Everything else is stripped | +| No script execution surface | `