auth-bundle-1 Phase 0: extract internal/auth/ from middleware package

Bundle 1 / Phase 0: pure refactor splitting auth surface out of internal/api/middleware so Bundle 2 (OIDC + sessions) and the broader RBAC primitive (roles, permissions, scoped grants) have a clean home.

Moved to internal/auth/: NamedAPIKey, HashAPIKey, AuthConfig, NewAuthWithNamedKeys, NewAuth, UserKey, AdminKey, GetUser, IsAdmin. Added testfixtures.go (WithActor / WithAdmin / WithActorAdmin) so handler tests don't construct context manually.

Stayed in internal/api/middleware/: RequestID, Logging, NewLogging, Recovery, RateLimitConfig, NewRateLimiter (now imports auth.GetUser for per-user keying per audit Category C), CORSConfig, NewCORS, ContentType, CORS, GetRequestID, responseWriter, Chain, audit middleware (now imports auth.GetUser).

Updated 22 caller files across cmd/, internal/api/handler/, internal/api/middleware/, internal/mcp/. Existing m008_admin_gate_test.go now scans for auth.IsAdmin( substring; Phase 3 will further evolve to track auth.RequirePermission. Behavior unchanged: all handler / middleware / service / connector / cmd / mcp tests pass with no test-logic edits, only import-path renames.

Phase 0 exit criteria: internal/auth/ exists with 6 files; middleware.go went 575 -> 422 lines (auth-related ~150 lines moved out); grep -rE 'middleware\.(GetUser|IsAdmin|UserKey|AdminKey|NamedAPIKey|HashAPIKey|NewAuth)' returns 0 hits; context.WithValue(.*middleware.UserKey/AdminKey) returns 0 hits; go vet ./... clean; go test -short ./... green across all packages tested.

Branch: dev/auth-bundle-1. Per cowork/auth-bundle-1-prompt.md, do not merge to master without (1) make verify green, (2) >= 2 external testers confirm, (3) >= 90% coverage on internal/auth/ in .github/coverage-thresholds.yml.
This commit is contained in:
shankar0123
2026-05-09 15:51:31 +00:00
parent aad95d315b
commit 69f860171e
32 changed files with 397 additions and 283 deletions
+8 -8
View File
@@ -14,19 +14,19 @@ import (
//
// The audit's request is "Admin-gated operation role-gate test coverage
// needs verification". Verified-already-clean recon: only one handler
// in internal/api/handler/ calls middleware.IsAdmin to gate access:
// in internal/api/handler/ calls auth.IsAdmin to gate access:
// bulk_revocation.go — which has 3 dedicated tests
// (NonAdmin_Returns403, AdminExplicitFalse_Returns403,
// AdminPermitted_ForwardsActor) covering all three branches.
//
// This test enforces the invariant going forward by walking every
// .go file in this package, finding every middleware.IsAdmin call
// .go file in this package, finding every auth.IsAdmin call
// site, and asserting the file appears in AdminGatedHandlers below.
// Adding a new middleware.IsAdmin call without updating the constant
// Adding a new auth.IsAdmin call without updating the constant
// AND adding a parallel test triplet fails CI.
// AdminGatedHandlers is the documented allowlist of handler files that
// gate access on middleware.IsAdmin. Every entry MUST have:
// gate access on auth.IsAdmin. Every entry MUST have:
// - a non-admin-rejection test ("_NonAdmin_Returns403")
// - an explicit-false-admin-rejection test ("_AdminExplicitFalse_Returns403")
// - an admin-allowed actor-attribution test ("_AdminPermitted_ForwardsActor")
@@ -43,7 +43,7 @@ var AdminGatedHandlers = map[string]string{
}
// InformationalIsAdminCallers is the documented allowlist of files that
// call middleware.IsAdmin without using the result to gate access. The
// call auth.IsAdmin without using the result to gate access. The
// only legitimate use of an informational call is reporting the flag to
// a downstream consumer (e.g. health.go::AuthCheck reports admin to the
// GUI so it can hide admin-only buttons).
@@ -64,7 +64,7 @@ func TestM008_AdminGatedHandlers_PinExpectedSet(t *testing.T) {
if !slicesEqual008(actual, expected) {
t.Errorf(
"middleware.IsAdmin call sites changed:\n"+
"auth.IsAdmin call sites changed:\n"+
" actual: %v\n"+
" expected: %v\n"+
"\n"+
@@ -143,10 +143,10 @@ func scanIsAdminCallers(dir string) ([]string, error) {
if parseErr != nil {
continue
}
// Substring-match middleware.IsAdmin — cheap and sufficient
// Substring-match auth.IsAdmin — cheap and sufficient
// because the import path is fixed and there's no aliasing
// shenanigans elsewhere in this package.
if strings.Contains(string(body), "middleware.IsAdmin(") {
if strings.Contains(string(body), "auth.IsAdmin(") {
out = append(out, name)
}
}