Close open issues #99-#102: rate limit partitioning, broker transport coverage, token lifecycle, event trail UI

.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

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:

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:<hostname>`, 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__<Key>` 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.

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<SessionOwnershipRequirement, SessionDetailDto>
 {
+    /// <summary>
+    /// 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.
+    /// </summary>
+    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);

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 <c>SessionDetailDto</c>.
 /// </summary>
-public sealed class SessionOwnershipRequirement : IAuthorizationRequirement { }
+public sealed class SessionOwnershipRequirement : IAuthorizationRequirement
+{
+    private SessionOwnershipRequirement(bool requiresActiveSession)
+        => RequiresActiveSession = requiresActiveSession;
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    public bool RequiresActiveSession { get; }
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    public static SessionOwnershipRequirement Read { get; } = new(requiresActiveSession: false);
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    public static SessionOwnershipRequirement Write { get; } = new(requiresActiveSession: true);
+}

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<ActionResult<AnalyticsIngestResponse>> 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(