Skip to content

Commit 1ef910e

Browse files
committed
Tighten getEntryClaims godoc and cover super-admin bypass
The godoc previously said the nil-claims-to-{} normalisation was "unreachable in practice" under authz — that's wrong for super-admin, who bypasses the subset check uniformly and would hit that branch when reading legacy or synced rows that lack claims. Reword to call out the super-admin path explicitly so the next reader doesn't trust a stale invariant. Add `TestGetEntryClaims_SuperAdminBypassesSubsetCheck` at the db layer: super-admin caller with non-covering JWT claims still reads the entry. Pairs with the existing cross-team-denied case to lock in both halves of the bypass contract.
1 parent d761d67 commit 1ef910e

2 files changed

Lines changed: 38 additions & 3 deletions

File tree

internal/api/v1/entries.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,10 @@ type entryClaimsResponse struct {
204204
//
205205
// The response envelope `{"claims": {...}}` is always a non-nil JSON object —
206206
// the impl normalises a missing/nil claims blob to `map[string]any{}`. Under
207-
// authz, that branch is unreachable in practice (publish forbids empty claims
208-
// per auth.md §6, and the gate denies empty-claim rows per §4); the
209-
// normalisation is for the auth-off / synced-source case.
207+
// authz, that branch is reachable only by super-admin (the publish path
208+
// forbids empty claims per auth.md §6, and the gate denies empty-claim rows
209+
// for everyone else per §4) or by callers in `skipAuthz=true` deployments.
210+
// Either way the response shape stays stable.
210211
//
211212
// @Summary Get entry claims
212213
// @Description Get the claims for a published entry name. Claims are stored at the

internal/service/db/get_entry_claims_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/stretchr/testify/assert"
99
"github.com/stretchr/testify/require"
1010

11+
"github.com/stacklok/toolhive-registry-server/internal/auth"
1112
"github.com/stacklok/toolhive-registry-server/internal/service"
1213
)
1314

@@ -97,6 +98,39 @@ func TestGetEntryClaims_ClaimsInsufficient(t *testing.T) {
9798
assert.ErrorIs(t, err, service.ErrClaimsInsufficient)
9899
}
99100

101+
func TestGetEntryClaims_SuperAdminBypassesSubsetCheck(t *testing.T) {
102+
t.Parallel()
103+
104+
svc, cleanup := setupTestService(t)
105+
defer cleanup()
106+
107+
createManagedSource(t, svc, "gec-superadmin")
108+
109+
ctx := context.Background()
110+
111+
// Publish an entry scoped to team=data with JWT claims that cover it.
112+
_, err := svc.PublishServerVersion(ctx,
113+
service.WithServerData(&upstreamv0.ServerJSON{
114+
Name: "com.test/gec-superadmin",
115+
Version: "1.0.0",
116+
}),
117+
service.WithClaims(map[string]any{"org": "acme", "team": "data"}),
118+
service.WithJWTClaims(map[string]any{"org": "acme", "team": []string{"data"}}),
119+
)
120+
require.NoError(t, err)
121+
122+
// A super-admin caller in a completely different org must still read the
123+
// entry's claims — the bypass is uniform across every claim check.
124+
superAdminCtx := auth.ContextWithRoles(ctx, []auth.Role{auth.RoleSuperAdmin})
125+
claims, err := svc.GetEntryClaims(superAdminCtx,
126+
service.WithEntryType(service.EntryTypeServer),
127+
service.WithName("com.test/gec-superadmin"),
128+
service.WithJWTClaims(map[string]any{"org": "contoso"}),
129+
)
130+
require.NoError(t, err)
131+
assert.Equal(t, map[string]any{"org": "acme", "team": "data"}, claims)
132+
}
133+
100134
func TestGetEntryClaims_ClaimsSufficient(t *testing.T) {
101135
t.Parallel()
102136

0 commit comments

Comments
 (0)