Skip to content

dcr: support RFC 8414 §3.1 path-insertion in discovery-URL → issuer derivation #5395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
juzerpatanwala wants to merge 2 commits into
base: main
Choose a base branch
from
+79 −11
Open

dcr: support RFC 8414 §3.1 path-insertion in discovery-URL → issuer derivation #5395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a26a8df
dcr: support RFC 8414 §3.1 path-insertion in discovery-URL → issuer d…
juzerpatanwala May 29, 2026
6d1d7cc
dcr: normalise trailing-slash bare well-known to origin (review feedb…
juzerpatanwala Jun 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 44 additions & 11 deletions pkg/auth/dcr/resolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -932,10 +939,36 @@ 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}.
//
// 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+"/"):
Copy link
Copy Markdown
Contributor

@tgrunnagle tgrunnagle May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CORRECTNESS · LOW] Trailing-slash bare well-known derives a spurious trailing-slash issuer (minor regression).

For https://host/.well-known/oauth-authorization-server/ the suffix arms don't match (path ends in /), so this HasPrefix arm fires: TrimPrefix leaves u.Path = "/" → issuer https://host/. Before this PR the same input hit default → https://host. The comment above ("a bare suffix … is already covered by the suffix cases above") is therefore inaccurate for the trailing-slash form, and the component-sensitive §3.3 exact-string check would reject the https://host/ derivation.

Narrow (discovery URLs rarely carry a trailing slash), but worth tightening:

Suggested change
case strings.HasPrefix(u.Path, oauthSuffix+"/"):
case strings.HasPrefix(u.Path, oauthSuffix+"/"):
u.Path = strings.TrimPrefix(u.Path, oauthSuffix)
if u.Path == "/" {
u.Path = ""
}

Also consider correcting the comment and adding a table case for the trailing-slash shape expecting https://host.

Copy link
Copy Markdown
Author

@juzerpatanwala juzerpatanwala Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 6d1d7cc. Collapsed the lone "/" back to "" in both path-insertion arms after TrimPrefix, retightened the comment to describe both shapes that reach this arm, and added two table cases (oauth + oidc trailing-slash) that pin the expected origin output. All 13 cases pass locally.

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 = ""
Expand Down
35 changes: 35 additions & 0 deletions pkg/auth/dcr/resolver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,41 @@ 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",
},
{
// 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",
Expand Down