From a26a8dfc8dd9bb61ec1fe8fd1a0f822197182bda Mon Sep 17 00:00:00 2001 From: Juzer Patanwala Date: Fri, 29 May 2026 11:13:18 +0530 Subject: [PATCH 1/2] =?UTF-8?q?dcr:=20support=20RFC=208414=20=C2=A73.1=20p?= =?UTF-8?q?ath-insertion=20in=20discovery-URL=20=E2=86=92=20issuer=20deriv?= =?UTF-8?q?ation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `deriveExpectedIssuerFromDiscoveryURL` recovers the issuer the upstream is expected to claim in its discovery document. It already handled the suffix-append form (e.g. https://mcp.atlassian.com/.well-known/oauth-authorization-server → https://mcp.atlassian.com) and the issuer-suffix multi-tenant style (.../tenants/acme/.well-known/openid-configuration → .../tenants/acme), but the comment block explicitly opted out of the RFC 8414 §3.1 path-insertion form — operators on that pattern had to fall back to `dcr_config.registration_endpoint` to bypass discovery entirely. That gap rejects providers that publish a path-component issuer per the letter of the RFC. Datadog's MCP authorization server is one such provider: its discovery URL `https://mcp.us5.datadoghq.com/.well-known/oauth-authorization-server/v1/mcp` declares issuer `https://mcp.us5.datadoghq.com/v1/mcp`, and DCR discovery aborts with: issuer mismatch (RFC 8414 §3.3): expected "https://mcp.us5.datadoghq.com", got "https://mcp.us5.datadoghq.com/v1/mcp" Recognise the path-insertion form by checking for the well-known segment as a path *prefix* followed by a tenant path (HasPrefix(path, suffix+"/")), trimming just the well-known segment to recover origin + tenant path. Disambiguated from the existing suffix-append case by position: the well-known segment at the end of the path is suffix-append; at the start with more path following is path-insertion. The two cases cannot both match a single URL. Tests cover the new branch for both the OAuth and OIDC suffix variants plus a multi-segment tenant. All existing cases continue to pass. Per RFC 8414 §3 (the well-known URI is formed by inserting the well-known suffix between host and path of the issuer) and RFC 8615 (well-known URI conventions). Signed-off-by: Juzer Patanwala --- pkg/auth/dcr/resolver.go | 40 +++++++++++++++++++++++++---------- pkg/auth/dcr/resolver_test.go | 20 ++++++++++++++++++ 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/pkg/auth/dcr/resolver.go b/pkg/auth/dcr/resolver.go index 2b1b743dfc..ca49802cc1 100644 --- a/pkg/auth/dcr/resolver.go +++ b/pkg/auth/dcr/resolver.go @@ -899,24 +899,31 @@ func resolveDCREndpoints( // upstream is expected to claim in its RFC 8414 / OIDC Discovery document, // given an operator-configured DiscoveryURL. // -// Two recognised conventions: +// Three recognised conventions: // // 1. Well-known suffix: the URL ends with /.well-known/oauth-authorization-server // or /.well-known/openid-configuration. The suffix is stripped to recover // the issuer; this covers single-tenant providers (e.g. // https://mcp.atlassian.com/.well-known/oauth-authorization-server → -// https://mcp.atlassian.com) and the issuer-suffix multi-tenant style -// (e.g. https://idp.example.com/tenants/acme/.well-known/openid-configuration +// https://mcp.atlassian.com) and the OIDC-style suffix-append shape used +// by some multi-tenant providers (e.g. +// https://idp.example.com/tenants/acme/.well-known/openid-configuration // → https://idp.example.com/tenants/acme). -// 2. Non-well-known path: the URL points at a custom metadata endpoint that -// does not end in either suffix. Origin (scheme://host) is used as a -// best-effort fallback; this matches the common shape where the upstream -// issuer is the host root. +// 2. RFC 8414 §3.1 path-insertion: the well-known path is inserted BETWEEN +// the host and the issuer's tenant path. Per RFC 8414 §3 / RFC 8615, +// this is the canonical form for issuers with a path component, e.g. +// issuer https://example.com/v1/mcp → +// discovery URL https://example.com/.well-known/oauth-authorization-server/v1/mcp. +// The tenant suffix that appears AFTER the well-known segment is +// re-attached to the origin to recover the issuer. +// 3. Non-well-known path: the URL points at a custom metadata endpoint that +// contains neither suffix in a recognisable position. Origin +// (scheme://host) is used as a best-effort fallback; this matches the +// common shape where the upstream issuer is the host root. // -// RFC 8414 §3.1's path-aware form (well-known path inserted between host and -// tenant path, e.g. https://example.com/.well-known/oauth-authorization-server/tenant) -// is not auto-detected here — operators on that pattern can switch to -// dcr_config.registration_endpoint to bypass discovery. +// Case (1) and case (2) are disambiguated by where the well-known segment +// sits in the path: at the end ⇒ suffix-append, immediately after the host +// with more path following ⇒ path-insertion. func deriveExpectedIssuerFromDiscoveryURL(discoveryURL string) (string, error) { const ( oauthSuffix = "/.well-known/oauth-authorization-server" @@ -932,10 +939,21 @@ func deriveExpectedIssuerFromDiscoveryURL(discoveryURL string) (string, error) { } switch { + // Suffix-append form (case 1): well-known segment at end of path. case strings.HasSuffix(u.Path, oauthSuffix): u.Path = strings.TrimSuffix(u.Path, oauthSuffix) case strings.HasSuffix(u.Path, oidcSuffix): u.Path = strings.TrimSuffix(u.Path, oidcSuffix) + // RFC 8414 §3.1 path-insertion form (case 2): well-known segment at the + // start of the path with tenant path following. Strip just the well-known + // segment to recover {origin}{tenant-path}. Note the "/" guard — a bare + // suffix (e.g. /.well-known/oauth-authorization-server) is already covered + // by the suffix cases above; we only enter this branch when there is a + // tenant path after the well-known segment. + case strings.HasPrefix(u.Path, oauthSuffix+"/"): + u.Path = strings.TrimPrefix(u.Path, oauthSuffix) + case strings.HasPrefix(u.Path, oidcSuffix+"/"): + u.Path = strings.TrimPrefix(u.Path, oidcSuffix) default: // Custom (non-well-known) discovery URL — fall back to origin. u.Path = "" diff --git a/pkg/auth/dcr/resolver_test.go b/pkg/auth/dcr/resolver_test.go index 57e66bb4d2..95cd1a9685 100644 --- a/pkg/auth/dcr/resolver_test.go +++ b/pkg/auth/dcr/resolver_test.go @@ -962,6 +962,26 @@ func TestDeriveExpectedIssuerFromDiscoveryURL(t *testing.T) { discoveryURL: "https://idp.example.com/tenants/acme/.well-known/openid-configuration", want: "https://idp.example.com/tenants/acme", }, + { + // RFC 8414 §3.1 path-insertion: well-known segment between host + // and tenant path. Issuer reconstructed as origin + tenant path. + // Matches the shape served by Datadog's MCP authorization server + // (mcp.us5.datadoghq.com) and any other provider with a path- + // component issuer that follows the RFC literally. + name: "oauth well-known path-insertion (RFC 8414 §3.1)", + discoveryURL: "https://mcp.us5.datadoghq.com/.well-known/oauth-authorization-server/v1/mcp", + want: "https://mcp.us5.datadoghq.com/v1/mcp", + }, + { + name: "oauth well-known path-insertion with multi-segment tenant", + discoveryURL: "https://idp.example.com/.well-known/oauth-authorization-server/tenants/acme", + want: "https://idp.example.com/tenants/acme", + }, + { + name: "oidc well-known path-insertion", + discoveryURL: "https://idp.example.com/.well-known/openid-configuration/tenants/acme", + want: "https://idp.example.com/tenants/acme", + }, { name: "non-well-known path falls back to origin", discoveryURL: "https://idp.example.com/tenants/acme/metadata", From 6d1d7cc7886e6109e7591fd1b0b63be0664d3a46 Mon Sep 17 00:00:00 2001 From: Juzer Patanwala Date: Mon, 1 Jun 2026 13:14:57 +0530 Subject: [PATCH 2/2] dcr: normalise trailing-slash bare well-known to origin (review feedback) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per @tgrunnagle's review on #5395: the path-insertion arms introduced in the previous commit accidentally regress one edge case. For an input where the path ends `/.well-known/oauth-authorization-server/` (trailing slash, no tenant), the suffix arms don't match (suffix test sees the trailing "/"), so the HasPrefix arm fires and TrimPrefix leaves `u.Path = "/"` → spurious issuer `https://host/` that fails the §3.3 byte-equality check against the upstream's declared `https://host`. Before this PR the same input hit the `default` arm and produced `https://host` correctly. Fix: after TrimPrefix in each path-insertion arm, collapse a lone `/` back to empty so the trailing-slash form converges on the same origin issuer the bare-suffix and `default` arms produce. Tightens the inline comment to describe both shapes that reach the HasPrefix arm (real tenant suffix and trailing-slash). Adds two table-driven cases — one each for the oauth and oidc trailing-slash forms — to lock in the expected origin output. Signed-off-by: Juzer Patanwala --- pkg/auth/dcr/resolver.go | 23 +++++++++++++++++++---- pkg/auth/dcr/resolver_test.go | 15 +++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/pkg/auth/dcr/resolver.go b/pkg/auth/dcr/resolver.go index ca49802cc1..6ffba819f0 100644 --- a/pkg/auth/dcr/resolver.go +++ b/pkg/auth/dcr/resolver.go @@ -946,14 +946,29 @@ func deriveExpectedIssuerFromDiscoveryURL(discoveryURL string) (string, error) { u.Path = strings.TrimSuffix(u.Path, oidcSuffix) // RFC 8414 §3.1 path-insertion form (case 2): well-known segment at the // start of the path with tenant path following. Strip just the well-known - // segment to recover {origin}{tenant-path}. Note the "/" guard — a bare - // suffix (e.g. /.well-known/oauth-authorization-server) is already covered - // by the suffix cases above; we only enter this branch when there is a - // tenant path after the well-known segment. + // segment to recover {origin}{tenant-path}. + // + // Two shapes hit this branch: + // 1. A real tenant suffix follows the well-known segment, e.g. + // /.well-known/oauth-authorization-server/v1/mcp → + // issuer https://host/v1/mcp. + // 2. A trailing slash with no tenant, e.g. + // /.well-known/oauth-authorization-server/ — TrimPrefix leaves + // a stray "/", which would yield a spurious issuer + // "https://host/" that fails the §3.3 byte-equality check + // against the upstream's declared "https://host". Normalise + // that stray "/" back to empty so case (2.2) and the bare + // suffix case derive the same origin issuer. case strings.HasPrefix(u.Path, oauthSuffix+"/"): u.Path = strings.TrimPrefix(u.Path, oauthSuffix) + if u.Path == "/" { + u.Path = "" + } case strings.HasPrefix(u.Path, oidcSuffix+"/"): u.Path = strings.TrimPrefix(u.Path, oidcSuffix) + if u.Path == "/" { + u.Path = "" + } default: // Custom (non-well-known) discovery URL — fall back to origin. u.Path = "" diff --git a/pkg/auth/dcr/resolver_test.go b/pkg/auth/dcr/resolver_test.go index 95cd1a9685..2c163e110d 100644 --- a/pkg/auth/dcr/resolver_test.go +++ b/pkg/auth/dcr/resolver_test.go @@ -982,6 +982,21 @@ func TestDeriveExpectedIssuerFromDiscoveryURL(t *testing.T) { discoveryURL: "https://idp.example.com/.well-known/openid-configuration/tenants/acme", want: "https://idp.example.com/tenants/acme", }, + { + // Trailing-slash edge case: hits the HasPrefix arm (path doesn't end + // at the bare suffix) but has no tenant after it. Without the empty- + // path normalisation, TrimPrefix would leave a stray "/" and produce + // a spurious "https://host/" that fails the RFC 8414 §3.3 byte + // equality check. + name: "oauth well-known with trailing slash normalises to origin", + discoveryURL: "https://mcp.example.com/.well-known/oauth-authorization-server/", + want: "https://mcp.example.com", + }, + { + name: "oidc well-known with trailing slash normalises to origin", + discoveryURL: "https://idp.example.com/.well-known/openid-configuration/", + want: "https://idp.example.com", + }, { name: "non-well-known path falls back to origin", discoveryURL: "https://idp.example.com/tenants/acme/metadata",