Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 45 additions & 10 deletions .claude/rules/auth.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,55 @@ bypass bugs. A user should never see an entry via one endpoint but not another.

## 4. Default-Deny When Authz Is Enabled

If an entry has claims the caller's JWT doesn't cover, the entry is invisible. Missing data
means "not permitted," not "open."
When authz is enabled, **unlabeled is not the same as public**. A resource with no claims
is treated as "no rule matches" → deny, not "open to all." This is the Saltzer–Schroeder
*fail-safe defaults* principle and matches how every Zero Trust framework (NIST SP
800-162, OWASP Access Control, AWS IAM "implicit deny") handles the absence of an
authorization rule.

**What must hold:**
- Per-user filtering on a row with non-empty claims fails closed.
- A row with claims the caller's JWT doesn't cover is invisible to that caller.
- A row with **empty/missing claims** is also invisible to claim-bearing callers — only
super-admin (or anonymous mode / authz-off, where the gate is bypassed entirely) can
reach it. "Public" must be expressed with an explicit positive claim that the role
config maps to all authenticated users.
- Registry access gate returns **403** (not 404) when the caller fails it. 404 would leak
registry existence.
- Empty-claims rows are visible to all authenticated users (deliberate — operators can
create "open" entries). Publishing with empty claims is **forbidden** when authz is
active (see §6).

**Detect**: new code paths that return an entry when the claims check is ambiguous; 404
responses on registry access denial; publish handlers that accept empty claims when
`authzEnabled`.
- The list filter and the single-resource gate must agree on this rule. Inconsistency
between list and get/put paths produces "visible via one endpoint, invisible via
another" bugs.
- Publishing with empty claims is **forbidden** when authz is active (see §6) — this is
the write-time enforcement that complements the read-time default-deny.

**Detect**: a gate that returns nil when the resource's claims are empty and the caller's
claims are non-empty; new "empty = open" carve-outs; 404 responses on registry access
denial; publish handlers that accept empty claims when `authzEnabled`.

**Implementation note**: `validateClaimsSubset` and `validateClaimsSubsetBytes` in
`internal/service/db/claims_filter.go` are package-level functions and intentionally
have no knowledge of `s.skipAuthz`. They short-circuit when `callerClaims == nil`, so
every callsite is responsible for passing `nil` when authz is off — the standard pattern
is `gateClaims := …; if s.skipAuthz { gateClaims = nil }`. Keeping the gate functions
pure preserves the layering: internal-state inspection lives at the callsite, not inside
the matching algorithm. Turning authz off disables the entire gate, including its
default-deny posture on empty claims.

**Upgrading from "empty = open" to default-deny**: deployments that ran with authz off,
or that ingest synced sources whose entries have no claims, will have rows in
`registry_entry`, `sources`, and `registries` with `claims IS NULL`. After turning authz
on (or upgrading to a release that includes default-deny), those rows are invisible to
every claim-bearing caller. Two recovery paths:

- **Per-entry**: a super-admin can read and re-tag each affected row via
`PUT /v1/entries/{type}/{name}/claims` (or the equivalent source/registry endpoints).
- **Operator-managed sources**: tag the managed source itself with a tenant-wide claim
(e.g. `{org: "acme"}`) in config so writers can reference it; otherwise no
non-super-admin caller can publish to it. This is the most common stumbling block when
enabling authz on an existing deployment — forgetting to tag the managed source looks
like a permissions bug at publish time.

There is intentionally no automatic "backfill claims from JWT" path — that would
re-introduce the "empty = the caller's identity" behavior that this rule rejects.

## 5. Subset Validation on Every Write

Expand Down
7 changes: 5 additions & 2 deletions internal/authz/authz_full_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,10 @@ func TestAuthzIntegration_FullAuthzMode(t *testing.T) {
assertStatus(t, resp, 403)
})

t.Run("outsider sees only internal source", func(t *testing.T) {
t.Run("outsider sees no sources", func(t *testing.T) {
// Default-deny on empty claims (auth.md §4) plus the tenant-wide
// {org: acme} claim on every fixture source means a contoso outsider
// covers nothing. There are no truly-public sources in this fixture.
resp := doRequest(t, "GET", env.baseURL+"/v1/sources", outsider, nil)
body := assertStatus(t, resp, 200)
var result struct {
Expand All @@ -213,7 +216,7 @@ func TestAuthzIntegration_FullAuthzMode(t *testing.T) {
} `json:"sources"`
}
require.NoError(t, json.Unmarshal([]byte(body), &result))
assert.Len(t, result.Sources, 1)
assert.Empty(t, result.Sources)
})

t.Run("super-admin sees all sources", func(t *testing.T) {
Expand Down
6 changes: 5 additions & 1 deletion internal/authz/authz_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,11 @@ func baseSourceConfigs(t *testing.T) []config.SourceConfig {
SyncPolicy: &config.SyncPolicyConfig{Interval: "10s"},
Claims: map[string]any{"org": "acme", "team": "data"},
},
{Name: "internal", Managed: &config.ManagedConfig{}},
// Tag the managed "internal" source with a tenant-wide claim so any acme
// caller can reference it. Default-deny on empty claims (auth.md §4)
// would otherwise make this source unreferenceable for everyone but
// super-admin.
{Name: "internal", Claims: map[string]any{"org": "acme"}, Managed: &config.ManagedConfig{}},
}
}

Expand Down
11 changes: 7 additions & 4 deletions internal/service/db/admin_claims_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,9 @@ func TestListSources_ReturnsAllMatchingWithMixedClaims(t *testing.T) {
_, err = svc.CreateSource(createCtx, "lsm-open", fileDataSourceReq(nil))
require.NoError(t, err)

// List with acme JWT — should see both acme sources and the open (no-claims) source
// List with acme JWT — should see both acme sources only.
// The open (no-claims) source is invisible by default-deny (auth.md §4):
// unlabeled resources are not visible to claim-bearing callers.
listCtx := auth.ContextWithClaims(t.Context(), jwt.MapClaims{"org": "acme"})
sources, err := svc.ListSources(listCtx)
require.NoError(t, err)
Expand All @@ -592,7 +594,7 @@ func TestListSources_ReturnsAllMatchingWithMixedClaims(t *testing.T) {
assert.Contains(t, names, "lsm-acme-1")
assert.Contains(t, names, "lsm-acme-2")
assert.NotContains(t, names, "lsm-contoso")
assert.Contains(t, names, "lsm-open")
assert.NotContains(t, names, "lsm-open")
}

func TestListRegistries_ReturnsAllMatchingWithMixedClaims(t *testing.T) {
Expand Down Expand Up @@ -632,7 +634,8 @@ func TestListRegistries_ReturnsAllMatchingWithMixedClaims(t *testing.T) {
})
require.NoError(t, err)

// List with acme JWT — should see both acme registries and the open one
// List with acme JWT — should see both acme registries only.
// The open (no-claims) registry is invisible by default-deny (auth.md §4).
listCtx := auth.ContextWithClaims(t.Context(), jwt.MapClaims{"org": "acme"})
registries, err := svc.ListRegistries(listCtx)
require.NoError(t, err)
Expand All @@ -641,7 +644,7 @@ func TestListRegistries_ReturnsAllMatchingWithMixedClaims(t *testing.T) {
assert.Contains(t, names, "lrm-acme-reg-1")
assert.Contains(t, names, "lrm-acme-reg-2")
assert.NotContains(t, names, "lrm-contoso-reg")
assert.Contains(t, names, "lrm-open-reg")
assert.NotContains(t, names, "lrm-open-reg")
}

func TestListSources_AnonymousReturnsAll(t *testing.T) {
Expand Down
54 changes: 45 additions & 9 deletions internal/service/db/claims_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,29 +43,34 @@ func checkClaimConsistency(incomingJSON, existingJSON []byte) error {

// validateClaimsSubset checks that callerClaims covers resourceClaims.
// Returns nil if:
// - callerClaims is nil (anonymous mode — no auth enforcement)
// - resourceClaims is nil/empty (open resource — no restriction)
// - callerClaims is nil (caller has opted out of the gate — callers pass
// nil when skipAuthz is enabled or in anonymous mode)
// - the caller is a super-admin (bypasses all claim checks)
// - callerClaims is a superset of resourceClaims
//
// Returns ErrClaimsInsufficient otherwise.
// Returns ErrClaimsInsufficient otherwise, including when resourceClaims
// is nil/empty (default-deny on unlabeled resources — see auth.md §4).
func validateClaimsSubset(ctx context.Context, callerClaims, resourceClaims map[string]any) error {
if callerClaims == nil || len(resourceClaims) == 0 {
if callerClaims == nil {
return nil
}
if auth.IsSuperAdmin(ctx) {
return nil
}
if len(resourceClaims) == 0 {
return fmt.Errorf("%w: resource has no claims", service.ErrClaimsInsufficient)
}
if !claimsContain(callerClaims, resourceClaims) {
return fmt.Errorf("%w: caller claims do not cover resource claims", service.ErrClaimsInsufficient)
}
return nil
}

// validateClaimsSubsetBytes is like validateClaimsSubset but accepts raw JSON
// for resourceClaims. Nil or empty JSON is treated as an open resource.
// for resourceClaims. An empty resource JSON is default-deny when callerClaims
// is non-nil (unlabeled resources are invisible to claim-bearing callers).
func validateClaimsSubsetBytes(ctx context.Context, callerClaims map[string]any, resourceClaimsJSON []byte) error {
if callerClaims == nil || len(resourceClaimsJSON) == 0 {
if callerClaims == nil {
return nil
}
resourceClaims := db.DeserializeClaims(resourceClaimsJSON)
Expand All @@ -90,9 +95,10 @@ func claimsFromCtx(ctx context.Context) map[string]any {
// caller's claims are non-empty, the record has stored claims, and they match.
// extract retrieves the raw claims JSON from a record; returning ok=false
// causes the filter to reject the record with a type error.
// Returns nil when callerClaims is nil or empty so the caller can skip
// filtering entirely. Also returns nil for super-admin users (they see all
// entries regardless of claims).
// Returns nil (no filter applied — every record visible) when:
// - callerClaims is nil/empty (callers pass nil when skipAuthz is enabled
// or in anonymous mode)
// - the caller is a super-admin (uniform bypass)
func newClaimsFilterWith(
ctx context.Context,
callerClaims map[string]any,
Expand Down Expand Up @@ -155,8 +161,17 @@ func checkClaims(callerJSON, recordJSON []byte) bool {
// An empty-array value (e.g. "teams": []) is vacuously satisfied by any caller
// value for that key — this is intentional since ValidateClaimValues accepts
// empty arrays, and presence of the key is the meaningful signal.
//
// Fails closed (returns false) when a record value has an unsupported type
// (nil, number, nested object, etc.). ValidateClaimValues blocks such values
// at the API edge, but rows persisted via direct DB writes or future sync
// paths could carry them — the gate must not silently treat them as
// vacuously-satisfied requirements.
func claimsContain(caller, record map[string]any) bool {
for k, rv := range record {
if !isValidClaimValue(rv) {
return false
}
cv, ok := caller[k]
if !ok {
return false
Expand All @@ -172,6 +187,27 @@ func claimsContain(caller, record map[string]any) bool {
return true
}

// isValidClaimValue reports whether v is a supported claim value (string,
// []string, or []any of strings). Used by claimsContain to fail closed on
// rows whose values bypassed ValidateClaimValues.
func isValidClaimValue(v any) bool {
switch val := v.(type) {
case string:
return true
case []string:
return true
case []any:
for _, elem := range val {
if _, ok := elem.(string); !ok {
return false
}
}
return true
default:
return false
}
}

// claimsEqual returns true when a and b have exactly the same keys and values.
// Used to enforce strict claim consistency on subsequent publishes of the same entry name.
func claimsEqual(a, b map[string]any) bool {
Expand Down
72 changes: 72 additions & 0 deletions internal/service/db/claims_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,46 @@ func TestCheckClaims(t *testing.T) {
recordJSON: []byte(`{"sub":"u1"}`),
want: false,
},
// Default-deny on unsupported record value types (auth.md §4).
// ValidateClaimValues blocks these at the API edge, but synced
// sources or direct DB writes could persist them — claimsContain
// must fail closed rather than treat them as vacuously satisfied.
{
name: "record null value drops record",
callerJSON: []byte(`{"team":"platform"}`),
recordJSON: []byte(`{"team":null}`),
want: false,
},
{
name: "record number value drops record",
callerJSON: []byte(`{"team":"platform"}`),
recordJSON: []byte(`{"team":42}`),
want: false,
},
{
name: "record nested object value drops record",
callerJSON: []byte(`{"team":"platform"}`),
recordJSON: []byte(`{"team":{"nested":"x"}}`),
want: false,
},
{
name: "record mixed-type array drops record",
callerJSON: []byte(`{"team":["platform","ops"]}`),
recordJSON: []byte(`{"team":["platform",42]}`),
want: false,
},
{
name: "record boolean value drops record",
callerJSON: []byte(`{"team":"platform"}`),
recordJSON: []byte(`{"team":true}`),
want: false,
},
{
name: "record empty array vacuously satisfied (intentional)",
callerJSON: []byte(`{"team":"platform"}`),
recordJSON: []byte(`{"team":[]}`),
want: true,
},
}

for _, tt := range tests {
Expand All @@ -203,6 +243,38 @@ func TestCheckClaims(t *testing.T) {
}
}

func TestIsValidClaimValue(t *testing.T) {
t.Parallel()

tests := []struct {
name string
v any
want bool
}{
{name: "string", v: "platform", want: true},
{name: "empty string", v: "", want: true},
{name: "[]string", v: []string{"a", "b"}, want: true},
{name: "empty []string", v: []string{}, want: true},
{name: "[]any of strings", v: []any{"a", "b"}, want: true},
{name: "empty []any", v: []any{}, want: true},
{name: "nil", v: nil, want: false},
{name: "int", v: 42, want: false},
{name: "float", v: 3.14, want: false},
{name: "bool", v: true, want: false},
{name: "nested map", v: map[string]any{"k": "v"}, want: false},
{name: "[]any with int", v: []any{"a", 42}, want: false},
{name: "[]any with nil", v: []any{"a", nil}, want: false},
{name: "[]any with nested map", v: []any{"a", map[string]any{}}, want: false},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
assert.Equal(t, tt.want, isValidClaimValue(tt.v))
})
}
}

func TestMarshalClaims(t *testing.T) {
t.Parallel()

Expand Down
25 changes: 16 additions & 9 deletions internal/service/db/claims_filter_validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,28 @@ func TestValidateClaimsSubset(t *testing.T) {
wantErr error
}{
{
name: "nil caller claims (anonymous) returns nil",
name: "nil caller claims (anonymous or skipAuthz) returns nil",
callerClaims: nil,
resourceClaims: map[string]any{"sub": "user1"},
wantErr: nil,
},
{
name: "nil resource claims returns nil",
name: "nil resource claims returns ErrClaimsInsufficient (default-deny)",
callerClaims: map[string]any{"sub": "user1"},
resourceClaims: nil,
wantErr: nil,
wantErr: service.ErrClaimsInsufficient,
},
{
name: "empty resource claims returns nil",
name: "empty resource claims returns ErrClaimsInsufficient (default-deny)",
callerClaims: map[string]any{"sub": "user1"},
resourceClaims: map[string]any{},
wantErr: service.ErrClaimsInsufficient,
},
{
name: "super-admin bypasses default-deny on empty resource claims",
callerClaims: map[string]any{"sub": "admin"},
resourceClaims: map[string]any{},
superAdmin: true,
wantErr: nil,
},
{
Expand Down Expand Up @@ -117,22 +124,22 @@ func TestValidateClaimsSubsetBytes(t *testing.T) {
wantErr error
}{
{
name: "nil caller claims returns nil",
name: "nil caller claims (anonymous or skipAuthz) returns nil",
callerClaims: nil,
resourceJSON: []byte(`{"sub":"user1"}`),
wantErr: nil,
},
{
name: "nil resource JSON returns nil",
name: "nil resource JSON returns ErrClaimsInsufficient (default-deny)",
callerClaims: map[string]any{"sub": "user1"},
resourceJSON: nil,
wantErr: nil,
wantErr: service.ErrClaimsInsufficient,
},
{
name: "empty resource JSON returns nil",
name: "empty resource JSON returns ErrClaimsInsufficient (default-deny)",
callerClaims: map[string]any{"sub": "user1"},
resourceJSON: []byte{},
wantErr: nil,
wantErr: service.ErrClaimsInsufficient,
},
{
name: "matching JSON returns nil",
Expand Down
2 changes: 2 additions & 0 deletions internal/service/db/impl_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ func (s *dbService) CheckReadiness(ctx context.Context) error {
// after verifying the caller's claims satisfy the registry's access gate.
// Returns ErrClaimsInsufficient if the caller's JWT claims do not cover the
// registry's claims. Returns ErrRegistryNotFound if the registry does not exist.
// Callers must pass nil for callerClaims when the gate should be bypassed
// (skipAuthz mode or anonymous mode).
func lookupRegistryIDWithGate(
ctx context.Context, pool sqlc.DBTX, registryName string, callerClaims map[string]any,
) (uuid.UUID, error) {
Expand Down
Loading
Loading