From ef4962e52aba0bc79bf72523de9101853c660654 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Mon, 22 Jun 2026 13:05:15 +0200 Subject: [PATCH] fix(oauth): restore bring-your-own CC token URL override (#9711) * fix(oauth): restore bring-your-own CC token URL override Re-add the optional resource-level token URL field for client-credentials connections, sent only with the caller's own client_id/secret. Updates the connect/create_account request schemas and bumps the EE ref. Co-Authored-By: Claude Opus 4.8 (1M context) * chore(oauth): keep openapi-deref unchanged from main The dereferenced specs are not regenerated per-PR (already stale on main, CI only lint-validates them). Revert the incidental full regen so the PR diff stays focused on openapi.yaml. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(oauth): host-pin CC token URL override server-side Add is_instance_templated_cc so the EE handlers can reject a bring-your-own token URL override for {instance}-templated providers (defense in depth for direct API callers). Bump the EE ref. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(oauth): serve cc_token_url in deref specs, enforce CC grant gate Add cc_token_url to the dereferenced OpenAPI artifacts served at /openapi.yaml and /openapi.json so generated clients see the new field (kept to a focused add rather than a full regen). Bump the EE ref for the grant-gate enforcement. Co-Authored-By: Claude Opus 4.8 (1M context) * chore: update ee-repo-ref to de49fda2320504ad9e7d2d31c7033d71dbf6ca43 This commit updates the EE repository reference after PR #625 was merged in windmill-ee-private. Previous ee-repo-ref: a939228d0314c21937687d43c8ef354bdc87c40e New ee-repo-ref: de49fda2320504ad9e7d2d31c7033d71dbf6ca43 Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: windmill-internal-app[bot] --- backend/ee-repo-ref.txt | 2 +- backend/windmill-api/openapi-deref.json | 8 +++ backend/windmill-api/openapi-deref.yaml | 13 +++++ backend/windmill-api/openapi.yaml | 6 +++ backend/windmill-oauth/src/lib.rs | 50 ++++++++++++++++--- .../src/lib/components/AppConnectInner.svelte | 36 +++++++++++-- 6 files changed, 105 insertions(+), 10 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 656c1f8a0e..f746dd3faf 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -8b12fa14ef7969e948169eadf1bf672d7928e5b1 +de49fda2320504ad9e7d2d31c7033d71dbf6ca43 diff --git a/backend/windmill-api/openapi-deref.json b/backend/windmill-api/openapi-deref.json index 135131af41..0a27c66f3a 100644 --- a/backend/windmill-api/openapi-deref.json +++ b/backend/windmill-api/openapi-deref.json @@ -9704,6 +9704,10 @@ "type": "string", "description": "Instance name for built-in providers whose client-credentials token URL is instance-templated; substituted into the fixed-host registry template server-side (client_credentials flow only). The token URL is never caller-supplied." }, + "cc_token_url": { + "type": "string", + "description": "Bring-your-own token endpoint override (client_credentials flow only). Only honored together with cc_client_id/cc_client_secret and mutually exclusive with cc_instance; ignored/rejected on the shared-instance path." + }, "mcp_server_url": { "type": "string", "description": "MCP server URL for MCP OAuth token refresh" @@ -9785,6 +9789,10 @@ "cc_instance": { "type": "string", "description": "Instance name for built-in providers whose client-credentials token URL is instance-templated; substituted into the fixed-host registry template server-side. The token URL is never caller-supplied." + }, + "cc_token_url": { + "type": "string", + "description": "Bring-your-own token endpoint override. Only honored together with cc_client_id/cc_client_secret and mutually exclusive with cc_instance; rejected on the shared-instance path." } } } diff --git a/backend/windmill-api/openapi-deref.yaml b/backend/windmill-api/openapi-deref.yaml index e45131d81b..bfc7a0e02c 100644 --- a/backend/windmill-api/openapi-deref.yaml +++ b/backend/windmill-api/openapi-deref.yaml @@ -8918,6 +8918,13 @@ paths: substituted into the fixed-host registry template server-side (client_credentials flow only). The token URL is never caller-supplied. + cc_token_url: + type: string + description: >- + Bring-your-own token endpoint override (client_credentials + flow only). Only honored together with + cc_client_id/cc_client_secret and mutually exclusive with + cc_instance; ignored/rejected on the shared-instance path. mcp_server_url: type: string description: MCP server URL for MCP OAuth token refresh @@ -8985,6 +8992,12 @@ paths: client-credentials token URL is instance-templated; substituted into the fixed-host registry template server-side. The token URL is never caller-supplied. + cc_token_url: + type: string + description: >- + Bring-your-own token endpoint override. Only honored together + with cc_client_id/cc_client_secret and mutually exclusive with + cc_instance; rejected on the shared-instance path. responses: '200': description: OAuth token response diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index d5ba5720e7..4ec1c04610 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -6291,6 +6291,9 @@ paths: cc_instance: type: string description: "Instance name for built-in providers whose client-credentials token URL is instance-templated; substituted into the fixed-host registry template server-side (client_credentials flow only). The token URL is never caller-supplied." + cc_token_url: + type: string + description: "Bring-your-own token endpoint override (client_credentials flow only). Only honored together with cc_client_id/cc_client_secret and mutually exclusive with cc_instance; ignored/rejected on the shared-instance path." mcp_server_url: type: string description: "MCP server URL for MCP OAuth token refresh" @@ -6346,6 +6349,9 @@ paths: cc_instance: type: string description: "Instance name for built-in providers whose client-credentials token URL is instance-templated; substituted into the fixed-host registry template server-side. The token URL is never caller-supplied." + cc_token_url: + type: string + description: "Bring-your-own token endpoint override. Only honored together with cc_client_id/cc_client_secret and mutually exclusive with cc_instance; rejected on the shared-instance path." responses: "200": description: OAuth token response diff --git a/backend/windmill-oauth/src/lib.rs b/backend/windmill-oauth/src/lib.rs index 2f28a6d4c7..e314f13d75 100644 --- a/backend/windmill-oauth/src/lib.rs +++ b/backend/windmill-oauth/src/lib.rs @@ -452,12 +452,12 @@ pub async fn build_client_credentials_oauth_client( let caller_supplied_creds = !client_id.is_empty() && !client_secret.is_empty(); - // Apply the server-resolved concrete token URL. Instance-templated providers - // (e.g. Coupa) carry an empty or `{instance}`-templated token URL in their - // registry config; the resolved value (host-pinned for bring-your-own, - // persisted on the row for refresh) is what completes it. The caller never - // supplies a free-form token URL: this value always comes from - // `resolve_cc_token_url_input` or a previously-resolved persisted URL. + // Apply the resolved concrete token URL. Instance-templated providers (e.g. + // Coupa) carry an empty or `{instance}`-templated token URL in their registry + // config; the resolved value (host-pinned for instance-name connections, + // persisted on the row for refresh) is what completes it. For bring-your-own + // connections this value may instead be a caller-supplied override — safe + // because only the caller's own credentials are ever sent to it. if let Some(url) = resolved_token_url { connect_config.token_url = url.to_string(); } @@ -652,6 +652,28 @@ pub fn resolve_cc_token_url_input( Ok(template.replace("{instance}", value)) } +/// Whether a built-in provider's client-credentials token URL is host-pinned via +/// an `{instance}` template (e.g. servicenow, snowflake, coupa). Such providers +/// only accept an instance name substituted into a fixed-host template, so a +/// free-form caller token URL override must be rejected for them — otherwise the +/// exchange host could be redirected, which is exactly what the template pins. +/// Fixed-host registry providers and custom (non-registry) providers return +/// `false`: an override is allowed there. +pub fn is_instance_templated_cc(connect_configs_json: &str, client_name: &str) -> bool { + serde_json::from_str::>(connect_configs_json) + .ok() + .and_then(|m| resolve_registry_config(&m, client_name)) + .map(|cfg| { + cfg.connect_config_template + .as_ref() + .map(|t| t.token_url.clone()) + .filter(|u| !u.is_empty()) + .unwrap_or(cfg.token_url) + .contains("{instance}") + }) + .unwrap_or(false) +} + /// Resolve the concrete bring-your-own client-credentials token URL for any /// provider, never from a caller-supplied URL: /// - **Built-in registry providers** resolve from the registry via @@ -1350,4 +1372,20 @@ mod tests { assert!(resolve_cc_token_url_input(CC_REGISTRY, "bad_host_tpl", Some("evil.com")).is_err()); assert!(resolve_cc_token_url_input(CC_REGISTRY, "bad_mid_tpl", Some("evil")).is_err()); } + + #[test] + fn instance_templated_cc_true_for_templated_providers() { + // Host-pinned via `{instance}`: a bring-your-own token URL override must be + // refused for these (only the instance-name path may set their URL). + assert!(is_instance_templated_cc(CC_REGISTRY, "coupa")); + assert!(is_instance_templated_cc(CC_REGISTRY, "servicenow")); + } + + #[test] + fn instance_templated_cc_false_for_fixed_host_and_unknown() { + // Fixed-host registry provider and custom (non-registry) provider both allow + // an override, so neither is reported as instance-templated. + assert!(!is_instance_templated_cc(CC_REGISTRY, "visma")); + assert!(!is_instance_templated_cc(CC_REGISTRY, "my_custom_thing")); + } } diff --git a/frontend/src/lib/components/AppConnectInner.svelte b/frontend/src/lib/components/AppConnectInner.svelte index 1046ee69b0..9edd80dd54 100644 --- a/frontend/src/lib/components/AppConnectInner.svelte +++ b/frontend/src/lib/components/AppConnectInner.svelte @@ -162,6 +162,9 @@ let clientId = $state('') let clientSecret = $state('') let ccInstance = $state('') + /** Bring-your-own resource-level token endpoint override (optional). Only sent + * for non-instance-templated providers, where it isn't host-pinned. */ + let tokenUrl = $state('') let resourceTypeInfo: ResourceType | undefined = $state(undefined) let resourceTypeNotFound = $state(false) @@ -222,6 +225,7 @@ clientId = '' clientSecret = '' ccInstance = '' + tokenUrl = '' scopes = [] } @@ -602,7 +606,13 @@ scopes: scopes, cc_client_id: trimmedClientId, cc_client_secret: trimmedClientSecret, - ...(needsInstance ? { cc_instance: trimmedInstance } : {}) + // Instance-templated providers are host-pinned via the instance + // name; only other providers accept a free-form token URL override. + ...(needsInstance + ? { cc_instance: trimmedInstance } + : tokenUrl.trim() + ? { cc_token_url: tokenUrl.trim() } + : {}) } }) @@ -734,10 +744,13 @@ accountData.cc_client_id = clientId.trim() accountData.cc_client_secret = clientSecret.trim() // Instance-templated providers send an instance name; the backend - // resolves and stores the host-pinned token URL. Other registry - // providers need nothing more (token URL comes from the registry). + // resolves and stores the host-pinned token URL. Other providers may + // send an optional token URL override (stored for refresh); without + // it the token URL comes from the registry/instance config. if (ccInstanceMeta) { accountData.cc_instance = ccInstance.trim() + } else if (tokenUrl.trim()) { + accountData.cc_token_url = tokenUrl.trim() } } @@ -1183,6 +1196,23 @@ bind:value={ccInstance} /> + {:else} + {/if} {/if}