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>

Author: amirejaz

pkg/authserver/server/handlers/dcr.go

@@ -77,7 +77,7 @@ func (h *Handler) RegisterClientHandler(w http.ResponseWriter, req *http.Request
 	// offline_access) — every DCR-registered client gains the ability to request
 	// these scopes at /oauth/authorize regardless of what they registered with.
 	if len(h.config.BaselineClientScopes) > 0 {
-		effective := unionScopes(scopes, h.config.BaselineClientScopes)
+		effective := registration.UnionScopes(scopes, h.config.BaselineClientScopes)
 		if !slices.Equal(effective, scopes) {
 			// Baseline-driven expansion is the intended behavior whenever
 			// baseline_client_scopes is configured, so per-registration

pkg/authserver/server/handlers/scopes.go

@@ -2,37 +2,3 @@
 // SPDX-License-Identifier: Apache-2.0
 
 package handlers
-
-// unionScopes returns the union of requested and baseline scopes, preserving
-// the order of requested first, then appending any baseline scopes not already
-// present. Duplicates are removed. Returns nil when the result is empty.
-//
-// This is used by the DCR registration handler to inject an
-// operator-configured scope baseline into the registered client's scope set:
-// if a client narrows the scope field at /oauth/register, the baseline scopes
-// are still part of the client's registered set so that the client can
-// request them later at /oauth/authorize without invalid_scope rejection.
-//
-// Both inputs must already be validated by the caller (e.g. via ValidateScopes
-// for client-supplied scopes). unionScopes does not filter empty strings or
-// validate scope syntax — it only deduplicates and merges in stable order.
-func unionScopes(requested, baseline []string) []string {
-	seen := make(map[string]bool, len(requested)+len(baseline))
-	out := make([]string, 0, len(requested)+len(baseline))
-	for _, s := range requested {
-		if !seen[s] {
-			seen[s] = true
-			out = append(out, s)
-		}
-	}
-	for _, s := range baseline {
-		if !seen[s] {
-			seen[s] = true
-			out = append(out, s)
-		}
-	}
-	if len(out) == 0 {
-		return nil
-	}
-	return out
-}

pkg/authserver/server/handlers/scopes_test.go

@@ -7,6 +7,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	"github.com/stacklok/toolhive/pkg/authserver/server/registration"
 )
 
 func TestUnionScopes(t *testing.T) {
@@ -108,7 +110,7 @@ func TestUnionScopes(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			got := unionScopes(tt.req, tt.baseline)
+			got := registration.UnionScopes(tt.req, tt.baseline)
 			assert.Equal(t, tt.want, got)
 		})
 	}

pkg/authserver/server/registration/dcr.go

@@ -327,3 +327,31 @@ func ValidateScopes(requestedScopes, allowedScopes []string) ([]string, *DCRErro
 
 	return scopes, nil
 }
+
+// UnionScopes returns the union of requested and baseline scopes, preserving
+// the order of requested first, then appending any baseline scopes not already
+// present. Duplicates are removed. Returns nil when the result is empty.
+//
+// Both inputs must already be validated by the caller. UnionScopes does not
+// filter empty strings or validate scope syntax — it only deduplicates and
+// merges in stable order.
+func UnionScopes(requested, baseline []string) []string {
+	seen := make(map[string]bool, len(requested)+len(baseline))
+	out := make([]string, 0, len(requested)+len(baseline))
+	for _, s := range requested {
+		if !seen[s] {
+			seen[s] = true
+			out = append(out, s)
+		}
+	}
+	for _, s := range baseline {
+		if !seen[s] {
+			seen[s] = true
+			out = append(out, s)
+		}
+	}
+	if len(out) == 0 {
+		return nil
+	}
+	return out
+}

pkg/authserver/server_impl.go

@@ -177,7 +177,14 @@ func newServer(ctx context.Context, cfg Config, stor storage.Storage, opts ...se
 	// so that GetClient calls for HTTPS client_id values are intercepted at the
 	// fosite level (not just the handler level).
 	if cfg.CIMDEnabled {
-		stor, err = storage.NewCIMDStorageDecorator(stor, true, cfg.CIMDCacheMaxSize, cfg.CIMDCacheFallbackTTL)
+		if len(cfg.BaselineClientScopes) > 0 {
+			slog.Warn("CIMD is enabled with baseline_client_scopes configured; "+
+				"all dynamically resolved CIMD clients will receive the baseline scopes",
+				"baseline_client_scopes", cfg.BaselineClientScopes)
+		}
+		stor, err = storage.NewCIMDStorageDecorator(
+			stor, true, cfg.CIMDCacheMaxSize, cfg.CIMDCacheFallbackTTL,
+			cfg.ScopesSupported, cfg.BaselineClientScopes)
 		if err != nil {
 			return nil, fmt.Errorf("failed to initialize CIMD storage decorator: %w", err)
 		}

pkg/authserver/storage/cimd_decorator.go

@@ -28,10 +28,12 @@ import (
 // Only GetClient is overridden. DCR clients (opaque IDs) continue to work
 // exactly as before.
 type CIMDStorageDecorator struct {
-	Storage                    // embed full interface — all methods delegate
-	sf      singleflight.Group // deduplicates concurrent fetches for the same URL
-	cache   *lru.Cache[string, *cimdCacheEntry]
-	ttl     time.Duration
+	Storage                                 // embed full interface — all methods delegate
+	sf                   singleflight.Group // deduplicates concurrent fetches for the same URL
+	cache                *lru.Cache[string, *cimdCacheEntry]
+	ttl                  time.Duration
+	scopesSupported      []string // AS-configured scopes; nil means accept any
+	baselineClientScopes []string // unioned into every client's scope set, same as DCR
 }
 
 type cimdCacheEntry struct {
@@ -43,11 +45,20 @@ type cimdCacheEntry struct {
 // it returns base unchanged (no allocation). cacheMaxSize must be >= 1;
 // fallbackTTL is the fixed TTL applied to every cache entry (Cache-Control
 // header parsing is not yet implemented; all entries use this value).
+// scopesSupported is the AS-configured scope allowlist; documents that declare
+// scopes outside this set are rejected at fetch time. In production this is
+// always non-nil because applyDefaults populates ScopesSupported before the
+// decorator is constructed. Pass nil only in tests that need unconstrained scope
+// passthrough.
+// baselineClientScopes mirrors the AS-level baseline: it is unioned into every
+// CIMD client's scope set after validation, matching DCR handler behaviour.
 func NewCIMDStorageDecorator(
 	base Storage,
 	enabled bool,
 	cacheMaxSize int,
 	fallbackTTL time.Duration,
+	scopesSupported []string,
+	baselineClientScopes []string,
 ) (Storage, error) {
 	if !enabled {
 		return base, nil
@@ -63,9 +74,11 @@ func NewCIMDStorageDecorator(
 	}
 
 	return &CIMDStorageDecorator{
-		Storage: base,
-		cache:   c,
-		ttl:     fallbackTTL,
+		Storage:              base,
+		cache:                c,
+		ttl:                  fallbackTTL,
+		scopesSupported:      scopesSupported,
+		baselineClientScopes: baselineClientScopes,
 	}, nil
 }
 
@@ -119,17 +132,75 @@ func (d *CIMDStorageDecorator) fetch(ctx context.Context, id string) (fosite.Cli
 	}
 
 	// Reject documents that declare an auth method this AS does not support.
-	// The embedded AS only advertises "none"; accepting a doc that says
-	// "private_key_jwt" and then silently treating the client as public would
-	// mislead operators and break clients that actually try to use JWT assertions.
+	// ErrInvalidClient: the document was fetched successfully but its declared
+	// metadata violates AS policy (distinct from ErrNotFound which means the
+	// document could not be fetched at all).
 	if m := doc.TokenEndpointAuthMethod; m != "" && m != defaultCIMDTokenEndpointAuthMethod {
 		return nil, fmt.Errorf("%w: CIMD document at %s claims token_endpoint_auth_method %q "+
 			"but this server only supports %q",
-			fosite.ErrNotFound.WithHint("unsupported token_endpoint_auth_method"),
+			fosite.ErrInvalidClient.WithHint("unsupported token_endpoint_auth_method"),
 			id, m, defaultCIMDTokenEndpointAuthMethod)
 	}
 
-	client := buildFositeClient(doc)
+	// Reject documents that declare grant_types the embedded AS does not support.
+	// Mirrors DCR's validateGrantTypes which restricts public clients to
+	// authorization_code + refresh_token and requires authorization_code to be present.
+	for _, gt := range doc.GrantTypes {
+		if !allowedCIMDGrantTypes[gt] {
+			return nil, fmt.Errorf("%w: CIMD document at %s claims grant_type %q "+
+				"but this server only supports %v for public clients",
+				fosite.ErrInvalidClient.WithHint("unsupported grant_type"),
+				id, gt, defaultCIMDGrantTypes)
+		}
+	}
+	if len(doc.GrantTypes) > 0 && !slices.Contains(doc.GrantTypes, "authorization_code") {
+		return nil, fmt.Errorf("%w: CIMD document at %s grant_types must include %q",
+			fosite.ErrInvalidClient.WithHint("grant_types must include authorization_code"),
+			id, "authorization_code")
+	}
+
+	// Reject documents that declare response_types the embedded AS does not support.
+	for _, rt := range doc.ResponseTypes {
+		if !allowedCIMDResponseTypes[rt] {
+			return nil, fmt.Errorf("%w: CIMD document at %s claims response_type %q "+
+				"but this server only supports %v",
+				fosite.ErrInvalidClient.WithHint("unsupported response_type"),
+				id, rt, defaultCIMDResponseTypes)
+		}
+	}
+
+	// Compute and validate the client scope list consistent with DCR.
+	// When ScopesSupported is configured:
+	//   - Declared scopes are validated via registration.ValidateScopes (same
+	//     function as the DCR handler).
+	//   - When the document omits scope, the client receives ScopesSupported
+	//     rather than DefaultScopes — a CIMD document that doesn't declare scope
+	//     means "whatever the AS supports", not "give me the full default set"
+	//     (which may exceed ScopesSupported).
+	// When ScopesSupported is not configured: no AS-level validation; declared
+	// scopes are used directly, or nil to let buildFositeClient apply DefaultScopes.
+	// In both cases BaselineClientScopes is unioned in after validation,
+	// matching the DCR handler's behaviour.
+	var resolvedScopes []string
+	if len(d.scopesSupported) > 0 {
+		if doc.Scope != "" {
+			computed, dcrErr := registration.ValidateScopes(strings.Fields(doc.Scope), d.scopesSupported)
+			if dcrErr != nil {
+				return nil, fmt.Errorf("%w: CIMD document at %s: %s",
+					fosite.ErrInvalidClient.WithHint(dcrErr.Error), id, dcrErr.ErrorDescription)
+			}
+			resolvedScopes = computed
+		} else {
+			resolvedScopes = slices.Clone(d.scopesSupported)
+		}
+	} else if doc.Scope != "" {
+		resolvedScopes = strings.Fields(doc.Scope)
+	}
+	if len(d.baselineClientScopes) > 0 {
+		resolvedScopes = registration.UnionScopes(resolvedScopes, d.baselineClientScopes)
+	}
+
+	client := buildFositeClient(doc, resolvedScopes)
 
 	d.cache.Add(id, &cimdCacheEntry{
 		client:  client,
@@ -144,10 +215,19 @@ func (d *CIMDStorageDecorator) fetch(ctx context.Context, id string) (fosite.Cli
 // that use the authorization code flow with refresh token rotation.
 var defaultCIMDGrantTypes = []string{"authorization_code", "refresh_token"}
 
+// allowedCIMDGrantTypes is the set of grant_type values a CIMD document may
+// declare. Values outside this set are rejected at fetch time, consistent with
+// DCR which restricts public clients to authorization_code + refresh_token.
+var allowedCIMDGrantTypes = map[string]bool{"authorization_code": true, "refresh_token": true}
+
 // defaultCIMDResponseTypes are the OAuth 2.0 response types applied when the
 // CIMD document omits response_types.
 var defaultCIMDResponseTypes = []string{"code"}
 
+// allowedCIMDResponseTypes is the set of response_type values a CIMD document
+// may declare. Values outside this set are rejected at fetch time.
+var allowedCIMDResponseTypes = map[string]bool{"code": true}
+
 // defaultCIMDTokenEndpointAuthMethod is the token endpoint authentication
 // method applied when the CIMD document omits token_endpoint_auth_method.
 // Documents that declare any other value are rejected by fetch() before
@@ -157,7 +237,9 @@ const defaultCIMDTokenEndpointAuthMethod = "none"
 // buildFositeClient converts a ClientMetadataDocument into a fosite.Client.
 // Redirect URIs containing http://localhost are wrapped in a LoopbackClient
 // so that RFC 8252 §7.3 dynamic port matching applies.
-func buildFositeClient(doc *cimd.ClientMetadataDocument) fosite.Client {
+// resolvedScopes is the already-validated scope list computed by fetch() via
+// registration.ValidateScopes; when nil, DefaultScopes is used (unconstrained AS).
+func buildFositeClient(doc *cimd.ClientMetadataDocument, resolvedScopes []string) fosite.Client {
 	grantTypes := doc.GrantTypes
 	if len(grantTypes) == 0 {
 		grantTypes = defaultCIMDGrantTypes
@@ -173,13 +255,12 @@ func buildFositeClient(doc *cimd.ClientMetadataDocument) fosite.Client {
 		tokenEndpointAuthMethod = defaultCIMDTokenEndpointAuthMethod
 	}
 
-	// When the document omits the scope field, apply the same defaults as DCR
-	// registration so CIMD clients can request openid/profile/email/offline_access
-	// without needing to enumerate them explicitly in the metadata document.
-	// Clone to avoid aliasing the package-level DefaultScopes slice.
-	scopes := slices.Clone(registration.DefaultScopes)
-	if doc.Scope != "" {
-		scopes = strings.Fields(doc.Scope)
+	// Scopes were computed and validated by fetch() via registration.ValidateScopes,
+	// consistent with the DCR handler. Fall back to DefaultScopes only when the
+	// decorator has no ScopesSupported restriction (unconstrained AS).
+	scopes := resolvedScopes
+	if len(scopes) == 0 {
+		scopes = slices.Clone(registration.DefaultScopes)
 	}
 
 	defaultClient := &fosite.DefaultClient{