Skip to content

Commit 05ca226

Browse files
juzerpatanwalajuzerpatanwala
authored
dcr: support RFC 8414 §3.1 path-insertion in discovery-URL → issuer derivation (#5395)
* dcr: support RFC 8414 §3.1 path-insertion in discovery-URL → issuer derivation `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 <juzer.patanwala@project44.com> * dcr: normalise trailing-slash bare well-known to origin (review feedback) 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 <juzer.patanwala@project44.com> --------- Signed-off-by: Juzer Patanwala <juzer.patanwala@project44.com>
1 parent 2d59159 commit 05ca226

2 files changed

Lines changed: 79 additions & 11 deletions

File tree

‎pkg/auth/dcr/resolver.go‎

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -899,24 +899,31 @@ func resolveDCREndpoints(
899899
// upstream is expected to claim in its RFC 8414 / OIDC Discovery document,
900900
// given an operator-configured DiscoveryURL.
901901
//
902-
// Two recognised conventions:
902+
// Three recognised conventions:
903903
//
904904
// 1. Well-known suffix: the URL ends with /.well-known/oauth-authorization-server
905905
// or /.well-known/openid-configuration. The suffix is stripped to recover
906906
// the issuer; this covers single-tenant providers (e.g.
907907
// https://mcp.atlassian.com/.well-known/oauth-authorization-server →
908-
// https://mcp.atlassian.com) and the issuer-suffix multi-tenant style
909-
// (e.g. https://idp.example.com/tenants/acme/.well-known/openid-configuration
908+
// https://mcp.atlassian.com) and the OIDC-style suffix-append shape used
909+
// by some multi-tenant providers (e.g.
910+
// https://idp.example.com/tenants/acme/.well-known/openid-configuration
910911
// → https://idp.example.com/tenants/acme).
911-
// 2. Non-well-known path: the URL points at a custom metadata endpoint that
912-
// does not end in either suffix. Origin (scheme://host) is used as a
913-
// best-effort fallback; this matches the common shape where the upstream
914-
// issuer is the host root.
912+
// 2. RFC 8414 §3.1 path-insertion: the well-known path is inserted BETWEEN
913+
// the host and the issuer's tenant path. Per RFC 8414 §3 / RFC 8615,
914+
// this is the canonical form for issuers with a path component, e.g.
915+
// issuer https://example.com/v1/mcp →
916+
// discovery URL https://example.com/.well-known/oauth-authorization-server/v1/mcp.
917+
// The tenant suffix that appears AFTER the well-known segment is
918+
// re-attached to the origin to recover the issuer.
919+
// 3. Non-well-known path: the URL points at a custom metadata endpoint that
920+
// contains neither suffix in a recognisable position. Origin
921+
// (scheme://host) is used as a best-effort fallback; this matches the
922+
// common shape where the upstream issuer is the host root.
915923
//
916-
// RFC 8414 §3.1's path-aware form (well-known path inserted between host and
917-
// tenant path, e.g. https://example.com/.well-known/oauth-authorization-server/tenant)
918-
// is not auto-detected here — operators on that pattern can switch to
919-
// dcr_config.registration_endpoint to bypass discovery.
924+
// Case (1) and case (2) are disambiguated by where the well-known segment
925+
// sits in the path: at the end ⇒ suffix-append, immediately after the host
926+
// with more path following ⇒ path-insertion.
920927
func deriveExpectedIssuerFromDiscoveryURL(discoveryURL string) (string, error) {
921928
const (
922929
oauthSuffix = "/.well-known/oauth-authorization-server"
@@ -932,10 +939,36 @@ func deriveExpectedIssuerFromDiscoveryURL(discoveryURL string) (string, error) {
932939
}
933940

934941
switch {
942+
// Suffix-append form (case 1): well-known segment at end of path.
935943
case strings.HasSuffix(u.Path, oauthSuffix):
936944
u.Path = strings.TrimSuffix(u.Path, oauthSuffix)
937945
case strings.HasSuffix(u.Path, oidcSuffix):
938946
u.Path = strings.TrimSuffix(u.Path, oidcSuffix)
947+
// RFC 8414 §3.1 path-insertion form (case 2): well-known segment at the
948+
// start of the path with tenant path following. Strip just the well-known
949+
// segment to recover {origin}{tenant-path}.
950+
//
951+
// Two shapes hit this branch:
952+
// 1. A real tenant suffix follows the well-known segment, e.g.
953+
// /.well-known/oauth-authorization-server/v1/mcp →
954+
// issuer https://host/v1/mcp.
955+
// 2. A trailing slash with no tenant, e.g.
956+
// /.well-known/oauth-authorization-server/ — TrimPrefix leaves
957+
// a stray "/", which would yield a spurious issuer
958+
// "https://host/" that fails the §3.3 byte-equality check
959+
// against the upstream's declared "https://host". Normalise
960+
// that stray "/" back to empty so case (2.2) and the bare
961+
// suffix case derive the same origin issuer.
962+
case strings.HasPrefix(u.Path, oauthSuffix+"/"):
963+
u.Path = strings.TrimPrefix(u.Path, oauthSuffix)
964+
if u.Path == "/" {
965+
u.Path = ""
966+
}
967+
case strings.HasPrefix(u.Path, oidcSuffix+"/"):
968+
u.Path = strings.TrimPrefix(u.Path, oidcSuffix)
969+
if u.Path == "/" {
970+
u.Path = ""
971+
}
939972
default:
940973
// Custom (non-well-known) discovery URL — fall back to origin.
941974
u.Path = ""

‎pkg/auth/dcr/resolver_test.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -962,6 +962,41 @@ func TestDeriveExpectedIssuerFromDiscoveryURL(t *testing.T) {
962962
discoveryURL: "https://idp.example.com/tenants/acme/.well-known/openid-configuration",
963963
want: "https://idp.example.com/tenants/acme",
964964
},
965+
{
966+
// RFC 8414 §3.1 path-insertion: well-known segment between host
967+
// and tenant path. Issuer reconstructed as origin + tenant path.
968+
// Matches the shape served by Datadog's MCP authorization server
969+
// (mcp.us5.datadoghq.com) and any other provider with a path-
970+
// component issuer that follows the RFC literally.
971+
name: "oauth well-known path-insertion (RFC 8414 §3.1)",
972+
discoveryURL: "https://mcp.us5.datadoghq.com/.well-known/oauth-authorization-server/v1/mcp",
973+
want: "https://mcp.us5.datadoghq.com/v1/mcp",
974+
},
975+
{
976+
name: "oauth well-known path-insertion with multi-segment tenant",
977+
discoveryURL: "https://idp.example.com/.well-known/oauth-authorization-server/tenants/acme",
978+
want: "https://idp.example.com/tenants/acme",
979+
},
980+
{
981+
name: "oidc well-known path-insertion",
982+
discoveryURL: "https://idp.example.com/.well-known/openid-configuration/tenants/acme",
983+
want: "https://idp.example.com/tenants/acme",
984+
},
985+
{
986+
// Trailing-slash edge case: hits the HasPrefix arm (path doesn't end
987+
// at the bare suffix) but has no tenant after it. Without the empty-
988+
// path normalisation, TrimPrefix would leave a stray "/" and produce
989+
// a spurious "https://host/" that fails the RFC 8414 §3.3 byte
990+
// equality check.
991+
name: "oauth well-known with trailing slash normalises to origin",
992+
discoveryURL: "https://mcp.example.com/.well-known/oauth-authorization-server/",
993+
want: "https://mcp.example.com",
994+
},
995+
{
996+
name: "oidc well-known with trailing slash normalises to origin",
997+
discoveryURL: "https://idp.example.com/.well-known/openid-configuration/",
998+
want: "https://idp.example.com",
999+
},
9651000
{
9661001
name: "non-well-known path falls back to origin",
9671002
discoveryURL: "https://idp.example.com/tenants/acme/metadata",

0 commit comments

Comments
 (0)