Skip to content

Commit fdb4793

Browse files
committed
Gate GetEntryClaims by JWT claim subset
Run `validateClaimsSubsetBytes` against the entry's stored claims inside `GetEntryClaims` and surface `ErrClaimsInsufficient` as 403 in the handler. Without this, a `manageEntries` caller scoped to one team could read the claim metadata of an entry scoped to a different team even though the matching `PUT` would deny them — that asymmetry contradicts the default-deny visibility rule in `auth.md` §4. `GetEntryClaimsOptions` gains a `JWTClaims` field and `setJWTClaims` setter so the handler can plumb the caller's JWT through `WithJWTClaims`, mirroring how `UpdateEntryClaims` already works. Super-admin still bypasses uniformly via the existing helper. Tests: handler-level case for `ErrClaimsInsufficient` → 403; db-layer cases for cross-team denied / covering caller succeeds; integration sub-test exercising a cross-team writer against a platform-scoped entry. Swagger regenerated for the new 403 response.
1 parent 03387aa commit fdb4793

9 files changed

Lines changed: 142 additions & 7 deletions

File tree

docs/thv-registry-api/docs.go

Lines changed: 13 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/thv-registry-api/swagger.json

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2049,6 +2049,19 @@
20492049
},
20502050
"description": "Bad request"
20512051
},
2052+
"403": {
2053+
"content": {
2054+
"application/json": {
2055+
"schema": {
2056+
"additionalProperties": {
2057+
"type": "string"
2058+
},
2059+
"type": "object"
2060+
}
2061+
}
2062+
},
2063+
"description": "Forbidden"
2064+
},
20522065
"404": {
20532066
"content": {
20542067
"application/json": {

docs/thv-registry-api/swagger.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1399,6 +1399,14 @@ paths:
13991399
type: string
14001400
type: object
14011401
description: Bad request
1402+
"403":
1403+
content:
1404+
application/json:
1405+
schema:
1406+
additionalProperties:
1407+
type: string
1408+
type: object
1409+
description: Forbidden
14021410
"404":
14031411
content:
14041412
application/json:

internal/api/v1/entries.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ type entryClaimsResponse struct {
203203
// @Param name path string true "Entry Name"
204204
// @Success 200 {object} entryClaimsResponse "Entry claims"
205205
// @Failure 400 {object} map[string]string "Bad request"
206+
// @Failure 403 {object} map[string]string "Forbidden"
206207
// @Failure 404 {object} map[string]string "Not found"
207208
// @Failure 500 {object} map[string]string "Internal server error"
208209
// @Failure 503 {object} map[string]string "No managed source available"
@@ -220,15 +221,24 @@ func (routes *Routes) getEntryClaims(w http.ResponseWriter, r *http.Request) {
220221
return
221222
}
222223

223-
claims, err := routes.service.GetEntryClaims(r.Context(),
224+
opts := []service.Option{
224225
service.WithEntryType(entryType),
225226
service.WithName(name),
226-
)
227+
}
228+
if jwtClaims := auth.ClaimsFromContext(r.Context()); jwtClaims != nil {
229+
opts = append(opts, service.WithJWTClaims(map[string]any(jwtClaims)))
230+
}
231+
232+
claims, err := routes.service.GetEntryClaims(r.Context(), opts...)
227233
if err != nil {
228234
if errors.Is(err, service.ErrInvalidEntryType) {
229235
common.WriteErrorResponse(w, err.Error(), http.StatusBadRequest)
230236
return
231237
}
238+
if errors.Is(err, service.ErrClaimsInsufficient) {
239+
common.WriteErrorResponse(w, err.Error(), http.StatusForbidden)
240+
return
241+
}
232242
if errors.Is(err, service.ErrNotFound) {
233243
common.WriteErrorResponse(w, err.Error(), http.StatusNotFound)
234244
return

internal/api/v1/entries_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,16 @@ func TestGetEntryClaims(t *testing.T) {
571571
wantStatus: http.StatusServiceUnavailable,
572572
wantError: "no managed source available",
573573
},
574+
{
575+
name: "claims insufficient",
576+
path: "/entries/server/test%2Fserver/claims",
577+
setupMock: func(m *mocks.MockRegistryService) {
578+
m.EXPECT().GetEntryClaims(gomock.Any(), gomock.Any(), gomock.Any()).
579+
Return(nil, service.ErrClaimsInsufficient)
580+
},
581+
wantStatus: http.StatusForbidden,
582+
wantError: "insufficient claims",
583+
},
574584
{
575585
name: "generic service error",
576586
path: "/entries/server/test%2Fserver/claims",

internal/authz/authz_get_entry_claims_test.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ import (
1313
)
1414

1515
// TestAuthzIntegration_GetEntryClaims exercises the GET /v1/entries/{type}/{name}/claims
16-
// endpoint across roles:
17-
// - manageEntries (writer) and superAdmin can read claims (200)
18-
// - admin (manageSources + manageRegistries only) is denied (403)
16+
// endpoint across roles and claim coverage:
17+
// - manageEntries (writer) covering the entry's claims and superAdmin can read (200)
18+
// - manageEntries (writer) NOT covering the entry's claims is denied (403)
19+
// - admin (manageSources + manageRegistries only) is denied by role gate (403)
1920
// - tokens with no matching role are denied (403)
2021
// - unauthenticated requests are rejected (401)
2122
//
@@ -32,6 +33,7 @@ func TestAuthzIntegration_GetEntryClaims(t *testing.T) {
3233
})
3334

3435
platformWriter := env.oidc.token(t, map[string]any{"org": "acme", "team": "platform", "role": "writer"})
36+
dataWriter := env.oidc.token(t, map[string]any{"org": "acme", "team": "data", "role": "writer"})
3537
platformAdmin := env.oidc.token(t, map[string]any{"org": "acme", "team": "platform", "role": "admin"})
3638
superAdmin := env.oidc.token(t, map[string]any{"org": "acme", "role": "super-admin"})
3739
noRole := env.oidc.token(t, map[string]any{"org": "acme"})
@@ -70,6 +72,12 @@ func TestAuthzIntegration_GetEntryClaims(t *testing.T) {
7072
assertClaims(t, body, publishedClaims)
7173
})
7274

75+
t.Run("cross-team writer denied", func(t *testing.T) {
76+
// dataWriter has manageEntries role but no claim coverage for team=platform.
77+
resp := doRequest(t, "GET", env.baseURL+claimsPath, dataWriter, nil)
78+
assertStatus(t, resp, 403)
79+
})
80+
7381
t.Run("manageSources/manageRegistries denied", func(t *testing.T) {
7482
resp := doRequest(t, "GET", env.baseURL+claimsPath, platformAdmin, nil)
7583
assertStatus(t, resp, 403)

internal/service/db/get_entry_claims_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,66 @@ func TestGetEntryClaims_EmptyClaimsReturnsNonNilMap(t *testing.T) {
6767
assert.Empty(t, claims)
6868
}
6969

70+
func TestGetEntryClaims_ClaimsInsufficient(t *testing.T) {
71+
t.Parallel()
72+
73+
svc, cleanup := setupTestService(t)
74+
defer cleanup()
75+
76+
createManagedSource(t, svc, "gec-insufficient")
77+
78+
ctx := context.Background()
79+
80+
// Publish an entry scoped to team=data.
81+
_, err := svc.PublishServerVersion(ctx,
82+
service.WithServerData(&upstreamv0.ServerJSON{
83+
Name: "com.test/gec-insufficient",
84+
Version: "1.0.0",
85+
}),
86+
service.WithClaims(map[string]any{"org": "acme", "team": "data"}),
87+
service.WithJWTClaims(map[string]any{"org": "acme", "team": []string{"data"}}),
88+
)
89+
require.NoError(t, err)
90+
91+
// A caller in team=platform must not be able to read its claims.
92+
_, err = svc.GetEntryClaims(ctx,
93+
service.WithEntryType(service.EntryTypeServer),
94+
service.WithName("com.test/gec-insufficient"),
95+
service.WithJWTClaims(map[string]any{"org": "acme", "team": "platform"}),
96+
)
97+
assert.ErrorIs(t, err, service.ErrClaimsInsufficient)
98+
}
99+
100+
func TestGetEntryClaims_ClaimsSufficient(t *testing.T) {
101+
t.Parallel()
102+
103+
svc, cleanup := setupTestService(t)
104+
defer cleanup()
105+
106+
createManagedSource(t, svc, "gec-sufficient")
107+
108+
ctx := context.Background()
109+
110+
_, err := svc.PublishServerVersion(ctx,
111+
service.WithServerData(&upstreamv0.ServerJSON{
112+
Name: "com.test/gec-sufficient",
113+
Version: "1.0.0",
114+
}),
115+
service.WithClaims(map[string]any{"org": "acme", "team": "platform"}),
116+
service.WithJWTClaims(map[string]any{"org": "acme", "team": []string{"platform"}}),
117+
)
118+
require.NoError(t, err)
119+
120+
// A caller whose JWT covers the entry's claims must succeed.
121+
claims, err := svc.GetEntryClaims(ctx,
122+
service.WithEntryType(service.EntryTypeServer),
123+
service.WithName("com.test/gec-sufficient"),
124+
service.WithJWTClaims(map[string]any{"org": "acme", "team": []string{"platform", "ops"}}),
125+
)
126+
require.NoError(t, err)
127+
assert.Equal(t, map[string]any{"org": "acme", "team": "platform"}, claims)
128+
}
129+
70130
func TestGetEntryClaims_EntryNotFound(t *testing.T) {
71131
t.Parallel()
72132

internal/service/db/impl_entries.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,9 @@ func mapEntryType(entryType string) (sqlc.EntryType, error) {
149149

150150
// GetEntryClaims returns the claims map for a published entry within the managed source.
151151
// The returned map is non-nil even when the entry has no claims set, so callers can
152-
// rely on a stable JSON shape. The endpoint is admin-side and gated by role; it does
153-
// not run a claim-subset check against the caller's JWT.
152+
// rely on a stable JSON shape. Access is gated by the manageEntries role plus a
153+
// JWT-subset check against the entry's claims, mirroring the matching PUT and the
154+
// default-deny visibility rule (auth.md §4).
154155
func (s *dbService) GetEntryClaims(ctx context.Context, opts ...service.Option) (map[string]any, error) {
155156
ctx, span := s.startSpan(ctx, "dbService.GetEntryClaims")
156157
defer span.End()
@@ -195,6 +196,11 @@ func (s *dbService) GetEntryClaims(ctx context.Context, opts ...service.Option)
195196
return nil, fmt.Errorf("failed to look up registry entry: %w", err)
196197
}
197198

199+
if err := s.validateClaimsSubsetBytes(ctx, options.JWTClaims, row.Claims); err != nil {
200+
otel.RecordError(span, err)
201+
return nil, err
202+
}
203+
198204
claims := db.DeserializeClaims(row.Claims)
199205
if claims == nil {
200206
claims = map[string]any{}

internal/service/options_entries.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ func (o *UpdateEntryClaimsOptions) setJWTClaims(claims map[string]any) error {
4242
type GetEntryClaimsOptions struct {
4343
EntryType string // EntryTypeServer or EntryTypeSkill
4444
Name string
45+
JWTClaims map[string]any
4546
}
4647

4748
func (o *GetEntryClaimsOptions) setEntryType(entryType string) error {
@@ -59,3 +60,9 @@ func (o *GetEntryClaimsOptions) setName(name string) error {
5960
o.Name = name
6061
return nil
6162
}
63+
64+
//nolint:unparam
65+
func (o *GetEntryClaimsOptions) setJWTClaims(claims map[string]any) error {
66+
o.JWTClaims = claims
67+
return nil
68+
}

0 commit comments

Comments
 (0)