From c09c0ed926a8f963a312ccfb5ae2b4efa8af5476 Mon Sep 17 00:00:00 2001 From: jt Date: Tue, 4 Aug 2026 22:31:59 -0700 Subject: [PATCH 1/3] Let an authorization code flow be cancelled instead of hanging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Starting a browser sign-in parked on a loopback listener with a hard 120s timeout and no way out. That is fine when the provider redirects back, and useless when it does not — an unregistered redirect URI makes the provider show an error page in the browser and never redirect at all, so nothing ever reaches the listener. The button sat on "Getting Token…" for two minutes with no indication of what had gone wrong. The flow now registers a cancel channel keyed by an id the frontend supplies, and selects on the callback against that channel, following the same watch-channel pattern the SSE streams already use. A Cancel button appears beside the button for the authorization code flow only, since it is the only grant type that waits on anything external. Cancelling drops the listener, which frees the port for an immediate retry. Two supporting changes: - The timeout goes from 2 minutes to 5. Two was short for a real sign-in involving MFA, an account chooser and a password manager, and the only reason to keep it tight was that it doubled as the escape hatch. It no longer has to. - The timeout message names the exact callback URL the provider must have registered, which is the actual fix for the case that motivated this. It reports the URL that was really used, which matters because the default port falls back to an ephemeral one when 17823 is taken. `flow_id` is optional on the Rust side, so a flow started without one behaves exactly as before rather than failing to deserialize. The Rust cancellation path is covered by compilation and clippy only — driving it end to end needs the built desktop app, since the browser dev server cannot invoke Tauri commands. The frontend contract around it is tested: that the button appears only for the right grant type, that cancelling uses the same id the flow was started with, and that a failed cancel does not mask the error the flow itself reports. Co-Authored-By: Claude Opus 5 --- src-tauri/src/lib.rs | 8 +- src-tauri/src/models.rs | 10 +++ src-tauri/src/oauth.rs | 83 +++++++++++++++++-- src/components/OAuthConfigurator.tsx | 24 +++++- src/hooks/useOAuth2TokenActions.ts | 24 +++++- src/services/oauth.ts | 20 ++++- src/test/oauthCancel.test.tsx | 117 +++++++++++++++++++++++++++ 7 files changed, 273 insertions(+), 13 deletions(-) create mode 100644 src/test/oauthCancel.test.tsx diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 770f4e8..5b06d36 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -8,7 +8,7 @@ mod websocket; use std::collections::HashMap; use std::sync::Mutex; -use models::{ActiveStreams, ClientWrapper}; +use models::{ActiveStreams, ClientWrapper, PendingOAuthFlows}; use websocket::ActiveWebSockets; #[cfg_attr(mobile, tauri::mobile_entry_point)] @@ -21,6 +21,10 @@ pub fn run() { connections: Mutex::new(HashMap::new()), }; + let pending_oauth_flows = PendingOAuthFlows { + flows: Mutex::new(HashMap::new()), + }; + tauri::Builder::default() .plugin(tauri_plugin_updater::Builder::new().build()) .plugin(tauri_plugin_http::init()) @@ -29,12 +33,14 @@ pub fn run() { .manage(ClientWrapper::new()) .manage(active_streams) .manage(active_websockets) + .manage(pending_oauth_flows) .invoke_handler(tauri::generate_handler![ http_client::send_request, streaming::stream_sse, streaming::cancel_stream, oauth::oauth2_token_exchange, oauth::oauth2_auth_code_flow, + oauth::oauth2_cancel_flow, oauth::oauth2_refresh, websocket::ws_connect, websocket::ws_send, diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index 7db8db7..e1a1634 100644 --- a/src-tauri/src/models.rs +++ b/src-tauri/src/models.rs @@ -144,3 +144,13 @@ impl ClientWrapper { pub struct ActiveStreams { pub streams: Mutex>>, } + +/// In-flight authorization code flows, keyed by a frontend-supplied id. +/// +/// The flow parks on a loopback listener waiting for the provider to redirect +/// back. If the provider refuses to redirect at all — an unregistered redirect +/// URI is the usual reason — nothing ever arrives, and without this the user is +/// stuck watching a spinner until the timeout expires with no way out. +pub struct PendingOAuthFlows { + pub flows: Mutex>>, +} diff --git a/src-tauri/src/oauth.rs b/src-tauri/src/oauth.rs index e079343..2cf4e2a 100644 --- a/src-tauri/src/oauth.rs +++ b/src-tauri/src/oauth.rs @@ -8,12 +8,18 @@ use tauri::Url; use tauri_plugin_http::reqwest; use tauri_plugin_opener::OpenerExt; -use crate::models::ClientWrapper; +use crate::models::{ClientWrapper, PendingOAuthFlows}; const OAUTH_TOKEN_ACCEPT_HEADER: &str = "application/json, application/x-www-form-urlencoded, text/plain"; const TOKEN_CONTAINER_KEYS: &[&str] = &["data", "token", "result", "response"]; +/// How long to wait for the provider to redirect back to the loopback listener. +/// +/// Deliberately generous: a real sign-in can involve MFA, an account chooser and +/// a password manager. Cancelling is the UI's job, not the timeout's. +const AUTH_CALLBACK_TIMEOUT_SECS: u64 = 300; + #[derive(Debug, Deserialize)] pub struct OAuth2TokenExchangeOptions { token_url: String, @@ -43,6 +49,10 @@ pub struct OAuth2AuthCodeOptions { scope: Option, use_pkce: Option, redirect_uri: Option, + /// Identifies this flow so the UI can cancel it. Optional — a flow started + /// without one simply cannot be cancelled, which keeps older callers working. + #[serde(default)] + flow_id: Option, } #[derive(Debug, Deserialize)] @@ -452,6 +462,7 @@ pub async fn oauth2_auth_code_flow( options: OAuth2AuthCodeOptions, app: tauri::AppHandle, client_wrapper: tauri::State<'_, ClientWrapper>, + pending_flows: tauri::State<'_, PendingOAuthFlows>, ) -> Result { let auth_url = required_field(options.auth_url, "auth_url")?; let token_url = required_field(options.token_url, "token_url")?; @@ -533,13 +544,46 @@ pub async fn oauth2_auth_code_flow( .open_url(auth_uri.as_str(), None::<&str>) .map_err(|e| format!("Failed to open browser: {}", e))?; - let (code, received_state) = tokio::time::timeout( - std::time::Duration::from_secs(120), - wait_for_callback(listener), - ) - .await - .map_err(|_| "Authorization timed out after 2 minutes".to_string())? - .map_err(|e| format!("Callback error: {}", e))?; + // Register a cancel channel so the UI can abort a flow that is never going + // to complete. Signing in can legitimately take a while (MFA, a password + // manager, picking an account), so the timeout is generous and the cancel + // button is the real escape hatch. + let (cancel_tx, mut cancel_rx) = tokio::sync::watch::channel(false); + let flow_id = normalize_optional_input(options.flow_id); + if let Some(id) = &flow_id { + pending_flows + .flows + .lock() + .map_err(|_| "Failed to register the authorization flow".to_string())? + .insert(id.clone(), cancel_tx); + } + + let outcome = tokio::select! { + result = tokio::time::timeout( + std::time::Duration::from_secs(AUTH_CALLBACK_TIMEOUT_SECS), + wait_for_callback(listener), + ) => match result { + Ok(inner) => inner.map_err(|e| format!("Callback error: {}", e)), + Err(_) => Err(format!( + "Timed out after {} minutes waiting for the provider to redirect back. \ + If your browser showed an error instead of a sign-in page, register \ + exactly this callback URL with the provider: {}", + AUTH_CALLBACK_TIMEOUT_SECS / 60, + redirect_uri + )), + }, + _ = cancel_rx.changed() => Err("Authorization cancelled".to_string()), + }; + + // Always deregister, whichever way the flow ended, so a retry with the same + // id does not find a stale sender. + if let Some(id) = &flow_id { + if let Ok(mut flows) = pending_flows.flows.lock() { + flows.remove(id); + } + } + + let (code, received_state) = outcome?; if received_state != state { return Err("State mismatch - possible CSRF attack".to_string()); @@ -574,6 +618,29 @@ pub async fn oauth2_auth_code_flow( parse_oauth_token_response(res).await } +/// Abort an in-flight authorization code flow. +/// +/// Returns whether a flow was actually waiting: the UI uses that to tell "we +/// stopped it" apart from "it had already finished or timed out". +#[tauri::command] +pub async fn oauth2_cancel_flow( + flow_id: String, + pending_flows: tauri::State<'_, PendingOAuthFlows>, +) -> Result { + let sender = pending_flows + .flows + .lock() + .map_err(|_| "Failed to read in-flight authorization flows".to_string())? + .remove(&flow_id); + + match sender { + // The receiver side treats any change as cancellation, so the value + // only has to differ from the `false` it was created with. + Some(sender) => Ok(sender.send(true).is_ok()), + None => Ok(false), + } +} + async fn wait_for_callback(listener: tokio::net::TcpListener) -> Result<(String, String), String> { use tokio::io::{AsyncReadExt, AsyncWriteExt}; diff --git a/src/components/OAuthConfigurator.tsx b/src/components/OAuthConfigurator.tsx index 6842f12..e8b1704 100644 --- a/src/components/OAuthConfigurator.tsx +++ b/src/components/OAuthConfigurator.tsx @@ -11,7 +11,7 @@ import { useEnvironmentStore } from "@/store/environments" import { detectEntraV1Url, fetchOidcDiscovery } from "@/utils/oidcDiscovery" import { substituteVariables } from "@/utils/variables" import { decodeToken } from "@/utils/jwt" -import { Loader2, KeyRound, RefreshCw, Globe, Shield, Wand2 } from "lucide-react" +import { Loader2, KeyRound, RefreshCw, Globe, Shield, Wand2, X } from "lucide-react" import { useMemo, useState } from "react" interface OAuthConfiguratorProps { @@ -75,6 +75,7 @@ export function OAuthConfigurator({ oauth2, onOAuth2Change }: OAuthConfiguratorP getNewToken, refreshToken, clearToken, + cancelTokenRequest, isExpired, expiresIn, } = useOAuth2TokenActions({ oauth2, onOAuth2Change }) @@ -456,7 +457,7 @@ export function OAuthConfigurator({ oauth2, onOAuth2Change }: OAuthConfiguratorP {isLoading ? ( <> - Getting Token… + {cancelTokenRequest ? 'Waiting for sign-in…' : 'Getting Token…'} ) : ( <> @@ -466,6 +467,25 @@ export function OAuthConfigurator({ oauth2, onOAuth2Change }: OAuthConfiguratorP )} + {/* + Only the authorization code flow parks waiting on the browser, and it + can wait forever: if the redirect URI is not registered the provider + shows an error page and never redirects back, so nothing ever arrives + on the callback listener. + */} + {cancelTokenRequest && ( + + )} + {oauth2.refreshToken && (