mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-14 08:02:58 +00:00
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011BzU9zd53JsTShCsbKzYzB * fix: surface oauth provider errors returned with http 200 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: update ee repo ref Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: show the unavailable sign-in option in the oauth resource form Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: name the oauth grant in both authentication card descriptions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * 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) <noreply@anthropic.com> Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
This commit is contained in:
hugocasa
co-authored by
Claude Opus 5.5
windmill-internal-app[bot]
Claude Opus 5.5
windmill-internal-app[bot]
parent
3e43e6f06f
commit
d8ac4f59c8
@@ -1 +1 @@
|
||||
40ab35596e6fcb82a4dfc56e186befb807f8e2af
|
||||
0a7c28718872bb5cd624d77e32ad7d75c6ce42de
|
||||
|
||||
@@ -790,7 +790,26 @@ pub async fn exchange_code<T: DeserializeOwned>(
|
||||
.with_client(http_client)
|
||||
.execute::<T>()
|
||||
.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::<oauth2::ErrorResponse>(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::<TokenResponse>(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]
|
||||
|
||||
@@ -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 @@
|
||||
</div>
|
||||
{#if apiTokenApps[resourceType].img}
|
||||
<div class="mt-4 w-full overflow-hidden">
|
||||
<img
|
||||
class="m-auto max-h-60"
|
||||
alt="connect"
|
||||
src={apiTokenApps[resourceType].img}
|
||||
/>
|
||||
<img class="m-auto max-h-60" alt="connect" src={apiTokenApps[resourceType].img} />
|
||||
</div>
|
||||
{/if}
|
||||
{/if}
|
||||
@@ -1514,14 +1519,14 @@
|
||||
{#if supportsClientCredentials}
|
||||
<div class="flex flex-col gap-1">
|
||||
<h3 class="text-sm font-semibold text-emphasis mb-1">Authentication</h3>
|
||||
{#if ccOnly || ccBringYourOwn}
|
||||
{#if (ccOnly || ccBringYourOwn) && !signInNeedsInstanceClient}
|
||||
<div class="text-xs text-secondary font-normal mb-2">
|
||||
{#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}
|
||||
</div>
|
||||
{:else}
|
||||
@@ -1534,17 +1539,20 @@
|
||||
>
|
||||
<RadioCard
|
||||
label={`Sign in through ${resourceType}`}
|
||||
description="Opens a browser window to log in and authorize. Connects as you."
|
||||
description={signInNeedsInstanceClient
|
||||
? `Authorization code grant. Needs an OAuth client for ${resourceType} in the instance settings, added by a superadmin.`
|
||||
: 'Authorization code grant. Opens a browser window to log in and authorize. Connects as you.'}
|
||||
selected={!useClientCredentials}
|
||||
onSelect={selectAuthCodeGrant}
|
||||
disabled={signInNeedsInstanceClient}
|
||||
/>
|
||||
<RadioCard
|
||||
label={useSharedInstanceCreds
|
||||
? 'Use the configured instance credentials'
|
||||
: 'Use a client ID and secret'}
|
||||
description={useSharedInstanceCreds
|
||||
? "Runs server-to-server with this instance's credentials. No input needed."
|
||||
: 'Runs server-to-server. Best for automation or service accounts.'}
|
||||
? "Client credentials grant. Runs server-to-server with this instance's credentials. No input needed."
|
||||
: 'Client credentials grant. Runs server-to-server. Best for automation or service accounts.'}
|
||||
selected={useClientCredentials}
|
||||
onSelect={() => enableClientCredentials()}
|
||||
/>
|
||||
|
||||
@@ -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
|
||||
)}
|
||||
>
|
||||
|
||||
Reference in New Issue
Block a user