mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 13:51:36 +00:00
auth-bundle-1 Phase 0-5 closure: demo-mode wire, named-key backfill, AuthCheck enrichment, OpenAPI schema, intermediate-ca comment refresh
Closes the 5 gaps the post-Phase-5 audit flagged on dev/auth-bundle-1.
C1: cmd/server/main.go now selects auth.NewDemoModeAuth() when
CERTCTL_AUTH_TYPE=none and falls back to auth.NewAuthWithNamedKeys
otherwise. Pre-closure, the no-op pass-through that
NewAuthWithNamedKeys returns for empty keys would have left
ActorIDKey / ActorTypeKey / TenantIDKey unpopulated and 401'd
every Phase-3.5 rbacGate-wrapped admin route + every Phase-4
RBAC handler in demo deployments. NewDemoModeAuth injects the
synthetic 'actor-demo-anon' actor seeded by migration 000029,
which holds r-admin at global scope.
C2: backfillNamedKeyActorRoles startup hook (cmd/server/auth_backfill.go)
iterates CERTCTL_API_KEYS_NAMED entries (and legacy
CERTCTL_AUTH_SECRET synthesized fallbacks) and grants r-admin
or r-viewer to each via authActorRoleRepo.Grant before the
HTTP server starts accepting requests. Idempotent via
ON CONFLICT DO NOTHING in the repo. Failures log a warning but
are non-fatal — the server still starts and the operator can
fix grants via /v1/auth/keys. Helper extracted from main.go so
the role-mapping invariant is pinned by 4 focused unit tests
(admin->r-admin, non-admin->r-viewer, empty no-op,
grant-error non-fatal, nil-logger safe).
M1: HealthHandler.AuthCheck now returns actor_id, actor_type,
tenant_id, roles, effective_permissions, and admin_via_role
when the optional AuthCheckResolver is wired (production path:
authCheckResolverAdapter wraps the postgres ActorRoleRepository
in main.go). Nil resolver preserves the legacy {status, user,
admin} contract for back-compat with pre-Bundle-1 GUIs and
test fixtures. Adds 2 regression tests + 1 fake resolver shim.
M2: refreshes the stale 'Admin gate: every method calls
auth.IsAdmin first' comment on IntermediateCAHandler — the gate
moved to router.go::rbacGate via auth.RequirePermission
middleware in Phase 3.5; the new comment block points readers
there.
M4: 11 RBAC routes (auth/me, auth/permissions, 5 role lifecycle,
2 role-permission grant/revoke, 2 actor-role grant/revoke) added
to api/openapi.yaml under the [Auth] tag with operationIds and
shared AuthRole / AuthRolePermission schemas. AuthCheck path
extended with the Bundle-1 enrichment fields. The 11 entries
removed from openapi_parity_test.go::SpecParityExceptions.
Tests: go vet + staticcheck + go test -short -count=1 green
across cmd/server/, internal/auth/, internal/api/router/, and
internal/api/handler/. New tests: 4 backfill unit tests,
2 AuthCheck M1 enrichment tests, 1 demo-mode + rbacGate chain
integration test (TestRBACGate_DemoModeChainReachesHandler).
Branch SECURITY.md (cowork/auth-bundle-1-SECURITY.md, not part
of this commit) captures the full posture of dev/auth-bundle-1
as of this closure for the operator's pre-merge review.
This commit is contained in:
@@ -0,0 +1,58 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"log/slog"
|
||||
|
||||
"github.com/certctl-io/certctl/internal/auth"
|
||||
"github.com/certctl-io/certctl/internal/domain"
|
||||
authdomain "github.com/certctl-io/certctl/internal/domain/auth"
|
||||
)
|
||||
|
||||
// actorRoleGranter is the narrow interface backfillNamedKeyActorRoles
|
||||
// needs from the postgres ActorRoleRepository. Pulled out so the unit
|
||||
// test can inject a fake without spinning up the full repo / DB.
|
||||
type actorRoleGranter interface {
|
||||
Grant(ctx context.Context, ar *authdomain.ActorRole) error
|
||||
}
|
||||
|
||||
// backfillNamedKeyActorRoles is the Bundle 1 Phase 3 closure (C2)
|
||||
// startup hook that ensures every CERTCTL_API_KEYS_NAMED entry — and
|
||||
// every legacy CERTCTL_AUTH_SECRET synthesized fallback — has an
|
||||
// actor_roles row before the HTTP server accepts requests. Admin-flagged
|
||||
// keys grant `r-admin` (full canonical permission set); non-admin keys
|
||||
// grant `r-viewer` (read-only surface), matching the pre-Phase-3.5
|
||||
// capability shape.
|
||||
//
|
||||
// Idempotent via ON CONFLICT DO NOTHING in the repo Grant — reboots
|
||||
// don't create duplicates. Failures are logged but non-fatal: the server
|
||||
// still starts, and the operator can fix the grant via the RBAC API.
|
||||
//
|
||||
// The function is package-private + extracted from main() so the unit
|
||||
// test in auth_backfill_test.go can pin the role-mapping invariant
|
||||
// without depending on the full server bootstrap path.
|
||||
func backfillNamedKeyActorRoles(
|
||||
ctx context.Context,
|
||||
repo actorRoleGranter,
|
||||
keys []auth.NamedAPIKey,
|
||||
logger *slog.Logger,
|
||||
) {
|
||||
for _, nk := range keys {
|
||||
role := authdomain.RoleIDViewer
|
||||
if nk.Admin {
|
||||
role = authdomain.RoleIDAdmin
|
||||
}
|
||||
if err := repo.Grant(ctx, &authdomain.ActorRole{
|
||||
ActorID: nk.Name,
|
||||
ActorType: authdomain.ActorTypeValue(domain.ActorTypeAPIKey),
|
||||
RoleID: role,
|
||||
TenantID: authdomain.DefaultTenantID,
|
||||
GrantedBy: "bootstrap",
|
||||
}); err != nil {
|
||||
if logger != nil {
|
||||
logger.Warn("api-key actor-role backfill failed; key authenticates but RBAC routes will 403 until grant is added via /v1/auth/keys",
|
||||
"key", nk.Name, "role", role, "err", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,116 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"io"
|
||||
"log/slog"
|
||||
"testing"
|
||||
|
||||
"github.com/certctl-io/certctl/internal/auth"
|
||||
authdomain "github.com/certctl-io/certctl/internal/domain/auth"
|
||||
)
|
||||
|
||||
// fakeGranter is a tiny in-memory stand-in for the postgres ActorRoleRepository
|
||||
// — enough surface area for backfillNamedKeyActorRoles to call Grant against.
|
||||
type fakeGranter struct {
|
||||
calls []*authdomain.ActorRole
|
||||
err error
|
||||
}
|
||||
|
||||
func (f *fakeGranter) Grant(_ context.Context, ar *authdomain.ActorRole) error {
|
||||
f.calls = append(f.calls, ar)
|
||||
return f.err
|
||||
}
|
||||
|
||||
// TestBackfillNamedKeyActorRoles_RoleMapping pins the Bundle 1 Phase 3
|
||||
// closure (C2) invariant: admin-flagged named keys grant r-admin,
|
||||
// non-admin keys grant r-viewer, both at TenantID t-default with
|
||||
// ActorType APIKey and GrantedBy=bootstrap.
|
||||
func TestBackfillNamedKeyActorRoles_RoleMapping(t *testing.T) {
|
||||
repo := &fakeGranter{}
|
||||
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||
|
||||
keys := []auth.NamedAPIKey{
|
||||
{Name: "alice-admin", Key: "AAA", Admin: true},
|
||||
{Name: "bob-viewer", Key: "BBB", Admin: false},
|
||||
{Name: "carol-admin", Key: "CCC", Admin: true},
|
||||
}
|
||||
backfillNamedKeyActorRoles(context.Background(), repo, keys, logger)
|
||||
|
||||
if len(repo.calls) != 3 {
|
||||
t.Fatalf("Grant call count = %d, want 3", len(repo.calls))
|
||||
}
|
||||
type want struct {
|
||||
actor, role string
|
||||
}
|
||||
wants := []want{
|
||||
{actor: "alice-admin", role: authdomain.RoleIDAdmin},
|
||||
{actor: "bob-viewer", role: authdomain.RoleIDViewer},
|
||||
{actor: "carol-admin", role: authdomain.RoleIDAdmin},
|
||||
}
|
||||
for i, w := range wants {
|
||||
got := repo.calls[i]
|
||||
if got.ActorID != w.actor {
|
||||
t.Errorf("call[%d].ActorID = %q, want %q", i, got.ActorID, w.actor)
|
||||
}
|
||||
if got.RoleID != w.role {
|
||||
t.Errorf("call[%d].RoleID = %q, want %q", i, got.RoleID, w.role)
|
||||
}
|
||||
if got.TenantID != authdomain.DefaultTenantID {
|
||||
t.Errorf("call[%d].TenantID = %q, want %q", i, got.TenantID, authdomain.DefaultTenantID)
|
||||
}
|
||||
if string(got.ActorType) != "APIKey" {
|
||||
t.Errorf("call[%d].ActorType = %q, want APIKey", i, got.ActorType)
|
||||
}
|
||||
if got.GrantedBy != "bootstrap" {
|
||||
t.Errorf("call[%d].GrantedBy = %q, want bootstrap", i, got.GrantedBy)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestBackfillNamedKeyActorRoles_EmptyKeysIsNoOp confirms the boot path
|
||||
// is safe when no named keys are configured (typical CERTCTL_AUTH_TYPE=
|
||||
// none deploy). No Grant calls; no panic.
|
||||
func TestBackfillNamedKeyActorRoles_EmptyKeysIsNoOp(t *testing.T) {
|
||||
repo := &fakeGranter{}
|
||||
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||
backfillNamedKeyActorRoles(context.Background(), repo, nil, logger)
|
||||
if len(repo.calls) != 0 {
|
||||
t.Errorf("Grant called %d times for empty keys, want 0", len(repo.calls))
|
||||
}
|
||||
}
|
||||
|
||||
// TestBackfillNamedKeyActorRoles_GrantErrorIsNonFatal confirms the
|
||||
// closure invariant that a Grant failure logs a warning and proceeds
|
||||
// rather than crashing the server during boot. Subsequent keys still
|
||||
// get processed.
|
||||
func TestBackfillNamedKeyActorRoles_GrantErrorIsNonFatal(t *testing.T) {
|
||||
repo := &fakeGranter{err: errors.New("simulated DB error")}
|
||||
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||
|
||||
keys := []auth.NamedAPIKey{
|
||||
{Name: "alice", Key: "A", Admin: true},
|
||||
{Name: "bob", Key: "B", Admin: false},
|
||||
}
|
||||
// Should not panic.
|
||||
backfillNamedKeyActorRoles(context.Background(), repo, keys, logger)
|
||||
|
||||
if len(repo.calls) != 2 {
|
||||
t.Errorf("Grant calls = %d, want 2 (every key processed even when prior Grant errored)", len(repo.calls))
|
||||
}
|
||||
}
|
||||
|
||||
// TestBackfillNamedKeyActorRoles_NilLoggerIsSafe pins that callers
|
||||
// passing nil for the logger don't NPE the goroutine. Belt-and-braces
|
||||
// for tests + future call sites that may not have a logger plumbed.
|
||||
func TestBackfillNamedKeyActorRoles_NilLoggerIsSafe(t *testing.T) {
|
||||
repo := &fakeGranter{err: errors.New("simulated")}
|
||||
keys := []auth.NamedAPIKey{
|
||||
{Name: "alice", Key: "A", Admin: true},
|
||||
}
|
||||
backfillNamedKeyActorRoles(context.Background(), repo, keys, nil)
|
||||
if len(repo.calls) != 1 {
|
||||
t.Errorf("Grant calls = %d, want 1", len(repo.calls))
|
||||
}
|
||||
}
|
||||
+67
-1
@@ -35,6 +35,7 @@ import (
|
||||
"github.com/certctl-io/certctl/internal/domain"
|
||||
authdomainAlias "github.com/certctl-io/certctl/internal/domain/auth"
|
||||
"github.com/certctl-io/certctl/internal/ratelimit"
|
||||
"github.com/certctl-io/certctl/internal/repository"
|
||||
"github.com/certctl-io/certctl/internal/repository/postgres"
|
||||
"github.com/certctl-io/certctl/internal/scep/intune"
|
||||
"github.com/certctl-io/certctl/internal/scheduler"
|
||||
@@ -678,6 +679,12 @@ func main() {
|
||||
// Bundle-5 / H-006: pass the *sql.DB pool so /ready can probe DB
|
||||
// connectivity via PingContext. /health stays shallow (liveness signal).
|
||||
healthHandler := handler.NewHealthHandler(cfg.Auth.Type, db)
|
||||
// Bundle 1 Phase 3 closure (M1): wire the AuthCheckResolver so
|
||||
// /v1/auth/check returns the caller's standing roles + effective
|
||||
// permissions in the same response. The shim is tiny — just a type-
|
||||
// erasure wrap around the repo so the handler layer doesn't have to
|
||||
// import internal/domain/auth or internal/repository/postgres.
|
||||
healthHandler.Resolver = authCheckResolverAdapter{repo: authActorRoleRepo}
|
||||
// U-3 ride-along (cat-u-no_version_endpoint, P2): the version handler
|
||||
// answers GET /api/v1/version with build identity (ldflags Version,
|
||||
// VCS commit/dirty/timestamp, Go runtime version). Wired through the
|
||||
@@ -1558,7 +1565,33 @@ func main() {
|
||||
}
|
||||
}
|
||||
}
|
||||
authMiddleware := auth.NewAuthWithNamedKeys(namedKeys)
|
||||
// Bundle 1 Phase 3 closure (C2): backfill actor_roles rows for every
|
||||
// CERTCTL_API_KEYS_NAMED entry (and the legacy CERTCTL_AUTH_SECRET
|
||||
// synthesized fallbacks) so RBAC checks have a row to match against.
|
||||
// Without this, named keys would land on a Phase-3 actor context
|
||||
// that authorizes every request through the legacy in-handler
|
||||
// auth.IsAdmin path but fails every Phase-3.5 rbacGate (no
|
||||
// actor_roles row → empty EffectivePermissions → 403). The helper
|
||||
// lives in cmd/server/auth_backfill.go so the role-mapping invariant
|
||||
// is pinned by a focused unit test without dragging in the full
|
||||
// server bootstrap path.
|
||||
backfillNamedKeyActorRoles(ctx, authActorRoleRepo, namedKeys, logger)
|
||||
// Bundle 1 Phase 3 closure (C1): when CERTCTL_AUTH_TYPE=none the
|
||||
// legacy NewAuthWithNamedKeys returns a no-op pass-through, which
|
||||
// would leave ActorIDKey / ActorTypeKey / TenantIDKey unpopulated
|
||||
// in context. Phase 3.5's rbacGate + Phase 4's RBAC handlers all
|
||||
// require an actor in context (or they 401), so demo mode would be
|
||||
// completely broken. NewDemoModeAuth injects the synthetic
|
||||
// `actor-demo-anon` actor seeded by migration 000029, which holds
|
||||
// the admin role at global scope; the demo + 5 examples in
|
||||
// examples/*/docker-compose.yml continue to work end-to-end.
|
||||
var authMiddleware func(http.Handler) http.Handler
|
||||
switch config.AuthType(cfg.Auth.Type) {
|
||||
case config.AuthTypeNone:
|
||||
authMiddleware = auth.NewDemoModeAuth()
|
||||
default:
|
||||
authMiddleware = auth.NewAuthWithNamedKeys(namedKeys)
|
||||
}
|
||||
corsMiddleware := middleware.NewCORS(middleware.CORSConfig{
|
||||
AllowedOrigins: cfg.CORS.AllowedOrigins,
|
||||
})
|
||||
@@ -2301,3 +2334,36 @@ func (ad authPermissionCheckerAdapter) CheckPermission(
|
||||
scopeID,
|
||||
)
|
||||
}
|
||||
|
||||
// authCheckResolverAdapter bridges the postgres ActorRoleRepository
|
||||
// (authdomain.ActorTypeValue) to handler.AuthCheckResolver
|
||||
// (domain.ActorType). Lives in cmd/server so the handler layer keeps its
|
||||
// existing import set; the GUI's /v1/auth/check probe round-trips
|
||||
// through this on every page load. Read-only — no caller / no audit row.
|
||||
//
|
||||
// Bundle 1 Phase 3 closure (M1): the equivalent surface area on
|
||||
// /v1/auth/me runs through the service layer's auth.role.list permission
|
||||
// gate, which the GUI may not yet hold during initial render. AuthCheck
|
||||
// has no permission gate (its only requirement is "the request
|
||||
// authenticated"), so the bypass is by design.
|
||||
type authCheckResolverAdapter struct {
|
||||
repo *postgres.ActorRoleRepository
|
||||
}
|
||||
|
||||
func (ad authCheckResolverAdapter) ListRoles(
|
||||
ctx context.Context,
|
||||
actorID string,
|
||||
actorType domain.ActorType,
|
||||
tenantID string,
|
||||
) ([]*authdomainAlias.ActorRole, error) {
|
||||
return ad.repo.ListByActor(ctx, actorID, authdomainAlias.ActorTypeValue(actorType), tenantID)
|
||||
}
|
||||
|
||||
func (ad authCheckResolverAdapter) EffectivePermissions(
|
||||
ctx context.Context,
|
||||
actorID string,
|
||||
actorType domain.ActorType,
|
||||
tenantID string,
|
||||
) ([]repository.EffectivePermission, error) {
|
||||
return ad.repo.EffectivePermissions(ctx, actorID, authdomainAlias.ActorTypeValue(actorType), tenantID)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user