From 33da13675f40396773dddf9f86e94d6c76558bce Mon Sep 17 00:00:00 2001 From: Oscar Sanderson Date: Wed, 23 Sep 2026 15:58:28 +0800 Subject: [PATCH 1/2] feat: add AllowedClientAuthMethods to federation automatic registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An automatically-registered RP previously chose any supported client authentication method via its own token_endpoint_auth_method metadata, with no operator-side restriction short of federation metadata policy. AllowedClientAuthMethods (on both federation.AutomaticRegistrationConfig and server.AutomaticRegistrationConfig) lets an operator hold federation RPs to a narrower set than its statically registered clients — e.g. private_key_jwt only while static clients use mTLS. An RP declaring an unlisted method fails to resolve, exactly like one with invalid metadata. Empty (the default) permits every method, so existing configurations are unaffected. Invalid entries are rejected at construction. Co-Authored-By: Claude Opus 5.5 --- federation/automatic_registration.go | 36 +++++++++++++++++-- federation/automatic_registration_test.go | 44 +++++++++++++++++++++++ server/automatic_registration_test.go | 28 +++++++++++++++ server/config.go | 8 +++++ server/server.go | 1 + 5 files changed, 115 insertions(+), 2 deletions(-) diff --git a/federation/automatic_registration.go b/federation/automatic_registration.go index eabc237..462fbf2 100644 --- a/federation/automatic_registration.go +++ b/federation/automatic_registration.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "net/url" + "slices" "sync" "time" @@ -77,6 +78,20 @@ type AutomaticRegistrationConfig struct { // been granted here. When false, that metadata is ignored entirely // and CIBA stays disabled for every automatically-registered client. AllowsCIBA bool + + // AllowedClientAuthMethods restricts which token_endpoint_auth_method + // an automatically-registered client may declare in its own + // metadata. An RP whose metadata names a method outside this list + // fails to resolve at all, exactly like one with malformed metadata. + // Empty (the default) permits every storage.ClientAuthMethod this + // package can read from metadata — each is a FAPI 2.0-permitted, + // sender-proving method, so the RP choosing one is a mechanism + // detail, not a capability grant like AllowsCIBA. Set it when an + // operator wants federation RPs held to a narrower set than its + // statically registered clients (e.g. only + // storage.ClientAuthMethodPrivateKeyJWT, even though some static + // clients use mTLS). Every entry must be a valid ClientAuthMethod. + AllowedClientAuthMethods []storage.ClientAuthMethod } // cachedClient is one Relying Party's resolved registration, cached @@ -121,7 +136,8 @@ type cachedClient struct { // ClientAuthMethodTLSClientAuth and its four SAN-typed siblings read // their own plain-string metadata parameter directly (RFC 8705 §2.1.2: // tls_client_auth_subject_dn/tls_client_auth_san_dns/_san_uri/_san_ip/ -// _san_email) — see registeredClientConfigFromMetadata. PAR/JAR-level +// _san_email) — see registeredClientConfigFromMetadata — unless +// AutomaticRegistrationConfig.AllowedClientAuthMethods narrows that set. PAR/JAR-level // enforcement of OpenID Federation 1.0 §12.1.1's own // aud/sub/jti Request Object rules is a request-handling concern, not a // client registration one — see storage.RegisteredClientConfig's own @@ -164,6 +180,11 @@ func NewAutomaticClientRepository(underlying storage.ClientRepository, resolver if cfg.MaxCacheAge <= 0 { return nil, fmt.Errorf("federation: config: max_cache_age must be positive") } + for _, method := range cfg.AllowedClientAuthMethods { + if !method.IsValid() { + return nil, fmt.Errorf("federation: config: allowed_client_auth_methods: invalid client auth method %v", method) + } + } if clock == nil { return nil, fmt.Errorf("federation: clock is required") } @@ -410,7 +431,8 @@ type relyingPartyMetadata struct { // // a.cfg supplies the operator-level capability grants (AllowedScopes, // AllowsClientCredentialsGrant, AllowsCIBA) that must never be inferred -// from raw itself — see AutomaticRegistrationConfig's own doc comments +// from raw itself, plus the AllowedClientAuthMethods restriction raw's +// own token_endpoint_auth_method is checked against — see AutomaticRegistrationConfig's own doc comments // for why. func (a *AutomaticClientRepository) registeredClientConfigFromMetadata(ctx context.Context, id fapi.ClientID, raw json.RawMessage) (storage.RegisteredClientConfig, json.RawMessage, error) { var m relyingPartyMetadata @@ -428,6 +450,9 @@ func (a *AutomaticClientRepository) registeredClientConfigFromMetadata(ctx conte if err != nil { return storage.RegisteredClientConfig{}, nil, fmt.Errorf("token_endpoint_auth_method: %w", err) } + if !a.allowsClientAuthMethod(authMethod) { + return storage.RegisteredClientConfig{}, nil, fmt.Errorf("token_endpoint_auth_method %q is not permitted by allowed_client_auth_methods", authMethod) + } jwks := m.JWKS if m.JWKSURI != "" { @@ -549,6 +574,13 @@ func applyClientAuthMethodFields(cfg *storage.RegisteredClientConfig, authMethod return nil } +// allowsClientAuthMethod reports whether method is permitted by +// AutomaticRegistrationConfig.AllowedClientAuthMethods — every method +// when that list is empty. +func (a *AutomaticClientRepository) allowsClientAuthMethod(method storage.ClientAuthMethod) bool { + return len(a.cfg.AllowedClientAuthMethods) == 0 || slices.Contains(a.cfg.AllowedClientAuthMethods, method) +} + // applyBackchannelAuthenticationFields sets cfg's CIBA fields from m // when this repository allows CIBA and m actually advertises it — a // no-op otherwise. Split out of registeredClientConfigFromMetadata for diff --git a/federation/automatic_registration_test.go b/federation/automatic_registration_test.go index ffb9efd..4d7c85a 100644 --- a/federation/automatic_registration_test.go +++ b/federation/automatic_registration_test.go @@ -275,6 +275,11 @@ func TestNewAutomaticClientRepositoryRejectsInvalidConfig(t *testing.T) { cfg.MaxCacheAge = 0 return alwaysFailsRepository{}, resolver, f.fetcher, cfg, fixedClock{now: f.now} }, + "invalid allowed client auth method": func() (storage.ClientRepository, *federation.Resolver, *fapihttp.Client, federation.AutomaticRegistrationConfig, federation.Clock) { + cfg := validAutomaticRegistrationConfig() + cfg.AllowedClientAuthMethods = []storage.ClientAuthMethod{storage.ClientAuthMethod(255)} + return alwaysFailsRepository{}, resolver, f.fetcher, cfg, fixedClock{now: f.now} + }, "nil clock": func() (storage.ClientRepository, *federation.Resolver, *fapihttp.Client, federation.AutomaticRegistrationConfig, federation.Clock) { return alwaysFailsRepository{}, resolver, f.fetcher, validAutomaticRegistrationConfig(), nil }, @@ -427,6 +432,45 @@ func TestAutomaticClientRepositoryResolveClientAllowsCIBAWhenConfigured(t *testi } } +func TestAutomaticClientRepositoryResolveClientAllowedClientAuthMethods(t *testing.T) { + // rpMetadataBuilder's RP declares private_key_jwt. + cases := map[string]struct { + allowed []storage.ClientAuthMethod + wantErr bool + }{ + "empty permits every method": {allowed: nil}, + "listed method permitted": {allowed: []storage.ClientAuthMethod{ + storage.ClientAuthMethodSelfSignedTLSClientAuth, storage.ClientAuthMethodPrivateKeyJWT, + }}, + "unlisted method rejected": {allowed: []storage.ClientAuthMethod{storage.ClientAuthMethodTLSClientAuth}, wantErr: true}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + f := setupAutomaticRegistrationFixture(t, rpMetadataBuilder(t)) + cfg := validAutomaticRegistrationConfig() + cfg.AllowedClientAuthMethods = tc.allowed + repo, err := federation.NewAutomaticClientRepository(alwaysFailsRepository{}, f.newResolver(t), f.fetcher, cfg, fixedClock{now: f.now}) + if err != nil { + t.Fatalf("NewAutomaticClientRepository: %v", err) + } + + got, err := repo.ResolveClient(context.Background(), fapi.ClientID(f.rpID)) + if tc.wantErr { + if err == nil { + t.Fatalf("ResolveClient = nil error, want error (private_key_jwt not in AllowedClientAuthMethods)") + } + return + } + if err != nil { + t.Fatalf("ResolveClient: %v", err) + } + if got.ClientAuthMethod() != storage.ClientAuthMethodPrivateKeyJWT { + t.Errorf("ClientAuthMethod() = %v, want ClientAuthMethodPrivateKeyJWT", got.ClientAuthMethod()) + } + }) + } +} + func TestAutomaticClientRepositoryResolveClientRejectsNonEntityID(t *testing.T) { f := setupAutomaticRegistrationFixture(t, nil) resolver := f.newResolver(t) diff --git a/server/automatic_registration_test.go b/server/automatic_registration_test.go index 58968f0..13d4e84 100644 --- a/server/automatic_registration_test.go +++ b/server/automatic_registration_test.go @@ -307,6 +307,34 @@ func TestNewWiresAutomaticRegistrationIntoPushAuthorizationRequest(t *testing.T) } } +// TestNewWiresAutomaticRegistrationAllowedClientAuthMethods proves +// server.New passes AutomaticRegistrationConfig.AllowedClientAuthMethods +// through to the federation repository: the fixture RP declares +// private_key_jwt, so an allowlist excluding it makes the RP +// unresolvable and its PAR request fails client authentication. +func TestNewWiresAutomaticRegistrationAllowedClientAuthMethods(t *testing.T) { + f := setupAutomaticRegistrationFixture(t, testRedirectURI) + srv := newAutomaticRegistrationTestServer(t, f, func(cfg *server.Config, deps *server.Dependencies) { + cfg.AutomaticRegistration.AllowedClientAuthMethods = []storage.ClientAuthMethod{storage.ClientAuthMethodTLSClientAuth} + }) + + assertion, err := clientassertion.CreateAssertion(clientassertion.AssertionRequest{ + Signer: f.rpOIDCKey, Algorithm: fapi.ES256, KeyID: "rp-oidc", + ClientID: f.rpID, Audience: testIssuer, + Now: f.now, Lifetime: 30 * time.Second, + }) + if err != nil { + t.Fatalf("CreateAssertion: %v", err) + } + + _, err = srv.PushAuthorizationRequest(context.Background(), server.PushAuthorizationRequest{ + HTTP: server.FormRequest{Parameters: plainFormParameters(t, assertion, nil)}, + }) + if code := serverErrorCode(t, err); code != server.ErrorInvalidClient { + t.Fatalf("error code = %q, want %q", code, server.ErrorInvalidClient) + } +} + // TestNewWiresAutomaticRegistrationRequestObjectFederationRules proves // server.New applies OpenID Federation 1.0 §12.1.1's stricter Request // Object rules (via diff --git a/server/config.go b/server/config.go index 15844a4..8c97fd7 100644 --- a/server/config.go +++ b/server/config.go @@ -6,6 +6,7 @@ import ( fapi "github.com/idfoundry/fapigo" "github.com/idfoundry/fapigo/extension" "github.com/idfoundry/fapigo/federation" + "github.com/idfoundry/fapigo/storage" ) // Profile selects which FAPI 2.0 security profile this server enforces. @@ -498,6 +499,13 @@ type AutomaticRegistrationConfig struct { // why this is a config-level switch, never inferred from an RP's own // metadata. AllowsCIBA bool + + // AllowedClientAuthMethods restricts which token_endpoint_auth_method + // an automatically-registered client may declare — see + // federation.AutomaticRegistrationConfig.AllowedClientAuthMethods. + // Empty (the default) permits every method. Statically registered + // clients are unaffected. + AllowedClientAuthMethods []storage.ClientAuthMethod } // FederationConfig configures this server's OpenID Federation 1.0 diff --git a/server/server.go b/server/server.go index 0005182..ba62a0b 100644 --- a/server/server.go +++ b/server/server.go @@ -60,6 +60,7 @@ func New(cfg Config, deps Dependencies) (*Server, error) { MaxCacheAge: cfg.AutomaticRegistration.MaxCacheAge, AllowsClientCredentialsGrant: cfg.AutomaticRegistration.AllowsClientCredentialsGrant, AllowsCIBA: cfg.AutomaticRegistration.AllowsCIBA, + AllowedClientAuthMethods: cfg.AutomaticRegistration.AllowedClientAuthMethods, }, deps.Clock) if err != nil { return nil, fmt.Errorf("server: config: automatic_registration: %w", err) From 83229a5a0e1a01a29d6756016d5b50f2816727e7 Mon Sep 17 00:00:00 2001 From: Oscar Sanderson Date: Wed, 23 Sep 2026 16:02:45 +0800 Subject: [PATCH 2/2] test: cover AllowedClientAuthMethods on the client_credentials grant Co-Authored-By: Claude Opus 5.5 --- server/automatic_registration_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/server/automatic_registration_test.go b/server/automatic_registration_test.go index 13d4e84..a20e2f5 100644 --- a/server/automatic_registration_test.go +++ b/server/automatic_registration_test.go @@ -451,6 +451,20 @@ func TestNewWiresAutomaticRegistrationIntoClientCredentialsGrant(t *testing.T) { t.Fatalf("RequestClientCredentialsToken (both switches set) = %v, want nil error", err) } }) + + t.Run("rejected when the RP's auth method is not in AllowedClientAuthMethods", func(t *testing.T) { + srv := newAutomaticRegistrationTestServer(t, f, func(cfg *server.Config, deps *server.Dependencies) { + cfg.ClientCredentialsGrant = true + cfg.AutomaticRegistration.AllowsClientCredentialsGrant = true + cfg.AutomaticRegistration.AllowedClientAuthMethods = []storage.ClientAuthMethod{storage.ClientAuthMethodTLSClientAuth} + }) + _, err := srv.RequestClientCredentialsToken(context.Background(), server.ClientCredentialsTokenRequest{ + HTTP: server.FormRequest{Parameters: params}, DPoPProofs: dpopProofs, + }) + if code := serverErrorCode(t, err); code != server.ErrorInvalidClient { + t.Fatalf("error code = %q, want %q", code, server.ErrorInvalidClient) + } + }) } // TestNewWiresAutomaticRegistrationIntoBeginBackchannelAuthentication