gsadmin/certctl
1
0
Fork 0
mirror of https://github.com/shankar0123/certctl.git synced 2026-06-07 14:51:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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 71ebccb8ba
commit 99a012e3be
32 changed files with 397 additions and 283 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/server/main.go
+6 -5
View File
@@ -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,
})
cmd/server/main_test.go
+5 -4
View File
@@ -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)
internal/api/handler/admin_crl_cache.go
+2 -2
View File
@@ -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
}
internal/api/handler/admin_crl_cache_test.go
+6 -5
View File
@@ -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()
internal/api/handler/admin_est.go
+3 -3
View File
@@ -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
}
internal/api/handler/admin_est_test.go
+10 -9
View File
@@ -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)
internal/api/handler/admin_scep_intune.go
+4 -4
View File
@@ -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
}
internal/api/handler/admin_scep_intune_test.go
+20 -19
View File
@@ -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)
internal/api/handler/approval.go
+3 -2
View File
@@ -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)
internal/api/handler/approval_test.go
+2 -2
View File
@@ -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)
internal/api/handler/bulk_partial_failure_test.go
+1 -1
View File
@@ -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{}
internal/api/handler/bulk_renewal_handler_test.go
+3 -2
View File
@@ -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)
}
}
internal/api/handler/bulk_revocation.go
+3 -2
View File
@@ -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
internal/api/handler/bulk_revocation_handler_test.go
+5 -4
View File
@@ -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()
internal/api/handler/health.go
+3 -3
View File
@@ -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)
}
internal/api/handler/health_test.go
+5 -5
View File
@@ -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()
internal/api/handler/intermediate_ca.go
+8 -7
View File
@@ -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)
internal/api/handler/intermediate_ca_test.go
+5 -5
View File
@@ -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)
internal/api/handler/m008_admin_gate_test.go
+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)
}
}
internal/api/handler/response.go
+2 -2
View File
@@ -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"
internal/api/middleware/audit.go
+3 -1
View File
@@ -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
}
internal/api/middleware/audit_test.go
+3 -1
View File
@@ -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()
internal/api/middleware/middleware.go
+25 -178
View File
@@ -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 <token>"}`, 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:<name>" key derived from the UserKey context value populated
// by NewAuthWithNamedKeys; everyone else falls back to "ip:<host>" 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:<host>"
// 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
internal/api/middleware/ratelimit_keyed_test.go
+6 -4
View File
@@ -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)
}
internal/auth/apikey.go
+28
View File
@@ -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[:])
}
internal/auth/context.go
+50
View File
@@ -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
}
internal/auth/middleware.go
+141
View File
@@ -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 <token>"}`, 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)
}
internal/api/middleware/auth_test.go → internal/auth/middleware_test.go
+1 -1
View File
@@ -1,4 +1,4 @@
package middleware
package auth
import (
"net/http"
internal/api/middleware/auth_l004_rotation_test.go → internal/auth/rotation_test.go
+1 -1
View File
@@ -1,4 +1,4 @@
package middleware
package auth
import (
"net/http"
internal/auth/testfixtures.go
+32
View File
@@ -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
}
internal/mcp/tools.go
+2 -2
View File
@@ -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
internal/mcp/types.go
+1 -1
View File
@@ -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.