From d8ac4f59c8d570777e43a7aa7ffbc97598897cc2 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Tue, 13 Oct 2026 15:40:24 +0200 Subject: [PATCH] fix: surface oauth provider errors returned with http 200 (#11713) * fix: bump git sync hub scripts to cli 1.830.0 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011BzU9zd53JsTShCsbKzYzB * fix: surface oauth provider errors returned with http 200 Co-Authored-By: Claude Opus 5.5 * chore: update ee repo ref Co-Authored-By: Claude Opus 5.5 * fix: show the unavailable sign-in option in the oauth resource form Co-Authored-By: Claude Opus 5.5 * fix: name the oauth grant in both authentication card descriptions Co-Authored-By: Claude Opus 5.5 * chore: update ee-repo-ref to 0a7c28718872bb5cd624d77e32ad7d75c6ce42de This commit updates the EE repository reference after PR #874 was merged in windmill-ee-private. Previous ee-repo-ref: 11425053010c3cd4670bc06f3606fa18cccc2091 New ee-repo-ref: 0a7c28718872bb5cd624d77e32ad7d75c6ce42de Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 5.5 (1M context) Co-authored-by: windmill-internal-app[bot] --- backend/ee-repo-ref.txt | 2 +- backend/windmill-oauth/src/lib.rs | 32 ++++++++++++++++++- .../src/lib/components/AppConnectInner.svelte | 32 ++++++++++++------- .../common/radioCard/RadioCard.svelte | 6 ++-- 4 files changed, 56 insertions(+), 16 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 695f6d248b..d68805fb8a 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -40ab35596e6fcb82a4dfc56e186befb807f8e2af +0a7c28718872bb5cd624d77e32ad7d75c6ce42de diff --git a/backend/windmill-oauth/src/lib.rs b/backend/windmill-oauth/src/lib.rs index 15f5ac6b9e..9768911813 100644 --- a/backend/windmill-oauth/src/lib.rs +++ b/backend/windmill-oauth/src/lib.rs @@ -790,7 +790,26 @@ pub async fn exchange_code( .with_client(http_client) .execute::() .await - .map_err(|e| error::Error::InternalErr(format!("{:?}", e))) + .map_err(|e| error::Error::InternalErr(token_error_message(&e))) +} + +/// Message for a failed token request. Some providers (Zoho) answer an OAuth error with +/// HTTP 200, which the client reports as an unparseable token response, not an error one. +pub fn token_error_message(e: &ExecuteError) -> String { + match e { + ExecuteError::ErrorResponse { error, .. } => error.to_string(), + ExecuteError::BadResponse { status, body, .. } => { + serde_json::from_slice::(body) + .map(|r| r.to_string()) + .unwrap_or_else(|_| { + format!( + "unexpected response ({status}): {}", + String::from_utf8_lossy(body) + ) + }) + } + e => format!("{e:?}"), + } } /// Internal token exchange implementation @@ -1212,6 +1231,17 @@ mod tests { assert!(verifier.verify("123", "body", "wrong_sig").is_err()); } + #[test] + fn token_error_message_reads_an_error_sent_with_http_200() { + let body: &[u8] = br#"{"error":"invalid_client"}"#; + let e = ExecuteError::BadResponse { + status: reqwest::StatusCode::OK, + body: body.into(), + error: serde_json::from_slice::(body).unwrap_err(), + }; + assert_eq!(token_error_message(&e), "invalid_client"); + } + // Sage Intacct's token endpoint rejects HTTP Basic client authentication on the // refresh_token grant (`invalid_client`), so its credentials must go in the form body. #[test] diff --git a/frontend/src/lib/components/AppConnectInner.svelte b/frontend/src/lib/components/AppConnectInner.svelte index 56bff89d8f..18a28ec95d 100644 --- a/frontend/src/lib/components/AppConnectInner.svelte +++ b/frontend/src/lib/components/AppConnectInner.svelte @@ -322,6 +322,15 @@ (registryCcCapable() && connectClient != '' && !(connects?.includes(connectClient) ?? false)) ) + /** The provider also has a browser sign-in, unavailable here only because the + * instance holds no OAuth client for it */ + let signInNeedsInstanceClient = $derived.by( + () => + (registryEntry()?.grant_types?.includes('authorization_code') ?? false) && + connectClient != '' && + !(connects?.includes(connectClient) ?? false) + ) + /** Clear CC inputs and scopes so a previous selection never leaks into a new one */ function resetClientCredentialsState() { supportsClientCredentials = false @@ -1399,11 +1408,7 @@ {#if apiTokenApps[resourceType].img}
- connect + connect
{/if} {/if} @@ -1514,14 +1519,14 @@ {#if supportsClientCredentials}

Authentication

- {#if ccOnly || ccBringYourOwn} + {#if (ccOnly || ccBringYourOwn) && !signInNeedsInstanceClient}
{#if useSharedInstanceCreds} {resourceType} connects server-to-server using the credentials configured for this - instance. The token is acquired and refreshed automatically. + instance. {:else} - {resourceType} connects server-to-server. Enter a client ID and secret; the token is - acquired and refreshed automatically. + {resourceType} connects server-to-server with the client credentials grant. Enter a + client ID and secret. {/if}
{:else} @@ -1534,17 +1539,20 @@ > enableClientCredentials()} /> diff --git a/frontend/src/lib/components/common/radioCard/RadioCard.svelte b/frontend/src/lib/components/common/radioCard/RadioCard.svelte index f9b7f675a4..e152324fce 100644 --- a/frontend/src/lib/components/common/radioCard/RadioCard.svelte +++ b/frontend/src/lib/components/common/radioCard/RadioCard.svelte @@ -51,8 +51,10 @@ // (FileExplorer, TriggersTable, RunnableRow, ArtifactVersionPicker). selected ? 'border-border-selected bg-surface-accent-selected' - : 'border-border-light hover:bg-surface-hover', - disabled && 'opacity-50 cursor-not-allowed', + : 'border-border-light enabled:hover:bg-surface-hover', + // `disabled:` variants, not plain utilities: the global `button:disabled` + // rule sets opacity and cursor and outranks a single class. + 'disabled:opacity-50 disabled:cursor-not-allowed', className )} >