Skip to content

Cut over to scheme-based auth pipeline behind a flag #695

Description

@sabbour

Part of epic #693. Design doc: docs/design/auth-architecture-plan.mdAppendix A.4.2, "PR 2", with the target architecture in A.2 and the policies in A.3.

PR 2 of 3. The substantial one. It ships off by default, so merging it changes nothing in production — the enablement is the event to manage, not the merge.

Depends on #694.

The flag

  • Config key: Auth:UseSchemeBasedPipeline (bool, default false) — consistent with the existing Auth:* namespace (Auth:Mode, Auth:ApiKey, Auth:GitHub:AllowedOrg, Auth:Entra:ClientId).
  • Deployment wiring: env Auth__UseSchemeBasedPipeline from an AUTH_USE_SCHEME_BASED_PIPELINE ConfigMap key — exactly how Auth__Mode comes from AUTH_MODE today (k8s/base/api-deployment.yaml:186-190, scripts/azure/lib/kustomize.mjs:176). No new mechanism.
  • Startup-time and mutually exclusive. Read once in Program.cs to select one of two pipeline constructions. Never register both and branch per request — that doubles the live surface and makes behaviour depend on config-reload timing. Log the resolved value at startup next to the existing "Running in {AuthMode} auth mode." line.
  • Orthogonal to Auth:Mode — the flag chooses how, Auth:Mode chooses which IdP. Both must work under both pipelines, so the tests run 2 × 2.
  • Temporary by construction. Clean up: retire old auth middleware + flag #696 deletes it.

Scope

1. The new pipeline (behind the flag)

Schemes (A.2.1) — reuse the existing validators, do not rewrite them:

Scheme Implementation Reuses
Entra AddJwtBearer EntraAccessTokenValidator (its Authority/Issuer/ClientId config; claim-shaping in OnTokenValidated)
McpOAuth AddJwtBearer McpTokenService.CreateValidationParameters + McpRefreshTokenStore.IsJtiDeniedAsync
GitHubToken custom handler the IMemoryCache + ValidateGitHubTokenAsync logic from ApiKeyAuthMiddleware.cs
InternalServiceKey custom handler the Auth:ApiKey block — switch to CryptographicOperations.FixedTimeEquals
TestBypass custom, Development-only the _bypassForTests / _testApiKeyMap block, LogCritical guard rails intact

Composition (A.2.2): one AddPolicyScheme("Agentweaver", ...) as the default, whose ForwardDefaultSelector picks the concrete scheme. One place encodes the mode branch; endpoint authors never name a scheme.

Authorization (A.3): UseAuthorization() with a mode-appropriate FallbackPolicyPlatformAccess for Entra (the policy already exists; PlatformRoleAuthorizationMiddleware is a hand-rolled invocation of it), GitHubOrgAccess for GitHubLegacy (preserving fail-closed-on-missing-config). Custom IAuthorizationMiddlewareResultHandler to preserve today's 401/403 response bodies.

2. Keep the old pipeline intact on the false branch

GitHubTokenAuthMiddleware, PlatformRoleAuthorizationMiddleware, GitHubOrgAuthorizationMiddleware unchanged. This is the safety net — it makes rollback a config flip.

3. Four traps to call out in review

  • The selector picks ONE handler — there is no fallback chain (A.2.2.1). NoResult means "this request is anonymous", not "let the next scheme try". Handlers must return Fail whenever a credential was presented and rejected; NoResult is correct only when there is no Authorization header. Getting this wrong silently downgrades an expired token to anonymous (risk R5). The selector must also never throw on an adversarial Authorization value — that would be a 500 on every request.
  • MCP OAuth in Entra mode is a behaviour change (A.2.2.2). Today the Entra branch returns unconditionally, so McpTokenService is only reached in GitHubLegacy mode — MCP tokens are rejected in Entra mode today. A naive selector would silently add that capability. Gate the McpOAuth branch on AUTH_MODE=GitHubLegacy to preserve parity; MCP-in-Entra-mode needs its own spec (what roles does an MCP principal carry? today it has none, so PlatformAccess would 403 it anyway), its own tests, and its own security review.
  • MCP issuer validation (A.2.4). Because OAuthServerConfig.ResolveIssuer is per-request but TokenValidationParameters is startup-resolved, this scheme relaxes issuer/audience validation. That is only safe if you also pin the signing key, pin the algorithm, and write an explicit iss/aud comparison in OnTokenValidated. ⚠️ Merely constructing a TokenValidationParameters in the event validates nothing — it is inert data. This reads as correct in review and is not (risk R6). Mandatory Seraph security review.
  • AuthModeEpochService (A.3.3) stays as its own small middleware before UseAuthentication() — not inside a scheme handler (its DB call would multiply) and not as a policy. Preserve the "only when an Authorization header is present" condition exactly, or health probes on a stale-epoch pod start failing (risk R8).

4. Complete the AllowAnonymous annotation

Verify against the full §2.3.1 inventory, not just the obvious endpoints. In particular the health/readiness routes are anonymous today only via the implicit non-/api rule and would otherwise be 401'd by the fallback policy — taking every pod's probes down (risk R13). Note /api/auth/* is not an AllowAnonymous case: it is "authenticated but exempt from the org check" and belongs in the policy.

Acceptance criteria

  • Flag defaults to false; merging changes nothing in any deployed environment.
  • Pipeline registration is startup-time and mutually exclusive — no per-request branch.
  • CI runs tests 1a and 1b against BOTH branches (flag off and on) and asserts identical outcomes. Primary parity control.
  • Per-scheme unit tests: accept/reject, exact claims, and NoResult vs Fail per the A.2.2.1 table.
  • Selector tests covering every row of A.2.2.1, including malformed-token fall-through and "resembles-but-does-not-equal the internal key".
  • Fallback policy non-null in both modes; a metadata-less endpoint is denied.
  • Response-shape golden tests: 401 body {"error":"unauthorized"}, 403 body preserved, WWW-Authenticate asserted verbatim (risks R1R3).
  • Hostile MCP matrix: foreign issuer, wrong audience, different signing key, unexpected algorithm, forged Host/X-Forwarded-Host — all 401.
  • Test 1c passes with the flag on (probes still anonymous).
  • TestBypass provably absent outside Development (risk R9).
  • Staging with the flag ON: agentweaver-api-harness, agentweaver-mcp-harness, agentweaver-ui-harness all green. The MCP harness matters most — the only automated coverage of a real third-party client doing discovery + challenge + token.
  • Production enablement is canary-first: one replica, compare its auth-outcome telemetry against the fleet for a stated window, then widen. Separate deliberate config change, not part of the merge.

Rollback

Set Auth:UseSchemeBasedPipeline=false and restart. No revert, no rebuild, no redeploy. If the problem is structural, reverting the PR is also clean because the default is off.

Out of scope

Deleting any old middleware or the flag — that is #696.

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