From 77433e6f6115c0b08b3badc41f6db5eb3a3aaa1c Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Wed, 10 Jun 2026 08:01:33 +0200 Subject: [PATCH 1/9] feat(auth): support both insert and append metadata discovery (#4310) Metadata discovery derived URLs using only the RFC 8414 insert convention (well-known segment at the authority root, issuer/AS path appended). Servers that publish metadata only under the OIDC Discovery append convention (well-known after the path) returned 404 and could not complete issuance or token flows without per-deployment nginx rewrites. Discovery now tries candidate locations in priority order and takes the first matching document: 1. insert (RFC 8414, unchanged happy path) 2. append (OIDC Discovery) 3. append openid-configuration (AS metadata only) When the identifier has no path both forms collapse to one URL, so spec-compliant servers still make a single request. Each candidate shares the identifier's host and is SSRF-validated via core.ParsePublicURL, and the existing identifier-match check (credential_issuer / issuer must equal the requested identifier) is enforced on the accepted document, so the fallback cannot be steered to an attacker-chosen file. On exhaustion the error names the identifier and reports only non-404 failures; a >= 500 failure still maps to 502 Bad Gateway. Assisted by AI (cherry picked from commit f01c39a0827a90dbba95530169123a367c74006a) --- auth/client/iam/client.go | 88 ++++++++++++++++++++--- auth/client/iam/client_test.go | 116 +++++++++++++++++++++++++----- auth/client/iam/openid4vp_test.go | 5 +- auth/oauth/types.go | 43 ++++++++++- auth/oauth/types_test.go | 34 +++++++++ auth/openid4vci/client.go | 72 ++++++++++--------- auth/openid4vci/client_test.go | 66 ++++++++++++++++- 7 files changed, 356 insertions(+), 68 deletions(-) diff --git a/auth/client/iam/client.go b/auth/client/iam/client.go index 3112ede9bf..9234d505ee 100644 --- a/auth/client/iam/client.go +++ b/auth/client/iam/client.go @@ -64,22 +64,88 @@ func (hb HTTPClient) OAuthAuthorizationServerMetadata(ctx context.Context, oauth // - did:web:example.com becomes https://example.com/.well-known/oauth-authorization-server // - did:web:example.com:iam:123 becomes https://example.com/.well-known/oauth-authorization-server/did:web:example.com:iam:123 - metadataURL, err := oauth.IssuerIdToWellKnown(oauthIssuer, oauth.AuthzServerWellKnown, hb.strictMode) + // Try the well-known locations in priority order and take the first that + // returns a matching document: + // 1. insert (RFC 8414): https://host/.well-known/oauth-authorization-server/ + // 2. append (OIDC Disc): https://host//.well-known/oauth-authorization-server + // 3. append openid-configuration: https://host//.well-known/openid-configuration + // Many authorization servers publish metadata only under the append convention. + candidates, err := oauth.WellKnownCandidates(oauthIssuer, oauth.AuthzServerWellKnown, hb.strictMode) if err != nil { return nil, err } - var metadata oauth.AuthorizationServerMetadata - if err = hb.doGet(ctx, metadataURL.String(), &metadata); err != nil { - // if this is a core.HttpError and the status code >= 500 then we want the caller to receive a 502 Bad Gateway - // we do this by changing the status code of the error - // any other error should result in a 400 Bad Request - if httpErr, ok := err.(core.HttpError); ok && httpErr.StatusCode >= 500 { - httpErr.StatusCode = http.StatusBadGateway - return nil, httpErr + // OpenID4VCI also permits retrieving AS metadata via OIDC Discovery's openid-configuration + // document; add its append form (the last candidate) as a final fallback. + oidcCandidates, err := oauth.WellKnownCandidates(oauthIssuer, oauth.OpenIdConfigurationWellKnown, hb.strictMode) + if err != nil { + return nil, err + } + candidates = append(candidates, oidcCandidates[len(oidcCandidates)-1]) + + var failures []candidateFailure + for _, candidate := range candidates { + var metadata oauth.AuthorizationServerMetadata + if getErr := hb.doGet(ctx, candidate, &metadata); getErr != nil { + failures = append(failures, candidateFailure{url: candidate, err: getErr, status: statusCodeOf(getErr)}) + continue + } + // Identifier-match check (RFC 8414 §3.3): the returned issuer MUST equal the + // requested identifier, so the fallback cannot be steered to a document the + // host serves under a different issuer. A mismatch falls through. + if metadata.Issuer != oauthIssuer { + failures = append(failures, candidateFailure{ + url: candidate, + err: fmt.Errorf("issuer %q does not match requested %q", metadata.Issuer, oauthIssuer), + }) + continue } - return nil, errors.Join(ErrInvalidClientCall, err) + return &metadata, nil + } + return nil, authorizationServerMetadataError(oauthIssuer, failures) +} + +// candidateFailure records why a single metadata candidate URL was rejected. +type candidateFailure struct { + url string + err error + status int +} + +// statusCodeOf extracts the HTTP status code from a core.HttpError, or 0 if the +// error is not an HTTP status error (e.g. a transport or decode failure). +func statusCodeOf(err error) int { + var httpErr core.HttpError + if errors.As(err, &httpErr) { + return httpErr.StatusCode + } + return 0 +} + +// authorizationServerMetadataError builds the error returned when every candidate +// was exhausted. It names the identifier and reports only the non-404 failures +// (a 404 just means "not at this location"). If any candidate failed with a +// >= 500 status, that failure is surfaced as a 502 Bad Gateway, preserving the +// existing severity mapping. +func authorizationServerMetadataError(identifier string, failures []candidateFailure) error { + var diagnostics []string + for i := range failures { + f := failures[i] + if f.status >= 500 { + // Map upstream server errors to 502 Bad Gateway, keeping the core.HttpError + // so callers can still assert on it. + if httpErr, ok := f.err.(core.HttpError); ok { + httpErr.StatusCode = http.StatusBadGateway + return httpErr + } + } + if f.status != http.StatusNotFound { + diagnostics = append(diagnostics, fmt.Sprintf("%s: %v", f.url, f.err)) + } + } + if len(diagnostics) == 0 { + return errors.Join(ErrInvalidClientCall, fmt.Errorf("OAuth authorization server metadata not found at any candidate location for %q", identifier)) } - return &metadata, err + return errors.Join(ErrInvalidClientCall, fmt.Errorf("failed to retrieve OAuth authorization server metadata for %q: %s", identifier, strings.Join(diagnostics, "; "))) } // ClientMetadata retrieves the client metadata from the client metadata endpoint given in the authorization request. diff --git a/auth/client/iam/client_test.go b/auth/client/iam/client_test.go index 560715c822..b8f9bd446f 100644 --- a/auth/client/iam/client_test.go +++ b/auth/client/iam/client_test.go @@ -50,37 +50,121 @@ import ( func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) { ctx := context.Background() - t.Run("ok using root web:did", func(t *testing.T) { - result := oauth.AuthorizationServerMetadata{TokenEndpoint: "/token"} - handler := http2.Handler{StatusCode: http.StatusOK, ResponseData: result} - tlsServer, client := testServerAndClient(t, &handler) + // metadataServer serves AuthorizationServerMetadata at the listed paths and returns + // missStatus for any other path. The served issuer equals the server URL plus issuerPath, + // so it matches an identifier of tlsServer.URL+issuerPath. It records every requested path. + metadataServer := func(t *testing.T, missStatus int, issuerPath string, servedPaths ...string) (*httptest.Server, *HTTPClient, *[]string) { + var requested []string + served := make(map[string]struct{}, len(servedPaths)) + var issuer string + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requested = append(requested, r.URL.Path) + if _, ok := served[r.URL.Path]; ok { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(oauth.AuthorizationServerMetadata{Issuer: issuer, TokenEndpoint: "/token"}) + return + } + w.WriteHeader(missStatus) + }) + tlsServer, client := testServerAndClient(t, handler) + issuer = tlsServer.URL + issuerPath + for _, p := range servedPaths { + served[p] = struct{}{} + } + return tlsServer, client, &requested + } + + t.Run("ok - insert form, root identifier, single request", func(t *testing.T) { + tlsServer, client, requested := metadataServer(t, http.StatusNotFound, "", "/.well-known/oauth-authorization-server") metadata, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL) require.NoError(t, err) require.NotNil(t, metadata) assert.Equal(t, "/token", metadata.TokenEndpoint) - require.NotNil(t, handler.Request) - assert.Equal(t, "GET", handler.Request.Method) - assert.Equal(t, "/.well-known/oauth-authorization-server", handler.Request.URL.Path) + assert.Equal(t, []string{"/.well-known/oauth-authorization-server"}, *requested) }) - t.Run("ok using user web:did", func(t *testing.T) { - result := oauth.AuthorizationServerMetadata{TokenEndpoint: "/token"} - handler := http2.Handler{StatusCode: http.StatusOK, ResponseData: result} - tlsServer, client := testServerAndClient(t, &handler) + t.Run("ok - insert form, identifier with path", func(t *testing.T) { + tlsServer, client, requested := metadataServer(t, http.StatusNotFound, "/iam/123", "/.well-known/oauth-authorization-server/iam/123") + + metadata, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL+"/iam/123") + + require.NoError(t, err) + require.NotNil(t, metadata) + // Insert form is tried first; no fallback request is made on the happy path. + assert.Equal(t, []string{"/.well-known/oauth-authorization-server/iam/123"}, *requested) + }) + t.Run("ok - append form when insert 404s", func(t *testing.T) { + tlsServer, client, requested := metadataServer(t, http.StatusNotFound, "/iam/123", "/iam/123/.well-known/oauth-authorization-server") + + metadata, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL+"/iam/123") + + require.NoError(t, err) + require.NotNil(t, metadata) + assert.Equal(t, []string{ + "/.well-known/oauth-authorization-server/iam/123", + "/iam/123/.well-known/oauth-authorization-server", + }, *requested) + }) + t.Run("ok - openid-configuration as third candidate", func(t *testing.T) { + tlsServer, client, requested := metadataServer(t, http.StatusNotFound, "/iam/123", "/iam/123/.well-known/openid-configuration") metadata, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL+"/iam/123") + require.NoError(t, err) + require.NotNil(t, metadata) + assert.Equal(t, []string{ + "/.well-known/oauth-authorization-server/iam/123", + "/iam/123/.well-known/oauth-authorization-server", + "/iam/123/.well-known/openid-configuration", + }, *requested) + }) + t.Run("error - all candidates 404 yields a plain not-found error naming the identifier", func(t *testing.T) { + tlsServer, client, requested := metadataServer(t, http.StatusNotFound, "/iam/123") + + _, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL+"/iam/123") + + require.Error(t, err) + assert.ErrorIs(t, err, ErrInvalidClientCall) + assert.Contains(t, err.Error(), "not found at any candidate location") + assert.Contains(t, err.Error(), tlsServer.URL+"/iam/123") + assert.Len(t, *requested, 3) + }) + t.Run("identifier mismatch on first candidate falls through to next", func(t *testing.T) { + // Insert form returns 200 but with a non-matching issuer; append form serves the match. + var issuer string + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/.well-known/oauth-authorization-server/iam/123": + _ = json.NewEncoder(w).Encode(oauth.AuthorizationServerMetadata{Issuer: "https://attacker.example", TokenEndpoint: "/evil"}) + case "/iam/123/.well-known/oauth-authorization-server": + _ = json.NewEncoder(w).Encode(oauth.AuthorizationServerMetadata{Issuer: issuer, TokenEndpoint: "/token"}) + default: + w.WriteHeader(http.StatusNotFound) + } + }) + tlsServer, client := testServerAndClient(t, handler) + issuer = tlsServer.URL + "/iam/123" + + metadata, err := client.OAuthAuthorizationServerMetadata(ctx, issuer) + require.NoError(t, err) require.NotNil(t, metadata) assert.Equal(t, "/token", metadata.TokenEndpoint) - require.NotNil(t, handler.Request) - assert.Equal(t, "GET", handler.Request.Method) - assert.Equal(t, "/.well-known/oauth-authorization-server/iam/123", handler.Request.URL.Path) + assert.Equal(t, issuer, metadata.Issuer) + }) + t.Run("error - non-404 status is preserved in the exhausted error", func(t *testing.T) { + tlsServer, client, _ := metadataServer(t, http.StatusForbidden, "/iam/123") + + _, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL+"/iam/123") + + require.Error(t, err) + assert.ErrorIs(t, err, ErrInvalidClientCall) + assert.Contains(t, err.Error(), "403") }) t.Run("error - server error changes status code to 502", func(t *testing.T) { - handler := http2.Handler{StatusCode: http.StatusInternalServerError} - tlsServer, client := testServerAndClient(t, &handler) + tlsServer, client, _ := metadataServer(t, http.StatusInternalServerError, "") _, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL) diff --git a/auth/client/iam/openid4vp_test.go b/auth/client/iam/openid4vp_test.go index 081617e082..539b1b8a2b 100644 --- a/auth/client/iam/openid4vp_test.go +++ b/auth/client/iam/openid4vp_test.go @@ -234,7 +234,7 @@ func TestIAMClient_AuthorizationServerMetadata(t *testing.T) { require.Error(t, err) assert.ErrorIs(t, err, ErrInvalidClientCall) - assert.ErrorContains(t, err, "server returned HTTP 404 (expected: 200)") + assert.ErrorContains(t, err, "not found at any candidate location") }) } @@ -382,7 +382,7 @@ func TestRelyingParty_RequestServiceAccessToken(t *testing.T) { require.Error(t, err) assert.ErrorIs(t, err, ErrInvalidClientCall) - assert.ErrorContains(t, err, "server returned HTTP 404 (expected: 200)") + assert.ErrorContains(t, err, "not found at any candidate location") }) t.Run("error - faulty presentation definition", func(t *testing.T) { ctx := createClientServerTestContext(t) @@ -1067,6 +1067,7 @@ func createClientServerTestContext(t *testing.T) *clientServerTestContext { ctx.verifierURL = test2.MustParseURL(ctx.tlsServer.URL) ctx.issuerDID = didweb.ServerURLToDIDWeb(t, ctx.tlsServer.URL+"/issuer") ctx.authzServerMetadata = metadata + ctx.authzServerMetadata.Issuer = ctx.tlsServer.URL ctx.authzServerMetadata.TokenEndpoint = ctx.tlsServer.URL + "/token" ctx.authzServerMetadata.PresentationDefinitionEndpoint = ctx.tlsServer.URL + "/presentation_definition" ctx.authzServerMetadata.AuthorizationEndpoint = ctx.tlsServer.URL + "/authorize" diff --git a/auth/oauth/types.go b/auth/oauth/types.go index 380909c0ec..e8232242f9 100644 --- a/auth/oauth/types.go +++ b/auth/oauth/types.go @@ -21,9 +21,11 @@ package oauth import ( "encoding/json" + "net/url" + "strings" + "github.com/lestrrat-go/jwx/v2/jwk" "github.com/nuts-foundation/nuts-node/core" - "net/url" ) // this file contains constants, variables and helper functions for OAuth related code @@ -255,6 +257,45 @@ func IssuerIdToWellKnown(issuer string, wellKnown string, strictmode bool) (*url return issuerURL.Parse(wellKnown + issuerURL.EscapedPath()) } +// WellKnownCandidates returns the metadata URLs to try for the given identifier +// and well-known document, in priority order: +// +// 1. insert (RFC 8414): https://host/.well-known// +// 2. append (OIDC Disc): https://host//.well-known/ +// +// When the identifier has no path, both forms collapse to the same URL and a +// single candidate is returned. The caller fetches the candidates in order and +// takes the first that returns a usable metadata document. Each candidate shares +// the identifier's scheme and host, so the single core.ParsePublicURL SSRF check +// on the identifier covers them all. +func WellKnownCandidates(identifier string, wellKnown string, strictmode bool) ([]string, error) { + identifierURL, err := core.ParsePublicURL(identifier, strictmode) + if err != nil { + return nil, err + } + // insert form (RFC 8414): well-known segment at the authority root, identifier path appended. + // Set RawPath alongside Path when present so url.String() does not re-escape pre-encoded + // characters like %2F via EscapedPath's reescaping pass. + insert := *identifierURL + if strings.Trim(identifierURL.Path, "/") == "" { + // No path: insert and append are identical; a single candidate suffices. + insert.Path = wellKnown + insert.RawPath = "" + return []string{insert.String()}, nil + } + insert.Path = wellKnown + identifierURL.Path + if identifierURL.RawPath != "" { + insert.RawPath = wellKnown + identifierURL.RawPath + } + // append form (OIDC Discovery): well-known segment after the identifier path. + appended := *identifierURL + appended.Path = strings.TrimSuffix(identifierURL.Path, "/") + wellKnown + if identifierURL.RawPath != "" { + appended.RawPath = strings.TrimSuffix(identifierURL.RawPath, "/") + wellKnown + } + return []string{insert.String(), appended.String()}, nil +} + // AuthorizationServerMetadata defines the OAuth Authorization Server metadata. // Specified by https://www.rfc-editor.org/rfc/rfc8414.txt type AuthorizationServerMetadata struct { diff --git a/auth/oauth/types_test.go b/auth/oauth/types_test.go index e0932a7991..4ea03b3b37 100644 --- a/auth/oauth/types_test.go +++ b/auth/oauth/types_test.go @@ -67,6 +67,40 @@ func TestIssuerIdToWellKnown(t *testing.T) { }) } +func TestWellKnownCandidates(t *testing.T) { + t.Run("identifier with path returns insert then append", func(t *testing.T) { + candidates, err := WellKnownCandidates("https://nuts.nl/iam/id", AuthzServerWellKnown, true) + require.NoError(t, err) + assert.Equal(t, []string{ + "https://nuts.nl/.well-known/oauth-authorization-server/iam/id", + "https://nuts.nl/iam/id/.well-known/oauth-authorization-server", + }, candidates) + }) + t.Run("no path collapses to a single candidate", func(t *testing.T) { + candidates, err := WellKnownCandidates("https://nuts.nl", AuthzServerWellKnown, true) + require.NoError(t, err) + assert.Equal(t, []string{"https://nuts.nl/.well-known/oauth-authorization-server"}, candidates) + }) + t.Run("trailing-slash-only path collapses to a single candidate", func(t *testing.T) { + candidates, err := WellKnownCandidates("https://nuts.nl/", AuthzServerWellKnown, true) + require.NoError(t, err) + assert.Equal(t, []string{"https://nuts.nl/.well-known/oauth-authorization-server"}, candidates) + }) + t.Run("percent-encoded path is not double-escaped", func(t *testing.T) { + candidates, err := WellKnownCandidates("https://nuts.nl/foo%2Fbar", OpenIdCredIssuerWellKnown, true) + require.NoError(t, err) + assert.Equal(t, []string{ + "https://nuts.nl/.well-known/openid-credential-issuer/foo%2Fbar", + "https://nuts.nl/foo%2Fbar/.well-known/openid-credential-issuer", + }, candidates) + }) + t.Run("invalid identifier returns the SSRF/parse error", func(t *testing.T) { + candidates, err := WellKnownCandidates("http://nuts.nl/iam/id", AuthzServerWellKnown, true) + assert.ErrorContains(t, err, "scheme must be https") + assert.Nil(t, candidates) + }) +} + func TestTokenResponse_Marshalling(t *testing.T) { expected := (&TokenResponse{AccessToken: "1234567", TokenType: "bearer", ExpiresIn: to.Ptr(5), Scope: to.Ptr("abc"), DPoPKid: to.Ptr("kid")}).With("c_nonce", "hello") diff --git a/auth/openid4vci/client.go b/auth/openid4vci/client.go index 9bef49ba9e..9ca4ccec83 100644 --- a/auth/openid4vci/client.go +++ b/auth/openid4vci/client.go @@ -26,15 +26,12 @@ import ( "io" "net/http" "net/url" + "strings" "github.com/nuts-foundation/nuts-node/auth/oauth" "github.com/nuts-foundation/nuts-node/core" ) -// wellKnownPath is the path segment defined in OpenID4VCI 1.0 §12.2.2 for -// retrieving the Credential Issuer Metadata document. -const wellKnownPath = "/.well-known/openid-credential-issuer" - // RequestCredentialOpts carries all parameters for a Credential Request. // // CredentialIdentifier and CredentialConfigurationID are mutually exclusive @@ -124,32 +121,59 @@ func (c *client) OpenIDCredentialIssuerMetadata(ctx context.Context, issuerURL s if parsed, _ := url.Parse(issuerURL); parsed != nil && (parsed.RawQuery != "" || parsed.Fragment != "") { return nil, fmt.Errorf("openid4vci: invalid issuer URL: query and fragment components are not allowed") } - wellKnownURL, err := credentialIssuerWellKnown(issuerURL) + // Try the well-known locations in priority order (insert per RFC 8414, then + // append per OIDC Discovery) and take the first that returns a matching + // document. Many issuers publish metadata only under the append convention. + candidates, err := oauth.WellKnownCandidates(issuerURL, oauth.OpenIdCredIssuerWellKnown, c.strictMode) if err != nil { return nil, fmt.Errorf("openid4vci: invalid issuer URL: %w", err) } - req, err := http.NewRequestWithContext(ctx, http.MethodGet, wellKnownURL, http.NoBody) + var failures []string + for _, candidate := range candidates { + metadata, status, fetchErr := c.fetchIssuerMetadata(ctx, candidate, issuerURL) + if fetchErr == nil { + return metadata, nil + } + // A 404 just means "not at this location" and is noise; only non-404 + // failures (403/405/5xx, decode errors, identifier mismatch) are worth + // surfacing when every candidate is exhausted. + if status != http.StatusNotFound { + failures = append(failures, fmt.Sprintf("%s: %v", candidate, fetchErr)) + } + } + if len(failures) == 0 { + return nil, fmt.Errorf("openid4vci: issuer metadata not found at any candidate location for %q", issuerURL) + } + return nil, fmt.Errorf("openid4vci: failed to retrieve issuer metadata for %q: %s", issuerURL, strings.Join(failures, "; ")) +} + +// fetchIssuerMetadata retrieves and validates the Credential Issuer Metadata from a +// single candidate URL. It returns the response status code (0 when no response was +// received) so the caller can distinguish a 404 from other failures. +func (c *client) fetchIssuerMetadata(ctx context.Context, candidateURL string, issuerURL string) (*OpenIDCredentialIssuerMetadata, int, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, candidateURL, http.NoBody) if err != nil { - return nil, err + return nil, 0, err } resp, err := c.httpClient.Do(req) if err != nil { - return nil, err + return nil, 0, err } defer resp.Body.Close() if resp.StatusCode < 200 || resp.StatusCode > 299 { - return nil, fmt.Errorf("openid4vci: fetching issuer metadata returned status %d", resp.StatusCode) + return nil, resp.StatusCode, fmt.Errorf("fetching issuer metadata returned status %d", resp.StatusCode) } var metadata OpenIDCredentialIssuerMetadata if err := json.NewDecoder(resp.Body).Decode(&metadata); err != nil { - return nil, fmt.Errorf("openid4vci: decoding issuer metadata: %w", err) + return nil, resp.StatusCode, fmt.Errorf("decoding issuer metadata: %w", err) } // Per §12.2.4: the credential_issuer value MUST match the issuer identifier - // the metadata document was retrieved for. Mismatched metadata MUST NOT be used. + // the metadata document was retrieved for. Mismatched metadata MUST NOT be used, + // so a mismatch falls through to the next candidate. if metadata.CredentialIssuer != issuerURL { - return nil, fmt.Errorf("openid4vci: credential_issuer %q does not match requested issuer %q", metadata.CredentialIssuer, issuerURL) + return nil, resp.StatusCode, fmt.Errorf("credential_issuer %q does not match requested issuer %q", metadata.CredentialIssuer, issuerURL) } - return &metadata, nil + return &metadata, resp.StatusCode, nil } func (c *client) RequestNonce(ctx context.Context, nonceEndpoint string) (string, error) { @@ -234,25 +258,3 @@ func (c *client) RequestCredential(ctx context.Context, opts RequestCredentialOp } return &credResp, nil } - -// credentialIssuerWellKnown returns the Credential Issuer Metadata URL for -// the given issuer identifier per RFC 8615: the well-known segment is -// inserted at the authority root, and the issuer's path is appended after. -// -// Example: https://example.com/oauth2/alice -// -// -> https://example.com/.well-known/openid-credential-issuer/oauth2/alice -func credentialIssuerWellKnown(issuerURL string) (string, error) { - u, err := url.Parse(issuerURL) - if err != nil { - return "", err - } - // Prepend the well-known segment to both Path (decoded) and RawPath - // (encoded) when the latter is set, so u.String() does not double-escape - // pre-encoded characters like %2F via EscapedPath's reescaping pass. - u.Path = wellKnownPath + u.Path - if u.RawPath != "" { - u.RawPath = wellKnownPath + u.RawPath - } - return u.String(), nil -} diff --git a/auth/openid4vci/client_test.go b/auth/openid4vci/client_test.go index 95f0e21fe3..942083a8f2 100644 --- a/auth/openid4vci/client_test.go +++ b/auth/openid4vci/client_test.go @@ -148,16 +148,76 @@ func TestClient_OpenIDCredentialIssuerMetadata(t *testing.T) { assert.Contains(t, err.Error(), "does not match") }) - t.Run("error on non-2xx", func(t *testing.T) { + t.Run("falls back to append form when insert form 404s", func(t *testing.T) { + var requested []string + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requested = append(requested, r.URL.Path) + if r.URL.Path != "/oauth2/alice/.well-known/openid-credential-issuer" { + http.Error(w, "not found", http.StatusNotFound) + return + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(OpenIDCredentialIssuerMetadata{CredentialIssuer: srv.URL + "/oauth2/alice"}) + })) + defer srv.Close() + + client := NewClient(srv.Client(), false) + metadata, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL+"/oauth2/alice") + require.NoError(t, err) + require.NotNil(t, metadata) + assert.Equal(t, []string{ + "/.well-known/openid-credential-issuer/oauth2/alice", + "/oauth2/alice/.well-known/openid-credential-issuer", + }, requested) + }) + + t.Run("identifier mismatch on first candidate falls through to next", func(t *testing.T) { + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/.well-known/openid-credential-issuer/oauth2/alice": + // 200 but with a non-matching credential_issuer: must be rejected and fallen through. + _ = json.NewEncoder(w).Encode(OpenIDCredentialIssuerMetadata{CredentialIssuer: "https://attacker.example/"}) + case "/oauth2/alice/.well-known/openid-credential-issuer": + _ = json.NewEncoder(w).Encode(OpenIDCredentialIssuerMetadata{CredentialIssuer: srv.URL + "/oauth2/alice"}) + default: + http.Error(w, "not found", http.StatusNotFound) + } + })) + defer srv.Close() + + client := NewClient(srv.Client(), false) + metadata, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL+"/oauth2/alice") + require.NoError(t, err) + require.NotNil(t, metadata) + assert.Equal(t, srv.URL+"/oauth2/alice", metadata.CredentialIssuer) + }) + + t.Run("all candidates 404 yields a plain not-found error naming the identifier", func(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Error(w, "not found", http.StatusNotFound) })) defer srv.Close() client := NewClient(srv.Client(), false) - _, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL) + _, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL+"/oauth2/alice") + require.Error(t, err) + assert.Contains(t, err.Error(), "not found at any candidate location") + assert.Contains(t, err.Error(), srv.URL+"/oauth2/alice") + }) + + t.Run("non-404 status is preserved in the exhausted error", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "forbidden", http.StatusForbidden) + })) + defer srv.Close() + + client := NewClient(srv.Client(), false) + _, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL+"/oauth2/alice") require.Error(t, err) - assert.Contains(t, err.Error(), "404") + assert.Contains(t, err.Error(), "403") }) t.Run("rejects non-https issuer URL in strict mode", func(t *testing.T) { From 1d537dbadc2c23cc8d56f6817d97320533d8b79e Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Wed, 10 Jun 2026 13:44:19 +0200 Subject: [PATCH 2/9] fix(auth): tolerate trailing slash in metadata identifier match Some authorization servers (e.g. IdentityServer) normalize the metadata issuer with a trailing slash, so a server reached at the append location with issuer "https://host/oauth/" was rejected against the requested identifier "https://host/oauth". Discovery then fell back to the credential issuer and surfaced an unrelated 404. Compare issuer / credential_issuer with a trailing-slash tolerance via oauth.IdentifiersMatch. Still rejects genuinely different identifiers, so the fallback cannot be steered to another document. Assisted by AI (cherry picked from commit b416b3dd039efc603b02a6c114457e676e45b6c8) --- auth/client/iam/client.go | 2 +- auth/client/iam/client_test.go | 21 +++++++++++++++++++++ auth/oauth/types.go | 10 ++++++++++ auth/oauth/types_test.go | 9 +++++++++ auth/openid4vci/client.go | 2 +- 5 files changed, 42 insertions(+), 2 deletions(-) diff --git a/auth/client/iam/client.go b/auth/client/iam/client.go index 9234d505ee..8e2beec0ce 100644 --- a/auth/client/iam/client.go +++ b/auth/client/iam/client.go @@ -92,7 +92,7 @@ func (hb HTTPClient) OAuthAuthorizationServerMetadata(ctx context.Context, oauth // Identifier-match check (RFC 8414 §3.3): the returned issuer MUST equal the // requested identifier, so the fallback cannot be steered to a document the // host serves under a different issuer. A mismatch falls through. - if metadata.Issuer != oauthIssuer { + if !oauth.IdentifiersMatch(metadata.Issuer, oauthIssuer) { failures = append(failures, candidateFailure{ url: candidate, err: fmt.Errorf("issuer %q does not match requested %q", metadata.Issuer, oauthIssuer), diff --git a/auth/client/iam/client_test.go b/auth/client/iam/client_test.go index b8f9bd446f..419df7e765 100644 --- a/auth/client/iam/client_test.go +++ b/auth/client/iam/client_test.go @@ -154,6 +154,27 @@ func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) { assert.Equal(t, "/token", metadata.TokenEndpoint) assert.Equal(t, issuer, metadata.Issuer) }) + t.Run("ok - metadata issuer with a trailing slash matches the requested identifier", func(t *testing.T) { + // Some servers (e.g. IdentityServer) normalize the issuer with a trailing slash; + // the append metadata is served with issuer = requested identifier + "/". + var issuer string + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/oauth/.well-known/oauth-authorization-server" { + w.WriteHeader(http.StatusNotFound) + return + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(oauth.AuthorizationServerMetadata{Issuer: issuer + "/", TokenEndpoint: "/token"}) + }) + tlsServer, client := testServerAndClient(t, handler) + issuer = tlsServer.URL + "/oauth" + + metadata, err := client.OAuthAuthorizationServerMetadata(ctx, issuer) + + require.NoError(t, err) + require.NotNil(t, metadata) + assert.Equal(t, issuer+"/", metadata.Issuer) + }) t.Run("error - non-404 status is preserved in the exhausted error", func(t *testing.T) { tlsServer, client, _ := metadataServer(t, http.StatusForbidden, "/iam/123") diff --git a/auth/oauth/types.go b/auth/oauth/types.go index e8232242f9..025d192ec7 100644 --- a/auth/oauth/types.go +++ b/auth/oauth/types.go @@ -257,6 +257,16 @@ func IssuerIdToWellKnown(issuer string, wellKnown string, strictmode bool) (*url return issuerURL.Parse(wellKnown + issuerURL.EscapedPath()) } +// IdentifiersMatch reports whether two issuer / credential-issuer identifiers are +// equal, tolerating a trailing-slash difference. RFC 8414 §3.3 and OpenID4VCI +// §12.2.4 require the identifier in a metadata document to be byte-identical to +// the requested identifier, but some servers (e.g. IdentityServer) normalize it +// with a trailing slash. Treat those as a match so discovery does not reject +// otherwise-valid metadata over a trailing slash. +func IdentifiersMatch(a string, b string) bool { + return strings.TrimRight(a, "/") == strings.TrimRight(b, "/") +} + // WellKnownCandidates returns the metadata URLs to try for the given identifier // and well-known document, in priority order: // diff --git a/auth/oauth/types_test.go b/auth/oauth/types_test.go index 4ea03b3b37..4910589ce9 100644 --- a/auth/oauth/types_test.go +++ b/auth/oauth/types_test.go @@ -67,6 +67,15 @@ func TestIssuerIdToWellKnown(t *testing.T) { }) } +func TestIdentifiersMatch(t *testing.T) { + assert.True(t, IdentifiersMatch("https://nuts.nl/oauth", "https://nuts.nl/oauth")) + assert.True(t, IdentifiersMatch("https://nuts.nl/oauth/", "https://nuts.nl/oauth"), "trailing slash on metadata issuer should match") + assert.True(t, IdentifiersMatch("https://nuts.nl/oauth", "https://nuts.nl/oauth/"), "trailing slash on requested identifier should match") + assert.True(t, IdentifiersMatch("https://nuts.nl", "https://nuts.nl/")) + assert.False(t, IdentifiersMatch("https://nuts.nl/oauth", "https://nuts.nl/other")) + assert.False(t, IdentifiersMatch("https://attacker.example", "https://nuts.nl")) +} + func TestWellKnownCandidates(t *testing.T) { t.Run("identifier with path returns insert then append", func(t *testing.T) { candidates, err := WellKnownCandidates("https://nuts.nl/iam/id", AuthzServerWellKnown, true) diff --git a/auth/openid4vci/client.go b/auth/openid4vci/client.go index 9ca4ccec83..7d18b25531 100644 --- a/auth/openid4vci/client.go +++ b/auth/openid4vci/client.go @@ -170,7 +170,7 @@ func (c *client) fetchIssuerMetadata(ctx context.Context, candidateURL string, i // Per §12.2.4: the credential_issuer value MUST match the issuer identifier // the metadata document was retrieved for. Mismatched metadata MUST NOT be used, // so a mismatch falls through to the next candidate. - if metadata.CredentialIssuer != issuerURL { + if !oauth.IdentifiersMatch(metadata.CredentialIssuer, issuerURL) { return nil, resp.StatusCode, fmt.Errorf("credential_issuer %q does not match requested issuer %q", metadata.CredentialIssuer, issuerURL) } return &metadata, resp.StatusCode, nil From 7295a16674d54cc94ac55c5278a01dc484a244e9 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 12 Jun 2026 08:32:02 +0200 Subject: [PATCH 3/9] refactor(auth): extract generic FetchMetadata helper for well-known discovery Replace the duplicated well-known metadata fetch-loops in iam/client.go and openid4vci/client.go with a single generic oauth.FetchMetadata[T]. Each metadata type provides its own well-known path via WellKnownPath() and its issuer via GetIssuer(); the helper derives the candidate URLs (insert + append placements), fetches the first match, validates the issuer, and lists the tried locations on failure. IdentifiersMatch is now package-internal. The signed-JWT OpenIDConfiguration keeps its own implementation. Assisted by AI --- auth/client/iam/client.go | 83 ++----------- auth/client/iam/client_test.go | 15 +-- auth/oauth/metadata.go | 147 ++++++++++++++++++++++ auth/oauth/metadata_test.go | 217 +++++++++++++++++++++++++++++++++ auth/oauth/types.go | 62 ++-------- auth/oauth/types_test.go | 46 +------ auth/openid4vci/client.go | 57 +-------- auth/openid4vci/client_test.go | 4 +- auth/openid4vci/types.go | 14 +++ 9 files changed, 409 insertions(+), 236 deletions(-) create mode 100644 auth/oauth/metadata.go create mode 100644 auth/oauth/metadata_test.go diff --git a/auth/client/iam/client.go b/auth/client/iam/client.go index 8e2beec0ce..1b965e4c7c 100644 --- a/auth/client/iam/client.go +++ b/auth/client/iam/client.go @@ -68,84 +68,19 @@ func (hb HTTPClient) OAuthAuthorizationServerMetadata(ctx context.Context, oauth // returns a matching document: // 1. insert (RFC 8414): https://host/.well-known/oauth-authorization-server/ // 2. append (OIDC Disc): https://host//.well-known/oauth-authorization-server - // 3. append openid-configuration: https://host//.well-known/openid-configuration // Many authorization servers publish metadata only under the append convention. - candidates, err := oauth.WellKnownCandidates(oauthIssuer, oauth.AuthzServerWellKnown, hb.strictMode) + metadata, err := oauth.FetchMetadata[oauth.AuthorizationServerMetadata](ctx, hb.httpClient, oauthIssuer, hb.strictMode) if err != nil { - return nil, err - } - // OpenID4VCI also permits retrieving AS metadata via OIDC Discovery's openid-configuration - // document; add its append form (the last candidate) as a final fallback. - oidcCandidates, err := oauth.WellKnownCandidates(oauthIssuer, oauth.OpenIdConfigurationWellKnown, hb.strictMode) - if err != nil { - return nil, err - } - candidates = append(candidates, oidcCandidates[len(oidcCandidates)-1]) - - var failures []candidateFailure - for _, candidate := range candidates { - var metadata oauth.AuthorizationServerMetadata - if getErr := hb.doGet(ctx, candidate, &metadata); getErr != nil { - failures = append(failures, candidateFailure{url: candidate, err: getErr, status: statusCodeOf(getErr)}) - continue - } - // Identifier-match check (RFC 8414 §3.3): the returned issuer MUST equal the - // requested identifier, so the fallback cannot be steered to a document the - // host serves under a different issuer. A mismatch falls through. - if !oauth.IdentifiersMatch(metadata.Issuer, oauthIssuer) { - failures = append(failures, candidateFailure{ - url: candidate, - err: fmt.Errorf("issuer %q does not match requested %q", metadata.Issuer, oauthIssuer), - }) - continue - } - return &metadata, nil - } - return nil, authorizationServerMetadataError(oauthIssuer, failures) -} - -// candidateFailure records why a single metadata candidate URL was rejected. -type candidateFailure struct { - url string - err error - status int -} - -// statusCodeOf extracts the HTTP status code from a core.HttpError, or 0 if the -// error is not an HTTP status error (e.g. a transport or decode failure). -func statusCodeOf(err error) int { - var httpErr core.HttpError - if errors.As(err, &httpErr) { - return httpErr.StatusCode - } - return 0 -} - -// authorizationServerMetadataError builds the error returned when every candidate -// was exhausted. It names the identifier and reports only the non-404 failures -// (a 404 just means "not at this location"). If any candidate failed with a -// >= 500 status, that failure is surfaced as a 502 Bad Gateway, preserving the -// existing severity mapping. -func authorizationServerMetadataError(identifier string, failures []candidateFailure) error { - var diagnostics []string - for i := range failures { - f := failures[i] - if f.status >= 500 { - // Map upstream server errors to 502 Bad Gateway, keeping the core.HttpError - // so callers can still assert on it. - if httpErr, ok := f.err.(core.HttpError); ok { - httpErr.StatusCode = http.StatusBadGateway - return httpErr - } - } - if f.status != http.StatusNotFound { - diagnostics = append(diagnostics, fmt.Sprintf("%s: %v", f.url, f.err)) + // An upstream server error (5xx) is surfaced as a core.HttpError; map it to 502 + // Bad Gateway. Any other failure is a bad client call. + httpErr, ok := errors.AsType[core.HttpError](err) + if ok && httpErr.StatusCode >= 500 { + httpErr.StatusCode = http.StatusBadGateway + return nil, httpErr } + return nil, errors.Join(ErrInvalidClientCall, err) } - if len(diagnostics) == 0 { - return errors.Join(ErrInvalidClientCall, fmt.Errorf("OAuth authorization server metadata not found at any candidate location for %q", identifier)) - } - return errors.Join(ErrInvalidClientCall, fmt.Errorf("failed to retrieve OAuth authorization server metadata for %q: %s", identifier, strings.Join(diagnostics, "; "))) + return metadata, nil } // ClientMetadata retrieves the client metadata from the client metadata endpoint given in the authorization request. diff --git a/auth/client/iam/client_test.go b/auth/client/iam/client_test.go index 419df7e765..b0740b378a 100644 --- a/auth/client/iam/client_test.go +++ b/auth/client/iam/client_test.go @@ -106,19 +106,6 @@ func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) { "/iam/123/.well-known/oauth-authorization-server", }, *requested) }) - t.Run("ok - openid-configuration as third candidate", func(t *testing.T) { - tlsServer, client, requested := metadataServer(t, http.StatusNotFound, "/iam/123", "/iam/123/.well-known/openid-configuration") - - metadata, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL+"/iam/123") - - require.NoError(t, err) - require.NotNil(t, metadata) - assert.Equal(t, []string{ - "/.well-known/oauth-authorization-server/iam/123", - "/iam/123/.well-known/oauth-authorization-server", - "/iam/123/.well-known/openid-configuration", - }, *requested) - }) t.Run("error - all candidates 404 yields a plain not-found error naming the identifier", func(t *testing.T) { tlsServer, client, requested := metadataServer(t, http.StatusNotFound, "/iam/123") @@ -128,7 +115,7 @@ func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) { assert.ErrorIs(t, err, ErrInvalidClientCall) assert.Contains(t, err.Error(), "not found at any candidate location") assert.Contains(t, err.Error(), tlsServer.URL+"/iam/123") - assert.Len(t, *requested, 3) + assert.Len(t, *requested, 2) }) t.Run("identifier mismatch on first candidate falls through to next", func(t *testing.T) { // Insert form returns 200 but with a non-matching issuer; append form serves the match. diff --git a/auth/oauth/metadata.go b/auth/oauth/metadata.go new file mode 100644 index 0000000000..4515dde512 --- /dev/null +++ b/auth/oauth/metadata.go @@ -0,0 +1,147 @@ +/* + * Copyright (C) 2026 Nuts community + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package oauth + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "strings" + + "github.com/nuts-foundation/nuts-node/core" +) + +// FetchMetadata retrieves and validates a JSON metadata document of type T for identifier. The +// well-known path is taken from T.WellKnownPath(); the document is tried in both placements in +// priority order and the first candidate that returns 200, JSON-decodes into T, and whose +// GetIssuer() matches identifier (a trailing slash difference is tolerated, see identifiersMatch) +// is returned: +// +// 1. insert (RFC 8414): https://host/.well-known// +// 2. append (OIDC Disc): https://host//.well-known/ +// +// When identifier has no path, insert and append collapse to a single URL. Every candidate +// shares identifier's scheme and host, so the single core.ParsePublicURL SSRF check on +// identifier covers them all. +// +// When every candidate fails it returns, in order of preference: an upstream server error +// (core.HttpError with a 5xx status) as-is so callers can map it to 502; otherwise an error +// listing the non-404 failures; otherwise a plain "not found" error naming the identifier. +func FetchMetadata[T interface { + WellKnownPath() string + GetIssuer() string +}](ctx context.Context, httpClient core.HTTPRequestDoer, identifier string, strictMode bool) (*T, error) { + var zero T + candidates, err := wellKnownCandidates(identifier, strictMode, zero.WellKnownPath()) + if err != nil { + return nil, err + } + // A 404 just means "not at this location" and is noise; only non-404 failures + // (403/405/5xx, decode errors, identifier mismatch) are worth surfacing. + var diagnostics []string + for _, candidate := range candidates { + metadata, status, fetchErr := fetchMetadataCandidate[T](ctx, httpClient, candidate, identifier) + if fetchErr == nil { + return metadata, nil + } + // Surface an upstream server error as-is (it is a core.HttpError carrying the + // status) so callers serving an API can map it to 502 Bad Gateway. + if status >= 500 { + return nil, fetchErr + } + if status != http.StatusNotFound { + diagnostics = append(diagnostics, fmt.Sprintf("%s: %v", candidate, fetchErr)) + } + } + if len(diagnostics) == 0 { + return nil, fmt.Errorf("metadata not found at any candidate location for %q (tried: %s)", identifier, strings.Join(candidates, ", ")) + } + return nil, fmt.Errorf("failed to retrieve metadata for %q: %s", identifier, strings.Join(diagnostics, "; ")) +} + +// fetchMetadataCandidate retrieves, JSON-decodes and validates the metadata document from a +// single candidate URL. It returns the HTTP status code (0 when no response was received) so the +// caller can distinguish a 404 from other failures and map upstream 5xx to 502. +func fetchMetadataCandidate[T interface{ GetIssuer() string }](ctx context.Context, httpClient core.HTTPRequestDoer, candidateURL string, identifier string) (*T, int, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, candidateURL, http.NoBody) + if err != nil { + return nil, 0, err + } + resp, err := httpClient.Do(req) + if err != nil { + return nil, 0, err + } + defer resp.Body.Close() + if httpErr := core.TestResponseCode(http.StatusOK, resp); httpErr != nil { + return nil, resp.StatusCode, httpErr + } + var metadata T + if err := json.NewDecoder(resp.Body).Decode(&metadata); err != nil { + return nil, resp.StatusCode, fmt.Errorf("decoding metadata: %w", err) + } + // The issuer in the document MUST match the requested identifier, so a host cannot + // steer discovery to metadata it serves under a different issuer. A mismatch falls + // through to the next candidate. + if !identifiersMatch(metadata.GetIssuer(), identifier) { + return nil, resp.StatusCode, fmt.Errorf("issuer %q does not match requested identifier %q", metadata.GetIssuer(), identifier) + } + return &metadata, resp.StatusCode, nil +} + +// identifiersMatch reports whether two issuer / credential-issuer identifiers are +// equal, tolerating a trailing-slash difference. RFC 8414 §3.3 and OpenID4VCI +// §12.2.4 require the identifier in a metadata document to be byte-identical to +// the requested identifier, but some servers (e.g. IdentityServer) normalize it +// with a trailing slash. Treat those as a match so discovery does not reject +// otherwise-valid metadata over a trailing slash. +func identifiersMatch(a string, b string) bool { + return strings.TrimRight(a, "/") == strings.TrimRight(b, "/") +} + +// wellKnownCandidates returns the metadata URLs to try for identifier, in priority order: +// the insert (RFC 8414) placement, then the append (OIDC Discovery) placement. When +// identifier has no path, both collapse to a single URL. +func wellKnownCandidates(identifier string, strictMode bool, wellKnown string) ([]string, error) { + identifierURL, err := core.ParsePublicURL(identifier, strictMode) + if err != nil { + return nil, err + } + // insert places the well-known segment at the authority root with the identifier path + // appended. RawPath is set alongside Path when present so url.String() does not re-escape + // pre-encoded characters like %2F. + insert := *identifierURL + if strings.Trim(identifierURL.Path, "/") == "" { + // No path: insert and append are identical; a single candidate suffices. + insert.Path = wellKnown + insert.RawPath = "" + return []string{insert.String()}, nil + } + insert.Path = wellKnown + identifierURL.Path + if identifierURL.RawPath != "" { + insert.RawPath = wellKnown + identifierURL.RawPath + } + // append places the well-known segment after the identifier path (OIDC Discovery). + appended := *identifierURL + appended.Path = strings.TrimSuffix(identifierURL.Path, "/") + wellKnown + if identifierURL.RawPath != "" { + appended.RawPath = strings.TrimSuffix(identifierURL.RawPath, "/") + wellKnown + } + return []string{insert.String(), appended.String()}, nil +} diff --git a/auth/oauth/metadata_test.go b/auth/oauth/metadata_test.go new file mode 100644 index 0000000000..41e0faffd4 --- /dev/null +++ b/auth/oauth/metadata_test.go @@ -0,0 +1,217 @@ +/* + * Copyright (C) 2026 Nuts community + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package oauth + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/nuts-foundation/nuts-node/core" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIdentifiersMatch(t *testing.T) { + assert.True(t, identifiersMatch("https://nuts.nl/oauth", "https://nuts.nl/oauth")) + assert.True(t, identifiersMatch("https://nuts.nl/oauth/", "https://nuts.nl/oauth"), "trailing slash on metadata issuer should match") + assert.True(t, identifiersMatch("https://nuts.nl/oauth", "https://nuts.nl/oauth/"), "trailing slash on requested identifier should match") + assert.True(t, identifiersMatch("https://nuts.nl", "https://nuts.nl/")) + assert.False(t, identifiersMatch("https://nuts.nl/oauth", "https://nuts.nl/other")) + assert.False(t, identifiersMatch("https://attacker.example", "https://nuts.nl")) +} + +func TestWellKnownCandidates(t *testing.T) { + t.Run("identifier with path returns insert then append", func(t *testing.T) { + candidates, err := wellKnownCandidates("https://nuts.nl/iam/id", true, AuthzServerWellKnown) + require.NoError(t, err) + assert.Equal(t, []string{ + "https://nuts.nl/.well-known/oauth-authorization-server/iam/id", + "https://nuts.nl/iam/id/.well-known/oauth-authorization-server", + }, candidates) + }) + t.Run("no path collapses to a single candidate", func(t *testing.T) { + candidates, err := wellKnownCandidates("https://nuts.nl", true, AuthzServerWellKnown) + require.NoError(t, err) + assert.Equal(t, []string{"https://nuts.nl/.well-known/oauth-authorization-server"}, candidates) + }) + t.Run("trailing-slash-only path collapses to a single candidate", func(t *testing.T) { + candidates, err := wellKnownCandidates("https://nuts.nl/", true, AuthzServerWellKnown) + require.NoError(t, err) + assert.Equal(t, []string{"https://nuts.nl/.well-known/oauth-authorization-server"}, candidates) + }) + t.Run("percent-encoded path is not double-escaped", func(t *testing.T) { + candidates, err := wellKnownCandidates("https://nuts.nl/foo%2Fbar", true, OpenIdCredIssuerWellKnown) + require.NoError(t, err) + assert.Equal(t, []string{ + "https://nuts.nl/.well-known/openid-credential-issuer/foo%2Fbar", + "https://nuts.nl/foo%2Fbar/.well-known/openid-credential-issuer", + }, candidates) + }) + t.Run("invalid identifier returns the SSRF/parse error", func(t *testing.T) { + candidates, err := wellKnownCandidates("http://nuts.nl/iam/id", true, AuthzServerWellKnown) + assert.ErrorContains(t, err, "scheme must be https") + assert.Nil(t, candidates) + }) +} + +func TestFetchMetadata(t *testing.T) { + ctx := context.Background() + + // metadataServer serves AuthorizationServerMetadata at the listed paths and returns + // missStatus for any other path. The served issuer equals the server URL plus issuerPath, + // so it matches an identifier of srv.URL+issuerPath. It records every requested path. + metadataServer := func(t *testing.T, missStatus int, issuerPath string, servedPaths ...string) (*httptest.Server, *[]string) { + var requested []string + served := make(map[string]struct{}, len(servedPaths)) + var issuer string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requested = append(requested, r.URL.Path) + if _, ok := served[r.URL.Path]; ok { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(AuthorizationServerMetadata{Issuer: issuer, TokenEndpoint: "/token"}) + return + } + w.WriteHeader(missStatus) + })) + t.Cleanup(srv.Close) + issuer = srv.URL + issuerPath + for _, p := range servedPaths { + served[p] = struct{}{} + } + return srv, &requested + } + + t.Run("ok - insert form, root identifier, single request", func(t *testing.T) { + srv, requested := metadataServer(t, http.StatusNotFound, "", "/.well-known/oauth-authorization-server") + + metadata, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), srv.URL, false) + + require.NoError(t, err) + require.NotNil(t, metadata) + assert.Equal(t, "/token", metadata.TokenEndpoint) + assert.Equal(t, []string{"/.well-known/oauth-authorization-server"}, *requested) + }) + t.Run("ok - append form when insert 404s", func(t *testing.T) { + srv, requested := metadataServer(t, http.StatusNotFound, "/iam/123", "/iam/123/.well-known/oauth-authorization-server") + + metadata, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), srv.URL+"/iam/123", false) + + require.NoError(t, err) + require.NotNil(t, metadata) + assert.Equal(t, []string{ + "/.well-known/oauth-authorization-server/iam/123", + "/iam/123/.well-known/oauth-authorization-server", + }, *requested) + }) + t.Run("identifier mismatch on first candidate falls through to next", func(t *testing.T) { + var issuer string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/.well-known/oauth-authorization-server/iam/123": + // 200 but with a non-matching issuer: must be rejected and fallen through. + _ = json.NewEncoder(w).Encode(AuthorizationServerMetadata{Issuer: "https://attacker.example", TokenEndpoint: "/evil"}) + case "/iam/123/.well-known/oauth-authorization-server": + _ = json.NewEncoder(w).Encode(AuthorizationServerMetadata{Issuer: issuer, TokenEndpoint: "/token"}) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(srv.Close) + issuer = srv.URL + "/iam/123" + + metadata, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), issuer, false) + + require.NoError(t, err) + require.NotNil(t, metadata) + assert.Equal(t, "/token", metadata.TokenEndpoint) + }) + t.Run("ok - issuer with a trailing slash matches the requested identifier", func(t *testing.T) { + var issuer string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/oauth/.well-known/oauth-authorization-server" { + w.WriteHeader(http.StatusNotFound) + return + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(AuthorizationServerMetadata{Issuer: issuer + "/", TokenEndpoint: "/token"}) + })) + t.Cleanup(srv.Close) + issuer = srv.URL + "/oauth" + + metadata, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), issuer, false) + + require.NoError(t, err) + require.NotNil(t, metadata) + assert.Equal(t, issuer+"/", metadata.Issuer) + }) + t.Run("error - all candidates 404 names the identifier and the tried locations", func(t *testing.T) { + srv, requested := metadataServer(t, http.StatusNotFound, "/iam/123") + + _, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), srv.URL+"/iam/123", false) + + require.Error(t, err) + assert.Contains(t, err.Error(), "not found at any candidate location") + assert.Contains(t, err.Error(), srv.URL+"/iam/123") + // Every tried location is listed in the error. + assert.Contains(t, err.Error(), srv.URL+"/.well-known/oauth-authorization-server/iam/123") + assert.Contains(t, err.Error(), srv.URL+"/iam/123/.well-known/oauth-authorization-server") + assert.Len(t, *requested, 2) + }) + t.Run("error - non-404 failure is reported with its status", func(t *testing.T) { + srv, _ := metadataServer(t, http.StatusForbidden, "/iam/123") + + _, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), srv.URL+"/iam/123", false) + + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to retrieve metadata") + assert.Contains(t, err.Error(), "403") + }) + t.Run("error - upstream 5xx is surfaced as a core.HttpError so callers can map it to 502", func(t *testing.T) { + srv, _ := metadataServer(t, http.StatusInternalServerError, "") + + _, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), srv.URL, false) + + require.Error(t, err) + var httpErr core.HttpError + require.ErrorAs(t, err, &httpErr) + assert.Equal(t, http.StatusInternalServerError, httpErr.StatusCode) + }) + t.Run("error - invalid JSON body", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte("{not valid json")) + })) + t.Cleanup(srv.Close) + + _, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), srv.URL, false) + + require.Error(t, err) + assert.Contains(t, err.Error(), "decoding metadata") + }) + t.Run("error - invalid identifier is rejected before any request", func(t *testing.T) { + _, err := FetchMetadata[AuthorizationServerMetadata](ctx, http.DefaultClient, "http://nuts.nl/iam/id", true) + + require.Error(t, err) + assert.ErrorContains(t, err, "scheme must be https") + }) +} diff --git a/auth/oauth/types.go b/auth/oauth/types.go index 025d192ec7..e1e3d8d580 100644 --- a/auth/oauth/types.go +++ b/auth/oauth/types.go @@ -22,7 +22,6 @@ package oauth import ( "encoding/json" "net/url" - "strings" "github.com/lestrrat-go/jwx/v2/jwk" "github.com/nuts-foundation/nuts-node/core" @@ -257,55 +256,6 @@ func IssuerIdToWellKnown(issuer string, wellKnown string, strictmode bool) (*url return issuerURL.Parse(wellKnown + issuerURL.EscapedPath()) } -// IdentifiersMatch reports whether two issuer / credential-issuer identifiers are -// equal, tolerating a trailing-slash difference. RFC 8414 §3.3 and OpenID4VCI -// §12.2.4 require the identifier in a metadata document to be byte-identical to -// the requested identifier, but some servers (e.g. IdentityServer) normalize it -// with a trailing slash. Treat those as a match so discovery does not reject -// otherwise-valid metadata over a trailing slash. -func IdentifiersMatch(a string, b string) bool { - return strings.TrimRight(a, "/") == strings.TrimRight(b, "/") -} - -// WellKnownCandidates returns the metadata URLs to try for the given identifier -// and well-known document, in priority order: -// -// 1. insert (RFC 8414): https://host/.well-known// -// 2. append (OIDC Disc): https://host//.well-known/ -// -// When the identifier has no path, both forms collapse to the same URL and a -// single candidate is returned. The caller fetches the candidates in order and -// takes the first that returns a usable metadata document. Each candidate shares -// the identifier's scheme and host, so the single core.ParsePublicURL SSRF check -// on the identifier covers them all. -func WellKnownCandidates(identifier string, wellKnown string, strictmode bool) ([]string, error) { - identifierURL, err := core.ParsePublicURL(identifier, strictmode) - if err != nil { - return nil, err - } - // insert form (RFC 8414): well-known segment at the authority root, identifier path appended. - // Set RawPath alongside Path when present so url.String() does not re-escape pre-encoded - // characters like %2F via EscapedPath's reescaping pass. - insert := *identifierURL - if strings.Trim(identifierURL.Path, "/") == "" { - // No path: insert and append are identical; a single candidate suffices. - insert.Path = wellKnown - insert.RawPath = "" - return []string{insert.String()}, nil - } - insert.Path = wellKnown + identifierURL.Path - if identifierURL.RawPath != "" { - insert.RawPath = wellKnown + identifierURL.RawPath - } - // append form (OIDC Discovery): well-known segment after the identifier path. - appended := *identifierURL - appended.Path = strings.TrimSuffix(identifierURL.Path, "/") + wellKnown - if identifierURL.RawPath != "" { - appended.RawPath = strings.TrimSuffix(identifierURL.RawPath, "/") + wellKnown - } - return []string{insert.String(), appended.String()}, nil -} - // AuthorizationServerMetadata defines the OAuth Authorization Server metadata. // Specified by https://www.rfc-editor.org/rfc/rfc8414.txt type AuthorizationServerMetadata struct { @@ -390,6 +340,18 @@ type AuthorizationServerMetadata struct { RequestObjectSigningAlgValuesSupported []string `json:"request_object_signing_alg_values_supported,omitempty"` } +// GetIssuer returns the authorization server's issuer identifier, for metadata +// discovery validation (see FetchMetadata). +func (m AuthorizationServerMetadata) GetIssuer() string { + return m.Issuer +} + +// WellKnownPath returns the well-known path under which this document is published +// (RFC 8414), used by FetchMetadata to derive the metadata URL. +func (m AuthorizationServerMetadata) WellKnownPath() string { + return AuthzServerWellKnown +} + // SupportsClientIDScheme checks if the Authorization Server supports the given client ID scheme. func (m AuthorizationServerMetadata) SupportsClientIDScheme(scheme string) bool { for _, method := range m.ClientIdSchemesSupported { diff --git a/auth/oauth/types_test.go b/auth/oauth/types_test.go index 4910589ce9..4aa4e8877f 100644 --- a/auth/oauth/types_test.go +++ b/auth/oauth/types_test.go @@ -20,10 +20,11 @@ package oauth import ( "encoding/json" + "testing" + "github.com/nuts-foundation/nuts-node/core/to" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "testing" ) func TestIssuerIdToWellKnown(t *testing.T) { @@ -67,49 +68,6 @@ func TestIssuerIdToWellKnown(t *testing.T) { }) } -func TestIdentifiersMatch(t *testing.T) { - assert.True(t, IdentifiersMatch("https://nuts.nl/oauth", "https://nuts.nl/oauth")) - assert.True(t, IdentifiersMatch("https://nuts.nl/oauth/", "https://nuts.nl/oauth"), "trailing slash on metadata issuer should match") - assert.True(t, IdentifiersMatch("https://nuts.nl/oauth", "https://nuts.nl/oauth/"), "trailing slash on requested identifier should match") - assert.True(t, IdentifiersMatch("https://nuts.nl", "https://nuts.nl/")) - assert.False(t, IdentifiersMatch("https://nuts.nl/oauth", "https://nuts.nl/other")) - assert.False(t, IdentifiersMatch("https://attacker.example", "https://nuts.nl")) -} - -func TestWellKnownCandidates(t *testing.T) { - t.Run("identifier with path returns insert then append", func(t *testing.T) { - candidates, err := WellKnownCandidates("https://nuts.nl/iam/id", AuthzServerWellKnown, true) - require.NoError(t, err) - assert.Equal(t, []string{ - "https://nuts.nl/.well-known/oauth-authorization-server/iam/id", - "https://nuts.nl/iam/id/.well-known/oauth-authorization-server", - }, candidates) - }) - t.Run("no path collapses to a single candidate", func(t *testing.T) { - candidates, err := WellKnownCandidates("https://nuts.nl", AuthzServerWellKnown, true) - require.NoError(t, err) - assert.Equal(t, []string{"https://nuts.nl/.well-known/oauth-authorization-server"}, candidates) - }) - t.Run("trailing-slash-only path collapses to a single candidate", func(t *testing.T) { - candidates, err := WellKnownCandidates("https://nuts.nl/", AuthzServerWellKnown, true) - require.NoError(t, err) - assert.Equal(t, []string{"https://nuts.nl/.well-known/oauth-authorization-server"}, candidates) - }) - t.Run("percent-encoded path is not double-escaped", func(t *testing.T) { - candidates, err := WellKnownCandidates("https://nuts.nl/foo%2Fbar", OpenIdCredIssuerWellKnown, true) - require.NoError(t, err) - assert.Equal(t, []string{ - "https://nuts.nl/.well-known/openid-credential-issuer/foo%2Fbar", - "https://nuts.nl/foo%2Fbar/.well-known/openid-credential-issuer", - }, candidates) - }) - t.Run("invalid identifier returns the SSRF/parse error", func(t *testing.T) { - candidates, err := WellKnownCandidates("http://nuts.nl/iam/id", AuthzServerWellKnown, true) - assert.ErrorContains(t, err, "scheme must be https") - assert.Nil(t, candidates) - }) -} - func TestTokenResponse_Marshalling(t *testing.T) { expected := (&TokenResponse{AccessToken: "1234567", TokenType: "bearer", ExpiresIn: to.Ptr(5), Scope: to.Ptr("abc"), DPoPKid: to.Ptr("kid")}).With("c_nonce", "hello") diff --git a/auth/openid4vci/client.go b/auth/openid4vci/client.go index 7d18b25531..43c51dde58 100644 --- a/auth/openid4vci/client.go +++ b/auth/openid4vci/client.go @@ -26,7 +26,6 @@ import ( "io" "net/http" "net/url" - "strings" "github.com/nuts-foundation/nuts-node/auth/oauth" "github.com/nuts-foundation/nuts-node/core" @@ -121,59 +120,13 @@ func (c *client) OpenIDCredentialIssuerMetadata(ctx context.Context, issuerURL s if parsed, _ := url.Parse(issuerURL); parsed != nil && (parsed.RawQuery != "" || parsed.Fragment != "") { return nil, fmt.Errorf("openid4vci: invalid issuer URL: query and fragment components are not allowed") } - // Try the well-known locations in priority order (insert per RFC 8414, then - // append per OIDC Discovery) and take the first that returns a matching - // document. Many issuers publish metadata only under the append convention. - candidates, err := oauth.WellKnownCandidates(issuerURL, oauth.OpenIdCredIssuerWellKnown, c.strictMode) + // Per §12.2.4 the credential_issuer in the document MUST match the requested issuer; + // FetchMetadata enforces that and tries the insert/append well-known placements. + metadata, err := oauth.FetchMetadata[OpenIDCredentialIssuerMetadata](ctx, c.httpClient, issuerURL, c.strictMode) if err != nil { - return nil, fmt.Errorf("openid4vci: invalid issuer URL: %w", err) + return nil, fmt.Errorf("openid4vci: %w", err) } - var failures []string - for _, candidate := range candidates { - metadata, status, fetchErr := c.fetchIssuerMetadata(ctx, candidate, issuerURL) - if fetchErr == nil { - return metadata, nil - } - // A 404 just means "not at this location" and is noise; only non-404 - // failures (403/405/5xx, decode errors, identifier mismatch) are worth - // surfacing when every candidate is exhausted. - if status != http.StatusNotFound { - failures = append(failures, fmt.Sprintf("%s: %v", candidate, fetchErr)) - } - } - if len(failures) == 0 { - return nil, fmt.Errorf("openid4vci: issuer metadata not found at any candidate location for %q", issuerURL) - } - return nil, fmt.Errorf("openid4vci: failed to retrieve issuer metadata for %q: %s", issuerURL, strings.Join(failures, "; ")) -} - -// fetchIssuerMetadata retrieves and validates the Credential Issuer Metadata from a -// single candidate URL. It returns the response status code (0 when no response was -// received) so the caller can distinguish a 404 from other failures. -func (c *client) fetchIssuerMetadata(ctx context.Context, candidateURL string, issuerURL string) (*OpenIDCredentialIssuerMetadata, int, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, candidateURL, http.NoBody) - if err != nil { - return nil, 0, err - } - resp, err := c.httpClient.Do(req) - if err != nil { - return nil, 0, err - } - defer resp.Body.Close() - if resp.StatusCode < 200 || resp.StatusCode > 299 { - return nil, resp.StatusCode, fmt.Errorf("fetching issuer metadata returned status %d", resp.StatusCode) - } - var metadata OpenIDCredentialIssuerMetadata - if err := json.NewDecoder(resp.Body).Decode(&metadata); err != nil { - return nil, resp.StatusCode, fmt.Errorf("decoding issuer metadata: %w", err) - } - // Per §12.2.4: the credential_issuer value MUST match the issuer identifier - // the metadata document was retrieved for. Mismatched metadata MUST NOT be used, - // so a mismatch falls through to the next candidate. - if !oauth.IdentifiersMatch(metadata.CredentialIssuer, issuerURL) { - return nil, resp.StatusCode, fmt.Errorf("credential_issuer %q does not match requested issuer %q", metadata.CredentialIssuer, issuerURL) - } - return &metadata, resp.StatusCode, nil + return metadata, nil } func (c *client) RequestNonce(ctx context.Context, nonceEndpoint string) (string, error) { diff --git a/auth/openid4vci/client_test.go b/auth/openid4vci/client_test.go index 942083a8f2..f5a1da75a0 100644 --- a/auth/openid4vci/client_test.go +++ b/auth/openid4vci/client_test.go @@ -144,7 +144,7 @@ func TestClient_OpenIDCredentialIssuerMetadata(t *testing.T) { client := NewClient(srv.Client(), false) _, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL) require.Error(t, err) - assert.Contains(t, err.Error(), "credential_issuer") + assert.Contains(t, err.Error(), "https://attacker.example") assert.Contains(t, err.Error(), "does not match") }) @@ -248,7 +248,7 @@ func TestClient_OpenIDCredentialIssuerMetadata(t *testing.T) { client := NewClient(srv.Client(), false) _, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL) require.Error(t, err) - assert.Contains(t, err.Error(), "decoding issuer metadata") + assert.Contains(t, err.Error(), "decoding metadata") }) } diff --git a/auth/openid4vci/types.go b/auth/openid4vci/types.go index c763664246..095d302a9e 100644 --- a/auth/openid4vci/types.go +++ b/auth/openid4vci/types.go @@ -32,6 +32,8 @@ package openid4vci import ( "encoding/json" + + "github.com/nuts-foundation/nuts-node/auth/oauth" ) // JWTTypeOpenID4VCIProof is the JWT typ claim value used in OpenID4VCI key @@ -50,6 +52,18 @@ type OpenIDCredentialIssuerMetadata struct { Display []map[string]string `json:"display,omitempty"` } +// GetIssuer returns the credential issuer identifier, for metadata discovery +// validation (see oauth.FetchMetadata). +func (m OpenIDCredentialIssuerMetadata) GetIssuer() string { + return m.CredentialIssuer +} + +// WellKnownPath returns the well-known path under which the Credential Issuer Metadata is +// published (§12.2), used by oauth.FetchMetadata to derive the metadata URL. +func (m OpenIDCredentialIssuerMetadata) WellKnownPath() string { + return oauth.OpenIdCredIssuerWellKnown +} + // NonceResponse is the body returned by the Nonce Endpoint (Section 7.2). type NonceResponse struct { CNonce string `json:"c_nonce"` From 1315a454d38802465d61ebd4b3c9e4b8e8e769ef Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Thu, 2 Jul 2026 11:44:21 +0200 Subject: [PATCH 4/9] fix(auth): try every metadata candidate before failing, drop issuer trailing-slash tolerance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on #4346: - FetchMetadata aborted on the first upstream 5xx instead of trying the remaining candidates, so a 500 on the insert-form URL could skip the append-form fallback entirely. It now always tries every candidate and joins their failures; a per-candidate core.HttpError is still recoverable via errors.AsType for the 502 mapping. - The issuer/credential_issuer match tolerated a trailing-slash difference, deviating from RFC 8414 §3.3 / OpenID4VCI §12.2.4's byte-exact comparison requirement. Dropped identifiersMatch in favor of a plain equality check. Assisted by AI --- auth/client/iam/client_test.go | 16 ++++---- auth/client/iam/openid4vp_test.go | 4 +- auth/oauth/metadata.go | 66 +++++++++++-------------------- auth/oauth/metadata_test.go | 32 ++++++++------- auth/openid4vci/client_test.go | 4 +- 5 files changed, 50 insertions(+), 72 deletions(-) diff --git a/auth/client/iam/client_test.go b/auth/client/iam/client_test.go index b0740b378a..d43af30785 100644 --- a/auth/client/iam/client_test.go +++ b/auth/client/iam/client_test.go @@ -106,14 +106,14 @@ func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) { "/iam/123/.well-known/oauth-authorization-server", }, *requested) }) - t.Run("error - all candidates 404 yields a plain not-found error naming the identifier", func(t *testing.T) { + t.Run("error - all candidates 404 names the identifier and the tried locations", func(t *testing.T) { tlsServer, client, requested := metadataServer(t, http.StatusNotFound, "/iam/123") _, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL+"/iam/123") require.Error(t, err) assert.ErrorIs(t, err, ErrInvalidClientCall) - assert.Contains(t, err.Error(), "not found at any candidate location") + assert.Contains(t, err.Error(), "failed to retrieve metadata") assert.Contains(t, err.Error(), tlsServer.URL+"/iam/123") assert.Len(t, *requested, 2) }) @@ -141,9 +141,8 @@ func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) { assert.Equal(t, "/token", metadata.TokenEndpoint) assert.Equal(t, issuer, metadata.Issuer) }) - t.Run("ok - metadata issuer with a trailing slash matches the requested identifier", func(t *testing.T) { - // Some servers (e.g. IdentityServer) normalize the issuer with a trailing slash; - // the append metadata is served with issuer = requested identifier + "/". + t.Run("error - metadata issuer with a trailing slash does not match the requested identifier", func(t *testing.T) { + // RFC 8414 §3.3 / OpenID4VCI §12.2.4 require a byte-exact comparison, no normalization. var issuer string handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/oauth/.well-known/oauth-authorization-server" { @@ -156,11 +155,10 @@ func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) { tlsServer, client := testServerAndClient(t, handler) issuer = tlsServer.URL + "/oauth" - metadata, err := client.OAuthAuthorizationServerMetadata(ctx, issuer) + _, err := client.OAuthAuthorizationServerMetadata(ctx, issuer) - require.NoError(t, err) - require.NotNil(t, metadata) - assert.Equal(t, issuer+"/", metadata.Issuer) + require.Error(t, err) + assert.ErrorIs(t, err, ErrInvalidClientCall) }) t.Run("error - non-404 status is preserved in the exhausted error", func(t *testing.T) { tlsServer, client, _ := metadataServer(t, http.StatusForbidden, "/iam/123") diff --git a/auth/client/iam/openid4vp_test.go b/auth/client/iam/openid4vp_test.go index 539b1b8a2b..c231d08552 100644 --- a/auth/client/iam/openid4vp_test.go +++ b/auth/client/iam/openid4vp_test.go @@ -234,7 +234,7 @@ func TestIAMClient_AuthorizationServerMetadata(t *testing.T) { require.Error(t, err) assert.ErrorIs(t, err, ErrInvalidClientCall) - assert.ErrorContains(t, err, "not found at any candidate location") + assert.ErrorContains(t, err, "failed to retrieve metadata") }) } @@ -382,7 +382,7 @@ func TestRelyingParty_RequestServiceAccessToken(t *testing.T) { require.Error(t, err) assert.ErrorIs(t, err, ErrInvalidClientCall) - assert.ErrorContains(t, err, "not found at any candidate location") + assert.ErrorContains(t, err, "failed to retrieve metadata") }) t.Run("error - faulty presentation definition", func(t *testing.T) { ctx := createClientServerTestContext(t) diff --git a/auth/oauth/metadata.go b/auth/oauth/metadata.go index 4515dde512..54f970c63c 100644 --- a/auth/oauth/metadata.go +++ b/auth/oauth/metadata.go @@ -21,6 +21,7 @@ package oauth import ( "context" "encoding/json" + "errors" "fmt" "net/http" "strings" @@ -31,8 +32,7 @@ import ( // FetchMetadata retrieves and validates a JSON metadata document of type T for identifier. The // well-known path is taken from T.WellKnownPath(); the document is tried in both placements in // priority order and the first candidate that returns 200, JSON-decodes into T, and whose -// GetIssuer() matches identifier (a trailing slash difference is tolerated, see identifiersMatch) -// is returned: +// GetIssuer() is identical to identifier is returned: // // 1. insert (RFC 8414): https://host/.well-known// // 2. append (OIDC Disc): https://host//.well-known/ @@ -41,9 +41,9 @@ import ( // shares identifier's scheme and host, so the single core.ParsePublicURL SSRF check on // identifier covers them all. // -// When every candidate fails it returns, in order of preference: an upstream server error -// (core.HttpError with a 5xx status) as-is so callers can map it to 502; otherwise an error -// listing the non-404 failures; otherwise a plain "not found" error naming the identifier. +// When every candidate fails, the returned error joins each candidate's failure. A per-candidate +// core.HttpError is preserved through the join (see errors.AsType), so callers can still detect +// an upstream 5xx and map it to 502. func FetchMetadata[T interface { WellKnownPath() string GetIssuer() string @@ -53,66 +53,44 @@ func FetchMetadata[T interface { if err != nil { return nil, err } - // A 404 just means "not at this location" and is noise; only non-404 failures - // (403/405/5xx, decode errors, identifier mismatch) are worth surfacing. - var diagnostics []string + var errs []error for _, candidate := range candidates { - metadata, status, fetchErr := fetchMetadataCandidate[T](ctx, httpClient, candidate, identifier) + metadata, fetchErr := fetchMetadataCandidate[T](ctx, httpClient, candidate, identifier) if fetchErr == nil { return metadata, nil } - // Surface an upstream server error as-is (it is a core.HttpError carrying the - // status) so callers serving an API can map it to 502 Bad Gateway. - if status >= 500 { - return nil, fetchErr - } - if status != http.StatusNotFound { - diagnostics = append(diagnostics, fmt.Sprintf("%s: %v", candidate, fetchErr)) - } + errs = append(errs, fmt.Errorf("%s: %w", candidate, fetchErr)) } - if len(diagnostics) == 0 { - return nil, fmt.Errorf("metadata not found at any candidate location for %q (tried: %s)", identifier, strings.Join(candidates, ", ")) - } - return nil, fmt.Errorf("failed to retrieve metadata for %q: %s", identifier, strings.Join(diagnostics, "; ")) + return nil, fmt.Errorf("failed to retrieve metadata for %q: %w", identifier, errors.Join(errs...)) } // fetchMetadataCandidate retrieves, JSON-decodes and validates the metadata document from a -// single candidate URL. It returns the HTTP status code (0 when no response was received) so the -// caller can distinguish a 404 from other failures and map upstream 5xx to 502. -func fetchMetadataCandidate[T interface{ GetIssuer() string }](ctx context.Context, httpClient core.HTTPRequestDoer, candidateURL string, identifier string) (*T, int, error) { +// single candidate URL. +func fetchMetadataCandidate[T interface{ GetIssuer() string }](ctx context.Context, httpClient core.HTTPRequestDoer, candidateURL string, identifier string) (*T, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, candidateURL, http.NoBody) if err != nil { - return nil, 0, err + return nil, err } resp, err := httpClient.Do(req) if err != nil { - return nil, 0, err + return nil, err } defer resp.Body.Close() if httpErr := core.TestResponseCode(http.StatusOK, resp); httpErr != nil { - return nil, resp.StatusCode, httpErr + return nil, httpErr } var metadata T if err := json.NewDecoder(resp.Body).Decode(&metadata); err != nil { - return nil, resp.StatusCode, fmt.Errorf("decoding metadata: %w", err) + return nil, fmt.Errorf("decoding metadata: %w", err) } - // The issuer in the document MUST match the requested identifier, so a host cannot - // steer discovery to metadata it serves under a different issuer. A mismatch falls - // through to the next candidate. - if !identifiersMatch(metadata.GetIssuer(), identifier) { - return nil, resp.StatusCode, fmt.Errorf("issuer %q does not match requested identifier %q", metadata.GetIssuer(), identifier) + // The issuer in the document MUST be identical to the requested identifier (RFC 8414 §3.3, + // OpenID4VCI §12.2.4: byte-comparison, no normalization), so a host cannot steer discovery + // to metadata it serves under a different issuer. A mismatch falls through to the next + // candidate. + if metadata.GetIssuer() != identifier { + return nil, fmt.Errorf("issuer %q does not match requested identifier %q", metadata.GetIssuer(), identifier) } - return &metadata, resp.StatusCode, nil -} - -// identifiersMatch reports whether two issuer / credential-issuer identifiers are -// equal, tolerating a trailing-slash difference. RFC 8414 §3.3 and OpenID4VCI -// §12.2.4 require the identifier in a metadata document to be byte-identical to -// the requested identifier, but some servers (e.g. IdentityServer) normalize it -// with a trailing slash. Treat those as a match so discovery does not reject -// otherwise-valid metadata over a trailing slash. -func identifiersMatch(a string, b string) bool { - return strings.TrimRight(a, "/") == strings.TrimRight(b, "/") + return &metadata, nil } // wellKnownCandidates returns the metadata URLs to try for identifier, in priority order: diff --git a/auth/oauth/metadata_test.go b/auth/oauth/metadata_test.go index 41e0faffd4..46966d7114 100644 --- a/auth/oauth/metadata_test.go +++ b/auth/oauth/metadata_test.go @@ -30,15 +30,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestIdentifiersMatch(t *testing.T) { - assert.True(t, identifiersMatch("https://nuts.nl/oauth", "https://nuts.nl/oauth")) - assert.True(t, identifiersMatch("https://nuts.nl/oauth/", "https://nuts.nl/oauth"), "trailing slash on metadata issuer should match") - assert.True(t, identifiersMatch("https://nuts.nl/oauth", "https://nuts.nl/oauth/"), "trailing slash on requested identifier should match") - assert.True(t, identifiersMatch("https://nuts.nl", "https://nuts.nl/")) - assert.False(t, identifiersMatch("https://nuts.nl/oauth", "https://nuts.nl/other")) - assert.False(t, identifiersMatch("https://attacker.example", "https://nuts.nl")) -} - func TestWellKnownCandidates(t *testing.T) { t.Run("identifier with path returns insert then append", func(t *testing.T) { candidates, err := wellKnownCandidates("https://nuts.nl/iam/id", true, AuthzServerWellKnown) @@ -145,7 +136,8 @@ func TestFetchMetadata(t *testing.T) { require.NotNil(t, metadata) assert.Equal(t, "/token", metadata.TokenEndpoint) }) - t.Run("ok - issuer with a trailing slash matches the requested identifier", func(t *testing.T) { + t.Run("error - issuer with a trailing slash does not match the requested identifier", func(t *testing.T) { + // RFC 8414 §3.3 / OpenID4VCI §12.2.4 require a byte-exact comparison, no normalization. var issuer string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/oauth/.well-known/oauth-authorization-server" { @@ -158,11 +150,10 @@ func TestFetchMetadata(t *testing.T) { t.Cleanup(srv.Close) issuer = srv.URL + "/oauth" - metadata, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), issuer, false) + _, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), issuer, false) - require.NoError(t, err) - require.NotNil(t, metadata) - assert.Equal(t, issuer+"/", metadata.Issuer) + require.Error(t, err) + assert.Contains(t, err.Error(), "does not match requested identifier") }) t.Run("error - all candidates 404 names the identifier and the tried locations", func(t *testing.T) { srv, requested := metadataServer(t, http.StatusNotFound, "/iam/123") @@ -170,7 +161,7 @@ func TestFetchMetadata(t *testing.T) { _, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), srv.URL+"/iam/123", false) require.Error(t, err) - assert.Contains(t, err.Error(), "not found at any candidate location") + assert.Contains(t, err.Error(), "failed to retrieve metadata") assert.Contains(t, err.Error(), srv.URL+"/iam/123") // Every tried location is listed in the error. assert.Contains(t, err.Error(), srv.URL+"/.well-known/oauth-authorization-server/iam/123") @@ -196,6 +187,17 @@ func TestFetchMetadata(t *testing.T) { require.ErrorAs(t, err, &httpErr) assert.Equal(t, http.StatusInternalServerError, httpErr.StatusCode) }) + t.Run("error - 5xx on every candidate is still surfaced as a core.HttpError after trying both", func(t *testing.T) { + srv, requested := metadataServer(t, http.StatusInternalServerError, "/iam/123") + + _, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), srv.URL+"/iam/123", false) + + require.Error(t, err) + var httpErr core.HttpError + require.ErrorAs(t, err, &httpErr) + assert.Equal(t, http.StatusInternalServerError, httpErr.StatusCode) + assert.Len(t, *requested, 2) + }) t.Run("error - invalid JSON body", func(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") diff --git a/auth/openid4vci/client_test.go b/auth/openid4vci/client_test.go index f5a1da75a0..a59e751241 100644 --- a/auth/openid4vci/client_test.go +++ b/auth/openid4vci/client_test.go @@ -195,7 +195,7 @@ func TestClient_OpenIDCredentialIssuerMetadata(t *testing.T) { assert.Equal(t, srv.URL+"/oauth2/alice", metadata.CredentialIssuer) }) - t.Run("all candidates 404 yields a plain not-found error naming the identifier", func(t *testing.T) { + t.Run("all candidates 404 names the identifier and the tried locations", func(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Error(w, "not found", http.StatusNotFound) })) @@ -204,7 +204,7 @@ func TestClient_OpenIDCredentialIssuerMetadata(t *testing.T) { client := NewClient(srv.Client(), false) _, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL+"/oauth2/alice") require.Error(t, err) - assert.Contains(t, err.Error(), "not found at any candidate location") + assert.Contains(t, err.Error(), "failed to retrieve metadata") assert.Contains(t, err.Error(), srv.URL+"/oauth2/alice") }) From e0dd192790b6695ad567a903682faa355f159d99 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Thu, 2 Jul 2026 11:55:22 +0200 Subject: [PATCH 5/9] fix(auth): don't overwrite upstream 5xx status with 502 Bad Gateway OAuthAuthorizationServerMetadata rewrote a core.HttpError's StatusCode field to 502 for any upstream 5xx, which discarded the actual upstream status (e.g. 503) and obfuscated the real failure. Return the error as-is instead. Assisted by AI --- auth/client/iam/client.go | 7 +++---- auth/client/iam/client_test.go | 8 ++++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/auth/client/iam/client.go b/auth/client/iam/client.go index 1b965e4c7c..1f709b77ea 100644 --- a/auth/client/iam/client.go +++ b/auth/client/iam/client.go @@ -71,12 +71,11 @@ func (hb HTTPClient) OAuthAuthorizationServerMetadata(ctx context.Context, oauth // Many authorization servers publish metadata only under the append convention. metadata, err := oauth.FetchMetadata[oauth.AuthorizationServerMetadata](ctx, hb.httpClient, oauthIssuer, hb.strictMode) if err != nil { - // An upstream server error (5xx) is surfaced as a core.HttpError; map it to 502 - // Bad Gateway. Any other failure is a bad client call. + // An upstream server error (5xx) is returned as-is, carrying its original status. + // Any other failure is a bad client call. httpErr, ok := errors.AsType[core.HttpError](err) if ok && httpErr.StatusCode >= 500 { - httpErr.StatusCode = http.StatusBadGateway - return nil, httpErr + return nil, err } return nil, errors.Join(ErrInvalidClientCall, err) } diff --git a/auth/client/iam/client_test.go b/auth/client/iam/client_test.go index d43af30785..d1370054b6 100644 --- a/auth/client/iam/client_test.go +++ b/auth/client/iam/client_test.go @@ -169,15 +169,15 @@ func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) { assert.ErrorIs(t, err, ErrInvalidClientCall) assert.Contains(t, err.Error(), "403") }) - t.Run("error - server error changes status code to 502", func(t *testing.T) { + t.Run("error - errors are returned as-is", func(t *testing.T) { tlsServer, client, _ := metadataServer(t, http.StatusInternalServerError, "") _, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL) require.Error(t, err) - httpErr, ok := err.(core.HttpError) - require.True(t, ok) - assert.Equal(t, http.StatusBadGateway, httpErr.StatusCode) + var httpErr core.HttpError + require.ErrorAs(t, err, &httpErr) + assert.Equal(t, http.StatusInternalServerError, httpErr.StatusCode) }) } From d8409eb4338ed73fc2fcfc112ee51d5b36cc84f7 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Thu, 2 Jul 2026 12:06:48 +0200 Subject: [PATCH 6/9] fix(auth): drop ErrInvalidClientCall wrapping, trim redundant status-code tests OAuthAuthorizationServerMetadata now returns FetchMetadata's error unchanged instead of classifying non-5xx failures as ErrInvalidClientCall (which mapped to 400 Bad Request) vs 5xx as an unclassified error. That distinction was already blurry (403s and identifier mismatches aren't really "bad client input"), and dropping it means every failure from this call now surfaces as the API's default 500. ErrInvalidClientCall had no other caller, so it and its dead entry in api.go's ResolveStatusCode map are removed. Also trimmed auth/oauth/metadata_test.go: FetchMetadata no longer branches on status code at all, so the 403-specific and single-candidate 5xx tests exercised the same code path as the 404 and two-candidate 5xx tests. Kept one representative case per behavior (exhaustion message format, and HttpError type surviving errors.Join). Assisted by AI --- auth/api/iam/api.go | 1 - auth/client/iam/client.go | 15 +-------------- auth/client/iam/client_test.go | 4 +--- auth/client/iam/openid4vp_test.go | 2 -- auth/oauth/metadata.go | 4 ++-- auth/oauth/metadata_test.go | 21 +-------------------- 6 files changed, 5 insertions(+), 42 deletions(-) diff --git a/auth/api/iam/api.go b/auth/api/iam/api.go index 76ed36fb3f..7c7f20315c 100644 --- a/auth/api/iam/api.go +++ b/auth/api/iam/api.go @@ -208,7 +208,6 @@ func (r Wrapper) ResolveStatusCode(err error) int { resolver.ErrDIDNotManagedByThisNode: http.StatusBadRequest, pe.ErrNoCredentials: http.StatusPreconditionFailed, didsubject.ErrSubjectNotFound: http.StatusNotFound, - iamclient.ErrInvalidClientCall: http.StatusBadRequest, iamclient.ErrBadGateway: http.StatusBadGateway, iamclient.ErrPreconditionFailed: http.StatusPreconditionFailed, }) diff --git a/auth/client/iam/client.go b/auth/client/iam/client.go index 1f709b77ea..8de7743260 100644 --- a/auth/client/iam/client.go +++ b/auth/client/iam/client.go @@ -41,9 +41,6 @@ import ( "github.com/nuts-foundation/nuts-node/vcr/pe" ) -// ErrInvalidClientCall is returned when the node makes a http call as client based on wrong information passed by the client. -var ErrInvalidClientCall = errors.New("invalid client call") - // ErrBadGateway is returned when the node makes a http call as client and the upstream returns an unexpected result. var ErrBadGateway = errors.New("upstream returned unexpected result") @@ -69,17 +66,7 @@ func (hb HTTPClient) OAuthAuthorizationServerMetadata(ctx context.Context, oauth // 1. insert (RFC 8414): https://host/.well-known/oauth-authorization-server/ // 2. append (OIDC Disc): https://host//.well-known/oauth-authorization-server // Many authorization servers publish metadata only under the append convention. - metadata, err := oauth.FetchMetadata[oauth.AuthorizationServerMetadata](ctx, hb.httpClient, oauthIssuer, hb.strictMode) - if err != nil { - // An upstream server error (5xx) is returned as-is, carrying its original status. - // Any other failure is a bad client call. - httpErr, ok := errors.AsType[core.HttpError](err) - if ok && httpErr.StatusCode >= 500 { - return nil, err - } - return nil, errors.Join(ErrInvalidClientCall, err) - } - return metadata, nil + return oauth.FetchMetadata[oauth.AuthorizationServerMetadata](ctx, hb.httpClient, oauthIssuer, hb.strictMode) } // ClientMetadata retrieves the client metadata from the client metadata endpoint given in the authorization request. diff --git a/auth/client/iam/client_test.go b/auth/client/iam/client_test.go index d1370054b6..048d66362d 100644 --- a/auth/client/iam/client_test.go +++ b/auth/client/iam/client_test.go @@ -112,7 +112,6 @@ func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) { _, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL+"/iam/123") require.Error(t, err) - assert.ErrorIs(t, err, ErrInvalidClientCall) assert.Contains(t, err.Error(), "failed to retrieve metadata") assert.Contains(t, err.Error(), tlsServer.URL+"/iam/123") assert.Len(t, *requested, 2) @@ -158,7 +157,7 @@ func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) { _, err := client.OAuthAuthorizationServerMetadata(ctx, issuer) require.Error(t, err) - assert.ErrorIs(t, err, ErrInvalidClientCall) + assert.Contains(t, err.Error(), "does not match requested identifier") }) t.Run("error - non-404 status is preserved in the exhausted error", func(t *testing.T) { tlsServer, client, _ := metadataServer(t, http.StatusForbidden, "/iam/123") @@ -166,7 +165,6 @@ func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) { _, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL+"/iam/123") require.Error(t, err) - assert.ErrorIs(t, err, ErrInvalidClientCall) assert.Contains(t, err.Error(), "403") }) t.Run("error - errors are returned as-is", func(t *testing.T) { diff --git a/auth/client/iam/openid4vp_test.go b/auth/client/iam/openid4vp_test.go index c231d08552..4e53d033cc 100644 --- a/auth/client/iam/openid4vp_test.go +++ b/auth/client/iam/openid4vp_test.go @@ -233,7 +233,6 @@ func TestIAMClient_AuthorizationServerMetadata(t *testing.T) { _, err := ctx.client.AuthorizationServerMetadata(context.Background(), ctx.tlsServer.URL) require.Error(t, err) - assert.ErrorIs(t, err, ErrInvalidClientCall) assert.ErrorContains(t, err, "failed to retrieve metadata") }) } @@ -381,7 +380,6 @@ func TestRelyingParty_RequestServiceAccessToken(t *testing.T) { _, err := ctx.client.RequestServiceAccessToken(context.Background(), subjectClientID, subjectID, ctx.verifierURL.String(), scopes, false, nil, nil, nil) require.Error(t, err) - assert.ErrorIs(t, err, ErrInvalidClientCall) assert.ErrorContains(t, err, "failed to retrieve metadata") }) t.Run("error - faulty presentation definition", func(t *testing.T) { diff --git a/auth/oauth/metadata.go b/auth/oauth/metadata.go index 54f970c63c..c322d5932a 100644 --- a/auth/oauth/metadata.go +++ b/auth/oauth/metadata.go @@ -42,8 +42,8 @@ import ( // identifier covers them all. // // When every candidate fails, the returned error joins each candidate's failure. A per-candidate -// core.HttpError is preserved through the join (see errors.AsType), so callers can still detect -// an upstream 5xx and map it to 502. +// core.HttpError stays recoverable through the join (see errors.AsType), so callers can still +// inspect the original upstream status. func FetchMetadata[T interface { WellKnownPath() string GetIssuer() string diff --git a/auth/oauth/metadata_test.go b/auth/oauth/metadata_test.go index 46966d7114..4139d4a63f 100644 --- a/auth/oauth/metadata_test.go +++ b/auth/oauth/metadata_test.go @@ -168,26 +168,7 @@ func TestFetchMetadata(t *testing.T) { assert.Contains(t, err.Error(), srv.URL+"/iam/123/.well-known/oauth-authorization-server") assert.Len(t, *requested, 2) }) - t.Run("error - non-404 failure is reported with its status", func(t *testing.T) { - srv, _ := metadataServer(t, http.StatusForbidden, "/iam/123") - - _, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), srv.URL+"/iam/123", false) - - require.Error(t, err) - assert.Contains(t, err.Error(), "failed to retrieve metadata") - assert.Contains(t, err.Error(), "403") - }) - t.Run("error - upstream 5xx is surfaced as a core.HttpError so callers can map it to 502", func(t *testing.T) { - srv, _ := metadataServer(t, http.StatusInternalServerError, "") - - _, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), srv.URL, false) - - require.Error(t, err) - var httpErr core.HttpError - require.ErrorAs(t, err, &httpErr) - assert.Equal(t, http.StatusInternalServerError, httpErr.StatusCode) - }) - t.Run("error - 5xx on every candidate is still surfaced as a core.HttpError after trying both", func(t *testing.T) { + t.Run("error - a candidate's core.HttpError stays recoverable through the join", func(t *testing.T) { srv, requested := metadataServer(t, http.StatusInternalServerError, "/iam/123") _, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), srv.URL+"/iam/123", false) From 25ba7cf3ea73ebce9f94d40e5888ca47e93a0a0f Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Thu, 2 Jul 2026 12:56:23 +0200 Subject: [PATCH 7/9] test(auth): trim wrapper-level tests duplicating oauth.FetchMetadata's suite OAuthAuthorizationServerMetadata and OpenIDCredentialIssuerMetadata are now thin wrappers around oauth.FetchMetadata with no branching of their own, but their test files kept re-testing the same insert/append fallback, identifier-match, and error-exhaustion behavior that auth/oauth/metadata_test.go already covers exhaustively. Trimmed each down to what's actually specific to the wrapper: correct wiring (well-known constant, httpClient, strictMode) and, for the iam client, a regression test for the earlier status-code-overwrite bug. Assisted by AI --- auth/client/iam/client_test.go | 85 ++-------------------------------- auth/openid4vci/client_test.go | 63 ++----------------------- 2 files changed, 8 insertions(+), 140 deletions(-) diff --git a/auth/client/iam/client_test.go b/auth/client/iam/client_test.go index 048d66362d..1425e6dc91 100644 --- a/auth/client/iam/client_test.go +++ b/auth/client/iam/client_test.go @@ -74,26 +74,10 @@ func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) { return tlsServer, client, &requested } - t.Run("ok - insert form, root identifier, single request", func(t *testing.T) { - tlsServer, client, requested := metadataServer(t, http.StatusNotFound, "", "/.well-known/oauth-authorization-server") - - metadata, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL) - - require.NoError(t, err) - require.NotNil(t, metadata) - assert.Equal(t, "/token", metadata.TokenEndpoint) - assert.Equal(t, []string{"/.well-known/oauth-authorization-server"}, *requested) - }) - t.Run("ok - insert form, identifier with path", func(t *testing.T) { - tlsServer, client, requested := metadataServer(t, http.StatusNotFound, "/iam/123", "/.well-known/oauth-authorization-server/iam/123") - - metadata, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL+"/iam/123") - - require.NoError(t, err) - require.NotNil(t, metadata) - // Insert form is tried first; no fallback request is made on the happy path. - assert.Equal(t, []string{"/.well-known/oauth-authorization-server/iam/123"}, *requested) - }) + // The insert/append fallback, identifier-match, and error-joining behavior is exhaustively + // covered by oauth.FetchMetadata's own tests; this wraps it with no extra logic, so these + // tests only need to confirm the wiring (well-known constant, httpClient, strictMode) and + // that this specific bug (rewriting an upstream status code) doesn't reappear. t.Run("ok - append form when insert 404s", func(t *testing.T) { tlsServer, client, requested := metadataServer(t, http.StatusNotFound, "/iam/123", "/iam/123/.well-known/oauth-authorization-server") @@ -106,67 +90,6 @@ func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) { "/iam/123/.well-known/oauth-authorization-server", }, *requested) }) - t.Run("error - all candidates 404 names the identifier and the tried locations", func(t *testing.T) { - tlsServer, client, requested := metadataServer(t, http.StatusNotFound, "/iam/123") - - _, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL+"/iam/123") - - require.Error(t, err) - assert.Contains(t, err.Error(), "failed to retrieve metadata") - assert.Contains(t, err.Error(), tlsServer.URL+"/iam/123") - assert.Len(t, *requested, 2) - }) - t.Run("identifier mismatch on first candidate falls through to next", func(t *testing.T) { - // Insert form returns 200 but with a non-matching issuer; append form serves the match. - var issuer string - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - switch r.URL.Path { - case "/.well-known/oauth-authorization-server/iam/123": - _ = json.NewEncoder(w).Encode(oauth.AuthorizationServerMetadata{Issuer: "https://attacker.example", TokenEndpoint: "/evil"}) - case "/iam/123/.well-known/oauth-authorization-server": - _ = json.NewEncoder(w).Encode(oauth.AuthorizationServerMetadata{Issuer: issuer, TokenEndpoint: "/token"}) - default: - w.WriteHeader(http.StatusNotFound) - } - }) - tlsServer, client := testServerAndClient(t, handler) - issuer = tlsServer.URL + "/iam/123" - - metadata, err := client.OAuthAuthorizationServerMetadata(ctx, issuer) - - require.NoError(t, err) - require.NotNil(t, metadata) - assert.Equal(t, "/token", metadata.TokenEndpoint) - assert.Equal(t, issuer, metadata.Issuer) - }) - t.Run("error - metadata issuer with a trailing slash does not match the requested identifier", func(t *testing.T) { - // RFC 8414 §3.3 / OpenID4VCI §12.2.4 require a byte-exact comparison, no normalization. - var issuer string - handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/oauth/.well-known/oauth-authorization-server" { - w.WriteHeader(http.StatusNotFound) - return - } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(oauth.AuthorizationServerMetadata{Issuer: issuer + "/", TokenEndpoint: "/token"}) - }) - tlsServer, client := testServerAndClient(t, handler) - issuer = tlsServer.URL + "/oauth" - - _, err := client.OAuthAuthorizationServerMetadata(ctx, issuer) - - require.Error(t, err) - assert.Contains(t, err.Error(), "does not match requested identifier") - }) - t.Run("error - non-404 status is preserved in the exhausted error", func(t *testing.T) { - tlsServer, client, _ := metadataServer(t, http.StatusForbidden, "/iam/123") - - _, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL+"/iam/123") - - require.Error(t, err) - assert.Contains(t, err.Error(), "403") - }) t.Run("error - errors are returned as-is", func(t *testing.T) { tlsServer, client, _ := metadataServer(t, http.StatusInternalServerError, "") diff --git a/auth/openid4vci/client_test.go b/auth/openid4vci/client_test.go index a59e751241..b14350197e 100644 --- a/auth/openid4vci/client_test.go +++ b/auth/openid4vci/client_test.go @@ -148,53 +148,10 @@ func TestClient_OpenIDCredentialIssuerMetadata(t *testing.T) { assert.Contains(t, err.Error(), "does not match") }) - t.Run("falls back to append form when insert form 404s", func(t *testing.T) { - var requested []string - var srv *httptest.Server - srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - requested = append(requested, r.URL.Path) - if r.URL.Path != "/oauth2/alice/.well-known/openid-credential-issuer" { - http.Error(w, "not found", http.StatusNotFound) - return - } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(OpenIDCredentialIssuerMetadata{CredentialIssuer: srv.URL + "/oauth2/alice"}) - })) - defer srv.Close() - - client := NewClient(srv.Client(), false) - metadata, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL+"/oauth2/alice") - require.NoError(t, err) - require.NotNil(t, metadata) - assert.Equal(t, []string{ - "/.well-known/openid-credential-issuer/oauth2/alice", - "/oauth2/alice/.well-known/openid-credential-issuer", - }, requested) - }) - - t.Run("identifier mismatch on first candidate falls through to next", func(t *testing.T) { - var srv *httptest.Server - srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - switch r.URL.Path { - case "/.well-known/openid-credential-issuer/oauth2/alice": - // 200 but with a non-matching credential_issuer: must be rejected and fallen through. - _ = json.NewEncoder(w).Encode(OpenIDCredentialIssuerMetadata{CredentialIssuer: "https://attacker.example/"}) - case "/oauth2/alice/.well-known/openid-credential-issuer": - _ = json.NewEncoder(w).Encode(OpenIDCredentialIssuerMetadata{CredentialIssuer: srv.URL + "/oauth2/alice"}) - default: - http.Error(w, "not found", http.StatusNotFound) - } - })) - defer srv.Close() - - client := NewClient(srv.Client(), false) - metadata, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL+"/oauth2/alice") - require.NoError(t, err) - require.NotNil(t, metadata) - assert.Equal(t, srv.URL+"/oauth2/alice", metadata.CredentialIssuer) - }) - + // The insert/append fallback, identifier-match, and error-joining behavior is exhaustively + // covered by oauth.FetchMetadata's own tests; this wraps it with no extra logic beyond the + // "openid4vci: " error prefix, so it's enough to confirm the wiring (well-known constant, + // httpClient, strictMode) and that prefix. t.Run("all candidates 404 names the identifier and the tried locations", func(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Error(w, "not found", http.StatusNotFound) @@ -208,18 +165,6 @@ func TestClient_OpenIDCredentialIssuerMetadata(t *testing.T) { assert.Contains(t, err.Error(), srv.URL+"/oauth2/alice") }) - t.Run("non-404 status is preserved in the exhausted error", func(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - http.Error(w, "forbidden", http.StatusForbidden) - })) - defer srv.Close() - - client := NewClient(srv.Client(), false) - _, err := client.OpenIDCredentialIssuerMetadata(context.Background(), srv.URL+"/oauth2/alice") - require.Error(t, err) - assert.Contains(t, err.Error(), "403") - }) - t.Run("rejects non-https issuer URL in strict mode", func(t *testing.T) { client := NewClient(http.DefaultClient, true) _, err := client.OpenIDCredentialIssuerMetadata(context.Background(), "http://issuer.example/") From 93246e1de34a77429c752d614370fd3e437fcd63 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Thu, 2 Jul 2026 13:07:48 +0200 Subject: [PATCH 8/9] feat(auth): reinstate ErrInvalidClientCall, scoped to all-4xx exhaustion FetchMetadata now tracks, while iterating candidates, whether every failure was a clean HTTP 4xx (as opposed to a 5xx, network failure, decode failure, or identifier mismatch). When it is, the returned error also wraps the new oauth.ErrAllCandidates4xx sentinel. iam.HTTPClient.OAuthAuthorizationServerMetadata uses that to reinstate ErrInvalidClientCall (400 Bad Request), but only for that narrow case: every candidate rejecting the request outright suggests the identifier itself is wrong. A 5xx, network error, or identifier mismatch still falls through to the API's default 500, since those aren't the client's fault. Assisted by AI --- auth/api/iam/api.go | 1 + auth/client/iam/client.go | 11 ++++++++++- auth/client/iam/client_test.go | 8 ++++++++ auth/oauth/metadata.go | 21 +++++++++++++++++++-- auth/oauth/metadata_test.go | 24 ++++++++++++++++++++++++ 5 files changed, 62 insertions(+), 3 deletions(-) diff --git a/auth/api/iam/api.go b/auth/api/iam/api.go index 7c7f20315c..76ed36fb3f 100644 --- a/auth/api/iam/api.go +++ b/auth/api/iam/api.go @@ -208,6 +208,7 @@ func (r Wrapper) ResolveStatusCode(err error) int { resolver.ErrDIDNotManagedByThisNode: http.StatusBadRequest, pe.ErrNoCredentials: http.StatusPreconditionFailed, didsubject.ErrSubjectNotFound: http.StatusNotFound, + iamclient.ErrInvalidClientCall: http.StatusBadRequest, iamclient.ErrBadGateway: http.StatusBadGateway, iamclient.ErrPreconditionFailed: http.StatusPreconditionFailed, }) diff --git a/auth/client/iam/client.go b/auth/client/iam/client.go index 8de7743260..8c4ef901bc 100644 --- a/auth/client/iam/client.go +++ b/auth/client/iam/client.go @@ -41,6 +41,9 @@ import ( "github.com/nuts-foundation/nuts-node/vcr/pe" ) +// ErrInvalidClientCall is returned when the node makes a http call as client based on wrong information passed by the client. +var ErrInvalidClientCall = errors.New("invalid client call") + // ErrBadGateway is returned when the node makes a http call as client and the upstream returns an unexpected result. var ErrBadGateway = errors.New("upstream returned unexpected result") @@ -66,7 +69,13 @@ func (hb HTTPClient) OAuthAuthorizationServerMetadata(ctx context.Context, oauth // 1. insert (RFC 8414): https://host/.well-known/oauth-authorization-server/ // 2. append (OIDC Disc): https://host//.well-known/oauth-authorization-server // Many authorization servers publish metadata only under the append convention. - return oauth.FetchMetadata[oauth.AuthorizationServerMetadata](ctx, hb.httpClient, oauthIssuer, hb.strictMode) + metadata, err := oauth.FetchMetadata[oauth.AuthorizationServerMetadata](ctx, hb.httpClient, oauthIssuer, hb.strictMode) + if err != nil && errors.Is(err, oauth.ErrAllCandidates4xx) { + // Every candidate rejected the request outright (no 5xx, no network/decode failure, no + // identifier mismatch): the identifier itself is most likely wrong. + return nil, errors.Join(ErrInvalidClientCall, err) + } + return metadata, err } // ClientMetadata retrieves the client metadata from the client metadata endpoint given in the authorization request. diff --git a/auth/client/iam/client_test.go b/auth/client/iam/client_test.go index 1425e6dc91..f9c89dddaa 100644 --- a/auth/client/iam/client_test.go +++ b/auth/client/iam/client_test.go @@ -100,6 +100,14 @@ func TestHTTPClient_OAuthAuthorizationServerMetadata(t *testing.T) { require.ErrorAs(t, err, &httpErr) assert.Equal(t, http.StatusInternalServerError, httpErr.StatusCode) }) + t.Run("error - all candidates 4xx classifies as ErrInvalidClientCall", func(t *testing.T) { + tlsServer, client, _ := metadataServer(t, http.StatusNotFound, "/iam/123") + + _, err := client.OAuthAuthorizationServerMetadata(ctx, tlsServer.URL+"/iam/123") + + require.Error(t, err) + assert.ErrorIs(t, err, ErrInvalidClientCall) + }) } func TestHTTPClient_PresentationDefinition(t *testing.T) { diff --git a/auth/oauth/metadata.go b/auth/oauth/metadata.go index c322d5932a..5f4d5ffd75 100644 --- a/auth/oauth/metadata.go +++ b/auth/oauth/metadata.go @@ -29,6 +29,12 @@ import ( "github.com/nuts-foundation/nuts-node/core" ) +// ErrAllCandidates4xx indicates every well-known candidate for a FetchMetadata call responded +// with an HTTP 4xx status. It typically means the requested identifier doesn't correspond to a +// real issuer/AS, rather than the server being down or misconfigured; callers may use it to +// classify the failure as a client error instead of the default server error. +var ErrAllCandidates4xx = errors.New("metadata not found: every candidate returned a client error") + // FetchMetadata retrieves and validates a JSON metadata document of type T for identifier. The // well-known path is taken from T.WellKnownPath(); the document is tried in both placements in // priority order and the first candidate that returns 200, JSON-decodes into T, and whose @@ -43,7 +49,9 @@ import ( // // When every candidate fails, the returned error joins each candidate's failure. A per-candidate // core.HttpError stays recoverable through the join (see errors.AsType), so callers can still -// inspect the original upstream status. +// inspect the original upstream status. If every candidate responded with an HTTP 4xx status (as +// opposed to a 5xx, network failure, decode failure, or identifier mismatch), the returned error +// also wraps ErrAllCandidates4xx. func FetchMetadata[T interface { WellKnownPath() string GetIssuer() string @@ -54,14 +62,23 @@ func FetchMetadata[T interface { return nil, err } var errs []error + allClientError := true for _, candidate := range candidates { metadata, fetchErr := fetchMetadataCandidate[T](ctx, httpClient, candidate, identifier) if fetchErr == nil { return metadata, nil } + var httpErr core.HttpError + if !errors.As(fetchErr, &httpErr) || httpErr.StatusCode < 400 || httpErr.StatusCode >= 500 { + allClientError = false + } errs = append(errs, fmt.Errorf("%s: %w", candidate, fetchErr)) } - return nil, fmt.Errorf("failed to retrieve metadata for %q: %w", identifier, errors.Join(errs...)) + joined := fmt.Errorf("failed to retrieve metadata for %q: %w", identifier, errors.Join(errs...)) + if allClientError { + return nil, errors.Join(ErrAllCandidates4xx, joined) + } + return nil, joined } // fetchMetadataCandidate retrieves, JSON-decodes and validates the metadata document from a diff --git a/auth/oauth/metadata_test.go b/auth/oauth/metadata_test.go index 4139d4a63f..ea9692a25d 100644 --- a/auth/oauth/metadata_test.go +++ b/auth/oauth/metadata_test.go @@ -167,6 +167,8 @@ func TestFetchMetadata(t *testing.T) { assert.Contains(t, err.Error(), srv.URL+"/.well-known/oauth-authorization-server/iam/123") assert.Contains(t, err.Error(), srv.URL+"/iam/123/.well-known/oauth-authorization-server") assert.Len(t, *requested, 2) + // Every candidate was a clean 4xx: callers may classify this as a client error. + assert.ErrorIs(t, err, ErrAllCandidates4xx) }) t.Run("error - a candidate's core.HttpError stays recoverable through the join", func(t *testing.T) { srv, requested := metadataServer(t, http.StatusInternalServerError, "/iam/123") @@ -178,6 +180,28 @@ func TestFetchMetadata(t *testing.T) { require.ErrorAs(t, err, &httpErr) assert.Equal(t, http.StatusInternalServerError, httpErr.StatusCode) assert.Len(t, *requested, 2) + // A 5xx isn't the client's fault, so it must not be classified as one. + assert.NotErrorIs(t, err, ErrAllCandidates4xx) + }) + t.Run("error - identifier mismatch does not count as a client error", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/oauth-authorization-server/iam/123": + w.WriteHeader(http.StatusNotFound) + case "/iam/123/.well-known/oauth-authorization-server": + // 200 but with a non-matching issuer: not an HTTP error status at all. + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(AuthorizationServerMetadata{Issuer: "https://attacker.example"}) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(srv.Close) + + _, err := FetchMetadata[AuthorizationServerMetadata](ctx, srv.Client(), srv.URL+"/iam/123", false) + + require.Error(t, err) + assert.NotErrorIs(t, err, ErrAllCandidates4xx) }) t.Run("error - invalid JSON body", func(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From 4dbe5869ffcaab3431facc5fad85aee2c513ddf6 Mon Sep 17 00:00:00 2001 From: Joris Scharp Date: Fri, 3 Jul 2026 14:52:59 +0200 Subject: [PATCH 9/9] fix(auth): strip terminating slash before inserting well-known segment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 8414 §3.1 requires any terminating "/" on the issuer path to be removed before the well-known segment is inserted. The insert candidate kept it, yielding .../id//... ; the append candidate already trimmed. Trim both Path and RawPath so the two placements stay consistent. --- auth/oauth/metadata.go | 5 +++-- auth/oauth/metadata_test.go | 10 ++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/auth/oauth/metadata.go b/auth/oauth/metadata.go index 5f4d5ffd75..c936ae4227 100644 --- a/auth/oauth/metadata.go +++ b/auth/oauth/metadata.go @@ -128,9 +128,10 @@ func wellKnownCandidates(identifier string, strictMode bool, wellKnown string) ( insert.RawPath = "" return []string{insert.String()}, nil } - insert.Path = wellKnown + identifierURL.Path + // RFC 8414 §3.1: any terminating "/" MUST be removed before inserting the well-known segment. + insert.Path = wellKnown + strings.TrimSuffix(identifierURL.Path, "/") if identifierURL.RawPath != "" { - insert.RawPath = wellKnown + identifierURL.RawPath + insert.RawPath = wellKnown + strings.TrimSuffix(identifierURL.RawPath, "/") } // append places the well-known segment after the identifier path (OIDC Discovery). appended := *identifierURL diff --git a/auth/oauth/metadata_test.go b/auth/oauth/metadata_test.go index ea9692a25d..133bf8971f 100644 --- a/auth/oauth/metadata_test.go +++ b/auth/oauth/metadata_test.go @@ -49,6 +49,16 @@ func TestWellKnownCandidates(t *testing.T) { require.NoError(t, err) assert.Equal(t, []string{"https://nuts.nl/.well-known/oauth-authorization-server"}, candidates) }) + t.Run("terminating slash on the path is removed in both candidates", func(t *testing.T) { + // RFC 8414 §3.1 / OIDC Discovery §4.1: any terminating "/" MUST be removed + // before inserting/appending the well-known segment. + candidates, err := wellKnownCandidates("https://nuts.nl/iam/id/", true, AuthzServerWellKnown) + require.NoError(t, err) + assert.Equal(t, []string{ + "https://nuts.nl/.well-known/oauth-authorization-server/iam/id", + "https://nuts.nl/iam/id/.well-known/oauth-authorization-server", + }, candidates) + }) t.Run("percent-encoded path is not double-escaped", func(t *testing.T) { candidates, err := wellKnownCandidates("https://nuts.nl/foo%2Fbar", true, OpenIdCredIssuerWellKnown) require.NoError(t, err)