diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7201e95..744137a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,8 +46,29 @@ jobs: --health-timeout 5s --health-retries 10 + # Carries session events between API replicas. The broker-backed transport tests skip + # themselves when nothing is listening, so this is what makes them actually run. The + # management plugin is needed by one of them, which drops a connection broker-side to prove + # the consumer recovers. + rabbitmq: + image: rabbitmq:3-management + env: + RABBITMQ_DEFAULT_USER: guest + RABBITMQ_DEFAULT_PASS: guest + ports: + - 5672:5672 + - 15672:15672 + options: >- + --health-cmd "rabbitmq-diagnostics -q check_running" + --health-interval 10s + --health-timeout 10s + --health-retries 12 + env: ConnectionStrings__OnboardingDb: "Host=localhost;Port=5432;Database=onboarding_test;Username=postgres;Password=postgres" + SESSIONEVENTS__RABBITMQ__URI: "amqp://guest:guest@localhost:5672/" + # A broker exists here, so a missing one must fail the build rather than skip the tests. + REQUIRE_BROKER_TESTS: "1" PERSONA_REPORT_DIR: ${{ github.workspace }}/persona-report steps: @@ -171,6 +192,14 @@ jobs: env: ConnectionStrings__OnboardingDb: "Host=localhost;Port=5432;Database=onboarding_test;Username=postgres;Password=postgres" + # These suites drive the API hard from a single address, which is one rate limit partition + # now that the "general" policy covers every controller endpoint. Rate limiting has its own + # dedicated tests; throttling here would only add flake, so the budgets are lifted to keep + # these jobs behaving as they did before the policy was attached. + RateLimiting__GeneralPerMinute: "100000" + RateLimiting__GlobalCeilingPerMinute: "100000" + RateLimiting__SessionStartPerMinute: "100000" + RateLimiting__AnalyticsIngestPerMinute: "100000" steps: - uses: actions/checkout@v4 @@ -263,6 +292,14 @@ jobs: env: ConnectionStrings__OnboardingDb: "Host=localhost;Port=5432;Database=onboarding_test;Username=postgres;Password=postgres" + # This suite drives the API from a single address, which is one rate limit partition now + # that the "general" policy covers every controller endpoint. Rate limiting has its own + # dedicated tests; throttling here would only add flake, so the budgets are lifted to keep + # this job behaving as it did before the policy was attached. + RateLimiting__GeneralPerMinute: "100000" + RateLimiting__GlobalCeilingPerMinute: "100000" + RateLimiting__SessionStartPerMinute: "100000" + RateLimiting__AnalyticsIngestPerMinute: "100000" steps: - uses: actions/checkout@v4 @@ -321,6 +358,14 @@ jobs: ASPNETCORE_URLS: http://localhost:5072 Authentication__ApiKey: playwright-api-key Authentication__JwtAuthority: "" + # These suites drive the API hard from a single address, which is one rate limit partition + # now that the "general" policy covers every controller endpoint. Rate limiting has its own + # dedicated tests; throttling here would only add flake, so the budgets are lifted to keep + # these jobs behaving as they did before the policy was attached. + RateLimiting__GeneralPerMinute: "100000" + RateLimiting__GlobalCeilingPerMinute: "100000" + RateLimiting__SessionStartPerMinute: "100000" + RateLimiting__AnalyticsIngestPerMinute: "100000" steps: - uses: actions/checkout@v4 diff --git a/docker-compose.yml b/docker-compose.yml index 4aa2790..02d63bc 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -15,6 +15,22 @@ services: timeout: 5s retries: 10 + # Only needed for multi-replica session event fan-out (SessionEvents:Transport=rabbitmq) and + # for the broker-backed tests, which skip themselves when nothing is listening here. + rabbitmq: + image: rabbitmq:3-management + environment: + RABBITMQ_DEFAULT_USER: guest + RABBITMQ_DEFAULT_PASS: guest + ports: + - "5672:5672" + - "15672:15672" + healthcheck: + test: ["CMD", "rabbitmq-diagnostics", "-q", "check_running"] + interval: 10s + timeout: 10s + retries: 10 + api: build: . ports: diff --git a/docs/runbook.md b/docs/runbook.md index d6b9614..9a231a7 100644 --- a/docs/runbook.md +++ b/docs/runbook.md @@ -135,8 +135,25 @@ response carries `applicantToken`, an HMAC-signed JWT the browser sends as - **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 +- **Lifetime.** Defaults to `SessionTimeoutMinutes` (1440), overridable with + `Authentication__ApplicantToken__LifetimeMinutes`. A shorter life only strands applicants mid-journey; a longer one outlives the session it names, which is abandoned by then anyway. +- **Renewal.** Step submission returns a fresh `applicantToken` and `applicantTokenExpiresAt` + alongside the next step, and the browser stores it in place of the one it held. Activity is what + extends the credential, so a journey being worked on never outlives its own token, and there is no + separate refresh endpoint to protect. Operators get no token back: their own credential already + covers the session. +- **Terminal sessions.** Once a session is `Completed` or `Abandoned` its token is refused for + writes — step submission and `POST /api/analytics/events` both return `403`. Reads still work, so + the completion screen renders. This bounds how long a credential left behind on a shared machine + stays useful: the visit, not the full lifetime. Operators are unaffected; the rule is about a + stale applicant credential, not about who may act on a finished session. Abandoning an already + terminal session stays idempotent rather than returning `403`, since it changes nothing. +- **Clearing.** The browser drops the token from `sessionStorage` when the journey completes. +- **Expiry, from the applicant's side.** An expired token returns `401`; a token for a finished + session returns `403`. The frontend treats both as an expired credential and shows a distinct + "your session has expired, start again" state rather than a generic error banner — an expired + credential is a dead end, not a fault to retry. - **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. @@ -168,6 +185,27 @@ Events are stored in the `AnalyticsEvents` table and pruned by a background swee 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. +#### Reading one session's trail + +The operator console shows a session's trail under **Sessions → (a session) → Event trail**, beside +its submissions. Submissions say what the applicant entered; the trail says how they got there — +the order steps were seen in, and the gap between consecutive events, which is what makes a stall +visible. It reads `GET /api/analytics/sessions/{sessionId}/events` with the admin session cookie, +and is never requested from the applicant page. + +Two empty states are worth recognising when reading it: + +- **Server events only.** Client events are self-reported by the applicant's browser and can be + absent entirely — an ad blocker or a tab closed before the batch flushed stops them reaching the + API. The panel says so. It is not evidence of a fault. +- **No events at all.** Either none were raised, or they aged past `Analytics__RetentionDays` and + were deleted. The two are indistinguishable after the sweep, so the panel names both rather than + implying data was lost. Expect this for any session older than the retention window. + +Aggregate drop-off in the flow analytics view is still derived from session status and submissions, +not from this event stream, so per-step timing is currently visible per session but not across a +flow. + ### 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`). @@ -423,10 +461,87 @@ A non-Development environment running the in-memory emitter logs a startup warni 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. +**Connection recovery.** The transport sets `AutomaticRecoveryEnabled` and `TopologyRecoveryEnabled` +explicitly rather than relying on the RabbitMQ client defaults. On a dropped connection the client +reconnects, re-declares the exclusive queue and its binding, and re-attaches the consumer, so +delivery resumes without restarting the instance. This is covered by a test that drops the +connection from the broker side and asserts events flow again. Each connection reports a name of +`open-onboarding-session-events:`, so the management UI identifies which replica holds +which queue. + +**Testing the transport.** The broker-backed tests skip themselves when nothing is listening, so +`dotnet test` stays dependency-free. To run them: + +```bash +docker compose up -d rabbitmq +dotnet test src/backend/OpenOnboarding.Application.Tests +``` + +Point them elsewhere with `SESSIONEVENTS__RABBITMQ__URI`. One of them — the recovery test — also +needs the management API on port 15672, and skips separately if the broker image does not include +it. CI declares a `rabbitmq:3-management` service container and sets `REQUIRE_BROKER_TESTS=1`, which +turns a missing broker into a failure rather than a skip: a silently skipped test is a green build +that proved nothing. + All other state is in PostgreSQL — safe for horizontal scaling. **Connection pool**: Configure `Maximum Pool Size` in the connection string (default: 100). For multi-instance, ensure total connections < PostgreSQL `max_connections` (default: 100). ### Rate Limits -Default rate limit: **100 requests per minute per IP** (configurable). Adjust in `Program.cs` `AddRateLimiter()`. +Every policy is **partitioned per caller**, so one client exhausting its budget does not affect any +other. Configure the limits under `RateLimiting` in `appsettings.json` or as +`RateLimiting__` environment variables. + +| Policy | Endpoint | Partitioned by | Setting | Default (per minute) | +| --- | --- | --- | --- | --- | +| `session-start` | `POST /api/workflow/sessions/start` | Client IP — the endpoint is anonymous by design | `RateLimiting:SessionStartPerMinute` | 100 | +| `analytics-ingest` | `POST /api/analytics/events` | Applicant session id from the token; operators fall back to their principal | `RateLimiting:AnalyticsIngestPerMinute` | 120 | +| `webhook-registration` | `POST /api/flows/{flowId}/webhooks` | Authenticated principal, falling back to client IP | `RateLimiting:WebhookRegistrationPerMinute` | 20 | +| `general` | every other controller endpoint | Authenticated principal, falling back to client IP | `RateLimiting:GeneralPerMinute` | 300 | +| global ceiling | every request | nothing — one bucket for the whole instance | `RateLimiting:GlobalCeilingPerMinute` | 3000 | + +The global ceiling runs **in addition to** the endpoint policy, so a flood spread across thousands +of partitions still has a bound. A request rejected by either returns `429` with `Retry-After: 60`. + +`general` is the fallback: it is attached to every controller endpoint that does not declare a +policy of its own, so no part of the API is unlimited. An endpoint naming a specific policy is +**not** additionally bound by it — the two do not stack, and `session-start` runs under its own +budget alone. Health check endpoints are outside it entirely, since throttling the endpoint a load +balancer polls turns a traffic spike into an instance being pulled out of service. + +One consequence worth sizing for: callers sharing a credential share a partition. Every integration +using the same API key spends one `general` budget between them, because the partition key is the +principal. Give separate integrations separate credentials, or raise the limit to cover them all. + +#### Behind a proxy + +The partition key is the connection address, which behind a load balancer is the balancer's own +address — every caller would land in one partition again. `X-Forwarded-For` corrects this, but only +from a proxy this API trusts; a header from anywhere else is an anonymous caller choosing their own +partition key, and is ignored. + +**Nothing is trusted by default.** The ASP.NET defaults (loopback) are cleared at startup, so +`X-Forwarded-For` has no effect until you name your proxy: + +```jsonc +"ForwardedHeaders": { + "KnownProxies": ["10.1.2.3"], // individual proxy addresses + "KnownNetworks": ["10.0.0.0/8"], // or CIDR ranges + "ForwardLimit": 1 // hops to walk back; raise only if you have chained proxies +} +``` + +If you deploy behind an ingress and leave this unset, every request partitions by the ingress +address and the limits behave as global ones. Set it. + +#### Multi-replica behaviour + +**Limits are per-instance, not cluster-wide.** The limiters hold their counters in process memory, +so with *N* replicas a caller's effective budget is up to *N* × the configured value, depending on +which instance each request lands on. This is accepted deliberately, for the same reason as the SSE +fan-out design: the alternative is a shared counter store on the path of every request. + +Size the limits accordingly — divide the budget you actually want by your replica count — and treat +these as a coarse abuse bound, not a precise quota. A cluster-wide limit belongs at the ingress or +in a shared store; if you need one, that is where to put it. diff --git a/src/backend/OpenOnboarding.Api/Authorization/SessionOwnershipHandler.cs b/src/backend/OpenOnboarding.Api/Authorization/SessionOwnershipHandler.cs index 0d31151..8051827 100644 --- a/src/backend/OpenOnboarding.Api/Authorization/SessionOwnershipHandler.cs +++ b/src/backend/OpenOnboarding.Api/Authorization/SessionOwnershipHandler.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Authorization; using OpenOnboarding.Api.Authentication; using OpenOnboarding.Application.Contracts; +using OpenOnboarding.Domain.Enums; namespace OpenOnboarding.Api.Authorization; @@ -13,6 +14,14 @@ namespace OpenOnboarding.Api.Authorization; public sealed class SessionOwnershipHandler : AuthorizationHandler { + /// + /// Statuses after which a journey is over. An applicant token is refused for writes against a + /// session in one of these, so the credential stops being useful when the visit ends rather + /// than when it expires up to a day later. + /// + private static bool IsTerminal(SessionStatus status) + => status is SessionStatus.Completed or SessionStatus.Abandoned; + protected override Task HandleRequirementAsync( AuthorizationHandlerContext context, SessionOwnershipRequirement requirement, @@ -24,6 +33,9 @@ protected override Task HandleRequirementAsync( return Task.CompletedTask; } + if (requirement.RequiresActiveSession && IsTerminal(resource.Status)) + return Task.CompletedTask; + // 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); diff --git a/src/backend/OpenOnboarding.Api/Authorization/SessionOwnershipRequirement.cs b/src/backend/OpenOnboarding.Api/Authorization/SessionOwnershipRequirement.cs index 0dc1900..0857faa 100644 --- a/src/backend/OpenOnboarding.Api/Authorization/SessionOwnershipRequirement.cs +++ b/src/backend/OpenOnboarding.Api/Authorization/SessionOwnershipRequirement.cs @@ -6,4 +6,28 @@ namespace OpenOnboarding.Api.Authorization; /// Requirement that the current user owns the session (or is an Operator). /// Used with resource-based authorization against SessionDetailDto. /// -public sealed class SessionOwnershipRequirement : IAuthorizationRequirement { } +public sealed class SessionOwnershipRequirement : IAuthorizationRequirement +{ + private SessionOwnershipRequirement(bool requiresActiveSession) + => RequiresActiveSession = requiresActiveSession; + + /// + /// Whether an applicant token is refused once its session has reached a terminal status. + /// Operators are never subject to this; the rule is about a credential outliving the visit that + /// created it, not about who may act on a finished session. + /// + public bool RequiresActiveSession { get; } + + /// + /// For reads. A completed session is still readable by the token that completed it, which is + /// what lets the completion screen render before the applicant closes the tab. + /// + public static SessionOwnershipRequirement Read { get; } = new(requiresActiveSession: false); + + /// + /// For anything that changes the session or writes against it. Once a session is Completed or + /// Abandoned its token stops being accepted here, so a credential left behind on a shared + /// machine cannot be used to write against the journey that ended. + /// + public static SessionOwnershipRequirement Write { get; } = new(requiresActiveSession: true); +} diff --git a/src/backend/OpenOnboarding.Api/Controllers/AnalyticsController.cs b/src/backend/OpenOnboarding.Api/Controllers/AnalyticsController.cs index b36c026..b377acf 100644 --- a/src/backend/OpenOnboarding.Api/Controllers/AnalyticsController.cs +++ b/src/backend/OpenOnboarding.Api/Controllers/AnalyticsController.cs @@ -6,6 +6,7 @@ using OpenOnboarding.Application.Contracts; using OpenOnboarding.Application.Interfaces; using OpenOnboarding.Domain.Entities; +using OpenOnboarding.Domain.Enums; namespace OpenOnboarding.Api.Controllers; @@ -79,6 +80,16 @@ public async Task> Ingest( } } + // A token for a finished journey stops being accepted here. It survives until expiry - up + // to a day - and on a shared machine that outlives the visit that created it. Operators are + // unaffected: they post for sessions they did not start, finished ones included. + if (!isOperator && Guid.TryParse(callerSessionId, out var callerSession)) + { + var session = await sessionAnalyticsService.GetSessionAsync(callerSession, cancellationToken); + if (session is not null && session.Status is SessionStatus.Completed or SessionStatus.Abandoned) + return Forbid(); + } + foreach (var incoming in request.Events) { await telemetryService.TrackAsync( diff --git a/src/backend/OpenOnboarding.Api/Controllers/WorkflowController.cs b/src/backend/OpenOnboarding.Api/Controllers/WorkflowController.cs index 05e5677..58b5c98 100644 --- a/src/backend/OpenOnboarding.Api/Controllers/WorkflowController.cs +++ b/src/backend/OpenOnboarding.Api/Controllers/WorkflowController.cs @@ -75,10 +75,23 @@ public async Task> StartSession([FromBody] St public async Task> SubmitStep([FromRoute] Guid sessionId, [FromRoute] Guid nodeId, [FromBody] SubmitStepRequest request, CancellationToken cancellationToken) { var session = await sessionAnalyticsService.GetSessionAsync(sessionId, cancellationToken); - var authResult = await authorizationService.AuthorizeAsync(User, session, new SessionOwnershipRequirement()); + var authResult = await authorizationService.AuthorizeAsync(User, session, SessionOwnershipRequirement.Write); if (!authResult.Succeeded) return Forbid(); var result = await workflowService.SubmitStepAsync(sessionId, nodeId, request, cancellationToken); + + // Renew on activity. The token is issued once at session start and nothing else extends it, + // so a long journey would otherwise reach a live session holding a credential the API no + // longer accepts. Renewing here needs no separate refresh endpoint: submitting a step is + // the activity that proves the journey is still being worked on. + if (!User.IsInRole(AppRoles.Operator) + && User.HasClaim(c => c.Type == ApplicantSessionAuthenticationDefaults.SessionIdClaim)) + { + var (token, expiresAt) = applicantTokenService.Issue(sessionId, session?.CustomerProfileId); + result.ApplicantToken = token; + result.ApplicantTokenExpiresAt = expiresAt; + } + return Ok(result); } @@ -95,7 +108,7 @@ public async Task> SubmitStep([FromRoute] Guid public async Task> GetNextStep([FromRoute] Guid sessionId, CancellationToken cancellationToken) { var session = await sessionAnalyticsService.GetSessionAsync(sessionId, cancellationToken); - var authResult = await authorizationService.AuthorizeAsync(User, session, new SessionOwnershipRequirement()); + var authResult = await authorizationService.AuthorizeAsync(User, session, SessionOwnershipRequirement.Read); if (!authResult.Succeeded) return Forbid(); var result = await workflowService.GetNextStepAsync(sessionId, cancellationToken); @@ -117,7 +130,9 @@ public async Task> GetNextStep([FromRoute] Gui public async Task> AbandonSession([FromRoute] Guid sessionId, CancellationToken cancellationToken) { var session = await sessionAnalyticsService.GetSessionAsync(sessionId, cancellationToken); - var authResult = await authorizationService.AuthorizeAsync(User, session, new SessionOwnershipRequirement()); + // Read, not Write: abandoning an already-terminal session is a documented no-op, + // so refusing the token here would break that idempotency without closing anything. + var authResult = await authorizationService.AuthorizeAsync(User, session, SessionOwnershipRequirement.Read); if (!authResult.Succeeded) return Forbid(); var result = await workflowService.AbandonSessionAsync(sessionId, cancellationToken); @@ -153,7 +168,7 @@ public async Task>> GetSubmissions([Fr public async Task> GetSession([FromRoute] Guid sessionId, CancellationToken cancellationToken) { var session = await sessionAnalyticsService.GetSessionAsync(sessionId, cancellationToken); - var authResult = await authorizationService.AuthorizeAsync(User, session, new SessionOwnershipRequirement()); + var authResult = await authorizationService.AuthorizeAsync(User, session, SessionOwnershipRequirement.Read); if (!authResult.Succeeded) return Forbid(); return Ok(session); @@ -291,7 +306,7 @@ public async Task StreamEvents([FromRoute] Guid sessionId, CancellationToken can // 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()); + var authResult = await authorizationService.AuthorizeAsync(User, session, SessionOwnershipRequirement.Read); if (!authResult.Succeeded) { Response.StatusCode = StatusCodes.Status403Forbidden; diff --git a/src/backend/OpenOnboarding.Api/Program.cs b/src/backend/OpenOnboarding.Api/Program.cs index 44603b5..b5f6f3c 100644 --- a/src/backend/OpenOnboarding.Api/Program.cs +++ b/src/backend/OpenOnboarding.Api/Program.cs @@ -5,14 +5,18 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Diagnostics; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.HttpOverrides; using Microsoft.AspNetCore.RateLimiting; using Microsoft.EntityFrameworkCore; using Microsoft.OpenApi.Models; +using System.Net; using System.Reflection; using System.Text.Json.Serialization; +using System.Threading.RateLimiting; using OpenOnboarding.Api.Authentication; using OpenOnboarding.Api.Authorization; using OpenOnboarding.Api.Configuration; +using OpenOnboarding.Api.RateLimiting; using OpenOnboarding.Application.Exceptions; using OpenOnboarding.Infrastructure.DependencyInjection; using OpenOnboarding.Infrastructure.Persistence; @@ -185,6 +189,30 @@ var environment = builder.Environment.EnvironmentName; var rateLimitingEnabled = !string.Equals(environment, "Testing", StringComparison.OrdinalIgnoreCase); +// Behind a proxy the connection address is the load balancer's, so every caller would land in one +// rate limit partition. X-Forwarded-For fixes that, but only from a proxy we trust - a header from +// anywhere else is an unauthenticated caller choosing their own partition key. The ASP.NET +// defaults (loopback) are cleared so nothing is trusted unless it is configured here. +builder.Services.Configure(options => +{ + options.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto; + options.ForwardLimit = builder.Configuration.GetValue("ForwardedHeaders:ForwardLimit") ?? 1; + options.KnownProxies.Clear(); + options.KnownIPNetworks.Clear(); + + foreach (var proxy in builder.Configuration.GetSection("ForwardedHeaders:KnownProxies").Get() ?? []) + { + if (IPAddress.TryParse(proxy, out var address)) + options.KnownProxies.Add(address); + } + + foreach (var network in builder.Configuration.GetSection("ForwardedHeaders:KnownNetworks").Get() ?? []) + { + if (System.Net.IPNetwork.TryParse(network, out var parsed)) + options.KnownIPNetworks.Add(parsed); + } +}); + builder.Services.AddRateLimiter(options => { options.RejectionStatusCode = StatusCodes.Status429TooManyRequests; @@ -204,52 +232,45 @@ await context.HttpContext.Response.WriteAsJsonAsync(new ProblemDetails var sessionStartLimit = builder.Configuration.GetValue("RateLimiting:SessionStartPerMinute", 100); var webhookRegLimit = builder.Configuration.GetValue("RateLimiting:WebhookRegistrationPerMinute", 20); var generalLimit = builder.Configuration.GetValue("RateLimiting:GeneralPerMinute", 300); + var analyticsIngestLimit = builder.Configuration.GetValue("RateLimiting:AnalyticsIngestPerMinute", 120); + var globalCeiling = builder.Configuration.GetValue("RateLimiting:GlobalCeilingPerMinute", 3000); - options.AddSlidingWindowLimiter("session-start", opt => + static SlidingWindowRateLimiterOptions PerMinute(int permitLimit) => new() { - opt.Window = TimeSpan.FromMinutes(1); - opt.SegmentsPerWindow = 4; - opt.PermitLimit = sessionStartLimit; - opt.QueueProcessingOrder = System.Threading.RateLimiting.QueueProcessingOrder.OldestFirst; - opt.QueueLimit = 0; - }); + Window = TimeSpan.FromMinutes(1), + SegmentsPerWindow = 4, + PermitLimit = permitLimit, + QueueProcessingOrder = QueueProcessingOrder.OldestFirst, + QueueLimit = 0 + }; - options.AddSlidingWindowLimiter("webhook-registration", opt => - { - opt.Window = TimeSpan.FromMinutes(1); - opt.SegmentsPerWindow = 4; - opt.PermitLimit = webhookRegLimit; - opt.QueueProcessingOrder = System.Threading.RateLimiting.QueueProcessingOrder.OldestFirst; - opt.QueueLimit = 0; - }); + // Each policy is partitioned by whoever the request belongs to. AddSlidingWindowLimiter + // would build a single limiter shared by every caller, which is what made one client able + // to lock out the rest. + options.AddPolicy("session-start", context => RateLimitPartition.GetSlidingWindowLimiter( + RateLimitPartitionKeys.ClientAddress(context), _ => PerMinute(sessionStartLimit))); - var analyticsIngestLimit = builder.Configuration.GetValue("RateLimiting:AnalyticsIngestPerMinute", 120); + options.AddPolicy("analytics-ingest", context => RateLimitPartition.GetSlidingWindowLimiter( + RateLimitPartitionKeys.ApplicantSession(context), _ => PerMinute(analyticsIngestLimit))); - 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.AddPolicy("webhook-registration", context => RateLimitPartition.GetSlidingWindowLimiter( + RateLimitPartitionKeys.Principal(context), _ => PerMinute(webhookRegLimit))); - options.AddSlidingWindowLimiter("general", opt => - { - opt.Window = TimeSpan.FromMinutes(1); - opt.SegmentsPerWindow = 4; - opt.PermitLimit = generalLimit; - opt.QueueProcessingOrder = System.Threading.RateLimiting.QueueProcessingOrder.OldestFirst; - opt.QueueLimit = 0; - }); + options.AddPolicy("general", context => RateLimitPartition.GetSlidingWindowLimiter( + RateLimitPartitionKeys.Principal(context), _ => PerMinute(generalLimit))); + + // A ceiling over every partition, so a flood spread across many callers still has a bound. + // It runs in addition to the endpoint policy, not instead of it. + options.GlobalLimiter = PartitionedRateLimiter.Create( + _ => RateLimitPartition.GetSlidingWindowLimiter("global", _ => PerMinute(globalCeiling))); } else { // Testing: add NoLimiter policies so attributes don't error - 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")); + options.AddPolicy("session-start", _ => RateLimitPartition.GetNoLimiter("testing")); + options.AddPolicy("webhook-registration", _ => RateLimitPartition.GetNoLimiter("testing")); + options.AddPolicy("general", _ => RateLimitPartition.GetNoLimiter("testing")); + options.AddPolicy("analytics-ingest", _ => RateLimitPartition.GetNoLimiter("testing")); } }); @@ -337,6 +358,9 @@ await context.Response.WriteAsJsonAsync(new ProblemDetails }); }); +// First in the pipeline: everything downstream that reads the client address - the rate limit +// partitioner above all - must see the real caller, not the proxy in front of it. +app.UseForwardedHeaders(); app.UseHttpsRedirection(); app.UseCors("FrontendDev"); app.UseMiddleware(); @@ -386,7 +410,21 @@ await context.Response.WriteAsJsonAsync(new ProblemDetails } }).AllowAnonymous(); -app.MapControllers(); +// Every controller endpoint falls under the "general" policy unless it names a more specific one. +// The policy was previously declared and attached to nothing, so most of the API was unlimited +// while appearing covered. +// +// Added as metadata only where no policy is already present, rather than with +// RequireRateLimiting: that applies to the whole group, and because the middleware takes the last +// matching metadata it would override the action-level attributes - session-start would silently +// run under the general budget instead of its own. Health checks are mapped above and stay outside. +app.MapControllers().Add(endpoint => +{ + if (endpoint.Metadata.OfType().Any()) return; + if (endpoint.Metadata.OfType().Any()) return; + + endpoint.Metadata.Add(new EnableRateLimitingAttribute("general")); +}); app.MapMetrics().RequireAuthorization().DisableRateLimiting(); diff --git a/src/backend/OpenOnboarding.Api/RateLimiting/RateLimitPartitionKeys.cs b/src/backend/OpenOnboarding.Api/RateLimiting/RateLimitPartitionKeys.cs new file mode 100644 index 0000000..ca70791 --- /dev/null +++ b/src/backend/OpenOnboarding.Api/RateLimiting/RateLimitPartitionKeys.cs @@ -0,0 +1,56 @@ +using System.Security.Claims; +using OpenOnboarding.Api.Authentication; + +namespace OpenOnboarding.Api.RateLimiting; + +/// +/// Derives the key that decides whose budget a request spends. +/// +/// A rate limit without a partition key is a shared bucket: one caller exhausting it locks out +/// every other caller. That is a denial of service against session-start in particular, +/// which is anonymous by design and is where every journey begins. +/// +/// +/// Every key is prefixed by the kind of thing it names, so an address can never collide with a +/// session id or a principal id that happens to have the same text. +/// +/// +public static class RateLimitPartitionKeys +{ + /// Key used when a request has no address at all - a unix socket or an in-process test host. + public const string Unknown = "addr:unknown"; + + /// + /// The caller's network address, after UseForwardedHeaders has resolved + /// X-Forwarded-For from a trusted proxy. A header from anywhere else is ignored, so a + /// caller cannot rotate it to mint fresh budgets. + /// + public static string ClientAddress(HttpContext context) + { + var address = context.Connection.RemoteIpAddress; + return address is null ? Unknown : $"addr:{address}"; + } + + /// + /// The authenticated principal, falling back to the client address while a request is still + /// anonymous - the limiter runs before authorization, so unauthenticated calls reach it too. + /// + public static string Principal(HttpContext context) + { + var id = context.User.FindFirstValue(ClaimTypes.NameIdentifier) + ?? context.User.Identity?.Name; + + return string.IsNullOrWhiteSpace(id) ? ClientAddress(context) : $"user:{id}"; + } + + /// + /// The applicant session the caller's token names, so one applicant's browser cannot throttle + /// another's. Operators carry no session claim and fall back to their own principal. + /// + public static string ApplicantSession(HttpContext context) + { + var sessionId = context.User.FindFirstValue(ApplicantSessionAuthenticationDefaults.SessionIdClaim); + + return string.IsNullOrWhiteSpace(sessionId) ? Principal(context) : $"session:{sessionId}"; + } +} diff --git a/src/backend/OpenOnboarding.Api/appsettings.json b/src/backend/OpenOnboarding.Api/appsettings.json index ec525a5..bed6534 100644 --- a/src/backend/OpenOnboarding.Api/appsettings.json +++ b/src/backend/OpenOnboarding.Api/appsettings.json @@ -37,7 +37,13 @@ "SessionStartPerMinute": 100, "WebhookRegistrationPerMinute": 20, "GeneralPerMinute": 300, - "AnalyticsIngestPerMinute": 120 + "AnalyticsIngestPerMinute": 120, + "GlobalCeilingPerMinute": 3000 + }, + "ForwardedHeaders": { + "KnownProxies": [], + "KnownNetworks": [], + "ForwardLimit": 1 }, "VirusScan": { "Enabled": false, diff --git a/src/backend/OpenOnboarding.Application.Tests/ApplicantTokenLifecycleTests.cs b/src/backend/OpenOnboarding.Application.Tests/ApplicantTokenLifecycleTests.cs new file mode 100644 index 0000000..c68b478 --- /dev/null +++ b/src/backend/OpenOnboarding.Application.Tests/ApplicantTokenLifecycleTests.cs @@ -0,0 +1,387 @@ +using System.Net; +using System.Net.Http.Headers; +using System.Net.Http.Json; +using System.Security.Claims; +using System.Text; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.IdentityModel.JsonWebTokens; +using Microsoft.IdentityModel.Tokens; +using OpenOnboarding.Api.Authentication; +using OpenOnboarding.Api.Authorization; + +namespace OpenOnboarding.Application.Tests; + +/// +/// The applicant token is issued once at session start and nothing renews or withdraws it. Two +/// things follow: a long journey outlives its own credential and strands the applicant with no way +/// forward but to start again, and a finished journey leaves a working credential behind for up to +/// a day - on a shared machine, well past the visit that created it. +/// +public sealed class ApplicantTokenLifecycleTests +{ + private const string OperatorApiKey = "test-api-key"; + + /// + /// Fixed so a test can mint its own token - an expired one in particular, which is otherwise + /// only reachable by waiting out the lifetime. + /// + private const string SigningKey = "applicant-token-lifecycle-tests-signing-key-long-enough-for-hmac-sha256"; + + private sealed record StepResponse( + 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 WebApplicationFactory CreateFactory() + => TestWebAppFactory.Create(configurationOverrides: new Dictionary + { + ["Authentication:ApplicantToken:SigningKey"] = SigningKey + }); + + private static HttpClient CreateOperatorClient(WebApplicationFactory factory) + { + var client = factory.CreateClient(); + client.DefaultRequestHeaders.Add("X-Api-Key", OperatorApiKey); + return client; + } + + /// A one-step flow: submitting its only step completes the session. + private static async Task CreateSingleStepFlowAsync(HttpClient operatorClient) + { + var response = await operatorClient.PostAsJsonAsync("/api/flows", new + { + name = $"Token lifecycle flow {Guid.NewGuid()}", + nodes = new[] + { + new { key = "start", type = "Form", title = "Start", isStartNode = true, jsonContent = "{\"fields\":[]}" } + }, + connections = Array.Empty() + }); + + response.EnsureSuccessStatusCode(); + return (await response.Content.ReadFromJsonAsync())!.Id; + } + + /// A two-step flow, so a journey can continue on the credential the first step returned. + private static async Task CreateTwoStepFlowAsync(HttpClient operatorClient) + { + var first = Guid.NewGuid(); + var second = Guid.NewGuid(); + + var response = await operatorClient.PostAsJsonAsync("/api/flows", new + { + name = $"Token lifecycle flow {Guid.NewGuid()}", + nodes = new object[] + { + new { id = first, key = "start", type = "Form", title = "Start", isStartNode = true, jsonContent = "{\"fields\":[]}" }, + new { id = second, key = "details", type = "Form", title = "Details", isStartNode = false, jsonContent = "{\"fields\":[]}" } + }, + connections = new object[] + { + new { sourceNodeId = first, targetNodeId = second, priority = 0 } + } + }); + + response.EnsureSuccessStatusCode(); + return (await response.Content.ReadFromJsonAsync())!.Id; + } + + private static async Task StartSessionAsync(HttpClient browser, Guid flowId) + { + var response = await browser.PostAsJsonAsync("/api/workflow/sessions/start", new { flowId }); + response.EnsureSuccessStatusCode(); + return (await response.Content.ReadFromJsonAsync())!; + } + + private static HttpRequestMessage SubmitRequest(Guid sessionId, Guid nodeId, string token) + { + var request = new HttpRequestMessage( + HttpMethod.Post, $"/api/workflow/sessions/{sessionId}/steps/{nodeId}/submit") + { + Content = JsonContent.Create(new { payload = new Dictionary() }) + }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + return request; + } + + private static HttpRequestMessage IngestRequest(Guid sessionId, Guid flowId, string token) + { + var request = new HttpRequestMessage(HttpMethod.Post, "/api/analytics/events") + { + Content = JsonContent.Create(new + { + events = new[] + { + new + { + eventType = "step_view", + journeyId = flowId.ToString(), + sessionId = sessionId.ToString() + } + } + }) + }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + return request; + } + + private static HttpRequestMessage ReadRequest(Guid sessionId, string token) + { + var request = new HttpRequestMessage(HttpMethod.Get, $"/api/workflow/sessions/{sessionId}"); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + return request; + } + + /// + /// Mints a token for the session that expired in the past. Signed with the same key the API + /// uses, so the only reason it can be refused is its lifetime. + /// + private static string ExpiredTokenFor(Guid sessionId) + { + var descriptor = new SecurityTokenDescriptor + { + Issuer = ApplicantSessionAuthenticationDefaults.Issuer, + Audience = ApplicantSessionAuthenticationDefaults.Audience, + IssuedAt = DateTime.UtcNow.AddHours(-2), + Expires = DateTime.UtcNow.AddHours(-1), + Claims = new Dictionary + { + [JwtRegisteredClaimNames.Sub] = sessionId.ToString(), + [ApplicantSessionAuthenticationDefaults.SessionIdClaim] = sessionId.ToString(), + [ClaimTypes.Role] = AppRoles.Applicant + }, + SigningCredentials = new SigningCredentials( + new SymmetricSecurityKey(Encoding.UTF8.GetBytes(SigningKey)), + SecurityAlgorithms.HmacSha256) + }; + + return new JsonWebTokenHandler().CreateToken(descriptor); + } + + // ── Renewal ──────────────────────────────────────────────────────────────── + + [Fact] + public async Task SubmittingAStep_ReturnsARefreshedToken() + { + using var factory = CreateFactory(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateTwoStepFlowAsync(operatorClient); + + using var browser = factory.CreateClient(); + var start = await StartSessionAsync(browser, flowId); + + var response = await browser.SendAsync( + SubmitRequest(start.SessionId, start.CurrentNode!.Id, start.ApplicantToken!)); + response.EnsureSuccessStatusCode(); + + var submitted = (await response.Content.ReadFromJsonAsync())!; + + Assert.False(string.IsNullOrWhiteSpace(submitted.ApplicantToken)); + Assert.True(submitted.ApplicantTokenExpiresAt > DateTimeOffset.UtcNow); + + // Not asserted to differ from the original: a JWT carries its expiry to the second, so two + // issuances inside the same second are byte-identical. What matters is that the response + // carries a usable credential for this session - see AJourneyContinues_OnTheRefreshedTokenAlone. + var claims = new JsonWebTokenHandler().ReadJsonWebToken(submitted.ApplicantToken); + Assert.Equal( + start.SessionId.ToString(), + claims.GetClaim(ApplicantSessionAuthenticationDefaults.SessionIdClaim).Value); + } + + [Fact] + public async Task TheRefreshedToken_ExpiresLaterThanTheOneItReplaces() + { + using var factory = CreateFactory(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateTwoStepFlowAsync(operatorClient); + + using var browser = factory.CreateClient(); + var start = await StartSessionAsync(browser, flowId); + + var response = await browser.SendAsync( + SubmitRequest(start.SessionId, start.CurrentNode!.Id, start.ApplicantToken!)); + var submitted = (await response.Content.ReadFromJsonAsync())!; + + // This is what keeps a journey alive: each step pushes the deadline out, so the session + // cannot outlive its own credential while it is still being worked on. + Assert.NotNull(submitted.ApplicantTokenExpiresAt); + Assert.True( + submitted.ApplicantTokenExpiresAt > start.ApplicantTokenExpiresAt, + "the refreshed token must expire later than the one it replaces"); + } + + [Fact] + public async Task AJourneyContinues_OnTheRefreshedTokenAlone() + { + using var factory = CreateFactory(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateTwoStepFlowAsync(operatorClient); + + using var browser = factory.CreateClient(); + var start = await StartSessionAsync(browser, flowId); + + var first = await browser.SendAsync( + SubmitRequest(start.SessionId, start.CurrentNode!.Id, start.ApplicantToken!)); + var afterFirst = (await first.Content.ReadFromJsonAsync())!; + + // The original credential is never used again - the journey finishes on the renewal. + var second = await browser.SendAsync( + SubmitRequest(afterFirst.SessionId, afterFirst.CurrentNode!.Id, afterFirst.ApplicantToken!)); + + Assert.Equal(HttpStatusCode.OK, second.StatusCode); + var afterSecond = (await second.Content.ReadFromJsonAsync())!; + Assert.True(afterSecond.IsCompleted); + } + + [Fact] + public async Task AnOperatorSubmitting_IsNotHandedAnApplicantToken() + { + using var factory = CreateFactory(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateTwoStepFlowAsync(operatorClient); + + var start = await StartSessionAsync(operatorClient, flowId); + + var response = await operatorClient.PostAsJsonAsync( + $"/api/workflow/sessions/{start.SessionId}/steps/{start.CurrentNode!.Id}/submit", + new { payload = new Dictionary() }); + var submitted = (await response.Content.ReadFromJsonAsync())!; + + // An operator credential already covers the session; minting a second one would be a + // credential nobody asked for. + Assert.Null(submitted.ApplicantToken); + Assert.Null(submitted.ApplicantTokenExpiresAt); + } + + // ── Expiry ───────────────────────────────────────────────────────────────── + + [Fact] + public async Task AnExpiredToken_IsRefused() + { + using var factory = CreateFactory(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateTwoStepFlowAsync(operatorClient); + + using var browser = factory.CreateClient(); + var start = await StartSessionAsync(browser, flowId); + + var response = await browser.SendAsync( + SubmitRequest(start.SessionId, start.CurrentNode!.Id, ExpiredTokenFor(start.SessionId))); + + // 401, not 403: the credential is not bad, it is over. The frontend tells the two apart to + // show "your session has expired" rather than a generic failure. + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + // ── Terminal sessions ────────────────────────────────────────────────────── + + [Fact] + public async Task ATokenForACompletedSession_IsRefusedFromStepSubmission() + { + using var factory = CreateFactory(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateSingleStepFlowAsync(operatorClient); + + using var browser = factory.CreateClient(); + var start = await StartSessionAsync(browser, flowId); + + var completing = await browser.SendAsync( + SubmitRequest(start.SessionId, start.CurrentNode!.Id, start.ApplicantToken!)); + var completed = (await completing.Content.ReadFromJsonAsync())!; + Assert.True(completed.IsCompleted); + + var again = await browser.SendAsync( + SubmitRequest(start.SessionId, start.CurrentNode.Id, completed.ApplicantToken ?? start.ApplicantToken!)); + + Assert.Equal(HttpStatusCode.Forbidden, again.StatusCode); + } + + [Fact] + public async Task ATokenForACompletedSession_IsRefusedFromAnalyticsIngest() + { + using var factory = CreateFactory(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateSingleStepFlowAsync(operatorClient); + + using var browser = factory.CreateClient(); + var start = await StartSessionAsync(browser, flowId); + + // Accepted while the journey is live. + var whileLive = await browser.SendAsync(IngestRequest(start.SessionId, flowId, start.ApplicantToken!)); + Assert.Equal(HttpStatusCode.Accepted, whileLive.StatusCode); + + var completing = await browser.SendAsync( + SubmitRequest(start.SessionId, start.CurrentNode!.Id, start.ApplicantToken!)); + var completed = (await completing.Content.ReadFromJsonAsync())!; + + var afterCompletion = await browser.SendAsync( + IngestRequest(start.SessionId, flowId, completed.ApplicantToken ?? start.ApplicantToken!)); + + Assert.Equal(HttpStatusCode.Forbidden, afterCompletion.StatusCode); + } + + [Fact] + public async Task ATokenForAnAbandonedSession_IsRefusedFromAnalyticsIngest() + { + using var factory = CreateFactory(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateTwoStepFlowAsync(operatorClient); + + using var browser = factory.CreateClient(); + var start = await StartSessionAsync(browser, flowId); + + var abandon = new HttpRequestMessage(HttpMethod.Delete, $"/api/workflow/sessions/{start.SessionId}"); + abandon.Headers.Authorization = new AuthenticationHeaderValue("Bearer", start.ApplicantToken!); + (await browser.SendAsync(abandon)).EnsureSuccessStatusCode(); + + var response = await browser.SendAsync(IngestRequest(start.SessionId, flowId, start.ApplicantToken!)); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + } + + [Fact] + public async Task ATokenForACompletedSession_StillServesTheCompletionScreenRead() + { + using var factory = CreateFactory(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateSingleStepFlowAsync(operatorClient); + + using var browser = factory.CreateClient(); + var start = await StartSessionAsync(browser, flowId); + + var completing = await browser.SendAsync( + SubmitRequest(start.SessionId, start.CurrentNode!.Id, start.ApplicantToken!)); + var completed = (await completing.Content.ReadFromJsonAsync())!; + + // The completion screen renders after the session is terminal; refusing this read would + // break the last thing the applicant sees. + var read = await browser.SendAsync( + ReadRequest(start.SessionId, completed.ApplicantToken ?? start.ApplicantToken!)); + + Assert.Equal(HttpStatusCode.OK, read.StatusCode); + } + + [Fact] + public async Task AnOperator_StillReachesACompletedSession() + { + using var factory = CreateFactory(); + using var operatorClient = CreateOperatorClient(factory); + var flowId = await CreateSingleStepFlowAsync(operatorClient); + + using var browser = factory.CreateClient(); + var start = await StartSessionAsync(browser, flowId); + await browser.SendAsync(SubmitRequest(start.SessionId, start.CurrentNode!.Id, start.ApplicantToken!)); + + // The terminal-status rule bounds a stale applicant credential; it says nothing about who + // may act on a finished session. + var read = await operatorClient.GetAsync($"/api/workflow/sessions/{start.SessionId}"); + + Assert.Equal(HttpStatusCode.OK, read.StatusCode); + } +} diff --git a/src/backend/OpenOnboarding.Application.Tests/Authorization/SessionOwnershipHandlerTests.cs b/src/backend/OpenOnboarding.Application.Tests/Authorization/SessionOwnershipHandlerTests.cs index c0e1576..736d5c2 100644 --- a/src/backend/OpenOnboarding.Application.Tests/Authorization/SessionOwnershipHandlerTests.cs +++ b/src/backend/OpenOnboarding.Application.Tests/Authorization/SessionOwnershipHandlerTests.cs @@ -2,12 +2,14 @@ using Microsoft.AspNetCore.Authorization; using OpenOnboarding.Api.Authorization; using OpenOnboarding.Application.Contracts; +using OpenOnboarding.Api.Authentication; +using OpenOnboarding.Domain.Enums; namespace OpenOnboarding.Application.Tests.Authorization; public sealed class SessionOwnershipHandlerTests { - private static readonly SessionOwnershipRequirement Requirement = new(); + private static readonly SessionOwnershipRequirement Requirement = SessionOwnershipRequirement.Read; private static readonly SessionOwnershipHandler Handler = new(); private static ClaimsPrincipal BuildPrincipal(string role, string? customerProfileId = null) @@ -23,8 +25,26 @@ private static ClaimsPrincipal BuildPrincipal(string role, string? customerProfi return new ClaimsPrincipal(new ClaimsIdentity(claims, "Test")); } - private static SessionDetailDto BuildSession(Guid? customerProfileId = null) => - new() { Id = Guid.NewGuid(), FlowId = Guid.NewGuid(), CustomerProfileId = customerProfileId }; + private static SessionDetailDto BuildSession( + Guid? customerProfileId = null, + SessionStatus status = SessionStatus.Started) => + new() + { + Id = Guid.NewGuid(), + FlowId = Guid.NewGuid(), + CustomerProfileId = customerProfileId, + Status = status + }; + + /// A principal carrying a per-session applicant token for the given session. + private static ClaimsPrincipal BuildSessionTokenPrincipal(Guid sessionId) + => new(new ClaimsIdentity( + [ + new Claim(ClaimTypes.NameIdentifier, sessionId.ToString()), + new Claim(ClaimTypes.Role, AppRoles.Applicant), + new Claim(ApplicantSessionAuthenticationDefaults.SessionIdClaim, sessionId.ToString()) + ], + "Test")); [Fact] public async Task HandleRequirementAsync_WhenOperator_Succeeds() @@ -75,4 +95,70 @@ public async Task HandleRequirementAsync_WhenApplicantHasNoCustomerProfileIdClai Assert.False(context.HasSucceeded); } + + // ── Terminal sessions ────────────────────────────────────────────────────── + // + // A token issued at session start stays valid until it expires - up to a day. Once the journey + // it names is over, that credential outlives the visit that created it, which on a shared or + // public machine is exactly the window worth closing. + + [Theory] + [InlineData(SessionStatus.Completed)] + [InlineData(SessionStatus.Abandoned)] + public async Task Write_AgainstATerminalSession_IsRefusedForAnApplicantToken(SessionStatus status) + { + var session = BuildSession(status: status); + var user = BuildSessionTokenPrincipal(session.Id); + + var context = new AuthorizationHandlerContext([SessionOwnershipRequirement.Write], user, session); + await Handler.HandleAsync(context); + + Assert.False(context.HasSucceeded); + } + + [Theory] + [InlineData(SessionStatus.Completed)] + [InlineData(SessionStatus.Abandoned)] + public async Task Read_AgainstATerminalSession_StillServesItsOwnToken(SessionStatus status) + { + // The completion screen renders after the session is Completed; refusing the read would + // break the last thing the applicant sees. + var session = BuildSession(status: status); + var user = BuildSessionTokenPrincipal(session.Id); + + var context = new AuthorizationHandlerContext([SessionOwnershipRequirement.Read], user, session); + await Handler.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + + [Theory] + [InlineData(SessionStatus.Started)] + [InlineData(SessionStatus.Error)] + public async Task Write_AgainstALiveSession_StillSucceeds(SessionStatus status) + { + var session = BuildSession(status: status); + var user = BuildSessionTokenPrincipal(session.Id); + + var context = new AuthorizationHandlerContext([SessionOwnershipRequirement.Write], user, session); + await Handler.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + + [Theory] + [InlineData(SessionStatus.Completed)] + [InlineData(SessionStatus.Abandoned)] + public async Task Write_AgainstATerminalSession_IsStillAllowedForAnOperator(SessionStatus status) + { + // The rule is about a stale applicant credential, not about who may act on a finished + // session. An operator reaches every session regardless of status. + var session = BuildSession(status: status); + var user = BuildPrincipal(AppRoles.Operator); + + var context = new AuthorizationHandlerContext([SessionOwnershipRequirement.Write], user, session); + await Handler.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } } diff --git a/src/backend/OpenOnboarding.Application.Tests/RabbitMqSessionEventTransportTests.cs b/src/backend/OpenOnboarding.Application.Tests/RabbitMqSessionEventTransportTests.cs new file mode 100644 index 0000000..6f464dd --- /dev/null +++ b/src/backend/OpenOnboarding.Application.Tests/RabbitMqSessionEventTransportTests.cs @@ -0,0 +1,299 @@ +using System.Collections.Concurrent; +using System.Text; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using OpenOnboarding.Application.Interfaces; +using OpenOnboarding.Application.Tests.TestHelpers; +using OpenOnboarding.Infrastructure.EventBus; +using RabbitMQ.Client; + +namespace OpenOnboarding.Application.Tests; + +/// +/// proves the fan-out contract through an in-process fake, so +/// it covers the emitter and the port but none of the broker adapter. This is the implementation +/// that actually runs in a multi-replica deployment, and a defect in it reproduces exactly the +/// failure the distributed transport exists to prevent: an SSE stream that stays open and delivers +/// nothing. +/// +/// The parts that need no broker are unit tested. The deployment-shaped case needs a real one and +/// skips with a clear reason when none is reachable. +/// +/// +public sealed class RabbitMqSessionEventTransportTests +{ + private static RabbitMqSessionEventTransport CreateTransport(string exchange, string? clientProvidedName = null) + => new(BrokerProbe.BrokerUri, exchange, NullLogger.Instance, clientProvidedName); + + // ── Wire format ──────────────────────────────────────────────────────────── + + [Fact] + public void Serialize_ThenDeserialize_RoundTripsTheWholeEnvelope() + { + var sessionId = Guid.NewGuid(); + var timestamp = new DateTimeOffset(2026, 9, 13, 10, 30, 45, 123, TimeSpan.Zero); + var original = new SessionEvent("step-completed", """{"nodeKey":"identity","attempt":2}""", timestamp); + + var envelope = RabbitMqSessionEventTransport.Deserialize( + RabbitMqSessionEventTransport.Serialize(sessionId, original)); + + Assert.NotNull(envelope); + Assert.Equal(sessionId, envelope!.SessionId); + Assert.Equal(original.EventType, envelope.EventType); + Assert.Equal(original.PayloadJson, envelope.PayloadJson); + Assert.Equal(original.Timestamp, envelope.Timestamp); + + var restored = envelope.ToSessionEvent(); + Assert.Equal(original.EventType, restored.EventType); + Assert.Equal(original.PayloadJson, restored.PayloadJson); + Assert.Equal(original.Timestamp, restored.Timestamp); + } + + [Fact] + public void Deserialize_PreservesTheTimestampOffset() + { + // A session trail read across regions is wrong by hours if the offset is dropped. + var timestamp = new DateTimeOffset(2026, 9, 13, 10, 30, 45, TimeSpan.FromHours(5.5)); + + var envelope = RabbitMqSessionEventTransport.Deserialize( + RabbitMqSessionEventTransport.Serialize(Guid.NewGuid(), new SessionEvent("x", "{}", timestamp))); + + Assert.Equal(timestamp.Offset, envelope!.Timestamp.Offset); + Assert.Equal(timestamp, envelope.Timestamp); + } + + // ── Poison messages ──────────────────────────────────────────────────────── + + [Fact] + public async Task Deliver_MalformedBody_IsLoggedAndSwallowed() + { + var logger = new CapturingLogger(); + var delivered = new List(); + + await RabbitMqSessionEventTransport.DeliverAsync( + Encoding.UTF8.GetBytes("{ this is not json"), + (sessionId, _) => { delivered.Add(sessionId); return Task.CompletedTask; }, + logger); + + Assert.Empty(delivered); + Assert.Contains(logger.Errors, message => message.Contains("Failed to deliver a session event")); + } + + [Fact] + public async Task Deliver_AfterAMalformedBody_KeepsDeliveringSubsequentMessages() + { + var logger = new CapturingLogger(); + var delivered = new List(); + Func handler = (_, evt) => + { + delivered.Add(evt.EventType); + return Task.CompletedTask; + }; + + await RabbitMqSessionEventTransport.DeliverAsync(Encoding.UTF8.GetBytes("not json at all"), handler, logger); + await RabbitMqSessionEventTransport.DeliverAsync( + RabbitMqSessionEventTransport.Serialize( + Guid.NewGuid(), new SessionEvent("step-completed", "{}", DateTimeOffset.UtcNow)), + handler, + logger); + + Assert.Equal(["step-completed"], delivered); + } + + [Fact] + public async Task Deliver_WhenTheHandlerThrows_DoesNotPropagate() + { + // The consumer callback is the only caller; an exception escaping it kills the consumer and + // with it every open stream on this instance. + var logger = new CapturingLogger(); + + await RabbitMqSessionEventTransport.DeliverAsync( + RabbitMqSessionEventTransport.Serialize( + Guid.NewGuid(), new SessionEvent("step-completed", "{}", DateTimeOffset.UtcNow)), + (_, _) => throw new InvalidOperationException("the stream went away"), + logger); + + Assert.Single(logger.Errors); + } + + [Fact] + public async Task Deliver_AJsonNullBody_IsIgnoredWithoutThrowing() + { + var logger = new CapturingLogger(); + var delivered = 0; + + await RabbitMqSessionEventTransport.DeliverAsync( + Encoding.UTF8.GetBytes("null"), + (_, _) => { delivered++; return Task.CompletedTask; }, + logger); + + Assert.Equal(0, delivered); + Assert.Empty(logger.Errors); + } + + // ── Lifecycle ────────────────────────────────────────────────────────────── + + [Fact] + public async Task BroadcastAsync_BeforeStartAsync_Throws() + { + await using var transport = CreateTransport("session-events-test"); + + var error = await Assert.ThrowsAsync( + () => transport.BroadcastAsync( + Guid.NewGuid(), new SessionEvent("step-completed", "{}", DateTimeOffset.UtcNow))); + + Assert.Contains("StartAsync", error.Message); + } + + [Fact] + public async Task DisposeAsync_BeforeStartAsync_DoesNotThrow() + { + var transport = CreateTransport("session-events-test"); + + await transport.DisposeAsync(); + } + + // ── Against a real broker ────────────────────────────────────────────────── + + [RequiresBrokerFact] + public async Task EventPublishedOnOneInstance_ReachesAnother() + { + var exchange = UniqueExchange(); + await using var publisher = CreateTransport(exchange); + await using var subscriber = CreateTransport(exchange); + + var received = new ConcurrentQueue<(Guid SessionId, SessionEvent Event)>(); + var arrived = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + // Both instances consume, as they do in a real deployment: the publisher receives its own + // broadcast too, which is what makes delivery uniform across instances. + await publisher.StartAsync((_, _) => Task.CompletedTask); + await subscriber.StartAsync((sessionId, evt) => + { + received.Enqueue((sessionId, evt)); + arrived.TrySetResult(); + return Task.CompletedTask; + }); + + var expectedSession = Guid.NewGuid(); + var expectedEvent = new SessionEvent( + "step-completed", """{"nodeKey":"identity"}""", DateTimeOffset.UtcNow); + + await publisher.BroadcastAsync(expectedSession, expectedEvent); + + await WaitForAsync(arrived.Task, "the subscriber never received the broadcast event"); + + Assert.True(received.TryDequeue(out var delivered)); + Assert.Equal(expectedSession, delivered.SessionId); + Assert.Equal(expectedEvent.EventType, delivered.Event.EventType); + Assert.Equal(expectedEvent.PayloadJson, delivered.Event.PayloadJson); + } + + [RequiresBrokerFact] + public async Task AMalformedMessageOnTheWire_DoesNotStopTheConsumer() + { + var exchange = UniqueExchange(); + await using var subscriber = CreateTransport(exchange); + + var good = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + await subscriber.StartAsync((_, _) => { good.TrySetResult(); return Task.CompletedTask; }); + + var factory = new ConnectionFactory { Uri = new Uri(BrokerProbe.BrokerUri) }; + await using var connection = await factory.CreateConnectionAsync(); + await using var channel = await connection.CreateChannelAsync(); + await channel.ExchangeDeclareAsync(exchange, ExchangeType.Fanout, durable: true, autoDelete: false); + + await channel.BasicPublishAsync( + exchange, routingKey: string.Empty, body: Encoding.UTF8.GetBytes("{ not an envelope")); + + await channel.BasicPublishAsync( + exchange, + routingKey: string.Empty, + body: RabbitMqSessionEventTransport.Serialize( + Guid.NewGuid(), new SessionEvent("step-completed", "{}", DateTimeOffset.UtcNow))); + + await WaitForAsync(good.Task, "the consumer stopped after a poison message"); + } + + [RequiresBrokerManagementFact] + public async Task DeliveryResumes_AfterTheBrokerConnectionIsDropped() + { + var exchange = UniqueExchange(); + var subscriberName = $"session-events-test-subscriber-{Guid.NewGuid():N}"; + + await using var publisher = CreateTransport(exchange); + await using var subscriber = CreateTransport(exchange, subscriberName); + + var beforeDrop = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var afterRecovery = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + await publisher.StartAsync((_, _) => Task.CompletedTask); + await subscriber.StartAsync((_, evt) => + { + if (evt.EventType == "before-drop") beforeDrop.TrySetResult(); + if (evt.EventType == "after-recovery") afterRecovery.TrySetResult(); + return Task.CompletedTask; + }); + + await publisher.BroadcastAsync(Guid.NewGuid(), new SessionEvent("before-drop", "{}", DateTimeOffset.UtcNow)); + await WaitForAsync(beforeDrop.Task, "the subscriber never received the pre-drop event"); + + // Kill the subscriber connection from the broker side. It has to come from there: the + // client treats an application-initiated close as deliberate and does not recover from it, + // so closing it locally would prove nothing. Automatic recovery must now reconnect, + // re-declare the exclusive queue and its binding, and re-attach the consumer - the topology + // recovery this adapter configures explicitly rather than inheriting from client defaults. + await BrokerProbe.ForceCloseConnectionAsync(subscriberName); + + // Recovery polls on NetworkRecoveryInterval, so keep republishing until it lands or we + // give up; a single publish into the gap would be lost with nothing bound to the exchange. + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(60)); + while (!afterRecovery.Task.IsCompleted && !cts.IsCancellationRequested) + { + try + { + await publisher.BroadcastAsync( + Guid.NewGuid(), new SessionEvent("after-recovery", "{}", DateTimeOffset.UtcNow)); + } + catch (Exception) + { + // The publisher may be mid-recovery itself; keep trying until the deadline. + } + + await Task.WhenAny(afterRecovery.Task, Task.Delay(TimeSpan.FromSeconds(1))); + } + + Assert.True( + afterRecovery.Task.IsCompletedSuccessfully, + "delivery did not resume within 60s of the broker connection being dropped"); + } + + private static string UniqueExchange() => $"session-events-test-{Guid.NewGuid():N}"; + + private static async Task WaitForAsync(Task task, string because) + { + var completed = await Task.WhenAny(task, Task.Delay(TimeSpan.FromSeconds(20))); + Assert.True(completed == task, because); + await task; + } + + private sealed class CapturingLogger : ILogger + { + public List Errors { 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) + { + if (logLevel >= LogLevel.Error) + Errors.Add(formatter(state, exception)); + } + } +} diff --git a/src/backend/OpenOnboarding.Application.Tests/RateLimitPartitioningTests.cs b/src/backend/OpenOnboarding.Application.Tests/RateLimitPartitioningTests.cs new file mode 100644 index 0000000..e005d47 --- /dev/null +++ b/src/backend/OpenOnboarding.Application.Tests/RateLimitPartitioningTests.cs @@ -0,0 +1,445 @@ +using System.Net; +using System.Net.Http.Headers; +using System.Net.Http.Json; +using System.Security.Claims; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Testing; +using OpenOnboarding.Api.Authentication; +using OpenOnboarding.Api.RateLimiting; +using OpenOnboarding.Application.Tests.TestHelpers; + +namespace OpenOnboarding.Application.Tests; + +/// +/// A rate limit is only a defence if it bounds the caller who is misbehaving. When every caller +/// shares one bucket the limit becomes the attack: 100 requests a minute to session-start +/// stops every applicant from beginning a journey. +/// +/// These run in the Development environment because the Testing environment swaps every limiter for +/// a no-op. +/// +/// +public sealed class RateLimitPartitioningTests +{ + private const int Limit = 3; + + private const string ClientA = "198.51.100.10"; + private const string ClientB = "198.51.100.20"; + private const string TrustedProxy = "10.1.2.3"; + private const string UntrustedHop = "203.0.113.77"; + + private sealed record StartResponse(Guid SessionId, string? ApplicantToken); + private sealed record FlowStub(Guid Id); + + private static WebApplicationFactory CreateFactory( + IReadOnlyDictionary? extraSettings = null) + { + var settings = new Dictionary + { + ["RateLimiting:SessionStartPerMinute"] = Limit.ToString(), + ["RateLimiting:AnalyticsIngestPerMinute"] = Limit.ToString(), + ["RateLimiting:WebhookRegistrationPerMinute"] = Limit.ToString(), + ["RateLimiting:GeneralPerMinute"] = "1000", + // High enough to stay out of the way of the per-partition assertions; the ceiling gets + // its own test. + ["RateLimiting:GlobalCeilingPerMinute"] = "1000" + }; + + if (extraSettings is not null) + { + foreach (var setting in extraSettings) + settings[setting.Key] = setting.Value; + } + + return TestWebAppFactory.Create(configurationOverrides: settings, environment: "Development"); + } + + private static HttpRequestMessage StartRequest(Guid flowId, string clientAddress, string? forwardedFor = null) + { + var request = new HttpRequestMessage(HttpMethod.Post, "/api/workflow/sessions/start") + { + Content = JsonContent.Create(new { flowId }) + }; + request.Headers.Add(ClientAddressStartupFilter.HeaderName, clientAddress); + + if (forwardedFor is not null) + request.Headers.Add("X-Forwarded-For", forwardedFor); + + return request; + } + + private static async Task CreateFlowAsync(HttpClient operatorClient) + { + var payload = new + { + name = $"Partition 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 ExhaustSessionStartAsync( + HttpClient client, Guid flowId, string clientAddress, string? forwardedFor = null) + { + for (var i = 0; i < Limit; i++) + { + var response = await client.SendAsync(StartRequest(flowId, clientAddress, forwardedFor)); + Assert.NotEqual(HttpStatusCode.TooManyRequests, response.StatusCode); + } + + var overTheLimit = await client.SendAsync(StartRequest(flowId, clientAddress, forwardedFor)); + Assert.Equal(HttpStatusCode.TooManyRequests, overTheLimit.StatusCode); + } + + [Fact] + public async Task SessionStart_OneClientExhaustingItsBudget_DoesNotLockOutAnother() + { + using var factory = CreateFactory(); + using var operatorClient = factory.CreateClient(); + operatorClient.DefaultRequestHeaders.Add("X-Api-Key", "test-api-key"); + var flowId = await CreateFlowAsync(operatorClient); + + using var client = factory.CreateClient(); + await ExhaustSessionStartAsync(client, flowId, ClientA); + + var otherClient = await client.SendAsync(StartRequest(flowId, ClientB)); + + Assert.NotEqual(HttpStatusCode.TooManyRequests, otherClient.StatusCode); + } + + [Fact] + public async Task SessionStart_EachClient_ReceivesItsFullPermitLimit() + { + using var factory = CreateFactory(); + using var operatorClient = factory.CreateClient(); + operatorClient.DefaultRequestHeaders.Add("X-Api-Key", "test-api-key"); + var flowId = await CreateFlowAsync(operatorClient); + + using var client = factory.CreateClient(); + await ExhaustSessionStartAsync(client, flowId, ClientA); + + // The second client starts from a full budget, not from whatever the first one left. + await ExhaustSessionStartAsync(client, flowId, ClientB); + } + + [Fact] + public async Task SessionStart_BehindATrustedProxy_PartitionsByTheForwardedClientAddress() + { + using var factory = CreateFactory(new Dictionary + { + ["ForwardedHeaders:KnownProxies:0"] = TrustedProxy + }); + + using var operatorClient = factory.CreateClient(); + operatorClient.DefaultRequestHeaders.Add("X-Api-Key", "test-api-key"); + var flowId = await CreateFlowAsync(operatorClient); + + using var client = factory.CreateClient(); + + // Every request arrives from the proxy; only X-Forwarded-For tells the two callers apart. + await ExhaustSessionStartAsync(client, flowId, TrustedProxy, forwardedFor: ClientA); + + var otherClient = await client.SendAsync(StartRequest(flowId, TrustedProxy, forwardedFor: ClientB)); + + Assert.NotEqual(HttpStatusCode.TooManyRequests, otherClient.StatusCode); + } + + [Fact] + public async Task SessionStart_ForgedForwardedFor_FromAnUntrustedSource_CannotEvadeTheLimit() + { + using var factory = CreateFactory(new Dictionary + { + ["ForwardedHeaders:KnownProxies:0"] = TrustedProxy + }); + + using var operatorClient = factory.CreateClient(); + operatorClient.DefaultRequestHeaders.Add("X-Api-Key", "test-api-key"); + var flowId = await CreateFlowAsync(operatorClient); + + using var client = factory.CreateClient(); + await ExhaustSessionStartAsync(client, flowId, UntrustedHop, forwardedFor: ClientA); + + // A fresh header value from the same untrusted connection must not mint a fresh budget. + var rotated = await client.SendAsync(StartRequest(flowId, UntrustedHop, forwardedFor: ClientB)); + + Assert.Equal(HttpStatusCode.TooManyRequests, rotated.StatusCode); + } + + [Fact] + public async Task AnalyticsIngest_IsPartitionedBySession_SoOneApplicantCannotThrottleAnother() + { + using var factory = CreateFactory(new Dictionary + { + ["RateLimiting:SessionStartPerMinute"] = "1000" + }); + + 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 first = await StartSessionAsync(browser, flowId); + var second = await StartSessionAsync(browser, flowId); + + for (var i = 0; i < Limit; i++) + { + var response = await browser.SendAsync(IngestRequest(first, flowId)); + Assert.Equal(HttpStatusCode.Accepted, response.StatusCode); + } + + var exhausted = await browser.SendAsync(IngestRequest(first, flowId)); + Assert.Equal(HttpStatusCode.TooManyRequests, exhausted.StatusCode); + + // The second applicant shares the client address but not the session. + var otherApplicant = await browser.SendAsync(IngestRequest(second, flowId)); + Assert.Equal(HttpStatusCode.Accepted, otherApplicant.StatusCode); + } + + [Fact] + public async Task WebhookRegistration_ExhaustedByOnePrincipal_StillAdmitsADifferentPartition() + { + using var factory = CreateFactory(); + using var client = factory.CreateClient(); + + for (var i = 0; i < Limit + 1; i++) + await client.SendAsync(WebhookRequest(ClientA, apiKey: "test-api-key")); + + var limited = await client.SendAsync(WebhookRequest(ClientA, apiKey: "test-api-key")); + Assert.Equal(HttpStatusCode.TooManyRequests, limited.StatusCode); + + // An anonymous caller from another address falls in a different partition, so it reaches + // authorization and is refused there - not by the exhausted operator limiter. + var anonymous = await client.SendAsync(WebhookRequest(ClientB, apiKey: null)); + + Assert.NotEqual(HttpStatusCode.TooManyRequests, anonymous.StatusCode); + } + + [Fact] + public async Task General_ExhaustedByOneCaller_StillAdmitsADifferentPartition() + { + // The fallback policy covers every controller endpoint that does not name a tighter one, + // which is most of the API. Partitioning matters here for the same reason it does on + // session-start: one caller must not be able to close the API to everyone else. + using var factory = CreateFactory(new Dictionary + { + ["RateLimiting:GeneralPerMinute"] = Limit.ToString() + }); + + using var client = factory.CreateClient(); + + for (var i = 0; i < Limit + 1; i++) + await client.SendAsync(GeneralRequest(ClientA)); + + var limited = await client.SendAsync(GeneralRequest(ClientA)); + Assert.Equal(HttpStatusCode.TooManyRequests, limited.StatusCode); + + // A different caller reaches authorization and is refused there, not by the exhausted + // caller's limiter. + var other = await client.SendAsync(GeneralRequest(ClientB)); + + Assert.NotEqual(HttpStatusCode.TooManyRequests, other.StatusCode); + } + + [Fact] + public async Task AnEndpointWithItsOwnPolicy_IsNotAlsoBoundByTheGeneralFallback() + { + // Attaching "general" to every controller must not silently stack a second limiter onto the + // endpoints that already declare one - session-start would then be capped by whichever of + // the two is tighter, which is not what either setting says. + using var factory = CreateFactory(new Dictionary + { + ["RateLimiting:GeneralPerMinute"] = "2", + ["RateLimiting:SessionStartPerMinute"] = "1000" + }); + + using var operatorClient = factory.CreateClient(); + operatorClient.DefaultRequestHeaders.Add("X-Api-Key", "test-api-key"); + var flowId = await CreateFlowAsync(operatorClient); + + using var client = factory.CreateClient(); + + for (var i = 0; i < 6; i++) + { + var response = await client.SendAsync(StartRequest(flowId, ClientA)); + Assert.NotEqual(HttpStatusCode.TooManyRequests, response.StatusCode); + } + } + + [Fact] + public async Task TheGeneralFallback_DoesNotApplyToHealthChecks() + { + // Health checks are what a load balancer polls, often several times a second. Rate limiting + // them turns a traffic spike into an instance being marked unhealthy and pulled out. + using var factory = CreateFactory(new Dictionary + { + ["RateLimiting:GeneralPerMinute"] = "2" + }); + + using var client = factory.CreateClient(); + + for (var i = 0; i < 6; i++) + { + var request = new HttpRequestMessage(HttpMethod.Get, "/health/live"); + request.Headers.Add(ClientAddressStartupFilter.HeaderName, ClientA); + var response = await client.SendAsync(request); + + Assert.NotEqual(HttpStatusCode.TooManyRequests, response.StatusCode); + } + } + + [Fact] + public async Task GlobalCeiling_BoundsAFloodSpreadAcrossManyPartitions() + { + const int ceiling = 5; + + using var factory = CreateFactory(new Dictionary + { + // Per-caller budgets far above the ceiling, so only the ceiling can reject. + ["RateLimiting:SessionStartPerMinute"] = "1000", + ["RateLimiting:GlobalCeilingPerMinute"] = ceiling.ToString() + }); + + using var client = factory.CreateClient(); + + var statuses = new List(); + for (var i = 0; i < ceiling + 3; i++) + { + // A different caller each time: nothing but the global ceiling is shared. + var response = await client.SendAsync(StartRequest(Guid.NewGuid(), $"198.51.100.{100 + i}")); + statuses.Add(response.StatusCode); + } + + Assert.Contains(HttpStatusCode.TooManyRequests, statuses); + } + + [Theory] + [InlineData("session-start")] + [InlineData("analytics-ingest")] + [InlineData("webhook-registration")] + [InlineData("general")] + public void EveryPolicy_DerivesADistinctKey_ForTwoDifferentCallers(string policy) + { + Func partitioner = policy switch + { + "session-start" => RateLimitPartitionKeys.ClientAddress, + "analytics-ingest" => RateLimitPartitionKeys.ApplicantSession, + _ => RateLimitPartitionKeys.Principal + }; + + var first = partitioner(CallerContext(ClientA, Guid.NewGuid(), "operator-one")); + var second = partitioner(CallerContext(ClientB, Guid.NewGuid(), "operator-two")); + + Assert.NotEqual(first, second); + } + + [Fact] + public void PartitionKeys_NeverCollideAcrossKinds() + { + // A principal id that reads exactly like an address must not spend the address budget. + var context = CallerContext(ClientA, sessionId: null, principal: ClientA); + + Assert.NotEqual(RateLimitPartitionKeys.ClientAddress(context), RateLimitPartitionKeys.Principal(context)); + } + + [Fact] + public void ApplicantSession_WithoutASessionClaim_FallsBackToThePrincipal() + { + var context = CallerContext(ClientA, sessionId: null, principal: "operator-one"); + + Assert.Equal( + RateLimitPartitionKeys.Principal(context), + RateLimitPartitionKeys.ApplicantSession(context)); + } + + [Fact] + public void Principal_WithoutACredential_FallsBackToTheClientAddress() + { + var context = CallerContext(ClientA, sessionId: null, principal: null); + + Assert.Equal( + RateLimitPartitionKeys.ClientAddress(context), + RateLimitPartitionKeys.Principal(context)); + } + + /// + /// A request to an endpoint that declares no policy of its own, so it falls to the "general" + /// fallback. Sent anonymously: the limiter runs before authorization, so an unauthenticated + /// caller still spends its partition's budget and a refusal tells the two apart by status. + /// + private static HttpRequestMessage GeneralRequest(string clientAddress) + { + var request = new HttpRequestMessage(HttpMethod.Get, "/api/flows"); + request.Headers.Add(ClientAddressStartupFilter.HeaderName, clientAddress); + return request; + } + + private static HttpRequestMessage WebhookRequest(string clientAddress, string? apiKey) + { + var request = new HttpRequestMessage(HttpMethod.Post, $"/api/flows/{Guid.NewGuid()}/webhooks") + { + Content = JsonContent.Create(new + { + url = "https://example.com/hook", + secret = "a-secret-long-enough-to-pass" + }) + }; + request.Headers.Add(ClientAddressStartupFilter.HeaderName, clientAddress); + + if (apiKey is not null) + request.Headers.Add("X-Api-Key", apiKey); + + return request; + } + + private static HttpContext CallerContext(string address, Guid? sessionId, string? principal) + { + var claims = new List(); + + if (principal is not null) + claims.Add(new Claim(ClaimTypes.NameIdentifier, principal)); + + if (sessionId is not null) + claims.Add(new Claim(ApplicantSessionAuthenticationDefaults.SessionIdClaim, sessionId.Value.ToString())); + + var context = new DefaultHttpContext + { + User = new ClaimsPrincipal(new ClaimsIdentity(claims, "test")) + }; + context.Connection.RemoteIpAddress = IPAddress.Parse(address); + return context; + } + + private static async Task StartSessionAsync(HttpClient client, Guid flowId) + { + var response = await client.SendAsync(StartRequest(flowId, ClientA)); + response.EnsureSuccessStatusCode(); + return (await response.Content.ReadFromJsonAsync())!; + } + + private static HttpRequestMessage IngestRequest(StartResponse session, Guid flowId) + { + var request = new HttpRequestMessage(HttpMethod.Post, "/api/analytics/events"); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", session.ApplicantToken); + request.Headers.Add(ClientAddressStartupFilter.HeaderName, ClientA); + request.Content = JsonContent.Create(new + { + events = new[] + { + new + { + eventType = "step_view", + journeyId = flowId.ToString(), + sessionId = session.SessionId.ToString() + } + } + }); + return request; + } +} diff --git a/src/backend/OpenOnboarding.Application.Tests/TestHelpers/ClientAddressStartupFilter.cs b/src/backend/OpenOnboarding.Application.Tests/TestHelpers/ClientAddressStartupFilter.cs new file mode 100644 index 0000000..3d07084 --- /dev/null +++ b/src/backend/OpenOnboarding.Application.Tests/TestHelpers/ClientAddressStartupFilter.cs @@ -0,0 +1,38 @@ +using System.Net; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; + +namespace OpenOnboarding.Application.Tests.TestHelpers; + +/// +/// Gives a test control over HttpContext.Connection.RemoteIpAddress, which +/// otherwise leaves null. +/// +/// Rate limit partitioning and the forwarded-headers trust check both read the connection address, +/// so a test cannot pose as two different callers without setting it. The middleware runs ahead of +/// everything in Program.cs - including UseForwardedHeaders - so the address it sets +/// is the one the proxy trust check sees. +/// +/// +internal sealed class ClientAddressStartupFilter : IStartupFilter +{ + /// Header a test sets to choose the address the request appears to come from. + public const string HeaderName = "X-Test-Client-Address"; + + public Action Configure(Action next) + => app => + { + app.Use(async (context, nextMiddleware) => + { + if (context.Request.Headers.TryGetValue(HeaderName, out var address) + && IPAddress.TryParse(address.ToString(), out var parsed)) + { + context.Connection.RemoteIpAddress = parsed; + } + + await nextMiddleware(); + }); + + next(app); + }; +} diff --git a/src/backend/OpenOnboarding.Application.Tests/TestHelpers/RequiresBrokerFactAttribute.cs b/src/backend/OpenOnboarding.Application.Tests/TestHelpers/RequiresBrokerFactAttribute.cs new file mode 100644 index 0000000..179a220 --- /dev/null +++ b/src/backend/OpenOnboarding.Application.Tests/TestHelpers/RequiresBrokerFactAttribute.cs @@ -0,0 +1,159 @@ +using System.Net.Http.Headers; +using System.Net.Sockets; +using System.Text; +using System.Text.Json; + +namespace OpenOnboarding.Application.Tests.TestHelpers; + +/// +/// A that skips itself when no RabbitMQ broker is reachable. +/// +/// The same bargain the git hooks make: dotnet test must pass on any machine with the SDKs +/// installed, while the suites that need real infrastructure still run where it exists - CI, or a +/// developer with docker compose up. Skipping is decided at discovery so the reachability +/// probe runs once per test class rather than once per test. +/// +/// +public sealed class RequiresBrokerFactAttribute : FactAttribute +{ + public RequiresBrokerFactAttribute() + { + if (!BrokerProbe.IsReachable && !BrokerProbe.BrokerIsRequired) + Skip = BrokerProbe.SkipReason; + } +} + +/// +/// As , but also needs the broker's management HTTP API. +/// +/// Only the connection-recovery test needs it: a dropped connection has to be dropped by the +/// broker, because the client treats an application-initiated close as deliberate and does not +/// recover from one. The management API is how you ask the broker to do that. +/// +/// +public sealed class RequiresBrokerManagementFactAttribute : FactAttribute +{ + public RequiresBrokerManagementFactAttribute() + { + if (BrokerProbe.BrokerIsRequired) + return; + + if (!BrokerProbe.IsReachable) + Skip = BrokerProbe.SkipReason; + else if (!BrokerProbe.IsManagementReachable) + Skip = BrokerProbe.ManagementSkipReason; + } +} + +/// +/// Resolves the broker to test against and reports whether it answers. +/// +public static class BrokerProbe +{ + /// Environment variable naming the broker, so CI can point at its service container. + public const string UriVariable = "SESSIONEVENTS__RABBITMQ__URI"; + + /// Port the management plugin listens on. Only the recovery test needs it. + public const int ManagementPort = 15672; + + /// + /// Set where a broker is guaranteed to exist - CI, which declares one as a service container. + /// There, a missing broker must fail the build rather than skip: a silently skipped test is a + /// green build that proved nothing, which is how this adapter came to have no coverage at all. + /// + public const string RequiredVariable = "REQUIRE_BROKER_TESTS"; + + /// Whether a missing broker should fail instead of skip. + public static bool BrokerIsRequired => + Environment.GetEnvironmentVariable(RequiredVariable) is "1" or "true"; + + private static readonly Lazy Reachable = new(() => Probe(AmqpPort), isThreadSafe: true); + private static readonly Lazy ManagementReachable = + new(() => Probe(ManagementPort), isThreadSafe: true); + + private const int AmqpPort = 5672; + + /// The broker URI under test, defaulting to a local one. + public static string BrokerUri => + Environment.GetEnvironmentVariable(UriVariable) is { Length: > 0 } configured + ? configured + : "amqp://guest:guest@localhost:5672/"; + + public static bool IsReachable => Reachable.Value; + + public static bool IsManagementReachable => ManagementReachable.Value; + + public static string SkipReason => + $"No RabbitMQ broker reachable at {BrokerUri}. Start one (docker compose up rabbitmq) " + + $"or set {UriVariable} to run this test."; + + public static string ManagementSkipReason => + $"The RabbitMQ broker at {BrokerUri} has no management API on port {ManagementPort}. " + + "Run an image that includes it (rabbitmq:3-management) to run this test."; + + /// + /// Asks the broker to close the connection with the given client-provided name, so the client + /// sees a failure and runs its recovery path. + /// + public static async Task ForceCloseConnectionAsync(string clientProvidedName) + { + var broker = new Uri(BrokerUri); + var credentials = broker.UserInfo.Split(':', 2); + + using var http = new HttpClient + { + BaseAddress = new UriBuilder("http", broker.Host, ManagementPort).Uri, + Timeout = TimeSpan.FromSeconds(10) + }; + http.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue( + "Basic", + Convert.ToBase64String(Encoding.UTF8.GetBytes( + $"{Uri.UnescapeDataString(credentials.ElementAtOrDefault(0) ?? "guest")}:" + + $"{Uri.UnescapeDataString(credentials.ElementAtOrDefault(1) ?? "guest")}"))); + + // The connection may take a moment to register in the management database. + var deadline = DateTime.UtcNow.AddSeconds(20); + while (DateTime.UtcNow < deadline) + { + using var listing = await http.GetAsync("/api/connections"); + listing.EnsureSuccessStatusCode(); + + using var document = JsonDocument.Parse(await listing.Content.ReadAsStringAsync()); + var match = document.RootElement.EnumerateArray().FirstOrDefault(connection => + connection.TryGetProperty("client_properties", out var properties) + && properties.TryGetProperty("connection_name", out var name) + && name.GetString() == clientProvidedName); + + if (match.ValueKind == JsonValueKind.Object) + { + var id = match.GetProperty("name").GetString()!; + using var delete = await http.DeleteAsync($"/api/connections/{Uri.EscapeDataString(id)}"); + delete.EnsureSuccessStatusCode(); + return; + } + + await Task.Delay(TimeSpan.FromSeconds(1)); + } + + throw new InvalidOperationException( + $"The broker never reported a connection named {clientProvidedName}."); + } + + private static bool Probe(int defaultPort) + { + try + { + var uri = new Uri(BrokerUri); + var port = defaultPort == AmqpPort && uri.Port > 0 ? uri.Port : defaultPort; + + using var client = new TcpClient(); + // Short: a missing broker is the normal case locally and must not slow the suite down. + return client.ConnectAsync(uri.Host, port).Wait(TimeSpan.FromSeconds(2)) + && client.Connected; + } + catch + { + return false; + } + } +} diff --git a/src/backend/OpenOnboarding.Application.Tests/TestWebAppFactory.cs b/src/backend/OpenOnboarding.Application.Tests/TestWebAppFactory.cs index bfaacd8..7a11130 100644 --- a/src/backend/OpenOnboarding.Application.Tests/TestWebAppFactory.cs +++ b/src/backend/OpenOnboarding.Application.Tests/TestWebAppFactory.cs @@ -5,6 +5,8 @@ using Microsoft.EntityFrameworkCore.Storage; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using OpenOnboarding.Application.Tests.TestHelpers; using OpenOnboarding.Infrastructure.Persistence; namespace OpenOnboarding.Application.Tests; @@ -54,6 +56,11 @@ public static WebApplicationFactory Create( }); builder.ConfigureTestServices(services => { + // Lets a test choose the address a request appears to come from; without it + // TestServer leaves RemoteIpAddress null and every caller shares one rate limit + // partition. Inert unless the test sends the header. + services.AddSingleton(); + // EF Core 10 uses TryAdd for IDbContextOptionsConfiguration, so the Npgsql // registration persists unless we explicitly remove it before adding the InMemory one. // Remove all DbContext-related registrations for OnboardingDbContext. diff --git a/src/backend/OpenOnboarding.Application/Contracts/SessionStepResponse.cs b/src/backend/OpenOnboarding.Application/Contracts/SessionStepResponse.cs index d08aa1b..44573b7 100644 --- a/src/backend/OpenOnboarding.Application/Contracts/SessionStepResponse.cs +++ b/src/backend/OpenOnboarding.Application/Contracts/SessionStepResponse.cs @@ -21,4 +21,20 @@ public sealed class SessionStepResponse /// null when the session is completed. /// public NodeDto? CurrentNode { get; set; } + + /// + /// A replacement applicant token, extending the credential for a journey still in progress. + /// + /// Set on step submission for applicant callers only. Without it a compliance-heavy journey + /// left open past the token lifetime returns to a live session and a credential the API no + /// longer accepts, with no way forward but to start again and lose everything entered. + /// + /// null when the caller is an operator, whose own credential already covers the session, + /// and on responses that do not renew - session start carries its token on + /// instead. + /// + public string? ApplicantToken { get; set; } + + /// When stops being accepted. + public DateTimeOffset? ApplicantTokenExpiresAt { get; set; } } diff --git a/src/backend/OpenOnboarding.Infrastructure/EventBus/RabbitMqSessionEventTransport.cs b/src/backend/OpenOnboarding.Infrastructure/EventBus/RabbitMqSessionEventTransport.cs index 457fdff..be6b355 100644 --- a/src/backend/OpenOnboarding.Infrastructure/EventBus/RabbitMqSessionEventTransport.cs +++ b/src/backend/OpenOnboarding.Infrastructure/EventBus/RabbitMqSessionEventTransport.cs @@ -21,19 +21,27 @@ public sealed class RabbitMqSessionEventTransport : ISessionEventTransport private readonly string _connectionString; private readonly string _exchangeName; + private readonly string _clientProvidedName; private readonly ILogger _logger; private IConnection? _connection; private IChannel? _channel; + /// + /// Name this connection reports to the broker. Defaults to one naming the host, so the + /// management UI identifies which replica owns which queue - without it every replica shows up + /// as an anonymous connection and a stuck consumer cannot be traced back to an instance. + /// public RabbitMqSessionEventTransport( string connectionString, string exchangeName, - ILogger logger) + ILogger logger, + string? clientProvidedName = null) { _connectionString = connectionString; _exchangeName = exchangeName; _logger = logger; + _clientProvidedName = clientProvidedName ?? $"open-onboarding-session-events:{Environment.MachineName}"; } /// Wire envelope. Kept separate from so the session id travels with it. @@ -51,9 +59,48 @@ internal static byte[] Serialize(Guid sessionId, SessionEvent evt) internal static Envelope? Deserialize(ReadOnlySpan body) => JsonSerializer.Deserialize(body, JsonOptions); + /// + /// Hands one received message to , swallowing anything it or the + /// deserialiser throws. + /// + /// A single malformed or unhandleable message must never tear down the consumer: this channel + /// carries every session on this instance, so one poison body would silently stop progress + /// reaching every open SSE stream - the exact failure the distributed transport exists to + /// prevent. Extracted from the consumer callback so it can be tested without a broker. + /// + /// + internal static async Task DeliverAsync( + ReadOnlyMemory body, + Func handler, + ILogger logger) + { + try + { + var envelope = Deserialize(body.Span); + if (envelope is not null) + await handler(envelope.SessionId, envelope.ToSessionEvent()); + } + catch (Exception ex) + { + logger.LogError(ex, "Failed to deliver a session event received from RabbitMQ."); + } + } + public async Task StartAsync(Func handler, CancellationToken cancellationToken = default) { - var factory = new ConnectionFactory { Uri = new Uri(_connectionString) }; + // Recovery is stated rather than inherited from the client defaults. On a dropped + // connection the client reconnects and replays the topology - the exclusive queue, its + // binding and the consumer - so delivery resumes without this instance restarting. Left + // implicit, a future client default could turn this off and the only symptom would be SSE + // streams that stay open and deliver nothing. + var factory = new ConnectionFactory + { + Uri = new Uri(_connectionString), + AutomaticRecoveryEnabled = true, + TopologyRecoveryEnabled = true, + NetworkRecoveryInterval = TimeSpan.FromSeconds(5), + ClientProvidedName = _clientProvidedName + }; _connection = await factory.CreateConnectionAsync(cancellationToken); _channel = await _connection.CreateChannelAsync(cancellationToken: cancellationToken); @@ -71,21 +118,7 @@ await _channel.QueueBindAsync( 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."); - } - }; + consumer.ReceivedAsync += (_, args) => DeliverAsync(args.Body, handler, _logger); await _channel.BasicConsumeAsync( queue.QueueName, autoAck: true, consumer, cancellationToken: cancellationToken); diff --git a/src/frontend/src/onboarding/ApplicantJourneyPage.tsx b/src/frontend/src/onboarding/ApplicantJourneyPage.tsx index 4a8b831..b5f0565 100644 --- a/src/frontend/src/onboarding/ApplicantJourneyPage.tsx +++ b/src/frontend/src/onboarding/ApplicantJourneyPage.tsx @@ -16,7 +16,7 @@ import { defaultFlowId } from '../journeys' * 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 { step, startSession, submitStep, isLoading, error, isExpired } = useOnboarding() const [visitedNodeIds, setVisitedNodeIds] = useState>(new Set()) const flowId = new URLSearchParams(search).get('flowId') ?? defaultFlowId @@ -63,7 +63,24 @@ export function ApplicantJourneyPage({ search }: { search: string }) {
- {error ? ( + {isExpired ? ( + // Shown instead of the error banner, not alongside it: an expired credential is not a + // fault to retry past, and the applicant needs the one instruction that works. +
+

Your session has expired.

+

+ For your security, applications left open too long are closed. Please start again. +

+ +
+ ) : null} + {error && !isExpired ? (

{error}

diff --git a/src/frontend/src/onboarding/api/applicant-token-lifecycle.test.ts b/src/frontend/src/onboarding/api/applicant-token-lifecycle.test.ts new file mode 100644 index 0000000..ff5b895 --- /dev/null +++ b/src/frontend/src/onboarding/api/applicant-token-lifecycle.test.ts @@ -0,0 +1,157 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { ExpiredSessionError, getNextStep, startSession, submitStep } from './workflow-api-client' +import { clearApplicantToken, getApplicantToken } from './applicant-session' + +/** + * The applicant credential is issued once at session start and renewed on each step submission. Two + * behaviours matter to the browser: it has to store the renewal, and it has to tell an expired + * credential apart from a server fault — one is a dead end the applicant must be told about, the + * other is a failure they can retry. + */ +describe('applicant token lifecycle', () => { + beforeEach(() => { + clearApplicantToken() + }) + + afterEach(() => { + vi.restoreAllMocks() + clearApplicantToken() + }) + + const jsonResponse = (status: number, body: unknown) => ({ + ok: status >= 200 && status < 300, + status, + json: async () => body, + } as Response) + + describe('renewal', () => { + it('stores the token returned by session start', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValue(jsonResponse(200, { + sessionId: 'session-1', + isCompleted: false, + currentNode: null, + applicantToken: 'token-from-start', + })) + + await startSession('', { flowId: 'flow-1' }) + + expect(getApplicantToken()).toBe('token-from-start') + }) + + it('replaces the stored token with the one a step submission returns', async () => { + vi.spyOn(globalThis, 'fetch') + .mockResolvedValueOnce(jsonResponse(200, { + sessionId: 'session-1', + isCompleted: false, + currentNode: null, + applicantToken: 'token-from-start', + })) + .mockResolvedValueOnce(jsonResponse(200, { + sessionId: 'session-1', + isCompleted: false, + currentNode: null, + applicantToken: 'refreshed-token', + })) + + await startSession('', { flowId: 'flow-1' }) + await submitStep('', 'session-1', 'node-1', { payload: {} }) + + // This is what keeps a long journey alive: without it the browser keeps presenting the + // original credential until it expires mid-application. + expect(getApplicantToken()).toBe('refreshed-token') + }) + + it('sends the refreshed token on the next request', async () => { + const fetchSpy = vi.spyOn(globalThis, 'fetch') + .mockResolvedValueOnce(jsonResponse(200, { + sessionId: 'session-1', + isCompleted: false, + currentNode: null, + applicantToken: 'token-from-start', + })) + .mockResolvedValueOnce(jsonResponse(200, { + sessionId: 'session-1', + isCompleted: false, + currentNode: null, + applicantToken: 'refreshed-token', + })) + .mockResolvedValueOnce(jsonResponse(200, { + sessionId: 'session-1', + isCompleted: false, + currentNode: null, + })) + + await startSession('', { flowId: 'flow-1' }) + await submitStep('', 'session-1', 'node-1', { payload: {} }) + await getNextStep('', 'session-1') + + const lastCall = fetchSpy.mock.calls.at(-1)! + const headers = (lastCall[1] as RequestInit).headers as Record + expect(headers.Authorization).toBe('Bearer refreshed-token') + }) + + it('keeps the stored token when a response carries no renewal', async () => { + // An operator-submitted step returns none; the browser must not wipe what it holds. + vi.spyOn(globalThis, 'fetch') + .mockResolvedValueOnce(jsonResponse(200, { + sessionId: 'session-1', + isCompleted: false, + currentNode: null, + applicantToken: 'token-from-start', + })) + .mockResolvedValueOnce(jsonResponse(200, { + sessionId: 'session-1', + isCompleted: false, + currentNode: null, + })) + + await startSession('', { flowId: 'flow-1' }) + await submitStep('', 'session-1', 'node-1', { payload: {} }) + + expect(getApplicantToken()).toBe('token-from-start') + }) + }) + + describe('expired credentials', () => { + it('raises ExpiredSessionError when a step submission is refused with 401', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValue(jsonResponse(401, {})) + + await expect(submitStep('', 'session-1', 'node-1', { payload: {} })) + .rejects.toBeInstanceOf(ExpiredSessionError) + }) + + it('raises ExpiredSessionError when the session has reached a terminal status', async () => { + // 403 on a session this browser was completing means its token is no longer accepted for + // writes. From the applicant side that is the same dead end as an expiry. + vi.spyOn(globalThis, 'fetch').mockResolvedValue(jsonResponse(403, {})) + + await expect(submitStep('', 'session-1', 'node-1', { payload: {} })) + .rejects.toBeInstanceOf(ExpiredSessionError) + }) + + it('raises ExpiredSessionError when reading the next step is refused with 401', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValue(jsonResponse(401, {})) + + await expect(getNextStep('', 'session-1')).rejects.toBeInstanceOf(ExpiredSessionError) + }) + + it('carries a message the applicant can act on', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValue(jsonResponse(401, {})) + + const error = await submitStep('', 'session-1', 'node-1', { payload: {} }).catch((e) => e) + + expect(error.message).toMatch(/expired/i) + }) + + it('leaves a server fault as a generic error, not an expiry', async () => { + // A 500 is a failure the applicant can retry past; telling them to start again would throw + // away a journey that is still perfectly valid. + vi.spyOn(globalThis, 'fetch').mockResolvedValue(jsonResponse(500, {})) + + const error = await submitStep('', 'session-1', 'node-1', { payload: {} }).catch((e) => e) + + expect(error).not.toBeInstanceOf(ExpiredSessionError) + expect(error).toBeInstanceOf(Error) + }) + }) +}) diff --git a/src/frontend/src/onboarding/api/workflow-api-client.ts b/src/frontend/src/onboarding/api/workflow-api-client.ts index 001a1d6..189ea12 100644 --- a/src/frontend/src/onboarding/api/workflow-api-client.ts +++ b/src/frontend/src/onboarding/api/workflow-api-client.ts @@ -7,6 +7,20 @@ export interface ComplianceViolation { ruleId?: string } +/** + * The applicant credential is no longer accepted — it expired, or its session ended. + * + * Distinct from a generic failure on purpose. An expired credential is not a fault the applicant + * can retry past: the only way forward is to start again, and the page has to say so rather than + * showing a server-error banner over a journey that will never resume. + */ +export class ExpiredSessionError extends Error { + constructor() { + super('Your session has expired. Please start again.') + this.name = 'ExpiredSessionError' + } +} + export class ComplianceError extends Error { readonly violations: ComplianceViolation[] @@ -51,11 +65,22 @@ export async function startSession( if (!res.ok) throw new Error(`startSession failed with status ${res.status}`) const session = await res.json() as SessionStartResponse - if (session.applicantToken) setApplicantToken(session.applicantToken) + captureRefreshedToken(session) return session } +/** + * Stores a credential the API handed back, if it handed one back. + * + * Session start always carries one; step submission carries a renewal, which is what keeps a long + * journey from outliving its own token. An operator-submitted step carries none, and the stored + * credential is left alone. + */ +function captureRefreshedToken(response: SessionStepResponse): void { + if (response.applicantToken) setApplicantToken(response.applicantToken) +} + export async function submitStep( baseUrl: string, sessionId: string, @@ -75,8 +100,24 @@ export async function submitStep( const problem = await res.json() as { violations?: ComplianceViolation[] } throw new ComplianceError(problem.violations ?? []) } + if (isCredentialRejection(res.status)) throw new ExpiredSessionError() if (!res.ok) throw new Error(`submitStep failed with status ${res.status}`) - return res.json() as Promise + + const next = await res.json() as SessionStepResponse + captureRefreshedToken(next) + + return next +} + +/** + * Whether a status means the credential is the problem. + * + * 401 is an expired or unreadable token. 403 on a session the browser was completing means the + * session reached a terminal status, after which its own token is refused for writes — from the + * applicant's side the two are the same dead end. + */ +function isCredentialRejection(status: number): boolean { + return status === 401 || status === 403 } export async function getNextStep( @@ -88,6 +129,7 @@ export async function getNextStep( method: 'GET', headers: buildHeaders(), }) + if (res.status === 401) throw new ExpiredSessionError() if (!res.ok) throw new Error(`getNextStep failed with status ${res.status}`) return res.json() as Promise } diff --git a/src/frontend/src/onboarding/hooks/useOnboarding.test.ts b/src/frontend/src/onboarding/hooks/useOnboarding.test.ts index 556fc50..1c1c9db 100644 --- a/src/frontend/src/onboarding/hooks/useOnboarding.test.ts +++ b/src/frontend/src/onboarding/hooks/useOnboarding.test.ts @@ -14,6 +14,12 @@ vi.mock('../api/workflow-api-client', () => ({ this.violations = violations } }, + ExpiredSessionError: class ExpiredSessionError extends Error { + constructor() { + super('Your session has expired. Please start again.') + this.name = 'ExpiredSessionError' + } + }, })) vi.mock('./session-event-source', () => ({ @@ -113,4 +119,84 @@ describe('useOnboarding', () => { expect(result.current.error).toBeNull() }) + + // ── Expired credentials ──────────────────────────────────────────────────── + + it('flags an expired credential apart from a generic error', async () => { + const { ExpiredSessionError } = await import('../api/workflow-api-client') + mockApiSubmitStep.mockRejectedValueOnce(new ExpiredSessionError()) + + const { result } = renderHook(() => useOnboarding()) + + await act(async () => { + try { + await result.current.submitStep('session-abc', 'node-1', { payload: {} }) + } catch { + // expected + } + }) + + expect(result.current.isExpired).toBe(true) + }) + + it('leaves isExpired false for an ordinary failure', async () => { + // A server fault is retryable; telling the applicant to start again would throw away a + // journey that is still valid. + mockApiSubmitStep.mockRejectedValueOnce(new Error('submitStep failed with status 500')) + + const { result } = renderHook(() => useOnboarding()) + + await act(async () => { + try { + await result.current.submitStep('session-abc', 'node-1', { payload: {} }) + } catch { + // expected + } + }) + + expect(result.current.isExpired).toBe(false) + expect(result.current.error).toBe('submitStep failed with status 500') + }) + + it('starts with no expired state', () => { + const { result } = renderHook(() => useOnboarding()) + + expect(result.current.isExpired).toBe(false) + }) + + it('drops the stored credential once the journey completes', async () => { + const { setApplicantToken, getApplicantToken } = await import('../api/applicant-session') + setApplicantToken('token-for-a-finished-journey') + + mockApiSubmitStep.mockResolvedValueOnce({ + sessionId: 'session-abc', + isCompleted: true, + currentNode: null, + }) + + const { result } = renderHook(() => useOnboarding()) + + await act(async () => { + await result.current.submitStep('session-abc', 'node-1', { payload: {} }) + }) + + // Nothing left to authorise, and a credential left in sessionStorage outlives the visit on a + // shared machine. + expect(getApplicantToken()).toBeNull() + }) + + it('keeps the credential while the journey is still in progress', async () => { + const { setApplicantToken, getApplicantToken } = await import('../api/applicant-session') + setApplicantToken('token-mid-journey') + + mockApiSubmitStep.mockResolvedValueOnce(makeStep()) + + const { result } = renderHook(() => useOnboarding()) + + await act(async () => { + await result.current.submitStep('session-abc', 'node-1', { payload: {} }) + }) + + expect(getApplicantToken()).toBe('token-mid-journey') + }) }) diff --git a/src/frontend/src/onboarding/hooks/useOnboarding.ts b/src/frontend/src/onboarding/hooks/useOnboarding.ts index 1dc813f..8a4f196 100644 --- a/src/frontend/src/onboarding/hooks/useOnboarding.ts +++ b/src/frontend/src/onboarding/hooks/useOnboarding.ts @@ -6,9 +6,10 @@ import { getNextStep as apiGetNextStep, resolveWorkflowApiBase, ComplianceError, + ExpiredSessionError, } from '../api/workflow-api-client' import { createSessionEventSource } from './session-event-source' -import { withApplicantTokenQuery } from '../api/applicant-session' +import { clearApplicantToken, withApplicantTokenQuery } from '../api/applicant-session' const serverBase = (import.meta.env.VITE_API_BASE_URL ?? '').replace(/\/$/, '') const workflowApiBase = resolveWorkflowApiBase(serverBase) @@ -18,6 +19,9 @@ export function useOnboarding() { const [isLoading, setIsLoading] = useState(false) const [error, setError] = useState(null) const [isCompleted, setIsCompleted] = useState(false) + // Tracked apart from `error`: an expired credential is a dead end the applicant cannot retry + // past, and saying so is the difference between "start again" and an unexplained failure. + const [isExpired, setIsExpired] = useState(false) const eventSourceRef = useRef(null) const pollingRef = useRef | null>(null) const sessionIdRef = useRef(null) @@ -46,6 +50,7 @@ export function useOnboarding() { setStep(next) return next } catch (requestError) { + if (requestError instanceof ExpiredSessionError) setIsExpired(true) setError(requestError instanceof Error ? requestError.message : 'Unknown onboarding error') throw requestError } finally { @@ -67,6 +72,10 @@ export function useOnboarding() { setIsCompleted(true) setStep((prev) => prev ? { ...prev, isCompleted: true, currentNode: null } : null) eventSourceRef.current = null + // The journey is over, so the credential has nothing left to authorise. Dropping it + // keeps a finished applicant's token off a shared machine rather than leaving it in + // sessionStorage until the tab closes. + clearApplicantToken() }, onAbandoned: () => { eventSourceRef.current = null @@ -110,8 +119,10 @@ export function useOnboarding() { try { const next = await apiSubmitStep(serverBase, sessionId, nodeId, payload) setStep(next) + if (next.isCompleted) clearApplicantToken() return next } catch (requestError) { + if (requestError instanceof ExpiredSessionError) setIsExpired(true) // ComplianceErrors are handled by the form component — don't set global error if (!(requestError instanceof ComplianceError)) { setError(requestError instanceof Error ? requestError.message : 'Unknown onboarding error') @@ -126,6 +137,7 @@ export function useOnboarding() { step, isLoading, error, + isExpired, isCompleted, startSession, submitStep, diff --git a/src/frontend/src/onboarding/types/flow.ts b/src/frontend/src/onboarding/types/flow.ts index c35681f..75b2000 100644 --- a/src/frontend/src/onboarding/types/flow.ts +++ b/src/frontend/src/onboarding/types/flow.ts @@ -17,14 +17,18 @@ export interface SessionStepResponse { sessionId: string isCompleted: boolean currentNode: FlowNode | null -} - -/** Session start also returns the credential used for the rest of the journey. */ -export interface SessionStartResponse extends SessionStepResponse { + /** + * A replacement credential, returned on step submission so a journey still being worked on never + * outlives its own token. Absent when an operator submits: their credential already covers the + * session. + */ applicantToken?: string | null applicantTokenExpiresAt?: string | null } +/** Session start also returns the credential used for the rest of the journey. */ +export type SessionStartResponse = SessionStepResponse + export interface StartSessionRequest { flowId: string customerProfileId?: string diff --git a/src/frontend/src/sessions/SessionDetail.tsx b/src/frontend/src/sessions/SessionDetail.tsx index 1358dd4..a1b78c5 100644 --- a/src/frontend/src/sessions/SessionDetail.tsx +++ b/src/frontend/src/sessions/SessionDetail.tsx @@ -1,5 +1,6 @@ import { useEffect, useState } from 'react' import { operatorFetch } from '../api/operator-api' +import { SessionEventTrail } from './SessionEventTrail' interface Submission { id: string @@ -74,6 +75,8 @@ export function SessionDetail({ sessionId, onBack }: SessionDetailProps) { {error &&

{error}

} {isLoading &&

Loading submissions…

} +

Submissions

+ {!isLoading && submissions.length === 0 && (

No submissions recorded for this session.

)} @@ -92,6 +95,10 @@ export function SessionDetail({ sessionId, onBack }: SessionDetailProps) { ))} )} + + {/* The complementary view: submissions say what was entered, the trail says how the applicant + got there and where the time went. */} + ) } diff --git a/src/frontend/src/sessions/SessionEventTrail.test.tsx b/src/frontend/src/sessions/SessionEventTrail.test.tsx new file mode 100644 index 0000000..4f27309 --- /dev/null +++ b/src/frontend/src/sessions/SessionEventTrail.test.tsx @@ -0,0 +1,190 @@ +import { render, screen, waitFor } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { SessionEventTrail } from './SessionEventTrail' +import { formatGap } from './eventTrailFormat' +import type { AnalyticsTrailEvent } from './SessionEventTrail' + +/** + * The persisted trail is what an operator reads to find out why an applicant stalled. Three things + * have to hold for it to be readable: the order has to be the order events happened, a client event + * has to be distinguishable from a server one, and an empty panel must say which kind of empty it + * is — a session with no client events is normal, and one whose events aged out of retention is not + * the same as one that never had any. + */ + +const SESSION_ID = '11111111-1111-1111-1111-111111111111' + +function event(overrides: Partial & { eventId: string }): AnalyticsTrailEvent { + return { + eventType: 'step_view', + journeyId: '22222222-2222-2222-2222-222222222222', + sessionId: SESSION_ID, + stepId: 'identity', + stepIndex: 0, + payload: {}, + occurredAt: '2026-09-13T10:00:00.000Z', + source: 'server', + ...overrides, + } +} + +function mockTrail(events: AnalyticsTrailEvent[]) { + return vi.spyOn(globalThis, 'fetch').mockResolvedValue({ + ok: true, + status: 200, + json: async () => events, + } as Response) +} + +describe('SessionEventTrail', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + + it('renders the trail in occurrence order', async () => { + // Deliberately out of order from the API, so the assertion is about the view, not the fixture. + mockTrail([ + event({ eventId: 'c', eventType: 'journey_complete', occurredAt: '2026-09-13T10:02:00.000Z' }), + event({ eventId: 'a', eventType: 'session_started', occurredAt: '2026-09-13T10:00:00.000Z' }), + event({ eventId: 'b', eventType: 'step_submitted', occurredAt: '2026-09-13T10:01:00.000Z' }), + ]) + + render() + + await waitFor(() => expect(screen.getAllByTestId('trail-event')).toHaveLength(3)) + + const rendered = screen.getAllByTestId('trail-event').map((node) => node.textContent) + expect(rendered[0]).toContain('session_started') + expect(rendered[1]).toContain('step_submitted') + expect(rendered[2]).toContain('journey_complete') + }) + + it('shows event type, step, timestamp and the gap since the previous event', async () => { + mockTrail([ + event({ eventId: 'a', eventType: 'session_started', occurredAt: '2026-09-13T10:00:00.000Z' }), + event({ + eventId: 'b', + eventType: 'step_submitted', + stepId: 'address', + stepIndex: 1, + occurredAt: '2026-09-13T10:02:30.000Z', + }), + ]) + + render() + + await waitFor(() => expect(screen.getAllByTestId('trail-event')).toHaveLength(2)) + + const second = screen.getAllByTestId('trail-event')[1] + expect(second.textContent).toContain('step_submitted') + expect(second.textContent).toContain('address') + expect(second.textContent).toContain('#1') + // The stall signal: two and a half minutes on one step. + expect(second.textContent).toContain('+2m 30s') + expect(second.textContent).toContain(new Date('2026-09-13T10:02:30.000Z').toLocaleString()) + }) + + it('distinguishes client events from server events', async () => { + mockTrail([ + event({ eventId: 'a', source: 'server', occurredAt: '2026-09-13T10:00:00.000Z' }), + event({ eventId: 'b', source: 'client', occurredAt: '2026-09-13T10:00:30.000Z' }), + ]) + + render() + + await waitFor(() => expect(screen.getAllByTestId('trail-event')).toHaveLength(2)) + + const [first, second] = screen.getAllByTestId('trail-event') + expect(first).toHaveAttribute('data-source', 'server') + expect(second).toHaveAttribute('data-source', 'client') + + // Labelled, not only coloured: a reader should not have to know the schema to interpret a gap. + expect(first.textContent).toContain('server') + expect(second.textContent).toContain('client') + }) + + it('says a trail with no client events is not an error', async () => { + mockTrail([ + event({ eventId: 'a', source: 'server' }), + event({ eventId: 'b', source: 'server', occurredAt: '2026-09-13T10:00:10.000Z' }), + ]) + + render() + + await waitFor(() => expect(screen.getAllByTestId('trail-event')).toHaveLength(2)) + + expect(screen.getByText(/Server events only/i)).toBeInTheDocument() + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + }) + + it('does not show the server-only note when client events are present', async () => { + mockTrail([ + event({ eventId: 'a', source: 'server' }), + event({ eventId: 'b', source: 'client', occurredAt: '2026-09-13T10:00:10.000Z' }), + ]) + + render() + + await waitFor(() => expect(screen.getAllByTestId('trail-event')).toHaveLength(2)) + + expect(screen.queryByText(/Server events only/i)).not.toBeInTheDocument() + }) + + it('explains an empty trail rather than rendering a blank panel', async () => { + mockTrail([]) + + render() + + await waitFor(() => expect(screen.getByText(/No events recorded/i)).toBeInTheDocument()) + + // A session old enough to have aged out looks identical to one that never had events, so the + // empty state has to name both possibilities rather than implying data was lost. + expect(screen.getByText(/retention window/i)).toBeInTheDocument() + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + }) + + it('reports a failed load as an error, not as an empty trail', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValue({ + ok: false, + status: 500, + json: async () => ({}), + } as Response) + + render() + + await waitFor(() => expect(screen.getByRole('alert')).toBeInTheDocument()) + expect(screen.queryByText(/No events recorded/i)).not.toBeInTheDocument() + }) + + it('requests the trail through the operator credential', async () => { + const fetchSpy = mockTrail([]) + + render() + + await waitFor(() => expect(fetchSpy).toHaveBeenCalled()) + + const [url, init] = fetchSpy.mock.calls[0] + expect(url).toBe(`/api/analytics/sessions/${SESSION_ID}/events`) + // The admin session cookie, like every other operator view. Never an API key. + expect((init as RequestInit).credentials).toBe('include') + }) +}) + +describe('formatGap', () => { + it('shows no gap for the first event', () => { + expect(formatGap(null, '2026-09-13T10:00:00.000Z')).toBe('—') + }) + + it('scales the unit to the size of the gap', () => { + expect(formatGap('2026-09-13T10:00:00.000Z', '2026-09-13T10:00:00.400Z')).toBe('+400ms') + expect(formatGap('2026-09-13T10:00:00.000Z', '2026-09-13T10:00:05.000Z')).toBe('+5.0s') + expect(formatGap('2026-09-13T10:00:00.000Z', '2026-09-13T10:03:20.000Z')).toBe('+3m 20s') + expect(formatGap('2026-09-13T10:00:00.000Z', '2026-09-13T12:30:00.000Z')).toBe('+2h 30m') + }) + + it('does not render a negative gap from clock skew', () => { + // A client event timestamped by the applicant browser can land fractionally before the server + // event it followed. Showing a negative gap would read as an ordering bug. + expect(formatGap('2026-09-13T10:00:05.000Z', '2026-09-13T10:00:04.000Z')).toBe('—') + }) +}) diff --git a/src/frontend/src/sessions/SessionEventTrail.tsx b/src/frontend/src/sessions/SessionEventTrail.tsx new file mode 100644 index 0000000..778f229 --- /dev/null +++ b/src/frontend/src/sessions/SessionEventTrail.tsx @@ -0,0 +1,161 @@ +import { useEffect, useMemo, useState } from 'react' +import { operatorFetch } from '../api/operator-api' +import { formatGap, isClient, payloadSummary } from './eventTrailFormat' + +/** + * One session's persisted analytics trail. + * + * The submissions list next to this shows what an applicant entered. The trail shows how they got + * there: the order steps were seen in, where time went, which validations failed. That is what an + * operator investigating a stalled application actually needs, and until now it could only be had + * by querying the API by hand. + * + * Operator-only, through `operatorFetch` and the admin session cookie, like every other operator + * view. The applicant page never requests it. + */ +export interface AnalyticsTrailEvent { + eventId: string + eventType: string + journeyId: string + sessionId: string + stepId?: string | null + stepIndex?: number | null + payload?: Record | null + occurredAt: string + /** `server` for events the engine emits, `client` for events a browser reported. */ + source: string +} + +interface SessionEventTrailProps { + sessionId: string +} + +export function SessionEventTrail({ sessionId }: SessionEventTrailProps) { + const [events, setEvents] = useState([]) + const [isLoading, setIsLoading] = useState(false) + const [error, setError] = useState(null) + + useEffect(() => { + const controller = new AbortController() + + const load = async () => { + setIsLoading(true) + setError(null) + try { + const res = await operatorFetch( + `/api/analytics/sessions/${sessionId}/events`, + { signal: controller.signal }, + ) + if (!res.ok) throw new Error(`Failed to load the event trail (${res.status})`) + setEvents((await res.json()) as AnalyticsTrailEvent[]) + } catch (err) { + if (err instanceof Error && err.name !== 'AbortError') { + setError(err.message) + } + } finally { + setIsLoading(false) + } + } + + void load() + return () => controller.abort() + }, [sessionId]) + + // The API returns the trail in order; sorting here keeps the view correct if that ever changes, + // and a trail read out of order is worse than useless. + const ordered = useMemo( + () => [...events].sort((a, b) => new Date(a.occurredAt).getTime() - new Date(b.occurredAt).getTime()), + [events], + ) + + const clientEventCount = ordered.filter((event) => isClient(event.source)).length + + return ( +
+
+

+ Event trail +

+ {ordered.length > 0 && ( +

+ {ordered.length} event{ordered.length === 1 ? '' : 's'} +

+ )} +
+ + {error &&

{error}

} + {isLoading &&

Loading event trail…

} + + {!isLoading && !error && ordered.length === 0 && ( +
+

No events recorded for this session.

+

+ Either none were raised, or they have passed the analytics retention window and been + deleted. Older sessions show nothing here even when the journey was completed normally. +

+
+ )} + + {!isLoading && !error && ordered.length > 0 && clientEventCount === 0 && ( +

+ Server events only. Client events are reported by the applicant's browser and can be + absent entirely — an ad blocker or a closed tab stops them reaching the API. This is not an + error. +

+ )} + + {ordered.length > 0 && ( +
    + {ordered.map((event, index) => { + const gap = formatGap(index === 0 ? null : ordered[index - 1].occurredAt, event.occurredAt) + const client = isClient(event.source) + const summary = payloadSummary(event.payload) + + return ( +
  1. +
    + {event.eventType} + + {client ? 'client' : 'server'} + + {gap} +
    + +

    + {new Date(event.occurredAt).toLocaleString()} + · + + Step:{' '} + {event.stepId + ? {event.stepId} + : none} + {typeof event.stepIndex === 'number' ? ` (#${event.stepIndex})` : ''} + +

    + + {summary && ( +

    {summary}

    + )} +
    2. + ) + })} +
+ )} +
+ ) +} diff --git a/src/frontend/src/sessions/eventTrailFormat.ts b/src/frontend/src/sessions/eventTrailFormat.ts new file mode 100644 index 0000000..a462f7a --- /dev/null +++ b/src/frontend/src/sessions/eventTrailFormat.ts @@ -0,0 +1,45 @@ +/** + * Formatting for the session event trail, kept apart from the component so both can be tested and + * so the component file exports only a component. + */ + +/** + * Renders the gap since the previous event. This is the column that makes a stall visible: a + * fifteen-minute pause on one step reads very differently from fifteen seconds. + */ +export function formatGap(previousIso: string | null, currentIso: string): string { + if (previousIso === null) return '—' + + const gapMs = new Date(currentIso).getTime() - new Date(previousIso).getTime() + if (!Number.isFinite(gapMs)) return '—' + + // Clock skew between a browser and the server can put a client event fractionally before the + // server event it followed. Showing a negative gap would read as an ordering bug. + if (gapMs < 0) return '—' + if (gapMs < 1000) return `+${gapMs}ms` + + const seconds = gapMs / 1000 + if (seconds < 60) return `+${seconds.toFixed(1)}s` + + const minutes = Math.floor(seconds / 60) + if (minutes < 60) return `+${minutes}m ${Math.round(seconds % 60)}s` + + const hours = Math.floor(minutes / 60) + return `+${hours}h ${minutes % 60}m` +} + +export function payloadSummary(payload: Record | null | undefined): string | null { + if (!payload) return null + + const entries = Object.entries(payload) + if (entries.length === 0) return null + + return entries + .slice(0, 4) + .map(([key, value]) => `${key}: ${String(value)}`) + .join(', ') + (entries.length > 4 ? ', …' : '') +} + +export function isClient(source: string): boolean { + return source.toLowerCase() === 'client' +}