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 f3638fb..66bcc0e 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 @@ -423,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 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)