feat: add OpenID Federation entity configuration endpoint - #514
Conversation
Implement /.well-known/openid-federation for both APIGW (issuer) and
verifier services per OpenID Federation 1.0 §5.2.
The endpoint serves a self-signed JWT containing:
- Entity metadata (openid_credential_issuer, openid_relying_party, etc.)
- JWKS with the service's signing key
- Authority hints for trust chain resolution
- Optional trust marks
New pkg/federation package provides:
- Config type for YAML configuration
- Service that builds and signs entity configuration JWTs
- EntityMetadata types for typed metadata sections
Configuration:
federation:
enabled: true
entity_id: "https://issuer.example.com" # defaults to public_url
authority_hints:
- "https://trust-anchor.example.com"
organization_name: "Example Org"
ttl: 86400
When federation.enabled is false (default), the endpoint returns 404.
Phase 3a of the client-id-strategy plan.
There was a problem hiding this comment.
Pull request overview
Implements OpenID Federation entity configuration (/.well-known/openid-federation) for both the APIGW (issuer) and verifier services, backed by a new pkg/federation package that constructs and signs entity-statement JWTs from YAML configuration.
Changes:
- Adds
pkg/federation(config/types + service for building/signing entity configuration JWTs) and initial unit tests. - Extends APIGW and Verifier YAML config models with an optional
federation:section. - Registers
/.well-known/openid-federationon both services and serves a self-signed entity statement JWT when enabled.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/model/config.go | Adds Federation *federation.Config to APIGW/Verifier configuration. |
| pkg/federation/entity_config.go | Introduces federation config schema + entity configuration/metadata/trust-mark types. |
| pkg/federation/service.go | Implements entity configuration JWT claim construction + signing. |
| pkg/federation/federation_test.go | Adds tests for federation types/claims structures (but not the signer-backed build path). |
| internal/apigw/httpserver/service.go | Registers the APIGW federation well-known endpoint. |
| internal/apigw/httpserver/endpoints_federation.go | Serves APIGW entity statement JWT (issuer/AS metadata). |
| internal/verifier/httpserver/service.go | Registers the verifier federation well-known endpoint. |
| internal/verifier/httpserver/endpoints_federation.go | Serves verifier entity statement JWT (RP metadata). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address review feedback: add TestBuildEntityConfigurationSignedJWT that generates an ephemeral ECDSA P-256 key, signs a real entity configuration JWT via BuildEntityConfiguration(), then parses the JWT with ParseUnverified and asserts all claims (iss, sub, authority_hints, metadata, jwks, trust_marks, exp, iat) are correctly structured.
Derive the request_object_signing_alg in verifier RP metadata from the actual SignerConfig key material instead of hard-coding ES256. Also use federation service's EntityID for client_id consistency. Addresses review comment on PR SUNET#514.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
internal/apigw/httpserver/endpoints_federation.go:19
- Returning nil after calling c.Status(http.StatusNotFound) will be overwritten by RegEndpoint's defaultStatus (it unconditionally calls c.Status(defaultStatus) when res == nil). Use AbortWithStatus (writes headers immediately) so the 404 is preserved when federation is disabled.
cfg := s.cfg.APIGW.Federation
if cfg == nil || !cfg.Enabled {
c.Status(http.StatusNotFound)
return nil, nil
}
internal/verifier/httpserver/endpoints_federation.go:19
- Returning nil after calling c.Status(http.StatusNotFound) will be overwritten by RegEndpoint's defaultStatus (it unconditionally calls c.Status(defaultStatus) when res == nil). Use AbortWithStatus (writes headers immediately) so the 404 is preserved when federation is disabled.
cfg := s.cfg.Verifier.Federation
if cfg == nil || !cfg.Enabled {
c.Status(http.StatusNotFound)
return nil, nil
}
- Omit vp_formats from openid_relying_party metadata entirely when PreferredVPFormats is unset, instead of serializing it as null. Extracted the map-building into buildOpenIDRelyingPartyMetadata for testability. - Make BuildEntityConfiguration clone the caller-supplied *EntityMetadata (and its maps) before injecting federation_entity fields, so callers that reuse a metadata instance across requests never see mutation or cross-request state leakage. - Add validate:"omitempty,dive" to Config.TrustMarks so nested TrustMarkConfig required fields (id/jwt) are actually checked by the validator, matching the dive convention used elsewhere in the codebase. - Fix a stale test comment that claimed no test used a real signer, even though TestBuildEntityConfigurationSignedJWT already does. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Addressed all 4 Copilot review comments:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
internal/apigw/httpserver/endpoints_federation.go:19
- This handler sets 404 via c.Status(...) but then returns (nil, nil). pkg/httphelpers/server.RegEndpoint will treat res==nil as success and unconditionally set the endpoint's defaultStatus (200), overwriting the 404. Use AbortWithStatus (writes headers immediately) to preserve 404 when federation is disabled.
cfg := s.cfg.APIGW.Federation
if cfg == nil || !cfg.Enabled {
c.Status(http.StatusNotFound)
return nil, nil
}
internal/verifier/httpserver/endpoints_federation.go:20
- This handler sets 404 via c.Status(...) but then returns (nil, nil). pkg/httphelpers/server.RegEndpoint will treat res==nil as success and unconditionally set the endpoint's defaultStatus (200), overwriting the 404. Use AbortWithStatus (writes headers immediately) to preserve 404 when federation is disabled.
cfg := s.cfg.Verifier.Federation
if cfg == nil || !cfg.Enabled {
c.Status(http.StatusNotFound)
return nil, nil
}
pkg/federation/service.go:90
- BuildEntityConfiguration assumes s.signer is non-nil and will panic on s.signer.GetJWK() if a nil signer was passed to NewService. Returning a clear error here avoids a hard crash and makes misuse easier to diagnose.
func (s *Service) BuildEntityConfiguration(metadata *EntityMetadata) (string, error) {
// Build JWKS from the signing key
jwk, err := s.signer.GetJWK()
if err != nil {
return "", fmt.Errorf("federation: get signing JWK: %w", err)
}
pkg/federation/service.go:26
- NewService dereferences cfg without a nil check (cfg.EntityID). Since NewService is exported, passing a nil cfg would panic at runtime. Consider defensively defaulting to an empty Config when cfg is nil to keep the constructor safe.
This issue also appears on line 85 of the same file.
func NewService(cfg *Config, signer *pki.SignerConfig, publicURL string) *Service {
entityID := cfg.EntityID
if entityID == "" {
entityID = publicURL
}
|
run make gen-config-docs |
- Move APIGW's federation entity-config handler logic out of the httpserver layer into apiv1 (new Client.FederationEntityConfig), matching the thin-handler pattern used by every other apigw endpoint (JWKS, SDJWTVCIssuerMetadata, etc.). Also switches the disabled-federation path to AbortWithStatus so the 404 isn't silently overwritten to 200 by RegEndpoint's default-status fallback. - Rename pkg/federation -> pkg/openidfederation to avoid ambiguity with other kinds of federation the project may add later. - Run `make gen-config-docs` to bring docs/CONFIGURATION.md back in sync with pkg/model/config.go (it was stale since 2026-06-12, predating this PR - the diff includes some already-existing, previously-undocumented fields like Kafka SASL/mTLS in addition to the new federation config).
|
Addressed both comments (commit ca5db01):
Also ran Heads up, not fixed here:
|
Per masv3971's follow-up comment on PR SUNET#514 - the internal Go identifiers (handler function names, file names, apiv1 method/type names) still said "Federation" after the package itself was renamed to openidfederation. Renamed: - endpoints_federation.go -> endpoints_openidfederation.go (apigw + verifier) - handlers_federation.go -> handlers_openidfederation.go (apigw apiv1) - endpointFederationEntityConfig -> endpointOpenIDFederationEntityConfig - apiv1.FederationEntityConfig -> apiv1.OpenIDFederationEntityConfig - FederationEntityConfigReply -> OpenIDFederationEntityConfigReply Left the yaml config key (federation:) and Cfg struct field name (Federation) as-is - that's the operator-facing config schema, a bigger change than what was asked for here.
# Conflicts: # pkg/model/config.go
|
Fixed (commit a1c624a) — renamed the internal Go identifiers to match the Left the yaml config key ( Also rebased onto current main (a lot has merged since — #481/#488/#519/#522/#493/#512 plus deps/security updates) and resolved the resulting conflict in |
# Conflicts: # docs/CONFIGURATION.md
- Rename openidfederation.NewService -> New (matches the New() convention used by every other package in this codebase). - Move cloneMetadata to a Clone() method on *EntityMetadata. - Accept a local Signer interface (GetJWK + SignJWT) instead of the concrete *pki.SignerConfig, and construct the signer/Service once in each service's New() instead of per-request (matches the underlying concern from an earlier Copilot comment on this same PR too - a request handler was re-loading key material on every single call). Full "reuse c.pkiSigner" wasn't possible as suggested: that field is typed as the narrower pki.Signer interface (Sign+Algorithm only), which doesn't expose GetJWK/SignJWT - so a separate signer is built once at service-construction time instead. - Rename Cfg.Federation -> Cfg.OpenIDFederation (Go field only; kept the `federation:` yaml key since it wasn't specifically called out and changing it isn't free the way the Go-only renames are). Applied to both apigw and verifier, which have the identical pattern.
# Conflicts: # docs/CONFIGURATION.md
|
Addressed all 4 (commit 6c013ed, rebased in 9c3c8f8):
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
pkg/openidfederation/service.go:117
- BuildEntityConfiguration always calls metadata.Clone(); since (*EntityMetadata).Clone() returns a non-nil empty struct on a nil receiver, this forces the JWT to include
"metadata": {}even when the caller passed nil and no org/logo injection is needed, defeating theomitemptyon the Metadata pointer.
metadata = metadata.Clone()
if s.config.OrganizationName != "" || s.config.LogoURI != "" {
docs/CONFIGURATION.md:720
- The new
federationsection in CONFIGURATION.md only documents the YAML path but not the available fields (enabled/entity_id/authority_hints/ttl/trust_marks/etc). This is inconsistent with other sections which provide a field table, so users won't know how to configure the endpoint.
### `federation`
> **Path:** `.apigw.federation`, `.verifier.federation`
pkg/openidfederation/service.go:1
- PR description says a new
pkg/federationpackage was added, but the implementation is introduced underpkg/openidfederation. If the rename is intentional, the PR summary should be updated; if not, the package path should be aligned to avoid confusion for reviewers and future contributors.
package openidfederation
- pkg/openidfederation/service.go: BuildEntityConfiguration always
called metadata.Clone(), and Clone() on a nil receiver returns a
non-nil empty *EntityMetadata. This forced "metadata": {} into the
signed JWT even when the caller passed nil and no org/logo injection
was configured, defeating the Metadata field's omitempty. Now only
clones when there's something to put in the result. Added
TestBuildEntityConfiguration_OmitsMetadataWhenNilAndNoOrgInfo.
- developer_tools/scripts/gen_config_docs/main.go: the doc generator's
hardcoded package directory list didn't include pkg/openidfederation,
so the generated `federation` section in CONFIGURATION.md had no
field table at all (just the YAML path) — inconsistent with every
other config section. Added it to the list and regenerated.
(The third suppressed comment — PR description says pkg/federation but
the code lives in pkg/openidfederation — is a stale description vs.
code naming note, not a code issue; the package was intentionally named
openidfederation.)
|
Found and fixed 2 real issues from Copilot's "suppressed due to low confidence" comments (commit 4b07e6c):
(The third suppressed comment — PR description says Build, vet, and |
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (6)
internal/verifier/httpserver/endpoints_federation_test.go:40
- This subtest should validate the standard "vp_formats_supported" key instead of "vp_formats" to match OpenID4VP client metadata naming used elsewhere in the repo.
t.Run("vp_formats present when configured", func(t *testing.T) {
formats := &openid4vp.VPFormatsSupported{
SDJWT: &openid4vp.SDJWTVCFormat{},
}
m := buildOpenIDRelyingPartyMetadata("https://verifier.example.com", "Example Verifier", "ES256", formats)
if m["vp_formats"] != formats {
t.Errorf("expected vp_formats = %v, got %v", formats, m["vp_formats"])
}
internal/verifier/httpserver/endpoints_federation_test.go:28
- This assertion should check for "vp_formats_supported" rather than "vp_formats" to match the OpenID4VP field name used elsewhere in the repo.
if _, ok := raw["vp_formats"]; ok {
t.Errorf("expected serialized JSON to omit \"vp_formats\", got %s", data)
}
internal/verifier/httpserver/endpoints_federation_test.go:51
- This assertion should check for "vp_formats_supported" rather than "vp_formats" to match the OpenID4VP field name used elsewhere in the repo.
if _, ok := raw["vp_formats"]; !ok {
t.Errorf("expected serialized JSON to include \"vp_formats\", got %s", data)
}
internal/verifier/httpserver/endpoints_openidfederation.go:63
- The verifier OpenID Federation metadata uses the key "vp_formats", but the rest of the codebase (and OpenID4VP client metadata) uses "vp_formats_supported" (see pkg/oauth2/client_metadata.go and pkg/openid4vp/request_object.go). Using a different key risks wallets ignoring the advertised formats.
// buildOpenIDRelyingPartyMetadata builds the openid_relying_party metadata
// map advertised in the entity configuration. vpFormats is optional; the
// map is untyped (map[string]any), so a struct json tag can't omit an
// unset field for us -- the "vp_formats" key is only added when vpFormats
// is non-nil, to avoid serializing "vp_formats": null.
func buildOpenIDRelyingPartyMetadata(clientID, clientName, signingAlg string, vpFormats *openid4vp.VPFormatsSupported) map[string]any {
m := map[string]any{
"client_id": clientID,
"response_types": []string{"vp_token"},
"client_name": clientName,
"request_object_signing_alg": signingAlg,
}
if vpFormats != nil {
m["vp_formats"] = vpFormats
}
pkg/openidfederation/service.go:134
- BuildEntityConfiguration only places the JWKS at the top-level "jwks" claim. Elsewhere in this repo, key extraction from OpenID Federation entity configuration (trust_metadata) expects JWKS nested under metadata.openid_relying_party/openid_provider -> jwks (see pkg/keyresolver/did_helpers.go and its tests). Without also embedding JWKS in the relevant metadata section, downstream key resolution can fail.
if metadata != nil || s.config.OrganizationName != "" || s.config.LogoURI != "" {
metadata = metadata.Clone()
if s.config.OrganizationName != "" || s.config.LogoURI != "" {
if metadata.FederationEntity == nil {
metadata.FederationEntity = make(map[string]any)
}
if s.config.OrganizationName != "" {
metadata.FederationEntity["organization_name"] = s.config.OrganizationName
}
if s.config.LogoURI != "" {
metadata.FederationEntity["logo_uri"] = s.config.LogoURI
}
}
}
internal/verifier/httpserver/endpoints_federation_test.go:16
- The tests are validating a "vp_formats" key, but OpenID4VP client metadata in this repo uses "vp_formats_supported" (e.g., pkg/oauth2/client_metadata.go). Align the test expectations with the on-the-wire field name to avoid locking in a non-standard key.
This issue also appears in the following locations of the same file:
- line 26
- line 31
- line 49
t.Run("vp_formats omitted when nil", func(t *testing.T) {
m := buildOpenIDRelyingPartyMetadata("https://verifier.example.com", "Example Verifier", "ES256", nil)
if _, ok := m["vp_formats"]; ok {
t.Errorf("expected \"vp_formats\" key to be absent, got %v", m["vp_formats"])
Summary
Implement
/.well-known/openid-federationfor both APIGW (issuer) and verifier services per OpenID Federation 1.0 §5.2.Changes
pkg/federationpackage providing:Configtype for YAML configurationServicethat builds and signs entity configuration JWTsEntityMetadatatypes for typed metadata sectionsfederation:section in YAML configConfiguration Example
When
federation.enabledis false (default), the endpoint returns 404.Context
Phase 3a of the client-id-strategy plan — enables SUNET/vc to participate in OpenID Federation trust chains.