From a123263498f9ce45ed0aad97287365b94bb548c9 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Mon, 11 May 2026 02:02:39 +0000 Subject: [PATCH] =?UTF-8?q?fix(auth/rbac):=20close=20HIGH-10=20lying=20fie?= =?UTF-8?q?ld=20=E2=80=94=20EffectivePermissions=20reads=20actor-role=20sc?= =?UTF-8?q?ope=20(A-1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit 2026-05-11 A-1 closure. Spec at cowork/auth-bundles-fixes-2026-05-11/01-crit-actor-role-scope-reads.md. WHAT. The HIGH-10 closure (commit 72b54ce on dev/auth-bundle-2) added `scope_type` + `scope_id` columns to `actor_roles` via migration 000043. The handler accepted them on POST /api/v1/auth/keys/{id}/roles. The repo Grant INSERTed them. The uniqueness tuple was extended to include them. The GUI exposed them as form inputs. But the load-bearing `EffectivePermissions` SQL at internal/repository/postgres/auth.go:470 never read them. The query only JOINed against rp.scope_type/rp.scope_id (role-permission scope) and ignored ar.scope_type/ar.scope_id (actor-role scope). Operator-visible failure: granting Alice r-operator scoped to profile=p-prod silently elevated her to r-operator GLOBALLY at authorization time. The Authorizer's matcher correctly handled whatever EffectivePermissions returned, but EffectivePermissions returned the rp.scope (typically global), not the ar.scope narrowing. This is the canonical CRIT-5 lying-field shape — a security control claimed, persisted across 4 layers, with unit tests at each isolated layer, but the load-bearing wire severed mid-flight. CLAUDE.md's 'Always take the complete path' rule was violated by the original HIGH-10 closure. Additionally, `scanActorRoles` failed to read the new columns even when present, so every GET-side path (ListByActor / ListByRole) returned ActorRole with zero-value scope fields — the GUI / MCP couldn't show operators what they had configured. HOW. internal/repository/postgres/auth.go: - EffectivePermissions SQL extended to intersect ar.scope with rp.scope via a CASE-in-subquery. The effective scope is the NARROWER of the two; disjoint tuples and scope-type mismatches drop the row entirely. WHERE filter on effective_scope_type IS NOT NULL excludes dropped rows. Match matrix (encoded by the CASE): ar.scope rp.scope effective_scope ───────── ───────── ────────────────── global global global / NULL global profile=X profile=X (rp narrows) profile=X global profile=X (ar narrows) profile=X profile=X profile=X (both agree) profile=X profile=Y ROW DROPPED (disjoint) profile=X issuer=* ROW DROPPED (type mismatch) - ListByActor + ListByRole SELECTs extended with scope_type + scope_id columns so the read-side surfaces what was persisted. - scanActorRoles reads the new columns into ActorRole.ScopeType + ScopeID via the existing sql.NullString + ScopeType cast pattern (mirrors RolePermission scan). internal/repository/postgres/auth_scope_test.go (NEW): Testcontainer-backed regression matrix. 8 cases: 1. ActorRoleGlobal_RolePermGlobal — trivial happy path. 2. ActorRoleGlobal_RolePermProfile — rp narrows. 3. ActorRoleProfile_RolePermGlobal_A1Closure — **load-bearing** post-fix case: profile-scoped grant narrows to profile. 4. BothScopedSameTuple_Matches — exact-match collapse. 5. BothScopedDifferentIDs_RowDropped — disjoint scopes produce no effective permission. 6. ScopeTypeMismatch_RowDropped — profile vs issuer mismatch. 7. ExpiredGrant_Excluded — pre-fix behavior preserved. 8. ListByActor_ReturnsScopeColumns — read-side surface check. Tests skip in -short mode (testcontainers-backed; require Docker on operator workstation). internal/service/auth/service_test.go: TestAuthorizer_ActorRoleProfileScope_OnlyNarrowedScopeAuthorizes_A1 — unit-level pin (sandbox-runnable, no Docker). Simulates the post-A-1 SQL emission (narrowed effective row at profile=p-prod) and asserts CheckPermission authorizes only matching profile, rejects other profiles AND rejects global. Existing matcher code is unchanged; this proves the integration point. CHANGELOG.md: Operator advisory in the new 'Security (BREAKING — silent-elevation closure)' section. Pre-existing scope-bound grants take effect on upgrade; operators audit `actor_roles WHERE scope_type != 'global'` to confirm intent. cowork/auth-bundles-audit-2026-05-10.md: HIGH-10 row gets an A-1 follow-on CLOSED 2026-05-11 annotation describing the regression + closure. VERIFY. - gofmt -l (no diff) - go vet ./internal/repository/postgres/... ./internal/service/auth/... ./internal/api/handler/... ./internal/auth/... ./cmd/server/... PASS - go test -short -count=1 ./internal/service/auth/... ./internal/repository/postgres/... ./internal/api/handler/... PASS - The testcontainer-backed regression matrix runs on operator workstation via 'go test -count=1 ./internal/repository/postgres/...' (skip in -short). Refs: cowork/auth-bundles-audit-2026-05-10.md HIGH-10 (A-1 follow-on) cowork/auth-bundles-fixes-2026-05-11/01-crit-actor-role-scope-reads.md CLAUDE.md 'Always take the complete path' rule --- CHANGELOG.md | 20 ++ internal/repository/postgres/auth.go | 81 ++++- .../repository/postgres/auth_scope_test.go | 305 ++++++++++++++++++ internal/service/auth/service_test.go | 50 +++ 4 files changed, 444 insertions(+), 12 deletions(-) create mode 100644 internal/repository/postgres/auth_scope_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e986ea..1b6c0cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,26 @@ ## Unreleased +### Security (BREAKING — silent-elevation closure) + +- **HIGH-10 actor-role scope is now enforced (Audit 2026-05-11 A-1).** + Pre-fix, `actor_roles.scope_type` / `scope_id` (added in migration 000043 + by the HIGH-10 closure) were persisted by Grant + accepted on the handler + body + surfaced through the GUI/MCP — but the load-bearing + `EffectivePermissions` SQL never read them. A profile-scoped grant + silently elevated to global at authorization time. Canonical CRIT-5 + lying-field shape, replicated. **The post-fix authorization narrows + correctly**: every existing `actor_roles` row with `scope_type != 'global'` + now takes effect. + + > **Operator advisory:** if you used the HIGH-10 scope-bound role-grant + > API between commit `551812b` and the v2.1.0 tag (the column was + > populated but ignored), the grants were silently global. After + > upgrading, audit `SELECT actor_id, role_id, scope_type, scope_id FROM + > actor_roles WHERE scope_type != 'global'` and confirm the narrowing + > reflects intent. If an actor was granted a scoped role but expected + > global behavior, re-grant with `scope_type=global`. + ### Security (BREAKING) - **`__Host-` cookie prefix on all three auth cookies (Audit 2026-05-10 MED-14).** diff --git a/internal/repository/postgres/auth.go b/internal/repository/postgres/auth.go index 8aa1713..52049bd 100644 --- a/internal/repository/postgres/auth.go +++ b/internal/repository/postgres/auth.go @@ -335,8 +335,13 @@ func NewActorRoleRepository(db *sql.DB) *ActorRoleRepository { } func (r *ActorRoleRepository) ListByActor(ctx context.Context, actorID string, actorType authdomain.ActorTypeValue, tenantID string) ([]*authdomain.ActorRole, error) { + // Audit 2026-05-11 A-1 — include scope_type + scope_id in the + // SELECT so the GUI / MCP surface can render which scope an + // actor's grant is bound to. Pre-fix, these columns were + // persisted by Grant (HIGH-10 closure) but never surfaced on + // read — operators couldn't see what they configured. rows, err := r.db.QueryContext(ctx, ` - SELECT id, actor_id, actor_type, role_id, granted_at, expires_at, granted_by, tenant_id + SELECT id, actor_id, actor_type, role_id, granted_at, expires_at, granted_by, tenant_id, scope_type, scope_id FROM actor_roles WHERE actor_id = $1 AND actor_type = $2 AND tenant_id = $3 ORDER BY granted_at @@ -349,7 +354,7 @@ func (r *ActorRoleRepository) ListByActor(ctx context.Context, actorID string, a func (r *ActorRoleRepository) ListByRole(ctx context.Context, roleID string) ([]*authdomain.ActorRole, error) { rows, err := r.db.QueryContext(ctx, ` - SELECT id, actor_id, actor_type, role_id, granted_at, expires_at, granted_by, tenant_id + SELECT id, actor_id, actor_type, role_id, granted_at, expires_at, granted_by, tenant_id, scope_type, scope_id FROM actor_roles WHERE role_id = $1 ORDER BY granted_at @@ -468,15 +473,55 @@ func (r *ActorRoleRepository) AdminExists(ctx context.Context, tenantID string) } func (r *ActorRoleRepository) EffectivePermissions(ctx context.Context, actorID string, actorType authdomain.ActorTypeValue, tenantID string) ([]repository.EffectivePermission, error) { + // Audit 2026-05-11 A-1 — effective scope is the intersection of + // the actor-role's scope (ar.scope_*) AND the role-permission's + // scope (rp.scope_*). Pre-fix, only rp.scope_* was read; an + // actor granted r-operator scoped to profile=p-prod silently + // got every r-operator permission at every scope rp emitted + // (typically global), defeating HIGH-10's per-actor scope knob. + // + // Matching rules (the inner CASE encodes them): + // + // ar.scope rp.scope effective_scope + // ───────── ───────── ────────────────────── + // global global global / NULL + // global profile=X profile=X (rp narrows) + // profile=X global profile=X (ar narrows) + // profile=X profile=X profile=X (both agree) + // profile=X profile=Y ROW DROPPED (disjoint scopes — no permission flows) + // profile=X issuer=* ROW DROPPED (scope-type mismatch) + // + // The HAVING-style filter is implemented via a subquery — Postgres + // doesn't allow referencing a CASE alias from HAVING in a SELECT + // DISTINCT context without a wrapping CTE. rows, err := r.db.QueryContext(ctx, ` - SELECT DISTINCT p.name, rp.scope_type, rp.scope_id - FROM actor_roles ar - JOIN role_permissions rp ON rp.role_id = ar.role_id - JOIN permissions p ON p.id = rp.permission_id - WHERE ar.actor_id = $1 - AND ar.actor_type = $2 - AND ar.tenant_id = $3 - AND (ar.expires_at IS NULL OR ar.expires_at > NOW()) + SELECT DISTINCT permission_name, effective_scope_type, effective_scope_id + FROM ( + SELECT + p.name AS permission_name, + CASE + WHEN ar.scope_type = 'global' AND rp.scope_type = 'global' THEN 'global' + WHEN ar.scope_type = 'global' THEN rp.scope_type + WHEN rp.scope_type = 'global' THEN ar.scope_type + WHEN ar.scope_type = rp.scope_type AND ar.scope_id IS NOT DISTINCT FROM rp.scope_id THEN ar.scope_type + ELSE NULL + END AS effective_scope_type, + CASE + WHEN ar.scope_type = 'global' AND rp.scope_type = 'global' THEN NULL + WHEN ar.scope_type = 'global' THEN rp.scope_id + WHEN rp.scope_type = 'global' THEN ar.scope_id + WHEN ar.scope_type = rp.scope_type AND ar.scope_id IS NOT DISTINCT FROM rp.scope_id THEN ar.scope_id + ELSE NULL + END AS effective_scope_id + FROM actor_roles ar + JOIN role_permissions rp ON rp.role_id = ar.role_id + JOIN permissions p ON p.id = rp.permission_id + WHERE ar.actor_id = $1 + AND ar.actor_type = $2 + AND ar.tenant_id = $3 + AND (ar.expires_at IS NULL OR ar.expires_at > NOW()) + ) AS intersected + WHERE effective_scope_type IS NOT NULL `, actorID, string(actorType), tenantID) if err != nil { return nil, fmt.Errorf("actorRole.effective: %w", err) @@ -505,9 +550,16 @@ func scanActorRoles(rows *sql.Rows) ([]*authdomain.ActorRole, error) { var out []*authdomain.ActorRole for rows.Next() { var ar authdomain.ActorRole - var actorType string + var actorType, scopeType string var expires sql.NullTime - if err := rows.Scan(&ar.ID, &ar.ActorID, &actorType, &ar.RoleID, &ar.GrantedAt, &expires, &ar.GrantedBy, &ar.TenantID); err != nil { + var scopeID sql.NullString + // Audit 2026-05-11 A-1 — scope_type + scope_id are persisted + // by Grant (HIGH-10 closure, migration 000043). Pre-fix they + // were never scanned, so callers received ActorRole with + // zero-value scope fields regardless of what the row held. + // EffectivePermissions narrowing depends on these being + // populated correctly. + if err := rows.Scan(&ar.ID, &ar.ActorID, &actorType, &ar.RoleID, &ar.GrantedAt, &expires, &ar.GrantedBy, &ar.TenantID, &scopeType, &scopeID); err != nil { return nil, fmt.Errorf("actorRole scan: %w", err) } ar.ActorType = authdomain.ActorTypeValue(actorType) @@ -515,6 +567,11 @@ func scanActorRoles(rows *sql.Rows) ([]*authdomain.ActorRole, error) { t := expires.Time ar.ExpiresAt = &t } + ar.ScopeType = authdomain.ScopeType(scopeType) + if scopeID.Valid { + s := scopeID.String + ar.ScopeID = &s + } out = append(out, &ar) } return out, rows.Err() diff --git a/internal/repository/postgres/auth_scope_test.go b/internal/repository/postgres/auth_scope_test.go new file mode 100644 index 0000000..58ba94c --- /dev/null +++ b/internal/repository/postgres/auth_scope_test.go @@ -0,0 +1,305 @@ +package postgres_test + +// Audit 2026-05-11 A-1 closure — EffectivePermissions scope-intersection +// regression matrix. Pre-fix, the SQL only narrowed by role-permission +// scope (rp.scope_*); actor-role scope (ar.scope_*) was ignored. An +// operator who scope-granted Alice `r-operator` to `profile=p-prod` +// silently elevated Alice to `r-operator` globally. Same shape as the +// original CRIT-5 lying field, replicated in the load-bearing auth +// check path. +// +// These tests exercise the SQL change in isolation against a real +// Postgres container. They cover the six effective-scope cases the +// fix encodes (see the EffectivePermissions SQL comment block): +// +// ar.scope rp.scope expected_effective +// ───────── ───────── ────────────────────────── +// global global global / NULL +// global profile=X profile=X (rp narrows) +// profile=X global profile=X (ar narrows) +// profile=X profile=X profile=X (both agree) +// profile=X profile=Y ROW DROPPED (disjoint) +// profile=X issuer=* ROW DROPPED (scope-type mismatch) + +import ( + "context" + "testing" + + authdomain "github.com/certctl-io/certctl/internal/domain/auth" + "github.com/certctl-io/certctl/internal/repository/postgres" +) + +// seedRoleWithPerm creates a role with one permission grant at the +// supplied scope and returns the role ID. Helper for the test matrix. +func seedRoleWithPerm(t *testing.T, ctx context.Context, roleRepo *postgres.RoleRepository, permRepo *postgres.PermissionRepository, roleSuffix, permName string, rpScopeType authdomain.ScopeType, rpScopeID *string) string { + t.Helper() + roleID := "r-" + roleSuffix + role := &authdomain.Role{ + ID: roleID, Name: "Test " + roleSuffix, Description: "scope-test role", TenantID: authdomain.DefaultTenantID, + } + if err := roleRepo.Create(ctx, role); err != nil { + t.Fatalf("seed role %s: %v", roleSuffix, err) + } + // Look up the permission ID (the catalogue is seeded by migrations, + // but for net-new test perms we'd need to Create — for this test + // we use a perm name from the existing default catalogue). + perm, err := permRepo.GetByName(ctx, permName) + if err != nil { + t.Fatalf("seed perm GetByName %s: %v", permName, err) + } + rp := &authdomain.RolePermission{ + RoleID: roleID, PermissionID: perm.ID, ScopeType: rpScopeType, ScopeID: rpScopeID, + } + if err := roleRepo.AddPermission(ctx, rp); err != nil { + t.Fatalf("seed AddPermission %s/%s: %v", roleSuffix, permName, err) + } + return roleID +} + +// grantActorRoleAtScope inserts an actor_roles row at the supplied +// scope. ScopeID nil = global. +func grantActorRoleAtScope(t *testing.T, ctx context.Context, repo *postgres.ActorRoleRepository, actorID, roleID string, scopeType authdomain.ScopeType, scopeID *string) { + t.Helper() + ar := &authdomain.ActorRole{ + ActorID: actorID, ActorType: authdomain.ActorTypeValue("APIKey"), RoleID: roleID, + TenantID: authdomain.DefaultTenantID, ScopeType: scopeType, ScopeID: scopeID, + } + if err := repo.Grant(ctx, ar); err != nil { + t.Fatalf("Grant %s -> %s@%s: %v", actorID, roleID, scopeType, err) + } +} + +func ptrStr(s string) *string { return &s } + +// effectivePermFor returns the single EffectivePermission for +// (actor, perm) or nil. Asserts at most one row matches the perm name — +// the SQL DISTINCT should fold duplicates. +func effectivePermFor(t *testing.T, ctx context.Context, repo *postgres.ActorRoleRepository, actorID, permName string) (authdomain.ScopeType, *string, bool) { + t.Helper() + rows, err := repo.EffectivePermissions(ctx, actorID, authdomain.ActorTypeValue("APIKey"), authdomain.DefaultTenantID) + if err != nil { + t.Fatalf("EffectivePermissions for %s: %v", actorID, err) + } + for _, r := range rows { + if r.PermissionName == permName { + return r.ScopeType, r.ScopeID, true + } + } + return "", nil, false +} + +// TestEffectivePermissions_ActorRoleGlobal_RolePermGlobal pins the +// trivial happy path — both global → effective global. +func TestEffectivePermissions_ActorRoleGlobal_RolePermGlobal(t *testing.T) { + if testing.Short() { + t.Skip("integration test in short mode") + } + db := getTestDB(t).freshSchema(t) + ctx := context.Background() + roleRepo := postgres.NewRoleRepository(db) + permRepo := postgres.NewPermissionRepository(db) + actorRepo := postgres.NewActorRoleRepository(db) + + rid := seedRoleWithPerm(t, ctx, roleRepo, permRepo, "ar-a1-globglob", "cert.read", authdomain.ScopeTypeGlobal, nil) + grantActorRoleAtScope(t, ctx, actorRepo, "alice-a1-globglob", rid, authdomain.ScopeTypeGlobal, nil) + + st, sid, ok := effectivePermFor(t, ctx, actorRepo, "alice-a1-globglob", "cert.read") + if !ok { + t.Fatal("expected cert.read in effective permissions") + } + if st != authdomain.ScopeTypeGlobal { + t.Errorf("effective scope_type = %q; want global", st) + } + if sid != nil { + t.Errorf("effective scope_id = %v; want nil", sid) + } +} + +// TestEffectivePermissions_ActorRoleGlobal_RolePermProfile pins that +// rp.scope narrows when ar is global — the permission flows through +// at the rp scope. +func TestEffectivePermissions_ActorRoleGlobal_RolePermProfile(t *testing.T) { + if testing.Short() { + t.Skip("integration test in short mode") + } + db := getTestDB(t).freshSchema(t) + ctx := context.Background() + roleRepo := postgres.NewRoleRepository(db) + permRepo := postgres.NewPermissionRepository(db) + actorRepo := postgres.NewActorRoleRepository(db) + + rid := seedRoleWithPerm(t, ctx, roleRepo, permRepo, "ar-a1-globprof", "cert.read", authdomain.ScopeTypeProfile, ptrStr("p-prod")) + grantActorRoleAtScope(t, ctx, actorRepo, "alice-a1-globprof", rid, authdomain.ScopeTypeGlobal, nil) + + st, sid, ok := effectivePermFor(t, ctx, actorRepo, "alice-a1-globprof", "cert.read") + if !ok { + t.Fatal("expected cert.read in effective permissions") + } + if st != authdomain.ScopeTypeProfile { + t.Errorf("effective scope_type = %q; want profile", st) + } + if sid == nil || *sid != "p-prod" { + t.Errorf("effective scope_id = %v; want p-prod", sid) + } +} + +// TestEffectivePermissions_ActorRoleProfile_RolePermGlobal is the +// load-bearing case the A-1 fix closes: pre-fix, ar.scope was ignored +// and Alice scoped to profile=p-prod silently got the rp global +// permission AT GLOBAL SCOPE (i.e. on profile=p-acme too). Post-fix, +// the effective scope must narrow to ar.scope (profile=p-prod). +func TestEffectivePermissions_ActorRoleProfile_RolePermGlobal_A1Closure(t *testing.T) { + if testing.Short() { + t.Skip("integration test in short mode") + } + db := getTestDB(t).freshSchema(t) + ctx := context.Background() + roleRepo := postgres.NewRoleRepository(db) + permRepo := postgres.NewPermissionRepository(db) + actorRepo := postgres.NewActorRoleRepository(db) + + rid := seedRoleWithPerm(t, ctx, roleRepo, permRepo, "ar-a1-profglob", "cert.read", authdomain.ScopeTypeGlobal, nil) + grantActorRoleAtScope(t, ctx, actorRepo, "alice-a1-profglob", rid, authdomain.ScopeTypeProfile, ptrStr("p-prod")) + + st, sid, ok := effectivePermFor(t, ctx, actorRepo, "alice-a1-profglob", "cert.read") + if !ok { + t.Fatal("expected cert.read in effective permissions") + } + if st != authdomain.ScopeTypeProfile { + t.Errorf("A-1 closure regression: effective scope_type = %q; want profile (narrowed to ar.scope)", st) + } + if sid == nil || *sid != "p-prod" { + t.Errorf("A-1 closure regression: effective scope_id = %v; want p-prod (narrowed to ar.scope_id)", sid) + } +} + +// TestEffectivePermissions_BothScopedSameTuple_Matches pins that +// (ar=profile=p-prod, rp=profile=p-prod) collapses to a single +// matching effective row at profile=p-prod. +func TestEffectivePermissions_BothScopedSameTuple_Matches(t *testing.T) { + if testing.Short() { + t.Skip("integration test in short mode") + } + db := getTestDB(t).freshSchema(t) + ctx := context.Background() + roleRepo := postgres.NewRoleRepository(db) + permRepo := postgres.NewPermissionRepository(db) + actorRepo := postgres.NewActorRoleRepository(db) + + rid := seedRoleWithPerm(t, ctx, roleRepo, permRepo, "ar-a1-bothsame", "cert.read", authdomain.ScopeTypeProfile, ptrStr("p-prod")) + grantActorRoleAtScope(t, ctx, actorRepo, "alice-a1-bothsame", rid, authdomain.ScopeTypeProfile, ptrStr("p-prod")) + + st, sid, ok := effectivePermFor(t, ctx, actorRepo, "alice-a1-bothsame", "cert.read") + if !ok { + t.Fatal("expected cert.read in effective permissions") + } + if st != authdomain.ScopeTypeProfile || sid == nil || *sid != "p-prod" { + t.Errorf("matching tuple did not produce profile=p-prod effective row; got (%q, %v)", st, sid) + } +} + +// TestEffectivePermissions_BothScopedDifferentIDs_RowDropped pins the +// disjoint-scope case: ar.profile=p-prod, rp.profile=p-acme → no +// permission row should appear in the effective set. Pre-A1 fix, the +// permission flowed through at rp.scope (p-acme) silently. +func TestEffectivePermissions_BothScopedDifferentIDs_RowDropped(t *testing.T) { + if testing.Short() { + t.Skip("integration test in short mode") + } + db := getTestDB(t).freshSchema(t) + ctx := context.Background() + roleRepo := postgres.NewRoleRepository(db) + permRepo := postgres.NewPermissionRepository(db) + actorRepo := postgres.NewActorRoleRepository(db) + + rid := seedRoleWithPerm(t, ctx, roleRepo, permRepo, "ar-a1-bothdiff", "cert.read", authdomain.ScopeTypeProfile, ptrStr("p-acme")) + grantActorRoleAtScope(t, ctx, actorRepo, "alice-a1-bothdiff", rid, authdomain.ScopeTypeProfile, ptrStr("p-prod")) + + _, _, ok := effectivePermFor(t, ctx, actorRepo, "alice-a1-bothdiff", "cert.read") + if ok { + t.Error("A-1 closure regression: disjoint scopes (ar=p-prod, rp=p-acme) should NOT produce an effective permission row") + } +} + +// TestEffectivePermissions_ScopeTypeMismatch_RowDropped pins the +// scope-type-disagreement case: ar.profile=p-prod, rp.issuer=iss-x → +// no permission. Cross-type narrowing is undefined. +func TestEffectivePermissions_ScopeTypeMismatch_RowDropped(t *testing.T) { + if testing.Short() { + t.Skip("integration test in short mode") + } + db := getTestDB(t).freshSchema(t) + ctx := context.Background() + roleRepo := postgres.NewRoleRepository(db) + permRepo := postgres.NewPermissionRepository(db) + actorRepo := postgres.NewActorRoleRepository(db) + + rid := seedRoleWithPerm(t, ctx, roleRepo, permRepo, "ar-a1-typemis", "cert.read", authdomain.ScopeTypeIssuer, ptrStr("iss-x")) + grantActorRoleAtScope(t, ctx, actorRepo, "alice-a1-typemis", rid, authdomain.ScopeTypeProfile, ptrStr("p-prod")) + + _, _, ok := effectivePermFor(t, ctx, actorRepo, "alice-a1-typemis", "cert.read") + if ok { + t.Error("A-1 closure regression: scope-type mismatch (ar=profile, rp=issuer) should NOT produce an effective permission row") + } +} + +// TestEffectivePermissions_ExpiredGrant_Excluded pins that +// ar.expires_at < NOW() excludes the grant from the effective set. +// This worked pre-A1; the test pins it stays correct under the new +// subquery shape. +func TestEffectivePermissions_ExpiredGrant_Excluded(t *testing.T) { + if testing.Short() { + t.Skip("integration test in short mode") + } + db := getTestDB(t).freshSchema(t) + ctx := context.Background() + roleRepo := postgres.NewRoleRepository(db) + permRepo := postgres.NewPermissionRepository(db) + actorRepo := postgres.NewActorRoleRepository(db) + + rid := seedRoleWithPerm(t, ctx, roleRepo, permRepo, "ar-a1-expired", "cert.read", authdomain.ScopeTypeGlobal, nil) + // Set an expired grant by post-hoc UPDATE since Grant doesn't accept + // past expires_at via the API — we mimic the "grant was made, + // expired since" steady state. + grantActorRoleAtScope(t, ctx, actorRepo, "alice-a1-expired", rid, authdomain.ScopeTypeGlobal, nil) + if _, err := db.ExecContext(ctx, `UPDATE actor_roles SET expires_at = NOW() - INTERVAL '1 hour' WHERE actor_id = $1`, "alice-a1-expired"); err != nil { + t.Fatalf("expire grant: %v", err) + } + + _, _, ok := effectivePermFor(t, ctx, actorRepo, "alice-a1-expired", "cert.read") + if ok { + t.Error("expired grant should not contribute to effective permissions") + } +} + +// TestListByActor_ReturnsScopeColumns pins that ar.scope_type / scope_id +// surface on the read-side ListByActor path. Pre-A1 fix, scanActorRoles +// didn't read these columns even when the row carried non-default +// values — operators couldn't see what they configured. +func TestListByActor_ReturnsScopeColumns(t *testing.T) { + if testing.Short() { + t.Skip("integration test in short mode") + } + db := getTestDB(t).freshSchema(t) + ctx := context.Background() + roleRepo := postgres.NewRoleRepository(db) + permRepo := postgres.NewPermissionRepository(db) + actorRepo := postgres.NewActorRoleRepository(db) + + rid := seedRoleWithPerm(t, ctx, roleRepo, permRepo, "ar-a1-listscope", "cert.read", authdomain.ScopeTypeGlobal, nil) + grantActorRoleAtScope(t, ctx, actorRepo, "alice-a1-listscope", rid, authdomain.ScopeTypeProfile, ptrStr("p-staging")) + + grants, err := actorRepo.ListByActor(ctx, "alice-a1-listscope", authdomain.ActorTypeValue("APIKey"), authdomain.DefaultTenantID) + if err != nil { + t.Fatalf("ListByActor: %v", err) + } + if len(grants) != 1 { + t.Fatalf("got %d grants; want 1", len(grants)) + } + if grants[0].ScopeType != authdomain.ScopeTypeProfile { + t.Errorf("ListByActor scope_type = %q; want profile", grants[0].ScopeType) + } + if grants[0].ScopeID == nil || *grants[0].ScopeID != "p-staging" { + t.Errorf("ListByActor scope_id = %v; want p-staging", grants[0].ScopeID) + } +} diff --git a/internal/service/auth/service_test.go b/internal/service/auth/service_test.go index 09e6dfe..4f5535e 100644 --- a/internal/service/auth/service_test.go +++ b/internal/service/auth/service_test.go @@ -282,6 +282,56 @@ func TestAuthorizer_SpecificScopeMatchesExactID(t *testing.T) { } } +// Audit 2026-05-11 A-1 — pin that when the SQL narrowed effective set +// reflects an actor-role-scope-narrowed permission, CheckPermission +// authorizes only the narrowed scope. This is the unit-level +// counterpart to TestEffectivePermissions_ActorRoleProfile_RolePermGlobal_A1Closure +// in internal/repository/postgres/auth_scope_test.go which exercises +// the actual SQL. +// +// Pre-fix, the SQL ignored ar.scope_*, so a profile-scoped grant +// produced a row with rp.scope (global), and CheckPermission would +// pass for ANY profile. Post-fix, the SQL narrows the row to +// (profile, p-prod), and CheckPermission only passes when the +// request scope matches. +func TestAuthorizer_ActorRoleProfileScope_OnlyNarrowedScopeAuthorizes_A1(t *testing.T) { + r := newFakeActorRoleRepo() + scope := "p-prod" + // Simulate the post-A-1 SQL emission: actor-role scoped to + // profile=p-prod + role-permission scoped global → narrowed + // effective row at profile=p-prod. + r.perms[actorKey("alice", authdomain.ActorTypeValue(domain.ActorTypeAPIKey))] = []repository.EffectivePermission{ + {PermissionName: "cert.read", ScopeType: authdomain.ScopeTypeProfile, ScopeID: &scope}, + } + az := NewAuthorizer(r) + + // Request scope matches narrowed grant → authorize. + matchID := "p-prod" + ok, err := az.CheckPermission(context.Background(), "alice", authdomain.ActorTypeValue(domain.ActorTypeAPIKey), authdomain.DefaultTenantID, "cert.read", authdomain.ScopeTypeProfile, &matchID) + if err != nil { + t.Fatalf("CheckPermission (matching scope): %v", err) + } + if !ok { + t.Error("A-1: profile-scoped grant must authorize matching profile request") + } + + // Different profile → reject (the load-bearing post-fix + // behavior). Pre-fix this would have returned true silently. + wrongID := "p-acme" + ok, _ = az.CheckPermission(context.Background(), "alice", authdomain.ActorTypeValue(domain.ActorTypeAPIKey), authdomain.DefaultTenantID, "cert.read", authdomain.ScopeTypeProfile, &wrongID) + if ok { + t.Error("A-1 regression: profile-scoped grant must NOT authorize a different profile (the canonical CRIT-5 shape)") + } + + // Global request → also reject. A profile-scoped actor-role + // grant doesn't elevate to global; same shape as RFC 9700 + // least-privilege. + ok, _ = az.CheckPermission(context.Background(), "alice", authdomain.ActorTypeValue(domain.ActorTypeAPIKey), authdomain.DefaultTenantID, "cert.read", authdomain.ScopeTypeGlobal, nil) + if ok { + t.Error("A-1: profile-scoped grant must NOT authorize a global request") + } +} + // ============================================================================= // RoleService tests // =============================================================================