From 99a012e3be26b3605d9be9b548ae06a78c308c1a Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Sat, 9 May 2026 15:51:31 +0000 Subject: [PATCH] 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. --- cmd/server/main.go | 11 +- cmd/server/main_test.go | 9 +- internal/api/handler/admin_crl_cache.go | 4 +- internal/api/handler/admin_crl_cache_test.go | 11 +- internal/api/handler/admin_est.go | 6 +- internal/api/handler/admin_est_test.go | 19 +- internal/api/handler/admin_scep_intune.go | 8 +- .../api/handler/admin_scep_intune_test.go | 39 ++-- internal/api/handler/approval.go | 5 +- internal/api/handler/approval_test.go | 4 +- .../api/handler/bulk_partial_failure_test.go | 2 +- .../api/handler/bulk_renewal_handler_test.go | 5 +- internal/api/handler/bulk_revocation.go | 5 +- .../handler/bulk_revocation_handler_test.go | 9 +- internal/api/handler/health.go | 6 +- internal/api/handler/health_test.go | 10 +- internal/api/handler/intermediate_ca.go | 15 +- internal/api/handler/intermediate_ca_test.go | 10 +- internal/api/handler/m008_admin_gate_test.go | 16 +- internal/api/handler/response.go | 4 +- internal/api/middleware/audit.go | 4 +- internal/api/middleware/audit_test.go | 4 +- internal/api/middleware/middleware.go | 203 +++--------------- .../api/middleware/ratelimit_keyed_test.go | 10 +- internal/auth/apikey.go | 28 +++ internal/auth/context.go | 50 +++++ internal/auth/middleware.go | 141 ++++++++++++ .../auth_test.go => auth/middleware_test.go} | 2 +- .../rotation_test.go} | 2 +- internal/auth/testfixtures.go | 32 +++ internal/mcp/tools.go | 4 +- internal/mcp/types.go | 2 +- 32 files changed, 397 insertions(+), 283 deletions(-) create mode 100644 internal/auth/apikey.go create mode 100644 internal/auth/context.go create mode 100644 internal/auth/middleware.go rename internal/{api/middleware/auth_test.go => auth/middleware_test.go} (99%) rename internal/{api/middleware/auth_l004_rotation_test.go => auth/rotation_test.go} (99%) create mode 100644 internal/auth/testfixtures.go diff --git a/cmd/server/main.go b/cmd/server/main.go index e5afa65..510c725 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -21,6 +21,7 @@ import ( "github.com/certctl-io/certctl/internal/api/handler" "github.com/certctl-io/certctl/internal/api/middleware" "github.com/certctl-io/certctl/internal/api/router" + "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/config" discoveryawssm "github.com/certctl-io/certctl/internal/connector/discovery/awssm" discoveryazurekv "github.com/certctl-io/certctl/internal/connector/discovery/azurekv" @@ -1483,13 +1484,13 @@ func main() { // Named keys come from CERTCTL_API_KEYS_NAMED (preferred). For backward // compatibility CERTCTL_AUTH_SECRET is synthesized into legacy-key-N // entries with Admin=false. - var namedKeys []middleware.NamedAPIKey + var namedKeys []auth.NamedAPIKey if config.AuthType(cfg.Auth.Type) != config.AuthTypeNone { - // Translate typed config.NamedAPIKey -> middleware.NamedAPIKey. The + // Translate typed config.NamedAPIKey -> auth.NamedAPIKey. The // two structs are field-compatible but live in different packages to // preserve the config→middleware dependency direction. for _, nk := range cfg.Auth.NamedKeys { - namedKeys = append(namedKeys, middleware.NamedAPIKey{ + namedKeys = append(namedKeys, auth.NamedAPIKey{ Name: nk.Name, Key: nk.Key, Admin: nk.Admin, @@ -1506,7 +1507,7 @@ func main() { if p == "" { continue } - namedKeys = append(namedKeys, middleware.NamedAPIKey{ + namedKeys = append(namedKeys, auth.NamedAPIKey{ Name: fmt.Sprintf("legacy-key-%d", idx), Key: p, Admin: false, @@ -1519,7 +1520,7 @@ func main() { } } } - authMiddleware := middleware.NewAuthWithNamedKeys(namedKeys) + authMiddleware := auth.NewAuthWithNamedKeys(namedKeys) corsMiddleware := middleware.NewCORS(middleware.CORSConfig{ AllowedOrigins: cfg.CORS.AllowedOrigins, }) diff --git a/cmd/server/main_test.go b/cmd/server/main_test.go index 643c111..2761e83 100644 --- a/cmd/server/main_test.go +++ b/cmd/server/main_test.go @@ -12,6 +12,7 @@ import ( "github.com/certctl-io/certctl/internal/api/middleware" "github.com/certctl-io/certctl/internal/api/router" + "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/config" "github.com/certctl-io/certctl/internal/service" ) @@ -44,7 +45,7 @@ func TestMain_HealthEndpointBypassesAuth(t *testing.T) { }) // Build the handler chain the same way main.go does - authMiddleware := middleware.NewAuthWithNamedKeys([]middleware.NamedAPIKey{ + authMiddleware := auth.NewAuthWithNamedKeys([]auth.NamedAPIKey{ {Name: "test", Key: "test-secret-key"}, }) @@ -159,7 +160,7 @@ func TestMain_AuthMiddlewareRejectsUnauthorized(t *testing.T) { }) // Wrap with auth middleware - authMiddleware := middleware.NewAuthWithNamedKeys([]middleware.NamedAPIKey{ + authMiddleware := auth.NewAuthWithNamedKeys([]auth.NamedAPIKey{ {Name: "test", Key: "test-secret-key"}, }) @@ -187,7 +188,7 @@ func TestMain_AuthMiddlewareAllowsWithValidKey(t *testing.T) { }) // Wrap with auth middleware - authMiddleware := middleware.NewAuthWithNamedKeys([]middleware.NamedAPIKey{ + authMiddleware := auth.NewAuthWithNamedKeys([]auth.NamedAPIKey{ {Name: "test", Key: testKey}, }) @@ -460,7 +461,7 @@ func TestMain_AuthNoneMode(t *testing.T) { // Wrap with auth middleware in "none" mode // auth=none equivalent: empty named-keys list is a no-op pass-through. - authMiddleware := middleware.NewAuthWithNamedKeys(nil) + authMiddleware := auth.NewAuthWithNamedKeys(nil) chainedHandler := middleware.Chain(protectedHandler, authMiddleware) diff --git a/internal/api/handler/admin_crl_cache.go b/internal/api/handler/admin_crl_cache.go index 5f08d73..986c056 100644 --- a/internal/api/handler/admin_crl_cache.go +++ b/internal/api/handler/admin_crl_cache.go @@ -5,7 +5,7 @@ import ( "net/http" "time" - "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/domain" "github.com/certctl-io/certctl/internal/repository" ) @@ -74,7 +74,7 @@ func (h AdminCRLCacheHandler) ListCache(w http.ResponseWriter, r *http.Request) Error(w, http.StatusMethodNotAllowed, "Method not allowed") return } - if !middleware.IsAdmin(r.Context()) { + if !auth.IsAdmin(r.Context()) { Error(w, http.StatusForbidden, "Admin access required") return } diff --git a/internal/api/handler/admin_crl_cache_test.go b/internal/api/handler/admin_crl_cache_test.go index da19635..174280e 100644 --- a/internal/api/handler/admin_crl_cache_test.go +++ b/internal/api/handler/admin_crl_cache_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" ) // fakeAdminCRLCacheService is the test stub for the @@ -67,7 +68,7 @@ func TestAdminCRLCache_AdminExplicitFalse_Returns403(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/crl/cache", nil) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, false) + ctx = context.WithValue(ctx, auth.AdminKey{}, false) req = req.WithContext(ctx) w := httptest.NewRecorder() @@ -99,8 +100,8 @@ func TestAdminCRLCache_AdminPermitted_ForwardsActor(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/crl/cache", nil) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, true) - ctx = context.WithValue(ctx, middleware.UserKey{}, "ops-admin") + ctx = context.WithValue(ctx, auth.AdminKey{}, true) + ctx = context.WithValue(ctx, auth.UserKey{}, "ops-admin") req = req.WithContext(ctx) w := httptest.NewRecorder() @@ -131,7 +132,7 @@ func TestAdminCRLCache_RejectsNonGetMethod(t *testing.T) { h := NewAdminCRLCacheHandler(&fakeAdminCRLCacheService{}) req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/crl/cache", nil) - ctx := context.WithValue(context.Background(), middleware.AdminKey{}, true) + ctx := context.WithValue(context.Background(), auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() @@ -150,7 +151,7 @@ func TestAdminCRLCache_PropagatesServiceError(t *testing.T) { h := NewAdminCRLCacheHandler(svc) req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/crl/cache", nil) - ctx := context.WithValue(context.Background(), middleware.AdminKey{}, true) + ctx := context.WithValue(context.Background(), auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() diff --git a/internal/api/handler/admin_est.go b/internal/api/handler/admin_est.go index 6efa530..b6a4af9 100644 --- a/internal/api/handler/admin_est.go +++ b/internal/api/handler/admin_est.go @@ -7,7 +7,7 @@ import ( "net/http" "time" - "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/service" ) @@ -76,7 +76,7 @@ func (h AdminESTHandler) Profiles(w http.ResponseWriter, r *http.Request) { Error(w, http.StatusMethodNotAllowed, "Method not allowed") return } - if !middleware.IsAdmin(r.Context()) { + if !auth.IsAdmin(r.Context()) { Error(w, http.StatusForbidden, "Admin access required") return } @@ -104,7 +104,7 @@ func (h AdminESTHandler) ReloadTrust(w http.ResponseWriter, r *http.Request) { Error(w, http.StatusMethodNotAllowed, "Method not allowed") return } - if !middleware.IsAdmin(r.Context()) { + if !auth.IsAdmin(r.Context()) { Error(w, http.StatusForbidden, "Admin access required") return } diff --git a/internal/api/handler/admin_est_test.go b/internal/api/handler/admin_est_test.go index 2a4c5f2..29abcfc 100644 --- a/internal/api/handler/admin_est_test.go +++ b/internal/api/handler/admin_est_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/service" ) @@ -65,7 +66,7 @@ func TestAdminEST_Profiles_AdminExplicitFalse_Returns403(t *testing.T) { h := NewAdminESTHandler(svc) req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/est/profiles", nil) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, false) + ctx = context.WithValue(ctx, auth.AdminKey{}, false) req = req.WithContext(ctx) w := httptest.NewRecorder() h.Profiles(w, req) @@ -86,7 +87,7 @@ func TestAdminEST_Profiles_AdminTrue_Returns200(t *testing.T) { h := NewAdminESTHandler(svc) req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/est/profiles", nil) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + ctx = context.WithValue(ctx, auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.Profiles(w, req) @@ -121,7 +122,7 @@ func TestAdminEST_Profiles_NilRowsSerializedAsEmptyArray(t *testing.T) { h := NewAdminESTHandler(svc) req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/est/profiles", nil) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + ctx = context.WithValue(ctx, auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.Profiles(w, req) @@ -157,7 +158,7 @@ func TestAdminEST_ReloadTrust_AdminExplicitFalse_Returns403(t *testing.T) { strings.NewReader(`{"path_id":"corp"}`)) req.ContentLength = int64(len(`{"path_id":"corp"}`)) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, false) + ctx = context.WithValue(ctx, auth.AdminKey{}, false) req = req.WithContext(ctx) w := httptest.NewRecorder() h.ReloadTrust(w, req) @@ -177,7 +178,7 @@ func TestAdminEST_ReloadTrust_HappyPath(t *testing.T) { strings.NewReader(body)) req.ContentLength = int64(len(body)) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + ctx = context.WithValue(ctx, auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.ReloadTrust(w, req) @@ -197,7 +198,7 @@ func TestAdminEST_ReloadTrust_UnknownPathID_Returns404(t *testing.T) { strings.NewReader(body)) req.ContentLength = int64(len(body)) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + ctx = context.WithValue(ctx, auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.ReloadTrust(w, req) @@ -214,7 +215,7 @@ func TestAdminEST_ReloadTrust_MTLSDisabled_Returns409(t *testing.T) { strings.NewReader(body)) req.ContentLength = int64(len(body)) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + ctx = context.WithValue(ctx, auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.ReloadTrust(w, req) @@ -231,7 +232,7 @@ func TestAdminEST_ReloadTrust_ParseError_Returns500(t *testing.T) { strings.NewReader(body)) req.ContentLength = int64(len(body)) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + ctx = context.WithValue(ctx, auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.ReloadTrust(w, req) @@ -248,7 +249,7 @@ func TestAdminEST_ReloadTrust_MalformedJSON_Returns400(t *testing.T) { strings.NewReader(body)) req.ContentLength = int64(len(body)) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + ctx = context.WithValue(ctx, auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.ReloadTrust(w, req) diff --git a/internal/api/handler/admin_scep_intune.go b/internal/api/handler/admin_scep_intune.go index bb75ba0..bd32c1d 100644 --- a/internal/api/handler/admin_scep_intune.go +++ b/internal/api/handler/admin_scep_intune.go @@ -7,7 +7,7 @@ import ( "net/http" "time" - "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/service" ) @@ -90,7 +90,7 @@ func (h AdminSCEPIntuneHandler) Profiles(w http.ResponseWriter, r *http.Request) Error(w, http.StatusMethodNotAllowed, "Method not allowed") return } - if !middleware.IsAdmin(r.Context()) { + if !auth.IsAdmin(r.Context()) { Error(w, http.StatusForbidden, "Admin access required") return } @@ -118,7 +118,7 @@ func (h AdminSCEPIntuneHandler) Stats(w http.ResponseWriter, r *http.Request) { Error(w, http.StatusMethodNotAllowed, "Method not allowed") return } - if !middleware.IsAdmin(r.Context()) { + if !auth.IsAdmin(r.Context()) { Error(w, http.StatusForbidden, "Admin access required") return } @@ -146,7 +146,7 @@ func (h AdminSCEPIntuneHandler) ReloadTrust(w http.ResponseWriter, r *http.Reque Error(w, http.StatusMethodNotAllowed, "Method not allowed") return } - if !middleware.IsAdmin(r.Context()) { + if !auth.IsAdmin(r.Context()) { Error(w, http.StatusForbidden, "Admin access required") return } diff --git a/internal/api/handler/admin_scep_intune_test.go b/internal/api/handler/admin_scep_intune_test.go index 625ce4b..7f9fa5d 100644 --- a/internal/api/handler/admin_scep_intune_test.go +++ b/internal/api/handler/admin_scep_intune_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/service" ) @@ -81,7 +82,7 @@ func TestAdminSCEPIntune_AdminExplicitFalse_Returns403(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/scep/intune/stats", nil) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, false) + ctx = context.WithValue(ctx, auth.AdminKey{}, false) req = req.WithContext(ctx) w := httptest.NewRecorder() @@ -106,8 +107,8 @@ func TestAdminSCEPIntune_AdminPermitted_ForwardsActor(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/scep/intune/stats", nil) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, true) - ctx = context.WithValue(ctx, middleware.UserKey{}, "ops-admin") + ctx = context.WithValue(ctx, auth.AdminKey{}, true) + ctx = context.WithValue(ctx, auth.UserKey{}, "ops-admin") req = req.WithContext(ctx) w := httptest.NewRecorder() @@ -160,7 +161,7 @@ func TestAdminSCEPIntuneReload_AdminExplicitFalse_Returns403(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/scep/intune/reload-trust", strings.NewReader(`{"path_id":"corp"}`)) req.ContentLength = int64(len(`{"path_id":"corp"}`)) - ctx := context.WithValue(context.Background(), middleware.AdminKey{}, false) + ctx := context.WithValue(context.Background(), auth.AdminKey{}, false) req = req.WithContext(ctx) w := httptest.NewRecorder() @@ -181,8 +182,8 @@ func TestAdminSCEPIntuneReload_AdminPermitted_ForwardsActor(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/scep/intune/reload-trust", strings.NewReader(body)) req.ContentLength = int64(len(body)) - ctx := context.WithValue(context.Background(), middleware.AdminKey{}, true) - ctx = context.WithValue(ctx, middleware.UserKey{}, "ops-admin") + ctx := context.WithValue(context.Background(), auth.AdminKey{}, true) + ctx = context.WithValue(ctx, auth.UserKey{}, "ops-admin") req = req.WithContext(ctx) w := httptest.NewRecorder() @@ -211,7 +212,7 @@ func TestAdminSCEPIntuneReload_AdminPermitted_ForwardsActor(t *testing.T) { func TestAdminSCEPIntuneStats_RejectsNonGetMethod(t *testing.T) { h := NewAdminSCEPIntuneHandler(&fakeAdminSCEPIntuneService{}) req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/scep/intune/stats", nil) - ctx := context.WithValue(context.Background(), middleware.AdminKey{}, true) + ctx := context.WithValue(context.Background(), auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.Stats(w, req) @@ -223,7 +224,7 @@ func TestAdminSCEPIntuneStats_RejectsNonGetMethod(t *testing.T) { func TestAdminSCEPIntuneReload_RejectsNonPostMethod(t *testing.T) { h := NewAdminSCEPIntuneHandler(&fakeAdminSCEPIntuneService{}) req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/scep/intune/reload-trust", nil) - ctx := context.WithValue(context.Background(), middleware.AdminKey{}, true) + ctx := context.WithValue(context.Background(), auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.ReloadTrust(w, req) @@ -236,7 +237,7 @@ func TestAdminSCEPIntuneStats_PropagatesServiceError(t *testing.T) { svc := &fakeAdminSCEPIntuneService{statsErr: errors.New("registry walk failed")} h := NewAdminSCEPIntuneHandler(svc) req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/scep/intune/stats", nil) - ctx := context.WithValue(context.Background(), middleware.AdminKey{}, true) + ctx := context.WithValue(context.Background(), auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.Stats(w, req) @@ -251,7 +252,7 @@ func TestAdminSCEPIntuneReload_ProfileNotFound_Returns404(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/scep/intune/reload-trust", strings.NewReader(`{"path_id":"nonexistent"}`)) req.ContentLength = int64(len(`{"path_id":"nonexistent"}`)) - ctx := context.WithValue(context.Background(), middleware.AdminKey{}, true) + ctx := context.WithValue(context.Background(), auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.ReloadTrust(w, req) @@ -266,7 +267,7 @@ func TestAdminSCEPIntuneReload_IntuneDisabled_Returns409(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/scep/intune/reload-trust", strings.NewReader(`{"path_id":"iot"}`)) req.ContentLength = int64(len(`{"path_id":"iot"}`)) - ctx := context.WithValue(context.Background(), middleware.AdminKey{}, true) + ctx := context.WithValue(context.Background(), auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.ReloadTrust(w, req) @@ -281,7 +282,7 @@ func TestAdminSCEPIntuneReload_BadReloadPropagates500(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/scep/intune/reload-trust", strings.NewReader(`{"path_id":"corp"}`)) req.ContentLength = int64(len(`{"path_id":"corp"}`)) - ctx := context.WithValue(context.Background(), middleware.AdminKey{}, true) + ctx := context.WithValue(context.Background(), auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.ReloadTrust(w, req) @@ -294,7 +295,7 @@ func TestAdminSCEPIntuneReload_EmptyBodyTargetsLegacyRoot(t *testing.T) { svc := &fakeAdminSCEPIntuneService{} h := NewAdminSCEPIntuneHandler(svc) req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/scep/intune/reload-trust", nil) - ctx := context.WithValue(context.Background(), middleware.AdminKey{}, true) + ctx := context.WithValue(context.Background(), auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.ReloadTrust(w, req) @@ -312,7 +313,7 @@ func TestAdminSCEPIntuneReload_RejectsMalformedJSON(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/scep/intune/reload-trust", strings.NewReader(bad)) req.ContentLength = int64(len(bad)) - ctx := context.WithValue(context.Background(), middleware.AdminKey{}, true) + ctx := context.WithValue(context.Background(), auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.ReloadTrust(w, req) @@ -379,7 +380,7 @@ func TestAdminSCEPProfiles_AdminExplicitFalse_Returns403(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/scep/profiles", nil) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, false) + ctx = context.WithValue(ctx, auth.AdminKey{}, false) req = req.WithContext(ctx) w := httptest.NewRecorder() @@ -417,8 +418,8 @@ func TestAdminSCEPProfiles_AdminPermitted_ForwardsActor(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/scep/profiles", nil) ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, true) - ctx = context.WithValue(ctx, middleware.UserKey{}, "ops-admin") + ctx = context.WithValue(ctx, auth.AdminKey{}, true) + ctx = context.WithValue(ctx, auth.UserKey{}, "ops-admin") req = req.WithContext(ctx) w := httptest.NewRecorder() @@ -461,7 +462,7 @@ func TestAdminSCEPProfiles_AdminPermitted_ForwardsActor(t *testing.T) { func TestAdminSCEPProfiles_RejectsNonGetMethod(t *testing.T) { h := NewAdminSCEPIntuneHandler(&fakeAdminSCEPIntuneService{}) req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/scep/profiles", nil) - ctx := context.WithValue(context.Background(), middleware.AdminKey{}, true) + ctx := context.WithValue(context.Background(), auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.Profiles(w, req) @@ -474,7 +475,7 @@ func TestAdminSCEPProfiles_PropagatesServiceError(t *testing.T) { svc := &fakeAdminSCEPIntuneService{profilesErr: errors.New("registry walk failed")} h := NewAdminSCEPIntuneHandler(svc) req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/scep/profiles", nil) - ctx := context.WithValue(context.Background(), middleware.AdminKey{}, true) + ctx := context.WithValue(context.Background(), auth.AdminKey{}, true) req = req.WithContext(ctx) w := httptest.NewRecorder() h.Profiles(w, req) diff --git a/internal/api/handler/approval.go b/internal/api/handler/approval.go index 9974b8c..7a3cd50 100644 --- a/internal/api/handler/approval.go +++ b/internal/api/handler/approval.go @@ -8,6 +8,7 @@ import ( "strconv" "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/domain" "github.com/certctl-io/certctl/internal/repository" "github.com/certctl-io/certctl/internal/service" @@ -111,7 +112,7 @@ func (h ApprovalHandler) GetApproval(w http.ResponseWriter, r *http.Request) { // Approve transitions a pending approval request to approved + transitions // the linked Job from AwaitingApproval to Pending. RBAC: the authenticated -// actor extracted via middleware.UserKey must NOT equal the request's +// actor extracted via auth.UserKey must NOT equal the request's // RequestedBy — the service-layer check enforces this and the handler // surfaces it as HTTP 403. // @@ -153,7 +154,7 @@ func (h ApprovalHandler) decision(w http.ResponseWriter, r *http.Request, action // Extract authenticated actor. The auth middleware sets UserKey to the // API-key NamedAPIKey.Name (or empty for unauthenticated). RBAC at the // service layer requires a non-empty actor. - actor, _ := r.Context().Value(middleware.UserKey{}).(string) + actor, _ := r.Context().Value(auth.UserKey{}).(string) if actor == "" { ErrorWithRequestID(w, http.StatusUnauthorized, "authentication required to approve / reject", requestID) diff --git a/internal/api/handler/approval_test.go b/internal/api/handler/approval_test.go index fc73a40..223ab3d 100644 --- a/internal/api/handler/approval_test.go +++ b/internal/api/handler/approval_test.go @@ -10,7 +10,7 @@ import ( "sync" "testing" - "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/domain" "github.com/certctl-io/certctl/internal/repository" "github.com/certctl-io/certctl/internal/service" @@ -117,7 +117,7 @@ func reqWithActor(t *testing.T, method, target string, body string, actor string } req.Header.Set("Content-Type", "application/json") if actor != "" { - req = req.WithContext(context.WithValue(req.Context(), middleware.UserKey{}, actor)) + req = req.WithContext(context.WithValue(req.Context(), auth.UserKey{}, actor)) } if pathID != "" { req.SetPathValue("id", pathID) diff --git a/internal/api/handler/bulk_partial_failure_test.go b/internal/api/handler/bulk_partial_failure_test.go index 8c24b9e..eb885b7 100644 --- a/internal/api/handler/bulk_partial_failure_test.go +++ b/internal/api/handler/bulk_partial_failure_test.go @@ -172,7 +172,7 @@ func authenticatedContext(actor string) context.Context { type userKey struct{} // The middleware UserKey is a private type in the middleware package, so // in this handler test we can't construct one directly. Bulk-renew and - // bulk-reassign read the actor through the same middleware.GetUser path + // bulk-reassign read the actor through the same auth.GetUser path // that bulk-revoke does — adminContext() in the existing test suite is // the canonical helper. Reuse it (delivers both UserKey and AdminKey). _ = userKey{} diff --git a/internal/api/handler/bulk_renewal_handler_test.go b/internal/api/handler/bulk_renewal_handler_test.go index 06a1952..548b325 100644 --- a/internal/api/handler/bulk_renewal_handler_test.go +++ b/internal/api/handler/bulk_renewal_handler_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/domain" ) @@ -30,7 +31,7 @@ func (m *mockBulkRenewalService) BulkRenew(ctx context.Context, criteria domain. // bulk-renew is NOT admin-gated, any authenticated caller can use it. func authedContext() context.Context { ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id-renew") - ctx = context.WithValue(ctx, middleware.UserKey{}, "alice") + ctx = context.WithValue(ctx, auth.UserKey{}, "alice") return ctx } @@ -126,7 +127,7 @@ func TestBulkRenew_Handler_ActorAttribution(t *testing.T) { h.BulkRenew(w, req) if capturedActor != "alice" { - t.Errorf("actor not threaded from middleware.UserKey: got %q, want 'alice'", capturedActor) + t.Errorf("actor not threaded from auth.UserKey: got %q, want 'alice'", capturedActor) } } diff --git a/internal/api/handler/bulk_revocation.go b/internal/api/handler/bulk_revocation.go index 2936f5a..b8ee854 100644 --- a/internal/api/handler/bulk_revocation.go +++ b/internal/api/handler/bulk_revocation.go @@ -6,6 +6,7 @@ import ( "net/http" "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/domain" ) @@ -53,7 +54,7 @@ func (h BulkRevocationHandler) BulkRevoke(w http.ResponseWriter, r *http.Request // M-003: admin-only gate. Non-admin callers are rejected before any // criteria/body processing to avoid leaking validation behavior to // unauthorized actors. - if !middleware.IsAdmin(r.Context()) { + if !auth.IsAdmin(r.Context()) { ErrorWithRequestID(w, http.StatusForbidden, "Bulk revocation requires admin privileges", requestID) @@ -127,7 +128,7 @@ func (h BulkRevocationHandler) BulkRevokeEST(w http.ResponseWriter, r *http.Requ return } requestID := middleware.GetRequestID(r.Context()) - if !middleware.IsAdmin(r.Context()) { + if !auth.IsAdmin(r.Context()) { ErrorWithRequestID(w, http.StatusForbidden, "EST bulk revocation requires admin privileges", requestID) return diff --git a/internal/api/handler/bulk_revocation_handler_test.go b/internal/api/handler/bulk_revocation_handler_test.go index 1c91297..1c9ca40 100644 --- a/internal/api/handler/bulk_revocation_handler_test.go +++ b/internal/api/handler/bulk_revocation_handler_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/domain" ) @@ -31,7 +32,7 @@ func (m *mockBulkRevocationService) BulkRevoke(ctx context.Context, criteria dom // M-003: bulk revocation handler requires admin context to reach the service. func adminContext() context.Context { ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id-bulk") - ctx = context.WithValue(ctx, middleware.AdminKey{}, true) + ctx = context.WithValue(ctx, auth.AdminKey{}, true) return ctx } @@ -243,7 +244,7 @@ func TestBulkRevoke_AdminExplicitFalse_Returns403(t *testing.T) { req.Header.Set("Content-Type", "application/json") ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, false) + ctx = context.WithValue(ctx, auth.AdminKey{}, false) req = req.WithContext(ctx) w := httptest.NewRecorder() @@ -273,8 +274,8 @@ func TestBulkRevoke_AdminPermitted_ForwardsActor(t *testing.T) { req.Header.Set("Content-Type", "application/json") ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id") - ctx = context.WithValue(ctx, middleware.AdminKey{}, true) - ctx = context.WithValue(ctx, middleware.UserKey{}, "ops-admin") + ctx = context.WithValue(ctx, auth.AdminKey{}, true) + ctx = context.WithValue(ctx, auth.UserKey{}, "ops-admin") req = req.WithContext(ctx) w := httptest.NewRecorder() diff --git a/internal/api/handler/health.go b/internal/api/handler/health.go index c9a3f64..a7950b7 100644 --- a/internal/api/handler/health.go +++ b/internal/api/handler/health.go @@ -6,7 +6,7 @@ import ( "net/http" "time" - "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" ) // HealthHandler handles health and readiness check endpoints. @@ -152,8 +152,8 @@ func (h HealthHandler) AuthInfo(w http.ResponseWriter, r *http.Request) { func (h HealthHandler) AuthCheck(w http.ResponseWriter, r *http.Request) { response := map[string]interface{}{ "status": "authenticated", - "user": middleware.GetUser(r.Context()), - "admin": middleware.IsAdmin(r.Context()), + "user": auth.GetUser(r.Context()), + "admin": auth.IsAdmin(r.Context()), } JSON(w, http.StatusOK, response) } diff --git a/internal/api/handler/health_test.go b/internal/api/handler/health_test.go index 83b1759..6b7cd2c 100644 --- a/internal/api/handler/health_test.go +++ b/internal/api/handler/health_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" _ "github.com/lib/pq" // Bundle-5 / H-006: postgres driver for /ready DB-probe regression test ) @@ -238,8 +238,8 @@ func TestAuthCheck_AdminCaller_ReportsAdminTrue(t *testing.T) { handler := NewHealthHandler("api-key", nil) req := httptest.NewRequest(http.MethodGet, "/api/v1/auth/check", nil) - ctx := context.WithValue(req.Context(), middleware.AdminKey{}, true) - ctx = context.WithValue(ctx, middleware.UserKey{}, "ops-admin") + ctx := context.WithValue(req.Context(), auth.AdminKey{}, true) + ctx = context.WithValue(ctx, auth.UserKey{}, "ops-admin") req = req.WithContext(ctx) w := httptest.NewRecorder() @@ -276,8 +276,8 @@ func TestAuthCheck_NonAdminCaller_ReportsAdminFalse(t *testing.T) { handler := NewHealthHandler("api-key", nil) req := httptest.NewRequest(http.MethodGet, "/api/v1/auth/check", nil) - ctx := context.WithValue(req.Context(), middleware.AdminKey{}, false) - ctx = context.WithValue(ctx, middleware.UserKey{}, "alice") + ctx := context.WithValue(req.Context(), auth.AdminKey{}, false) + ctx = context.WithValue(ctx, auth.UserKey{}, "alice") req = req.WithContext(ctx) w := httptest.NewRecorder() diff --git a/internal/api/handler/intermediate_ca.go b/internal/api/handler/intermediate_ca.go index 3e424f4..0e26093 100644 --- a/internal/api/handler/intermediate_ca.go +++ b/internal/api/handler/intermediate_ca.go @@ -9,6 +9,7 @@ import ( "time" "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/crypto/signer" "github.com/certctl-io/certctl/internal/domain" "github.com/certctl-io/certctl/internal/service" @@ -36,7 +37,7 @@ type IntermediateCAServicer interface { // All routes are pinned at /api/v1/issuers/{id}/intermediates and // /api/v1/intermediates/{id}. // -// Admin gate: every method calls middleware.IsAdmin first and surfaces +// Admin gate: every method calls auth.IsAdmin first and surfaces // HTTP 403 for non-admin Bearer callers (M-003 admin-gating pattern, // matches AdminCRLCacheHandler / AdminESTHandler / AdminSCEPIntuneHandler). // CA hierarchy management is a high-blast-radius surface — adding a @@ -111,7 +112,7 @@ func (h IntermediateCAHandler) Create(w http.ResponseWriter, r *http.Request) { Error(w, http.StatusMethodNotAllowed, "Method not allowed") return } - if !middleware.IsAdmin(r.Context()) { + if !auth.IsAdmin(r.Context()) { Error(w, http.StatusForbidden, "Admin access required") return } @@ -122,7 +123,7 @@ func (h IntermediateCAHandler) Create(w http.ResponseWriter, r *http.Request) { ErrorWithRequestID(w, http.StatusBadRequest, "issuer id required", requestID) return } - actor, _ := r.Context().Value(middleware.UserKey{}).(string) + actor, _ := r.Context().Value(auth.UserKey{}).(string) if actor == "" { ErrorWithRequestID(w, http.StatusUnauthorized, "authentication required", requestID) @@ -211,7 +212,7 @@ func (h IntermediateCAHandler) List(w http.ResponseWriter, r *http.Request) { Error(w, http.StatusMethodNotAllowed, "Method not allowed") return } - if !middleware.IsAdmin(r.Context()) { + if !auth.IsAdmin(r.Context()) { Error(w, http.StatusForbidden, "Admin access required") return } @@ -237,7 +238,7 @@ func (h IntermediateCAHandler) Get(w http.ResponseWriter, r *http.Request) { Error(w, http.StatusMethodNotAllowed, "Method not allowed") return } - if !middleware.IsAdmin(r.Context()) { + if !auth.IsAdmin(r.Context()) { Error(w, http.StatusForbidden, "Admin access required") return } @@ -270,7 +271,7 @@ func (h IntermediateCAHandler) Retire(w http.ResponseWriter, r *http.Request) { Error(w, http.StatusMethodNotAllowed, "Method not allowed") return } - if !middleware.IsAdmin(r.Context()) { + if !auth.IsAdmin(r.Context()) { Error(w, http.StatusForbidden, "Admin access required") return } @@ -281,7 +282,7 @@ func (h IntermediateCAHandler) Retire(w http.ResponseWriter, r *http.Request) { ErrorWithRequestID(w, http.StatusBadRequest, "id required", requestID) return } - actor, _ := r.Context().Value(middleware.UserKey{}).(string) + actor, _ := r.Context().Value(auth.UserKey{}).(string) if actor == "" { ErrorWithRequestID(w, http.StatusUnauthorized, "authentication required", requestID) diff --git a/internal/api/handler/intermediate_ca_test.go b/internal/api/handler/intermediate_ca_test.go index 8318794..136314a 100644 --- a/internal/api/handler/intermediate_ca_test.go +++ b/internal/api/handler/intermediate_ca_test.go @@ -16,7 +16,7 @@ import ( "testing" "time" - "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/domain" "github.com/certctl-io/certctl/internal/service" ) @@ -80,8 +80,8 @@ func (m *mockIntermediateCAService) LoadHierarchy(ctx context.Context, issuerID // authenticated user — the standard "admin caller" shape for these // tests. func withAdmin(actor string, admin bool) context.Context { - ctx := context.WithValue(context.Background(), middleware.UserKey{}, actor) - ctx = context.WithValue(ctx, middleware.AdminKey{}, admin) + ctx := context.WithValue(context.Background(), auth.UserKey{}, actor) + ctx = context.WithValue(ctx, auth.AdminKey{}, admin) return ctx } @@ -177,8 +177,8 @@ func TestIntermediateCA_Handler_AdminExplicitFalse_Returns403(t *testing.T) { bytes.NewReader([]byte(`{"name":"r"}`))) req.SetPathValue("id", "iss-1") // AdminKey explicitly set to false — distinct from missing key. - ctx := context.WithValue(context.Background(), middleware.UserKey{}, "alice") - ctx = context.WithValue(ctx, middleware.AdminKey{}, false) + ctx := context.WithValue(context.Background(), auth.UserKey{}, "alice") + ctx = context.WithValue(ctx, auth.AdminKey{}, false) req = req.WithContext(ctx) w := httptest.NewRecorder() h.Create(w, req) diff --git a/internal/api/handler/m008_admin_gate_test.go b/internal/api/handler/m008_admin_gate_test.go index 9cfb5b4..9331d9b 100644 --- a/internal/api/handler/m008_admin_gate_test.go +++ b/internal/api/handler/m008_admin_gate_test.go @@ -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) } } diff --git a/internal/api/handler/response.go b/internal/api/handler/response.go index 92a9312..f4bbc2c 100644 --- a/internal/api/handler/response.go +++ b/internal/api/handler/response.go @@ -9,7 +9,7 @@ import ( "strings" "time" - "github.com/certctl-io/certctl/internal/api/middleware" + "github.com/certctl-io/certctl/internal/auth" ) // resolveActor extracts the authenticated named-key identity from the request @@ -23,7 +23,7 @@ import ( // or "api" — always go through this helper so the named-key identity flows to // services and the audit trail. func resolveActor(ctx context.Context) string { - if user := middleware.GetUser(ctx); user != "" { + if user := auth.GetUser(ctx); user != "" { return user } return "api" diff --git a/internal/api/middleware/audit.go b/internal/api/middleware/audit.go index 7199802..722eabb 100644 --- a/internal/api/middleware/audit.go +++ b/internal/api/middleware/audit.go @@ -12,6 +12,8 @@ import ( "strings" "sync" "time" + + "github.com/certctl-io/certctl/internal/auth" ) // AuditRecorder is the interface that the audit middleware uses to record API calls. @@ -115,7 +117,7 @@ func (a *AuditMiddleware) Middleware(next http.Handler) http.Handler { // Extract actor from auth context actor := "anonymous" - if user := GetUser(r.Context()); user != "" { + if user := auth.GetUser(r.Context()); user != "" { actor = user } diff --git a/internal/api/middleware/audit_test.go b/internal/api/middleware/audit_test.go index 35993a5..e4e703d 100644 --- a/internal/api/middleware/audit_test.go +++ b/internal/api/middleware/audit_test.go @@ -11,6 +11,8 @@ import ( "sync" "testing" "time" + + "github.com/certctl-io/certctl/internal/auth" ) // mockAuditRecorder captures RecordAPICall invocations for testing. @@ -271,7 +273,7 @@ func TestAuditLog_ExtractsAuthenticatedActor(t *testing.T) { req := httptest.NewRequest(http.MethodDelete, "/api/v1/certificates/mc-1", nil) // Simulate auth middleware having set the named-key identity in context // (post-M-002: actor is the named-key name, not the old "api-key-user"). - ctx := context.WithValue(req.Context(), UserKey{}, "ops-admin") + ctx := context.WithValue(req.Context(), auth.UserKey{}, "ops-admin") req = req.WithContext(ctx) rr := httptest.NewRecorder() diff --git a/internal/api/middleware/middleware.go b/internal/api/middleware/middleware.go index 1c7ca6c..10e27bd 100644 --- a/internal/api/middleware/middleware.go +++ b/internal/api/middleware/middleware.go @@ -2,9 +2,6 @@ package middleware import ( "context" - "crypto/sha256" - "crypto/subtle" - "encoding/hex" "fmt" "log" "log/slog" @@ -14,24 +11,22 @@ import ( "time" "github.com/google/uuid" + + "github.com/certctl-io/certctl/internal/auth" ) +// Bundle 1 / Phase 0: the auth surface (NamedAPIKey, HashAPIKey, AuthConfig, +// NewAuthWithNamedKeys, NewAuth, UserKey, AdminKey, GetUser, IsAdmin) moved +// to internal/auth/. The rate limiter below still keys per-user via +// auth.GetUser(ctx); other middlewares in this package are auth-agnostic. +// +// Existing callers continue to import internal/auth/middleware "as +// middleware" only for the non-auth helpers below; auth-related references +// have been migrated to the new package. + // RequestIDKey is the context key for storing request IDs. type RequestIDKey struct{} -// UserKey is the context key for storing authenticated user information. -type UserKey struct{} - -// AdminKey is the context key for storing admin flag information. -type AdminKey struct{} - -// NamedAPIKey represents a named API key with optional admin flag. -type NamedAPIKey struct { - Name string - Key string - Admin bool -} - // RequestID middleware generates a unique request ID and adds it to the request context and response headers. func RequestID(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -46,7 +41,7 @@ func RequestID(next http.Handler) http.Handler { // Deprecated: Use NewLogging for structured logging with slog. // // CWE-117 log-injection defense: r.Method and r.URL.Path are -// attacker-controllable (request-line bytes — Go's net/http leaves +// attacker-controllable (request-line bytes; Go's net/http leaves // percent-decoded path segments in r.URL.Path, which can include CR/LF // in the decoded form even though the raw HTTP request line cannot). // strings.ReplaceAll on CR/LF/NUL strips the forgery vector before the @@ -54,7 +49,7 @@ func RequestID(next http.Handler) http.Handler { // // The replacement is intentionally inlined at the call site (literal // strings.ReplaceAll chains) because CodeQL's go/log-injection -// taint tracker only recognizes that exact pattern as a sanitizer — +// taint tracker only recognizes that exact pattern as a sanitizer; // strings.NewReplacer / wrapper helpers don't trigger the recognition, // reopening the alert. The OWASP example in the CodeQL rule docs uses // the same pattern. @@ -71,7 +66,7 @@ func Logging(next http.Handler) http.Handler { requestID := getRequestID(r.Context()) // Strip CR/LF/NUL from attacker-controllable request fields - // before logging. Inlined per CodeQL #32 — the ReplaceAll + // before logging. Inlined per CodeQL #32; the ReplaceAll // chain is the pattern the analyzer pattern-matches as a // sanitizer. method := strings.ReplaceAll(r.Method, "\n", "") @@ -133,143 +128,11 @@ func Recovery(next http.Handler) http.Handler { }) } -// HashAPIKey computes the SHA-256 hash of an API key for secure storage. -// We use SHA-256 rather than bcrypt because API keys are high-entropy -// random strings (not user-chosen passwords), so rainbow tables and -// brute-force attacks are not a practical concern. -func HashAPIKey(key string) string { - h := sha256.Sum256([]byte(key)) - return hex.EncodeToString(h[:]) -} - -// AuthConfig holds configuration for the Auth middleware. -// -// G-1 (P1): valid Type values are "api-key" or "none" only. "jwt" was -// removed because no JWT middleware ships with certctl (silent auth -// downgrade pre-G-1). The single source of truth for the allowed set -// lives at internal/config.AuthType / config.ValidAuthTypes() — prefer -// those constants over string literals when comparing. -type AuthConfig struct { - Type string // "api-key" or "none" (see config.AuthType constants) - Secret string // The raw API key or comma-separated list of valid API keys -} - -// NewAuthWithNamedKeys creates an authentication middleware that validates -// Bearer tokens against a set of named API keys. Each key carries a name -// (propagated as the actor via context) and an admin flag (consulted by -// authorization gates such as bulk revocation). -// -// When namedKeys is empty the returned middleware is a no-op pass-through, -// which is used in demo/development mode (CERTCTL_AUTH_TYPE=none). When one -// or more keys are provided, requests must include a matching Bearer token -// or they are rejected with 401. -func NewAuthWithNamedKeys(namedKeys []NamedAPIKey) func(http.Handler) http.Handler { - if len(namedKeys) == 0 { - return func(next http.Handler) http.Handler { - return next - } - } - - // Pre-compute hashes of all valid keys for constant-time comparison. - type keyEntry struct { - hash string - name string - admin bool - } - var entries []keyEntry - for _, nk := range namedKeys { - entries = append(entries, keyEntry{ - hash: HashAPIKey(nk.Key), - name: nk.Name, - admin: nk.Admin, - }) - } - - // Warn if only one key is configured in production mode - if len(entries) == 1 { - slog.Warn("only one API key configured — consider adding a rotation key for zero-downtime rotation") - } - - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - authHeader := r.Header.Get("Authorization") - if authHeader == "" { - w.Header().Set("Content-Type", "application/json; charset=utf-8") - w.Header().Set("WWW-Authenticate", `Bearer realm="certctl"`) - http.Error(w, `{"error":"Authorization header required"}`, http.StatusUnauthorized) - return - } - - // Extract Bearer token - if len(authHeader) < 8 || authHeader[:7] != "Bearer " { - w.Header().Set("Content-Type", "application/json; charset=utf-8") - http.Error(w, `{"error":"Invalid Authorization header format, expected: Bearer "}`, http.StatusUnauthorized) - return - } - - token := authHeader[7:] - tokenHash := HashAPIKey(token) - - // Check against all valid keys using constant-time comparison - var matched *keyEntry - for i := range entries { - if subtle.ConstantTimeCompare([]byte(tokenHash), []byte(entries[i].hash)) == 1 { - matched = &entries[i] - break - } - } - - if matched == nil { - w.Header().Set("Content-Type", "application/json; charset=utf-8") - http.Error(w, `{"error":"Invalid API key"}`, http.StatusUnauthorized) - return - } - - // Store the authenticated identity and admin flag in context - ctx := context.WithValue(r.Context(), UserKey{}, matched.name) - ctx = context.WithValue(ctx, AdminKey{}, matched.admin) - next.ServeHTTP(w, r.WithContext(ctx)) - }) - } -} - -// NewAuth is a legacy shim that converts a comma-separated Secret list into -// synthesized legacy-key-N named entries and delegates to NewAuthWithNamedKeys. -// It preserves the pre-M-002 behavior for callers that still pass raw AuthConfig -// (primarily cmd/server/main_test.go). The synthesized actor is "legacy-key-N" -// rather than the old hardcoded "api-key-user" so audit events carry -// meaningful identity even on the legacy path. -// -// Deprecated: Use NewAuthWithNamedKeys with explicit NamedAPIKey entries. -func NewAuth(cfg AuthConfig) func(http.Handler) http.Handler { - if cfg.Type == "none" { - return func(next http.Handler) http.Handler { - return next - } - } - - var namedKeys []NamedAPIKey - idx := 0 - for _, k := range strings.Split(cfg.Secret, ",") { - k = strings.TrimSpace(k) - if k == "" { - continue - } - namedKeys = append(namedKeys, NamedAPIKey{ - Name: fmt.Sprintf("legacy-key-%d", idx), - Key: k, - Admin: false, - }) - idx++ - } - return NewAuthWithNamedKeys(namedKeys) -} - // RateLimitConfig holds configuration for the rate limiter. // // Bundle B / Audit M-025 (OWASP ASVS L2 §11.2.1) extends this with per-user // and per-IP keying. The historic RPS / BurstSize fields are preserved for -// source compatibility — they now describe the per-key budget rather than +// source compatibility; they now describe the per-key budget rather than // the global budget. PerUserRPS / PerUserBurstSize, when non-zero, override // RPS / BurstSize for authenticated callers; the IP-keyed fallback // continues to use RPS / BurstSize so unauthenticated callers don't get @@ -278,8 +141,9 @@ type RateLimitConfig struct { RPS float64 // Tokens per second per key (default applies to IP-keyed buckets) BurstSize int // Max tokens per key (default applies to IP-keyed buckets) - // PerUserRPS overrides RPS for authenticated callers (keyed by UserKey - // in context). Zero means "use RPS as the authenticated budget too". + // PerUserRPS overrides RPS for authenticated callers (keyed by + // auth.UserKey in context). Zero means "use RPS as the authenticated + // budget too". PerUserRPS float64 // PerUserBurstSize overrides BurstSize for authenticated callers. @@ -295,11 +159,11 @@ type RateLimitConfig struct { // authenticated user and each unauthenticated IP gets its own bucket. Keys // are computed per request: // -// - Authenticated: "user:" + middleware.GetUser(ctx) +// - Authenticated: "user:" + auth.GetUser(ctx) // - Unauthenticated: "ip:" + r.RemoteAddr's host portion // // The bucket map is sync.RWMutex-guarded; create-on-demand for new keys. -// There is no eviction — for a long-running server with millions of unique +// There is no eviction; for a long-running server with millions of unique // IPs this can leak memory. A future enhancement is per-key TTL via a // lazy sweeper. For now the leak is bounded by realistic operator IP // fan-out and is acceptable per OWASP ASVS L2 (the threat model is abuse @@ -339,9 +203,9 @@ func NewRateLimiter(cfg RateLimitConfig) func(http.Handler) http.Handler { // rateLimitKey computes the per-request bucket key. Authenticated callers // get a "user:" key derived from the UserKey context value populated -// by NewAuthWithNamedKeys; everyone else falls back to "ip:" parsed -// from r.RemoteAddr (X-Forwarded-For is intentionally NOT consulted here -// — operators behind a trusted proxy must configure that proxy to set +// by auth.NewAuthWithNamedKeys; everyone else falls back to "ip:" +// parsed from r.RemoteAddr (X-Forwarded-For is intentionally NOT consulted +// here; operators behind a trusted proxy must configure that proxy to set // RemoteAddr correctly, or the rate limiter would be trivially bypassable // by spoofing the header). // @@ -349,7 +213,7 @@ func NewRateLimiter(cfg RateLimitConfig) func(http.Handler) http.Handler { // unauthenticated so a misconfigured auth middleware doesn't grant the // same bucket to every anonymous request. func rateLimitKey(r *http.Request) (string, bool) { - if user := GetUser(r.Context()); user != "" { + if user := auth.GetUser(r.Context()); user != "" { return "user:" + user, true } host := r.RemoteAddr @@ -463,7 +327,7 @@ func NewCORS(cfg CORSConfig) func(http.Handler) http.Handler { // Security default: deny CORS when no origins are configured. // This prevents CSRF attacks from arbitrary origins. if len(cfg.AllowedOrigins) == 0 { - // No CORS headers set — only same-origin requests can read response + // No CORS headers set; only same-origin requests can read response if r.Method == http.MethodOptions { w.WriteHeader(http.StatusNoContent) return @@ -538,23 +402,6 @@ func getRequestID(ctx context.Context) string { return id } -// GetUser extracts the authenticated user from context. -// Returns the name of the matched API key and whether it was found. -func GetUser(ctx context.Context) string { - user, ok := ctx.Value(UserKey{}).(string) - if !ok { - return "" - } - return user -} - -// IsAdmin extracts the admin flag from context. -// Returns true if the authenticated user has admin privileges. -func IsAdmin(ctx context.Context) bool { - admin, ok := ctx.Value(AdminKey{}).(bool) - return ok && admin -} - // responseWriter wraps http.ResponseWriter to capture the status code. type responseWriter struct { http.ResponseWriter diff --git a/internal/api/middleware/ratelimit_keyed_test.go b/internal/api/middleware/ratelimit_keyed_test.go index 2a7cd02..e9971c0 100644 --- a/internal/api/middleware/ratelimit_keyed_test.go +++ b/internal/api/middleware/ratelimit_keyed_test.go @@ -5,6 +5,8 @@ import ( "net/http" "net/http/httptest" "testing" + + "github.com/certctl-io/certctl/internal/auth" ) // Bundle B / Audit M-025 (OWASP ASVS L2 §11.2.1): per-key rate-limiter @@ -61,7 +63,7 @@ func TestRateLimiter_M025_SameUserDifferentIPsShareBucket(t *testing.T) { mkReq := func(remote string) *http.Request { req := httptest.NewRequest(http.MethodGet, "/", nil) req.RemoteAddr = remote - ctx := context.WithValue(req.Context(), UserKey{}, "alice") + ctx := context.WithValue(req.Context(), auth.UserKey{}, "alice") return req.WithContext(ctx) } @@ -88,7 +90,7 @@ func TestRateLimiter_M025_TwoUsersHaveIndependentBuckets(t *testing.T) { mkReq := func(user string) *http.Request { req := httptest.NewRequest(http.MethodGet, "/", nil) req.RemoteAddr = "10.0.0.1:54321" - ctx := context.WithValue(req.Context(), UserKey{}, user) + ctx := context.WithValue(req.Context(), auth.UserKey{}, user) return req.WithContext(ctx) } @@ -145,7 +147,7 @@ func TestRateLimiter_M025_PerUserBudgetOverride(t *testing.T) { userReq := func() *http.Request { req := httptest.NewRequest(http.MethodGet, "/", nil) req.RemoteAddr = "10.0.0.42:54321" - ctx := context.WithValue(req.Context(), UserKey{}, "carol") + ctx := context.WithValue(req.Context(), auth.UserKey{}, "carol") return req.WithContext(ctx) } for i := 1; i <= 5; i++ { @@ -171,7 +173,7 @@ func TestRateLimiter_M025_EmptyUserKeyTreatedAsAnonymous(t *testing.T) { mkReq := func(remote string) *http.Request { req := httptest.NewRequest(http.MethodGet, "/", nil) req.RemoteAddr = remote - ctx := context.WithValue(req.Context(), UserKey{}, "") + ctx := context.WithValue(req.Context(), auth.UserKey{}, "") return req.WithContext(ctx) } diff --git a/internal/auth/apikey.go b/internal/auth/apikey.go new file mode 100644 index 0000000..76c5d94 --- /dev/null +++ b/internal/auth/apikey.go @@ -0,0 +1,28 @@ +package auth + +import ( + "crypto/sha256" + "encoding/hex" +) + +// NamedAPIKey represents a named API key with optional admin flag. +// +// Name is the canonical actor identity propagated through the request +// context (UserKey) and into the audit trail. Two NamedAPIKey rows +// MAY share a Name during a rotation overlap window per audit L-004 +// (CWE-924); both keys validate to the same actor + admin flag so the +// per-user rate-limit bucket stays consistent during rotation. +type NamedAPIKey struct { + Name string + Key string + Admin bool +} + +// HashAPIKey computes the SHA-256 hash of an API key for secure storage. +// We use SHA-256 rather than bcrypt because API keys are high-entropy +// random strings (not user-chosen passwords), so rainbow tables and +// brute-force attacks are not a practical concern. +func HashAPIKey(key string) string { + h := sha256.Sum256([]byte(key)) + return hex.EncodeToString(h[:]) +} diff --git a/internal/auth/context.go b/internal/auth/context.go new file mode 100644 index 0000000..9bc5970 --- /dev/null +++ b/internal/auth/context.go @@ -0,0 +1,50 @@ +// Package auth holds the certctl auth surface: API-key validation, the +// authenticated-actor context keys, and the helpers that consumers across +// the codebase use to read the actor identity (rate limiter, audit +// recorder, handler-level admin gates, GUI affordance hints). +// +// Bundle 1 / Phase 0 split this code out of internal/api/middleware so +// Bundle 2 (OIDC + sessions) and the broader RBAC primitive (roles + +// permissions + scoped grants) have a clean home that doesn't bloat the +// generic-middleware package. Phase 0 is a pure refactor; behaviour +// matches the pre-extract NewAuthWithNamedKeys / NewAuth surface +// byte-for-byte. +package auth + +import "context" + +// UserKey is the context key for storing the authenticated actor's +// canonical name. Populated by Middleware (a.k.a. NewAuthWithNamedKeys) +// from the matched NamedAPIKey.Name. Read by GetUser. +type UserKey struct{} + +// AdminKey is the context key for storing the admin flag. Populated by +// Middleware from the matched NamedAPIKey.Admin. Read by IsAdmin. +// +// Bundle 1 keeps the boolean shape for backwards compatibility with the +// pre-RBAC handler gates. Phase 3 introduces RequirePermission and the +// boolean becomes informational only (admin role membership ↔ this flag). +type AdminKey struct{} + +// GetUser extracts the authenticated user from context. Returns the name +// of the matched API key, or "" if the request was not authenticated +// (none mode, missing Bearer, or a misconfigured chain). +func GetUser(ctx context.Context) string { + user, ok := ctx.Value(UserKey{}).(string) + if !ok { + return "" + } + return user +} + +// IsAdmin extracts the admin flag from context. Returns true only when +// the authenticated actor's NamedAPIKey carried Admin=true. +// +// Bundle 1 maintains the boolean for back-compat. Bundle 1 Phase 3 +// introduces auth.RequirePermission as the load-bearing authorization +// gate; legacy IsAdmin callers (5 admin handlers tracked in M-008) +// migrate to RequirePermission in that phase. +func IsAdmin(ctx context.Context) bool { + admin, ok := ctx.Value(AdminKey{}).(bool) + return ok && admin +} diff --git a/internal/auth/middleware.go b/internal/auth/middleware.go new file mode 100644 index 0000000..29b55fe --- /dev/null +++ b/internal/auth/middleware.go @@ -0,0 +1,141 @@ +package auth + +import ( + "context" + "crypto/subtle" + "fmt" + "log/slog" + "net/http" + "strings" +) + +// AuthConfig holds configuration for the legacy NewAuth shim. +// +// G-1 (P1): valid Type values are "api-key" or "none" only. "jwt" was +// removed because no JWT middleware ships with certctl (silent auth +// downgrade pre-G-1). The single source of truth for the allowed set +// lives at internal/config.AuthType / config.ValidAuthTypes(); prefer +// those constants over string literals when comparing. +// +// Bundle 2 will extend ValidAuthTypes() with "oidc"; Bundle 1 leaves +// the surface unchanged. +type AuthConfig struct { + Type string // "api-key" or "none" (see config.AuthType constants) + Secret string // The raw API key or comma-separated list of valid API keys +} + +// NewAuthWithNamedKeys creates an authentication middleware that validates +// Bearer tokens against a set of named API keys. Each key carries a name +// (propagated as the actor via context) and an admin flag (consulted by +// authorization gates such as bulk revocation). +// +// When namedKeys is empty the returned middleware is a no-op pass-through, +// which is used in demo/development mode (CERTCTL_AUTH_TYPE=none). When one +// or more keys are provided, requests must include a matching Bearer token +// or they are rejected with 401. +// +// Bundle 1 Phase 3 extends Middleware with the RBAC primitive. This +// function continues to exist as the API-key validator; Phase 3 wraps it +// with the role lookup that populates the future ActorIDKey / RolesKey +// context values. +func NewAuthWithNamedKeys(namedKeys []NamedAPIKey) func(http.Handler) http.Handler { + if len(namedKeys) == 0 { + return func(next http.Handler) http.Handler { + return next + } + } + + // Pre-compute hashes of all valid keys for constant-time comparison. + type keyEntry struct { + hash string + name string + admin bool + } + var entries []keyEntry + for _, nk := range namedKeys { + entries = append(entries, keyEntry{ + hash: HashAPIKey(nk.Key), + name: nk.Name, + admin: nk.Admin, + }) + } + + // Warn if only one key is configured in production mode + if len(entries) == 1 { + slog.Warn("only one API key configured — consider adding a rotation key for zero-downtime rotation") + } + + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + authHeader := r.Header.Get("Authorization") + if authHeader == "" { + w.Header().Set("Content-Type", "application/json; charset=utf-8") + w.Header().Set("WWW-Authenticate", `Bearer realm="certctl"`) + http.Error(w, `{"error":"Authorization header required"}`, http.StatusUnauthorized) + return + } + + // Extract Bearer token + if len(authHeader) < 8 || authHeader[:7] != "Bearer " { + w.Header().Set("Content-Type", "application/json; charset=utf-8") + http.Error(w, `{"error":"Invalid Authorization header format, expected: Bearer "}`, http.StatusUnauthorized) + return + } + + token := authHeader[7:] + tokenHash := HashAPIKey(token) + + // Check against all valid keys using constant-time comparison + var matched *keyEntry + for i := range entries { + if subtle.ConstantTimeCompare([]byte(tokenHash), []byte(entries[i].hash)) == 1 { + matched = &entries[i] + break + } + } + + if matched == nil { + w.Header().Set("Content-Type", "application/json; charset=utf-8") + http.Error(w, `{"error":"Invalid API key"}`, http.StatusUnauthorized) + return + } + + // Store the authenticated identity and admin flag in context + ctx := context.WithValue(r.Context(), UserKey{}, matched.name) + ctx = context.WithValue(ctx, AdminKey{}, matched.admin) + next.ServeHTTP(w, r.WithContext(ctx)) + }) + } +} + +// NewAuth is a legacy shim that converts a comma-separated Secret list into +// synthesized legacy-key-N named entries and delegates to NewAuthWithNamedKeys. +// It preserves the pre-M-002 behavior for callers that still pass raw AuthConfig +// (primarily cmd/server/main_test.go). The synthesized actor is "legacy-key-N" +// rather than the old hardcoded "api-key-user" so audit events carry +// meaningful identity even on the legacy path. +// +// Deprecated: Use NewAuthWithNamedKeys with explicit NamedAPIKey entries. +func NewAuth(cfg AuthConfig) func(http.Handler) http.Handler { + if cfg.Type == "none" { + return func(next http.Handler) http.Handler { + return next + } + } + + var namedKeys []NamedAPIKey + idx := 0 + for _, k := range strings.Split(cfg.Secret, ",") { + k = strings.TrimSpace(k) + if k == "" { + continue + } + namedKeys = append(namedKeys, NamedAPIKey{ + Name: fmt.Sprintf("legacy-key-%d", idx), + Key: k, + Admin: false, + }) + idx++ + } + return NewAuthWithNamedKeys(namedKeys) +} diff --git a/internal/api/middleware/auth_test.go b/internal/auth/middleware_test.go similarity index 99% rename from internal/api/middleware/auth_test.go rename to internal/auth/middleware_test.go index 55b96cb..36189ae 100644 --- a/internal/api/middleware/auth_test.go +++ b/internal/auth/middleware_test.go @@ -1,4 +1,4 @@ -package middleware +package auth import ( "net/http" diff --git a/internal/api/middleware/auth_l004_rotation_test.go b/internal/auth/rotation_test.go similarity index 99% rename from internal/api/middleware/auth_l004_rotation_test.go rename to internal/auth/rotation_test.go index 5ab290b..6d20bed 100644 --- a/internal/api/middleware/auth_l004_rotation_test.go +++ b/internal/auth/rotation_test.go @@ -1,4 +1,4 @@ -package middleware +package auth import ( "net/http" diff --git a/internal/auth/testfixtures.go b/internal/auth/testfixtures.go new file mode 100644 index 0000000..7c46574 --- /dev/null +++ b/internal/auth/testfixtures.go @@ -0,0 +1,32 @@ +package auth + +import "context" + +// WithActor builds a context with UserKey populated, mirroring what +// NewAuthWithNamedKeys produces for a real authenticated request. Used +// by handler / service / middleware tests so they don't construct the +// context manually with internal context-key types. +// +// Phase 0 ships UserKey + AdminKey only; Phase 3 of Bundle 1 introduces +// the RBAC context (ActorIDKey, ActorTypeKey, RolesKey) and this helper +// will be extended to populate those too. Until then, admin should be +// passed via WithAdmin (separate helper below) to mirror the matched-key +// flag. +func WithActor(ctx context.Context, name string) context.Context { + return context.WithValue(ctx, UserKey{}, name) +} + +// WithAdmin sets the AdminKey flag on the supplied context. Tests calling +// WithActor + WithAdmin together produce a context indistinguishable from +// what NewAuthWithNamedKeys produces for an admin-flagged NamedAPIKey. +func WithAdmin(ctx context.Context, admin bool) context.Context { + return context.WithValue(ctx, AdminKey{}, admin) +} + +// WithActorAdmin is a convenience for the common "admin caller named X" +// pattern across handler tests. +func WithActorAdmin(ctx context.Context, name string, admin bool) context.Context { + ctx = WithActor(ctx, name) + ctx = WithAdmin(ctx, admin) + return ctx +} diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index fa898a9..f4375e4 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -1310,7 +1310,7 @@ func registerHealthTools(s *gomcp.Server, c *Client) { // assistants for cert-renewal in regulated environments need natural-language // approve/reject. The service layer enforces ErrApproveBySameActor (the // requesting actor cannot self-approve) and the handler extracts the -// decided_by actor from middleware.UserKey — so the MCP server's API key +// decided_by actor from auth.UserKey — so the MCP server's API key // identity becomes the audit-trail actor automatically. Two-person integrity // is preserved as long as the MCP server's key is distinct from the // requesting actor's; the tool inputs deliberately omit any actor_id field @@ -1706,7 +1706,7 @@ func registerDiscoveryReadTools(s *gomcp.Server, c *Client) { // // 2026-05-05 CLI/API/MCP↔GUI parity audit closure. Rank 8 primitive // (multi-level CA hierarchy management). The handlers are admin-gated via -// middleware.IsAdmin — non-admin callers see HTTP 403 regardless of MCP +// auth.IsAdmin — non-admin callers see HTTP 403 regardless of MCP // surface. We expose the full management API rather than carving it off // because the operator ran the original Rank 8 deliverable to make this // a first-class managed primitive; gating by API key role at the handler diff --git a/internal/mcp/types.go b/internal/mcp/types.go index 426c344..f048528 100644 --- a/internal/mcp/types.go +++ b/internal/mcp/types.go @@ -361,7 +361,7 @@ type ListApprovalsInput struct { // ApprovalDecisionInput is the MCP tool input for approve / reject endpoints. // The decided_by actor is derived server-side from the authenticated API-key -// name (middleware.UserKey) — NOT from this body. The two-person-integrity +// name (auth.UserKey) — NOT from this body. The two-person-integrity // contract (ErrApproveBySameActor) is enforced regardless of who pushes the // decision through MCP, so as long as the MCP server's API key identity is // distinct from the requesting actor, the contract holds.