From 1fb82f09b28378d4997b624645ec5992c39fe306 Mon Sep 17 00:00:00 2001 From: Jon Chery Date: Fri, 7 Aug 2026 11:03:42 +0000 Subject: [PATCH] fix(P06): ACL rewrite to OIDC claims (REQ-145, REQ-122, F1) ---ci--- project: orca phase: 6 milestone: v0.12 status: execute ---/ci--- Add KindOidc to ACL: OIDCClaims struct, OidcIdentity, OidcGroupIdentity, CheckOidc (checks user sub + group: prefix entries). KindToken now always denies (R-021: no Orca-issued tokens). Existing acl.json entries with KindToken are inert (P07 removes, P22 migrates). acl.json file mode tightened to 0600. Deny-by-default enforced. 4 new OIDC ACL tests + deprecation test. Existing tests migrated to KindOidc. All pass. --- internal/acl/acl.go | 86 ++++++++++++++++++++++++++++++++++++---- internal/acl/acl_test.go | 83 +++++++++++++++++++++++++++++++++----- 2 files changed, 150 insertions(+), 19 deletions(-) diff --git a/internal/acl/acl.go b/internal/acl/acl.go index 508f141..3d6f032 100644 --- a/internal/acl/acl.go +++ b/internal/acl/acl.go @@ -1,10 +1,17 @@ -// Package acl implements the orca access-control layer (P02, v0.11). +// Package acl implements the orca access-control layer. // -// An Identity is either a SPIFFE workload identity (verified SVID whose -// URI is spiffe://orca.local/ns//sa//) or an operator -// token (a bare token ID carrying an explicit namespace claim). Each -// identity is granted a set of Permissions on a namespace; checks are -// deny-by-default — if no entry matches the (identity, namespace) +// An Identity is one of: +// - KindSpiffe: a verified SPIFFE workload SVID whose URI is +// spiffe://orca.local/ns//sa// (machine identity). +// - KindOidc: a verified OIDC ID token whose subject (sub) + groups +// map to namespace permissions (human identity, R-021). +// +// KindToken is DEPRECATED and always denies (R-021: no Orca-issued +// tokens). Existing acl.json entries with KindToken are inert; P07 +// removes them and P22 migrates them. +// +// Each identity is granted a set of Permissions on a namespace; checks +// are deny-by-default — if no entry matches the (identity, namespace) // pair the check returns false. package acl @@ -17,7 +24,8 @@ import ( const ( KindSpiffe = "spiffe" - KindToken = "token" + KindToken = "token" // DEPRECATED: always denies (R-021). Removed by P07. + KindOidc = "oidc" ) // Permission is a bitmask of access rights on a namespace. @@ -99,11 +107,19 @@ func (a *ACL) Revoke(identity Identity, ns string) { // Check reports whether identity has perm on ns. Admin implies Read and // Write: an admin entry satisfies Read and Write checks. Returns false -// (deny-by-default) if no entry matches. +// (deny-by-default) if no entry matches. KindToken always denies +// (R-021: no Orca-issued tokens); existing acl.json entries with +// KindToken are inert. func (a *ACL) Check(identity Identity, ns string, perm Permission) bool { + if identity.Kind == KindToken { + return false + } a.mu.RLock() defer a.mu.RUnlock() for _, e := range a.entries { + if e.Identity.Kind == KindToken { + continue + } if e.Identity.Kind != identity.Kind || e.Identity.ID != identity.ID || e.Namespace != ns { continue } @@ -150,3 +166,57 @@ func SpiffeNamespace(uri string) (string, error) { } return parts[1], nil } + +// OIDCClaims holds the verified claims from an OIDC ID token used by +// the ACL layer. The Subject (sub) is the stable user identifier; +// Groups are the group memberships used to match group-based grants. +type OIDCClaims struct { + Subject string + Groups []string +} + +// OidcIdentity builds an Identity from verified OIDC claims. The ID +// is the OIDC subject (sub). The Namespace is empty (OIDC identities +// are not namespace-scoped at the identity layer; the ACL check takes +// the namespace as a separate argument). +func OidcIdentity(claims OIDCClaims) Identity { + return Identity{ + Kind: KindOidc, + ID: claims.Subject, + } +} + +// OidcGroupIdentity builds an Identity for a group-based grant. The +// ID is the group name prefixed with "group:". This allows ACL +// entries to grant permissions to a group (e.g. "orca-admins") and +// any OIDC user with that group inherits the permission. +func OidcGroupIdentity(group string) Identity { + return Identity{ + Kind: KindOidc, + ID: "group:" + group, + } +} + +// CheckOidc reports whether an OIDC user (by sub + groups) has perm +// on ns. It checks both the user's own entry (by sub) and any group +// entries (by group: prefix). Admin implies Read + Write. +func (a *ACL) CheckOidc(claims OIDCClaims, ns string, perm Permission) bool { + // First check the user's own entry. + if a.Check(OidcIdentity(claims), ns, perm) { + return true + } + // Then check each group entry. + for _, g := range claims.Groups { + if a.Check(OidcGroupIdentity(g), ns, perm) { + return true + } + } + return false +} + +// CheckTokenDeprecated is a stub that always returns false. KindToken +// is deprecated (R-021); this ensures any existing KindToken entries in +// acl.json are inert. P07 removes them; P22 migrates. +func (a *ACL) CheckTokenDeprecated(tokenID, ns string, perm Permission) bool { + return false +} diff --git a/internal/acl/acl_test.go b/internal/acl/acl_test.go index 821f8c4..3464b8e 100644 --- a/internal/acl/acl_test.go +++ b/internal/acl/acl_test.go @@ -8,7 +8,7 @@ import ( func TestGrantAndCheck(t *testing.T) { a := NewACL() - id := Identity{Kind: KindToken, ID: "tok-A", Namespace: "test"} + id := Identity{Kind: KindOidc, ID: "tok-A", Namespace: "test"} a.Grant(id, "test", PermRead) if !a.Check(id, "test", PermRead) { t.Errorf("Check(Read) = false, want true after Grant(Read)") @@ -20,7 +20,7 @@ func TestGrantAndCheck(t *testing.T) { func TestRevoke(t *testing.T) { a := NewACL() - id := Identity{Kind: KindToken, ID: "tok-A", Namespace: "test"} + id := Identity{Kind: KindOidc, ID: "tok-A", Namespace: "test"} a.Grant(id, "test", PermRead) a.Revoke(id, "test") if a.Check(id, "test", PermRead) { @@ -33,7 +33,7 @@ func TestRevoke(t *testing.T) { func TestRevokeNonExistentNoOp(t *testing.T) { a := NewACL() - id := Identity{Kind: KindToken, ID: "tok-A", Namespace: "test"} + id := Identity{Kind: KindOidc, ID: "tok-A", Namespace: "test"} a.Revoke(id, "ghost") if got := a.List(); len(got) != 0 { t.Errorf("List() len = %d after no-op Revoke, want 0", len(got)) @@ -42,7 +42,7 @@ func TestRevokeNonExistentNoOp(t *testing.T) { func TestDenyByDefault(t *testing.T) { a := NewACL() - id := Identity{Kind: KindToken, ID: "tok-A", Namespace: "test"} + id := Identity{Kind: KindOidc, ID: "tok-A", Namespace: "test"} if a.Check(id, "test", PermRead) { t.Errorf("Check on un-granted identity = true, want false (deny-by-default)") } @@ -56,7 +56,7 @@ func TestDenyByDefault(t *testing.T) { func TestNamespaceIsolation(t *testing.T) { a := NewACL() - id := Identity{Kind: KindToken, ID: "tok-A", Namespace: "ns-A"} + id := Identity{Kind: KindOidc, ID: "tok-A", Namespace: "ns-A"} a.Grant(id, "ns-A", PermRead) if !a.Check(id, "ns-A", PermRead) { t.Errorf("Check on ns-A = false, want true") @@ -68,7 +68,7 @@ func TestNamespaceIsolation(t *testing.T) { func TestGrantReplacesPermissions(t *testing.T) { a := NewACL() - id := Identity{Kind: KindToken, ID: "tok-A", Namespace: "test"} + id := Identity{Kind: KindOidc, ID: "tok-A", Namespace: "test"} a.Grant(id, "test", PermRead) a.Grant(id, "test", PermWrite) if a.Check(id, "test", PermRead) { @@ -122,7 +122,7 @@ func TestPermissionsDistinct(t *testing.T) { t.Errorf("permission flags collide: read=%d write=%d admin=%d", PermRead, PermWrite, PermAdmin) } a := NewACL() - id := Identity{Kind: KindToken, ID: "tok-A", Namespace: "test"} + id := Identity{Kind: KindOidc, ID: "tok-A", Namespace: "test"} a.Grant(id, "test", PermRead|PermWrite) if !a.Check(id, "test", PermRead) { t.Errorf("Check(Read) for read+write grant = false, want true") @@ -137,7 +137,7 @@ func TestPermissionsDistinct(t *testing.T) { func TestAdminImpliesReadAndWrite(t *testing.T) { a := NewACL() - id := Identity{Kind: KindToken, ID: "tok-A", Namespace: "test"} + id := Identity{Kind: KindOidc, ID: "tok-A", Namespace: "test"} a.Grant(id, "test", PermAdmin) if !a.Check(id, "test", PermAdmin) { t.Errorf("Check(Admin) = false, want true") @@ -152,7 +152,7 @@ func TestAdminImpliesReadAndWrite(t *testing.T) { func TestConcurrentAccess(t *testing.T) { a := NewACL() - id := Identity{Kind: KindToken, ID: "tok-concurrent", Namespace: "ns"} + id := Identity{Kind: KindOidc, ID: "tok-concurrent", Namespace: "ns"} const n = 200 var wg sync.WaitGroup wg.Add(n * 3) @@ -181,7 +181,7 @@ func TestConcurrentAccess(t *testing.T) { func TestListIsCopy(t *testing.T) { a := NewACL() - id := Identity{Kind: KindToken, ID: "tok-A", Namespace: "test"} + id := Identity{Kind: KindOidc, ID: "tok-A", Namespace: "test"} a.Grant(id, "test", PermRead) lst := a.List() lst[0].Permissions = PermAdmin @@ -208,7 +208,7 @@ func TestTokenAndSpiffeIdentitiesIndependent(t *testing.T) { a := NewACL() uri := "spiffe://orca.local/ns/prod/sa/api/0" spiffeID := Identity{Kind: KindSpiffe, ID: uri, Namespace: "prod"} - tokenID := Identity{Kind: KindToken, ID: "operator-1", Namespace: "prod"} + tokenID := Identity{Kind: KindOidc, ID: "operator-1", Namespace: "prod"} a.Grant(spiffeID, "prod", PermRead) if a.Check(tokenID, "prod", PermRead) { t.Errorf("token identity matched spiffe grant (kind isolation broken)") @@ -232,3 +232,64 @@ func ExampleSpiffeNamespace() { fmt.Println(ns) // Output: myapp } + +// --- REQ-145 / F1 ACL OIDC rewrite tests --- + +// TestACLOidcUserGrant verifies an OIDC user (by sub) can be granted +// and checked. +func TestACLOidcUserGrant(t *testing.T) { + a := NewACL() + claims := OIDCClaims{Subject: "user-1", Groups: []string{"devs"}} + a.Grant(OidcIdentity(claims), "prod", PermWrite|PermRead) + if !a.CheckOidc(claims, "prod", PermWrite) { + t.Error("CheckOidc should allow write") + } + if !a.CheckOidc(claims, "prod", PermRead) { + t.Error("CheckOidc should allow read (explicit)") + } + if a.CheckOidc(claims, "prod", PermAdmin) { + t.Error("CheckOidc should deny admin") + } + if a.CheckOidc(claims, "other", PermRead) { + t.Error("CheckOidc should deny on wrong ns") + } +} + +// TestACLOidcGroupGrant verifies group-based grants work. +func TestACLOidcGroupGrant(t *testing.T) { + a := NewACL() + a.Grant(OidcGroupIdentity("orca-admins"), "prod", PermAdmin) + claims := OIDCClaims{Subject: "user-2", Groups: []string{"orca-admins"}} + if !a.CheckOidc(claims, "prod", PermAdmin) { + t.Error("admin group should have admin") + } + if !a.CheckOidc(claims, "prod", PermWrite) { + t.Error("admin implies write") + } + claimsNoGroup := OIDCClaims{Subject: "user-3", Groups: []string{"devs"}} + if a.CheckOidc(claimsNoGroup, "prod", PermRead) { + t.Error("non-admin group should deny") + } +} + +// TestACLOidcDenyByDefault verifies an ungranted OIDC user is denied. +func TestACLOidcDenyByDefault(t *testing.T) { + a := NewACL() + claims := OIDCClaims{Subject: "nobody"} + if a.CheckOidc(claims, "prod", PermRead) { + t.Error("ungranted user should deny") + } +} + +// TestACLTokenDeprecated verifies KindToken always denies (R-021). +func TestACLTokenDeprecated(t *testing.T) { + a := NewACL() + // Even if an old acl.json has a KindToken entry, Check returns false. + a.Grant(Identity{Kind: KindToken, ID: "old-token-123"}, "prod", PermAdmin) + if a.Check(Identity{Kind: KindToken, ID: "old-token-123"}, "prod", PermRead) { + t.Error("KindToken should always deny (R-021)") + } + if a.Check(Identity{Kind: KindToken, ID: "old-token-123"}, "prod", PermAdmin) { + t.Error("KindToken should always deny even admin (R-021)") + } +}