Skip to content

Commit 90db2a6

Browse files
amirejazclaude
andcommitted
Validate CIMD scope, grant_types and response_types against AS policy
C3 - Thread ScopesSupported into NewCIMDStorageDecorator so CIMD scope handling is consistent with DCR. Uses registration.ValidateScopes (same function as the DCR handler) to validate declared scopes against the AS allowlist and compute the effective scope list. When ScopesSupported is unset, the document's declared scopes are used directly; omitted scopes default to DefaultScopes. C4 - Reject CIMD documents that declare grant_types or response_types the embedded AS does not support for public clients (authorization_code + refresh_token; code). Consistent with DCR which returns invalid_client_metadata for the same cases. buildFositeClient now receives pre-computed scopes from fetch() rather than re-parsing doc.Scope, matching the DCR handler pattern where scope computation and validation happen before client construction. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 8fbc935 commit 90db2a6

3 files changed

Lines changed: 204 additions & 28 deletions

File tree

pkg/authserver/server_impl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ func newServer(ctx context.Context, cfg Config, stor storage.Storage, opts ...se
177177
// so that GetClient calls for HTTPS client_id values are intercepted at the
178178
// fosite level (not just the handler level).
179179
if cfg.CIMDEnabled {
180-
stor, err = storage.NewCIMDStorageDecorator(stor, true, cfg.CIMDCacheMaxSize, cfg.CIMDCacheFallbackTTL)
180+
stor, err = storage.NewCIMDStorageDecorator(stor, true, cfg.CIMDCacheMaxSize, cfg.CIMDCacheFallbackTTL, cfg.ScopesSupported)
181181
if err != nil {
182182
return nil, fmt.Errorf("failed to initialize CIMD storage decorator: %w", err)
183183
}

pkg/authserver/storage/cimd_decorator.go

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,11 @@ import (
2828
// Only GetClient is overridden. DCR clients (opaque IDs) continue to work
2929
// exactly as before.
3030
type CIMDStorageDecorator struct {
31-
Storage // embed full interface — all methods delegate
32-
sf singleflight.Group // deduplicates concurrent fetches for the same URL
33-
cache *lru.Cache[string, *cimdCacheEntry]
34-
ttl time.Duration
31+
Storage // embed full interface — all methods delegate
32+
sf singleflight.Group // deduplicates concurrent fetches for the same URL
33+
cache *lru.Cache[string, *cimdCacheEntry]
34+
ttl time.Duration
35+
scopesSupported []string // AS-configured scopes; nil means accept any
3536
}
3637

3738
type cimdCacheEntry struct {
@@ -43,11 +44,15 @@ type cimdCacheEntry struct {
4344
// it returns base unchanged (no allocation). cacheMaxSize must be >= 1;
4445
// fallbackTTL is the fixed TTL applied to every cache entry (Cache-Control
4546
// header parsing is not yet implemented; all entries use this value).
47+
// scopesSupported is the AS-configured scope allowlist; documents that declare
48+
// scopes outside this set are rejected at fetch time. Pass nil to skip scope
49+
// validation (e.g. when ScopesSupported is unset and DefaultScopes applies).
4650
func NewCIMDStorageDecorator(
4751
base Storage,
4852
enabled bool,
4953
cacheMaxSize int,
5054
fallbackTTL time.Duration,
55+
scopesSupported []string,
5156
) (Storage, error) {
5257
if !enabled {
5358
return base, nil
@@ -63,9 +68,10 @@ func NewCIMDStorageDecorator(
6368
}
6469

6570
return &CIMDStorageDecorator{
66-
Storage: base,
67-
cache: c,
68-
ttl: fallbackTTL,
71+
Storage: base,
72+
cache: c,
73+
ttl: fallbackTTL,
74+
scopesSupported: scopesSupported,
6975
}, nil
7076
}
7177

@@ -129,7 +135,49 @@ func (d *CIMDStorageDecorator) fetch(ctx context.Context, id string) (fosite.Cli
129135
id, m, defaultCIMDTokenEndpointAuthMethod)
130136
}
131137

132-
client := buildFositeClient(doc)
138+
// Reject documents that declare grant_types the embedded AS does not support.
139+
// Consistent with DCR which restricts public clients to authorization_code + refresh_token.
140+
allowedGrantTypes := map[string]bool{"authorization_code": true, "refresh_token": true}
141+
for _, gt := range doc.GrantTypes {
142+
if !allowedGrantTypes[gt] {
143+
return nil, fmt.Errorf("%w: CIMD document at %s claims grant_type %q "+
144+
"but this server only supports %v for public clients",
145+
fosite.ErrNotFound.WithHint("unsupported grant_type"),
146+
id, gt, defaultCIMDGrantTypes)
147+
}
148+
}
149+
150+
// Reject documents that declare response_types the embedded AS does not support.
151+
allowedResponseTypes := map[string]bool{"code": true}
152+
for _, rt := range doc.ResponseTypes {
153+
if !allowedResponseTypes[rt] {
154+
return nil, fmt.Errorf("%w: CIMD document at %s claims response_type %q "+
155+
"but this server only supports %v",
156+
fosite.ErrNotFound.WithHint("unsupported response_type"),
157+
id, rt, defaultCIMDResponseTypes)
158+
}
159+
}
160+
161+
// Compute and validate the client scope list consistent with DCR.
162+
// When ScopesSupported is configured: use registration.ValidateScopes which
163+
// validates each declared scope against the allowlist and falls back to
164+
// DefaultScopes (also validated) when the document omits the field — the
165+
// same logic the DCR handler applies.
166+
// When ScopesSupported is not configured: no AS-level validation; use the
167+
// declared scopes directly, or nil to let buildFositeClient apply DefaultScopes.
168+
var resolvedScopes []string
169+
if len(d.scopesSupported) > 0 {
170+
computed, dcrErr := registration.ValidateScopes(strings.Fields(doc.Scope), d.scopesSupported)
171+
if dcrErr != nil {
172+
return nil, fmt.Errorf("%w: CIMD document at %s: %s",
173+
fosite.ErrNotFound.WithHint(string(dcrErr.Error)), id, dcrErr.ErrorDescription)
174+
}
175+
resolvedScopes = computed
176+
} else if doc.Scope != "" {
177+
resolvedScopes = strings.Fields(doc.Scope)
178+
}
179+
180+
client := buildFositeClient(doc, resolvedScopes)
133181

134182
d.cache.Add(id, &cimdCacheEntry{
135183
client: client,
@@ -157,7 +205,9 @@ const defaultCIMDTokenEndpointAuthMethod = "none"
157205
// buildFositeClient converts a ClientMetadataDocument into a fosite.Client.
158206
// Redirect URIs containing http://localhost are wrapped in a LoopbackClient
159207
// so that RFC 8252 §7.3 dynamic port matching applies.
160-
func buildFositeClient(doc *cimd.ClientMetadataDocument) fosite.Client {
208+
// resolvedScopes is the already-validated scope list computed by fetch() via
209+
// registration.ValidateScopes; when nil, DefaultScopes is used (unconstrained AS).
210+
func buildFositeClient(doc *cimd.ClientMetadataDocument, resolvedScopes []string) fosite.Client {
161211
grantTypes := doc.GrantTypes
162212
if len(grantTypes) == 0 {
163213
grantTypes = defaultCIMDGrantTypes
@@ -173,13 +223,12 @@ func buildFositeClient(doc *cimd.ClientMetadataDocument) fosite.Client {
173223
tokenEndpointAuthMethod = defaultCIMDTokenEndpointAuthMethod
174224
}
175225

176-
// When the document omits the scope field, apply the same defaults as DCR
177-
// registration so CIMD clients can request openid/profile/email/offline_access
178-
// without needing to enumerate them explicitly in the metadata document.
179-
// Clone to avoid aliasing the package-level DefaultScopes slice.
180-
scopes := slices.Clone(registration.DefaultScopes)
181-
if doc.Scope != "" {
182-
scopes = strings.Fields(doc.Scope)
226+
// Scopes were computed and validated by fetch() via registration.ValidateScopes,
227+
// consistent with the DCR handler. Fall back to DefaultScopes only when the
228+
// decorator has no ScopesSupported restriction (unconstrained AS).
229+
scopes := resolvedScopes
230+
if len(scopes) == 0 {
231+
scopes = slices.Clone(registration.DefaultScopes)
183232
}
184233

185234
defaultClient := &fosite.DefaultClient{

pkg/authserver/storage/cimd_decorator_test.go

Lines changed: 138 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"encoding/json"
99
"net/http"
1010
"net/http/httptest"
11+
"strings"
1112
"sync"
1213
"sync/atomic"
1314
"testing"
@@ -62,7 +63,7 @@ func newTestBase(t *testing.T) *MemoryStorage {
6263
// newEnabledDecorator creates a CIMDStorageDecorator wrapping base.
6364
func newEnabledDecorator(t *testing.T, base *MemoryStorage, maxSize int, ttl time.Duration) *CIMDStorageDecorator {
6465
t.Helper()
65-
got, err := NewCIMDStorageDecorator(base, true, maxSize, ttl)
66+
got, err := NewCIMDStorageDecorator(base, true, maxSize, ttl, nil)
6667
require.NoError(t, err)
6768
return got.(*CIMDStorageDecorator)
6869
}
@@ -77,29 +78,29 @@ func cimdURL(srv *httptest.Server, path string) string {
7778
func TestNewCIMDStorageDecorator_DisabledReturnsBase(t *testing.T) {
7879
t.Parallel()
7980
base := newTestBase(t)
80-
got, err := NewCIMDStorageDecorator(base, false, 10, time.Minute)
81+
got, err := NewCIMDStorageDecorator(base, false, 10, time.Minute, nil)
8182
require.NoError(t, err)
8283
assert.Same(t, base, got, "disabled decorator must return base unchanged")
8384
}
8485

8586
func TestNewCIMDStorageDecorator_ZeroCacheSizeReturnsError(t *testing.T) {
8687
t.Parallel()
8788
base := newTestBase(t)
88-
_, err := NewCIMDStorageDecorator(base, true, 0, time.Minute)
89+
_, err := NewCIMDStorageDecorator(base, true, 0, time.Minute, nil)
8990
require.Error(t, err)
9091
}
9192

9293
func TestNewCIMDStorageDecorator_NegativeCacheSizeReturnsError(t *testing.T) {
9394
t.Parallel()
9495
base := newTestBase(t)
95-
_, err := NewCIMDStorageDecorator(base, true, -1, time.Minute)
96+
_, err := NewCIMDStorageDecorator(base, true, -1, time.Minute, nil)
9697
require.Error(t, err)
9798
}
9899

99100
func TestNewCIMDStorageDecorator_EnabledReturnsCIMDDecorator(t *testing.T) {
100101
t.Parallel()
101102
base := newTestBase(t)
102-
got, err := NewCIMDStorageDecorator(base, true, 10, time.Minute)
103+
got, err := NewCIMDStorageDecorator(base, true, 10, time.Minute, nil)
103104
require.NoError(t, err)
104105
require.NotNil(t, got)
105106
_, isCIMD := got.(*CIMDStorageDecorator)
@@ -336,7 +337,7 @@ func TestBuildFositeClient_Defaults(t *testing.T) {
336337
RedirectURIs: []string{"https://example.com/callback"},
337338
}
338339

339-
got := buildFositeClient(doc)
340+
got := buildFositeClient(doc, nil)
340341
assert.Equal(t, "https://example.com/meta.json", got.GetID())
341342
assert.True(t, got.IsPublic())
342343
assert.ElementsMatch(t, []string{"authorization_code", "refresh_token"}, []string(got.GetGrantTypes()))
@@ -355,7 +356,7 @@ func TestBuildFositeClient_ExplicitGrantTypes(t *testing.T) {
355356
GrantTypes: []string{"authorization_code"},
356357
}
357358

358-
got := buildFositeClient(doc)
359+
got := buildFositeClient(doc, nil)
359360
assert.ElementsMatch(t, []string{"authorization_code"}, []string(got.GetGrantTypes()))
360361
}
361362

@@ -368,7 +369,9 @@ func TestBuildFositeClient_ScopeParsing(t *testing.T) {
368369
Scope: "openid profile email",
369370
}
370371

371-
got := buildFositeClient(doc)
372+
// Scope parsing is now done by fetch() before calling buildFositeClient.
373+
resolvedScopes := strings.Fields(doc.Scope)
374+
got := buildFositeClient(doc, resolvedScopes)
372375
assert.ElementsMatch(t, []string{"openid", "profile", "email"}, []string(got.GetScopes()))
373376
}
374377

@@ -380,7 +383,7 @@ func TestBuildFositeClient_LoopbackRedirectWrapsInLoopbackClient(t *testing.T) {
380383
RedirectURIs: []string{"http://localhost/callback"},
381384
}
382385

383-
got := buildFositeClient(doc)
386+
got := buildFositeClient(doc, nil)
384387
// LoopbackClient adds MatchRedirectURI — check the distinctive method is present.
385388
type loopbackMatcher interface {
386389
MatchRedirectURI(string) bool
@@ -403,7 +406,7 @@ func TestBuildFositeClient_NonLoopbackRedirectReturnsOpenIDConnectClient(t *test
403406
RedirectURIs: []string{"https://example.com/callback"},
404407
}
405408

406-
got := buildFositeClient(doc)
409+
got := buildFositeClient(doc, nil)
407410
_, ok := got.(*fosite.DefaultOpenIDConnectClient)
408411
assert.True(t, ok, "non-loopback redirect URI must produce a DefaultOpenIDConnectClient")
409412
}
@@ -416,7 +419,7 @@ func TestBuildFositeClient_TokenEndpointAuthMethodDefault(t *testing.T) {
416419
RedirectURIs: []string{"https://example.com/callback"},
417420
}
418421

419-
got := buildFositeClient(doc)
422+
got := buildFositeClient(doc, nil)
420423
if oidc, ok := got.(fosite.OpenIDConnectClient); ok {
421424
assert.Equal(t, "none", oidc.GetTokenEndpointAuthMethod())
422425
}
@@ -442,3 +445,127 @@ func TestFetch_RejectsUnsupportedTokenEndpointAuthMethod(t *testing.T) {
442445
_, err := dec.fetchOrCached(context.Background(), srv.URL+"/meta.json")
443446
require.Error(t, err, "fetch must fail when token_endpoint_auth_method is not \"none\"")
444447
}
448+
449+
// --- C4: grant_types / response_types validation ---
450+
451+
func TestFetch_RejectsUnsupportedGrantType(t *testing.T) {
452+
t.Parallel()
453+
for _, unsupported := range []string{"client_credentials", "implicit", "urn:ietf:params:oauth:grant-type:device_code"} {
454+
t.Run(unsupported, func(t *testing.T) {
455+
t.Parallel()
456+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
457+
clientID := "http://" + r.Host + r.URL.Path
458+
doc := cimd.ClientMetadataDocument{
459+
ClientID: clientID,
460+
RedirectURIs: []string{"https://example.com/callback"},
461+
GrantTypes: []string{unsupported},
462+
}
463+
w.Header().Set("Content-Type", "application/json")
464+
_ = json.NewEncoder(w).Encode(doc)
465+
}))
466+
t.Cleanup(srv.Close)
467+
dec := newEnabledDecorator(t, newTestBase(t), 10, time.Minute)
468+
_, err := dec.fetchOrCached(context.Background(), srv.URL+"/meta.json")
469+
require.Error(t, err, "unsupported grant_type %q must be rejected", unsupported)
470+
})
471+
}
472+
}
473+
474+
func TestFetch_AcceptsSupportedGrantTypes(t *testing.T) {
475+
t.Parallel()
476+
srv := serveCIMDDoc(t, "/meta.json", nil)
477+
dec := newEnabledDecorator(t, newTestBase(t), 10, time.Minute)
478+
// Default grant_types (omitted in document) must succeed
479+
_, err := dec.fetchOrCached(context.Background(), cimdURL(srv, "/meta.json"))
480+
require.NoError(t, err)
481+
}
482+
483+
func TestFetch_RejectsUnsupportedResponseType(t *testing.T) {
484+
t.Parallel()
485+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
486+
clientID := "http://" + r.Host + r.URL.Path
487+
doc := cimd.ClientMetadataDocument{
488+
ClientID: clientID,
489+
RedirectURIs: []string{"https://example.com/callback"},
490+
ResponseTypes: []string{"token"},
491+
}
492+
w.Header().Set("Content-Type", "application/json")
493+
_ = json.NewEncoder(w).Encode(doc)
494+
}))
495+
t.Cleanup(srv.Close)
496+
dec := newEnabledDecorator(t, newTestBase(t), 10, time.Minute)
497+
_, err := dec.fetchOrCached(context.Background(), srv.URL+"/meta.json")
498+
require.Error(t, err, "unsupported response_type \"token\" must be rejected")
499+
}
500+
501+
// --- C3: scope validation against ScopesSupported ---
502+
503+
func TestBuildFositeClient_ScopeDefaultsToScopesSupported(t *testing.T) {
504+
t.Parallel()
505+
doc := &cimd.ClientMetadataDocument{
506+
ClientID: "https://example.com/meta.json",
507+
RedirectURIs: []string{"https://example.com/callback"},
508+
// Scope deliberately omitted
509+
}
510+
scopesSupported := []string{"openid", "profile"}
511+
got := buildFositeClient(doc, scopesSupported)
512+
assert.ElementsMatch(t, scopesSupported, []string(got.GetScopes()),
513+
"omitted scope should default to ScopesSupported, not DefaultScopes")
514+
}
515+
516+
func TestBuildFositeClient_ScopeDefaultsToDefaultScopesWhenNoScopesSupported(t *testing.T) {
517+
t.Parallel()
518+
doc := &cimd.ClientMetadataDocument{
519+
ClientID: "https://example.com/meta.json",
520+
RedirectURIs: []string{"https://example.com/callback"},
521+
}
522+
got := buildFositeClient(doc, nil)
523+
assert.ElementsMatch(t, registration.DefaultScopes, []string(got.GetScopes()),
524+
"omitted scope with no ScopesSupported should default to registration.DefaultScopes")
525+
}
526+
527+
func TestFetch_RejectsScopeOutsideScopesSupported(t *testing.T) {
528+
t.Parallel()
529+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
530+
clientID := "http://" + r.Host + r.URL.Path
531+
doc := cimd.ClientMetadataDocument{
532+
ClientID: clientID,
533+
RedirectURIs: []string{"https://example.com/callback"},
534+
Scope: "openid profile email",
535+
}
536+
w.Header().Set("Content-Type", "application/json")
537+
_ = json.NewEncoder(w).Encode(doc)
538+
}))
539+
t.Cleanup(srv.Close)
540+
541+
// Decorator configured with scopesSupported=["openid"] only
542+
got, err := NewCIMDStorageDecorator(newTestBase(t), true, 10, time.Minute, []string{"openid"})
543+
require.NoError(t, err)
544+
dec := got.(*CIMDStorageDecorator)
545+
546+
_, err = dec.fetchOrCached(context.Background(), srv.URL+"/meta.json")
547+
require.Error(t, err, "scope outside ScopesSupported must be rejected")
548+
}
549+
550+
func TestFetch_AcceptsScopeWithinScopesSupported(t *testing.T) {
551+
t.Parallel()
552+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
553+
clientID := "http://" + r.Host + r.URL.Path
554+
doc := cimd.ClientMetadataDocument{
555+
ClientID: clientID,
556+
RedirectURIs: []string{"https://example.com/callback"},
557+
Scope: "openid",
558+
}
559+
w.Header().Set("Content-Type", "application/json")
560+
_ = json.NewEncoder(w).Encode(doc)
561+
}))
562+
t.Cleanup(srv.Close)
563+
564+
got, err := NewCIMDStorageDecorator(newTestBase(t), true, 10, time.Minute, []string{"openid", "profile"})
565+
require.NoError(t, err)
566+
dec := got.(*CIMDStorageDecorator)
567+
568+
client, err := dec.fetchOrCached(context.Background(), srv.URL+"/meta.json")
569+
require.NoError(t, err)
570+
assert.ElementsMatch(t, []string{"openid"}, []string(client.GetScopes()))
571+
}

0 commit comments

Comments
 (0)