Skip to content

Prepare: claims consolidation + golden auth decision-matrix test #694

Description

@sabbour

Part of epic #693. Design doc: docs/design/auth-architecture-plan.mdAppendix A.4.2, "PR 1 — Prepare", plus §2.3.1, A.2.5, and A.5.2 (1a/1b/1c).

PR 1 of 3. Zero enforcement change — nothing about who is allowed in changes. This PR exists to make the next one safe.

Scope

1. Claims consolidation

  • Make ClaimsPrincipal the single source of truth. Add agentweaver_primary_role and agentweaver_org (currently only on CallerContext).
  • Add the explicit agentweaver_auth_scheme claim, stamped by each authentication path (A.2.5.1). IsOAuthJwt and IsInternalServiceCaller must project from this claim — not from Identity.AuthenticationType, which is not guaranteed to equal the registered scheme name and can be silently rewritten by any principal-rebuilding code (risk R14). Strip inbound agentweaver_* claims before stamping so a token cannot self-assert its own scheme.
  • Rewrite ApiKeyAuthMiddleware.GetCaller(HttpContext) to project from context.User instead of HttpContext.Items. SetCaller keeps writing both for now. This one change makes all 73 GetCaller call sites correct with a zero-line diff — do not touch them individually.
  • Preserve CallerContext.Owns semantics exactly (it matches User or GitHubLogin). A regression here is broken access control, not a cosmetic bug (risk R7).
  • Keep the raw GitHub bearer token out of ClaimsPrincipal (A.2.5.2). Claims get serialised into cookies, emitted by diagnostics endpoints, attached to telemetry, and dumped by structured logging — a credential in a claim leaks somewhere and nothing fails. Re-read the Authorization header after authentication, or use a private request-scoped feature.

2. Migrate the two direct HttpContext.Items readers ⚠️

The GetCaller shim does not cover these, and they will break when PR 3 deletes the Items write (risk R12):

Site Note
Blueprints/HttpContextAuthenticatedOwnerContext.cs:18 reads GitHubTokenAuthMiddleware.CallerItemKey — owner-scoped blueprint operations
Auth/GitHubOrgAuthorizationMiddleware.cs:117 reads the string literal "agentweaver.caller" — invisible to a CallerItemKey search

Migrate both to project from context.User, and add a repo-wide grep guard test that fails the build if anything outside the auth handlers references CallerItemKey, the literal "agentweaver.caller", or reads an auth caller out of HttpContext.Items. That guard is a CI gate for the whole flag lifetime and an explicit precondition for #696.

3. Auth-outcome telemetry

Emit a counter dimensioned by (pipeline, scheme, status, endpoint-classification) from the current pipeline. It lands here rather than in #695 so a baseline already exists before anything changes — otherwise the first datapoint and the first behaviour change arrive together and tell you nothing. This is the instrument that makes #695's canary readable, and it is one of the three controls substituting for the shadow mode this plan deliberately does not have.

4. Golden tests, written against the current middleware

That ordering is the whole point: these capture existing behaviour as the parity baseline, rather than describing the code that has not been written yet.

  • 1a — endpoint classification enumeration. Enumerate EndpointDataSource and assert every mapped endpoint's classification (anonymous / webhook-authenticated / protected, and which policy). Pure metadata — invokes nothing, no fixtures, no side effects, 100% endpoint coverage. Check in as a golden file so any change to who can reach what is a visible diff. Expectations must be derived from the complete exemption inventory in §2.3.1 — including the implicit "not under /api" rule, which is the largest exemption in the system and the one that keeps the health probes anonymous today.
  • 1b — authorization outcomes over representative route fixtures. ~12 hand-picked routes, one per authorization shape, each crossed with the credential matrix, asserting each fixture's own expected status. Deliberately not "every endpoint × every credential": most endpoints legitimately return 400/404/409 with perfect credentials, so an auth regression would hide behind an expected-400, and blindly invoking every route executes deletes, run submissions and outbound GitHub calls.
  • 1c — health-probe anonymity. GET /health, /api/health, /api/ping, /healthz/workspace return 200 with no credentials. Small and separate because of what it protects: these carry no .AllowAnonymous() at all today, so a default-deny fallback policy would 401 them and crash-loop every pod (risk R13).

Acceptance criteria

  • GetCaller projects from context.User; no call sites changed.
  • agentweaver_auth_scheme stamped and projected from; forged inbound values stripped, with a test.
  • No claim value equals the presented credential (asserted by test).
  • Both direct Items readers migrated; grep guard test green and wired into CI.
  • Auth-outcome telemetry emitting from the current pipeline.
  • Tests 1a / 1b / 1c checked in and green against the existing middleware.
  • HttpContextAuthenticatedOwnerContextTests and ProjectOwnershipAuthorizationTests pass with unchanged assertions.
  • Full suite green: dotnet test tests/Agentweaver.Tests/Agentweaver.Tests.csproj.
  • No authentication or authorization outcome changes anywhere.

Rollback

Trivial revert — no enforcement behaviour changed.

Out of scope

Any new scheme, handler, or policy (that is #695). Migrating the 73 GetCaller call sites to inject ClaimsPrincipal directly — worthwhile later, but bundling it makes this diff unreviewable.

Metadata

Metadata

Assignees

No one assigned

    Labels

    release:backlogNot yet targetedsquad:tankAssigned to Tank (Backend Engineer)type:choreMaintenance, refactoring, cleanup

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions