Skip to content

Clean up: retire old auth middleware + flag #696

Description

@sabbour

Part of epic #693. Design doc: docs/design/auth-architecture-plan.mdAppendix A.4.2, "PR 3 — Clean up".

PR 3 of 3 — the one that actually pays off the tech debt. Everything before this adds code; this deletes the old auth stack and the migration flag, leaving the scheme-based pipeline as the only path.

Gates

Depends on #695, and:

  • Auth:UseSchemeBasedPipeline has been on in production for a stated bake period with no auth incidents. ⚠️ State the period explicitly in the PR body (a full release cycle is a reasonable default) — an unbounded "when it feels safe" is exactly how a temporary flag becomes permanent (risk R11).
  • The grep guard test from Prepare: claims consolidation + golden auth decision-matrix test #694 is green — nothing outside the auth handlers still reads the caller from HttpContext.Items. Deleting the Items write while HttpContextAuthenticatedOwnerContext still reads it throws at runtime on owner-scoped blueprint operations, on a path no health probe or smoke test covers (risk R12).

Scope

1. Delete the old pipeline

  • apps/Agentweaver.Api/Security/ApiKeyAuthMiddleware.csGitHubTokenAuthMiddleware
  • apps/Agentweaver.Api/Auth/PlatformRoleAuthorizationMiddleware.cs
  • apps/Agentweaver.Api/Auth/GitHubOrgAuthorizationMiddleware.cs
  • The Auth:UseSchemeBasedPipeline flag, its Auth__UseSchemeBasedPipeline env wiring, and the AUTH_USE_SCHEME_BASED_PIPELINE ConfigMap key (k8s/base/api-deployment.yaml, scripts/azure/lib/kustomize.mjs).
  • After merge, verify no environment still sets the removed key.

2. Cleanup and hardening

  • Delete SetCaller's HttpContext.Items write and the CallerItemKey constant (retrieval stopped using it in Prepare: claims consolidation + golden auth decision-matrix test #694).
  • Retire OpenApiSecurityTransformers.IsPublicPath's path heuristics in favour of the same endpoint metadata, so the published OpenAPI contract can no longer disagree with runtime behaviour. (This is the fourth copy of the allowlist that started this whole epic.)
  • Switch ProjectAuthorization.IsInternalServiceCaller to a test on the agentweaver_auth_scheme claim instead of a username string comparison (A.2.5.1 / A.3.4, risk R10). Today anyone whose resolved principal name equals the configured Auth:User inherits the Tier-2 bypass regardless of how they authenticated — a pre-existing weakness this refactor is the natural moment to close. Add a test that a non-InternalServiceKey caller with a colliding name is denied.
  • Fixed-time comparison for the internal API key, if not already done in Cut over to scheme-based auth pipeline behind a flag #695.
  • Keep test 1a (endpoint classification enumeration) as the permanent CI guard that no endpoint is unclassified. This is the backstop that makes the #690 bug class impossible to reintroduce.

3. Simplify the tests

Collapse the both-branches runs of 1a/1b down to the single remaining pipeline.

4. Docs

Update docs/guide/ auth pages and docs/mcp-oauth.md for the new scheme names, WWW-Authenticate behaviour, and any changed status codes. Docs are part of the definition of done per CONTRIBUTING.md.

Acceptance criteria

  • All three middlewares deleted; no references remain.
  • Flag, env var, and ConfigMap key deleted; no environment still sets them.
  • HttpContext.Items-based caller stash fully removed; grep guard still green.
  • OpenApiSecurityTransformers derives public-ness from endpoint metadata.
  • IsInternalServiceCaller is scheme-claim-based, with a colliding-name denial test.
  • Test 1a green and blocking in CI.
  • Full suite green: dotnet test tests/Agentweaver.Tests/Agentweaver.Tests.csproj.
  • All three harness suites re-run green against staging after the old pipeline is gone.
  • Docs updated.

Rollback note

This is the one PR whose revert restores a flag that is expected to be true in the environment. If it is reverted, verify the config value is still consistent with the restored code before assuming the system is back to a known state.

Definition of done for the whole epic

Authentication and authorization are declarative and derived from endpoint metadata; there is exactly one source of truth for "is this endpoint public"; and the recurring "exempt X from auth middleware" bug class has a CI test that makes recurrence impossible.

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