Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 34 additions & 2 deletions federation/automatic_registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/json"
"fmt"
"net/url"
"slices"
"sync"
"time"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -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
Expand All @@ -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 != "" {
Expand Down Expand Up @@ -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
Expand Down
44 changes: 44 additions & 0 deletions federation/automatic_registration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down Expand Up @@ -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)
Expand Down
42 changes: 42 additions & 0 deletions server/automatic_registration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
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)
Expand Down Expand Up @@ -482,7 +483,7 @@
// validateDependencies purely to keep that function's own cognitive
// complexity manageable. Only ever called once cfg.Assurance is
// already known to be AssuranceProduction.
func validateProductionAssurance(cfg Config, deps Dependencies, cibaEnabled bool) error {

Check failure on line 486 in server/server.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=IDFoundry_FAPIgo&issues=AaDNSIIb1lIZgCDsK-ME&open=AaDNSIIb1lIZgCDsK-ME&pullRequest=366
if deps.Audit == nil {
return fmt.Errorf("server: dependencies: audit is required under AssuranceProduction")
}
Expand Down
Loading