From fc28c156329489039e79d4de4f441f992afa8135 Mon Sep 17 00:00:00 2001 From: MaximumTrainer Date: Sat, 12 Sep 2026 20:15:12 +0100 Subject: [PATCH 1/8] fix(frontend): make npm test green and enforce it in CI vitest.config.ts excluded 'playwright/**', but the Playwright specs live in e2e/ (playwright.config.ts testDir). Vitest therefore collected all six e2e specs and every one failed on test.describe(), so `npm test` exited 1 on a clean checkout while the 108 real unit tests passed. - Scope vitest to src/** and exclude e2e/** so the two runners own disjoint directories. - Delete src/frontend/playwright/, a byte-identical copy of e2e/onboarding-journeys.spec.ts sitting outside Playwright's testDir, so no runner ever executed it. - Run `npm test` in the frontend-ci job. The job previously ran lint and build only, which is why the red suite went unnoticed. - Ignore playwright-report/ and test-results/. Verified: npm test 14 files / 108 tests green, exit 0; a deliberately failing assertion exits 1; npx playwright test --list still finds 26 tests in 6 files. Closes #92 Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 5 +- .gitignore | 4 + .../playwright/onboarding-journeys.spec.ts | 329 ------------------ src/frontend/vitest.config.ts | 5 +- 4 files changed, 12 insertions(+), 331 deletions(-) delete mode 100644 src/frontend/playwright/onboarding-journeys.spec.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ecd0e8e..7201e95 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -92,7 +92,7 @@ jobs: # ── Frontend ─────────────────────────────────────────────── frontend-ci: - name: Frontend — lint & build (Node 22) + name: Frontend — lint, test & build (Node 22) runs-on: ubuntu-latest defaults: @@ -116,6 +116,9 @@ jobs: - name: Lint run: npm run lint + - name: Unit tests (vitest) + run: npm test + - name: Build run: npm run build diff --git a/.gitignore b/.gitignore index 220c6fc..a1ca8c2 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,7 @@ **/node_modules/ **/dist/ .copilot-specs-cache/ + +# Playwright output (generated by test:e2e, not committed) +playwright-report/ +test-results/ diff --git a/src/frontend/playwright/onboarding-journeys.spec.ts b/src/frontend/playwright/onboarding-journeys.spec.ts deleted file mode 100644 index a6d192e..0000000 --- a/src/frontend/playwright/onboarding-journeys.spec.ts +++ /dev/null @@ -1,329 +0,0 @@ -import { expect, test } from '@playwright/test' - -type MockNode = { - id: string - key: string - type: 'Form' | 'DocumentUpload' | 'Information' | 'Logic' | 'Redirect' - title: string - jsonContent: string - isStartNode: boolean -} - -type MockJourney = { - id: string - name: string - description: string - nodes: MockNode[] - connections: Array<{ - id: string - sourceNodeId: string - targetNodeId: string - priority: number - }> -} - -const mockJourneys: Record = { - '11111111-1111-1111-1111-111111111111': { - id: '11111111-1111-1111-1111-111111111111', - name: 'Small business onboarding', - description: 'Small business journey', - nodes: [ - { - id: 'small-node-1', - key: 'small-business-details', - type: 'Form', - title: 'Small business details', - jsonContent: - '{"fields":[{"name":"BusinessName","type":"text","required":true},{"name":"BusinessAddress","type":"textarea","required":true},{"name":"AnnualRevenue","type":"number","required":true},{"name":"BusinessOwner","type":"text","required":true},{"name":"SanctionsDeclarationConfirmed","type":"checkbox","required":true}]}', - isStartNode: true, - }, - { - id: 'small-node-2', - key: 'small-complete', - type: 'Information', - title: 'Small business onboarding checks complete.', - jsonContent: '{}', - isStartNode: false, - }, - ], - connections: [ - { - id: 'small-conn-1', - sourceNodeId: 'small-node-1', - targetNodeId: 'small-node-2', - priority: 0, - }, - ], - }, - '22222222-2222-2222-2222-222222222222': { - id: '22222222-2222-2222-2222-222222222222', - name: 'Medium business onboarding', - description: 'Medium business journey', - nodes: [ - { - id: 'medium-node-1', - key: 'medium-business-details', - type: 'Form', - title: 'Medium business details', - jsonContent: - '{"fields":[{"name":"BusinessName","type":"text","required":true},{"name":"PrimaryAddress","type":"textarea","required":true},{"name":"AnnualRevenue","type":"number","required":true},{"name":"BusinessOwner","type":"text","required":true},{"name":"SecondaryBusinessOwner","type":"text","required":true},{"name":"NumberOfOutlets","type":"number","required":true},{"name":"BusinessStaffCount","type":"number","required":true},{"name":"BeneficialOwnersConfirmed","type":"checkbox","required":true},{"name":"TaxComplianceConfirmed","type":"checkbox","required":true}]}', - isStartNode: true, - }, - { - id: 'medium-node-2', - key: 'medium-document-verification', - type: 'DocumentUpload', - title: 'Upload ownership and trading documents', - jsonContent: - '{"acceptedFileTypes":["application/pdf","image/jpeg","image/png"],"maxFiles":2}', - isStartNode: false, - }, - ], - connections: [ - { - id: 'medium-conn-1', - sourceNodeId: 'medium-node-1', - targetNodeId: 'medium-node-2', - priority: 0, - }, - ], - }, - '33333333-3333-3333-3333-333333333333': { - id: '33333333-3333-3333-3333-333333333333', - name: 'Large nationwide business onboarding', - description: 'Large business journey', - nodes: [ - { - id: 'large-node-1', - key: 'large-business-profile', - type: 'Form', - title: 'Large business profile', - jsonContent: - '{"fields":[{"name":"BusinessName","type":"text","required":true},{"name":"LegalStructure","type":"select","required":true,"options":["LimitedCompany","Partnership","PublicLimitedCompany","Other"]},{"name":"PrimaryAddress","type":"textarea","required":true},{"name":"AnnualRevenue","type":"number","required":true},{"name":"BusinessOwner","type":"text","required":true},{"name":"SecondaryBusinessOwner","type":"text","required":true},{"name":"NumberOfOutlets","type":"number","required":true},{"name":"BusinessStaffCount","type":"number","required":true}]}', - isStartNode: true, - }, - { - id: 'large-node-2', - key: 'large-compliance-questionnaire', - type: 'Form', - title: 'Compliance questionnaire', - jsonContent: - '{"fields":[{"name":"RegulatoryLicensesConfirmed","type":"checkbox","required":true},{"name":"SanctionsScreeningCompleted","type":"checkbox","required":true},{"name":"BeneficialOwnershipReviewed","type":"checkbox","required":true}]}', - isStartNode: false, - }, - { - id: 'large-node-3', - key: 'large-complete', - type: 'Information', - title: 'Large nationwide business onboarding checks complete.', - jsonContent: '{}', - isStartNode: false, - }, - ], - connections: [ - { - id: 'large-conn-1', - sourceNodeId: 'large-node-1', - targetNodeId: 'large-node-2', - priority: 0, - }, - { - id: 'large-conn-2', - sourceNodeId: 'large-node-2', - targetNodeId: 'large-node-3', - priority: 0, - }, - ], - }, -} - -let documentUploads: Array<{ sessionId: string; nodeId: string; apiKey: string | null }> = [] - -test.beforeEach(async ({ page }) => { - const sessions = new Map() - documentUploads = [] - let sessionCounter = 0 - - await page.route('**/api/flows/*', async (route) => { - const flowId = route.request().url().split('/').pop()! - const journey = mockJourneys[flowId] - - if (!journey) { - await route.fulfill({ status: 404, body: 'Flow not found' }) - return - } - - await route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify(journey), - }) - }) - - await page.route('**/api/workflow/sessions/start', async (route) => { - const body = route.request().postDataJSON() as { flowId: string } - const journey = mockJourneys[body.flowId] - - if (!journey) { - await route.fulfill({ status: 400, body: 'Unknown flow' }) - return - } - - sessionCounter += 1 - const sessionId = `session-${sessionCounter}` - sessions.set(sessionId, { journeyId: journey.id, nodeIndex: 0 }) - - await route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify({ - sessionId, - isCompleted: false, - currentNode: journey.nodes[0], - }), - }) - }) - - await page.route('**/api/workflow/sessions/*/steps/*/submit', async (route) => { - const match = route.request().url().match(/\/api\/workflow\/sessions\/([^/]+)\/steps\//) - const sessionId = match?.[1] - - if (!sessionId || !sessions.has(sessionId)) { - await route.fulfill({ status: 404, body: 'Session not found' }) - return - } - - const state = sessions.get(sessionId)! - const journey = mockJourneys[state.journeyId] - const nextIndex = state.nodeIndex + 1 - const nextNode = journey.nodes[nextIndex] ?? null - - if (nextNode) { - state.nodeIndex = nextIndex - await route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify({ - sessionId, - isCompleted: false, - currentNode: nextNode, - }), - }) - return - } - - await route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify({ - sessionId, - isCompleted: true, - currentNode: null, - }), - }) - }) - - await page.route('**/api/workflow/sessions/*/steps/*/documents', async (route) => { - const match = route.request().url().match(/\/api\/workflow\/sessions\/([^/]+)\/steps\/([^/]+)\/documents/) - const sessionId = match?.[1] - const nodeId = match?.[2] - - if (!sessionId || !nodeId || !sessions.has(sessionId)) { - await route.fulfill({ status: 404, body: 'Session or node not found' }) - return - } - - documentUploads.push({ - sessionId, - nodeId, - apiKey: route.request().headers()['x-api-key'] ?? null, - }) - - await route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify([{ fileId: 'file-1', fileName: 'ownership-proof.pdf' }]), - }) - }) - - await page.route('**/api/workflow/sessions/*/events', async (route) => { - await route.fulfill({ - status: 200, - contentType: 'text/event-stream', - body: '', - }) - }) -}) - -test('journey 1 runs simple small business onboarding flow', async ({ page }) => { - await page.goto('/') - - await expect(page.locator('form').getByText('Small business details')).toBeVisible() - - await page.locator('#BusinessName').fill('Acme Local Ltd') - await page.locator('#BusinessAddress').fill('1 High Street, London') - await page.locator('#AnnualRevenue').fill('250000') - await page.locator('#BusinessOwner').fill('Jamie Owner') - await page.locator('#SanctionsDeclarationConfirmed').check() - await page.getByRole('button', { name: 'Submit' }).click() - - await expect(page.locator('section.rounded-lg').getByText('Small business onboarding checks complete.')).toBeVisible() -}) - -test('journey 2 reaches medium business document verification step', async ({ page }) => { - await page.goto('/') - - await page.selectOption('#journey-select', '22222222-2222-2222-2222-222222222222') - await expect(page.locator('form').getByText('Medium business details')).toBeVisible() - - await page.locator('#BusinessName').fill('Acme Regional Ltd') - await page.locator('#PrimaryAddress').fill('200 King Road, Manchester') - await page.locator('#AnnualRevenue').fill('3500000') - await page.locator('#BusinessOwner').fill('Jordan Owner') - await page.locator('#SecondaryBusinessOwner').fill('Taylor Coowner') - await page.locator('#NumberOfOutlets').fill('18') - await page.locator('#BusinessStaffCount').fill('120') - await page.locator('#BeneficialOwnersConfirmed').check() - await page.locator('#TaxComplianceConfirmed').check() - await page.getByRole('button', { name: 'Submit' }).click() - - await expect(page.locator('section.rounded-lg').getByText('Upload ownership and trading documents')).toBeVisible() - await page.locator('input[type="file"]').setInputFiles({ - name: 'ownership-proof.pdf', - mimeType: 'application/pdf', - buffer: Buffer.from('sample document content'), - }) - - await expect(page.getByText('✓ ownership-proof.pdf')).toBeVisible() - await expect(page.getByRole('button', { name: 'Continue' })).toBeEnabled() - await page.getByRole('button', { name: 'Continue' }).click() - - expect(documentUploads).toHaveLength(1) - expect(documentUploads[0].nodeId).toBe('medium-node-2') - expect(documentUploads[0].apiKey).toBe('playwright-api-key') -}) - -test('journey 3 runs large nationwide onboarding and compliance questions', async ({ page }) => { - await page.goto('/') - - await page.selectOption('#journey-select', '33333333-3333-3333-3333-333333333333') - await expect(page.locator('form').getByText('Large business profile')).toBeVisible() - - await page.locator('#BusinessName').fill('Acme National PLC') - await page.locator('#LegalStructure').selectOption('PublicLimitedCompany') - await page.locator('#PrimaryAddress').fill('500 Enterprise Way, Birmingham') - await page.locator('#AnnualRevenue').fill('25000000') - await page.locator('#BusinessOwner').fill('Alex Executive') - await page.locator('#SecondaryBusinessOwner').fill('Sam Director') - await page.locator('#NumberOfOutlets').fill('220') - await page.locator('#BusinessStaffCount').fill('2500') - await page.getByRole('button', { name: 'Submit' }).click() - - await expect(page.locator('form').getByText('Compliance questionnaire')).toBeVisible() - await page.locator('#RegulatoryLicensesConfirmed').check() - await page.locator('#SanctionsScreeningCompleted').check() - await page.locator('#BeneficialOwnershipReviewed').check() - await page.getByRole('button', { name: 'Submit' }).click() - - await expect(page.locator('section.rounded-lg').getByText('Large nationwide business onboarding checks complete.')).toBeVisible() -}) diff --git a/src/frontend/vitest.config.ts b/src/frontend/vitest.config.ts index 01fa470..509f6cb 100644 --- a/src/frontend/vitest.config.ts +++ b/src/frontend/vitest.config.ts @@ -6,6 +6,9 @@ export default defineConfig({ globals: true, setupFiles: ['./src/test-setup.ts'], testTimeout: 30000, - exclude: [...configDefaults.exclude, 'playwright/**'], + // Vitest owns src/**; Playwright owns e2e/** (see playwright.config.ts testDir). + // Collecting the Playwright specs here makes them fail on test.describe(). + include: ['src/**/*.{test,spec}.{ts,tsx}'], + exclude: [...configDefaults.exclude, 'e2e/**'], }, }) From 8e147c7028023ca7cf4d21466d5565e3e03984ee Mon Sep 17 00:00:00 2001 From: MaximumTrainer Date: Sat, 12 Sep 2026 20:24:12 +0100 Subject: [PATCH 2/8] feat(saml): support encrypted assertions and report certificate expiry Metadata advertised KeyDescriptor use="encryption" but the callback never configured a decryption key, so an IdP that honoured the advertised key produced assertions the ACS could not unwrap - every such login failed as a generic saml_invalid_assertion. - Register the SP certificate in Saml2Configuration.DecryptionCertificates so EncryptedAssertion responses decrypt. No new configuration is needed: the certificate published in metadata is the one used to decrypt. - Add Authentication:Saml:RequireEncryptedAssertion. When true, an unencrypted assertion is rejected with saml_assertion_not_encrypted. - Detect an expired SP or IdP certificate and redirect with saml_certificate_expired. Login checks the SP certificate it signs with; the callback also checks the IdP certificate it verifies against. - Log decryption failures distinctly from signature failures, so an operator can tell a key mismatch from a tampered assertion behind the deliberately generic browser-facing error. This makes every code in the frontend LOGIN_ERROR_MESSAGES table reachable; saml_certificate_expired and saml_assertion_not_encrypted were previously dead entries. Tests: 7 added covering the encrypted happy path, encryption to an unknown key, both RequireEncryptedAssertion settings, and expired SP/IdP certificates. SamlAuthControllerTests 19/19, suite 245/245. Closes #94 Co-Authored-By: Claude Opus 5 (1M context) --- docs/runbook.md | 28 +++ .../Controllers/SamlAuthController.cs | 128 ++++++++++- .../SamlAuthControllerTests.cs | 209 +++++++++++++++++- 3 files changed, 354 insertions(+), 11 deletions(-) diff --git a/docs/runbook.md b/docs/runbook.md index 1a360b0..6ab1a21 100644 --- a/docs/runbook.md +++ b/docs/runbook.md @@ -124,6 +124,7 @@ Import `GET /auth/saml/metadata` into the IdP to register Waymark as a service p | `Authentication__Saml__AllowedReturnOrigins__0` | optional | — | Absolute origins accepted for `returnUrl`; anything else falls back to a relative path. | | `Authentication__Saml__RelayStateTimeoutMinutes` | optional | `5` | Lifetime of the relay-state and AuthnRequest-ID cookies. | | `Authentication__Saml__SessionDurationHours` | optional | `8` | Admin session lifetime after a successful assertion. | +| `Authentication__Saml__RequireEncryptedAssertion` | optional | `false` | When `true`, an unencrypted assertion is rejected with `saml_assertion_not_encrypted`. | ### Security Notes @@ -132,6 +133,33 @@ Import `GET /auth/saml/metadata` into the IdP to register Waymark as a service p - File uploads are scanned before storage; rejected files return HTTP 422. - SAML responses are rejected unless the XML signature verifies against `Authentication__Saml__IdpCertificate`, `InResponseTo` matches the AuthnRequest this server issued, the `Destination` names our ACS URL, and the assertion is within its validity window. +#### Assertion encryption + +`GET /auth/saml/metadata` publishes the SP certificate under both `KeyDescriptor use="signing"` and +`KeyDescriptor use="encryption"`, so an IdP may encrypt the assertion to it. The same +`Authentication__Saml__SpCertificate` / `Authentication__Saml__SpPrivateKey` pair is registered as the +decryption key, so **no extra configuration is needed** to accept encrypted assertions — enable +encryption on the IdP side and it works. + +Set `Authentication__Saml__RequireEncryptedAssertion=true` to reject assertions that arrive +unencrypted. Leave it unset while migrating an IdP to encryption, then turn it on once the IdP is +confirmed to be encrypting. + +#### SAML login error codes + +The callback redirects to `/login?error=` on failure. Each code has a distinct cause: + +| Code | Cause | +|------|-------| +| `saml_csrf_failed` | `RelayState` did not match the cookie issued at login. | +| `saml_certificate_expired` | The configured SP or IdP certificate is past its `NotAfter`. Rotate it. | +| `saml_assertion_not_encrypted` | `RequireEncryptedAssertion` is enabled and the assertion was not encrypted. | +| `saml_access_denied` | The assertion's NameID is not in `Authentication__Saml__AllowedNameIds`. | +| `saml_invalid_assertion` | Signature, `InResponseTo`, `Destination`, validity window, or decryption failed. | + +`saml_invalid_assertion` is deliberately generic to the browser; the server log distinguishes a +decryption failure from a signature failure. + --- ## Health Monitoring diff --git a/src/backend/OpenOnboarding.Api/Controllers/SamlAuthController.cs b/src/backend/OpenOnboarding.Api/Controllers/SamlAuthController.cs index db273f7..a3364d8 100644 --- a/src/backend/OpenOnboarding.Api/Controllers/SamlAuthController.cs +++ b/src/backend/OpenOnboarding.Api/Controllers/SamlAuthController.cs @@ -2,6 +2,7 @@ using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Text; +using System.Xml; using ITfoxtec.Identity.Saml2; using ITfoxtec.Identity.Saml2.MvcCore; using ITfoxtec.Identity.Saml2.Schemas; @@ -17,10 +18,13 @@ namespace OpenOnboarding.Api.Controllers; [ApiController] [Route("auth/saml")] [AllowAnonymous] -public sealed class SamlAuthController(IConfiguration configuration) : ControllerBase +public sealed class SamlAuthController( + IConfiguration configuration, + ILogger logger) : ControllerBase { private const string RelayStateCookie = "__Secure-waymark-saml-relay-state"; private const string AuthnIdCookie = "__Secure-waymark-saml-authn-id"; + private const string AssertionNamespace = "urn:oasis:names:tc:SAML:2.0:assertion"; [HttpGet("metadata")] [Produces("application/samlmetadata+xml")] @@ -60,9 +64,20 @@ public IActionResult Metadata() [HttpGet("login")] public IActionResult Login([FromQuery] string? returnUrl = null) { + var safeReturnUrl = NormalizeReturnUrl(returnUrl); + + var expiredCertificate = FindExpiredCertificate(includeIdpCertificate: false); + if (expiredCertificate is not null) + { + logger.LogError( + "SAML login refused: the {Role} certificate expired at {NotAfter:u}.", + expiredCertificate.Value.Role, + expiredCertificate.Value.NotAfter); + return Redirect(BuildLoginErrorRedirect("saml_certificate_expired", safeReturnUrl)); + } + var config = BuildSamlConfiguration(); var relayState = Convert.ToHexString(RandomNumberGenerator.GetBytes(24)); - var safeReturnUrl = NormalizeReturnUrl(returnUrl); var relayStateTimeoutMinutes = GetRelayStateTimeoutMinutes(); Response.Cookies.Append(RelayStateCookie, relayState, new CookieOptions @@ -119,8 +134,20 @@ public async Task Callback() if (string.IsNullOrWhiteSpace(expectedAuthnId)) return Redirect(BuildLoginErrorRedirect("saml_invalid_assertion", safeReturnUrl)); + var assertionWasEncrypted = ResponseContainsEncryptedAssertion(form["SAMLResponse"].ToString()); + try { + var expiredCertificate = FindExpiredCertificate(); + if (expiredCertificate is not null) + { + logger.LogError( + "SAML callback rejected: the {Role} certificate expired at {NotAfter:u}.", + expiredCertificate.Value.Role, + expiredCertificate.Value.NotAfter); + return Redirect(BuildLoginErrorRedirect("saml_certificate_expired", safeReturnUrl)); + } + var config = BuildSamlConfiguration(); var authnResponse = new Saml2AuthnResponse(config); var httpRequest = Request.ToGenericHttpRequest(validate: true); @@ -136,6 +163,15 @@ public async Task Callback() if (authnResponse.Status != Saml2StatusCodes.Success) return Redirect(BuildLoginErrorRedirect("saml_invalid_assertion", safeReturnUrl)); + if (RequireEncryptedAssertion() && !assertionWasEncrypted) + { + logger.LogWarning( + "SAML callback rejected: Authentication:Saml:RequireEncryptedAssertion is enabled " + + "but the IdP returned an unencrypted assertion."); + return Redirect(BuildLoginErrorRedirect("saml_assertion_not_encrypted", safeReturnUrl)); + } + + // Unbind decrypts an EncryptedAssertion and validates the XML signature. httpRequest.Binding.Unbind(httpRequest, authnResponse); var nameId = authnResponse.NameId?.Value?.Trim(); @@ -191,8 +227,24 @@ await HttpContext.SignInAsync( { throw; } - catch + catch (Exception ex) { + if (assertionWasEncrypted && IsDecryptionFailure(ex)) + { + logger.LogError( + ex, + "SAML callback rejected: the encrypted assertion could not be decrypted with the " + + "configured SP private key. Confirm the IdP is encrypting to the certificate " + + "published in /auth/saml/metadata."); + } + else + { + logger.LogError( + ex, + "SAML callback rejected: {Encryption} assertion failed validation.", + assertionWasEncrypted ? "encrypted" : "unencrypted"); + } + return Redirect(BuildLoginErrorRedirect("saml_invalid_assertion", safeReturnUrl)); } } @@ -204,6 +256,72 @@ public async Task Logout() return NoContent(); } + private bool RequireEncryptedAssertion() + => configuration.GetValue("Authentication:Saml:RequireEncryptedAssertion", false); + + /// + /// Reports whether the IdP wrapped the assertion in <saml:EncryptedAssertion>. + /// Read from the raw response so the answer is available before Unbind decrypts it, and so a + /// decryption failure can be reported as such rather than as a generic validation failure. + /// + private static bool ResponseContainsEncryptedAssertion(string? encodedResponse) + { + if (string.IsNullOrWhiteSpace(encodedResponse)) + return false; + + try + { + var xml = Encoding.UTF8.GetString(Convert.FromBase64String(encodedResponse)); + var document = new XmlDocument { XmlResolver = null }; + document.LoadXml(xml); + return document.GetElementsByTagName("EncryptedAssertion", AssertionNamespace).Count > 0; + } + catch (Exception ex) when (ex is FormatException or XmlException or DecoderFallbackException) + { + // A malformed response is rejected by Unbind further down; treat it as unencrypted here. + return false; + } + } + + private static bool IsDecryptionFailure(Exception exception) + { + for (var current = exception; current is not null; current = current.InnerException) + { + if (current is CryptographicException) + return true; + + if (current.Message.Contains("decrypt", StringComparison.OrdinalIgnoreCase)) + return true; + } + + return false; + } + + /// + /// Returns the first configured certificate that has passed its expiry, or null when + /// all of them are valid. An expired certificate produces an unusable signature or an assertion + /// that cannot be verified, so it is reported as its own error code rather than folded into a + /// generic failure. Each endpoint checks only the certificates it actually uses: login signs + /// with the SP key, while the callback also verifies against the IdP certificate. + /// + private (string Role, DateTime NotAfter)? FindExpiredCertificate(bool includeIdpCertificate = true) + { + var now = DateTime.Now; + + var spCert = LoadSpCertificate(); + if (spCert.NotAfter < now) + return ("service provider", spCert.NotAfter); + + if (!includeIdpCertificate) + return null; + + var idpCert = LoadIdpCertificate(); + if (idpCert.NotAfter < now) + return ("identity provider", idpCert.NotAfter); + + return null; + } + private Saml2Configuration BuildSamlConfiguration() { var spCert = LoadSpCertificate(); @@ -223,6 +341,10 @@ private Saml2Configuration BuildSamlConfiguration() }; config.SignatureValidationCertificates.Add(idpCert); + // The SP certificate is published in metadata under KeyDescriptor use="encryption", so an + // IdP may encrypt the assertion to it. Without a decryption certificate the assertion + // cannot be unwrapped and every such login fails. + config.DecryptionCertificates.Add(spCert); config.AllowedAudienceUris.Add(issuer); return config; diff --git a/src/backend/OpenOnboarding.Application.Tests/SamlAuthControllerTests.cs b/src/backend/OpenOnboarding.Application.Tests/SamlAuthControllerTests.cs index 40cbb0d..9b9b702 100644 --- a/src/backend/OpenOnboarding.Application.Tests/SamlAuthControllerTests.cs +++ b/src/backend/OpenOnboarding.Application.Tests/SamlAuthControllerTests.cs @@ -29,14 +29,15 @@ private sealed class TestCertPair public string PemKey { get; private init; } = ""; public static TestCertPair Generate() + => Generate(DateTimeOffset.UtcNow.AddYears(-1), DateTimeOffset.UtcNow.AddYears(10)); + + public static TestCertPair Generate(DateTimeOffset notBefore, DateTimeOffset notAfter) { using var rsa = RSA.Create(2048); var req = new CertificateRequest( "CN=test-saml", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); // Use a temp cert to export the key PEM; CreateSelfSigned disposes the key on some platforms - var cert = req.CreateSelfSigned( - DateTimeOffset.UtcNow.AddYears(-1), - DateTimeOffset.UtcNow.AddYears(10)); + var cert = req.CreateSelfSigned(notBefore, notAfter); return new TestCertPair { @@ -44,6 +45,9 @@ public static TestCertPair Generate() PemKey = rsa.ExportPkcs8PrivateKeyPem() }; } + + /// Public-only certificate, as an IdP would hold it for encrypting to the SP. + public X509Certificate2 PublicCertificate() => X509Certificate2.CreateFromPem(PemCert); } // ----------------------------------------------------------------------- @@ -51,25 +55,36 @@ public static TestCertPair Generate() // ----------------------------------------------------------------------- private static Dictionary SamlConfig( TestCertPair? certs = null, - string nameId = "admin@example.com") + string nameId = "admin@example.com", + bool? requireEncryptedAssertion = null, + TestCertPair? idpCerts = null) { var c = certs ?? SharedCerts; - return new Dictionary + var idp = idpCerts ?? c; + var config = new Dictionary { ["Authentication:Saml:Issuer"] = "test-sp", ["Authentication:Saml:IdpSsoUrl"] = "https://test-idp.local/sso", ["Authentication:Saml:SpCertificate"] = c.PemCert, ["Authentication:Saml:SpPrivateKey"] = c.PemKey, - // Same cert plays the IdP role in tests - ["Authentication:Saml:IdpCertificate"] = c.PemCert, + // Same cert plays the IdP role in tests unless a separate one is supplied + ["Authentication:Saml:IdpCertificate"] = idp.PemCert, ["Authentication:Saml:AllowedNameIds:0"] = nameId }; + + if (requireEncryptedAssertion is not null) + { + config["Authentication:Saml:RequireEncryptedAssertion"] = + requireEncryptedAssertion.Value ? "true" : "false"; + } + + return config; } // ----------------------------------------------------------------------- // Helper: build the IdP-side Saml2Configuration for generating test responses // ----------------------------------------------------------------------- - private static Saml2Configuration IdpConfig(TestCertPair certs) + private static Saml2Configuration IdpConfig(TestCertPair certs, X509Certificate2? encryptTo = null) { var cfg = new Saml2Configuration { @@ -81,6 +96,11 @@ private static Saml2Configuration IdpConfig(TestCertPair certs) RevocationMode = X509RevocationMode.NoCheck }; cfg.AllowedAudienceUris.Add("test-sp"); + + // When set, ITfoxtec wraps the assertion in encrypted to this key. + if (encryptTo is not null) + cfg.EncryptionCertificate = encryptTo; + return cfg; } @@ -517,4 +537,177 @@ private sealed class AuthMeResponse public bool Authenticated { get; init; } public string[]? Roles { get; init; } } + + // ======================================================================= + // 13. Encrypted assertion decrypts and signs in (metadata advertises + // KeyDescriptor use="encryption", so this must work) + // ======================================================================= + [Fact] + public async Task Callback_EncryptedAssertion_SignsInAndIssuesSession() + { + using var factory = TestWebAppFactory.Create(configurationOverrides: SamlConfig()); + using var client = RawClient(factory); + + var (relayState, authnId, rsCookieRaw, authnIdCookieRaw) = await DoLoginAsync(client); + + using var spPublic = SharedCerts.PublicCertificate(); + var samlResp = BuildSamlResponse(IdpConfig(SharedCerts, encryptTo: spPublic), authnId); + + // Guard the fixture itself: the response must really carry an EncryptedAssertion. + var xml = Encoding.UTF8.GetString(Convert.FromBase64String(samlResp)); + Assert.Contains("EncryptedAssertion", xml); + Assert.DoesNotContain("admin@example.com", xml); + + var resp = await PostCallbackAsync( + client, samlResp, relayState, rsCookieRaw, authnIdCookieRaw); + + Assert.Equal(HttpStatusCode.Redirect, resp.StatusCode); + Assert.Equal("/admin/journey-builder", resp.Headers.Location?.ToString()); + + var sessionCookie = ExtractCookieValue(resp, AdminSessionAuthenticationDefaults.CookieName); + Assert.NotNull(sessionCookie); + + var meReq = new HttpRequestMessage(HttpMethod.Get, "/api/auth/me"); + meReq.Headers.Add("Cookie", + $"{AdminSessionAuthenticationDefaults.CookieName}={sessionCookie}"); + var me = await (await client.SendAsync(meReq)).Content.ReadFromJsonAsync(); + + Assert.NotNull(me); + Assert.True(me!.Authenticated); + Assert.Contains("Operator", me.Roles ?? []); + } + + // ======================================================================= + // 14. Assertion encrypted to a key we do not hold -> saml_invalid_assertion + // ======================================================================= + [Fact] + public async Task Callback_EncryptedToUnknownKey_RedirectsToSamlInvalidAssertion() + { + using var factory = TestWebAppFactory.Create(configurationOverrides: SamlConfig()); + using var client = RawClient(factory); + + var (relayState, authnId, rsCookieRaw, authnIdCookieRaw) = await DoLoginAsync(client); + + // Encrypt to an unrelated certificate the SP has no private key for. + var strangerCerts = TestCertPair.Generate(); + using var strangerPublic = strangerCerts.PublicCertificate(); + var samlResp = BuildSamlResponse(IdpConfig(SharedCerts, encryptTo: strangerPublic), authnId); + + var resp = await PostCallbackAsync( + client, samlResp, relayState, rsCookieRaw, authnIdCookieRaw); + + Assert.Equal(HttpStatusCode.Redirect, resp.StatusCode); + Assert.Contains("error=saml_invalid_assertion", + resp.Headers.Location?.ToString() ?? ""); + } + + // ======================================================================= + // 15. RequireEncryptedAssertion=true rejects an unencrypted assertion + // ======================================================================= + [Fact] + public async Task Callback_UnencryptedAssertion_WhenEncryptionRequired_RedirectsToNotEncrypted() + { + using var factory = TestWebAppFactory.Create( + configurationOverrides: SamlConfig(requireEncryptedAssertion: true)); + using var client = RawClient(factory); + + var (relayState, authnId, rsCookieRaw, authnIdCookieRaw) = await DoLoginAsync(client); + + var samlResp = BuildSamlResponse(IdpConfig(SharedCerts), authnId); + + var resp = await PostCallbackAsync( + client, samlResp, relayState, rsCookieRaw, authnIdCookieRaw); + + Assert.Equal(HttpStatusCode.Redirect, resp.StatusCode); + Assert.Contains("error=saml_assertion_not_encrypted", + resp.Headers.Location?.ToString() ?? ""); + } + + // ======================================================================= + // 16. RequireEncryptedAssertion=true accepts an encrypted assertion + // ======================================================================= + [Fact] + public async Task Callback_EncryptedAssertion_WhenEncryptionRequired_SignsIn() + { + using var factory = TestWebAppFactory.Create( + configurationOverrides: SamlConfig(requireEncryptedAssertion: true)); + using var client = RawClient(factory); + + var (relayState, authnId, rsCookieRaw, authnIdCookieRaw) = await DoLoginAsync(client); + + using var spPublic = SharedCerts.PublicCertificate(); + var samlResp = BuildSamlResponse(IdpConfig(SharedCerts, encryptTo: spPublic), authnId); + + var resp = await PostCallbackAsync( + client, samlResp, relayState, rsCookieRaw, authnIdCookieRaw); + + Assert.Equal(HttpStatusCode.Redirect, resp.StatusCode); + Assert.NotNull(ExtractCookieValue(resp, AdminSessionAuthenticationDefaults.CookieName)); + } + + // ======================================================================= + // 17. RequireEncryptedAssertion=false keeps unencrypted assertions working + // ======================================================================= + [Fact] + public async Task Callback_UnencryptedAssertion_WhenEncryptionNotRequired_SignsIn() + { + using var factory = TestWebAppFactory.Create( + configurationOverrides: SamlConfig(requireEncryptedAssertion: false)); + using var client = RawClient(factory); + + var (relayState, authnId, rsCookieRaw, authnIdCookieRaw) = await DoLoginAsync(client); + + var samlResp = BuildSamlResponse(IdpConfig(SharedCerts), authnId); + + var resp = await PostCallbackAsync( + client, samlResp, relayState, rsCookieRaw, authnIdCookieRaw); + + Assert.Equal(HttpStatusCode.Redirect, resp.StatusCode); + Assert.NotNull(ExtractCookieValue(resp, AdminSessionAuthenticationDefaults.CookieName)); + } + + // ======================================================================= + // 18. Expired SP certificate -> saml_certificate_expired on login + // ======================================================================= + [Fact] + public async Task Login_ExpiredSpCertificate_RedirectsToCertificateExpired() + { + var expired = TestCertPair.Generate( + DateTimeOffset.UtcNow.AddYears(-2), DateTimeOffset.UtcNow.AddDays(-1)); + + using var factory = TestWebAppFactory.Create(configurationOverrides: SamlConfig(expired)); + using var client = RawClient(factory); + + var resp = await client.GetAsync("/auth/saml/login"); + + Assert.Equal(HttpStatusCode.Redirect, resp.StatusCode); + Assert.Contains("error=saml_certificate_expired", + resp.Headers.Location?.ToString() ?? ""); + } + + // ======================================================================= + // 19. Expired IdP certificate -> saml_certificate_expired on callback + // ======================================================================= + [Fact] + public async Task Callback_ExpiredIdpCertificate_RedirectsToCertificateExpired() + { + var expiredIdp = TestCertPair.Generate( + DateTimeOffset.UtcNow.AddYears(-2), DateTimeOffset.UtcNow.AddDays(-1)); + + // Valid SP certificate so login succeeds and the callback reaches the IdP cert check. + using var factory = TestWebAppFactory.Create( + configurationOverrides: SamlConfig(idpCerts: expiredIdp)); + using var client = RawClient(factory); + + var (relayState, authnId, rsCookieRaw, authnIdCookieRaw) = await DoLoginAsync(client); + + var samlResp = BuildSamlResponse(IdpConfig(expiredIdp), authnId); + + var resp = await PostCallbackAsync( + client, samlResp, relayState, rsCookieRaw, authnIdCookieRaw); + + Assert.Equal(HttpStatusCode.Redirect, resp.StatusCode); + Assert.Contains("error=saml_certificate_expired", + resp.Headers.Location?.ToString() ?? ""); + } } From 92f770c3bd49615489f86f8cccf5e0bc2c6c2aa2 Mon Sep 17 00:00:00 2001 From: MaximumTrainer Date: Sat, 12 Sep 2026 20:36:24 +0100 Subject: [PATCH 3/8] feat(sse): deliver session events across API replicas An SSE stream is pinned to the instance that accepted it, but InMemorySessionEventEmitter was the only ISessionEventEmitter and kept per-session channels in process memory. Above one replica an event raised on instance A never reached a stream held by instance B: no error, no closed stream, the applicant just stops receiving progress. Both CD workflows deploy to platforms that scale out by default. - Add ISessionEventTransport, a port for fanning an event out to every instance, with RabbitMqSessionEventTransport as the adapter (fanout exchange, per-instance exclusive auto-delete queue, transient messages). - Add DistributedSessionEventEmitter, which broadcasts over the transport and delivers to local channels on receipt. It subscribes during startup as an IHostedService, before the first request is served. - Extract the channel mechanics into SessionEventChannels so capacity bounds, terminal-event completion and stale-channel cleanup are the same code for both emitters rather than two implementations to keep in step. - Select by SessionEvents:Transport, mirroring how IEventBus is selected. The RabbitMQ URI falls back to the event bus URI, so a deployment already running RabbitMQ needs only the one setting. In-memory stays the default for development, tests and single-replica deployments. - Warn at startup when a non-Development environment uses the in-memory emitter, since the failure it risks is otherwise silent. Tests: 16 added, including two emitter instances sharing one transport where the event is raised on A and asserted on B's stream, terminal events completing streams on every instance, and capacity/cleanup behaviour asserted against both implementations from one theory. Suite 259/259. Closes #93 Co-Authored-By: Claude Opus 5 (1M context) --- docs/runbook.md | 21 +- ...ructureServiceCollectionExtensionsTests.cs | 57 +++++ .../SessionEventEmitterTests.cs | 229 ++++++++++++++++++ .../Interfaces/ISessionEventTransport.cs | 25 ++ .../ServiceCollectionExtensions.cs | 41 +++- .../EventBus/RabbitMqSessionEventTransport.cs | 125 ++++++++++ .../DistributedSessionEventEmitter.cs | 66 +++++ .../Services/InMemorySessionEventEmitter.cs | 60 +---- .../InMemorySessionEventEmitterWarning.cs | 32 +++ .../Services/SessionEventChannels.cs | 78 ++++++ 10 files changed, 682 insertions(+), 52 deletions(-) create mode 100644 src/backend/OpenOnboarding.Application.Tests/SessionEventEmitterTests.cs create mode 100644 src/backend/OpenOnboarding.Application/Interfaces/ISessionEventTransport.cs create mode 100644 src/backend/OpenOnboarding.Infrastructure/EventBus/RabbitMqSessionEventTransport.cs create mode 100644 src/backend/OpenOnboarding.Infrastructure/Services/DistributedSessionEventEmitter.cs create mode 100644 src/backend/OpenOnboarding.Infrastructure/Services/InMemorySessionEventEmitterWarning.cs create mode 100644 src/backend/OpenOnboarding.Infrastructure/Services/SessionEventChannels.cs diff --git a/docs/runbook.md b/docs/runbook.md index 6ab1a21..1cd2351 100644 --- a/docs/runbook.md +++ b/docs/runbook.md @@ -343,9 +343,24 @@ docker compose exec -T postgres psql -U postgres onboarding < backup-20241201.sq ### Horizontal Scaling (Multiple API Instances) -The API is **stateless** with one caveat: `InMemorySessionEventEmitter` (SSE) holds per-session event channels in memory. In a multi-instance setup: -- SSE clients must connect to the same instance that holds their session's channel, **or** -- Replace `InMemorySessionEventEmitter` with a distributed adapter (e.g., Redis Pub/Sub) +The API is **stateless** with one caveat: an SSE stream is pinned to the instance that accepted it, +so a session event raised while handling a request on another instance has to travel between +instances to reach that stream. + +**Running more than one replica requires a distributed session event transport.** Without one the +failure is silent: the stream stays open, no error is raised, and the applicant simply never +receives step progress until they reload. A rolling deploy or scale-in re-breaks it mid-journey, so +sticky sessions are not a substitute. + +| Variable | Required | Default | Description | +|----------|----------|---------|-------------| +| `SessionEvents__Transport` | ✅ for >1 replica | — | `rabbitmq` enables the distributed emitter. Unset uses the in-memory emitter. | +| `SessionEvents__RabbitMq__Uri` | optional | `EventBus__RabbitMq__Uri`, else `amqp://guest:guest@localhost:5672/` | Broker connection. Defaults to the event bus broker so an existing RabbitMQ deployment needs only `SessionEvents__Transport`. | +| `SessionEvents__RabbitMq__Exchange` | optional | `waymark-session-events` | Fanout exchange name. Each instance binds its own exclusive auto-delete queue. | + +A non-Development environment running the in-memory emitter logs a startup warning naming this +limitation. Events are transient and not persisted: they are only useful to a stream that is open +at the time, so an instance that was down missed the stream too. All other state is in PostgreSQL — safe for horizontal scaling. diff --git a/src/backend/OpenOnboarding.Application.Tests/InfrastructureServiceCollectionExtensionsTests.cs b/src/backend/OpenOnboarding.Application.Tests/InfrastructureServiceCollectionExtensionsTests.cs index ec26d3b..f63d1eb 100644 --- a/src/backend/OpenOnboarding.Application.Tests/InfrastructureServiceCollectionExtensionsTests.cs +++ b/src/backend/OpenOnboarding.Application.Tests/InfrastructureServiceCollectionExtensionsTests.cs @@ -37,6 +37,63 @@ public void AddInfrastructure_DoesNotRegisterConsoleAnalyticsProvider_WhenDisabl descriptor.ImplementationType == typeof(ConsoleAnalyticsProvider)); } + + [Fact] + public void AddInfrastructure_RegistersInMemorySessionEventEmitter_ByDefault() + { + var services = new ServiceCollection(); + + services.AddInfrastructure(BuildConfiguration()); + + Assert.Contains(services, descriptor => + descriptor.ServiceType == typeof(ISessionEventEmitter) && + descriptor.ImplementationType == typeof(InMemorySessionEventEmitter)); + + Assert.DoesNotContain(services, descriptor => + descriptor.ServiceType == typeof(ISessionEventTransport)); + } + + [Fact] + public void AddInfrastructure_RegistersDistributedSessionEventEmitter_WhenTransportConfigured() + { + var services = new ServiceCollection(); + var configuration = BuildConfiguration(new Dictionary + { + ["SessionEvents:Transport"] = "rabbitmq" + }); + + services.AddInfrastructure(configuration); + + Assert.Contains(services, descriptor => + descriptor.ServiceType == typeof(ISessionEventTransport)); + + Assert.Contains(services, descriptor => + descriptor.ServiceType == typeof(DistributedSessionEventEmitter)); + + Assert.DoesNotContain(services, descriptor => + descriptor.ServiceType == typeof(ISessionEventEmitter) && + descriptor.ImplementationType == typeof(InMemorySessionEventEmitter)); + } + + [Fact] + public void AddInfrastructure_RegistersStartupWarning_OnlyForTheInMemoryEmitter() + { + var withInMemory = new ServiceCollection(); + withInMemory.AddInfrastructure(BuildConfiguration()); + + Assert.Contains(withInMemory, descriptor => + descriptor.ImplementationType == typeof(InMemorySessionEventEmitterWarning)); + + var withTransport = new ServiceCollection(); + withTransport.AddInfrastructure(BuildConfiguration(new Dictionary + { + ["SessionEvents:Transport"] = "rabbitmq" + })); + + Assert.DoesNotContain(withTransport, descriptor => + descriptor.ImplementationType == typeof(InMemorySessionEventEmitterWarning)); + } + private static IConfiguration BuildConfiguration(Dictionary? overrides = null) { var values = new Dictionary diff --git a/src/backend/OpenOnboarding.Application.Tests/SessionEventEmitterTests.cs b/src/backend/OpenOnboarding.Application.Tests/SessionEventEmitterTests.cs new file mode 100644 index 0000000..530405e --- /dev/null +++ b/src/backend/OpenOnboarding.Application.Tests/SessionEventEmitterTests.cs @@ -0,0 +1,229 @@ +using System.Collections.Concurrent; +using Microsoft.Extensions.Logging.Abstractions; +using OpenOnboarding.Application.Interfaces; +using OpenOnboarding.Infrastructure.Services; + +namespace OpenOnboarding.Application.Tests; + +/// +/// Behaviour shared by every implementation, plus the fan-out +/// contract that only the distributed emitter satisfies. +/// +public sealed class SessionEventEmitterTests +{ + // ----------------------------------------------------------------------- + // In-process transport: stands in for a broker so two "instances" can be + // wired together without one. Fans every broadcast out to all subscribers, + // the publisher included, exactly as a RabbitMQ fanout exchange does. + // ----------------------------------------------------------------------- + private sealed class InProcessSessionEventTransport : ISessionEventTransport + { + private readonly ConcurrentBag> _handlers = []; + + public Task StartAsync(Func handler, CancellationToken cancellationToken = default) + { + _handlers.Add(handler); + return Task.CompletedTask; + } + + public async Task BroadcastAsync(Guid sessionId, SessionEvent sessionEvent, CancellationToken cancellationToken = default) + { + foreach (var handler in _handlers) + await handler(sessionId, sessionEvent); + } + + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + } + + private static DistributedSessionEventEmitter CreateDistributed(ISessionEventTransport transport) + => new(transport, NullLogger.Instance); + + private static InMemorySessionEventEmitter CreateInMemory() + => new(NullLogger.Instance); + + /// Reads up to events, giving up after a timeout. + private static async Task> ReadAsync( + ISessionEventEmitter emitter, Guid sessionId, int count, TimeSpan? timeout = null) + { + using var cts = new CancellationTokenSource(timeout ?? TimeSpan.FromSeconds(5)); + var collected = new List(); + + try + { + await foreach (var evt in emitter.SubscribeAsync(sessionId, cts.Token)) + { + collected.Add(evt); + if (collected.Count >= count) + break; + } + } + catch (OperationCanceledException) + { + // Timed out; return what arrived so the assertion reports the real shortfall. + } + + return collected; + } + + // ======================================================================= + // Fan-out: the defect this issue is about + // ======================================================================= + [Fact] + public async Task DistributedEmitter_EventRaisedOnInstanceA_ReachesStreamOnInstanceB() + { + await using var transport = new InProcessSessionEventTransport(); + + // Two API instances sharing one transport. + await using var instanceA = CreateDistributed(transport); + await using var instanceB = CreateDistributed(transport); + await instanceA.StartAsync(CancellationToken.None); + await instanceB.StartAsync(CancellationToken.None); + + var sessionId = Guid.NewGuid(); + + // The applicant's SSE stream is held by instance B. + var streamOnB = ReadAsync(instanceB, sessionId, count: 1); + + // Their step submission is handled by instance A. + await instanceA.EmitAsync(sessionId, "step-advanced", new { nodeKey = "address" }); + + var received = await streamOnB; + + var evt = Assert.Single(received); + Assert.Equal("step-advanced", evt.EventType); + Assert.Contains("address", evt.PayloadJson); + } + + [Fact] + public async Task InMemoryEmitter_EventRaisedOnInstanceA_DoesNotReachInstanceB() + { + // Documents the limitation the distributed emitter exists to remove: two in-memory + // emitters are two isolated processes. + var instanceA = CreateInMemory(); + var instanceB = CreateInMemory(); + var sessionId = Guid.NewGuid(); + + var streamOnB = ReadAsync(instanceB, sessionId, count: 1, timeout: TimeSpan.FromMilliseconds(300)); + await instanceA.EmitAsync(sessionId, "step-advanced", new { nodeKey = "address" }); + + Assert.Empty(await streamOnB); + } + + [Fact] + public async Task DistributedEmitter_DoesNotDeliverTwiceToTheEmittingInstance() + { + await using var transport = new InProcessSessionEventTransport(); + await using var instance = CreateDistributed(transport); + await instance.StartAsync(CancellationToken.None); + + var sessionId = Guid.NewGuid(); + var stream = ReadAsync(instance, sessionId, count: 2, timeout: TimeSpan.FromMilliseconds(500)); + + await instance.EmitAsync(sessionId, "step-advanced", new { step = 1 }); + + // Only one event was emitted; a second arrival would mean the transport echo was + // published locally as well. + var received = await stream; + Assert.Single(received); + } + + // ======================================================================= + // Terminal events close the stream on every instance + // ======================================================================= + [Theory] + [InlineData("session-completed")] + [InlineData("session-abandoned")] + public async Task DistributedEmitter_TerminalEvent_CompletesStreamOnEveryInstance(string terminalEvent) + { + await using var transport = new InProcessSessionEventTransport(); + await using var instanceA = CreateDistributed(transport); + await using var instanceB = CreateDistributed(transport); + await instanceA.StartAsync(CancellationToken.None); + await instanceB.StartAsync(CancellationToken.None); + + var sessionId = Guid.NewGuid(); + + // Ask for more events than will ever arrive: the stream must end on its own when the + // terminal event completes the channel, rather than running to the timeout. + var streamOnB = ReadAsync(instanceB, sessionId, count: 99, timeout: TimeSpan.FromSeconds(5)); + + await instanceA.EmitAsync(sessionId, terminalEvent, new { reason = "done" }); + + var received = await streamOnB; + var evt = Assert.Single(received); + Assert.Equal(terminalEvent, evt.EventType); + } + + [Theory] + [InlineData("session-completed")] + [InlineData("session-abandoned")] + public async Task InMemoryEmitter_TerminalEvent_CompletesStream(string terminalEvent) + { + var emitter = CreateInMemory(); + var sessionId = Guid.NewGuid(); + + var stream = ReadAsync(emitter, sessionId, count: 99, timeout: TimeSpan.FromSeconds(5)); + await emitter.EmitAsync(sessionId, terminalEvent, new { reason = "done" }); + + var evt = Assert.Single(await stream); + Assert.Equal(terminalEvent, evt.EventType); + } + + // ======================================================================= + // Capacity and cleanup behave identically for both implementations + // ======================================================================= + public static TheoryData EmitterKinds => new() { "in-memory", "distributed" }; + + private static async Task CreateAsync(string kind, ISessionEventTransport transport) + { + if (kind == "in-memory") + return CreateInMemory(); + + var emitter = CreateDistributed(transport); + await emitter.StartAsync(CancellationToken.None); + return emitter; + } + + [Theory] + [MemberData(nameof(EmitterKinds))] + public async Task Emitter_BoundedChannel_KeepsMostRecentEventsWhenNoSubscriberDrains(string kind) + { + await using var transport = new InProcessSessionEventTransport(); + var emitter = await CreateAsync(kind, transport); + var sessionId = Guid.NewGuid(); + + // Overfill the bounded channel with no subscriber attached. + var overflow = SessionEventChannels.ChannelCapacity + 20; + for (var i = 0; i < overflow; i++) + await emitter.EmitAsync(sessionId, "step-advanced", new { index = i }); + + var received = await ReadAsync( + emitter, sessionId, SessionEventChannels.ChannelCapacity, TimeSpan.FromSeconds(5)); + + // DropOldest: capacity retained, and the newest event survived. + Assert.Equal(SessionEventChannels.ChannelCapacity, received.Count); + Assert.Contains($"{overflow - 1}", received[^1].PayloadJson); + Assert.DoesNotContain(received, e => e.PayloadJson.Contains("\"index\":0}")); + } + + [Theory] + [MemberData(nameof(EmitterKinds))] + public async Task Emitter_DrainedSubscriber_LeavesNoStaleChannel(string kind) + { + await using var transport = new InProcessSessionEventTransport(); + var emitter = await CreateAsync(kind, transport); + var sessionId = Guid.NewGuid(); + + var stream = ReadAsync(emitter, sessionId, count: 99, timeout: TimeSpan.FromSeconds(5)); + await emitter.EmitAsync(sessionId, "session-completed", new { }); + await stream; + + // The channel was removed on drain, so a fresh subscriber gets a live channel rather than + // the completed one - it receives a subsequent event instead of ending immediately. + var secondStream = ReadAsync(emitter, sessionId, count: 1, timeout: TimeSpan.FromSeconds(5)); + await emitter.EmitAsync(sessionId, "step-advanced", new { step = "restarted" }); + + var evt = Assert.Single(await secondStream); + Assert.Equal("step-advanced", evt.EventType); + } +} diff --git a/src/backend/OpenOnboarding.Application/Interfaces/ISessionEventTransport.cs b/src/backend/OpenOnboarding.Application/Interfaces/ISessionEventTransport.cs new file mode 100644 index 0000000..fd4ea74 --- /dev/null +++ b/src/backend/OpenOnboarding.Application/Interfaces/ISessionEventTransport.cs @@ -0,0 +1,25 @@ +namespace OpenOnboarding.Application.Interfaces; + +/// +/// Port for a transport that fans a session event out to every API instance. +/// +/// Session progress is delivered to browsers over SSE, and an SSE stream is pinned to the single +/// instance that accepted it. An event raised while handling a request on another instance must +/// therefore travel between instances to reach that stream. This port is that hop; adapters +/// implement it over a broadcast-capable broker (RabbitMQ fanout, Redis pub/sub, ...). +/// +/// +public interface ISessionEventTransport : IAsyncDisposable +{ + /// + /// Begins delivering broadcast events to . Called once during + /// startup. The handler is invoked for events published by every instance, the caller's + /// own included, so a broadcast is delivered exactly once everywhere. + /// + Task StartAsync(Func handler, CancellationToken cancellationToken = default); + + /// + /// Publishes an event to every instance subscribed to the transport. + /// + Task BroadcastAsync(Guid sessionId, SessionEvent sessionEvent, CancellationToken cancellationToken = default); +} diff --git a/src/backend/OpenOnboarding.Infrastructure/DependencyInjection/ServiceCollectionExtensions.cs b/src/backend/OpenOnboarding.Infrastructure/DependencyInjection/ServiceCollectionExtensions.cs index 093f10f..32db886 100644 --- a/src/backend/OpenOnboarding.Infrastructure/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/backend/OpenOnboarding.Infrastructure/DependencyInjection/ServiceCollectionExtensions.cs @@ -64,7 +64,7 @@ public static IServiceCollection AddInfrastructure(this IServiceCollection servi if (configuration.GetValue("DocumentStorage:EnableCleanup", true)) services.AddHostedService(); - services.AddSingleton(); + AddSessionEventEmitter(services, configuration); services.AddHttpClient("Webhook"); services.AddScoped(); @@ -120,4 +120,43 @@ public static IServiceCollection AddInfrastructure(this IServiceCollection servi return services; } + + /// + /// Selects the session event emitter. SSE streams are pinned to the instance that accepted + /// them, so with more than one replica an event raised elsewhere never reaches the stream + /// unless a distributed transport carries it. Configure SessionEvents:Transport to + /// rabbitmq for multi-replica deployments; the in-memory emitter remains the default + /// for local development, tests and single-replica deployments. + /// + private static void AddSessionEventEmitter(IServiceCollection services, IConfiguration configuration) + { + var transport = configuration.GetValue("SessionEvents:Transport"); + + if (transport?.Equals("rabbitmq", StringComparison.OrdinalIgnoreCase) == true) + { + // Fall back to the event bus connection so a deployment that already runs RabbitMQ + // needs only SessionEvents:Transport to turn this on. + var uri = configuration.GetValue("SessionEvents:RabbitMq:Uri") + ?? configuration.GetValue("EventBus:RabbitMq:Uri") + ?? "amqp://guest:guest@localhost:5672/"; + var exchange = configuration.GetValue("SessionEvents:RabbitMq:Exchange") + ?? "waymark-session-events"; + + services.AddSingleton(sp => + new RabbitMqSessionEventTransport( + uri, + exchange, + sp.GetRequiredService>())); + + services.AddSingleton(); + services.AddSingleton(sp => + sp.GetRequiredService()); + services.AddSingleton(sp => + sp.GetRequiredService()); + return; + } + + services.AddSingleton(); + services.AddSingleton(); + } } diff --git a/src/backend/OpenOnboarding.Infrastructure/EventBus/RabbitMqSessionEventTransport.cs b/src/backend/OpenOnboarding.Infrastructure/EventBus/RabbitMqSessionEventTransport.cs new file mode 100644 index 0000000..457fdff --- /dev/null +++ b/src/backend/OpenOnboarding.Infrastructure/EventBus/RabbitMqSessionEventTransport.cs @@ -0,0 +1,125 @@ +using System.Text.Json; +using Microsoft.Extensions.Logging; +using OpenOnboarding.Application.Interfaces; +using RabbitMQ.Client; +using RabbitMQ.Client.Events; + +namespace OpenOnboarding.Infrastructure.EventBus; + +/// +/// Fans session events out to every API instance over a RabbitMQ fanout exchange. +/// +/// Each instance declares its own exclusive, auto-delete queue bound to the shared exchange, so a +/// published event reaches every running instance and leaves nothing behind when one stops. +/// Messages are transient: a session event is only useful to a stream that is open right now, and +/// an instance that was down missed the stream too. +/// +/// +public sealed class RabbitMqSessionEventTransport : ISessionEventTransport +{ + private static readonly JsonSerializerOptions JsonOptions = new(JsonSerializerDefaults.Web); + + private readonly string _connectionString; + private readonly string _exchangeName; + private readonly ILogger _logger; + + private IConnection? _connection; + private IChannel? _channel; + + public RabbitMqSessionEventTransport( + string connectionString, + string exchangeName, + ILogger logger) + { + _connectionString = connectionString; + _exchangeName = exchangeName; + _logger = logger; + } + + /// Wire envelope. Kept separate from so the session id travels with it. + internal sealed record Envelope(Guid SessionId, string EventType, string PayloadJson, DateTimeOffset Timestamp) + { + public static Envelope From(Guid sessionId, SessionEvent evt) + => new(sessionId, evt.EventType, evt.PayloadJson, evt.Timestamp); + + public SessionEvent ToSessionEvent() => new(EventType, PayloadJson, Timestamp); + } + + internal static byte[] Serialize(Guid sessionId, SessionEvent evt) + => JsonSerializer.SerializeToUtf8Bytes(Envelope.From(sessionId, evt), JsonOptions); + + internal static Envelope? Deserialize(ReadOnlySpan body) + => JsonSerializer.Deserialize(body, JsonOptions); + + public async Task StartAsync(Func handler, CancellationToken cancellationToken = default) + { + var factory = new ConnectionFactory { Uri = new Uri(_connectionString) }; + _connection = await factory.CreateConnectionAsync(cancellationToken); + _channel = await _connection.CreateChannelAsync(cancellationToken: cancellationToken); + + await _channel.ExchangeDeclareAsync( + _exchangeName, ExchangeType.Fanout, durable: true, autoDelete: false, + cancellationToken: cancellationToken); + + // Exclusive + auto-delete: this queue belongs to this instance and disappears with it. + var queue = await _channel.QueueDeclareAsync( + queue: string.Empty, durable: false, exclusive: true, autoDelete: true, + cancellationToken: cancellationToken); + + await _channel.QueueBindAsync( + queue.QueueName, _exchangeName, routingKey: string.Empty, + cancellationToken: cancellationToken); + + var consumer = new AsyncEventingBasicConsumer(_channel); + consumer.ReceivedAsync += async (_, args) => + { + try + { + var envelope = Deserialize(args.Body.Span); + if (envelope is not null) + await handler(envelope.SessionId, envelope.ToSessionEvent()); + } + catch (Exception ex) + { + // A poison message must not tear down the consumer; the stream stays live for + // every other session. + _logger.LogError(ex, "Failed to deliver a session event received from RabbitMQ."); + } + }; + + await _channel.BasicConsumeAsync( + queue.QueueName, autoAck: true, consumer, cancellationToken: cancellationToken); + + _logger.LogInformation( + "Subscribed to session event exchange {Exchange} on queue {Queue}.", + _exchangeName, queue.QueueName); + } + + public async Task BroadcastAsync( + Guid sessionId, + SessionEvent sessionEvent, + CancellationToken cancellationToken = default) + { + if (_channel is null) + throw new InvalidOperationException("StartAsync must be called before broadcasting session events."); + + var props = new BasicProperties { Persistent = false, ContentType = "application/json" }; + + await _channel.BasicPublishAsync( + _exchangeName, + routingKey: string.Empty, + mandatory: false, + basicProperties: props, + body: Serialize(sessionId, sessionEvent), + cancellationToken: cancellationToken); + } + + public async ValueTask DisposeAsync() + { + if (_channel is not null) + await _channel.DisposeAsync(); + + if (_connection is not null) + await _connection.DisposeAsync(); + } +} diff --git a/src/backend/OpenOnboarding.Infrastructure/Services/DistributedSessionEventEmitter.cs b/src/backend/OpenOnboarding.Infrastructure/Services/DistributedSessionEventEmitter.cs new file mode 100644 index 0000000..33594c5 --- /dev/null +++ b/src/backend/OpenOnboarding.Infrastructure/Services/DistributedSessionEventEmitter.cs @@ -0,0 +1,66 @@ +using System.Text.Json; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using OpenOnboarding.Application.Interfaces; + +namespace OpenOnboarding.Infrastructure.Services; + +/// +/// Session event emitter for multi-replica deployments. Every event is broadcast over an +/// and delivered to the local SSE channels of every instance, +/// so a stream held by one replica still receives events raised on another. +/// +/// Registered as an so the transport subscription is established +/// during startup, before the first request is served. +/// +/// +public sealed class DistributedSessionEventEmitter : ISessionEventEmitter, IHostedService, IAsyncDisposable +{ + private static readonly JsonSerializerOptions JsonOptions = new(JsonSerializerDefaults.Web); + + private readonly ISessionEventTransport _transport; + private readonly ILogger _logger; + private readonly SessionEventChannels _channels; + + public DistributedSessionEventEmitter( + ISessionEventTransport transport, + ILogger logger) + { + _transport = transport; + _logger = logger; + _channels = new SessionEventChannels(logger); + } + + public async Task StartAsync(CancellationToken cancellationToken) + { + await _transport.StartAsync(DeliverLocallyAsync, cancellationToken); + _logger.LogInformation( + "Distributed session event transport started; SSE streams will receive events raised on any instance."); + } + + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + public async Task EmitAsync( + Guid sessionId, + string eventType, + object payload, + CancellationToken cancellationToken = default) + { + var evt = new SessionEvent(eventType, JsonSerializer.Serialize(payload, JsonOptions), DateTimeOffset.UtcNow); + + // Broadcast only. The transport echoes back to this instance, so publishing locally here + // as well would deliver the event twice to a stream held by the emitting instance. + await _transport.BroadcastAsync(sessionId, evt, cancellationToken); + } + + public IAsyncEnumerable SubscribeAsync(Guid sessionId, CancellationToken cancellationToken = default) + => _channels.SubscribeAsync(sessionId, cancellationToken); + + private Task DeliverLocallyAsync(Guid sessionId, SessionEvent sessionEvent) + { + _channels.Publish(sessionId, sessionEvent); + return Task.CompletedTask; + } + + public ValueTask DisposeAsync() => _transport.DisposeAsync(); +} diff --git a/src/backend/OpenOnboarding.Infrastructure/Services/InMemorySessionEventEmitter.cs b/src/backend/OpenOnboarding.Infrastructure/Services/InMemorySessionEventEmitter.cs index e7ec2e9..ee785e0 100644 --- a/src/backend/OpenOnboarding.Infrastructure/Services/InMemorySessionEventEmitter.cs +++ b/src/backend/OpenOnboarding.Infrastructure/Services/InMemorySessionEventEmitter.cs @@ -1,69 +1,33 @@ -using System.Collections.Concurrent; -using System.Runtime.CompilerServices; using System.Text.Json; -using System.Threading.Channels; using Microsoft.Extensions.Logging; using OpenOnboarding.Application.Interfaces; namespace OpenOnboarding.Infrastructure.Services; +/// +/// Single-instance session event emitter. Events are delivered only to SSE streams held by this +/// process, so it is correct for local development, tests, and a deployment pinned to one replica. +/// Scale beyond one replica requires ; see +/// SessionEvents:Transport in the runbook. +/// public sealed class InMemorySessionEventEmitter : ISessionEventEmitter { - private const int ChannelCapacity = 100; - private readonly ConcurrentDictionary> _channels = new(); - private readonly ILogger _logger; private static readonly JsonSerializerOptions JsonOptions = new(JsonSerializerDefaults.Web); + private readonly SessionEventChannels _channels; + public InMemorySessionEventEmitter(ILogger logger) { - _logger = logger; + _channels = new SessionEventChannels(logger); } public Task EmitAsync(Guid sessionId, string eventType, object payload, CancellationToken cancellationToken = default) { - var channel = _channels.GetOrAdd(sessionId, _ => CreateChannel()); var evt = new SessionEvent(eventType, JsonSerializer.Serialize(payload, JsonOptions), DateTimeOffset.UtcNow); - - if (channel.Reader.Count >= ChannelCapacity) - _logger.LogWarning("Session event channel for {SessionId} is full; oldest event dropped.", sessionId); - - // If the existing channel is already completed, replace it with a fresh one - if (!channel.Writer.TryWrite(evt)) - { - var fresh = CreateChannel(); - _channels[sessionId] = fresh; - fresh.Writer.TryWrite(evt); - channel = fresh; - } - - if (eventType is "session-completed" or "session-abandoned") - { - channel.Writer.TryComplete(); - // Channel stays in dict until SubscribeAsync drains it, preventing a new empty channel - // being returned to a subscriber that arrives just after completion. - } - + _channels.Publish(sessionId, evt); return Task.CompletedTask; } - public async IAsyncEnumerable SubscribeAsync(Guid sessionId, [EnumeratorCancellation] CancellationToken cancellationToken = default) - { - var channel = _channels.GetOrAdd(sessionId, _ => CreateChannel()); - - await foreach (var evt in channel.Reader.ReadAllAsync(cancellationToken)) - { - yield return evt; - } - - // Remove the entry after the subscriber has drained it so no stale channels accumulate. - _channels.TryRemove(sessionId, out _); - } - - private static Channel CreateChannel() => - Channel.CreateBounded(new BoundedChannelOptions(ChannelCapacity) - { - FullMode = BoundedChannelFullMode.DropOldest, - SingleReader = false, - SingleWriter = false - }); + public IAsyncEnumerable SubscribeAsync(Guid sessionId, CancellationToken cancellationToken = default) + => _channels.SubscribeAsync(sessionId, cancellationToken); } diff --git a/src/backend/OpenOnboarding.Infrastructure/Services/InMemorySessionEventEmitterWarning.cs b/src/backend/OpenOnboarding.Infrastructure/Services/InMemorySessionEventEmitterWarning.cs new file mode 100644 index 0000000..02f1c2f --- /dev/null +++ b/src/backend/OpenOnboarding.Infrastructure/Services/InMemorySessionEventEmitterWarning.cs @@ -0,0 +1,32 @@ +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; + +namespace OpenOnboarding.Infrastructure.Services; + +/// +/// Warns at startup when a non-Development environment is running the single-instance session +/// event emitter. The failure it guards against is silent — an SSE stream held by another replica +/// simply never receives progress — so it is worth naming loudly in the logs of any deployment +/// that might scale out. +/// +public sealed class InMemorySessionEventEmitterWarning( + IHostEnvironment environment, + ILogger logger) : IHostedService +{ + public Task StartAsync(CancellationToken cancellationToken) + { + if (!environment.IsDevelopment() && !environment.IsEnvironment("Testing")) + { + logger.LogWarning( + "Session events are using the in-memory emitter in the {Environment} environment. " + + "SSE progress is delivered only by the instance that raised the event, so running " + + "more than one replica will silently stop applicants receiving step progress. " + + "Set SessionEvents:Transport=rabbitmq for multi-replica deployments.", + environment.EnvironmentName); + } + + return Task.CompletedTask; + } + + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; +} diff --git a/src/backend/OpenOnboarding.Infrastructure/Services/SessionEventChannels.cs b/src/backend/OpenOnboarding.Infrastructure/Services/SessionEventChannels.cs new file mode 100644 index 0000000..be41b61 --- /dev/null +++ b/src/backend/OpenOnboarding.Infrastructure/Services/SessionEventChannels.cs @@ -0,0 +1,78 @@ +using System.Collections.Concurrent; +using System.Runtime.CompilerServices; +using System.Threading.Channels; +using Microsoft.Extensions.Logging; +using OpenOnboarding.Application.Interfaces; + +namespace OpenOnboarding.Infrastructure.Services; + +/// +/// Per-session in-process delivery channels shared by every +/// implementation. An SSE stream is always served from the channels of the instance holding it; +/// what differs between implementations is only how an event reaches this instance. +/// +/// Keeping the channel mechanics here means capacity bounds, terminal-event completion and +/// stale-channel cleanup behave identically whether the event was raised locally or arrived over +/// a distributed transport. +/// +/// +public sealed class SessionEventChannels(ILogger logger) +{ + public const int ChannelCapacity = 100; + + private readonly ConcurrentDictionary> _channels = new(); + + /// Event types after which no further event can arrive for the session. + public static bool IsTerminal(string eventType) + => eventType is "session-completed" or "session-abandoned"; + + /// + /// Delivers an event to this instance's subscriber for the session, if any. + /// + public void Publish(Guid sessionId, SessionEvent sessionEvent) + { + var channel = _channels.GetOrAdd(sessionId, _ => CreateChannel()); + + if (channel.Reader.Count >= ChannelCapacity) + logger.LogWarning("Session event channel for {SessionId} is full; oldest event dropped.", sessionId); + + // If the existing channel is already completed, replace it with a fresh one + if (!channel.Writer.TryWrite(sessionEvent)) + { + var fresh = CreateChannel(); + _channels[sessionId] = fresh; + fresh.Writer.TryWrite(sessionEvent); + channel = fresh; + } + + if (IsTerminal(sessionEvent.EventType)) + { + channel.Writer.TryComplete(); + // Channel stays in dict until SubscribeAsync drains it, preventing a new empty channel + // being returned to a subscriber that arrives just after completion. + } + } + + public async IAsyncEnumerable SubscribeAsync( + Guid sessionId, + [EnumeratorCancellation] CancellationToken cancellationToken = default) + { + var channel = _channels.GetOrAdd(sessionId, _ => CreateChannel()); + + await foreach (var evt in channel.Reader.ReadAllAsync(cancellationToken)) + { + yield return evt; + } + + // Remove the entry after the subscriber has drained it so no stale channels accumulate. + _channels.TryRemove(sessionId, out _); + } + + private static Channel CreateChannel() => + Channel.CreateBounded(new BoundedChannelOptions(ChannelCapacity) + { + FullMode = BoundedChannelFullMode.DropOldest, + SingleReader = false, + SingleWriter = false + }); +} From e72f93ba28f293af66f472ba30b8684dc9baac18 Mon Sep 17 00:00:00 2001 From: MaximumTrainer Date: Sat, 12 Sep 2026 21:19:25 +0100 Subject: [PATCH 4/8] feat(auth): replace the browser API key with a per-session applicant token VITE_API_KEY was inlined into the published bundle by Vite, and ApiKeyAuthenticationHandler maps any valid key to a full Operator principal. Every visitor to the public onboarding page could read the key out of the JavaScript and then list all sessions, read every submission's PII, rewrite flow definitions, or register a webhook to exfiltrate future events. SessionOwnershipHandler short-circuits on IsInRole(Operator), so per-session checks were no defence either. Backend: - POST /api/workflow/sessions/start is now anonymous - it is the entry point of a public journey, so there is no credential to present yet. It keeps the existing session-start rate limit and returns an applicant token scoped to the session it just created. - Add ApplicantSessionTokenService and an ApplicantSession scheme. The token is an HMAC-signed JWT naming one session and carrying only the Applicant role. The policy scheme routes bearer tokens by issuer, so applicant tokens and external IdP tokens coexist. - SessionOwnershipHandler accepts a token whose sessionId claim matches the session, which does not depend on a customer profile existing. - Require Authentication:ApplicantToken:SigningKey outside Development, mirroring JwtAuthorityValidator; Development uses an ephemeral key. - Enforce ownership on GET sessions/{id}/events, which had none: any Applicant or Operator could stream any session. EventSource cannot set headers, so that endpoint also accepts the token as an access_token query parameter, checked identically. - Pin Microsoft.IdentityModel.JsonWebTokens to match the Tokens version ITfoxtec pulls; the mismatched pair threw MissingMethodException when reading a JWT. Frontend: - Applicant requests carry the session token; operator surfaces go through operatorFetch, which sends the AdminSession cookie and no API key. - Drop the apiKey prop from every component and VITE_API_KEY from source and .env.example. Contracts: credentials are no longer recorded in the Pact interactions - the consumer's token means nothing to the provider - and the provider verifier injects a real token minted for the seeded session. Verified: a production build with VITE_API_KEY set contains neither the key nor any X-Api-Key header. Backend 272/272, frontend 117/117, mocked e2e journeys 3/3. 13 integration tests cover the token being accepted for its own session and rejected for another session, another session's event stream, and every operator endpoint. Closes #90 Co-Authored-By: Claude Opus 5 (1M context) --- docs/runbook.md | 38 ++- .../ApplicantSessionAuthenticationDefaults.cs | 22 ++ .../ApplicantSessionAuthenticationHandler.cs | 57 ++++ .../ApplicantSessionTokenService.cs | 116 ++++++++ .../Authorization/SessionOwnershipHandler.cs | 17 +- .../ApplicantTokenKeyValidator.cs | 33 ++ .../Controllers/WorkflowController.cs | 40 ++- .../OpenOnboarding.Api.csproj | 61 ++-- src/backend/OpenOnboarding.Api/Program.cs | 30 +- .../OpenOnboarding.Api/appsettings.json | 4 + .../ApplicantSessionTokenTests.cs | 281 ++++++++++++++++++ .../Contracts/SessionStartResponse.cs | 40 +++ .../Providers/WorkflowProviderTests.cs | 11 +- src/frontend/.env.example | 13 +- src/frontend/e2e/onboarding-journeys.spec.ts | 15 +- src/frontend/src/App.tsx | 10 +- src/frontend/src/analytics/FlowAnalytics.tsx | 9 +- src/frontend/src/api/operator-api.ts | 16 + .../src/builder/AdminJourneyBuilderPage.tsx | 13 +- .../src/builder/FlowAuthoringPanel.tsx | 15 +- src/frontend/src/flows/FlowVersionHistory.tsx | 12 +- .../onboarding/api/applicant-session.test.ts | 87 ++++++ .../src/onboarding/api/applicant-session.ts | 65 ++++ .../src/onboarding/api/workflow-api-client.ts | 33 +- .../onboarding/components/StepRenderer.tsx | 14 +- src/frontend/src/onboarding/hooks/useFlow.ts | 8 +- .../src/onboarding/hooks/useOnboarding.ts | 11 +- src/frontend/src/onboarding/types/flow.ts | 6 + .../src/pact/workflow.consumer.test.ts | 23 +- src/frontend/src/sessions/SessionDetail.tsx | 13 +- src/frontend/src/sessions/SessionList.tsx | 13 +- .../src/webhooks/WebhookDeliveries.tsx | 24 +- 32 files changed, 990 insertions(+), 160 deletions(-) create mode 100644 src/backend/OpenOnboarding.Api/Authentication/ApplicantSessionAuthenticationDefaults.cs create mode 100644 src/backend/OpenOnboarding.Api/Authentication/ApplicantSessionAuthenticationHandler.cs create mode 100644 src/backend/OpenOnboarding.Api/Authentication/ApplicantSessionTokenService.cs create mode 100644 src/backend/OpenOnboarding.Api/Configuration/ApplicantTokenKeyValidator.cs create mode 100644 src/backend/OpenOnboarding.Application.Tests/ApplicantSessionTokenTests.cs create mode 100644 src/backend/OpenOnboarding.Application/Contracts/SessionStartResponse.cs create mode 100644 src/frontend/src/api/operator-api.ts create mode 100644 src/frontend/src/onboarding/api/applicant-session.test.ts create mode 100644 src/frontend/src/onboarding/api/applicant-session.ts diff --git a/docs/runbook.md b/docs/runbook.md index 1cd2351..1724a26 100644 --- a/docs/runbook.md +++ b/docs/runbook.md @@ -100,13 +100,45 @@ All configuration can be set via environment variables or `appsettings.json`. | `ConnectionStrings__OnboardingDb` | ✅ | — | PostgreSQL connection string | | `Authentication__JwtAuthority` | ✅ (non-dev) | — | OIDC issuer URL. Empty string = JWT disabled (dev only). | | `Authentication__JwtAudience` | ✅ (non-dev) | — | Expected JWT audience claim. | -| `Authentication__ApiKey` | optional | — | Shared API key for X-Api-Key header auth. | -| `ApiKeys__operator` | optional | — | Per-role API keys. Key = role name, value = key. | -| `ApiKeys__applicant` | optional | — | Applicant-role API key. | +| `Authentication__ApiKey` | optional | — | **Server-to-server only.** Grants a full Operator principal — never send it from a browser. See [API credentials](#api-credentials). | +| `Authentication__ApplicantToken__SigningKey` | ✅ (non-dev) | — | HMAC key signing per-session applicant tokens, min 32 chars. **Store as a secret.** Startup fails without it outside Development. | +| `Authentication__ApplicantToken__LifetimeMinutes` | optional | `SessionTimeoutMinutes` | Applicant token lifetime. Defaults to the session timeout, since the token is useless once its session is abandoned. | | `SessionTimeoutMinutes` | optional | `60` | Inactivity timeout before sessions are auto-abandoned. | | `Logging__LogLevel__Default` | optional | `Information` | Log verbosity. | | `ASPNETCORE_ENVIRONMENT` | optional | `Production` | `Development`, `Staging`, or `Production`. | +### API credentials + +Three credentials reach the API, and they are not interchangeable. + +| Credential | Who holds it | Grants | +|------------|--------------|--------| +| Applicant session token | The applicant's browser | The one session named in the token. Nothing else. | +| `AdminSession` cookie | An operator's browser, after SAML SSO | Operator role, subject to the SSO NameID allowlist. | +| `Authentication__ApiKey` | Server-to-server integrations | A full Operator principal. | + +**The API key must never be sent from a browser.** It maps to an Operator principal with no further +checks, so a key present in a page is a key every visitor to that page holds. The public onboarding +app does not use it: it starts a session anonymously and is handed a token for that session. + +#### Applicant session tokens + +`POST /api/workflow/sessions/start` is anonymous — it is the entry point of a public journey, so +there is no credential to present yet. It is rate limited by the `session-start` policy. The +response carries `applicantToken`, an HMAC-signed JWT the browser sends as +`Authorization: Bearer ` for the rest of the journey. + +- **Scope.** The token names one session. It is rejected for any other session, and for every + operator endpoint, including the submissions readout for its own session. +- **Lifetime.** Defaults to `SessionTimeoutMinutes`. A shorter life only strands applicants + mid-journey; a longer one outlives the session it names, which is abandoned by then anyway. +- **Signing key.** `Authentication__ApplicantToken__SigningKey` must be set outside Development or + startup fails. All replicas need the same key, or a token issued by one is rejected by another. + In Development an ephemeral key is generated per process, so tokens stop working on restart. +- **SSE.** The browser `EventSource` API cannot set request headers, so + `GET /api/workflow/sessions/{id}/events` also accepts the token as an `access_token` query + parameter. Ownership is enforced identically on both paths. + ### SAML Single Sign-On (Admin UI) The admin UI signs operators in over SAML 2.0 (`GET /auth/saml/login` → IdP → `POST /auth/saml/callback`). diff --git a/src/backend/OpenOnboarding.Api/Authentication/ApplicantSessionAuthenticationDefaults.cs b/src/backend/OpenOnboarding.Api/Authentication/ApplicantSessionAuthenticationDefaults.cs new file mode 100644 index 0000000..89ad580 --- /dev/null +++ b/src/backend/OpenOnboarding.Api/Authentication/ApplicantSessionAuthenticationDefaults.cs @@ -0,0 +1,22 @@ +namespace OpenOnboarding.Api.Authentication; + +public static class ApplicantSessionAuthenticationDefaults +{ + public const string SchemeName = "ApplicantSession"; + + /// Issuer stamped on tokens this API mints, used to tell them from external IdP tokens. + public const string Issuer = "waymark-applicant"; + + public const string Audience = "waymark-applicant-session"; + + /// Claim carrying the single session the token is allowed to act on. + public const string SessionIdClaim = "sessionId"; + + public const string CustomerProfileIdClaim = "customerProfileId"; + + /// + /// Query parameter accepted in place of the Authorization header. The browser EventSource API + /// cannot set request headers, so the SSE endpoint has no other way to present a credential. + /// + public const string QueryParameterName = "access_token"; +} diff --git a/src/backend/OpenOnboarding.Api/Authentication/ApplicantSessionAuthenticationHandler.cs b/src/backend/OpenOnboarding.Api/Authentication/ApplicantSessionAuthenticationHandler.cs new file mode 100644 index 0000000..2fd72c9 --- /dev/null +++ b/src/backend/OpenOnboarding.Api/Authentication/ApplicantSessionAuthenticationHandler.cs @@ -0,0 +1,57 @@ +using System.Security.Claims; +using System.Text.Encodings.Web; +using Microsoft.AspNetCore.Authentication; +using Microsoft.Extensions.Options; + +namespace OpenOnboarding.Api.Authentication; + +/// +/// Authenticates the per-session applicant credential issued at session start, presented either as +/// Authorization: Bearer <token> or, for the SSE endpoint that the browser EventSource +/// API cannot add headers to, as an access_token query parameter. +/// +public sealed class ApplicantSessionAuthenticationHandler( + IOptionsMonitor options, + ILoggerFactory logger, + UrlEncoder encoder, + ApplicantSessionTokenService tokenService) + : AuthenticationHandler(options, logger, encoder) +{ + public const string SchemeName = ApplicantSessionAuthenticationDefaults.SchemeName; + + /// + /// Extracts the token from the request, or null when none is present. + /// + public static string? ExtractToken(HttpRequest request) + { + var header = request.Headers.Authorization.FirstOrDefault(); + if (!string.IsNullOrWhiteSpace(header) && + header.StartsWith("Bearer ", StringComparison.OrdinalIgnoreCase)) + { + var token = header["Bearer ".Length..].Trim(); + if (!string.IsNullOrWhiteSpace(token)) + return token; + } + + var queryToken = request.Query[ApplicantSessionAuthenticationDefaults.QueryParameterName] + .FirstOrDefault(); + + return string.IsNullOrWhiteSpace(queryToken) ? null : queryToken; + } + + protected override async Task HandleAuthenticateAsync() + { + var token = ExtractToken(Request); + if (token is null) + return AuthenticateResult.NoResult(); + + var result = await tokenService.ValidateAsync(token); + if (!result.IsValid) + return AuthenticateResult.Fail("Invalid applicant session token."); + + var identity = new ClaimsIdentity(result.ClaimsIdentity.Claims, Scheme.Name, ClaimTypes.Name, ClaimTypes.Role); + var principal = new ClaimsPrincipal(identity); + + return AuthenticateResult.Success(new AuthenticationTicket(principal, Scheme.Name)); + } +} diff --git a/src/backend/OpenOnboarding.Api/Authentication/ApplicantSessionTokenService.cs b/src/backend/OpenOnboarding.Api/Authentication/ApplicantSessionTokenService.cs new file mode 100644 index 0000000..d4b100b --- /dev/null +++ b/src/backend/OpenOnboarding.Api/Authentication/ApplicantSessionTokenService.cs @@ -0,0 +1,116 @@ +using System.Security.Claims; +using System.Security.Cryptography; +using System.Text; +using Microsoft.IdentityModel.JsonWebTokens; +using Microsoft.IdentityModel.Tokens; +using OpenOnboarding.Api.Authorization; + +namespace OpenOnboarding.Api.Authentication; + +/// +/// Issues and validates the per-session credential handed to the applicant's browser when a session +/// starts. +/// +/// The token authorises exactly one session: it carries that session's id and the +/// role only, so a leaked token exposes the journey its holder was +/// already completing and nothing else. This is what lets the public onboarding app stop shipping a +/// shared operator API key to every visitor. +/// +/// +public sealed class ApplicantSessionTokenService +{ + private readonly SymmetricSecurityKey _signingKey; + private readonly int _lifetimeMinutes; + private static readonly JsonWebTokenHandler Handler = new(); + + public ApplicantSessionTokenService(IConfiguration configuration) + { + _signingKey = new SymmetricSecurityKey(ResolveSigningKey(configuration)); + + // Default to the session timeout: the token is useless once the session it names is + // abandoned, so a shorter life would only strand applicants mid-journey. + var configured = configuration.GetValue("Authentication:ApplicantToken:LifetimeMinutes"); + var sessionTimeout = configuration.GetValue("SessionTimeoutMinutes"); + _lifetimeMinutes = configured is > 0 ? configured.Value + : sessionTimeout is > 0 ? sessionTimeout.Value + : 1440; + } + + public TimeSpan Lifetime => TimeSpan.FromMinutes(_lifetimeMinutes); + + public (string Token, DateTimeOffset ExpiresAt) Issue(Guid sessionId, Guid? customerProfileId) + { + var expiresAt = DateTimeOffset.UtcNow.Add(Lifetime); + + var claims = new Dictionary + { + [JwtRegisteredClaimNames.Sub] = sessionId.ToString(), + [ApplicantSessionAuthenticationDefaults.SessionIdClaim] = sessionId.ToString(), + [ClaimTypes.Role] = AppRoles.Applicant + }; + + if (customerProfileId is not null) + claims[ApplicantSessionAuthenticationDefaults.CustomerProfileIdClaim] = customerProfileId.Value.ToString(); + + var descriptor = new SecurityTokenDescriptor + { + Issuer = ApplicantSessionAuthenticationDefaults.Issuer, + Audience = ApplicantSessionAuthenticationDefaults.Audience, + Expires = expiresAt.UtcDateTime, + IssuedAt = DateTime.UtcNow, + Claims = claims, + SigningCredentials = new SigningCredentials(_signingKey, SecurityAlgorithms.HmacSha256) + }; + + return (Handler.CreateToken(descriptor), expiresAt); + } + + public Task ValidateAsync(string token) + => Handler.ValidateTokenAsync(token, new TokenValidationParameters + { + ValidIssuer = ApplicantSessionAuthenticationDefaults.Issuer, + ValidAudience = ApplicantSessionAuthenticationDefaults.Audience, + IssuerSigningKey = _signingKey, + ValidateIssuer = true, + ValidateAudience = true, + ValidateIssuerSigningKey = true, + ValidateLifetime = true, + ClockSkew = TimeSpan.FromSeconds(30), + RoleClaimType = ClaimTypes.Role, + NameClaimType = JwtRegisteredClaimNames.Sub + }); + + /// + /// Reports whether a bearer token looks like one this API issued, without validating it. + /// Used only to route the request to the right authentication scheme; the scheme then + /// validates properly. + /// + public static bool LooksLikeApplicantToken(string token) + { + try + { + if (!Handler.CanReadToken(token)) + return false; + + return Handler.ReadJsonWebToken(token).Issuer == ApplicantSessionAuthenticationDefaults.Issuer; + } + catch (ArgumentException) + { + return false; + } + } + + private static byte[] ResolveSigningKey(IConfiguration configuration) + { + var configured = configuration["Authentication:ApplicantToken:SigningKey"]; + + if (!string.IsNullOrWhiteSpace(configured)) + return Encoding.UTF8.GetBytes(configured); + + // Validated at startup for non-Development environments (see ApplicantTokenKeyValidator), + // so reaching here means development or tests: use an ephemeral key. Tokens then stop + // working across restarts, which is correct - they are not meant to outlive the process + // that issued them in that setting. + return RandomNumberGenerator.GetBytes(64); + } +} diff --git a/src/backend/OpenOnboarding.Api/Authorization/SessionOwnershipHandler.cs b/src/backend/OpenOnboarding.Api/Authorization/SessionOwnershipHandler.cs index 61022f5..0d31151 100644 --- a/src/backend/OpenOnboarding.Api/Authorization/SessionOwnershipHandler.cs +++ b/src/backend/OpenOnboarding.Api/Authorization/SessionOwnershipHandler.cs @@ -1,12 +1,14 @@ using System.Security.Claims; using Microsoft.AspNetCore.Authorization; +using OpenOnboarding.Api.Authentication; using OpenOnboarding.Application.Contracts; namespace OpenOnboarding.Api.Authorization; /// -/// Resource-based authorization handler that succeeds when the current user is an Operator -/// or is an Applicant whose customerProfileId claim matches the session's CustomerProfileId. +/// Resource-based authorization handler that succeeds when the current user is an Operator, holds +/// an applicant session token naming this session, or is an Applicant whose +/// customerProfileId claim matches the session's CustomerProfileId. /// public sealed class SessionOwnershipHandler : AuthorizationHandler @@ -22,7 +24,16 @@ protected override Task HandleRequirementAsync( return Task.CompletedTask; } - var customerProfileId = context.User.FindFirstValue("customerProfileId"); + // A per-session applicant token names the one session it may act on. This is the path the + // public onboarding app takes; it does not depend on a customer profile existing. + var sessionId = context.User.FindFirstValue(ApplicantSessionAuthenticationDefaults.SessionIdClaim); + if (sessionId is not null && resource.Id.ToString() == sessionId) + { + context.Succeed(requirement); + return Task.CompletedTask; + } + + var customerProfileId = context.User.FindFirstValue(ApplicantSessionAuthenticationDefaults.CustomerProfileIdClaim); if (customerProfileId is not null && resource.CustomerProfileId?.ToString() == customerProfileId) { diff --git a/src/backend/OpenOnboarding.Api/Configuration/ApplicantTokenKeyValidator.cs b/src/backend/OpenOnboarding.Api/Configuration/ApplicantTokenKeyValidator.cs new file mode 100644 index 0000000..2e2a51c --- /dev/null +++ b/src/backend/OpenOnboarding.Api/Configuration/ApplicantTokenKeyValidator.cs @@ -0,0 +1,33 @@ +namespace OpenOnboarding.Api.Configuration; + +public static class ApplicantTokenKeyValidator +{ + /// Shortest key accepted for HMAC-SHA256; below this the signature is weak. + public const int MinimumKeyLength = 32; + + public static void ValidateOrThrow(string? signingKey, string environmentName) + { + var isProduction = !string.Equals(environmentName, "Development", StringComparison.OrdinalIgnoreCase) + && !string.Equals(environmentName, "Testing", StringComparison.OrdinalIgnoreCase); + + if (!isProduction) + { + // Development and tests fall back to an ephemeral per-process key. + return; + } + + if (string.IsNullOrWhiteSpace(signingKey)) + { + throw new InvalidOperationException( + "Authentication:ApplicantToken:SigningKey is required in non-Development environments. " + + "Applicant session tokens are signed with it; without a stable key, tokens stop working " + + "on restart and across replicas. Store it as a secret."); + } + + if (signingKey.Length < MinimumKeyLength) + { + throw new InvalidOperationException( + $"Authentication:ApplicantToken:SigningKey must be at least {MinimumKeyLength} characters."); + } + } +} diff --git a/src/backend/OpenOnboarding.Api/Controllers/WorkflowController.cs b/src/backend/OpenOnboarding.Api/Controllers/WorkflowController.cs index 0ab77f7..05e5677 100644 --- a/src/backend/OpenOnboarding.Api/Controllers/WorkflowController.cs +++ b/src/backend/OpenOnboarding.Api/Controllers/WorkflowController.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.RateLimiting; using Microsoft.Extensions.Configuration; +using OpenOnboarding.Api.Authentication; using OpenOnboarding.Api.Authorization; using OpenOnboarding.Application.Contracts; using OpenOnboarding.Application.Interfaces; @@ -21,25 +22,42 @@ public sealed class WorkflowController( ISessionAnalyticsService sessionAnalyticsService, IAuthorizationService authorizationService, ISessionEventEmitter sessionEventEmitter, + ApplicantSessionTokenService applicantTokenService, IConfiguration configuration) : ControllerBase { /// /// Starts a new onboarding session for the given workflow flow. /// + /// + /// Anonymous: this is the entry point of a public onboarding journey, so there is no credential + /// to present yet. The response carries an applicant token scoped to the new session, which the + /// caller sends as a bearer token for every subsequent request. Rate limited per the + /// session-start policy. + /// [HttpPost("sessions/start")] [EnableRateLimiting("session-start")] - [Authorize(Policy = "ApplicantOrOperator")] + [AllowAnonymous] [Consumes("application/json")] - [ProducesResponseType(typeof(SessionStepResponse), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(SessionStartResponse), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] - [ProducesResponseType(StatusCodes.Status401Unauthorized)] - [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status500InternalServerError)] - public async Task> StartSession([FromBody] StartSessionRequest request, CancellationToken cancellationToken) + public async Task> StartSession([FromBody] StartSessionRequest request, CancellationToken cancellationToken) { var result = await workflowService.StartSessionAsync(request, cancellationToken); - return Ok(result); + var response = SessionStartResponse.From(result); + + // An operator's own credential already covers the session, so only mint a token for the + // anonymous applicant case. + if (!User.IsInRole(AppRoles.Operator)) + { + var session = await sessionAnalyticsService.GetSessionAsync(result.SessionId, cancellationToken); + var (token, expiresAt) = applicantTokenService.Issue(result.SessionId, session?.CustomerProfileId); + response.ApplicantToken = token; + response.ApplicantTokenExpiresAt = expiresAt; + } + + return Ok(response); } /// @@ -270,6 +288,16 @@ public async Task GetDocument( [Authorize(Policy = "ApplicantOrOperator")] public async Task StreamEvents([FromRoute] Guid sessionId, CancellationToken cancellationToken) { + // Same ownership rule as every other session endpoint: a credential for one session must + // not open another session's event stream. + var session = await sessionAnalyticsService.GetSessionAsync(sessionId, cancellationToken); + var authResult = await authorizationService.AuthorizeAsync(User, session, new SessionOwnershipRequirement()); + if (!authResult.Succeeded) + { + Response.StatusCode = StatusCodes.Status403Forbidden; + return; + } + Response.Headers["Content-Type"] = "text/event-stream"; Response.Headers["Cache-Control"] = "no-cache"; Response.Headers["X-Accel-Buffering"] = "no"; diff --git a/src/backend/OpenOnboarding.Api/OpenOnboarding.Api.csproj b/src/backend/OpenOnboarding.Api/OpenOnboarding.Api.csproj index 756afa1..0f9e967 100644 --- a/src/backend/OpenOnboarding.Api/OpenOnboarding.Api.csproj +++ b/src/backend/OpenOnboarding.Api/OpenOnboarding.Api.csproj @@ -1,31 +1,36 @@ - - - - net10.0 - enable - enable - true - $(NoWarn);1591 - - - - - - - - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - - - - - + + + + net10.0 + enable + enable + true + $(NoWarn);1591 + + + + + + + + + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + + + + + + - - - + + + diff --git a/src/backend/OpenOnboarding.Api/Program.cs b/src/backend/OpenOnboarding.Api/Program.cs index d6223ad..3869b94 100644 --- a/src/backend/OpenOnboarding.Api/Program.cs +++ b/src/backend/OpenOnboarding.Api/Program.cs @@ -24,7 +24,11 @@ JwtAuthorityValidator.ValidateOrThrow( builder.Configuration["Authentication:JwtAuthority"], builder.Environment.EnvironmentName); +ApplicantTokenKeyValidator.ValidateOrThrow( + builder.Configuration["Authentication:ApplicantToken:SigningKey"], + builder.Environment.EnvironmentName); +builder.Services.AddSingleton(); builder.Services.AddControllers() .AddJsonOptions(options => { @@ -81,14 +85,24 @@ options.DefaultScheme = "Combined"; options.DefaultChallengeScheme = "Combined"; }) - .AddPolicyScheme("Combined", "JWT or ApiKey", options => + .AddPolicyScheme("Combined", "Applicant token, admin cookie, API key or JWT", options => { options.ForwardDefaultSelector = context => - context.Request.Headers.ContainsKey("X-Api-Key") - ? ApiKeyAuthenticationHandler.SchemeName - : context.Request.Cookies.ContainsKey(AdminSessionAuthenticationDefaults.CookieName) - ? AdminSessionAuthenticationDefaults.SchemeName - : JwtBearerDefaults.AuthenticationScheme; + { + if (context.Request.Headers.ContainsKey("X-Api-Key")) + return ApiKeyAuthenticationHandler.SchemeName; + + if (context.Request.Cookies.ContainsKey(AdminSessionAuthenticationDefaults.CookieName)) + return AdminSessionAuthenticationDefaults.SchemeName; + + // Applicant session tokens and external IdP tokens both arrive as bearer tokens, so + // route on the issuer we stamp. The chosen scheme still validates the token properly. + var bearer = ApplicantSessionAuthenticationHandler.ExtractToken(context.Request); + if (bearer is not null && ApplicantSessionTokenService.LooksLikeApplicantToken(bearer)) + return ApplicantSessionAuthenticationDefaults.SchemeName; + + return JwtBearerDefaults.AuthenticationScheme; + }; }) .AddCookie(AdminSessionAuthenticationDefaults.SchemeName, options => { @@ -116,7 +130,9 @@ } }) .AddScheme( - ApiKeyAuthenticationHandler.SchemeName, _ => { }); + ApiKeyAuthenticationHandler.SchemeName, _ => { }) + .AddScheme( + ApplicantSessionAuthenticationDefaults.SchemeName, _ => { }); builder.Services.AddAuthorization(options => { diff --git a/src/backend/OpenOnboarding.Api/appsettings.json b/src/backend/OpenOnboarding.Api/appsettings.json index 15334f6..30f9445 100644 --- a/src/backend/OpenOnboarding.Api/appsettings.json +++ b/src/backend/OpenOnboarding.Api/appsettings.json @@ -12,6 +12,10 @@ "JwtAuthority": "", "JwtAudience": "open-onboarding-api", "ApiKey": "dev-api-key-change-in-production", + "ApplicantToken": { + "SigningKey": "", + "LifetimeMinutes": 0 + }, "Saml": { "Issuer": "waymark-service-provider", "AcsUrl": "", diff --git a/src/backend/OpenOnboarding.Application.Tests/ApplicantSessionTokenTests.cs b/src/backend/OpenOnboarding.Application.Tests/ApplicantSessionTokenTests.cs new file mode 100644 index 0000000..aa5864e --- /dev/null +++ b/src/backend/OpenOnboarding.Application.Tests/ApplicantSessionTokenTests.cs @@ -0,0 +1,281 @@ +using System.Net; +using System.Net.Http.Headers; +using System.Net.Http.Json; +using Microsoft.AspNetCore.Mvc.Testing; + +namespace OpenOnboarding.Application.Tests; + +/// +/// The public onboarding app must complete a journey without ever holding an operator credential, +/// and the credential it does hold must reach exactly one session and no operator data. +/// +public sealed class ApplicantSessionTokenTests +{ + private const string OperatorApiKey = "test-api-key"; + + private sealed record StartResponse( + Guid SessionId, + bool IsCompleted, + NodeStub? CurrentNode, + string? ApplicantToken, + DateTimeOffset? ApplicantTokenExpiresAt); + + private sealed record NodeStub(Guid Id, string Key, string Type); + + private sealed record FlowStub(Guid Id); + + private static HttpClient CreateClient(WebApplicationFactory factory) + => factory.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false }); + + private static HttpClient CreateOperatorClient(WebApplicationFactory factory) + { + var client = CreateClient(factory); + client.DefaultRequestHeaders.Add("X-Api-Key", OperatorApiKey); + return client; + } + + private static async Task CreateFlowAsync(HttpClient operatorClient) + { + var payload = new + { + name = $"Applicant token flow {Guid.NewGuid()}", + nodes = new[] + { + new { key = "start", type = "Form", title = "Start", isStartNode = true, jsonContent = "{\"fields\":[]}" } + }, + connections = Array.Empty() + }; + + var response = await operatorClient.PostAsJsonAsync("/api/flows", payload); + response.EnsureSuccessStatusCode(); + var flow = await response.Content.ReadFromJsonAsync(); + return flow!.Id; + } + + /// Starts a session the way a browser does: no credential of any kind. + private static async Task StartAnonymousSessionAsync(HttpClient client, Guid flowId) + { + var response = await client.PostAsJsonAsync("/api/workflow/sessions/start", new { flowId }); + response.EnsureSuccessStatusCode(); + return (await response.Content.ReadFromJsonAsync())!; + } + + private static HttpRequestMessage WithToken(HttpMethod method, string url, string token) + { + var request = new HttpRequestMessage(method, url); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + return request; + } + + // ======================================================================= + // The journey works with no operator credential in the browser + // ======================================================================= + [Fact] + public async Task StartSession_WithoutAnyCredential_SucceedsAndReturnsApplicantToken() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var request = new HttpRequestMessage(HttpMethod.Post, "/api/workflow/sessions/start") + { + Content = JsonContent.Create(new { flowId }) + }; + var response = await browser.SendAsync(request); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.False(request.Headers.Contains("X-Api-Key")); + + var start = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(start); + Assert.NotEqual(Guid.Empty, start!.SessionId); + Assert.False(string.IsNullOrWhiteSpace(start.ApplicantToken)); + Assert.True(start.ApplicantTokenExpiresAt > DateTimeOffset.UtcNow); + } + + [Fact] + public async Task ApplicantToken_CanSubmitAStepForItsOwnSession() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var start = await StartAnonymousSessionAsync(browser, flowId); + + var submit = WithToken( + HttpMethod.Post, + $"/api/workflow/sessions/{start.SessionId}/steps/{start.CurrentNode!.Id}/submit", + start.ApplicantToken!); + submit.Content = JsonContent.Create(new { payload = new Dictionary() }); + + var response = await browser.SendAsync(submit); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + [Fact] + public async Task ApplicantToken_CanReadItsOwnSession() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var start = await StartAnonymousSessionAsync(browser, flowId); + + var response = await browser.SendAsync( + WithToken(HttpMethod.Get, $"/api/workflow/sessions/{start.SessionId}", start.ApplicantToken!)); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + // ======================================================================= + // A token for session A must not reach session B + // ======================================================================= + [Fact] + public async Task ApplicantToken_IsRejectedForAnotherSession() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var sessionA = await StartAnonymousSessionAsync(browser, flowId); + var sessionB = await StartAnonymousSessionAsync(browser, flowId); + + // Read + var read = await browser.SendAsync( + WithToken(HttpMethod.Get, $"/api/workflow/sessions/{sessionB.SessionId}", sessionA.ApplicantToken!)); + Assert.Equal(HttpStatusCode.Forbidden, read.StatusCode); + + // Submit + var submit = WithToken( + HttpMethod.Post, + $"/api/workflow/sessions/{sessionB.SessionId}/steps/{sessionB.CurrentNode!.Id}/submit", + sessionA.ApplicantToken!); + submit.Content = JsonContent.Create(new { payload = new Dictionary() }); + Assert.Equal(HttpStatusCode.Forbidden, (await browser.SendAsync(submit)).StatusCode); + + // Next step + var next = await browser.SendAsync( + WithToken(HttpMethod.Get, $"/api/workflow/sessions/{sessionB.SessionId}/next", sessionA.ApplicantToken!)); + Assert.Equal(HttpStatusCode.Forbidden, next.StatusCode); + + // Abandon + var abandon = await browser.SendAsync( + WithToken(HttpMethod.Delete, $"/api/workflow/sessions/{sessionB.SessionId}", sessionA.ApplicantToken!)); + Assert.Equal(HttpStatusCode.Forbidden, abandon.StatusCode); + } + + [Fact] + public async Task ApplicantToken_IsRejectedForAnotherSessionsEventStream() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var sessionA = await StartAnonymousSessionAsync(browser, flowId); + var sessionB = await StartAnonymousSessionAsync(browser, flowId); + + // EventSource cannot set headers, so the SSE endpoint also accepts the token as a query + // parameter - it must be scoped just as tightly. + var response = await browser.GetAsync( + $"/api/workflow/sessions/{sessionB.SessionId}/events?access_token={sessionA.ApplicantToken}", + HttpCompletionOption.ResponseHeadersRead); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + } + + // ======================================================================= + // A token must reach no operator data at all + // ======================================================================= + /// Operator-only reads. {flowId} is substituted with a real flow. + public static TheoryData OperatorOnlyEndpoints => new() + { + "/api/workflow/sessions", + "/api/flows", + "/api/flows/{flowId}/webhooks", + "/api/flows/{flowId}/webhook-deliveries", + "/api/customers" + }; + + [Theory] + [MemberData(nameof(OperatorOnlyEndpoints))] + public async Task ApplicantToken_IsRejectedFromOperatorOnlyEndpoints(string url) + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var start = await StartAnonymousSessionAsync(browser, flowId); + + var resolvedUrl = url.Replace("{flowId}", flowId.ToString()); + var response = await browser.SendAsync(WithToken(HttpMethod.Get, resolvedUrl, start.ApplicantToken!)); + + Assert.True( + response.StatusCode is HttpStatusCode.Forbidden or HttpStatusCode.Unauthorized, + $"{resolvedUrl} returned {(int)response.StatusCode} for an applicant token; expected 401 or 403."); + } + + [Fact] + public async Task ApplicantToken_IsRejectedFromSubmissionsAndFlowStats() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var start = await StartAnonymousSessionAsync(browser, flowId); + + // Submissions for the applicant's own session are still operator-only: the PII readout is + // an operator view, not part of completing the journey. + var submissions = await browser.SendAsync(WithToken( + HttpMethod.Get, $"/api/workflow/sessions/{start.SessionId}/submissions", start.ApplicantToken!)); + Assert.Equal(HttpStatusCode.Forbidden, submissions.StatusCode); + + var stats = await browser.SendAsync(WithToken( + HttpMethod.Get, $"/api/workflow/flows/{flowId}/stats", start.ApplicantToken!)); + Assert.Equal(HttpStatusCode.Forbidden, stats.StatusCode); + } + + // ======================================================================= + // Token integrity + // ======================================================================= + [Fact] + public async Task TamperedApplicantToken_IsRejected() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var start = await StartAnonymousSessionAsync(browser, flowId); + + // Flip the last character of the signature. + var token = start.ApplicantToken!; + var tampered = token[..^1] + (token[^1] == 'A' ? 'B' : 'A'); + + var response = await browser.SendAsync( + WithToken(HttpMethod.Get, $"/api/workflow/sessions/{start.SessionId}", tampered)); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + [Fact] + public async Task OperatorStartedSession_DoesNotMintAnApplicantToken() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + var response = await operatorClient.PostAsJsonAsync("/api/workflow/sessions/start", new { flowId }); + response.EnsureSuccessStatusCode(); + + var start = await response.Content.ReadFromJsonAsync(); + Assert.Null(start!.ApplicantToken); + } +} diff --git a/src/backend/OpenOnboarding.Application/Contracts/SessionStartResponse.cs b/src/backend/OpenOnboarding.Application/Contracts/SessionStartResponse.cs new file mode 100644 index 0000000..b597045 --- /dev/null +++ b/src/backend/OpenOnboarding.Application/Contracts/SessionStartResponse.cs @@ -0,0 +1,40 @@ +namespace OpenOnboarding.Application.Contracts; + +/// +/// The state of a session immediately after it is started, together with the credential the +/// applicant's browser uses for the rest of the journey. +/// +public sealed class SessionStartResponse +{ + /// The unique identifier of the onboarding session. + public Guid SessionId { get; set; } + + /// + /// Indicates whether the onboarding workflow has been completed. + /// When true, will be null. + /// + public bool IsCompleted { get; set; } + + /// + /// The current workflow node/step to be rendered to the user. + /// null when the session is completed. + /// + public NodeDto? CurrentNode { get; set; } + + /// + /// Bearer token authorising requests for this session only. Present when the session was + /// started anonymously by an applicant; null when an operator started it, as the + /// operator's own credential already covers the session. + /// + public string? ApplicantToken { get; set; } + + /// When stops being accepted. + public DateTimeOffset? ApplicantTokenExpiresAt { get; set; } + + public static SessionStartResponse From(SessionStepResponse step) => new() + { + SessionId = step.SessionId, + IsCompleted = step.IsCompleted, + CurrentNode = step.CurrentNode + }; +} diff --git a/src/backend/OpenOnboarding.Pact.Tests/Providers/WorkflowProviderTests.cs b/src/backend/OpenOnboarding.Pact.Tests/Providers/WorkflowProviderTests.cs index c7e059c..6d1b01a 100644 --- a/src/backend/OpenOnboarding.Pact.Tests/Providers/WorkflowProviderTests.cs +++ b/src/backend/OpenOnboarding.Pact.Tests/Providers/WorkflowProviderTests.cs @@ -1,3 +1,5 @@ +using Microsoft.Extensions.DependencyInjection; +using OpenOnboarding.Api.Authentication; using PactNet; using PactNet.Verifier; using OpenOnboarding.Pact.Tests.Infrastructure; @@ -38,16 +40,23 @@ public async Task OpenOnboardingApi_HonoursWorkflowConsumerPact() using var fixture = new PactProviderFixture(); var (flowId, nodeId) = await fixture.SeedFlowAsync(); - await fixture.SeedSessionAsync(flowId, nodeId); + var sessionId = await fixture.SeedSessionAsync(flowId, nodeId); var config = new PactVerifierConfig { Outputters = [new XunitOutput(output)] }; + // Credentials are not part of a contract - the consumer's token is meaningless to this + // provider. Mint a real one for the seeded session and inject it, which is the standard + // Pact approach to authenticated interactions. + var tokenService = fixture.Services.GetRequiredService(); + var (applicantToken, _) = tokenService.Issue(sessionId, customerProfileId: null); + new PactVerifier("open-onboarding-api", config) .WithHttpEndpoint(fixture.ServerUri) .WithFileSource(new FileInfo(pactFile)) + .WithCustomHeader("Authorization", $"Bearer {applicantToken}") .Verify(); } diff --git a/src/frontend/.env.example b/src/frontend/.env.example index cac99bd..a58bcf7 100644 --- a/src/frontend/.env.example +++ b/src/frontend/.env.example @@ -1,9 +1,14 @@ # Copy this file to .env.local for local development. -# Vite exposes only variables prefixed with VITE_ to the browser. +# Vite exposes only variables prefixed with VITE_ to the browser, and inlines them into the built +# bundle as literal strings. Never put a credential here: anything in this file is readable by +# every visitor who loads the app. # Base URL for the backend host (the app will append /api/workflow automatically) VITE_API_BASE_URL=http://localhost:5072 -# API key for X-Api-Key authentication. -# Must match Authentication:ApiKey in appsettings.Development.json. -VITE_API_KEY=dev-api-key-change-in-production +# Authentication needs no configuration here: +# - The public onboarding journey starts anonymously. POST /api/workflow/sessions/start returns an +# applicant token scoped to that one session, which the app sends for every later request. +# - Operator screens authenticate with the AdminSession cookie from SAML SSO (/auth/saml/login). +# - Authentication:ApiKey is a server-to-server credential and must never be sent from a browser: +# it maps to a full Operator principal. diff --git a/src/frontend/e2e/onboarding-journeys.spec.ts b/src/frontend/e2e/onboarding-journeys.spec.ts index a6d192e..8c470ab 100644 --- a/src/frontend/e2e/onboarding-journeys.spec.ts +++ b/src/frontend/e2e/onboarding-journeys.spec.ts @@ -137,7 +137,12 @@ const mockJourneys: Record = { }, } -let documentUploads: Array<{ sessionId: string; nodeId: string; apiKey: string | null }> = [] +let documentUploads: Array<{ + sessionId: string + nodeId: string + apiKey: string | null + authorization: string | null +}> = [] test.beforeEach(async ({ page }) => { const sessions = new Map() @@ -180,6 +185,8 @@ test.beforeEach(async ({ page }) => { sessionId, isCompleted: false, currentNode: journey.nodes[0], + applicantToken: `applicant-token-${sessionId}`, + applicantTokenExpiresAt: new Date(Date.now() + 3_600_000).toISOString(), }), }) }) @@ -237,6 +244,7 @@ test.beforeEach(async ({ page }) => { sessionId, nodeId, apiKey: route.request().headers()['x-api-key'] ?? null, + authorization: route.request().headers()['authorization'] ?? null, }) await route.fulfill({ @@ -300,7 +308,10 @@ test('journey 2 reaches medium business document verification step', async ({ pa expect(documentUploads).toHaveLength(1) expect(documentUploads[0].nodeId).toBe('medium-node-2') - expect(documentUploads[0].apiKey).toBe('playwright-api-key') + // The browser authenticates with the per-session applicant token issued at session start, and + // must never send the operator API key - it is inlined into the bundle for every visitor to read. + expect(documentUploads[0].apiKey).toBeNull() + expect(documentUploads[0].authorization).toMatch(/^Bearer applicant-token-session-\d+$/) }) test('journey 3 runs large nationwide onboarding and compliance questions', async ({ page }) => { diff --git a/src/frontend/src/App.tsx b/src/frontend/src/App.tsx index bd23209..8f27d3b 100644 --- a/src/frontend/src/App.tsx +++ b/src/frontend/src/App.tsx @@ -104,7 +104,6 @@ function buildReturnUrl(path: string) { function App() { const { step, startSession, submitStep, isLoading, error } = useOnboarding() - const apiKey = import.meta.env.VITE_API_KEY || undefined const [currentPath, setCurrentPath] = useState(() => window.location.pathname) const [currentSearch, setCurrentSearch] = useState(() => window.location.search) const [adminAuthState, setAdminAuthState] = useState('checking') @@ -327,7 +326,6 @@ function App() { {builderFlowId && ( undefined} /> @@ -355,7 +353,6 @@ function App() { node={step.currentNode} sessionId={step.sessionId} nodeId={step.currentNode?.id} - apiKey={apiKey} onSubmit={async (payload) => { if (!step.currentNode) { return @@ -369,7 +366,7 @@ function App() {

Flow Analytics

- +
@@ -377,17 +374,16 @@ function App() { {selectedSession ? ( setSelectedSession(null)} /> ) : ( - + )}

Webhook Deliveries

- +
diff --git a/src/frontend/src/analytics/FlowAnalytics.tsx b/src/frontend/src/analytics/FlowAnalytics.tsx index 802f1f0..5a51e64 100644 --- a/src/frontend/src/analytics/FlowAnalytics.tsx +++ b/src/frontend/src/analytics/FlowAnalytics.tsx @@ -1,4 +1,5 @@ import { useEffect, useState } from 'react' +import { operatorFetch } from '../api/operator-api' interface FlowAnalyticsData { flowId: string @@ -13,7 +14,6 @@ interface FlowAnalyticsData { interface FlowAnalyticsProps { flowId: string - apiKey?: string } function formatDuration(seconds: number): string { @@ -40,7 +40,7 @@ function SkeletonCard() { ) } -export function FlowAnalytics({ flowId, apiKey }: FlowAnalyticsProps) { +export function FlowAnalytics({ flowId }: FlowAnalyticsProps) { const [data, setData] = useState(null) const [isLoading, setIsLoading] = useState(false) const [error, setError] = useState(null) @@ -53,9 +53,8 @@ export function FlowAnalytics({ flowId, apiKey }: FlowAnalyticsProps) { setIsLoading(true) setError(null) const headers: Record = {} - if (apiKey) headers['X-Api-Key'] = apiKey try { - const res = await fetch(`/api/analytics/flows/${flowId}`, { headers }) + const res = await operatorFetch(`/api/analytics/flows/${flowId}`, { headers }) if (!res.ok) throw new Error(`Failed to load analytics (${res.status})`) if (!cancelled) setData((await res.json()) as FlowAnalyticsData) } catch (err) { @@ -67,7 +66,7 @@ export function FlowAnalytics({ flowId, apiKey }: FlowAnalyticsProps) { void load() return () => { cancelled = true } - }, [flowId, apiKey, refreshKey]) + }, [flowId, refreshKey]) return (
diff --git a/src/frontend/src/api/operator-api.ts b/src/frontend/src/api/operator-api.ts new file mode 100644 index 0000000..a6dcc43 --- /dev/null +++ b/src/frontend/src/api/operator-api.ts @@ -0,0 +1,16 @@ +/** + * `fetch` for operator-only endpoints. + * + * Operator surfaces authenticate with the `AdminSession` cookie established by SAML SSO, which is + * why `credentials: 'include'` is set here rather than left to each call site. They must never send + * an API key: `X-Api-Key` maps to a full Operator principal, and a key the browser holds is a key + * every visitor holds. + */ +export function operatorFetch(input: string, init: RequestInit = {}): Promise { + return fetch(input, { ...init, credentials: 'include' }) +} + +/** Headers for an operator request with a JSON body. */ +export function operatorJsonHeaders(): Record { + return { 'Content-Type': 'application/json' } +} diff --git a/src/frontend/src/builder/AdminJourneyBuilderPage.tsx b/src/frontend/src/builder/AdminJourneyBuilderPage.tsx index d653753..10ed05d 100644 --- a/src/frontend/src/builder/AdminJourneyBuilderPage.tsx +++ b/src/frontend/src/builder/AdminJourneyBuilderPage.tsx @@ -1,15 +1,12 @@ import type { FlowDefinition } from '../onboarding/types/flow' import type { FlowDraft } from './flowAuthoring' import { VisualJourneyBuilder } from './VisualJourneyBuilder' +import { operatorFetch, operatorJsonHeaders } from '../api/operator-api' const serverBase = (import.meta.env.VITE_API_BASE_URL ?? '').replace(/\/$/, '') -const apiKey = import.meta.env.VITE_API_KEY function buildHeaders(includeJson: boolean): Record { - const headers: Record = {} - if (includeJson) headers['Content-Type'] = 'application/json' - if (apiKey) headers['X-Api-Key'] = apiKey - return headers + return includeJson ? operatorJsonHeaders() : {} } async function readErrorMessage(response: Response): Promise { @@ -30,7 +27,7 @@ async function readErrorMessage(response: Response): Promise { } async function loadFlow(flowId: string): Promise { - const response = await fetch(`${serverBase}/api/flows/${flowId}`, { + const response = await operatorFetch(`${serverBase}/api/flows/${flowId}`, { headers: buildHeaders(false), }) if (!response.ok) throw new Error(await readErrorMessage(response)) @@ -38,7 +35,7 @@ async function loadFlow(flowId: string): Promise { } async function saveFlow(flowId: string, draft: FlowDraft): Promise { - const response = await fetch(`${serverBase}/api/flows/${flowId}`, { + const response = await operatorFetch(`${serverBase}/api/flows/${flowId}`, { method: 'PUT', headers: buildHeaders(true), body: JSON.stringify(draft), @@ -48,7 +45,7 @@ async function saveFlow(flowId: string, draft: FlowDraft): Promise { - const response = await fetch(`${serverBase}/api/flows`, { + const response = await operatorFetch(`${serverBase}/api/flows`, { method: 'POST', headers: buildHeaders(true), body: JSON.stringify(draft), diff --git a/src/frontend/src/builder/FlowAuthoringPanel.tsx b/src/frontend/src/builder/FlowAuthoringPanel.tsx index 1913bb8..f28cf66 100644 --- a/src/frontend/src/builder/FlowAuthoringPanel.tsx +++ b/src/frontend/src/builder/FlowAuthoringPanel.tsx @@ -8,9 +8,9 @@ import { validateFlowDraft, type FlowDraft, } from './flowAuthoring' +import { operatorFetch, operatorJsonHeaders } from '../api/operator-api' const serverBase = (import.meta.env.VITE_API_BASE_URL ?? '').replace(/\/$/, '') -const apiKey = import.meta.env.VITE_API_KEY type FlowAuthoringPanelProps = { onFlowSelected: (flowId: string | null, version: number | null) => void @@ -22,10 +22,7 @@ type StatusState = { } function buildHeaders(includeJsonContentType: boolean): Record { - const headers: Record = {} - if (includeJsonContentType) headers['Content-Type'] = 'application/json' - if (apiKey) headers['X-Api-Key'] = apiKey - return headers + return includeJsonContentType ? operatorJsonHeaders() : {} } async function readErrorMessage(response: Response): Promise { @@ -109,7 +106,7 @@ export function FlowAuthoringPanel({ onFlowSelected }: FlowAuthoringPanelProps) setIsBusy(true) setStatus({ kind: 'idle', message: '' }) try { - const response = await fetch(`${serverBase}/api/flows/${flowIdInput.trim()}`, { + const response = await operatorFetch(`${serverBase}/api/flows/${flowIdInput.trim()}`, { headers: buildHeaders(false), }) if (!response.ok) { @@ -156,7 +153,7 @@ export function FlowAuthoringPanel({ onFlowSelected }: FlowAuthoringPanelProps) ? `${serverBase}/api/flows` : `${serverBase}/api/flows/${flowIdInput.trim()}` const method = mode === 'create' ? 'POST' : 'PUT' - const response = await fetch(url, { + const response = await operatorFetch(url, { method, headers: buildHeaders(true), body: JSON.stringify(buildFlowWritePayload(draft)), @@ -195,7 +192,7 @@ export function FlowAuthoringPanel({ onFlowSelected }: FlowAuthoringPanelProps) setIsBusy(true) setStatus({ kind: 'idle', message: '' }) try { - const response = await fetch(`${serverBase}/api/flows/${flowIdInput.trim()}`, { + const response = await operatorFetch(`${serverBase}/api/flows/${flowIdInput.trim()}`, { method: 'DELETE', headers: buildHeaders(false), }) @@ -236,7 +233,7 @@ export function FlowAuthoringPanel({ onFlowSelected }: FlowAuthoringPanelProps) setIsBusy(true) setStatus({ kind: 'idle', message: '' }) try { - const response = await fetch(`${serverBase}/api/flows/${flowIdInput.trim()}`, { + const response = await operatorFetch(`${serverBase}/api/flows/${flowIdInput.trim()}`, { headers: buildHeaders(false), }) if (!response.ok) { diff --git a/src/frontend/src/flows/FlowVersionHistory.tsx b/src/frontend/src/flows/FlowVersionHistory.tsx index ef78dae..f570d2b 100644 --- a/src/frontend/src/flows/FlowVersionHistory.tsx +++ b/src/frontend/src/flows/FlowVersionHistory.tsx @@ -1,4 +1,5 @@ import { useCallback, useEffect, useMemo, useState } from 'react' +import { operatorFetch } from '../api/operator-api' interface VersionSummary { versionNumber: number @@ -8,27 +9,26 @@ interface VersionSummary { interface FlowVersionHistoryProps { flowId: string - apiKey?: string onRestore?: (versionNumber: number) => void activePersonasByVersion?: Record } const API_BASE_URL = (import.meta.env.VITE_API_BASE_URL ?? '').replace(/\/$/, '') -export function FlowVersionHistory({ flowId, apiKey, onRestore, activePersonasByVersion = {} }: FlowVersionHistoryProps) { +export function FlowVersionHistory({ flowId, onRestore, activePersonasByVersion = {} }: FlowVersionHistoryProps) { const [versions, setVersions] = useState(null) const [error, setError] = useState(null) const [restoring, setRestoring] = useState(null) const [confirmVersion, setConfirmVersion] = useState(null) const headers = useMemo( - (): Record => (apiKey ? { 'X-Api-Key': apiKey } : {}), - [apiKey], + (): Record => ({}), + [], ) useEffect(() => { let cancelled = false - fetch(`${API_BASE_URL}/api/flows/${flowId}/versions`, { headers }) + operatorFetch(`${API_BASE_URL}/api/flows/${flowId}/versions`, { headers }) .then(r => { if (!r.ok) throw new Error(`HTTP ${r.status}`) return r.json() as Promise @@ -42,7 +42,7 @@ export function FlowVersionHistory({ flowId, apiKey, onRestore, activePersonasBy setRestoring(versionNumber) setConfirmVersion(null) try { - const r = await fetch(`${API_BASE_URL}/api/flows/${flowId}/versions/${versionNumber}/restore`, { + const r = await operatorFetch(`${API_BASE_URL}/api/flows/${flowId}/versions/${versionNumber}/restore`, { method: 'POST', headers, }) diff --git a/src/frontend/src/onboarding/api/applicant-session.test.ts b/src/frontend/src/onboarding/api/applicant-session.test.ts new file mode 100644 index 0000000..d099333 --- /dev/null +++ b/src/frontend/src/onboarding/api/applicant-session.test.ts @@ -0,0 +1,87 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + applicantAuthHeaders, + clearApplicantToken, + getApplicantToken, + setApplicantToken, + withApplicantTokenQuery, +} from './applicant-session' + +describe('applicant session credential', () => { + beforeEach(() => { + clearApplicantToken() + sessionStorage.clear() + }) + + afterEach(() => { + vi.restoreAllMocks() + clearApplicantToken() + }) + + it('has no credential before a session starts', () => { + expect(getApplicantToken()).toBeNull() + expect(applicantAuthHeaders()).toEqual({}) + }) + + it('returns a bearer header once a session has started', () => { + setApplicantToken('token-abc') + + expect(applicantAuthHeaders()).toEqual({ Authorization: 'Bearer token-abc' }) + }) + + it('never emits an X-Api-Key header', () => { + setApplicantToken('token-abc') + + expect(Object.keys(applicantAuthHeaders())).not.toContain('X-Api-Key') + }) + + it('survives a reload by reading the token back from sessionStorage', () => { + setApplicantToken('token-abc') + + // Simulate a fresh page: the module-level cache is gone but storage is not. + clearApplicantToken() + sessionStorage.setItem('waymark.applicantToken', 'token-abc') + + expect(getApplicantToken()).toBe('token-abc') + }) + + it('clears the token from storage', () => { + setApplicantToken('token-abc') + clearApplicantToken() + + expect(getApplicantToken()).toBeNull() + expect(sessionStorage.getItem('waymark.applicantToken')).toBeNull() + }) + + it('keeps working when sessionStorage throws', () => { + // Private-mode browsers and blocked site data both throw here; the journey must not break. + vi.spyOn(Storage.prototype, 'setItem').mockImplementation(() => { + throw new Error('storage disabled') + }) + vi.spyOn(Storage.prototype, 'getItem').mockImplementation(() => { + throw new Error('storage disabled') + }) + + expect(() => setApplicantToken('token-abc')).not.toThrow() + expect(getApplicantToken()).toBe('token-abc') + }) + + describe('SSE query parameter', () => { + it('appends the token, since EventSource cannot send headers', () => { + setApplicantToken('token abc/=') + + expect(withApplicantTokenQuery('/api/workflow/sessions/1/events')) + .toBe('/api/workflow/sessions/1/events?access_token=token%20abc%2F%3D') + }) + + it('uses & when the url already has a query string', () => { + setApplicantToken('t') + + expect(withApplicantTokenQuery('/events?foo=1')).toBe('/events?foo=1&access_token=t') + }) + + it('leaves the url untouched when there is no credential', () => { + expect(withApplicantTokenQuery('/events')).toBe('/events') + }) + }) +}) diff --git a/src/frontend/src/onboarding/api/applicant-session.ts b/src/frontend/src/onboarding/api/applicant-session.ts new file mode 100644 index 0000000..93d0bb5 --- /dev/null +++ b/src/frontend/src/onboarding/api/applicant-session.ts @@ -0,0 +1,65 @@ +/** + * Holds the per-session applicant credential returned by POST /sessions/start. + * + * This replaces the shared API key the app used to ship in its own bundle. The token authorises + * one session and carries no operator rights, so the worst a leaked one exposes is the journey its + * holder was already completing. + * + * Stored in sessionStorage so a page reload mid-journey does not strand the applicant, and so the + * credential dies with the tab. Every access is guarded: sessionStorage throws in private-mode + * browsers and when site data is blocked, and losing the token must not break the journey in + * progress — the in-memory copy still serves it. + */ +const STORAGE_KEY = 'waymark.applicantToken' + +let inMemoryToken: string | null = null + +export function setApplicantToken(token: string | null): void { + inMemoryToken = token + + try { + if (token === null) { + sessionStorage.removeItem(STORAGE_KEY) + } else { + sessionStorage.setItem(STORAGE_KEY, token) + } + } catch { + // Storage unavailable; the in-memory token covers this page view. + } +} + +export function getApplicantToken(): string | null { + if (inMemoryToken !== null) return inMemoryToken + + try { + inMemoryToken = sessionStorage.getItem(STORAGE_KEY) + } catch { + inMemoryToken = null + } + + return inMemoryToken +} + +export function clearApplicantToken(): void { + setApplicantToken(null) +} + +/** Authorization header for the current applicant session, or `{}` when there is none. */ +export function applicantAuthHeaders(): Record { + const token = getApplicantToken() + return token ? { Authorization: `Bearer ${token}` } : {} +} + +/** + * Appends the credential to a URL as a query parameter. + * + * Only for the SSE endpoint: the browser EventSource API cannot set request headers, so a query + * parameter is the sole way to present a credential on that request. + */ +export function withApplicantTokenQuery(url: string): string { + const token = getApplicantToken() + if (!token) return url + + const separator = url.includes('?') ? '&' : '?' + return `${url}${separator}access_token=${encodeURIComponent(token)}` +} diff --git a/src/frontend/src/onboarding/api/workflow-api-client.ts b/src/frontend/src/onboarding/api/workflow-api-client.ts index 016e754..001a1d6 100644 --- a/src/frontend/src/onboarding/api/workflow-api-client.ts +++ b/src/frontend/src/onboarding/api/workflow-api-client.ts @@ -1,4 +1,5 @@ -import type { SessionStepResponse, StartSessionRequest, SubmitStepRequest } from '../types/flow' +import type { SessionStartResponse, SessionStepResponse, StartSessionRequest, SubmitStepRequest } from '../types/flow' +import { applicantAuthHeaders, setApplicantToken } from './applicant-session' export interface ComplianceViolation { field: string @@ -16,14 +17,15 @@ export class ComplianceError extends Error { } } -const DEFAULT_API_KEY = import.meta.env.VITE_API_KEY ?? '' const WORKFLOW_API_BASE_PATH = '/api/workflow' -function buildHeaders(apiKey?: string): Record { - const key = apiKey ?? DEFAULT_API_KEY - const headers: Record = { 'Content-Type': 'application/json' } - if (key) headers['X-Api-Key'] = key - return headers +/** + * Requests carry the per-session applicant token issued at session start. No API key is sent from + * the browser: the key maps to a full Operator principal and inlining it in the bundle would hand + * every visitor operator access. + */ +function buildHeaders(): Record { + return { 'Content-Type': 'application/json', ...applicantAuthHeaders() } } export function resolveWorkflowApiBase(baseUrl: string): string { @@ -37,16 +39,21 @@ export function resolveWorkflowApiBase(baseUrl: string): string { export async function startSession( baseUrl: string, payload: StartSessionRequest, - apiKey?: string, ): Promise { const workflowBase = resolveWorkflowApiBase(baseUrl) + // Anonymous by design: this is the entry point of a public journey, and the response is what + // hands the browser its credential for everything that follows. const res = await fetch(`${workflowBase}/sessions/start`, { method: 'POST', - headers: buildHeaders(apiKey), + headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(payload), }) if (!res.ok) throw new Error(`startSession failed with status ${res.status}`) - return res.json() as Promise + + const session = await res.json() as SessionStartResponse + if (session.applicantToken) setApplicantToken(session.applicantToken) + + return session } export async function submitStep( @@ -54,14 +61,13 @@ export async function submitStep( sessionId: string, nodeId: string, payload: SubmitStepRequest, - apiKey?: string, ): Promise { const workflowBase = resolveWorkflowApiBase(baseUrl) const res = await fetch( `${workflowBase}/sessions/${sessionId}/steps/${nodeId}/submit`, { method: 'POST', - headers: buildHeaders(apiKey), + headers: buildHeaders(), body: JSON.stringify(payload), }, ) @@ -76,12 +82,11 @@ export async function submitStep( export async function getNextStep( baseUrl: string, sessionId: string, - apiKey?: string, ): Promise { const workflowBase = resolveWorkflowApiBase(baseUrl) const res = await fetch(`${workflowBase}/sessions/${sessionId}/next`, { method: 'GET', - headers: buildHeaders(apiKey), + headers: buildHeaders(), }) if (!res.ok) throw new Error(`getNextStep failed with status ${res.status}`) return res.json() as Promise diff --git a/src/frontend/src/onboarding/components/StepRenderer.tsx b/src/frontend/src/onboarding/components/StepRenderer.tsx index f9ede30..7c7e978 100644 --- a/src/frontend/src/onboarding/components/StepRenderer.tsx +++ b/src/frontend/src/onboarding/components/StepRenderer.tsx @@ -3,6 +3,7 @@ import { Label } from '@radix-ui/react-label' import type { FlowNode } from '../types/flow' import { ComplianceError } from '../api/workflow-api-client' import { useJourneyAnalytics } from '../../analytics/JourneyAnalyticsContext' +import { applicantAuthHeaders } from '../api/applicant-session' // ── Error boundary ──────────────────────────────────────────────────────────── type ErrorBoundaryState = { hasError: boolean } @@ -238,13 +239,11 @@ function DocumentUploadStep({ sessionId, nodeId, onSubmit, - apiKey, }: { node: FlowNode sessionId?: string nodeId?: string onSubmit: (payload: Record) => Promise - apiKey?: string }) { const content = (() => { try { @@ -279,8 +278,10 @@ function DocumentUploadStep({ const xhr = new XMLHttpRequest() xhr.open('POST', `/api/workflow/sessions/${sessionId}/steps/${nodeId}/documents`) - if (apiKey) { - xhr.setRequestHeader('X-Api-Key', apiKey) + // Document upload belongs to the applicant's own session, so it uses the session credential. + const authHeaders = applicantAuthHeaders() + for (const [header, value] of Object.entries(authHeaders)) { + xhr.setRequestHeader(header, value) } xhr.upload.onprogress = (e) => { @@ -385,7 +386,6 @@ type StepRendererProps = { sessionId?: string nodeId?: string onSubmit: (payload: Record) => Promise - apiKey?: string } const cardClassName = 'rounded-lg border border-slate-200 bg-white p-4 shadow-sm' @@ -428,7 +428,7 @@ function FormStep({ return } -export function StepRenderer({ node, sessionId, nodeId, onSubmit, apiKey }: StepRendererProps) { +export function StepRenderer({ node, sessionId, nodeId, onSubmit }: StepRendererProps) { const { track } = useJourneyAnalytics() useEffect(() => { @@ -463,7 +463,7 @@ export function StepRenderer({ node, sessionId, nodeId, onSubmit, apiKey }: Step case 'DocumentUpload': return ( - + ) case 'Redirect': diff --git a/src/frontend/src/onboarding/hooks/useFlow.ts b/src/frontend/src/onboarding/hooks/useFlow.ts index 4860dce..2e670d6 100644 --- a/src/frontend/src/onboarding/hooks/useFlow.ts +++ b/src/frontend/src/onboarding/hooks/useFlow.ts @@ -1,14 +1,12 @@ import { useEffect, useReducer } from 'react' import type { FlowDefinition } from '../types/flow' -const API_KEY = import.meta.env.VITE_API_KEY as string | undefined const API_BASE_URL = (import.meta.env.VITE_API_BASE_URL ?? '').replace(/\/$/, '') async function fetchFlowDefinition(flowId: string): Promise { - const headers: Record = {} - if (API_KEY) headers['X-Api-Key'] = API_KEY - - const response = await fetch(`${API_BASE_URL}/api/flows/${flowId}`, { headers }) + // Flow definitions are operator data: authenticated by the admin session cookie, never by an + // API key shipped to the browser. + const response = await fetch(`${API_BASE_URL}/api/flows/${flowId}`, { credentials: 'include' }) if (!response.ok) throw new Error(`Failed to fetch flow: ${response.status}`) return response.json() as Promise } diff --git a/src/frontend/src/onboarding/hooks/useOnboarding.ts b/src/frontend/src/onboarding/hooks/useOnboarding.ts index ee588b1..1dc813f 100644 --- a/src/frontend/src/onboarding/hooks/useOnboarding.ts +++ b/src/frontend/src/onboarding/hooks/useOnboarding.ts @@ -8,10 +8,10 @@ import { ComplianceError, } from '../api/workflow-api-client' import { createSessionEventSource } from './session-event-source' +import { withApplicantTokenQuery } from '../api/applicant-session' const serverBase = (import.meta.env.VITE_API_BASE_URL ?? '').replace(/\/$/, '') const workflowApiBase = resolveWorkflowApiBase(serverBase) -const apiKey = import.meta.env.VITE_API_KEY || undefined export function useOnboarding() { const [step, setStep] = useState(null) @@ -42,7 +42,7 @@ export function useOnboarding() { setIsLoading(true) setError(null) try { - const next = await apiGetNextStep(serverBase, sessionId, apiKey) + const next = await apiGetNextStep(serverBase, sessionId) setStep(next) return next } catch (requestError) { @@ -59,7 +59,8 @@ export function useOnboarding() { if (typeof EventSource !== 'undefined') { const handle = createSessionEventSource( - `${workflowApiBase}/sessions/${sessionId}/events`, + // EventSource cannot set headers, so the credential rides in the query string. + withApplicantTokenQuery(`${workflowApiBase}/sessions/${sessionId}/events`), { onStepAdvanced: (data) => setStep(data), onCompleted: () => { @@ -90,7 +91,7 @@ export function useOnboarding() { setIsLoading(true) setError(null) try { - const next = await apiStartSession(serverBase, payload, apiKey) + const next = await apiStartSession(serverBase, payload) setStep(next) setIsCompleted(false) openEventStream(next.sessionId) @@ -107,7 +108,7 @@ export function useOnboarding() { setIsLoading(true) setError(null) try { - const next = await apiSubmitStep(serverBase, sessionId, nodeId, payload, apiKey) + const next = await apiSubmitStep(serverBase, sessionId, nodeId, payload) setStep(next) return next } catch (requestError) { diff --git a/src/frontend/src/onboarding/types/flow.ts b/src/frontend/src/onboarding/types/flow.ts index 1b6e3be..c35681f 100644 --- a/src/frontend/src/onboarding/types/flow.ts +++ b/src/frontend/src/onboarding/types/flow.ts @@ -19,6 +19,12 @@ export interface SessionStepResponse { currentNode: FlowNode | null } +/** Session start also returns the credential used for the rest of the journey. */ +export interface SessionStartResponse extends SessionStepResponse { + applicantToken?: string | null + applicantTokenExpiresAt?: string | null +} + export interface StartSessionRequest { flowId: string customerProfileId?: string diff --git a/src/frontend/src/pact/workflow.consumer.test.ts b/src/frontend/src/pact/workflow.consumer.test.ts index 6e21371..19ebdde 100644 --- a/src/frontend/src/pact/workflow.consumer.test.ts +++ b/src/frontend/src/pact/workflow.consumer.test.ts @@ -1,8 +1,9 @@ -import { describe, it, expect } from 'vitest' +import { describe, it, expect, beforeAll, afterAll } from 'vitest' import { PactV3, MatchersV3 } from '@pact-foundation/pact' import { fileURLToPath } from 'url' import { dirname, resolve } from 'path' import { startSession, submitStep, getNextStep } from '../onboarding/api/workflow-api-client' +import { clearApplicantToken, setApplicantToken } from '../onboarding/api/applicant-session' const { like, uuid, string } = MatchersV3 @@ -16,7 +17,6 @@ const provider = new PactV3({ logLevel: 'warn', }) -const TEST_API_KEY = 'test-api-key' const FLOW_ID = '550e8400-e29b-41d4-a716-446655440000' const NODE_ID = '660e8400-e29b-41d4-a716-446655440001' const SESSION_ID = '770e8400-e29b-41d4-a716-446655440002' @@ -34,6 +34,12 @@ const sessionStepResponseBody = { } describe('Workflow API — consumer contract', () => { + // The applicant session token is a credential, so it is not part of the contract: interactions + // declare only the headers they assert, and the provider verifier supplies a real token of its + // own. Setting one here keeps the consumer exercising its true code path. + beforeAll(() => setApplicantToken('consumer-contract-applicant-token')) + afterAll(() => clearApplicantToken()) + it('starts an onboarding session', async () => { await provider .addInteraction({ @@ -43,7 +49,6 @@ describe('Workflow API — consumer contract', () => { method: 'POST', path: '/api/workflow/sessions/start', headers: { - 'X-Api-Key': TEST_API_KEY, 'Content-Type': 'application/json', }, body: { flowId: FLOW_ID }, @@ -54,7 +59,7 @@ describe('Workflow API — consumer contract', () => { }, }) .executeTest(async (mockServer) => { - const result = await startSession(mockServer.url, { flowId: FLOW_ID }, TEST_API_KEY) + const result = await startSession(mockServer.url, { flowId: FLOW_ID }) expect(result.sessionId).toBeDefined() expect(result.isCompleted).toBe(false) expect(result.currentNode).not.toBeNull() @@ -74,7 +79,6 @@ describe('Workflow API — consumer contract', () => { method: 'POST', path: `/api/workflow/sessions/${SESSION_ID}/steps/${NODE_ID}/submit`, headers: { - 'X-Api-Key': TEST_API_KEY, 'Content-Type': 'application/json', }, body: { payload: like({ field: 'value' }) }, @@ -90,7 +94,6 @@ describe('Workflow API — consumer contract', () => { SESSION_ID, NODE_ID, { payload: { field: 'value' } }, - TEST_API_KEY, ) expect(result.sessionId).toBeDefined() expect(result.isCompleted).toBe(false) @@ -105,9 +108,6 @@ describe('Workflow API — consumer contract', () => { withRequest: { method: 'GET', path: `/api/workflow/sessions/${SESSION_ID}/next`, - headers: { - 'X-Api-Key': TEST_API_KEY, - }, }, willRespondWith: { status: 200, @@ -115,7 +115,7 @@ describe('Workflow API — consumer contract', () => { }, }) .executeTest(async (mockServer) => { - const result = await getNextStep(mockServer.url, SESSION_ID, TEST_API_KEY) + const result = await getNextStep(mockServer.url, SESSION_ID) expect(result.sessionId).toBeDefined() expect(result.isCompleted).toBeDefined() }) @@ -130,7 +130,6 @@ describe('Workflow API — consumer contract', () => { method: 'POST', path: '/api/workflow/sessions/start', headers: { - 'X-Api-Key': TEST_API_KEY, 'Content-Type': 'application/json', }, body: {}, @@ -145,7 +144,7 @@ describe('Workflow API — consumer contract', () => { }) .executeTest(async (mockServer) => { await expect( - startSession(mockServer.url, {} as Parameters[1], TEST_API_KEY), + startSession(mockServer.url, {} as Parameters[1]), ).rejects.toThrow() }) }) diff --git a/src/frontend/src/sessions/SessionDetail.tsx b/src/frontend/src/sessions/SessionDetail.tsx index dbbd0aa..1358dd4 100644 --- a/src/frontend/src/sessions/SessionDetail.tsx +++ b/src/frontend/src/sessions/SessionDetail.tsx @@ -1,4 +1,5 @@ import { useEffect, useState } from 'react' +import { operatorFetch } from '../api/operator-api' interface Submission { id: string @@ -10,13 +11,11 @@ interface Submission { interface SessionDetailProps { sessionId: string - apiKey?: string onBack: () => void } -function buildHeaders(apiKey?: string): Record { +function buildHeaders(): Record { const h: Record = {} - if (apiKey) h['X-Api-Key'] = apiKey return h } @@ -29,7 +28,7 @@ function payloadSummary(payload: Record): string { .join(', ') + (entries.length > 3 ? ', …' : '') } -export function SessionDetail({ sessionId, apiKey, onBack }: SessionDetailProps) { +export function SessionDetail({ sessionId, onBack }: SessionDetailProps) { const [submissions, setSubmissions] = useState([]) const [isLoading, setIsLoading] = useState(false) const [error, setError] = useState(null) @@ -41,9 +40,9 @@ export function SessionDetail({ sessionId, apiKey, onBack }: SessionDetailProps) setIsLoading(true) setError(null) try { - const res = await fetch( + const res = await operatorFetch( `/api/workflow/sessions/${sessionId}/submissions`, - { headers: buildHeaders(apiKey), signal: controller.signal }, + { headers: buildHeaders(), signal: controller.signal }, ) if (!res.ok) throw new Error(`Failed to load submissions (${res.status})`) setSubmissions((await res.json()) as Submission[]) @@ -58,7 +57,7 @@ export function SessionDetail({ sessionId, apiKey, onBack }: SessionDetailProps) void load() return () => controller.abort() - }, [sessionId, apiKey]) + }, [sessionId]) return (
diff --git a/src/frontend/src/sessions/SessionList.tsx b/src/frontend/src/sessions/SessionList.tsx index 60ac0d8..e7dfa8d 100644 --- a/src/frontend/src/sessions/SessionList.tsx +++ b/src/frontend/src/sessions/SessionList.tsx @@ -1,4 +1,5 @@ import { useEffect, useState } from 'react' +import { operatorFetch } from '../api/operator-api' export interface SessionSummary { id: string @@ -10,7 +11,6 @@ export interface SessionSummary { } interface SessionListProps { - apiKey?: string onSelectSession: (session: SessionSummary) => void } @@ -20,13 +20,12 @@ const statusBadge: Record = { Abandoned: 'bg-red-100 text-red-800', } -function buildHeaders(apiKey?: string): Record { +function buildHeaders(): Record { const h: Record = {} - if (apiKey) h['X-Api-Key'] = apiKey return h } -export function SessionList({ apiKey, onSelectSession }: SessionListProps) { +export function SessionList({ onSelectSession }: SessionListProps) { const [sessions, setSessions] = useState([]) const [page, setPage] = useState(1) const [hasMore, setHasMore] = useState(false) @@ -42,9 +41,9 @@ export function SessionList({ apiKey, onSelectSession }: SessionListProps) { setIsLoading(true) setError(null) try { - const res = await fetch( + const res = await operatorFetch( `/api/workflow/sessions?page=${page}&pageSize=${pageSize}`, - { headers: buildHeaders(apiKey) }, + { headers: buildHeaders() }, ) if (!res.ok) throw new Error(`Failed to load sessions (${res.status})`) const data = (await res.json()) as SessionSummary[] @@ -61,7 +60,7 @@ export function SessionList({ apiKey, onSelectSession }: SessionListProps) { void load() return () => { cancelled = true } - }, [apiKey, page]) + }, [page]) return (
diff --git a/src/frontend/src/webhooks/WebhookDeliveries.tsx b/src/frontend/src/webhooks/WebhookDeliveries.tsx index 2d063af..74af189 100644 --- a/src/frontend/src/webhooks/WebhookDeliveries.tsx +++ b/src/frontend/src/webhooks/WebhookDeliveries.tsx @@ -1,4 +1,5 @@ import { useCallback, useEffect, useState } from 'react' +import { operatorFetch } from '../api/operator-api' interface Webhook { id: string @@ -17,23 +18,18 @@ interface WebhookDelivery { createdAt: string } -interface WebhookDeliveriesProps { - apiKey?: string -} - const statusColors: Record = { Pending: 'bg-yellow-100 text-yellow-800', Delivered: 'bg-green-100 text-green-800', Failed: 'bg-red-100 text-red-800', } -function buildHeaders(apiKey?: string): Record { +function buildHeaders(): Record { const h: Record = { 'Content-Type': 'application/json' } - if (apiKey) h['X-Api-Key'] = apiKey return h } -export function WebhookDeliveries({ apiKey }: WebhookDeliveriesProps) { +export function WebhookDeliveries() { const [webhooks, setWebhooks] = useState([]) const [selectedWebhook, setSelectedWebhook] = useState(null) const [deliveries, setDeliveries] = useState([]) @@ -50,7 +46,7 @@ export function WebhookDeliveries({ apiKey }: WebhookDeliveriesProps) { setIsLoadingWebhooks(true) setWebhookError(null) try { - const res = await fetch('/api/webhooks', { headers: buildHeaders(apiKey) }) + const res = await operatorFetch('/api/webhooks', { headers: buildHeaders() }) if (!res.ok) throw new Error(`Failed to load webhooks (${res.status})`) const data = (await res.json()) as Webhook[] if (!cancelled) setWebhooks(data) @@ -63,13 +59,13 @@ export function WebhookDeliveries({ apiKey }: WebhookDeliveriesProps) { void load() return () => { cancelled = true } - }, [apiKey]) + }, []) const fetchDeliveries = useCallback((webhook: Webhook) => { setIsLoadingDeliveries(true) setDeliveryError(null) - fetch(`/api/webhooks/${webhook.id}/deliveries`, { headers: buildHeaders(apiKey) }) + operatorFetch(`/api/webhooks/${webhook.id}/deliveries`, { headers: buildHeaders() }) .then((res) => { if (!res.ok) throw new Error(`Failed to load deliveries (${res.status})`) return res.json() as Promise @@ -77,14 +73,14 @@ export function WebhookDeliveries({ apiKey }: WebhookDeliveriesProps) { .then((data) => setDeliveries(data)) .catch((err: unknown) => setDeliveryError(err instanceof Error ? err.message : 'Unknown error')) .finally(() => setIsLoadingDeliveries(false)) - }, [apiKey]) + }, []) const retryDelivery = useCallback((deliveryId: string, webhook: Webhook) => { setRetryingId(deliveryId) - fetch(`/api/webhooks/deliveries/${deliveryId}/retry`, { + operatorFetch(`/api/webhooks/deliveries/${deliveryId}/retry`, { method: 'POST', - headers: buildHeaders(apiKey), + headers: buildHeaders(), }) .then((res) => { if (!res.ok) throw new Error(`Retry failed (${res.status})`) @@ -92,7 +88,7 @@ export function WebhookDeliveries({ apiKey }: WebhookDeliveriesProps) { }) .catch((err: unknown) => setDeliveryError(err instanceof Error ? err.message : 'Retry failed')) .finally(() => setRetryingId(null)) - }, [apiKey, fetchDeliveries]) + }, [fetchDeliveries]) const handleSelectWebhook = (webhook: Webhook) => { setSelectedWebhook(webhook) From bf408713e44b4298e9c74e0874dc145169e5c8ee Mon Sep 17 00:00:00 2001 From: MaximumTrainer Date: Sun, 13 Sep 2026 08:09:50 +0100 Subject: [PATCH 5/8] feat(frontend): move every operator surface behind the Operator check App.tsx guarded only /admin/journey-builder. The root route - the page an applicant loads to complete onboarding - rendered flow authoring, version history, analytics, the session list (including applicants' submitted fields) and webhook deliveries below the step renderer, with no auth gate. It also offered journey and persona pickers that let an applicant switch themselves onto any seeded flow. - Add src/routing/routes.ts as the single route -> required-role table. Its default is operator-only: an unmatched /admin path is guarded rather than falling through to the applicant page, so a new admin surface cannot be added without a guard, which is how this arose. - Add AdminConsolePage at /admin holding the moved operator sections, and reduce the applicant route to ApplicantJourneyPage: the step renderer, progress, and completion. - Applicants arrive on a link naming their journey (/?flowId=...) instead of choosing from a picker of every seeded flow. - Check identity only on routes that need it, so the public page makes no /api/auth/me call at all. - Derive the auth state from the path it was fetched for, so navigating between admin routes re-checks rather than reusing the previous answer. - Polyfill ResizeObserver and DOMMatrixReadOnly in test-setup; jsdom has neither and React Flow calls both on mount. Tests: routes.ts has its own unit tests including the unmatched-admin-path case. App.routing.test.tsx asserts the unauthenticated root renders no operator heading, no journey or persona selector, and issues no request to any operator endpoint. Playwright covers the redirect from /admin and from an unrouted admin path, and that the public page stays clean. Frontend 139/139, typecheck and lint clean, 16 mocked e2e passing. The 13 remaining e2e are the journey-a/b/c API specs, which need a backend on :5072 that CI provides. Closes #91 Co-Authored-By: Claude Opus 5 (1M context) --- src/frontend/e2e/admin-sso-auth.spec.ts | 62 ++++ src/frontend/e2e/onboarding-journeys.spec.ts | 6 +- src/frontend/e2e/step-renderer.spec.ts | 4 +- src/frontend/src/App.routing.test.tsx | 178 ++++++++++ src/frontend/src/App.tsx | 333 +++--------------- src/frontend/src/admin/AdminConsolePage.tsx | 169 +++++++++ src/frontend/src/journeys.ts | 62 ++++ .../src/onboarding/ApplicantJourneyPage.tsx | 88 +++++ src/frontend/src/routing/routes.test.ts | 44 +++ src/frontend/src/routing/routes.ts | 67 ++++ src/frontend/src/test-setup.ts | 16 + 11 files changed, 737 insertions(+), 292 deletions(-) create mode 100644 src/frontend/src/App.routing.test.tsx create mode 100644 src/frontend/src/admin/AdminConsolePage.tsx create mode 100644 src/frontend/src/journeys.ts create mode 100644 src/frontend/src/onboarding/ApplicantJourneyPage.tsx create mode 100644 src/frontend/src/routing/routes.test.ts create mode 100644 src/frontend/src/routing/routes.ts diff --git a/src/frontend/e2e/admin-sso-auth.spec.ts b/src/frontend/e2e/admin-sso-auth.spec.ts index a7be843..e3e6501 100644 --- a/src/frontend/e2e/admin-sso-auth.spec.ts +++ b/src/frontend/e2e/admin-sso-auth.spec.ts @@ -69,4 +69,66 @@ test.describe('Admin SSO auth guard', () => { await expect(page).toHaveURL(/\/login\?error=saml_access_denied/) await expect(page.getByRole('alert')).toContainText('Access Denied') }) + + test('operator console requires a session and leaks nothing before the check resolves', async ({ page }) => { + await page.route('**/api/auth/me', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ authenticated: false }), + }) + }) + + await page.goto('/admin') + + await expect(page).toHaveURL(/\/login\?returnUrl=/) + await expect(page.getByRole('heading', { name: /Waymark Operator Console/i })).toHaveCount(0) + await expect(page.getByRole('heading', { name: /Session History/i })).toHaveCount(0) + }) + + test('an admin path with no route entry is still guarded', async ({ page }) => { + await page.route('**/api/auth/me', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ authenticated: false }), + }) + }) + + await page.goto('/admin/not-built-yet') + + await expect(page).toHaveURL(/\/login\?returnUrl=/) + }) + + test('the public applicant page renders no operator surface', async ({ page }) => { + let authMeCalls = 0 + await page.route('**/api/auth/me', async (route) => { + authMeCalls += 1 + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ authenticated: false }), + }) + }) + + const operatorRequests: string[] = [] + await page.route('**/api/workflow/sessions?**', async (route) => { + operatorRequests.push(route.request().url()) + await route.fulfill({ status: 200, contentType: 'application/json', body: '{}' }) + }) + await page.route('**/api/webhooks**', async (route) => { + operatorRequests.push(route.request().url()) + await route.fulfill({ status: 200, contentType: 'application/json', body: '[]' }) + }) + + await page.goto('/') + + await expect(page.getByRole('heading', { name: /Welcome/i })).toBeVisible() + await expect(page.locator('#journey-select')).toHaveCount(0) + await expect(page.locator('#persona-select')).toHaveCount(0) + await expect(page.getByRole('heading', { name: /Session History/i })).toHaveCount(0) + await expect(page.getByRole('heading', { name: /Webhook Deliveries/i })).toHaveCount(0) + expect(operatorRequests).toEqual([]) + expect(authMeCalls).toBe(0) + }) }) diff --git a/src/frontend/e2e/onboarding-journeys.spec.ts b/src/frontend/e2e/onboarding-journeys.spec.ts index 8c470ab..a3129ad 100644 --- a/src/frontend/e2e/onboarding-journeys.spec.ts +++ b/src/frontend/e2e/onboarding-journeys.spec.ts @@ -279,9 +279,8 @@ test('journey 1 runs simple small business onboarding flow', async ({ page }) => }) test('journey 2 reaches medium business document verification step', async ({ page }) => { - await page.goto('/') + await page.goto('/?flowId=22222222-2222-2222-2222-222222222222') - await page.selectOption('#journey-select', '22222222-2222-2222-2222-222222222222') await expect(page.locator('form').getByText('Medium business details')).toBeVisible() await page.locator('#BusinessName').fill('Acme Regional Ltd') @@ -315,9 +314,8 @@ test('journey 2 reaches medium business document verification step', async ({ pa }) test('journey 3 runs large nationwide onboarding and compliance questions', async ({ page }) => { - await page.goto('/') + await page.goto('/?flowId=33333333-3333-3333-3333-333333333333') - await page.selectOption('#journey-select', '33333333-3333-3333-3333-333333333333') await expect(page.locator('form').getByText('Large business profile')).toBeVisible() await page.locator('#BusinessName').fill('Acme National PLC') diff --git a/src/frontend/e2e/step-renderer.spec.ts b/src/frontend/e2e/step-renderer.spec.ts index e64a423..6460ff3 100644 --- a/src/frontend/e2e/step-renderer.spec.ts +++ b/src/frontend/e2e/step-renderer.spec.ts @@ -107,7 +107,9 @@ test.describe('StepRenderer – Information node', () => { await page.goto('/') await page.getByRole('button', { name: /continue/i }).click() - await expect(page.getByText(/all steps finished/i)).toBeVisible() + // The journey-map visualisation that used to show completion here is an operator tool and no + // longer renders for applicants; this is the applicant-facing completion state. + await expect(page.getByText(/your application is complete/i)).toBeVisible() const parsed = JSON.parse(submitBody ?? '{}') as Record expect(parsed).toEqual({ payload: {} }) }) diff --git a/src/frontend/src/App.routing.test.tsx b/src/frontend/src/App.routing.test.tsx new file mode 100644 index 0000000..06ef9d1 --- /dev/null +++ b/src/frontend/src/App.routing.test.tsx @@ -0,0 +1,178 @@ +import { render, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import App from './App' + +/** + * The public applicant page must contain no operator surface at all: not the data, not the + * components, and not the requests that fetch them. + */ +describe('App routing and route guards', () => { + let fetchMock: ReturnType + + function setPath(path: string, search = '') { + window.history.replaceState({}, '', `${path}${search}`) + } + + beforeEach(() => { + fetchMock = vi.fn(async (input: RequestInfo | URL) => { + const url = typeof input === 'string' ? input : input.toString() + + if (url.includes('/api/workflow/sessions/start')) { + return new Response( + JSON.stringify({ + sessionId: 'session-1', + isCompleted: false, + currentNode: { + id: 'node-1', + key: 'start', + type: 'Information', + title: 'Welcome aboard', + jsonContent: JSON.stringify({ message: 'Hello' }), + }, + applicantToken: 'applicant-token', + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ) + } + + if (url.includes('/api/auth/me')) { + return new Response(JSON.stringify({ authenticated: false }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + } + + return new Response('{}', { status: 200, headers: { 'Content-Type': 'application/json' } }) + }) + + vi.stubGlobal('fetch', fetchMock) + vi.stubGlobal('EventSource', undefined) + }) + + afterEach(() => { + vi.unstubAllGlobals() + vi.restoreAllMocks() + setPath('/') + }) + + function requestedUrls(): string[] { + return fetchMock.mock.calls.map(([input]) => + typeof input === 'string' ? input : String(input), + ) + } + + describe('public applicant route', () => { + beforeEach(() => setPath('/')) + + it('renders the onboarding step and nothing operator-facing', async () => { + render() + + await screen.findByText('Welcome aboard') + + // Each of these headings belongs to an operator surface that used to render here. + for (const heading of [ + /Journey Dashboard/i, + /Flow Analytics/i, + /Session History/i, + /Webhook Deliveries/i, + /Flow Authoring/i, + /Version History/i, + /Operator Console/i, + ]) { + expect(screen.queryByText(heading)).toBeNull() + } + }) + + it('does not render the journey or persona selectors', async () => { + render() + await screen.findByText('Welcome aboard') + + expect(document.querySelector('#journey-select')).toBeNull() + expect(document.querySelector('#persona-select')).toBeNull() + }) + + it('requests no operator-only endpoint', async () => { + render() + await screen.findByText('Welcome aboard') + + const operatorPaths = [ + '/api/workflow/sessions?', + '/api/webhooks', + '/api/flows/', + '/api/analytics/', + '/api/customers', + '/api/auth/me', + ] + + for (const path of operatorPaths) { + expect(requestedUrls().filter((url) => url.includes(path))).toEqual([]) + } + }) + + it('starts the journey named by the link', async () => { + setPath('/', '?flowId=22222222-2222-2222-2222-222222222222') + render() + + await waitFor(() => { + const startCall = fetchMock.mock.calls.find(([input]) => + String(input).includes('/sessions/start'), + ) + expect(startCall).toBeDefined() + expect(String(startCall![1]?.body)).toContain('22222222-2222-2222-2222-222222222222') + }) + }) + }) + + describe('operator routes', () => { + it.each(['/admin', '/admin/journey-builder', '/admin/not-built-yet'])( + 'redirects an unauthenticated visitor from %s to /login without rendering operator data', + async (path) => { + setPath(path) + render() + + await waitFor(() => expect(window.location.pathname).toBe('/login')) + + // Headings, not free text: the login page legitimately mentions the operator console. + expect(screen.queryByRole('heading', { name: /Waymark Operator Console/i })).toBeNull() + expect(screen.queryByRole('heading', { name: /Session History/i })).toBeNull() + expect(screen.queryByRole('heading', { name: /Webhook Deliveries/i })).toBeNull() + }, + ) + + it('renders the operator console once the Operator role is confirmed', async () => { + fetchMock.mockImplementation(async (input: RequestInfo | URL) => { + const url = String(input) + if (url.includes('/api/auth/me')) { + return new Response( + JSON.stringify({ authenticated: true, roles: ['Operator'] }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ) + } + return new Response('[]', { status: 200, headers: { 'Content-Type': 'application/json' } }) + }) + + setPath('/admin') + render() + + expect( + await screen.findByRole('heading', { name: /Waymark Operator Console/i }), + ).toBeInTheDocument() + }) + + it('shows the checking state and no operator data while the role is unconfirmed', async () => { + // Auth never resolves: the console must not render in the meantime. + fetchMock.mockImplementation( + (input: RequestInfo | URL) => + String(input).includes('/api/auth/me') + ? new Promise(() => {}) + : Promise.resolve(new Response('[]', { status: 200 })), + ) + + setPath('/admin') + render() + + expect(screen.getByText(/Checking admin session/i)).toBeInTheDocument() + expect(screen.queryByRole('heading', { name: /Waymark Operator Console/i })).toBeNull() + }) + }) +}) diff --git a/src/frontend/src/App.tsx b/src/frontend/src/App.tsx index 8f27d3b..2f65e45 100644 --- a/src/frontend/src/App.tsx +++ b/src/frontend/src/App.tsx @@ -1,75 +1,8 @@ import { useCallback, useEffect, useState } from 'react' +import { AdminConsolePage } from './admin/AdminConsolePage' import { AdminJourneyBuilderPage } from './builder/AdminJourneyBuilderPage' -import { FlowAuthoringPanel } from './builder/FlowAuthoringPanel' -import { JourneyBuilder } from './builder/JourneyBuilder' -import { buildVersionToPersonaMap, resolveFlowIdForPersona, upsertPersonaAssignment, type PersonaAssignment } from './builder/personaRouting' -import { StepRenderer } from './onboarding/components/StepRenderer' -import { useFlow } from './onboarding/hooks/useFlow' -import { useOnboarding } from './onboarding/hooks/useOnboarding' -import { FlowAnalytics } from './analytics/FlowAnalytics' -import { JourneyAnalyticsProvider } from './analytics/JourneyAnalyticsContext' -import { consoleAnalyticsSink } from './analytics/consoleAnalyticsSink' -import { WebhookDeliveries } from './webhooks/WebhookDeliveries' -import { SessionList } from './sessions/SessionList' -import { SessionDetail } from './sessions/SessionDetail' -import type { SessionSummary } from './sessions/SessionList' -import { FlowVersionHistory } from './flows/FlowVersionHistory' - -type JourneyOption = { - id: string - label: string - description: string -} - -type PersonaOption = { - key: string - label: string -} - -const JOURNEYS: JourneyOption[] = [ - { - id: '11111111-1111-1111-1111-111111111111', - label: 'Journey 1 — Small business onboarding', - description: - 'Simple flow collecting business name, address, revenue, owner details and mocked online document verification.', - }, - { - id: '22222222-2222-2222-2222-222222222222', - label: 'Journey 2 — Medium business onboarding', - description: - 'Medium flow with primary and secondary owners, outlets/staff size, document upload, and mocked Experian + Companies House checks.', - }, - { - id: '33333333-3333-3333-3333-333333333333', - label: 'Journey 3 — Large nationwide business onboarding', - description: - 'High-complexity flow with legal structure, advanced compliance questionnaire, outlets/staff size, and mocked Experian + Companies House checks.', - }, - { - id: 'a1a1a1a1-a1a1-a1a1-a1a1-a1a1a1a1a1a1', - label: 'Journey A — Linear basic (test journey)', - description: - 'Two-step linear journey: contact details form followed by a confirmation screen. Used for Playwright E2E test verification.', - }, - { - id: 'b2b2b2b2-b2b2-b2b2-b2b2-b2b2b2b2b2b2', - label: 'Journey B — Conditional branch (test journey)', - description: - 'Demonstrates conditional routing: EU applicants (France, Germany) see a GDPR disclosure; others see global terms.', - }, - { - id: 'c3c3c3c3-c3c3-c3c3-c3c3-c3c3c3c3c3c3', - label: 'Journey C — Compliance heavy (test journey)', - description: - 'Strict compliance rules, national ID pattern validation, document upload, and redirect to external verification service.', - }, -] -const defaultFlowId = JOURNEYS[0].id -const PERSONAS: PersonaOption[] = [ - { key: 'new-user', label: 'New User' }, - { key: 'enterprise-admin', label: 'Enterprise Admin' }, - { key: 'legacy-migratee', label: 'Legacy Migratee' }, -] +import { ApplicantJourneyPage } from './onboarding/ApplicantJourneyPage' +import { resolveRoute } from './routing/routes' type AuthMeResponse = { authenticated: boolean @@ -78,6 +11,9 @@ type AuthMeResponse = { type AdminAuthState = 'checking' | 'authorized' | 'unauthorized' +/** An auth result is only meaningful for the path it was fetched for. */ +type AdminAuthResult = { path: string; state: Exclude } + const LOGIN_ERROR_MESSAGES: Record = { saml_access_denied: 'Access Denied: User not authorized in IdP.', saml_certificate_expired: 'SAML certificate expired.', @@ -103,49 +39,17 @@ function buildReturnUrl(path: string) { } function App() { - const { step, startSession, submitStep, isLoading, error } = useOnboarding() const [currentPath, setCurrentPath] = useState(() => window.location.pathname) const [currentSearch, setCurrentSearch] = useState(() => window.location.search) - const [adminAuthState, setAdminAuthState] = useState('checking') - const [selectedFlowId, setSelectedFlowId] = useState(JOURNEYS[0].id) - const [selectedPersona, setSelectedPersona] = useState(PERSONAS[0].key) - const [personaAssignments, setPersonaAssignments] = useState([]) - const [visitedNodeIds, setVisitedNodeIds] = useState>(new Set()) - const [builderFlowId, setBuilderFlowId] = useState(defaultFlowId) - const [latestSelectedVersion, setLatestSelectedVersion] = useState(null) - const [selectedSession, setSelectedSession] = useState(null) - const { flow: builderFlow } = useFlow(builderFlowId) + const [adminAuthResult, setAdminAuthResult] = useState(null) - const effectiveFlowId = resolveFlowIdForPersona(personaAssignments, selectedPersona, selectedFlowId) - const activePersonasByVersion = buildVersionToPersonaMap( - personaAssignments.filter((a) => a.flowId === builderFlowId), - ) + const route = resolveRoute(currentPath) + const requiresOperatorRole = route.requiredRole === 'Operator' - useEffect(() => { - if (currentPath === '/login') { - return - } - - if (currentPath.startsWith('/admin/journey-builder') && adminAuthState !== 'authorized') { - return - } - - startSession({ flowId: effectiveFlowId }) - .then((result) => { - if (result.currentNode?.id) { - setVisitedNodeIds(new Set([result.currentNode.id])) - } else { - setVisitedNodeIds(new Set()) - } - }) - .catch(() => undefined) - }, [adminAuthState, currentPath, effectiveFlowId, startSession]) - - const selectedJourney = JOURNEYS.find((j) => j.id === selectedFlowId) ?? JOURNEYS[0] - const assignedFlowIdForPersona = resolveFlowIdForPersona(personaAssignments, selectedPersona, selectedFlowId) - const selectedPersonaLabel = PERSONAS.find((persona) => persona.key === selectedPersona)?.label ?? selectedPersona - const isAdminJourneyBuilderRoute = currentPath.startsWith('/admin/journey-builder') - const isLoginRoute = currentPath === '/login' + // Derived rather than stored, so a result fetched for a previous path can never be mistaken for + // this one's - navigating to another admin route re-checks instead of reusing the old answer. + const adminAuthState: AdminAuthState = + adminAuthResult?.path === currentPath ? adminAuthResult.state : 'checking' useEffect(() => { const handlePopState = () => { @@ -163,15 +67,15 @@ function App() { } else { window.history.pushState({}, '', path) } - if (path.startsWith('/admin/journey-builder')) { - setAdminAuthState('checking') - } setCurrentPath(window.location.pathname) setCurrentSearch(window.location.search) }, []) + // Identity is checked only for routes that need it, so the public page makes no auth call. useEffect(() => { - if (!isAdminJourneyBuilderRoute) return + if (!requiresOperatorRole) return + + let cancelled = false fetch(buildApiUrl('/api/auth/me'), { credentials: 'include' }) .then(async (response) => { @@ -179,25 +83,34 @@ function App() { return (await response.json()) as AuthMeResponse }) .then((identity) => { + if (cancelled) return const isAuthorized = identity.authenticated && (identity.roles ?? []).includes('Operator') if (!isAuthorized) { - setAdminAuthState('unauthorized') + setAdminAuthResult({ path: currentPath, state: 'unauthorized' }) navigate(`/login?returnUrl=${encodeURIComponent(buildReturnUrl(currentPath))}`, true) return } - setAdminAuthState('authorized') + setAdminAuthResult({ path: currentPath, state: 'authorized' }) }) .catch(() => { - setAdminAuthState('unauthorized') - navigate(`/login?error=auth_check_failed&returnUrl=${encodeURIComponent(buildReturnUrl(currentPath))}`, true) + if (cancelled) return + setAdminAuthResult({ path: currentPath, state: 'unauthorized' }) + navigate( + `/login?error=auth_check_failed&returnUrl=${encodeURIComponent(buildReturnUrl(currentPath))}`, + true, + ) }) - }, [currentPath, isAdminJourneyBuilderRoute, navigate]) - if (isLoginRoute) { + return () => { + cancelled = true + } + }, [currentPath, requiresOperatorRole, navigate]) + + if (route.id === 'login') { const params = new URLSearchParams(currentSearch) const errorCode = params.get('error') - const returnUrl = params.get('returnUrl') ?? buildReturnUrl('/admin/journey-builder') + const returnUrl = params.get('returnUrl') ?? buildReturnUrl('/admin') const message = errorCode ? (LOGIN_ERROR_MESSAGES[errorCode] ?? 'Authentication failed.') : null const loginUrl = `${buildApiUrl('/auth/saml/login')}?${new URLSearchParams({ returnUrl }).toString()}` @@ -205,7 +118,7 @@ function App() {

Admin Login

-

Sign in to access the Journey Builder admin experience.

+

Sign in to access the Waymark operator console.

{message ?

{message}

: null}
- ) - } + if (requiresOperatorRole) { + // Nothing operator-facing renders until the Operator role is confirmed. The redirect to /login + // happens in the effect above; this is what the user sees until it does. + if (adminAuthState !== 'authorized') { + return ( +
+

Checking admin session…

+
+ ) + } - if (isAdminJourneyBuilderRoute && adminAuthState === 'authorized') { - return + return route.id === 'admin-journey-builder' ? : } - return ( - -
-
-

Open Onboarding

-

Schema-driven onboarding UI with workflow branching and compliance checks.

-
- -
-

Journey Dashboard

-
- - -
-

{selectedJourney.description}

-
-

- Active persona route: {selectedPersonaLabel} →{' '} - {assignedFlowIdForPersona} -

- -
- {personaAssignments.length > 0 && ( -
    - {personaAssignments.map((assignment) => ( -
  • - {assignment.personaKey} → {assignment.flowId} (live v{assignment.liveVersion ?? 'n/a'}) -
    • - ))} -
- )} -
- - { - setBuilderFlowId(flowId) - setLatestSelectedVersion(version) - }} - /> - - {builderFlowId && ( - undefined} - /> - )} - -
-

Visual Journey Builder (React Flow)

- -
- -
-

Dynamic UI Engine (Step Renderer)

- {error ?

{error}

: null} - {isLoading ?

Loading session…

: null} - {!isLoading && !error && !step ? ( -

Session has not started yet.

- ) : null} - {step ? ( - { - if (!step.currentNode) { - return - } - const nodeId = step.currentNode.id - setVisitedNodeIds((prev) => new Set([...prev, nodeId])) - await submitStep(step.sessionId, nodeId, { payload }) - }} - /> - ) : null} -
-
-

Flow Analytics

- -
- -
-

Session History

- {selectedSession ? ( - setSelectedSession(null)} - /> - ) : ( - - )} -
- -
-

Webhook Deliveries

- -
-
- - ) + return } export default App diff --git a/src/frontend/src/admin/AdminConsolePage.tsx b/src/frontend/src/admin/AdminConsolePage.tsx new file mode 100644 index 0000000..1574acb --- /dev/null +++ b/src/frontend/src/admin/AdminConsolePage.tsx @@ -0,0 +1,169 @@ +import { useState } from 'react' +import { FlowAuthoringPanel } from '../builder/FlowAuthoringPanel' +import { JourneyBuilder } from '../builder/JourneyBuilder' +import { + buildVersionToPersonaMap, + upsertPersonaAssignment, + type PersonaAssignment, +} from '../builder/personaRouting' +import { FlowAnalytics } from '../analytics/FlowAnalytics' +import { FlowVersionHistory } from '../flows/FlowVersionHistory' +import { SessionDetail } from '../sessions/SessionDetail' +import { SessionList, type SessionSummary } from '../sessions/SessionList' +import { WebhookDeliveries } from '../webhooks/WebhookDeliveries' +import { JOURNEYS, PERSONAS, defaultFlowId } from '../journeys' + +/** + * Operator console: flow authoring, version history, analytics, session history and webhook + * deliveries. + * + * Every section here reads operator-only data — session lists include applicants' submitted + * fields — so the whole page sits behind the Operator check in App. It used to render below the + * step renderer on the public applicant page. + */ +export function AdminConsolePage() { + const [selectedFlowId, setSelectedFlowId] = useState(defaultFlowId) + const [selectedPersona, setSelectedPersona] = useState(PERSONAS[0].key) + const [personaAssignments, setPersonaAssignments] = useState([]) + const [builderFlowId, setBuilderFlowId] = useState(defaultFlowId) + const [latestSelectedVersion, setLatestSelectedVersion] = useState(null) + const [selectedSession, setSelectedSession] = useState(null) + + const selectedJourney = JOURNEYS.find((j) => j.id === selectedFlowId) ?? JOURNEYS[0] + const selectedPersonaLabel = + PERSONAS.find((persona) => persona.key === selectedPersona)?.label ?? selectedPersona + const activePersonasByVersion = buildVersionToPersonaMap( + personaAssignments.filter((a) => a.flowId === builderFlowId), + ) + + return ( +
+
+

Waymark Operator Console

+

+ Author journeys, review sessions, and inspect webhook deliveries. +

+ + Open the visual Journey Builder → + +
+ +
+

Journey Dashboard

+
+ + +
+

{selectedJourney.description}

+
+

+ Preview this journey as an applicant:{' '} + + /?flowId={selectedFlowId} + +

+

+ Selected persona: {selectedPersonaLabel} +

+ +
+ {personaAssignments.length > 0 && ( +
    + {personaAssignments.map((assignment) => ( +
  • + {assignment.personaKey} → {assignment.flowId} (live v{assignment.liveVersion ?? 'n/a'}) +
    • + ))} +
+ )} +
+ + { + setBuilderFlowId(flowId) + setLatestSelectedVersion(version) + }} + /> + + {builderFlowId && ( + undefined} + /> + )} + +
+

Journey Map

+ +
+ +
+

Flow Analytics

+ +
+ +
+

Session History

+ {selectedSession ? ( + setSelectedSession(null)} /> + ) : ( + + )} +
+ +
+

Webhook Deliveries

+ +
+
+ ) +} diff --git a/src/frontend/src/journeys.ts b/src/frontend/src/journeys.ts new file mode 100644 index 0000000..1ffe93c --- /dev/null +++ b/src/frontend/src/journeys.ts @@ -0,0 +1,62 @@ +/** + * Seeded demo journeys and personas. + * + * Operator-facing catalogue: the console lists these so an operator can pick a journey to author + * or preview. The applicant page does not choose from them - it renders the journey its link names. + */ +export type JourneyOption = { + id: string + label: string + description: string +} + +export type PersonaOption = { + key: string + label: string +} + +export const JOURNEYS: JourneyOption[] = [ + { + id: '11111111-1111-1111-1111-111111111111', + label: 'Journey 1 — Small business onboarding', + description: + 'Simple flow collecting business name, address, revenue, owner details and mocked online document verification.', + }, + { + id: '22222222-2222-2222-2222-222222222222', + label: 'Journey 2 — Medium business onboarding', + description: + 'Medium flow with primary and secondary owners, outlets/staff size, document upload, and mocked Experian + Companies House checks.', + }, + { + id: '33333333-3333-3333-3333-333333333333', + label: 'Journey 3 — Large nationwide business onboarding', + description: + 'High-complexity flow with legal structure, advanced compliance questionnaire, outlets/staff size, and mocked Experian + Companies House checks.', + }, + { + id: 'a1a1a1a1-a1a1-a1a1-a1a1-a1a1a1a1a1a1', + label: 'Journey A — Linear basic (test journey)', + description: + 'Two-step linear journey: contact details form followed by a confirmation screen. Used for Playwright E2E test verification.', + }, + { + id: 'b2b2b2b2-b2b2-b2b2-b2b2-b2b2b2b2b2b2', + label: 'Journey B — Conditional branch (test journey)', + description: + 'Demonstrates conditional routing: EU applicants (France, Germany) see a GDPR disclosure; others see global terms.', + }, + { + id: 'c3c3c3c3-c3c3-c3c3-c3c3-c3c3c3c3c3c3', + label: 'Journey C — Compliance heavy (test journey)', + description: + 'Strict compliance rules, national ID pattern validation, document upload, and redirect to external verification service.', + }, +] +export const defaultFlowId = JOURNEYS[0].id +export const PERSONAS: PersonaOption[] = [ + { key: 'new-user', label: 'New User' }, + { key: 'enterprise-admin', label: 'Enterprise Admin' }, + { key: 'legacy-migratee', label: 'Legacy Migratee' }, +] + diff --git a/src/frontend/src/onboarding/ApplicantJourneyPage.tsx b/src/frontend/src/onboarding/ApplicantJourneyPage.tsx new file mode 100644 index 0000000..a959a72 --- /dev/null +++ b/src/frontend/src/onboarding/ApplicantJourneyPage.tsx @@ -0,0 +1,88 @@ +import { useEffect, useState } from 'react' +import { JourneyAnalyticsProvider } from '../analytics/JourneyAnalyticsContext' +import { consoleAnalyticsSink } from '../analytics/consoleAnalyticsSink' +import { StepRenderer } from './components/StepRenderer' +import { useOnboarding } from './hooks/useOnboarding' +import { defaultFlowId } from '../journeys' + +/** + * The public onboarding experience: one journey, one step at a time. + * + * Deliberately holds nothing an operator would see. Flow authoring, session history, analytics and + * webhook deliveries live in the operator console behind the SSO check; they used to render on this + * page, exposing every applicant's submitted data to anyone who opened it. + * + * The journey comes from the link the applicant was sent (`/?flowId=...`), not from a picker. + */ +export function ApplicantJourneyPage({ search }: { search: string }) { + const { step, startSession, submitStep, isLoading, error } = useOnboarding() + const [visitedNodeIds, setVisitedNodeIds] = useState>(new Set()) + + const flowId = new URLSearchParams(search).get('flowId') ?? defaultFlowId + + useEffect(() => { + startSession({ flowId }) + .then((result) => { + setVisitedNodeIds(result.currentNode?.id ? new Set([result.currentNode.id]) : new Set()) + }) + .catch(() => undefined) + }, [flowId, startSession]) + + const totalVisited = visitedNodeIds.size + + return ( + +
+
+

Welcome

+

+ Complete the steps below to finish your application. +

+
+ +
+ {error ? ( +

+ {error} +

+ ) : null} + {isLoading ?

Loading…

: null} + {!isLoading && !error && !step ? ( +

+ Your application has not started yet. +

+ ) : null} + + {step?.isCompleted ? ( +

+ Your application is complete. Thank you. +

+ ) : null} + + {step && !step.isCompleted ? ( + <> +

+ Step {totalVisited} +

+ { + if (!step.currentNode) return + const nodeId = step.currentNode.id + setVisitedNodeIds((prev) => new Set([...prev, nodeId])) + await submitStep(step.sessionId, nodeId, { payload }) + }} + /> + + ) : null} +
+
+ + ) +} diff --git a/src/frontend/src/routing/routes.test.ts b/src/frontend/src/routing/routes.test.ts new file mode 100644 index 0000000..18be3a1 --- /dev/null +++ b/src/frontend/src/routing/routes.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest' +import { requiresOperator, resolveRoute } from './routes' + +describe('route table', () => { + it('serves the applicant journey at the root', () => { + expect(resolveRoute('/')).toEqual({ id: 'applicant', requiredRole: 'public' }) + }) + + it('leaves the login page public', () => { + expect(resolveRoute('/login')).toEqual({ id: 'login', requiredRole: 'public' }) + }) + + it.each([ + '/admin', + '/admin/', + '/admin/journey-builder', + '/admin/journey-builder/nested', + '/admin/sessions', + ])('requires an operator for %s', (path) => { + expect(requiresOperator(path)).toBe(true) + }) + + it.each(['/', '/login', '/anything-else'])('does not require an operator for %s', (path) => { + expect(requiresOperator(path)).toBe(false) + }) + + it('picks the journey builder over the general admin console', () => { + expect(resolveRoute('/admin/journey-builder').id).toBe('admin-journey-builder') + expect(resolveRoute('/admin').id).toBe('admin-console') + }) + + it('treats an unknown admin path as operator-only rather than falling through to the applicant page', () => { + // The regression this guards: a new admin surface added without a route entry must not end up + // rendering on the public page. + expect(resolveRoute('/admin/not-built-yet')).toEqual({ + id: 'admin-console', + requiredRole: 'Operator', + }) + }) + + it('renders an unknown non-admin path as the applicant page', () => { + expect(resolveRoute('/typo')).toEqual({ id: 'applicant', requiredRole: 'public' }) + }) +}) diff --git a/src/frontend/src/routing/routes.ts b/src/frontend/src/routing/routes.ts new file mode 100644 index 0000000..20bc72e --- /dev/null +++ b/src/frontend/src/routing/routes.ts @@ -0,0 +1,67 @@ +/** + * The one place that says which routes need which role. + * + * Guarding was previously inline in App.tsx and covered a single path, so every operator surface + * added afterwards rendered on the public applicant page by default. Here the default is the other + * way round: an unknown path is treated as operator-only, and a route is public only by being + * listed as such. + */ +export type RequiredRole = 'public' | 'Operator' + +export interface RouteDefinition { + /** Route id, used to pick the page component. */ + readonly id: 'applicant' | 'login' | 'admin-console' | 'admin-journey-builder' + readonly requiredRole: RequiredRole +} + +const ROUTES: ReadonlyArray<{ readonly match: (path: string) => boolean } & RouteDefinition> = [ + { + id: 'login', + requiredRole: 'public', + match: (path) => path === '/login', + }, + { + id: 'admin-journey-builder', + requiredRole: 'Operator', + match: (path) => path.startsWith('/admin/journey-builder'), + }, + { + id: 'admin-console', + requiredRole: 'Operator', + match: (path) => path === '/admin' || path.startsWith('/admin/'), + }, + { + id: 'applicant', + requiredRole: 'public', + match: (path) => path === '/' || path === '', + }, +] + +/** + * An unmatched path under /admin must never fall through to the applicant page, and an unmatched + * path elsewhere is a 404 that the applicant page renders. Anything beginning with /admin is + * therefore operator-only even when no route matches it. + */ +const UNKNOWN_ADMIN: RouteDefinition = { id: 'admin-console', requiredRole: 'Operator' } +const UNKNOWN_PUBLIC: RouteDefinition = { id: 'applicant', requiredRole: 'public' } + +export function resolveRoute(pathname: string): RouteDefinition { + const path = normalize(pathname) + + const matched = ROUTES.find((route) => route.match(path)) + if (matched) return { id: matched.id, requiredRole: matched.requiredRole } + + return path.startsWith('/admin') ? UNKNOWN_ADMIN : UNKNOWN_PUBLIC +} + +export function requiresOperator(pathname: string): boolean { + return resolveRoute(pathname).requiredRole === 'Operator' +} + +/** Trailing slashes are equivalent: /admin/ is the same route as /admin. */ +function normalize(pathname: string): string { + if (pathname.length > 1 && pathname.endsWith('/')) { + return pathname.slice(0, -1) + } + return pathname +} diff --git a/src/frontend/src/test-setup.ts b/src/frontend/src/test-setup.ts index a9d0dd3..5fdeec6 100644 --- a/src/frontend/src/test-setup.ts +++ b/src/frontend/src/test-setup.ts @@ -1 +1,17 @@ import '@testing-library/jest-dom/vitest' + +// jsdom implements neither of these, and React Flow (the journey canvas) calls both on mount. +// Without them any test that renders a page containing the canvas throws before asserting. +if (!('ResizeObserver' in globalThis)) { + globalThis.ResizeObserver = class ResizeObserver { + observe() {} + unobserve() {} + disconnect() {} + } +} + +if (!('DOMMatrixReadOnly' in globalThis)) { + globalThis.DOMMatrixReadOnly = class DOMMatrixReadOnly { + m22 = 1 + } as unknown as typeof DOMMatrixReadOnly +} From e2512d2a99d5cf69456c153d1487753dfe96ff3f Mon Sep 17 00:00:00 2001 From: MaximumTrainer Date: Sun, 13 Sep 2026 08:22:16 +0100 Subject: [PATCH 6/8] feat(analytics): persist the journey event trail and ingest client events The analytics pipeline was complete except for its ends: the only IAnalyticsProvider wrote to the logger, and browser-side events had nowhere to go - there was no ingestion endpoint at all. Per-step timing, drop-off points and every client signal were unreadable after the fact. Backend: - Add the AnalyticsEvents table, DatabaseAnalyticsProvider and an IAnalyticsEventStore port for reading a trail back and pruning it. Selected by Analytics:DatabaseProvider:Enabled, like IEventBus and IVirusScanService; disabling it leaves the console provider working. - Add POST /api/analytics/events. Every event in a batch must name the session the caller's applicant token authorises or the whole batch is refused, so a client cannot write events against another journey. The server stamps source=client rather than trusting the caller, and the endpoint is rate limited and capped at 100 events per batch. - Add GET /api/analytics/sessions/{id}/events, the ordered operator-only trail. - Add GET /api/analytics/flows/{flowId}. The operator analytics view has been calling this all along and getting a 404; the existing stats live at /api/workflow/flows/{id}/stats in a different shape. - Prune events after Analytics:RetentionDays, mirroring the document cleanup service. Retention is measured from write time, not the client-reported timestamp a caller controls. Frontend: - Add an HTTP analytics sink that batches by size and interval, flushes on pagehide so the tail of a journey is not lost when the tab closes, and drops a failed batch instead of retrying. Analytics must never be able to stall an application form, and an unbounded retry queue against a broken endpoint would. Tests: 11 backend ingestion tests including cross-session rejection and that a mixed batch is refused whole; 4 retention tests including that a backdated client timestamp cannot evict a trail early; 11 sink tests covering batching, auth header, and failure isolation. Backend 287/287, frontend 150/150, 16 mocked e2e, lint and build clean. Closes #95 Co-Authored-By: Claude Opus 5 (1M context) --- docs/runbook.md | 29 + .../Controllers/AnalyticsController.cs | 149 +++++ src/backend/OpenOnboarding.Api/Program.cs | 12 + .../OpenOnboarding.Api/appsettings.json | 9 +- .../AnalyticsIngestionTests.cs | 336 ++++++++++ .../AnalyticsRetentionTests.cs | 121 ++++ .../Contracts/AnalyticsEvent.cs | 9 + .../Contracts/AnalyticsIngestContracts.cs | 52 ++ .../Interfaces/IAnalyticsEventStore.cs | 26 + .../Entities/AnalyticsEventRecord.cs | 46 ++ .../ServiceCollectionExtensions.cs | 9 + ...60913071337_AddAnalyticsEvents.Designer.cs | 585 ++++++++++++++++++ .../20260913071337_AddAnalyticsEvents.cs | 57 ++ .../OnboardingDbContextModelSnapshot.cs | 76 ++- .../Persistence/OnboardingDbContext.cs | 16 + .../CleanupExpiredAnalyticsEventsService.cs | 56 ++ .../Services/DatabaseAnalyticsEventStore.cs | 67 ++ .../Services/DatabaseAnalyticsProvider.cs | 43 ++ src/frontend/src/App.routing.test.tsx | 5 +- .../src/analytics/httpAnalyticsSink.test.ts | 159 +++++ .../src/analytics/httpAnalyticsSink.ts | 119 ++++ .../src/onboarding/ApplicantJourneyPage.tsx | 22 +- 22 files changed, 1988 insertions(+), 15 deletions(-) create mode 100644 src/backend/OpenOnboarding.Api/Controllers/AnalyticsController.cs create mode 100644 src/backend/OpenOnboarding.Application.Tests/AnalyticsIngestionTests.cs create mode 100644 src/backend/OpenOnboarding.Application.Tests/AnalyticsRetentionTests.cs create mode 100644 src/backend/OpenOnboarding.Application/Contracts/AnalyticsIngestContracts.cs create mode 100644 src/backend/OpenOnboarding.Application/Interfaces/IAnalyticsEventStore.cs create mode 100644 src/backend/OpenOnboarding.Domain/Entities/AnalyticsEventRecord.cs create mode 100644 src/backend/OpenOnboarding.Infrastructure/Migrations/20260913071337_AddAnalyticsEvents.Designer.cs create mode 100644 src/backend/OpenOnboarding.Infrastructure/Migrations/20260913071337_AddAnalyticsEvents.cs create mode 100644 src/backend/OpenOnboarding.Infrastructure/Services/CleanupExpiredAnalyticsEventsService.cs create mode 100644 src/backend/OpenOnboarding.Infrastructure/Services/DatabaseAnalyticsEventStore.cs create mode 100644 src/backend/OpenOnboarding.Infrastructure/Services/DatabaseAnalyticsProvider.cs create mode 100644 src/frontend/src/analytics/httpAnalyticsSink.test.ts create mode 100644 src/frontend/src/analytics/httpAnalyticsSink.ts diff --git a/docs/runbook.md b/docs/runbook.md index 1724a26..d6b9614 100644 --- a/docs/runbook.md +++ b/docs/runbook.md @@ -104,6 +104,11 @@ All configuration can be set via environment variables or `appsettings.json`. | `Authentication__ApplicantToken__SigningKey` | ✅ (non-dev) | — | HMAC key signing per-session applicant tokens, min 32 chars. **Store as a secret.** Startup fails without it outside Development. | | `Authentication__ApplicantToken__LifetimeMinutes` | optional | `SessionTimeoutMinutes` | Applicant token lifetime. Defaults to the session timeout, since the token is useless once its session is abandoned. | | `SessionTimeoutMinutes` | optional | `60` | Inactivity timeout before sessions are auto-abandoned. | +| `Analytics__DatabaseProvider__Enabled` | optional | `true` | Persist analytics events so a session's trail can be read back. `false` leaves only the console provider. | +| `Analytics__ConsoleProvider__Enabled` | optional | `true` | Write every event to the application log. | +| `Analytics__RetentionDays` | optional | `365` | Age at which stored analytics events are deleted. | +| `Analytics__CleanupIntervalHours` | optional | `24` | How often the analytics retention sweep runs. | +| `RateLimiting__AnalyticsIngestPerMinute` | optional | `120` | Per-caller limit on `POST /api/analytics/events`. | | `Logging__LogLevel__Default` | optional | `Information` | Log verbosity. | | `ASPNETCORE_ENVIRONMENT` | optional | `Production` | `Development`, `Staging`, or `Production`. | @@ -139,6 +144,30 @@ response carries `applicantToken`, an HMAC-signed JWT the browser sends as `GET /api/workflow/sessions/{id}/events` also accepts the token as an `access_token` query parameter. Ownership is enforced identically on both paths. +### Journey analytics + +Two things are counted separately. + +**Aggregate flow figures** — completion rate, drop-off, average duration — are derived from sessions +and submissions on demand at `GET /api/analytics/flows/{flowId}` (operator only). They need no +event history and are unaffected by retention. + +**The event trail** is the per-step record of what happened in one session, readable at +`GET /api/analytics/sessions/{sessionId}/events` (operator only), ordered by occurrence. + +- Server-raised events (session started, step advanced) are emitted by the journey engine. +- Client-raised events are posted by the browser to `POST /api/analytics/events` in batches. The + caller's applicant token must name the session every event in the batch belongs to, or the whole + batch is rejected with `403` — a client cannot write events against someone else's journey. The + server stamps `source`, so a client cannot pass its events off as server-raised. +- Delivery from the browser is best-effort: a failed batch is dropped rather than retried, so a + broken analytics endpoint can never stall an application form. + +Events are stored in the `AnalyticsEvents` table and pruned by a background sweep after +`Analytics__RetentionDays`, measured from **write** time rather than the client-reported timestamp, +which a caller controls. Set `Analytics__DatabaseProvider__Enabled=false` to turn storage off +entirely; the application keeps working and events go only to the log. + ### SAML Single Sign-On (Admin UI) The admin UI signs operators in over SAML 2.0 (`GET /auth/saml/login` → IdP → `POST /auth/saml/callback`). diff --git a/src/backend/OpenOnboarding.Api/Controllers/AnalyticsController.cs b/src/backend/OpenOnboarding.Api/Controllers/AnalyticsController.cs new file mode 100644 index 0000000..b36c026 --- /dev/null +++ b/src/backend/OpenOnboarding.Api/Controllers/AnalyticsController.cs @@ -0,0 +1,149 @@ +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.RateLimiting; +using OpenOnboarding.Api.Authentication; +using OpenOnboarding.Api.Authorization; +using OpenOnboarding.Application.Contracts; +using OpenOnboarding.Application.Interfaces; +using OpenOnboarding.Domain.Entities; + +namespace OpenOnboarding.Api.Controllers; + +/// +/// Ingests client-raised journey analytics and reads back the durable trail. +/// +[ApiController] +[Route("api/analytics")] +[Produces("application/json")] +[Authorize] +public sealed class AnalyticsController( + ITelemetryService telemetryService, + IAnalyticsEventStore analyticsEventStore, + ISessionAnalyticsService sessionAnalyticsService, + IFlowService flowService) : ControllerBase +{ + /// Most events a single batch may carry. + public const int MaxBatchSize = 100; + + /// + /// Records a batch of analytics events raised in the browser. + /// + /// + /// Every event must name the session the caller's own credential authorises, so a client cannot + /// write events against someone else's journey. Operators may post for any session. + /// + [HttpPost("events")] + [EnableRateLimiting("analytics-ingest")] + [Authorize(Policy = "ApplicantOrOperator")] + [Consumes("application/json")] + [ProducesResponseType(typeof(AnalyticsIngestResponse), StatusCodes.Status202Accepted)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] + [ProducesResponseType(StatusCodes.Status403Forbidden)] + public async Task> Ingest( + [FromBody] AnalyticsIngestRequest request, + CancellationToken cancellationToken) + { + if (request.Events.Count == 0) + return Ok(new AnalyticsIngestResponse { Accepted = 0 }); + + if (request.Events.Count > MaxBatchSize) + return BadRequest(new ProblemDetails + { + Status = StatusCodes.Status400BadRequest, + Title = "Batch too large", + Detail = $"A batch may contain at most {MaxBatchSize} events." + }); + + var isOperator = User.IsInRole(AppRoles.Operator); + var callerSessionId = User.FindFirst(ApplicantSessionAuthenticationDefaults.SessionIdClaim)?.Value; + + foreach (var incoming in request.Events) + { + if (string.IsNullOrWhiteSpace(incoming.EventType) || + string.IsNullOrWhiteSpace(incoming.SessionId) || + string.IsNullOrWhiteSpace(incoming.JourneyId)) + { + return BadRequest(new ProblemDetails + { + Status = StatusCodes.Status400BadRequest, + Title = "Invalid analytics event", + Detail = "eventType, journeyId and sessionId are required on every event." + }); + } + + // An applicant token authorises one session; anything else in the batch is forged. + if (!isOperator && + !string.Equals(incoming.SessionId, callerSessionId, StringComparison.OrdinalIgnoreCase)) + { + return Forbid(); + } + } + + foreach (var incoming in request.Events) + { + await telemetryService.TrackAsync( + new AnalyticsEvent + { + EventId = incoming.EventId ?? Guid.NewGuid(), + EventType = incoming.EventType, + JourneyId = incoming.JourneyId, + SessionId = incoming.SessionId, + StepId = incoming.StepId, + StepIndex = incoming.StepIndex, + Payload = incoming.Payload, + OccurredAt = incoming.OccurredAt ?? DateTimeOffset.UtcNow, + // Stamped here, never taken from the caller: a client cannot pass its events + // off as server-raised. + Source = AnalyticsEventSources.Client + }, + cancellationToken); + } + + return Accepted(new AnalyticsIngestResponse { Accepted = request.Events.Count }); + } + + /// + /// Returns one session's analytics trail in the order events occurred. + /// + [HttpGet("sessions/{sessionId:guid}/events")] + [Authorize(Policy = "OperatorOnly")] + [ProducesResponseType(typeof(IReadOnlyList), StatusCodes.Status200OK)] + public async Task>> GetSessionTrail( + [FromRoute] Guid sessionId, + CancellationToken cancellationToken) + => Ok(await analyticsEventStore.GetSessionTrailAsync(sessionId.ToString(), cancellationToken)); + + /// + /// Returns aggregate performance figures for a flow. + /// + [HttpGet("flows/{flowId:guid}")] + [Authorize(Policy = "OperatorOnly")] + [ProducesResponseType(typeof(FlowAnalyticsDto), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + public async Task> GetFlowAnalytics( + [FromRoute] Guid flowId, + CancellationToken cancellationToken) + { + var flow = await flowService.GetFlowAsync(flowId, cancellationToken); + var stats = await sessionAnalyticsService.GetFlowStatsAsync(flowId, cancellationToken); + + var topAbandonment = stats.DropOffByNodeKey + .OrderByDescending(pair => pair.Value) + .Select(pair => pair.Key) + .FirstOrDefault(); + + return Ok(new FlowAnalyticsDto + { + FlowId = flowId, + FlowName = flow.Name, + TotalSessions = stats.TotalSessions, + CompletedSessions = stats.CompletedSessions, + AbandonedSessions = stats.AbandonedSessions, + AverageDurationSeconds = stats.AverageCompletionTimeSeconds, + CompletionRate = stats.TotalSessions == 0 + ? 0 + : (double)stats.CompletedSessions / stats.TotalSessions, + TopAbandonmentNodeTitle = topAbandonment + }); + } +} diff --git a/src/backend/OpenOnboarding.Api/Program.cs b/src/backend/OpenOnboarding.Api/Program.cs index 3869b94..44603b5 100644 --- a/src/backend/OpenOnboarding.Api/Program.cs +++ b/src/backend/OpenOnboarding.Api/Program.cs @@ -223,6 +223,17 @@ await context.HttpContext.Response.WriteAsJsonAsync(new ProblemDetails opt.QueueLimit = 0; }); + var analyticsIngestLimit = builder.Configuration.GetValue("RateLimiting:AnalyticsIngestPerMinute", 120); + + options.AddSlidingWindowLimiter("analytics-ingest", opt => + { + opt.Window = TimeSpan.FromMinutes(1); + opt.SegmentsPerWindow = 4; + opt.PermitLimit = analyticsIngestLimit; + opt.QueueProcessingOrder = System.Threading.RateLimiting.QueueProcessingOrder.OldestFirst; + opt.QueueLimit = 0; + }); + options.AddSlidingWindowLimiter("general", opt => { opt.Window = TimeSpan.FromMinutes(1); @@ -238,6 +249,7 @@ await context.HttpContext.Response.WriteAsJsonAsync(new ProblemDetails options.AddPolicy("session-start", _ => System.Threading.RateLimiting.RateLimitPartition.GetNoLimiter("testing")); options.AddPolicy("webhook-registration", _ => System.Threading.RateLimiting.RateLimitPartition.GetNoLimiter("testing")); options.AddPolicy("general", _ => System.Threading.RateLimiting.RateLimitPartition.GetNoLimiter("testing")); + options.AddPolicy("analytics-ingest", _ => System.Threading.RateLimiting.RateLimitPartition.GetNoLimiter("testing")); } }); diff --git a/src/backend/OpenOnboarding.Api/appsettings.json b/src/backend/OpenOnboarding.Api/appsettings.json index 30f9445..ec525a5 100644 --- a/src/backend/OpenOnboarding.Api/appsettings.json +++ b/src/backend/OpenOnboarding.Api/appsettings.json @@ -24,13 +24,20 @@ } }, "SessionTimeoutMinutes": 1440, + "Analytics": { + "ConsoleProvider": { "Enabled": true }, + "DatabaseProvider": { "Enabled": true }, + "RetentionDays": 365, + "CleanupIntervalHours": 24 + }, "DocumentUpload": { "MaxFileSizeBytes": 10485760 }, "RateLimiting": { "SessionStartPerMinute": 100, "WebhookRegistrationPerMinute": 20, - "GeneralPerMinute": 300 + "GeneralPerMinute": 300, + "AnalyticsIngestPerMinute": 120 }, "VirusScan": { "Enabled": false, diff --git a/src/backend/OpenOnboarding.Application.Tests/AnalyticsIngestionTests.cs b/src/backend/OpenOnboarding.Application.Tests/AnalyticsIngestionTests.cs new file mode 100644 index 0000000..51b23a8 --- /dev/null +++ b/src/backend/OpenOnboarding.Application.Tests/AnalyticsIngestionTests.cs @@ -0,0 +1,336 @@ +using System.Net; +using System.Net.Http.Headers; +using System.Net.Http.Json; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using OpenOnboarding.Application.Contracts; +using OpenOnboarding.Application.Interfaces; +using OpenOnboarding.Domain.Entities; +using OpenOnboarding.Infrastructure.DependencyInjection; +using OpenOnboarding.Infrastructure.Services; + +namespace OpenOnboarding.Application.Tests; + +/// +/// Analytics events must survive past the log: a completed journey should leave a readable, +/// ordered trail, and a browser must be able to add to it without being able to forge events +/// against another session. +/// +public sealed class AnalyticsIngestionTests +{ + private const string OperatorApiKey = "test-api-key"; + + private sealed record StartResponse(Guid SessionId, NodeStub? CurrentNode, string? ApplicantToken); + private sealed record NodeStub(Guid Id); + private sealed record FlowStub(Guid Id); + private sealed record TrailEvent(string EventType, string SessionId, string Source, DateTimeOffset OccurredAt); + + private static HttpClient CreateClient(WebApplicationFactory factory) + => factory.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false }); + + private static HttpClient CreateOperatorClient(WebApplicationFactory factory) + { + var client = CreateClient(factory); + client.DefaultRequestHeaders.Add("X-Api-Key", OperatorApiKey); + return client; + } + + private static async Task CreateFlowAsync(HttpClient operatorClient) + { + var payload = new + { + name = $"Analytics flow {Guid.NewGuid()}", + nodes = new[] + { + new { key = "start", type = "Form", title = "Start", isStartNode = true, jsonContent = "{\"fields\":[]}" } + }, + connections = Array.Empty() + }; + + var response = await operatorClient.PostAsJsonAsync("/api/flows", payload); + response.EnsureSuccessStatusCode(); + return (await response.Content.ReadFromJsonAsync())!.Id; + } + + private static async Task StartSessionAsync(HttpClient client, Guid flowId) + { + var response = await client.PostAsJsonAsync("/api/workflow/sessions/start", new { flowId }); + response.EnsureSuccessStatusCode(); + return (await response.Content.ReadFromJsonAsync())!; + } + + private static HttpRequestMessage WithToken(HttpMethod method, string url, string token) + { + var request = new HttpRequestMessage(method, url); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + return request; + } + + private static object IngestBatch(Guid sessionId, Guid flowId, params string[] eventTypes) => new + { + events = eventTypes.Select((eventType, index) => new + { + eventId = Guid.NewGuid(), + eventType, + journeyId = flowId.ToString(), + sessionId = sessionId.ToString(), + stepIndex = index, + payload = new Dictionary { ["index"] = index }, + occurredAt = DateTimeOffset.UtcNow.AddSeconds(index) + }).ToArray() + }; + + // ======================================================================= + // Server-raised events are persisted, not just logged + // ======================================================================= + [Fact] + public async Task CompletingAJourney_LeavesAnOrderedTrail() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var start = await StartSessionAsync(browser, flowId); + + var submit = WithToken( + HttpMethod.Post, + $"/api/workflow/sessions/{start.SessionId}/steps/{start.CurrentNode!.Id}/submit", + start.ApplicantToken!); + submit.Content = JsonContent.Create(new { payload = new Dictionary() }); + (await browser.SendAsync(submit)).EnsureSuccessStatusCode(); + + var trail = await operatorClient.GetFromJsonAsync>( + $"/api/analytics/sessions/{start.SessionId}/events"); + + Assert.NotNull(trail); + Assert.NotEmpty(trail!); + Assert.All(trail, e => Assert.Equal(start.SessionId.ToString(), e.SessionId)); + Assert.All(trail, e => Assert.Equal(AnalyticsEventSources.Server, e.Source)); + Assert.Equal(trail.OrderBy(e => e.OccurredAt).Select(e => e.EventType), trail.Select(e => e.EventType)); + } + + // ======================================================================= + // Client ingestion + // ======================================================================= + [Fact] + public async Task ClientBatch_IsAcceptedAndJoinsTheTrailMarkedAsClient() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var start = await StartSessionAsync(browser, flowId); + + var ingest = WithToken(HttpMethod.Post, "/api/analytics/events", start.ApplicantToken!); + ingest.Content = JsonContent.Create(IngestBatch(start.SessionId, flowId, "step_viewed", "field_focused")); + + var response = await browser.SendAsync(ingest); + Assert.Equal(HttpStatusCode.Accepted, response.StatusCode); + + var trail = await operatorClient.GetFromJsonAsync>( + $"/api/analytics/sessions/{start.SessionId}/events"); + + var clientEvents = trail!.Where(e => e.Source == AnalyticsEventSources.Client).ToList(); + Assert.Equal(2, clientEvents.Count); + Assert.Contains(clientEvents, e => e.EventType == "step_viewed"); + Assert.Contains(clientEvents, e => e.EventType == "field_focused"); + } + + [Fact] + public async Task ClientBatch_ForAnotherSession_IsForbidden() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var sessionA = await StartSessionAsync(browser, flowId); + var sessionB = await StartSessionAsync(browser, flowId); + + // Session A's credential, session B's events. + var ingest = WithToken(HttpMethod.Post, "/api/analytics/events", sessionA.ApplicantToken!); + ingest.Content = JsonContent.Create(IngestBatch(sessionB.SessionId, flowId, "step_viewed")); + + Assert.Equal(HttpStatusCode.Forbidden, (await browser.SendAsync(ingest)).StatusCode); + } + + [Fact] + public async Task ClientBatch_MixingInAnotherSession_IsRejectedEntirely() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var sessionA = await StartSessionAsync(browser, flowId); + var sessionB = await StartSessionAsync(browser, flowId); + + var mixed = new + { + events = new[] + { + new { eventType = "step_viewed", journeyId = flowId.ToString(), sessionId = sessionA.SessionId.ToString() }, + new { eventType = "step_viewed", journeyId = flowId.ToString(), sessionId = sessionB.SessionId.ToString() } + } + }; + + var ingest = WithToken(HttpMethod.Post, "/api/analytics/events", sessionA.ApplicantToken!); + ingest.Content = JsonContent.Create(mixed); + Assert.Equal(HttpStatusCode.Forbidden, (await browser.SendAsync(ingest)).StatusCode); + + // The valid half must not have been written either. + var trail = await operatorClient.GetFromJsonAsync>( + $"/api/analytics/sessions/{sessionA.SessionId}/events"); + Assert.DoesNotContain(trail!, e => e.Source == AnalyticsEventSources.Client); + } + + [Fact] + public async Task ClientBatch_WithoutAnyCredential_IsRejected() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var start = await StartSessionAsync(browser, flowId); + + var response = await browser.PostAsJsonAsync( + "/api/analytics/events", IngestBatch(start.SessionId, flowId, "step_viewed")); + + Assert.True(response.StatusCode is HttpStatusCode.Unauthorized or HttpStatusCode.Forbidden); + } + + [Fact] + public async Task ClientBatch_OverTheSizeLimit_IsRejected() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var start = await StartSessionAsync(browser, flowId); + + var tooMany = Enumerable.Repeat("step_viewed", 101).ToArray(); + var ingest = WithToken(HttpMethod.Post, "/api/analytics/events", start.ApplicantToken!); + ingest.Content = JsonContent.Create(IngestBatch(start.SessionId, flowId, tooMany)); + + Assert.Equal(HttpStatusCode.BadRequest, (await browser.SendAsync(ingest)).StatusCode); + } + + [Fact] + public async Task ClientBatch_MissingRequiredFields_IsRejected() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var start = await StartSessionAsync(browser, flowId); + + var ingest = WithToken(HttpMethod.Post, "/api/analytics/events", start.ApplicantToken!); + ingest.Content = JsonContent.Create(new + { + events = new[] { new { eventType = "", journeyId = "", sessionId = start.SessionId.ToString() } } + }); + + Assert.Equal(HttpStatusCode.BadRequest, (await browser.SendAsync(ingest)).StatusCode); + } + + // ======================================================================= + // The trail is operator-only + // ======================================================================= + [Fact] + public async Task SessionTrail_IsNotReadableWithAnApplicantToken() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + var start = await StartSessionAsync(browser, flowId); + + var response = await browser.SendAsync(WithToken( + HttpMethod.Get, $"/api/analytics/sessions/{start.SessionId}/events", start.ApplicantToken!)); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + } + + // ======================================================================= + // Flow dashboard endpoint the operator analytics view calls + // ======================================================================= + [Fact] + public async Task FlowAnalytics_ReturnsTheShapeTheOperatorViewExpects() + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = CreateClient(factory); + await StartSessionAsync(browser, flowId); + + var response = await operatorClient.GetAsync($"/api/analytics/flows/{flowId}"); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var dto = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(dto); + Assert.Equal(flowId, dto!.FlowId); + Assert.False(string.IsNullOrWhiteSpace(dto.FlowName)); + Assert.Equal(1, dto.TotalSessions); + Assert.Equal(0, dto.CompletionRate); + } + + // ======================================================================= + // Provider registration is configuration-driven + // ======================================================================= + private static IConfiguration BuildConfiguration(Dictionary? overrides = null) + { + var values = new Dictionary + { + ["ConnectionStrings:OnboardingDb"] = "Host=localhost;Database=open_onboarding;Username=postgres;Password=postgres", + ["VirusScan:Enabled"] = "false" + }; + + foreach (var entry in overrides ?? []) + values[entry.Key] = entry.Value; + + return new ConfigurationBuilder().AddInMemoryCollection(values).Build(); + } + + [Fact] + public void DatabaseAnalyticsProvider_IsRegisteredByDefault_WithItsRetentionSweep() + { + var services = new ServiceCollection(); + services.AddInfrastructure(BuildConfiguration()); + + Assert.Contains(services, d => + d.ServiceType == typeof(IAnalyticsProvider) && + d.ImplementationType == typeof(DatabaseAnalyticsProvider)); + + Assert.Contains(services, d => d.ImplementationType == typeof(CleanupExpiredAnalyticsEventsService)); + } + + [Fact] + public void DatabaseAnalyticsProvider_CanBeDisabled_LeavingTheConsoleProvider() + { + var services = new ServiceCollection(); + services.AddInfrastructure(BuildConfiguration(new Dictionary + { + ["Analytics:DatabaseProvider:Enabled"] = "false" + })); + + Assert.DoesNotContain(services, d => + d.ServiceType == typeof(IAnalyticsProvider) && + d.ImplementationType == typeof(DatabaseAnalyticsProvider)); + + // Nothing to prune, so no sweep either. + Assert.DoesNotContain(services, d => d.ImplementationType == typeof(CleanupExpiredAnalyticsEventsService)); + + // The app still has a working analytics pipeline. + Assert.Contains(services, d => + d.ServiceType == typeof(IAnalyticsProvider) && + d.ImplementationType == typeof(ConsoleAnalyticsProvider)); + } +} diff --git a/src/backend/OpenOnboarding.Application.Tests/AnalyticsRetentionTests.cs b/src/backend/OpenOnboarding.Application.Tests/AnalyticsRetentionTests.cs new file mode 100644 index 0000000..d08e1b6 --- /dev/null +++ b/src/backend/OpenOnboarding.Application.Tests/AnalyticsRetentionTests.cs @@ -0,0 +1,121 @@ +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using OpenOnboarding.Application.Interfaces; +using OpenOnboarding.Domain.Entities; +using OpenOnboarding.Infrastructure.Persistence; +using OpenOnboarding.Infrastructure.Services; + +namespace OpenOnboarding.Application.Tests; + +/// +/// Analytics rows accumulate on every step of every session, so they need a retention window the +/// way stored documents do. +/// +public sealed class AnalyticsRetentionTests +{ + private static OnboardingDbContext BuildDbContext() + => new(new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options); + + private static AnalyticsEventRecord Event(string sessionId, DateTimeOffset recordedAt) => new() + { + EventType = "step_viewed", + JourneyId = "journey-1", + SessionId = sessionId, + RecordedAt = recordedAt, + OccurredAt = recordedAt + }; + + private static CleanupExpiredAnalyticsEventsService BuildService(OnboardingDbContext db) + => new( + new TestServiceScopeFactory(db), + NullLogger.Instance, + new ConfigurationBuilder().AddInMemoryCollection(new Dictionary()).Build()); + + [Fact] + public async Task Cleanup_RemovesEventsPastTheRetentionWindow_AndKeepsTheRest() + { + using var db = BuildDbContext(); + db.AnalyticsEvents.AddRange( + Event("old-1", DateTimeOffset.UtcNow.AddDays(-400)), + Event("old-2", DateTimeOffset.UtcNow.AddDays(-366)), + Event("recent", DateTimeOffset.UtcNow.AddDays(-10))); + await db.SaveChangesAsync(); + + await BuildService(db).RunCleanupAsync(retentionDays: 365); + + var remaining = await db.AnalyticsEvents.Select(e => e.SessionId).ToListAsync(); + Assert.Equal(["recent"], remaining); + } + + [Fact] + public async Task Cleanup_LeavesEverythingWhenNothingHasExpired() + { + using var db = BuildDbContext(); + db.AnalyticsEvents.Add(Event("recent", DateTimeOffset.UtcNow.AddDays(-1))); + await db.SaveChangesAsync(); + + await BuildService(db).RunCleanupAsync(retentionDays: 365); + + Assert.Equal(1, await db.AnalyticsEvents.CountAsync()); + } + + [Fact] + public async Task Retention_MeasuresWriteTimeNotTheClientReportedTimestamp() + { + // A client controls OccurredAt and could backdate it; deleting on that would let a caller + // evict its own trail early. + using var db = BuildDbContext(); + db.AnalyticsEvents.Add(new AnalyticsEventRecord + { + EventType = "step_viewed", + JourneyId = "journey-1", + SessionId = "backdated", + OccurredAt = DateTimeOffset.UtcNow.AddYears(-10), + RecordedAt = DateTimeOffset.UtcNow + }); + await db.SaveChangesAsync(); + + await BuildService(db).RunCleanupAsync(retentionDays: 365); + + Assert.Equal(1, await db.AnalyticsEvents.CountAsync()); + } + + [Fact] + public async Task Store_ReturnsOnlyTheRequestedSessionsTrail() + { + using var db = BuildDbContext(); + db.AnalyticsEvents.AddRange( + Event("session-a", DateTimeOffset.UtcNow.AddMinutes(-2)), + Event("session-b", DateTimeOffset.UtcNow.AddMinutes(-1))); + await db.SaveChangesAsync(); + + var trail = await new DatabaseAnalyticsEventStore(db).GetSessionTrailAsync("session-a"); + + var only = Assert.Single(trail); + Assert.Equal("session-a", only.SessionId); + } + + private sealed class TestServiceScopeFactory(OnboardingDbContext db) : IServiceScopeFactory + { + public IServiceScope CreateScope() => new TestServiceScope(db); + } + + private sealed class TestServiceScope(OnboardingDbContext db) : IServiceScope, IAsyncDisposable + { + public IServiceProvider ServiceProvider { get; } = new TestServiceProvider(db); + public void Dispose() { } + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + } + + private sealed class TestServiceProvider(OnboardingDbContext db) : IServiceProvider, IAsyncDisposable + { + public object? GetService(Type serviceType) + => serviceType == typeof(IAnalyticsEventStore) ? new DatabaseAnalyticsEventStore(db) : null; + + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + } +} diff --git a/src/backend/OpenOnboarding.Application/Contracts/AnalyticsEvent.cs b/src/backend/OpenOnboarding.Application/Contracts/AnalyticsEvent.cs index 57fba66..6e6df1a 100644 --- a/src/backend/OpenOnboarding.Application/Contracts/AnalyticsEvent.cs +++ b/src/backend/OpenOnboarding.Application/Contracts/AnalyticsEvent.cs @@ -1,3 +1,5 @@ +using OpenOnboarding.Domain.Entities; + namespace OpenOnboarding.Application.Contracts; /// @@ -29,4 +31,11 @@ public sealed record AnalyticsEvent /// UTC timestamp when the event occurred. public DateTimeOffset OccurredAt { get; init; } = DateTimeOffset.UtcNow; + + /// + /// Where the event was raised: server for events the journey engine emits, client + /// for events ingested from a browser. Client events are self-reported, so keeping the two + /// apart matters when reading a trail. + /// + public string Source { get; init; } = AnalyticsEventSources.Server; } diff --git a/src/backend/OpenOnboarding.Application/Contracts/AnalyticsIngestContracts.cs b/src/backend/OpenOnboarding.Application/Contracts/AnalyticsIngestContracts.cs new file mode 100644 index 0000000..5821c05 --- /dev/null +++ b/src/backend/OpenOnboarding.Application/Contracts/AnalyticsIngestContracts.cs @@ -0,0 +1,52 @@ +namespace OpenOnboarding.Application.Contracts; + +/// A batch of client-raised analytics events. +public sealed class AnalyticsIngestRequest +{ + public List Events { get; set; } = []; +} + +/// +/// One client-raised event. Deliberately narrower than : the server +/// stamps the source and will not accept a caller-supplied one. +/// +public sealed class AnalyticsIngestEvent +{ + /// Client-generated id, used to make a retried batch idempotent. + public Guid? EventId { get; set; } + + public string EventType { get; set; } = string.Empty; + + public string JourneyId { get; set; } = string.Empty; + + public string SessionId { get; set; } = string.Empty; + + public string? StepId { get; set; } + + public int? StepIndex { get; set; } + + public Dictionary Payload { get; set; } = []; + + public DateTimeOffset? OccurredAt { get; set; } +} + +/// Outcome of an ingest call. +public sealed class AnalyticsIngestResponse +{ + public int Accepted { get; set; } +} + +/// +/// Flow-level dashboard figures, shaped for the operator analytics view. +/// +public sealed class FlowAnalyticsDto +{ + public Guid FlowId { get; set; } + public string FlowName { get; set; } = string.Empty; + public int TotalSessions { get; set; } + public int CompletedSessions { get; set; } + public int AbandonedSessions { get; set; } + public double AverageDurationSeconds { get; set; } + public double CompletionRate { get; set; } + public string? TopAbandonmentNodeTitle { get; set; } +} diff --git a/src/backend/OpenOnboarding.Application/Interfaces/IAnalyticsEventStore.cs b/src/backend/OpenOnboarding.Application/Interfaces/IAnalyticsEventStore.cs new file mode 100644 index 0000000..c0303ed --- /dev/null +++ b/src/backend/OpenOnboarding.Application/Interfaces/IAnalyticsEventStore.cs @@ -0,0 +1,26 @@ +using OpenOnboarding.Application.Contracts; + +namespace OpenOnboarding.Application.Interfaces; + +/// +/// Reads back and prunes the durable analytics event trail. +/// +/// Writing goes through so every sink sees the event; this port is +/// only for the queries and the retention sweep, which are specific to the durable store. +/// +/// +public interface IAnalyticsEventStore +{ + /// Returns one session's events in the order they occurred. + Task> GetSessionTrailAsync( + string sessionId, + CancellationToken cancellationToken = default); + + /// + /// Deletes events recorded before and returns how many were + /// removed. Measured on write time, which a client cannot backdate. + /// + Task DeleteRecordedBeforeAsync( + DateTimeOffset threshold, + CancellationToken cancellationToken = default); +} diff --git a/src/backend/OpenOnboarding.Domain/Entities/AnalyticsEventRecord.cs b/src/backend/OpenOnboarding.Domain/Entities/AnalyticsEventRecord.cs new file mode 100644 index 0000000..c3576b4 --- /dev/null +++ b/src/backend/OpenOnboarding.Domain/Entities/AnalyticsEventRecord.cs @@ -0,0 +1,46 @@ +namespace OpenOnboarding.Domain.Entities; + +/// +/// A persisted journey analytics event. +/// +/// Events were previously written to the application log and nothing else, so the per-step trail +/// that shows where applicants stall could not be queried or exported. Rows here are the durable +/// record; aggregate flow statistics are still derived from sessions and submissions. +/// +/// +public sealed class AnalyticsEventRecord +{ + /// The originating event's id. Doubles as the idempotency key for client retries. + public Guid Id { get; set; } = Guid.NewGuid(); + + public string EventType { get; set; } = string.Empty; + + public string JourneyId { get; set; } = string.Empty; + + public string SessionId { get; set; } = string.Empty; + + public string? StepId { get; set; } + + public int? StepIndex { get; set; } + + /// Event-specific payload, stored as JSON so new event types need no schema change. + public string PayloadJson { get; set; } = "{}"; + + /// Where the event was raised: server or client. + public string Source { get; set; } = AnalyticsEventSources.Server; + + /// When the event happened, as reported by its origin. + public DateTimeOffset OccurredAt { get; set; } = DateTimeOffset.UtcNow; + + /// + /// When the row was written. Retention is measured from this, not , + /// which a client controls and could backdate. + /// + public DateTimeOffset RecordedAt { get; set; } = DateTimeOffset.UtcNow; +} + +public static class AnalyticsEventSources +{ + public const string Server = "server"; + public const string Client = "client"; +} diff --git a/src/backend/OpenOnboarding.Infrastructure/DependencyInjection/ServiceCollectionExtensions.cs b/src/backend/OpenOnboarding.Infrastructure/DependencyInjection/ServiceCollectionExtensions.cs index 32db886..b7b1a99 100644 --- a/src/backend/OpenOnboarding.Infrastructure/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/backend/OpenOnboarding.Infrastructure/DependencyInjection/ServiceCollectionExtensions.cs @@ -79,6 +79,15 @@ public static IServiceCollection AddInfrastructure(this IServiceCollection servi if (configuration.GetValue("Analytics:ConsoleProvider:Enabled", true)) services.AddSingleton(); + // Durable sink. Without it events reach the log and nothing else, so no trail can be read + // back; with it, analytics rows accumulate and need the retention sweep below. + services.AddScoped(); + if (configuration.GetValue("Analytics:DatabaseProvider:Enabled", true)) + { + services.AddSingleton(); + services.AddHostedService(); + } + services.AddSingleton(); // Virus scanning: use real ClamAV adapter when enabled, otherwise no-op diff --git a/src/backend/OpenOnboarding.Infrastructure/Migrations/20260913071337_AddAnalyticsEvents.Designer.cs b/src/backend/OpenOnboarding.Infrastructure/Migrations/20260913071337_AddAnalyticsEvents.Designer.cs new file mode 100644 index 0000000..68dde65 --- /dev/null +++ b/src/backend/OpenOnboarding.Infrastructure/Migrations/20260913071337_AddAnalyticsEvents.Designer.cs @@ -0,0 +1,585 @@ +// +using System; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; +using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata; +using OpenOnboarding.Infrastructure.Persistence; + +#nullable disable + +namespace OpenOnboarding.Infrastructure.Migrations +{ + [DbContext(typeof(OnboardingDbContext))] + [Migration("20260913071337_AddAnalyticsEvents")] + partial class AddAnalyticsEvents + { + /// + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder + .HasAnnotation("ProductVersion", "10.0.7") + .HasAnnotation("Relational:MaxIdentifierLength", 63); + + NpgsqlModelBuilderExtensions.UseIdentityByDefaultColumns(modelBuilder); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.AnalyticsEventRecord", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("EventType") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("character varying(100)"); + + b.Property("JourneyId") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("character varying(100)"); + + b.Property("OccurredAt") + .HasColumnType("timestamp with time zone"); + + b.Property("PayloadJson") + .IsRequired() + .HasColumnType("text"); + + b.Property("RecordedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("SessionId") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("character varying(100)"); + + b.Property("Source") + .IsRequired() + .HasMaxLength(20) + .HasColumnType("character varying(20)"); + + b.Property("StepId") + .HasMaxLength(100) + .HasColumnType("character varying(100)"); + + b.Property("StepIndex") + .HasColumnType("integer"); + + b.HasKey("Id"); + + b.HasIndex("RecordedAt"); + + b.HasIndex("JourneyId", "OccurredAt"); + + b.HasIndex("SessionId", "OccurredAt"); + + b.ToTable("AnalyticsEvents"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Connection", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("ConditionField") + .HasColumnType("text"); + + b.Property("ConditionOperator") + .HasColumnType("integer"); + + b.Property("ConditionValue") + .HasColumnType("text"); + + b.Property("FlowId") + .HasColumnType("uuid"); + + b.Property("Priority") + .HasColumnType("integer"); + + b.Property("SourceNodeId") + .HasColumnType("uuid"); + + b.Property("TargetNodeId") + .HasColumnType("uuid"); + + b.HasKey("Id"); + + b.HasIndex("FlowId"); + + b.ToTable("Connections"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.CustomerProfile", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("Country") + .IsRequired() + .HasMaxLength(10) + .HasColumnType("character varying(10)"); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("Email") + .IsRequired() + .HasMaxLength(320) + .HasColumnType("character varying(320)"); + + b.Property("ExternalCustomerId") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("MetadataJson") + .IsRequired() + .HasColumnType("text"); + + b.Property("UpdatedAt") + .HasColumnType("timestamp with time zone"); + + b.HasKey("Id"); + + b.HasIndex("ExternalCustomerId") + .IsUnique(); + + b.ToTable("CustomerProfiles"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Flow", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("Description") + .HasMaxLength(2000) + .HasColumnType("character varying(2000)"); + + b.Property("Name") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("UpdatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("Version") + .HasColumnType("integer"); + + b.HasKey("Id"); + + b.ToTable("Flows"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.FlowVersion", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("CreatedBy") + .HasColumnType("text"); + + b.Property("FlowId") + .HasColumnType("uuid"); + + b.Property("SnapshotJson") + .IsRequired() + .HasColumnType("text"); + + b.Property("VersionNumber") + .HasColumnType("integer"); + + b.HasKey("Id"); + + b.HasIndex("FlowId", "VersionNumber") + .IsUnique(); + + b.ToTable("FlowVersions"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Node", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("ComplianceRuleJson") + .HasColumnType("text"); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("ExecutionErrorJson") + .HasColumnType("text"); + + b.Property("FlowId") + .HasColumnType("uuid"); + + b.Property("IsStartNode") + .HasColumnType("boolean"); + + b.Property("JsonContent") + .IsRequired() + .HasColumnType("text"); + + b.Property("Key") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("character varying(100)"); + + b.Property("Title") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("Type") + .HasColumnType("integer"); + + b.Property("UpdatedAt") + .HasColumnType("timestamp with time zone"); + + b.HasKey("Id"); + + b.HasIndex("FlowId", "IsStartNode"); + + b.HasIndex("FlowId", "Key") + .IsUnique(); + + b.ToTable("Nodes"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Session", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("CurrentNodeId") + .HasColumnType("uuid"); + + b.Property("CustomerProfileId") + .HasColumnType("uuid"); + + b.Property("FlowId") + .HasColumnType("uuid"); + + b.Property("Status") + .HasColumnType("integer"); + + b.Property("UpdatedAt") + .HasColumnType("timestamp with time zone"); + + b.HasKey("Id"); + + b.HasIndex("CustomerProfileId"); + + b.HasIndex("Status"); + + b.HasIndex("FlowId", "Status"); + + b.ToTable("Sessions"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Submission", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("DataJson") + .IsRequired() + .HasColumnType("text"); + + b.Property("NodeId") + .HasColumnType("uuid"); + + b.Property("SessionId") + .HasColumnType("uuid"); + + b.Property("SubmittedAt") + .HasColumnType("timestamp with time zone"); + + b.HasKey("Id"); + + b.HasIndex("SessionId"); + + b.ToTable("Submissions"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Webhook", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("FlowId") + .HasColumnType("uuid"); + + b.Property("Secret") + .IsRequired() + .HasMaxLength(512) + .HasColumnType("character varying(512)"); + + b.Property("UpdatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("Url") + .IsRequired() + .HasMaxLength(2048) + .HasColumnType("character varying(2048)"); + + b.HasKey("Id"); + + b.HasIndex("FlowId"); + + b.ToTable("Webhooks"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.WebhookDelivery", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("AttemptCount") + .HasColumnType("integer"); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("DeliveredAt") + .HasColumnType("timestamp with time zone"); + + b.Property("EventType") + .IsRequired() + .HasColumnType("text"); + + b.Property("LastResponseBody") + .HasColumnType("text"); + + b.Property("LastStatusCode") + .HasColumnType("integer"); + + b.Property("PayloadJson") + .IsRequired() + .HasColumnType("text"); + + b.Property("SessionId") + .HasColumnType("uuid"); + + b.Property("Status") + .IsRequired() + .HasColumnType("text"); + + b.Property("WebhookId") + .HasColumnType("uuid"); + + b.HasKey("Id"); + + b.HasIndex("WebhookId", "Status"); + + b.ToTable("WebhookDeliveries"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.ReadModels.SessionReadModel", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("AbandonedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("CompletedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("CompletionDurationSeconds") + .HasColumnType("double precision"); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("CurrentNodeId") + .HasColumnType("uuid"); + + b.Property("CurrentNodeKey") + .HasMaxLength(100) + .HasColumnType("character varying(100)"); + + b.Property("CurrentNodeTitle") + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("CustomerCountry") + .HasMaxLength(10) + .HasColumnType("character varying(10)"); + + b.Property("CustomerEmail") + .HasMaxLength(320) + .HasColumnType("character varying(320)"); + + b.Property("ExternalCustomerId") + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("FlowId") + .HasColumnType("uuid"); + + b.Property("FlowName") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("StatusName") + .IsRequired() + .HasMaxLength(20) + .HasColumnType("character varying(20)"); + + b.Property("StepCount") + .HasColumnType("integer"); + + b.Property("UpdatedAt") + .HasColumnType("timestamp with time zone"); + + b.HasKey("Id"); + + b.HasIndex("FlowId"); + + b.HasIndex("StatusName"); + + b.HasIndex("FlowId", "StatusName"); + + b.ToTable("SessionReadModels", (string)null); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Connection", b => + { + b.HasOne("OpenOnboarding.Domain.Entities.Flow", "Flow") + .WithMany("Connections") + .HasForeignKey("FlowId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("Flow"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.FlowVersion", b => + { + b.HasOne("OpenOnboarding.Domain.Entities.Flow", "Flow") + .WithMany("Versions") + .HasForeignKey("FlowId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("Flow"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Node", b => + { + b.HasOne("OpenOnboarding.Domain.Entities.Flow", "Flow") + .WithMany("Nodes") + .HasForeignKey("FlowId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("Flow"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Session", b => + { + b.HasOne("OpenOnboarding.Domain.Entities.CustomerProfile", "CustomerProfile") + .WithMany("Sessions") + .HasForeignKey("CustomerProfileId") + .OnDelete(DeleteBehavior.SetNull); + + b.HasOne("OpenOnboarding.Domain.Entities.Flow", "Flow") + .WithMany() + .HasForeignKey("FlowId") + .OnDelete(DeleteBehavior.Restrict) + .IsRequired(); + + b.Navigation("CustomerProfile"); + + b.Navigation("Flow"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Submission", b => + { + b.HasOne("OpenOnboarding.Domain.Entities.Session", "Session") + .WithMany("Submissions") + .HasForeignKey("SessionId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("Session"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Webhook", b => + { + b.HasOne("OpenOnboarding.Domain.Entities.Flow", "Flow") + .WithMany() + .HasForeignKey("FlowId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("Flow"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.WebhookDelivery", b => + { + b.HasOne("OpenOnboarding.Domain.Entities.Webhook", "Webhook") + .WithMany("Deliveries") + .HasForeignKey("WebhookId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + + b.Navigation("Webhook"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.CustomerProfile", b => + { + b.Navigation("Sessions"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Flow", b => + { + b.Navigation("Connections"); + + b.Navigation("Nodes"); + + b.Navigation("Versions"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Session", b => + { + b.Navigation("Submissions"); + }); + + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Webhook", b => + { + b.Navigation("Deliveries"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/src/backend/OpenOnboarding.Infrastructure/Migrations/20260913071337_AddAnalyticsEvents.cs b/src/backend/OpenOnboarding.Infrastructure/Migrations/20260913071337_AddAnalyticsEvents.cs new file mode 100644 index 0000000..d518c58 --- /dev/null +++ b/src/backend/OpenOnboarding.Infrastructure/Migrations/20260913071337_AddAnalyticsEvents.cs @@ -0,0 +1,57 @@ +using System; +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace OpenOnboarding.Infrastructure.Migrations +{ + /// + public partial class AddAnalyticsEvents : Migration + { + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.CreateTable( + name: "AnalyticsEvents", + columns: table => new + { + Id = table.Column(type: "uuid", nullable: false), + EventType = table.Column(type: "character varying(100)", maxLength: 100, nullable: false), + JourneyId = table.Column(type: "character varying(100)", maxLength: 100, nullable: false), + SessionId = table.Column(type: "character varying(100)", maxLength: 100, nullable: false), + StepId = table.Column(type: "character varying(100)", maxLength: 100, nullable: true), + StepIndex = table.Column(type: "integer", nullable: true), + PayloadJson = table.Column(type: "text", nullable: false), + Source = table.Column(type: "character varying(20)", maxLength: 20, nullable: false), + OccurredAt = table.Column(type: "timestamp with time zone", nullable: false), + RecordedAt = table.Column(type: "timestamp with time zone", nullable: false) + }, + constraints: table => + { + table.PrimaryKey("PK_AnalyticsEvents", x => x.Id); + }); + + migrationBuilder.CreateIndex( + name: "IX_AnalyticsEvents_JourneyId_OccurredAt", + table: "AnalyticsEvents", + columns: new[] { "JourneyId", "OccurredAt" }); + + migrationBuilder.CreateIndex( + name: "IX_AnalyticsEvents_RecordedAt", + table: "AnalyticsEvents", + column: "RecordedAt"); + + migrationBuilder.CreateIndex( + name: "IX_AnalyticsEvents_SessionId_OccurredAt", + table: "AnalyticsEvents", + columns: new[] { "SessionId", "OccurredAt" }); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropTable( + name: "AnalyticsEvents"); + } + } +} diff --git a/src/backend/OpenOnboarding.Infrastructure/Migrations/OnboardingDbContextModelSnapshot.cs b/src/backend/OpenOnboarding.Infrastructure/Migrations/OnboardingDbContextModelSnapshot.cs index aadf691..8ad34d1 100644 --- a/src/backend/OpenOnboarding.Infrastructure/Migrations/OnboardingDbContextModelSnapshot.cs +++ b/src/backend/OpenOnboarding.Infrastructure/Migrations/OnboardingDbContextModelSnapshot.cs @@ -22,6 +22,60 @@ protected override void BuildModel(ModelBuilder modelBuilder) NpgsqlModelBuilderExtensions.UseIdentityByDefaultColumns(modelBuilder); + modelBuilder.Entity("OpenOnboarding.Domain.Entities.AnalyticsEventRecord", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("uuid"); + + b.Property("EventType") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("character varying(100)"); + + b.Property("JourneyId") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("character varying(100)"); + + b.Property("OccurredAt") + .HasColumnType("timestamp with time zone"); + + b.Property("PayloadJson") + .IsRequired() + .HasColumnType("text"); + + b.Property("RecordedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("SessionId") + .IsRequired() + .HasMaxLength(100) + .HasColumnType("character varying(100)"); + + b.Property("Source") + .IsRequired() + .HasMaxLength(20) + .HasColumnType("character varying(20)"); + + b.Property("StepId") + .HasMaxLength(100) + .HasColumnType("character varying(100)"); + + b.Property("StepIndex") + .HasColumnType("integer"); + + b.HasKey("Id"); + + b.HasIndex("RecordedAt"); + + b.HasIndex("JourneyId", "OccurredAt"); + + b.HasIndex("SessionId", "OccurredAt"); + + b.ToTable("AnalyticsEvents"); + }); + modelBuilder.Entity("OpenOnboarding.Domain.Entities.Connection", b => { b.Property("Id") @@ -362,14 +416,6 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.Property("CreatedAt") .HasColumnType("timestamp with time zone"); - b.Property("CustomerCountry") - .HasMaxLength(10) - .HasColumnType("character varying(10)"); - - b.Property("CustomerEmail") - .HasMaxLength(320) - .HasColumnType("character varying(320)"); - b.Property("CurrentNodeId") .HasColumnType("uuid"); @@ -381,6 +427,14 @@ protected override void BuildModel(ModelBuilder modelBuilder) .HasMaxLength(200) .HasColumnType("character varying(200)"); + b.Property("CustomerCountry") + .HasMaxLength(10) + .HasColumnType("character varying(10)"); + + b.Property("CustomerEmail") + .HasMaxLength(320) + .HasColumnType("character varying(320)"); + b.Property("ExternalCustomerId") .HasMaxLength(200) .HasColumnType("character varying(200)"); @@ -408,11 +462,11 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasIndex("FlowId"); - b.HasIndex("FlowId", "StatusName"); - b.HasIndex("StatusName"); - b.ToTable("SessionReadModels"); + b.HasIndex("FlowId", "StatusName"); + + b.ToTable("SessionReadModels", (string)null); }); modelBuilder.Entity("OpenOnboarding.Domain.Entities.Connection", b => diff --git a/src/backend/OpenOnboarding.Infrastructure/Persistence/OnboardingDbContext.cs b/src/backend/OpenOnboarding.Infrastructure/Persistence/OnboardingDbContext.cs index 151348c..59548b3 100644 --- a/src/backend/OpenOnboarding.Infrastructure/Persistence/OnboardingDbContext.cs +++ b/src/backend/OpenOnboarding.Infrastructure/Persistence/OnboardingDbContext.cs @@ -16,6 +16,7 @@ public sealed class OnboardingDbContext(DbContextOptions op public DbSet Webhooks => Set(); public DbSet WebhookDeliveries => Set(); public DbSet SessionReadModels => Set(); + public DbSet AnalyticsEvents => Set(); public override Task SaveChangesAsync(CancellationToken cancellationToken = default) { @@ -34,6 +35,21 @@ public override Task SaveChangesAsync(CancellationToken cancellationToken = protected override void OnModelCreating(ModelBuilder modelBuilder) { + // ── AnalyticsEventRecord ────────────────────────────────────────── + modelBuilder.Entity(b => + { + b.Property(x => x.EventType).HasMaxLength(100).IsRequired(); + b.Property(x => x.JourneyId).HasMaxLength(100).IsRequired(); + b.Property(x => x.SessionId).HasMaxLength(100).IsRequired(); + b.Property(x => x.StepId).HasMaxLength(100); + b.Property(x => x.Source).HasMaxLength(20).IsRequired(); + // Reading one session's trail in order is the common query. + b.HasIndex(x => new { x.SessionId, x.OccurredAt }); + b.HasIndex(x => new { x.JourneyId, x.OccurredAt }); + // Retention sweeps delete by write time. + b.HasIndex(x => x.RecordedAt); + }); + // ── Flow ───────────────────────────────────────────────────────── modelBuilder.Entity(b => { diff --git a/src/backend/OpenOnboarding.Infrastructure/Services/CleanupExpiredAnalyticsEventsService.cs b/src/backend/OpenOnboarding.Infrastructure/Services/CleanupExpiredAnalyticsEventsService.cs new file mode 100644 index 0000000..80a8c1d --- /dev/null +++ b/src/backend/OpenOnboarding.Infrastructure/Services/CleanupExpiredAnalyticsEventsService.cs @@ -0,0 +1,56 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using OpenOnboarding.Application.Interfaces; + +namespace OpenOnboarding.Infrastructure.Services; + +/// +/// Prunes analytics events past their retention window, mirroring +/// . Registered only when the durable provider is on, +/// since there is nothing to prune otherwise. +/// +public sealed class CleanupExpiredAnalyticsEventsService( + IServiceScopeFactory scopeFactory, + ILogger logger, + IConfiguration configuration) : BackgroundService +{ + protected override async Task ExecuteAsync(CancellationToken stoppingToken) + { + var retentionDays = configuration.GetValue("Analytics:RetentionDays", 365); + var intervalHours = configuration.GetValue("Analytics:CleanupIntervalHours", 24); + + while (!stoppingToken.IsCancellationRequested) + { + await RunCleanupAsync(retentionDays, stoppingToken); + + await Task.Delay(TimeSpan.FromHours(intervalHours), stoppingToken).ConfigureAwait(false); + } + } + + /// Exposed as public for testability. + public async Task RunCleanupAsync(int retentionDays, CancellationToken stoppingToken = default) + { + try + { + await using var scope = scopeFactory.CreateAsyncScope(); + var store = scope.ServiceProvider.GetRequiredService(); + + var threshold = DateTimeOffset.UtcNow.AddDays(-retentionDays); + var deleted = await store.DeleteRecordedBeforeAsync(threshold, stoppingToken); + + if (deleted > 0) + logger.LogInformation( + "Deleted {Count} analytics event(s) older than {Days} days", deleted, retentionDays); + } + catch (OperationCanceledException) + { + throw; + } + catch (Exception ex) + { + logger.LogError(ex, "Error during analytics event cleanup"); + } + } +} diff --git a/src/backend/OpenOnboarding.Infrastructure/Services/DatabaseAnalyticsEventStore.cs b/src/backend/OpenOnboarding.Infrastructure/Services/DatabaseAnalyticsEventStore.cs new file mode 100644 index 0000000..517d30e --- /dev/null +++ b/src/backend/OpenOnboarding.Infrastructure/Services/DatabaseAnalyticsEventStore.cs @@ -0,0 +1,67 @@ +using System.Text.Json; +using Microsoft.EntityFrameworkCore; +using OpenOnboarding.Application.Contracts; +using OpenOnboarding.Application.Interfaces; +using OpenOnboarding.Infrastructure.Persistence; + +namespace OpenOnboarding.Infrastructure.Services; + +public sealed class DatabaseAnalyticsEventStore(OnboardingDbContext db) : IAnalyticsEventStore +{ + private static readonly JsonSerializerOptions JsonOptions = new(JsonSerializerDefaults.Web); + + public async Task> GetSessionTrailAsync( + string sessionId, + CancellationToken cancellationToken = default) + { + var records = await db.AnalyticsEvents + .AsNoTracking() + .Where(x => x.SessionId == sessionId) + .OrderBy(x => x.OccurredAt) + .ThenBy(x => x.RecordedAt) + .ToListAsync(cancellationToken); + + return records.Select(record => new AnalyticsEvent + { + EventId = record.Id, + EventType = record.EventType, + JourneyId = record.JourneyId, + SessionId = record.SessionId, + StepId = record.StepId, + StepIndex = record.StepIndex, + Payload = DeserializePayload(record.PayloadJson), + OccurredAt = record.OccurredAt, + Source = record.Source + }).ToList(); + } + + public async Task DeleteRecordedBeforeAsync( + DateTimeOffset threshold, + CancellationToken cancellationToken = default) + { + var expired = await db.AnalyticsEvents + .Where(x => x.RecordedAt < threshold) + .ToListAsync(cancellationToken); + + if (expired.Count == 0) + return 0; + + db.AnalyticsEvents.RemoveRange(expired); + await db.SaveChangesAsync(cancellationToken); + return expired.Count; + } + + private static IReadOnlyDictionary DeserializePayload(string payloadJson) + { + try + { + return JsonSerializer.Deserialize>(payloadJson, JsonOptions) + ?? new Dictionary(); + } + catch (JsonException) + { + // A malformed payload must not make the whole trail unreadable. + return new Dictionary(); + } + } +} diff --git a/src/backend/OpenOnboarding.Infrastructure/Services/DatabaseAnalyticsProvider.cs b/src/backend/OpenOnboarding.Infrastructure/Services/DatabaseAnalyticsProvider.cs new file mode 100644 index 0000000..fea023c --- /dev/null +++ b/src/backend/OpenOnboarding.Infrastructure/Services/DatabaseAnalyticsProvider.cs @@ -0,0 +1,43 @@ +using System.Text.Json; +using Microsoft.Extensions.DependencyInjection; +using OpenOnboarding.Application.Contracts; +using OpenOnboarding.Application.Interfaces; +using OpenOnboarding.Domain.Entities; +using OpenOnboarding.Infrastructure.Persistence; + +namespace OpenOnboarding.Infrastructure.Services; + +/// +/// Durable analytics sink: writes every dispatched event to the database so the per-session trail +/// can be queried and exported. +/// +/// Resolves its own scope because it is a singleton sink invoked from background dispatch, while +/// the DbContext is scoped. +/// +/// +public sealed class DatabaseAnalyticsProvider(IServiceScopeFactory scopeFactory) : IAnalyticsProvider +{ + private static readonly JsonSerializerOptions JsonOptions = new(JsonSerializerDefaults.Web); + + public async Task TrackEventAsync(AnalyticsEvent @event, CancellationToken cancellationToken = default) + { + await using var scope = scopeFactory.CreateAsyncScope(); + var db = scope.ServiceProvider.GetRequiredService(); + + db.AnalyticsEvents.Add(new AnalyticsEventRecord + { + Id = @event.EventId, + EventType = @event.EventType, + JourneyId = @event.JourneyId, + SessionId = @event.SessionId, + StepId = @event.StepId, + StepIndex = @event.StepIndex, + PayloadJson = JsonSerializer.Serialize(@event.Payload, JsonOptions), + Source = @event.Source, + OccurredAt = @event.OccurredAt, + RecordedAt = DateTimeOffset.UtcNow + }); + + await db.SaveChangesAsync(cancellationToken); + } +} diff --git a/src/frontend/src/App.routing.test.tsx b/src/frontend/src/App.routing.test.tsx index 06ef9d1..7d6043d 100644 --- a/src/frontend/src/App.routing.test.tsx +++ b/src/frontend/src/App.routing.test.tsx @@ -95,11 +95,14 @@ describe('App routing and route guards', () => { render() await screen.findByText('Welcome aboard') + // Operator-only reads. /api/analytics/events is excluded deliberately: that is the + // applicant's own analytics ingest, scoped to their session token. const operatorPaths = [ '/api/workflow/sessions?', '/api/webhooks', '/api/flows/', - '/api/analytics/', + '/api/analytics/flows', + '/api/analytics/sessions', '/api/customers', '/api/auth/me', ] diff --git a/src/frontend/src/analytics/httpAnalyticsSink.test.ts b/src/frontend/src/analytics/httpAnalyticsSink.test.ts new file mode 100644 index 0000000..a44de2b --- /dev/null +++ b/src/frontend/src/analytics/httpAnalyticsSink.test.ts @@ -0,0 +1,159 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createHttpAnalyticsSink } from './httpAnalyticsSink' +import type { JourneyAnalyticsEvent } from './JourneyAnalyticsContext' +import { clearApplicantToken, setApplicantToken } from '../onboarding/api/applicant-session' + +function event(overrides: Partial = {}): JourneyAnalyticsEvent { + return { + eventId: crypto.randomUUID(), + eventType: 'step_view', + journeyId: 'journey-1', + sessionId: 'session-1', + stepId: 'node-1', + stepIndex: 0, + payload: {}, + occurredAt: new Date().toISOString(), + ...overrides, + } as JourneyAnalyticsEvent +} + +describe('http analytics sink', () => { + let fetchImpl: ReturnType + + beforeEach(() => { + vi.useFakeTimers() + fetchImpl = vi.fn().mockResolvedValue(new Response('{}', { status: 202 })) + setApplicantToken('token-abc') + }) + + afterEach(() => { + vi.useRealTimers() + clearApplicantToken() + }) + + function bodyOf(call: number): { events: Array> } { + return JSON.parse(String(fetchImpl.mock.calls[call][1].body)) + } + + it('does not send a request per event', () => { + const sink = createHttpAnalyticsSink({ fetchImpl, batchSize: 5 }) + + sink.track(event()) + sink.track(event()) + + expect(fetchImpl).not.toHaveBeenCalled() + }) + + it('sends once the batch is full', async () => { + const sink = createHttpAnalyticsSink({ fetchImpl, batchSize: 3 }) + + sink.track(event({ eventType: 'step_view' })) + sink.track(event({ eventType: 'interaction' })) + sink.track(event({ eventType: 'navigation_next' })) + await vi.runAllTimersAsync() + + expect(fetchImpl).toHaveBeenCalledTimes(1) + expect(bodyOf(0).events).toHaveLength(3) + }) + + it('sends a partial batch once the interval elapses', async () => { + const sink = createHttpAnalyticsSink({ fetchImpl, batchSize: 10, flushIntervalMs: 1000 }) + + sink.track(event()) + expect(fetchImpl).not.toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(1000) + + expect(fetchImpl).toHaveBeenCalledTimes(1) + expect(bodyOf(0).events).toHaveLength(1) + }) + + it('authenticates with the applicant session token', async () => { + const sink = createHttpAnalyticsSink({ fetchImpl, batchSize: 1 }) + + sink.track(event()) + await vi.runAllTimersAsync() + + const headers = fetchImpl.mock.calls[0][1].headers as Record + expect(headers.Authorization).toBe('Bearer token-abc') + expect(headers['X-Api-Key']).toBeUndefined() + }) + + it('drops events that have no session, since they cannot be attributed', async () => { + const sink = createHttpAnalyticsSink({ fetchImpl, batchSize: 2 }) + + sink.track(event({ sessionId: null })) + sink.track(event({ sessionId: null })) + await vi.runAllTimersAsync() + + expect(fetchImpl).not.toHaveBeenCalled() + }) + + it('never throws when the endpoint fails', async () => { + fetchImpl.mockRejectedValue(new Error('network down')) + const sink = createHttpAnalyticsSink({ fetchImpl, batchSize: 1 }) + + expect(() => sink.track(event())).not.toThrow() + await expect(vi.runAllTimersAsync()).resolves.not.toThrow() + await expect(sink.flush()).resolves.toBeUndefined() + }) + + it('keeps accepting events after a failed flush', async () => { + fetchImpl.mockRejectedValueOnce(new Error('network down')) + const sink = createHttpAnalyticsSink({ fetchImpl, batchSize: 1 }) + + sink.track(event({ eventType: 'step_view' })) + await vi.runAllTimersAsync() + + sink.track(event({ eventType: 'journey_complete' })) + await vi.runAllTimersAsync() + + expect(fetchImpl).toHaveBeenCalledTimes(2) + expect(bodyOf(1).events[0].eventType).toBe('journey_complete') + }) + + it('does not resend events from a failed batch', async () => { + // Retrying forever would grow the queue without bound on a long-broken endpoint. + fetchImpl.mockRejectedValueOnce(new Error('network down')) + const sink = createHttpAnalyticsSink({ fetchImpl, batchSize: 1 }) + + sink.track(event({ eventType: 'validation_error' })) + await vi.runAllTimersAsync() + await sink.flush() + + expect(fetchImpl).toHaveBeenCalledTimes(1) + }) + + it('flush sends what is queued and is a no-op when empty', async () => { + const sink = createHttpAnalyticsSink({ fetchImpl, batchSize: 50 }) + + await sink.flush() + expect(fetchImpl).not.toHaveBeenCalled() + + sink.track(event()) + await sink.flush() + + expect(fetchImpl).toHaveBeenCalledTimes(1) + }) + + it('caps a batch at the server limit', async () => { + const sink = createHttpAnalyticsSink({ fetchImpl, batchSize: 500 }) + + for (let i = 0; i < 120; i++) sink.track(event()) + await vi.runAllTimersAsync() + + expect(bodyOf(0).events.length).toBeLessThanOrEqual(100) + }) + + it('close flushes and stops the timer', async () => { + const sink = createHttpAnalyticsSink({ fetchImpl, batchSize: 50, flushIntervalMs: 1000 }) + + sink.track(event()) + await sink.close() + + expect(fetchImpl).toHaveBeenCalledTimes(1) + + await vi.advanceTimersByTimeAsync(5000) + expect(fetchImpl).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/frontend/src/analytics/httpAnalyticsSink.ts b/src/frontend/src/analytics/httpAnalyticsSink.ts new file mode 100644 index 0000000..8b3c88a --- /dev/null +++ b/src/frontend/src/analytics/httpAnalyticsSink.ts @@ -0,0 +1,119 @@ +import type { JourneyAnalyticsEvent, JourneyAnalyticsSink } from './JourneyAnalyticsContext' +import { applicantAuthHeaders } from '../onboarding/api/applicant-session' + +/** + * Sends journey events to the API so they join the durable trail. + * + * Events are batched: a journey raises one on every step view and interaction, and a request each + * would put the analytics pipeline in the critical path of the form the applicant is filling in. + * + * Delivery is strictly best-effort. A failed flush is dropped rather than retried forever, and no + * failure is ever surfaced — analytics must not be able to break an onboarding journey. + */ +export interface HttpAnalyticsSinkOptions { + /** Endpoint to post batches to. */ + url?: string + /** Flush once this many events are queued. */ + batchSize?: number + /** Flush this long after the first event in a batch, even if the batch is not full. */ + flushIntervalMs?: number + /** Injectable for tests. */ + fetchImpl?: typeof fetch +} + +const DEFAULT_URL = '/api/analytics/events' +const DEFAULT_BATCH_SIZE = 20 +const DEFAULT_FLUSH_INTERVAL_MS = 5000 + +/** Matches the server's per-batch cap; a larger batch would be rejected wholesale. */ +const MAX_BATCH_SIZE = 100 + +export interface HttpAnalyticsSink extends JourneyAnalyticsSink { + /** Sends anything queued now. Safe to call when the queue is empty. */ + flush(): Promise + /** Stops the timer and flushes. */ + close(): Promise +} + +export function createHttpAnalyticsSink(options: HttpAnalyticsSinkOptions = {}): HttpAnalyticsSink { + const url = options.url ?? DEFAULT_URL + const batchSize = Math.min(options.batchSize ?? DEFAULT_BATCH_SIZE, MAX_BATCH_SIZE) + const flushIntervalMs = options.flushIntervalMs ?? DEFAULT_FLUSH_INTERVAL_MS + const fetchImpl = options.fetchImpl ?? ((...args: Parameters) => fetch(...args)) + + let queue: JourneyAnalyticsEvent[] = [] + let timer: ReturnType | null = null + + function clearTimer() { + if (timer !== null) { + clearTimeout(timer) + timer = null + } + } + + async function flush(): Promise { + clearTimer() + if (queue.length === 0) return + + // Take the queue before awaiting so events raised during the request are not lost or resent. + const batch = queue.slice(0, MAX_BATCH_SIZE) + queue = queue.slice(MAX_BATCH_SIZE) + + const events = batch + // An event with no session cannot be attributed, and the server would reject the batch. + .filter((event) => event.sessionId !== null) + .map((event) => ({ + eventId: event.eventId, + eventType: event.eventType, + journeyId: event.journeyId, + sessionId: event.sessionId, + stepId: event.stepId, + stepIndex: event.stepIndex, + payload: event.payload, + occurredAt: event.occurredAt, + })) + + if (events.length === 0) return + + try { + await fetchImpl(url, { + method: 'POST', + headers: { 'Content-Type': 'application/json', ...applicantAuthHeaders() }, + body: JSON.stringify({ events }), + keepalive: true, + }) + } catch { + // Dropped. Retrying risks an unbounded queue on a long-broken endpoint, and a lost + // analytics event matters far less than a stalled application form. + } + + if (queue.length > 0) scheduleFlush() + } + + function scheduleFlush() { + if (timer !== null) return + timer = setTimeout(() => void flush(), flushIntervalMs) + } + + return { + name: 'http', + + track(event: JourneyAnalyticsEvent): void { + queue.push(event) + + if (queue.length >= batchSize) { + void flush() + return + } + + scheduleFlush() + }, + + flush, + + async close(): Promise { + clearTimer() + await flush() + }, + } +} diff --git a/src/frontend/src/onboarding/ApplicantJourneyPage.tsx b/src/frontend/src/onboarding/ApplicantJourneyPage.tsx index a959a72..4a8b831 100644 --- a/src/frontend/src/onboarding/ApplicantJourneyPage.tsx +++ b/src/frontend/src/onboarding/ApplicantJourneyPage.tsx @@ -1,6 +1,7 @@ -import { useEffect, useState } from 'react' +import { useEffect, useMemo, useState } from 'react' import { JourneyAnalyticsProvider } from '../analytics/JourneyAnalyticsContext' import { consoleAnalyticsSink } from '../analytics/consoleAnalyticsSink' +import { createHttpAnalyticsSink } from '../analytics/httpAnalyticsSink' import { StepRenderer } from './components/StepRenderer' import { useOnboarding } from './hooks/useOnboarding' import { defaultFlowId } from '../journeys' @@ -20,6 +21,23 @@ export function ApplicantJourneyPage({ search }: { search: string }) { const flowId = new URLSearchParams(search).get('flowId') ?? defaultFlowId + // One sink for the lifetime of the page so its batch queue survives re-renders. The console sink + // stays alongside it for local debugging. + const httpSink = useMemo(() => createHttpAnalyticsSink(), []) + const analyticsSinks = useMemo(() => [consoleAnalyticsSink, httpSink], [httpSink]) + + useEffect(() => { + // A journey often ends by the applicant closing the tab, so flush what is queued before the + // page goes away rather than losing the tail of every session. + const flushOnHide = () => { void httpSink.flush() } + window.addEventListener('pagehide', flushOnHide) + + return () => { + window.removeEventListener('pagehide', flushOnHide) + void httpSink.close() + } + }, [httpSink]) + useEffect(() => { startSession({ flowId }) .then((result) => { @@ -34,7 +52,7 @@ export function ApplicantJourneyPage({ search }: { search: string }) {
From 883c4a8c20a497920a4cbf50ca3424265d6a97ae Mon Sep 17 00:00:00 2001 From: MaximumTrainer Date: Sun, 13 Sep 2026 08:47:26 +0100 Subject: [PATCH 7/8] chore: add versioned git hooks to enforce quality before review Quality checks lived only in CI, so a broken build or a leaked credential was found after push at the earliest. These catch the cheap cases locally. Hooks live in .githooks/ and are enabled per clone through core.hooksPath, so a change to them is reviewed like any other change - unlike scripts copied into .git/hooks. `npm install` in src/frontend runs the installer, and scripts/install-git-hooks.{sh,ps1} do it explicitly. - pre-commit: scans the staged diff for PEM private keys, VITE_* secrets and non-placeholder credentials in configuration, then lints or builds only the areas the commit touches. The VITE_* rule encodes the #90 lesson: Vite inlines those into the browser bundle as literal strings. - commit-msg: requires a Conventional Commits subject, the format this history already follows and what a release tool needs to derive a version from the log. - pre-push: runs the backend and frontend suites and refuses a direct push to main. Scope is deliberate. Only suites that need no external service run in the hooks; Pact provider verification and Playwright need PostgreSQL and a browser and stay in CI. A hook that passes only on a correctly provisioned machine is one people learn to bypass, and a bypassed hook enforces nothing. Every hook honours SKIP_HOOKS=1, which is preferable to --no-verify because it is visible in the command that used it. Also corrects getting-started.md, which still told developers to put an Operator-equivalent API key in .env.local - the practice the pre-commit secret scan now blocks - and documents the current routes. Co-Authored-By: Claude Opus 5 (1M context) --- .githooks/README.md | 56 ++++++++++++++++++++++++++ .githooks/_lib.sh | 68 +++++++++++++++++++++++++++++++ .githooks/commit-msg | 53 ++++++++++++++++++++++++ .githooks/pre-commit | 76 +++++++++++++++++++++++++++++++++++ .githooks/pre-push | 50 +++++++++++++++++++++++ getting-started.md | 38 +++++++++++++++--- scripts/install-git-hooks.ps1 | 32 +++++++++++++++ scripts/install-git-hooks.sh | 30 ++++++++++++++ src/frontend/package.json | 3 +- 9 files changed, 400 insertions(+), 6 deletions(-) create mode 100644 .githooks/README.md create mode 100755 .githooks/_lib.sh create mode 100755 .githooks/commit-msg create mode 100755 .githooks/pre-commit create mode 100755 .githooks/pre-push create mode 100644 scripts/install-git-hooks.ps1 create mode 100755 scripts/install-git-hooks.sh diff --git a/.githooks/README.md b/.githooks/README.md new file mode 100644 index 0000000..9a3e865 --- /dev/null +++ b/.githooks/README.md @@ -0,0 +1,56 @@ +# Git hooks + +Versioned hooks, enabled per clone by pointing `core.hooksPath` here: + +```bash +sh scripts/install-git-hooks.sh # macOS / Linux / Git Bash +pwsh scripts/install-git-hooks.ps1 # Windows PowerShell +``` + +`npm ci` or `npm install` in `src/frontend` also runs the installer, so a frontend setup enables +them without a separate step. + +## What runs when + +| Hook | Checks | Typical cost | +|------|--------|--------------| +| `pre-commit` | Secret scan of the staged diff; `eslint` if frontend files changed; `dotnet build` if backend files changed | seconds | +| `commit-msg` | Conventional Commits subject line | instant | +| `pre-push` | `dotnet test` (Application.Tests), `npm test`, `npm run build`; refuses a direct push to `main` | ~1 minute | + +Work is scoped to what the commit actually touches, so a docs-only change runs neither toolchain. + +## What is deliberately not here + +Suites needing PostgreSQL, RabbitMQ or a browser — Pact provider verification and Playwright — stay +in CI. A hook that only passes on a machine with the right services running is a hook people learn +to bypass, and a bypassed hook enforces nothing. + +CI remains the authority. These hooks exist to catch the cheap mistakes before review, not to +replace it. + +## Bypassing + +```bash +SKIP_HOOKS=1 git commit ... +SKIP_HOOKS=1 git push ... +``` + +Preferred over `--no-verify` because it is visible in the command that used it, and it works for +`commit-msg`, which `--no-verify` also skips. + +`ALLOW_PUSH_TO_MAIN=1` overrides the `main` branch guard specifically. + +## The secret scan + +Only lines the commit *adds* are scanned, for: + +- PEM private key blocks. +- `VITE_*` variables holding a key, secret, token or password. Vite inlines these into the browser + bundle as literal strings, so a secret named this way is readable by every visitor — the defect + fixed in #90. +- Non-empty `SigningKey`, `SpPrivateKey`, `ClientSecret`, `Secret`, `Password` or `ApiKey` values in + configuration. Recognisable local placeholders (`dev-`, `test-`, `example`, `change-in-production`) + are allowed so committed development defaults stay editable. + +It is a backstop for the obvious cases, not a substitute for secret scanning on the remote. diff --git a/.githooks/_lib.sh b/.githooks/_lib.sh new file mode 100755 index 0000000..8c17656 --- /dev/null +++ b/.githooks/_lib.sh @@ -0,0 +1,68 @@ +#!/bin/sh +# Shared helpers for the repository's git hooks. +# +# Sourced by every hook in this directory. POSIX sh: Git for Windows runs hooks under its bundled +# sh, so bashisms are avoided. + +REPO_ROOT=$(git rev-parse --show-toplevel) +export REPO_ROOT + +# Colour only when attached to a terminal, so hook output stays readable when piped or logged. +if [ -t 1 ]; then + C_RED=$(printf '\033[31m') + C_GREEN=$(printf '\033[32m') + C_YELLOW=$(printf '\033[33m') + C_DIM=$(printf '\033[2m') + C_OFF=$(printf '\033[0m') +else + C_RED='' C_GREEN='' C_YELLOW='' C_DIM='' C_OFF='' +fi + +hook_info() { printf '%s\n' "${C_DIM}[hook] $1${C_OFF}"; } +hook_ok() { printf '%s\n' "${C_GREEN}[hook] $1${C_OFF}"; } +hook_warn() { printf '%s\n' "${C_YELLOW}[hook] $1${C_OFF}"; } +hook_error() { printf '%s\n' "${C_RED}[hook] $1${C_OFF}" >&2; } + +# Every hook honours SKIP_HOOKS=1, which is friendlier than --no-verify because it is visible in +# the command that used it. +hooks_skipped() { + if [ "${SKIP_HOOKS:-0}" = "1" ]; then + hook_warn "SKIP_HOOKS=1 set - skipping $1." + return 0 + fi + return 1 +} + +# Files staged for commit, added/copied/modified/renamed only: a path being deleted must not +# trigger a build of the area it used to live in. +staged_files() { + git diff --cached --name-only --diff-filter=ACMR +} + +# True when any of the given paths match the supplied prefix pattern. +paths_touch() { + pattern=$1 + shift + printf '%s\n' "$@" | grep -Eq "$pattern" +} + +require_command() { + if ! command -v "$1" >/dev/null 2>&1; then + hook_error "'$1' is not on PATH; cannot run $2." + hook_error "Install it, or bypass this once with SKIP_HOOKS=1." + return 1 + fi + return 0 +} + +# Runs a command from the repository root, printing it first and failing loudly. +run_step() { + label=$1 + shift + hook_info "$label" + if ! ( cd "$REPO_ROOT" && "$@" ); then + hook_error "$label FAILED" + return 1 + fi + return 0 +} diff --git a/.githooks/commit-msg b/.githooks/commit-msg new file mode 100755 index 0000000..5b79cbb --- /dev/null +++ b/.githooks/commit-msg @@ -0,0 +1,53 @@ +#!/bin/sh +# Enforces Conventional Commits, the format this repository's history already follows. +# +# The format is not decoration: `feat:` / `fix:` / `BREAKING CHANGE` are what let a release tool +# derive a version and changelog from the log. +set -e + +. "$(dirname "$0")/_lib.sh" + +hooks_skipped "commit-msg" && exit 0 + +MSG_FILE=$1 +SUBJECT=$(head -n 1 "$MSG_FILE") + +# Git's own generated messages and rebase helpers are not authored commits. +case "$SUBJECT" in + "Merge "*|"Revert "*|"fixup!"*|"squash!"*|"amend!"*) + exit 0 + ;; +esac + +# Comment-only content means an aborted commit; git handles that itself. +if [ -z "$(printf '%s' "$SUBJECT" | tr -d '[:space:]')" ]; then + exit 0 +fi + +TYPES='build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test' +PATTERN="^(${TYPES})(\([a-z0-9._/-]+\))?!?: .+" + +if ! printf '%s' "$SUBJECT" | grep -Eq "$PATTERN"; then + hook_error "Commit message does not follow Conventional Commits." + hook_error "" + hook_error " got: $SUBJECT" + hook_error " expected: [(scope)][!]: " + hook_error " types: ${TYPES}" + hook_error "" + hook_error " e.g. feat(saml): support encrypted assertions" + hook_error " fix: propagate the cancellation token through webhook retries" + hook_error "" + hook_error "Bypass once with: SKIP_HOOKS=1 git commit ..." + exit 1 +fi + +# A subject that needs more than 100 characters belongs in the body. +SUBJECT_LENGTH=$(printf '%s' "$SUBJECT" | wc -c | tr -d ' ') +if [ "$SUBJECT_LENGTH" -gt 100 ]; then + hook_error "Commit subject is ${SUBJECT_LENGTH} characters; keep it under 100 and use the body." + exit 1 +fi + +if [ "$SUBJECT_LENGTH" -gt 72 ]; then + hook_warn "Commit subject is ${SUBJECT_LENGTH} characters; 72 or fewer reads better in git log." +fi diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 0000000..36295db --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,76 @@ +#!/bin/sh +# Fast checks on what is actually staged. +# +# Kept to seconds: a slow pre-commit gets bypassed, and a bypassed hook enforces nothing. The full +# test suites run at pre-push, and CI remains the authority. +set -e + +. "$(dirname "$0")/_lib.sh" + +hooks_skipped "pre-commit" && exit 0 + +FILES=$(staged_files) +if [ -z "$FILES" ]; then + exit 0 +fi + +FAILED=0 + +# ── 1. Secrets ──────────────────────────────────────────────────────────────── +# Scans only the lines this commit adds. A credential is far cheaper to catch here than to rotate +# after it reaches a branch. +ADDED=$(git diff --cached --unified=0 --diff-filter=ACMR | grep '^+' | grep -v '^+++' || true) + +if printf '%s\n' "$ADDED" | grep -Eq -- '-----BEGIN [A-Z ]*PRIVATE KEY-----'; then + hook_error "A PEM private key block is being committed." + FAILED=1 +fi + +# Vite inlines every VITE_* variable into the published bundle as a literal string, so a secret +# named this way is readable by every visitor. This is the defect #90 fixed. +if printf '%s\n' "$ADDED" | grep -Eiq '^\+[^#]*VITE_[A-Z0-9_]*(KEY|SECRET|TOKEN|PASSWORD)[A-Z0-9_]*[[:space:]]*[=:][[:space:]]*[^[:space:]"'"'"']'; then + hook_error "A VITE_* secret is being committed with a value." + hook_error "Vite inlines these into the browser bundle - every visitor can read them." + FAILED=1 +fi + +# Non-empty secret values in configuration. Obvious local placeholders are allowed so the committed +# development defaults stay editable. +SECRET_ASSIGNMENTS=$(printf '%s\n' "$ADDED" \ + | grep -Ei '"(SigningKey|SpPrivateKey|ClientSecret|Secret|Password|ApiKey)"[[:space:]]*:[[:space:]]*"[^"]+"' \ + | grep -Eiv '(change-in-production|^\+[^"]*"(dev|test|example|sample|placeholder|postgres)[^"]*"|"(dev|test|example|sample|placeholder|postgres)-)' \ + || true) + +if [ -n "$SECRET_ASSIGNMENTS" ]; then + hook_error "A non-placeholder secret value is being committed in configuration:" + printf '%s\n' "$SECRET_ASSIGNMENTS" >&2 + hook_error "Supply it as an environment variable or deployment secret instead." + FAILED=1 +fi + +if [ "$FAILED" -ne 0 ]; then + hook_error "" + hook_error "If this is a false positive, bypass once with: SKIP_HOOKS=1 git commit ..." + exit 1 +fi + +# ── 2. Frontend ─────────────────────────────────────────────────────────────── +if paths_touch '^src/frontend/.*\.(ts|tsx|js|jsx)$' $FILES; then + require_command npm "the frontend lint step" || exit 1 + run_step "frontend: eslint" sh -c 'cd src/frontend && npm run --silent lint' || FAILED=1 +fi + +# ── 3. Backend ──────────────────────────────────────────────────────────────── +# Compile only. Tests are at pre-push, where the wait is expected. +if paths_touch '^src/backend/.*\.cs$|^src/backend/.*\.csproj$|^src/backend/.*\.slnx$' $FILES; then + require_command dotnet "the backend build step" || exit 1 + run_step "backend: build" dotnet build src/backend/OpenOnboarding.slnx --nologo -v quiet || FAILED=1 +fi + +if [ "$FAILED" -ne 0 ]; then + hook_error "" + hook_error "Commit aborted. Fix the above, or bypass once with: SKIP_HOOKS=1 git commit ..." + exit 1 +fi + +hook_ok "pre-commit checks passed." diff --git a/.githooks/pre-push b/.githooks/pre-push new file mode 100755 index 0000000..650fe41 --- /dev/null +++ b/.githooks/pre-push @@ -0,0 +1,50 @@ +#!/bin/sh +# The real gate: the test suites that run without external dependencies. +# +# Scope is deliberate. These run against an in-memory database and mocked transports, so they pass +# on any machine with the two SDKs installed. The suites that need PostgreSQL, a broker or a +# browser (Pact provider verification, Playwright) stay in CI, where those services exist. +set -e + +. "$(dirname "$0")/_lib.sh" + +hooks_skipped "pre-push" && exit 0 + +# ── Protect the default branch ──────────────────────────────────────────────── +# Changes reach main through a reviewed PR; CI is wired to run on pull requests. +REMOTE_REF=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo '') +if [ "$REMOTE_REF" = "main" ] && [ "${ALLOW_PUSH_TO_MAIN:-0}" != "1" ]; then + hook_error "Refusing to push directly to main." + hook_error "Branch and open a pull request, or set ALLOW_PUSH_TO_MAIN=1 if you mean it." + exit 1 +fi + +FAILED=0 + +# ── Backend ─────────────────────────────────────────────────────────────────── +if [ -d "$REPO_ROOT/src/backend" ]; then + require_command dotnet "the backend test step" || exit 1 + run_step "backend: dotnet test" \ + dotnet test src/backend/OpenOnboarding.Application.Tests/OpenOnboarding.Application.Tests.csproj \ + --nologo -v quiet || FAILED=1 +fi + +# ── Frontend ────────────────────────────────────────────────────────────────── +if [ -d "$REPO_ROOT/src/frontend" ]; then + require_command npm "the frontend test step" || exit 1 + + if [ ! -d "$REPO_ROOT/src/frontend/node_modules" ]; then + hook_warn "src/frontend/node_modules is missing; skipping frontend checks. Run npm ci." + else + run_step "frontend: vitest" sh -c 'cd src/frontend && npm test --silent' || FAILED=1 + run_step "frontend: build" sh -c 'cd src/frontend && npm run --silent build' || FAILED=1 + fi +fi + +if [ "$FAILED" -ne 0 ]; then + hook_error "" + hook_error "Push aborted. Fix the above, or bypass once with: SKIP_HOOKS=1 git push ..." + exit 1 +fi + +hook_ok "pre-push checks passed." diff --git a/getting-started.md b/getting-started.md index 152386a..de99a20 100644 --- a/getting-started.md +++ b/getting-started.md @@ -49,14 +49,20 @@ npm install npm run dev ``` -Recommended `.env.local` values: +Only one value is needed: - `VITE_API_BASE_URL=http://localhost:5072` -- `VITE_API_KEY=dev-api-key-change-in-production` + +**Do not put an API key in `.env.local`.** Vite inlines every `VITE_*` variable into the built +bundle as a literal string, and `Authentication:ApiKey` maps to a full Operator principal. The app +needs no credential of its own: the onboarding journey starts anonymously and is handed a token +scoped to that session, and operator screens use the SSO cookie. See +[API credentials](./docs/runbook.md#api-credentials). Frontend dev server: `http://localhost:5173` Available routes: -- `/` — developer dashboard (journey selector, step renderer, analytics, session history) +- `/` — the applicant onboarding journey. `/?flowId=` selects a specific journey. +- `/admin` — operator console: flow authoring, version history, analytics, sessions, webhook deliveries (requires Operator SSO) - `/admin/journey-builder` — Visual Journey Builder admin UI (requires Operator SSO — see [`user-guide.md`](./user-guide.md) section 8 for usage) ## 6. Run validation commands @@ -80,12 +86,34 @@ dotnet build src/backend/OpenOnboarding.slnx --no-restore -c Release dotnet test src/backend/OpenOnboarding.slnx -c Release ``` -## 7. Contribution workflow +## 7. Install the git hooks + +```bash +sh scripts/install-git-hooks.sh # macOS / Linux / Git Bash +pwsh scripts/install-git-hooks.ps1 # Windows PowerShell +``` + +`npm install` in `src/frontend` runs this for you, so step 5 already covers it. + +The hooks live in [`.githooks/`](./.githooks/README.md) and are enabled per clone via +`core.hooksPath`: + +- **pre-commit** — scans the staged diff for secrets, then lints or builds only the areas you + touched. +- **commit-msg** — requires a Conventional Commits subject (`feat:`, `fix:`, `docs:`, …). +- **pre-push** — runs the backend and frontend test suites, and refuses a direct push to `main`. + +Bypass once with `SKIP_HOOKS=1` when you need to. CI remains the authority; the hooks exist to +catch the cheap mistakes before review. + +## 8. Contribution workflow 1. Create a feature/fix branch. 2. Add or update tests first for all code changes (TDD: Red → Green → Refactor). 3. Keep API/controller changes thin; place workflow logic in application/infrastructure services. -4. Run backend + frontend validation commands before pushing. +4. Run backend + frontend validation commands before pushing. The pre-push hook runs the + dependency-free suites for you; the Pact and Playwright suites need PostgreSQL and a browser and + run in CI. 5. Open a PR and ensure CI passes. 6. Do not merge behavior changes unless corresponding automated tests are included and passing. 7. For documentation-only changes, validate command examples against a local setup and ensure CI references remain accurate. diff --git a/scripts/install-git-hooks.ps1 b/scripts/install-git-hooks.ps1 new file mode 100644 index 0000000..bc40bd1 --- /dev/null +++ b/scripts/install-git-hooks.ps1 @@ -0,0 +1,32 @@ +<# +.SYNOPSIS + Points this clone's git at the versioned hooks in .githooks/. + +.DESCRIPTION + core.hooksPath is per-clone and cannot be committed, so every clone runs this once. + Windows equivalent of scripts/install-git-hooks.sh. The hooks themselves are POSIX sh and run + under the sh that ships with Git for Windows. +#> + +$ErrorActionPreference = 'Stop' + +$repoRoot = (git rev-parse --show-toplevel).Trim() +Set-Location $repoRoot + +git config core.hooksPath .githooks + +# Git ignores a hook without the executable bit. On Windows that bit lives in the index. +Get-ChildItem -Path (Join-Path $repoRoot '.githooks') -File | + Where-Object { $_.Extension -ne '.md' } | + ForEach-Object { + $relative = ".githooks/$($_.Name)" + git update-index --chmod=+x $relative 2>$null + } + +Write-Host 'Git hooks installed: core.hooksPath -> .githooks' +Write-Host '' +Write-Host ' pre-commit secret scan, lint and build for the areas you touched' +Write-Host ' commit-msg Conventional Commits' +Write-Host ' pre-push backend and frontend test suites' +Write-Host '' +Write-Host 'Bypass any of them once with SKIP_HOOKS=1, e.g. SKIP_HOOKS=1 git commit ...' diff --git a/scripts/install-git-hooks.sh b/scripts/install-git-hooks.sh new file mode 100755 index 0000000..042ef09 --- /dev/null +++ b/scripts/install-git-hooks.sh @@ -0,0 +1,30 @@ +#!/bin/sh +# Points this clone's git at the versioned hooks in .githooks/. +# +# core.hooksPath is per-clone and cannot be committed, so every clone runs this once. Versioning +# the hooks themselves means a change to them is reviewed like any other change, which copying +# scripts into .git/hooks would not give. +set -e + +REPO_ROOT=$(git rev-parse --show-toplevel) +cd "$REPO_ROOT" + +git config core.hooksPath .githooks + +# Git ignores a hook that is not executable. On Windows the bit is carried in the index, so set it +# there too rather than only on disk. +for hook in .githooks/*; do + case "$hook" in + *.md) continue ;; + esac + chmod +x "$hook" 2>/dev/null || true + git update-index --chmod=+x "$hook" 2>/dev/null || true +done + +echo "Git hooks installed: core.hooksPath -> .githooks" +echo +echo " pre-commit secret scan, lint and build for the areas you touched" +echo " commit-msg Conventional Commits" +echo " pre-push backend and frontend test suites" +echo +echo "Bypass any of them once with SKIP_HOOKS=1, e.g. SKIP_HOOKS=1 git commit ..." diff --git a/src/frontend/package.json b/src/frontend/package.json index d8c6da4..8126f72 100644 --- a/src/frontend/package.json +++ b/src/frontend/package.json @@ -10,7 +10,8 @@ "preview": "vite preview", "test": "vitest run", "test:pact": "vitest run src/pact", - "test:e2e": "playwright test" + "test:e2e": "playwright test", + "prepare": "cd ../.. && sh scripts/install-git-hooks.sh || true" }, "dependencies": { "@radix-ui/react-label": "^2.1.8", From 71bdd961404f68d1f456a8ae98689b7f640b6123 Mon Sep 17 00:00:00 2001 From: MaximumTrainer Date: Sun, 13 Sep 2026 09:00:40 +0100 Subject: [PATCH 8/8] test: close acceptance-criteria gaps left in the gap-review work An audit of #90, #92, #93 and #95 against their acceptance criteria found four claims that were implemented but never actually asserted. - #95 required the analytics ingest endpoint to be rate limited. The policy was wired but nothing proved it, and no test in the repository exercised a real limiter: the Testing environment swaps every one for a no-op, so the existing RateLimitingTests only assert that no-op. Give TestWebAppFactory an environment parameter and assert a genuine 429 plus its Retry-After header. - #93 required a startup warning when a non-Development environment runs the single-instance emitter. Only its registration was covered. The failure it guards is silent at runtime, so the warning is the operator's only signal: assert it fires for Production and Staging, names the environment and the fix, and stays quiet for Development and Testing. - #90 required operator views to depend on the AdminSession cookie rather than an API key. Cover the case directly: every operator endpoint rejects a request carrying no credential, and still serves one that does. Backend suite 287 -> 300. Co-Authored-By: Claude Opus 5 (1M context) --- .../AnalyticsRateLimitTests.cs | 116 ++++++++++++++++++ .../ApplicantSessionTokenTests.cs | 33 +++++ ...InMemorySessionEventEmitterWarningTests.cs | 85 +++++++++++++ .../TestWebAppFactory.cs | 9 +- 4 files changed, 241 insertions(+), 2 deletions(-) create mode 100644 src/backend/OpenOnboarding.Application.Tests/AnalyticsRateLimitTests.cs create mode 100644 src/backend/OpenOnboarding.Application.Tests/InMemorySessionEventEmitterWarningTests.cs diff --git a/src/backend/OpenOnboarding.Application.Tests/AnalyticsRateLimitTests.cs b/src/backend/OpenOnboarding.Application.Tests/AnalyticsRateLimitTests.cs new file mode 100644 index 0000000..b8a20f8 --- /dev/null +++ b/src/backend/OpenOnboarding.Application.Tests/AnalyticsRateLimitTests.cs @@ -0,0 +1,116 @@ +using System.Net; +using System.Net.Http.Headers; +using System.Net.Http.Json; +using Microsoft.AspNetCore.Mvc.Testing; + +namespace OpenOnboarding.Application.Tests; + +/// +/// Analytics ingest is the one endpoint a browser calls in a loop, so an unbounded one would let a +/// single page fill the event table. These run in the Development environment because the Testing +/// environment deliberately swaps every limiter for a no-op. +/// +public sealed class AnalyticsRateLimitTests +{ + private const int Limit = 3; + + private sealed record StartResponse(Guid SessionId, string? ApplicantToken); + private sealed record FlowStub(Guid Id); + + private static WebApplicationFactory CreateRateLimitedFactory() + => TestWebAppFactory.Create( + configurationOverrides: new Dictionary + { + ["RateLimiting:AnalyticsIngestPerMinute"] = Limit.ToString(), + // Keep the other limiters out of the way of the fixture setup. + ["RateLimiting:SessionStartPerMinute"] = "1000", + ["RateLimiting:GeneralPerMinute"] = "1000" + }, + environment: "Development"); + + private static async Task CreateFlowAsync(HttpClient operatorClient) + { + var payload = new + { + name = $"Rate limit flow {Guid.NewGuid()}", + nodes = new[] + { + new { key = "start", type = "Form", title = "Start", isStartNode = true, jsonContent = "{\"fields\":[]}" } + }, + connections = Array.Empty() + }; + + var response = await operatorClient.PostAsJsonAsync("/api/flows", payload); + response.EnsureSuccessStatusCode(); + return (await response.Content.ReadFromJsonAsync())!.Id; + } + + private static HttpRequestMessage IngestRequest(Guid sessionId, Guid flowId, string token) + { + var request = new HttpRequestMessage(HttpMethod.Post, "/api/analytics/events"); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + request.Content = JsonContent.Create(new + { + events = new[] + { + new + { + eventType = "step_view", + journeyId = flowId.ToString(), + sessionId = sessionId.ToString() + } + } + }); + return request; + } + + [Fact] + public async Task AnalyticsIngest_BeyondTheConfiguredLimit_Returns429() + { + using var factory = CreateRateLimitedFactory(); + + using var operatorClient = factory.CreateClient(); + operatorClient.DefaultRequestHeaders.Add("X-Api-Key", "test-api-key"); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = factory.CreateClient(); + var startResponse = await browser.PostAsJsonAsync("/api/workflow/sessions/start", new { flowId }); + startResponse.EnsureSuccessStatusCode(); + var start = (await startResponse.Content.ReadFromJsonAsync())!; + + var statuses = new List(); + for (var i = 0; i < Limit + 2; i++) + { + var response = await browser.SendAsync(IngestRequest(start.SessionId, flowId, start.ApplicantToken!)); + statuses.Add(response.StatusCode); + } + + Assert.Equal(Limit, statuses.Count(s => s == HttpStatusCode.Accepted)); + Assert.Contains(HttpStatusCode.TooManyRequests, statuses); + } + + [Fact] + public async Task AnalyticsIngest_RateLimitedResponse_TellsTheCallerWhenToRetry() + { + using var factory = CreateRateLimitedFactory(); + + using var operatorClient = factory.CreateClient(); + operatorClient.DefaultRequestHeaders.Add("X-Api-Key", "test-api-key"); + var flowId = await CreateFlowAsync(operatorClient); + + using var browser = factory.CreateClient(); + var startResponse = await browser.PostAsJsonAsync("/api/workflow/sessions/start", new { flowId }); + var start = (await startResponse.Content.ReadFromJsonAsync())!; + + HttpResponseMessage? limited = null; + for (var i = 0; i < Limit + 2 && limited is null; i++) + { + var response = await browser.SendAsync(IngestRequest(start.SessionId, flowId, start.ApplicantToken!)); + if (response.StatusCode == HttpStatusCode.TooManyRequests) + limited = response; + } + + Assert.NotNull(limited); + Assert.True(limited!.Headers.Contains("Retry-After")); + } +} diff --git a/src/backend/OpenOnboarding.Application.Tests/ApplicantSessionTokenTests.cs b/src/backend/OpenOnboarding.Application.Tests/ApplicantSessionTokenTests.cs index aa5864e..4722962 100644 --- a/src/backend/OpenOnboarding.Application.Tests/ApplicantSessionTokenTests.cs +++ b/src/backend/OpenOnboarding.Application.Tests/ApplicantSessionTokenTests.cs @@ -278,4 +278,37 @@ public async Task OperatorStartedSession_DoesNotMintAnApplicantToken() var start = await response.Content.ReadFromJsonAsync(); Assert.Null(start!.ApplicantToken); } + + // ======================================================================= + // The operator surface depends on a real credential, not an API key the + // browser happens to hold + // ======================================================================= + [Theory] + [MemberData(nameof(OperatorOnlyEndpoints))] + public async Task OperatorEndpoints_RejectARequestCarryingNoCredential(string url) + { + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + // Exactly what an operator screen sends once its AdminSession cookie is gone. + using var browser = CreateClient(factory); + var response = await browser.GetAsync(url.Replace("{flowId}", flowId.ToString())); + + Assert.True( + response.StatusCode is HttpStatusCode.Unauthorized or HttpStatusCode.Forbidden, + $"{url} returned {(int)response.StatusCode} with no credential; expected 401 or 403."); + } + + [Fact] + public async Task OperatorEndpoints_AreReachableWithTheOperatorCredential() + { + // The counterpart to the test above: the endpoints are not simply broken. + using var factory = TestWebAppFactory.Create(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateFlowAsync(operatorClient); + + Assert.Equal(HttpStatusCode.OK, (await operatorClient.GetAsync("/api/workflow/sessions")).StatusCode); + Assert.Equal(HttpStatusCode.OK, (await operatorClient.GetAsync($"/api/flows/{flowId}/webhooks")).StatusCode); + } } diff --git a/src/backend/OpenOnboarding.Application.Tests/InMemorySessionEventEmitterWarningTests.cs b/src/backend/OpenOnboarding.Application.Tests/InMemorySessionEventEmitterWarningTests.cs new file mode 100644 index 0000000..690b6b0 --- /dev/null +++ b/src/backend/OpenOnboarding.Application.Tests/InMemorySessionEventEmitterWarningTests.cs @@ -0,0 +1,85 @@ +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using OpenOnboarding.Infrastructure.Services; + +namespace OpenOnboarding.Application.Tests; + +/// +/// The scale-out limitation of the in-memory emitter fails silently at runtime - the SSE stream +/// stays open and simply never delivers - so the startup warning is the only signal an operator +/// gets. It has to actually fire. +/// +public sealed class InMemorySessionEventEmitterWarningTests +{ + private sealed record LogEntry(LogLevel Level, string Message); + + private sealed class CapturingLogger : ILogger + { + public List Entries { get; } = []; + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter) + => Entries.Add(new LogEntry(logLevel, formatter(state, exception))); + } + + private sealed class StubHostEnvironment(string environmentName) : IHostEnvironment + { + public string EnvironmentName { get; set; } = environmentName; + public string ApplicationName { get; set; } = "OpenOnboarding.Api"; + public string ContentRootPath { get; set; } = AppContext.BaseDirectory; + public Microsoft.Extensions.FileProviders.IFileProvider ContentRootFileProvider { get; set; } = + new Microsoft.Extensions.FileProviders.NullFileProvider(); + } + + private static async Task> StartInAsync(string environmentName) + { + var logger = new CapturingLogger(); + var service = new InMemorySessionEventEmitterWarning(new StubHostEnvironment(environmentName), logger); + + await service.StartAsync(CancellationToken.None); + + return logger.Entries; + } + + [Theory] + [InlineData("Production")] + [InlineData("Staging")] + public async Task NonDevelopmentEnvironment_WarnsAndNamesTheScaleOutLimitation(string environmentName) + { + var entries = await StartInAsync(environmentName); + + var warning = Assert.Single(entries); + Assert.Equal(LogLevel.Warning, warning.Level); + Assert.Contains(environmentName, warning.Message); + Assert.Contains("replica", warning.Message, StringComparison.OrdinalIgnoreCase); + // The message has to say what to do about it, not just that something is wrong. + Assert.Contains("SessionEvents:Transport", warning.Message); + } + + [Theory] + [InlineData("Development")] + [InlineData("Testing")] + public async Task DevelopmentAndTesting_StaySilent(string environmentName) + { + // Single-instance by definition; a warning on every local run teaches people to ignore it. + Assert.Empty(await StartInAsync(environmentName)); + } + + [Fact] + public async Task StopAsync_IsANoOp() + { + var service = new InMemorySessionEventEmitterWarning( + new StubHostEnvironment("Production"), + new CapturingLogger()); + + await service.StopAsync(CancellationToken.None); + } +} diff --git a/src/backend/OpenOnboarding.Application.Tests/TestWebAppFactory.cs b/src/backend/OpenOnboarding.Application.Tests/TestWebAppFactory.cs index bfbbce0..bfaacd8 100644 --- a/src/backend/OpenOnboarding.Application.Tests/TestWebAppFactory.cs +++ b/src/backend/OpenOnboarding.Application.Tests/TestWebAppFactory.cs @@ -14,9 +14,14 @@ namespace OpenOnboarding.Application.Tests; /// internal static class TestWebAppFactory { + /// + /// Defaults to Testing, which disables rate limiting so unrelated tests are not throttled. + /// Pass Development to exercise the real limiters. + /// public static WebApplicationFactory Create( string? dbName = null, - IReadOnlyDictionary? configurationOverrides = null) + IReadOnlyDictionary? configurationOverrides = null, + string environment = "Testing") { // A shared InMemoryDatabaseRoot ensures all DbContext instances across DI scopes // (startup seed scope, test seed scope, request scope) read from the same store. @@ -25,7 +30,7 @@ public static WebApplicationFactory Create( return new WebApplicationFactory().WithWebHostBuilder(builder => { - builder.UseEnvironment("Testing"); + builder.UseEnvironment(environment); builder.ConfigureAppConfiguration((_, config) => { var settings = new Dictionary