Skip to content

Commit 024c042

Browse files
rdimitrovclaude
rdimitrov
and
claude
authored
Merge commit from fork
* Bind OAuth callback to the initiating browser The embedded authorization server loaded a pending authorization at /oauth/callback purely by the upstream state parameter. Nothing tied the record to the browser that called /oauth/authorize, so an attacker could start a flow for their own client, hand the upstream IdP URL to a victim, and receive an authorization code minted for the victim's identity (GHSA-2gjv-f568-6cxp). /oauth/authorize now sets a per-flow, HttpOnly, SameSite=Lax cookie whose SHA-256 is stored on the pending record, and the callback completes a leg only when the calling browser presents a matching cookie. The cookie uses the __Host- prefix whenever the browser-facing authorize URL is https so a sibling subdomain cannot plant a matching value. Each multi-upstream chain leg mints its own binding. A callback without a valid binding is answered with a plain 400 and consumes the record; it is never redirected to the client's redirect URI. Records without a binding hash are refused by storage and rejected at the callback. Config validation warns at startup when an upstream's redirect_uri host differs from the authorize host, since a host-only cookie cannot bridge them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Bind device-flow login to the submitting browser The device flow's verification page shares /oauth/callback with the authorization-code flow and had the same gap: POST /oauth/device stored a PendingDeviceLogin keyed only by the upstream state, so whoever reached the callback with that state completed the login. A user_code holder could hand the upstream URL to someone else and have that person's identity land on the confirmation page for the holder's device (RFC 8628 Section 5.4). The submit handler now mints the same browser-binding cookie and stores its hash on the PendingDeviceLogin. Both the completion path and the upstream-error path require the cookie; a foreign browser gets 400, the login is consumed, and the DeviceRequest is left pending rather than authorized or denied by a browser that did not start the login. Storage refuses a device login without a binding hash. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Simplify browser binding helpers Collapse the per-record verify wrappers into one function that takes the stored hash, consume the pending record before the binding check so a single rejection helper serves both flows, and drop the cookie-name method that only forwarded to the package function. Remove the cookie-clearing step and the storage-level empty-hash refusal: the record is deleted on first use so a leftover cookie is inert, and the callback already rejects a record without a hash, which is where that guarantee is enforced and tested. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Require an anti-forgery token on the device form POST /oauth/device accepted any form submission, so a cross-site page could auto-submit an attacker's user_code from a victim's browser. That browser would then be issued the device-login binding cookie and could complete the upstream callback, which left the binding proving only that the browser posted the form, not that the user chose to. GET /oauth/device now sets a random cookie and embeds the same value in a hidden form_token field, and the submit handler accepts the form only when the two match in constant time. The cookie has the same shape as the binding cookie, so a cross-site POST neither carries it nor can plant it. A rejected submission re-renders the form with a fresh token and an error; nothing is minted. Drivers that post a user_code without loading the form must now load it first. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Advertise device page on the browser-facing host The device authorization response advertised the verification page from the issuer while the callback lives on the browser-facing authorize base URL, so a deployment that sets authorizationEndpointBaseUrl to another host stranded the binding cookie on the issuer host and every device login failed at the callback with no diagnostic. The page is now advertised from the same base URL as /oauth/authorize. The device form's anti-forgery cookie was rotated on every render, so a second open form, or a second local server sharing the browser's cookie jar, invalidated the first. The cookie the browser already holds is now reused and only minted when absent; a cross-site page can neither read it nor make the browser send it, so the double-submit check keeps its strength. Config validation also warns when an upstream callback is plain http on a non-loopback host while the authorize URL is https: the binding cookie is Secure there and browsers will not send it to the callback. Two tests drive both flows through a real HTTP server with a cookie jar, issuer on 127.0.0.1 and browser-facing base on localhost, so host-only cookie delivery is exercised rather than simulated; the device one fails without the first change above. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent dfb0713 commit 024c042

34 files changed

Lines changed: 1569 additions & 183 deletions

‎cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -944,6 +944,10 @@ type EmbeddedAuthServerConfig struct {
944944
// All other endpoints (token, registration, JWKS) remain derived from the issuer.
945945
// This is useful when the browser-facing authorization endpoint needs to be on a
946946
// different host than the issuer used for backend-to-backend calls.
947+
// The upstream callback (`redirectUri` on each upstream provider, defaulting to
948+
// `{resourceUrl}/oauth/callback`) must share this hostname: the callback only
949+
// completes a login for the browser that started it, bound by a host-only cookie
950+
// set on this endpoint, so a different callback host rejects every browser login.
947951
// Must be a valid HTTPS URL (or HTTP for localhost, or HTTP for trusted in-cluster hosts
948952
// when insecureAllowHTTP is true) without query, fragment, or trailing slash.
949953
// +kubebuilder:validation:Pattern=`^https?://[^\s?#]+[^/\s?#]$`
@@ -1525,6 +1529,9 @@ type OIDCUpstreamConfig struct {
15251529
// RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
15261530
// When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the
15271531
// URL associated with the resource (e.g., MCPServer or vMCP) using this config.
1532+
// Its hostname must match the browser-facing authorization endpoint (`issuer`, or
1533+
// `authorizationEndpointBaseUrl` when set): the callback is bound to the browser that
1534+
// started the login by a host-only cookie, so a different host rejects every browser login.
15281535
// +optional
15291536
RedirectURI string `json:"redirectUri,omitempty"`
15301537

@@ -1689,6 +1696,9 @@ type OAuth2UpstreamConfig struct {
16891696
// RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
16901697
// When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the
16911698
// URL associated with the resource (e.g., MCPServer or vMCP) using this config.
1699+
// Its hostname must match the browser-facing authorization endpoint (`issuer`, or
1700+
// `authorizationEndpointBaseUrl` when set): the callback is bound to the browser that
1701+
// started the login by a host-only cookie, so a different host rejects every browser login.
16921702
// +optional
16931703
RedirectURI string `json:"redirectUri,omitempty"`
16941704

‎deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,10 @@ spec:
242242
All other endpoints (token, registration, JWKS) remain derived from the issuer.
243243
This is useful when the browser-facing authorization endpoint needs to be on a
244244
different host than the issuer used for backend-to-backend calls.
245+
The upstream callback (`redirectUri` on each upstream provider, defaulting to
246+
`{resourceUrl}/oauth/callback`) must share this hostname: the callback only
247+
completes a login for the browser that started it, bound by a host-only cookie
248+
set on this endpoint, so a different callback host rejects every browser login.
245249
Must be a valid HTTPS URL (or HTTP for localhost, or HTTP for trusted in-cluster hosts
246250
when insecureAllowHTTP is true) without query, fragment, or trailing slash.
247251
pattern: ^https?://[^\s?#]+[^/\s?#]$
@@ -1774,6 +1778,9 @@ spec:
17741778
RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
17751779
When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the
17761780
URL associated with the resource (e.g., MCPServer or vMCP) using this config.
1781+
Its hostname must match the browser-facing authorization endpoint (`issuer`, or
1782+
`authorizationEndpointBaseUrl` when set): the callback is bound to the browser that
1783+
started the login by a host-only cookie, so a different host rejects every browser login.
17771784
type: string
17781785
scopes:
17791786
description: Scopes are the OAuth scopes to request
@@ -2111,6 +2118,9 @@ spec:
21112118
RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
21122119
When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the
21132120
URL associated with the resource (e.g., MCPServer or vMCP) using this config.
2121+
Its hostname must match the browser-facing authorization endpoint (`issuer`, or
2122+
`authorizationEndpointBaseUrl` when set): the callback is bound to the browser that
2123+
started the login by a host-only cookie, so a different host rejects every browser login.
21142124
type: string
21152125
scopes:
21162126
description: |-
@@ -3077,6 +3087,10 @@ spec:
30773087
All other endpoints (token, registration, JWKS) remain derived from the issuer.
30783088
This is useful when the browser-facing authorization endpoint needs to be on a
30793089
different host than the issuer used for backend-to-backend calls.
3090+
The upstream callback (`redirectUri` on each upstream provider, defaulting to
3091+
`{resourceUrl}/oauth/callback`) must share this hostname: the callback only
3092+
completes a login for the browser that started it, bound by a host-only cookie
3093+
set on this endpoint, so a different callback host rejects every browser login.
30803094
Must be a valid HTTPS URL (or HTTP for localhost, or HTTP for trusted in-cluster hosts
30813095
when insecureAllowHTTP is true) without query, fragment, or trailing slash.
30823096
pattern: ^https?://[^\s?#]+[^/\s?#]$
@@ -4609,6 +4623,9 @@ spec:
46094623
RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
46104624
When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the
46114625
URL associated with the resource (e.g., MCPServer or vMCP) using this config.
4626+
Its hostname must match the browser-facing authorization endpoint (`issuer`, or
4627+
`authorizationEndpointBaseUrl` when set): the callback is bound to the browser that
4628+
started the login by a host-only cookie, so a different host rejects every browser login.
46124629
type: string
46134630
scopes:
46144631
description: Scopes are the OAuth scopes to request
@@ -4946,6 +4963,9 @@ spec:
49464963
RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
49474964
When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the
49484965
URL associated with the resource (e.g., MCPServer or vMCP) using this config.
4966+
Its hostname must match the browser-facing authorization endpoint (`issuer`, or
4967+
`authorizationEndpointBaseUrl` when set): the callback is bound to the browser that
4968+
started the login by a host-only cookie, so a different host rejects every browser login.
49494969
type: string
49504970
scopes:
49514971
description: |-

‎deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,10 @@ spec:
118118
All other endpoints (token, registration, JWKS) remain derived from the issuer.
119119
This is useful when the browser-facing authorization endpoint needs to be on a
120120
different host than the issuer used for backend-to-backend calls.
121+
The upstream callback (`redirectUri` on each upstream provider, defaulting to
122+
`{resourceUrl}/oauth/callback`) must share this hostname: the callback only
123+
completes a login for the browser that started it, bound by a host-only cookie
124+
set on this endpoint, so a different callback host rejects every browser login.
121125
Must be a valid HTTPS URL (or HTTP for localhost, or HTTP for trusted in-cluster hosts
122126
when insecureAllowHTTP is true) without query, fragment, or trailing slash.
123127
pattern: ^https?://[^\s?#]+[^/\s?#]$
@@ -1650,6 +1654,9 @@ spec:
16501654
RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
16511655
When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the
16521656
URL associated with the resource (e.g., MCPServer or vMCP) using this config.
1657+
Its hostname must match the browser-facing authorization endpoint (`issuer`, or
1658+
`authorizationEndpointBaseUrl` when set): the callback is bound to the browser that
1659+
started the login by a host-only cookie, so a different host rejects every browser login.
16531660
type: string
16541661
scopes:
16551662
description: Scopes are the OAuth scopes to request
@@ -1987,6 +1994,9 @@ spec:
19871994
RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
19881995
When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the
19891996
URL associated with the resource (e.g., MCPServer or vMCP) using this config.
1997+
Its hostname must match the browser-facing authorization endpoint (`issuer`, or
1998+
`authorizationEndpointBaseUrl` when set): the callback is bound to the browser that
1999+
started the login by a host-only cookie, so a different host rejects every browser login.
19902000
type: string
19912001
scopes:
19922002
description: |-
@@ -5150,6 +5160,10 @@ spec:
51505160
All other endpoints (token, registration, JWKS) remain derived from the issuer.
51515161
This is useful when the browser-facing authorization endpoint needs to be on a
51525162
different host than the issuer used for backend-to-backend calls.
5163+
The upstream callback (`redirectUri` on each upstream provider, defaulting to
5164+
`{resourceUrl}/oauth/callback`) must share this hostname: the callback only
5165+
completes a login for the browser that started it, bound by a host-only cookie
5166+
set on this endpoint, so a different callback host rejects every browser login.
51535167
Must be a valid HTTPS URL (or HTTP for localhost, or HTTP for trusted in-cluster hosts
51545168
when insecureAllowHTTP is true) without query, fragment, or trailing slash.
51555169
pattern: ^https?://[^\s?#]+[^/\s?#]$
@@ -6682,6 +6696,9 @@ spec:
66826696
RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
66836697
When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the
66846698
URL associated with the resource (e.g., MCPServer or vMCP) using this config.
6699+
Its hostname must match the browser-facing authorization endpoint (`issuer`, or
6700+
`authorizationEndpointBaseUrl` when set): the callback is bound to the browser that
6701+
started the login by a host-only cookie, so a different host rejects every browser login.
66856702
type: string
66866703
scopes:
66876704
description: Scopes are the OAuth scopes to request
@@ -7019,6 +7036,9 @@ spec:
70197036
RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
70207037
When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the
70217038
URL associated with the resource (e.g., MCPServer or vMCP) using this config.
7039+
Its hostname must match the browser-facing authorization endpoint (`issuer`, or
7040+
`authorizationEndpointBaseUrl` when set): the callback is bound to the browser that
7041+
started the login by a host-only cookie, so a different host rejects every browser login.
70227042
type: string
70237043
scopes:
70247044
description: |-

‎deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,10 @@ spec:
245245
All other endpoints (token, registration, JWKS) remain derived from the issuer.
246246
This is useful when the browser-facing authorization endpoint needs to be on a
247247
different host than the issuer used for backend-to-backend calls.
248+
The upstream callback (`redirectUri` on each upstream provider, defaulting to
249+
`{resourceUrl}/oauth/callback`) must share this hostname: the callback only
250+
completes a login for the browser that started it, bound by a host-only cookie
251+
set on this endpoint, so a different callback host rejects every browser login.
248252
Must be a valid HTTPS URL (or HTTP for localhost, or HTTP for trusted in-cluster hosts
249253
when insecureAllowHTTP is true) without query, fragment, or trailing slash.
250254
pattern: ^https?://[^\s?#]+[^/\s?#]$
@@ -1777,6 +1781,9 @@ spec:
17771781
RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
17781782
When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the
17791783
URL associated with the resource (e.g., MCPServer or vMCP) using this config.
1784+
Its hostname must match the browser-facing authorization endpoint (`issuer`, or
1785+
`authorizationEndpointBaseUrl` when set): the callback is bound to the browser that
1786+
started the login by a host-only cookie, so a different host rejects every browser login.
17801787
type: string
17811788
scopes:
17821789
description: Scopes are the OAuth scopes to request
@@ -2114,6 +2121,9 @@ spec:
21142121
RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
21152122
When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the
21162123
URL associated with the resource (e.g., MCPServer or vMCP) using this config.
2124+
Its hostname must match the browser-facing authorization endpoint (`issuer`, or
2125+
`authorizationEndpointBaseUrl` when set): the callback is bound to the browser that
2126+
started the login by a host-only cookie, so a different host rejects every browser login.
21172127
type: string
21182128
scopes:
21192129
description: |-
@@ -3080,6 +3090,10 @@ spec:
30803090
All other endpoints (token, registration, JWKS) remain derived from the issuer.
30813091
This is useful when the browser-facing authorization endpoint needs to be on a
30823092
different host than the issuer used for backend-to-backend calls.
3093+
The upstream callback (`redirectUri` on each upstream provider, defaulting to
3094+
`{resourceUrl}/oauth/callback`) must share this hostname: the callback only
3095+
completes a login for the browser that started it, bound by a host-only cookie
3096+
set on this endpoint, so a different callback host rejects every browser login.
30833097
Must be a valid HTTPS URL (or HTTP for localhost, or HTTP for trusted in-cluster hosts
30843098
when insecureAllowHTTP is true) without query, fragment, or trailing slash.
30853099
pattern: ^https?://[^\s?#]+[^/\s?#]$
@@ -4612,6 +4626,9 @@ spec:
46124626
RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
46134627
When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the
46144628
URL associated with the resource (e.g., MCPServer or vMCP) using this config.
4629+
Its hostname must match the browser-facing authorization endpoint (`issuer`, or
4630+
`authorizationEndpointBaseUrl` when set): the callback is bound to the browser that
4631+
started the login by a host-only cookie, so a different host rejects every browser login.
46154632
type: string
46164633
scopes:
46174634
description: Scopes are the OAuth scopes to request
@@ -4949,6 +4966,9 @@ spec:
49494966
RedirectURI is the callback URL where the upstream IdP will redirect after authentication.
49504967
When not specified, defaults to `{resourceUrl}/oauth/callback` where `resourceUrl` is the
49514968
URL associated with the resource (e.g., MCPServer or vMCP) using this config.
4969+
Its hostname must match the browser-facing authorization endpoint (`issuer`, or
4970+
`authorizationEndpointBaseUrl` when set): the callback is bound to the browser that
4971+
started the login by a host-only cookie, so a different host rejects every browser login.
49524972
type: string
49534973
scopes:
49544974
description: |-

0 commit comments

Comments
 (0)