fix: improve mcp oauth connection flow

This commit is contained in:
centdix
2026-06-08 17:23:48 +02:00
parent 6156e2372a
commit fb585d7737
5 changed files with 481 additions and 149 deletions
+15 -3
View File
@@ -101,9 +101,21 @@ pub(crate) async fn get_mcp_tools(
None
};
let client = windmill_mcp::McpClient::from_resource(mcp_resource, token)
.await
.map_err(|e| Error::ExecutionErr(format!("Failed to connect to MCP server: {}", e)))?;
let client = match windmill_mcp::McpClient::from_resource(mcp_resource, token).await {
Ok(client) => client,
Err(e) if windmill_mcp::is_auth_required_error(&e) => {
return Err(Error::NotAuthorized(format!(
"MCP server requires authentication: {}",
e
)));
}
Err(e) => {
return Err(Error::ExecutionErr(format!(
"Failed to connect to MCP server: {}",
e
)));
}
};
let tools: Vec<serde_json::Value> = client
.available_tools()
+27 -2
View File
@@ -14,9 +14,10 @@ use rmcp::{
CallToolRequestParams, ClientCapabilities, ClientInfo, Implementation,
InitializeRequestParams, Tool as McpTool,
},
service::RunningService,
service::{ClientInitializeError, RunningService},
transport::{
streamable_http_client::StreamableHttpClientTransportConfig, StreamableHttpClientTransport,
streamable_http_client::{StreamableHttpClientTransportConfig, StreamableHttpError},
StreamableHttpClientTransport,
},
RoleClient, ServiceExt,
};
@@ -31,6 +32,30 @@ pub struct McpClient {
available_tools: Vec<McpTool>,
}
pub fn is_auth_required_error(error: &anyhow::Error) -> bool {
error.chain().any(|source| {
if matches!(
source.downcast_ref::<StreamableHttpError<reqwest::Error>>(),
Some(StreamableHttpError::AuthRequired(_))
) {
return true;
}
if let Some(ClientInitializeError::TransportError { error, .. }) =
source.downcast_ref::<ClientInitializeError>()
{
return matches!(
error
.error
.downcast_ref::<StreamableHttpError<reqwest::Error>>(),
Some(StreamableHttpError::AuthRequired(_))
);
}
false
})
}
impl McpClient {
/// Create a new MCP client from a resource configuration.
///
+1 -1
View File
@@ -20,7 +20,7 @@ pub use common::{
};
// Re-export client types at crate root for backward compatibility
pub use client::{McpClient, McpResource, McpToolSource};
pub use client::{is_auth_required_error, McpClient, McpResource, McpToolSource};
// Re-export rmcp types for client usage
pub use rmcp::model::Tool as McpTool;
@@ -12,14 +12,32 @@
import Path from '$lib/components/Path.svelte'
import { sendUserToast } from '$lib/toast'
import { sameTopDomainOrigin } from '$lib/cookies'
import { onDestroy } from 'svelte'
import { onDestroy, onMount } from 'svelte'
interface Props {
onConnected: (resourcePath: string, resourceName: string) => void
onCancel: () => void
onCancel?: () => void
onDiscoveryStatus?: (
status: 'discovering' | 'supported' | 'unsupported',
error?: string
) => void
initialServerUrl?: string
initialResourceName?: string
initialResourcePath?: string
updateExistingResource?: boolean
hideUntilDiscovered?: boolean
}
let { onConnected, onCancel }: Props = $props()
let {
onConnected,
onCancel,
onDiscoveryStatus,
initialServerUrl = '',
initialResourceName = '',
initialResourcePath = '',
updateExistingResource = false,
hideUntilDiscovered = false
}: Props = $props()
let serverUrl = $state('')
let discoveryResult = $state<DiscoverMcpOauthResponse | null>(null)
@@ -27,29 +45,60 @@
let resourceName = $state('')
let resourcePath = $state('')
let pathError = $state('')
let status = $state<'idle' | 'discovering' | 'discovered' | 'connecting'>('idle')
let status = $state<'idle' | 'discovering' | 'discovered' | 'unsupported' | 'connecting'>(
'idle'
)
let error = $state<string | null>(null)
let connectDisabled = $derived(
!resourcePath || (!updateExistingResource && pathError !== '')
)
let shouldRender = $derived(
!hideUntilDiscovered || status === 'discovered' || status === 'connecting'
)
initializeFromProps()
function initializeFromProps() {
serverUrl = initialServerUrl
resourceName = initialResourceName
resourcePath = initialResourcePath
if (initialServerUrl) {
status = 'discovering'
}
}
onMount(() => {
if (initialServerUrl) {
void discoverOAuth()
}
})
async function discoverOAuth() {
status = 'discovering'
error = null
onDiscoveryStatus?.('discovering')
try {
discoveryResult = await McpOauthService.discoverMcpOauth({
requestBody: { mcp_server_url: serverUrl }
})
selectedScopes = discoveryResult?.scopes_supported ?? []
try {
const urlObj = new URL(serverUrl)
resourceName = urlObj.hostname.replace(/\./g, '_')
} catch {
resourceName = 'mcp_server'
if (!resourceName.trim()) {
try {
const urlObj = new URL(serverUrl)
resourceName = urlObj.hostname.replace(/\./g, '_')
} catch {
resourceName = 'mcp_server'
}
}
status = 'discovered'
} catch (e) {
onDiscoveryStatus?.('supported')
} catch (e: any) {
console.error('Error discovering OAuth settings', e)
const errorMessage = e.body?.message || e.body || e.message || 'Unknown error'
error = `Failed to discover OAuth settings: ${errorMessage}`
status = 'idle'
status = 'unsupported'
onDiscoveryStatus?.('unsupported', error)
}
}
@@ -102,6 +151,38 @@
window.removeEventListener('storage', handleStorageEvent)
}
function getVariablePathFromTokenRef(token: unknown): string | undefined {
if (typeof token !== 'string') {
return undefined
}
const variablePath = token.trim().startsWith('$var:')
? token.trim().slice('$var:'.length).trim()
: ''
return variablePath.length > 0 ? variablePath : undefined
}
async function getTokenVariablePath(existingTokenPath?: string): Promise<string> {
if (existingTokenPath) {
return existingTokenPath
}
const basePath = `${resourcePath}_token`
for (let index = 0; index < 20; index += 1) {
const candidatePath = index === 0 ? basePath : `${basePath}_${index + 1}`
const exists = await VariableService.existsVariable({
workspace: $workspaceStore!,
path: candidatePath
})
if (!exists) {
return candidatePath
}
}
throw new Error('Could not find an available variable path for the MCP OAuth token')
}
async function createMcpResource(data: {
access_token: string
refresh_token?: string
@@ -123,31 +204,79 @@
accountId = Number(accountIdStr)
}
await VariableService.createVariable({
let existingValue: Record<string, unknown> = {}
if (updateExistingResource) {
const existingResource = await ResourceService.getResource({
workspace: $workspaceStore!,
path: resourcePath
})
existingValue =
existingResource.value && typeof existingResource.value === 'object'
? (existingResource.value as Record<string, unknown>)
: {}
}
const existingTokenPath = getVariablePathFromTokenRef(existingValue.token)
const tokenVariablePath = await getTokenVariablePath(existingTokenPath)
const variableDescription = `MCP OAuth token for ${data.mcp_server_url}`
const variableExists = await VariableService.existsVariable({
workspace: $workspaceStore!,
requestBody: {
path: resourcePath,
value: data.access_token,
is_secret: true,
is_oauth: true,
account: accountId,
description: `MCP OAuth token for ${data.mcp_server_url}`
}
path: tokenVariablePath
})
await ResourceService.createResource({
workspace: $workspaceStore!,
requestBody: {
resource_type: 'mcp',
if (variableExists) {
await VariableService.updateVariable({
workspace: $workspaceStore!,
path: tokenVariablePath,
requestBody: {
path: tokenVariablePath,
value: data.access_token,
is_secret: true,
description: variableDescription
}
})
} else {
await VariableService.createVariable({
workspace: $workspaceStore!,
requestBody: {
path: tokenVariablePath,
value: data.access_token,
is_secret: true,
is_oauth: true,
account: accountId,
description: variableDescription
}
})
}
const updatedResourceValue = {
name: resourceName,
url: data.mcp_server_url,
token: `$var:${tokenVariablePath}`
}
if (updateExistingResource) {
await ResourceService.updateResourceValue({
workspace: $workspaceStore!,
path: resourcePath,
value: {
name: resourceName,
url: data.mcp_server_url,
token: `$var:${resourcePath}`
},
description: `MCP server connected via OAuth`
}
})
requestBody: {
value: {
...existingValue,
...updatedResourceValue
}
}
})
} else {
await ResourceService.createResource({
workspace: $workspaceStore!,
requestBody: {
resource_type: 'mcp',
path: resourcePath,
value: updatedResourceValue,
description: `MCP server connected via OAuth`
}
})
}
sendUserToast('Connected to MCP server')
onConnected(resourcePath, resourceName)
@@ -160,84 +289,105 @@
onDestroy(cleanup)
</script>
<div class="border rounded p-4 bg-surface-secondary flex flex-col gap-4">
<div class="flex justify-between items-center">
<span class="font-semibold text-sm">Connect MCP Server with OAuth</span>
<Button size="xs" color="light" onClick={onCancel}>Cancel</Button>
</div>
<Label label="MCP Server URL">
<input
type="url"
bind:value={serverUrl}
placeholder="https://mcp.example.com"
class="text-sm w-full"
disabled={status === 'connecting'}
/>
</Label>
{#if status === 'idle'}
<Button size="sm" onClick={discoverOAuth} disabled={!serverUrl}>Discover OAuth Settings</Button>
{:else if status === 'discovering'}
<div class="text-sm text-secondary">Discovering OAuth settings...</div>
{:else if status === 'discovered' && discoveryResult}
<div class="text-xs text-green-600 dark:text-green-400">
&#10003; OAuth supported
{#if discoveryResult.supports_dynamic_registration}
(Dynamic Client Registration available)
{#if shouldRender}
<div class="border rounded p-4 bg-surface-secondary flex flex-col gap-4">
<div class="flex justify-between items-center">
<span class="font-semibold text-sm">
{updateExistingResource
? 'Connect selected MCP resource with OAuth'
: 'Connect MCP Server with OAuth'}
</span>
{#if onCancel}
<Button size="xs" color="light" onClick={onCancel}>Cancel</Button>
{/if}
</div>
{#if discoveryResult.scopes_supported && discoveryResult.scopes_supported.length > 0}
<Label label="Select Scopes">
<div class="flex flex-col flex-wrap gap-2">
{#each discoveryResult.scopes_supported as scope}
<label class="flex flex-row items-center gap-2 text-xs cursor-pointer">
<input
type="checkbox"
checked={selectedScopes.includes(scope)}
onchange={(e) => {
const target = e.target as HTMLInputElement
if (target.checked) {
selectedScopes = [...selectedScopes, scope]
} else {
selectedScopes = selectedScopes.filter((s) => s !== scope)
}
}}
class="!w-4 !h-4"
/>
{scope}
</label>
{/each}
</div>
</Label>
{/if}
<Label label="Resource Name">
<Label label="MCP Server URL">
<input
type="text"
bind:value={resourceName}
placeholder="my-mcp-server"
type="url"
bind:value={serverUrl}
placeholder="https://mcp.example.com"
class="text-sm w-full"
disabled={status === 'connecting'}
/>
</Label>
<Path
bind:path={resourcePath}
bind:error={pathError}
initialPath=""
namePlaceholder={resourceName}
kind="resource"
/>
{#if status === 'idle'}
<Button size="sm" onClick={discoverOAuth} disabled={!serverUrl}>Discover OAuth Settings</Button>
{:else if status === 'discovering'}
<div class="text-sm text-secondary">Discovering OAuth settings...</div>
{:else if status === 'unsupported'}
<div class="flex flex-col gap-1 text-xs text-red-600 dark:text-red-400">
<div>OAuth is not available for this MCP server.</div>
{#if error}
<div>{error}</div>
{/if}
</div>
{:else if status === 'discovered' && discoveryResult}
<div class="text-xs text-green-600 dark:text-green-400">
&#10003; OAuth supported
{#if discoveryResult.supports_dynamic_registration}
(Dynamic Client Registration available)
{/if}
</div>
<Button size="sm" onClick={startOAuth} disabled={!resourcePath || pathError !== ''}>
Connect with OAuth
</Button>
{:else if status === 'connecting'}
<div class="text-sm text-secondary">Complete authentication in popup window...</div>
{/if}
{#if discoveryResult.scopes_supported && discoveryResult.scopes_supported.length > 0}
<Label label="Select Scopes">
<div class="flex flex-col flex-wrap gap-2">
{#each discoveryResult.scopes_supported as scope (scope)}
<label class="flex flex-row items-center gap-2 text-xs cursor-pointer">
<input
type="checkbox"
checked={selectedScopes.includes(scope)}
onchange={(e) => {
const target = e.target as HTMLInputElement
if (target.checked) {
selectedScopes = [...selectedScopes, scope]
} else {
selectedScopes = selectedScopes.filter((s) => s !== scope)
}
}}
class="!w-4 !h-4"
/>
{scope}
</label>
{/each}
</div>
</Label>
{/if}
{#if error}
<div class="text-xs text-red-600 dark:text-red-400">{error}</div>
{/if}
</div>
<Label label="Resource Name">
<input
type="text"
bind:value={resourceName}
placeholder="my-mcp-server"
class="text-sm w-full"
/>
</Label>
{#if updateExistingResource}
<Label label="Resource Path">
<div class="text-sm w-full rounded border bg-surface px-2 py-1">{resourcePath}</div>
</Label>
{:else}
<Path
bind:path={resourcePath}
bind:error={pathError}
initialPath=""
namePlaceholder={resourceName}
kind="resource"
/>
{/if}
<Button size="sm" onClick={startOAuth} disabled={connectDisabled}>
Connect with OAuth
</Button>
{:else if status === 'connecting'}
<div class="text-sm text-secondary">Complete authentication in popup window...</div>
{/if}
{#if error && status !== 'unsupported'}
<div class="text-xs text-red-600 dark:text-red-400">{error}</div>
{/if}
</div>
{/if}
@@ -27,32 +27,146 @@
import ResourcePicker from '$lib/components/ResourcePicker.svelte'
import { usePromise } from '$lib/svelte5Utils.svelte'
import { untrack } from 'svelte'
import Alert from '$lib/components/common/alert/Alert.svelte'
import McpOAuthConnect from './McpOAuthConnect.svelte'
import Alert from '$lib/components/common/alert/Alert.svelte'
interface Props {
tool: McpTool
}
type McpResourceValue = {
name?: string
url: string
token?: string | null
}
type OAuthDiscoveryStatus = 'none' | 'discovering' | 'supported' | 'unsupported'
function getMcpResourceValue(value: unknown): McpResourceValue | undefined {
if (!value || typeof value !== 'object') {
return undefined
}
const candidate = value as Record<string, unknown>
const url = typeof candidate.url === 'string' ? candidate.url : undefined
if (!url) {
return undefined
}
return {
name: typeof candidate.name === 'string' ? candidate.name : undefined,
url,
token:
typeof candidate.token === 'string' || candidate.token === null
? candidate.token
: undefined
}
}
function getFallbackMcpResourceName(path: string, url?: string): string {
if (path) {
const parts = path.split('/')
return parts[parts.length - 1] || path
}
if (url) {
try {
return new URL(url).hostname.replace(/\./g, '_')
} catch {
return 'mcp_server'
}
}
return 'mcp_server'
}
function getErrorMessage(error: any): string | undefined {
return error?.body?.message || error?.body || error?.message
}
function isUnauthorizedError(error: any): boolean {
const status = error?.status
const message = String(getErrorMessage(error) ?? '')
return (
status === 401 ||
status === 403 ||
/\b(unauthorized|unauthorised|401|403)\b/i.test(message)
)
}
let { tool = $bindable() }: Props = $props()
let showOAuthForm = $state(false)
let refreshCount = $state(0)
let resourcePicker: ResourcePicker | undefined = $state()
let lastResourcePath = $state<string | undefined>()
let oauthDiscoveryStatus = $state<OAuthDiscoveryStatus>('none')
let resourcePath = $derived(tool.value.resource_path)
let tools = usePromise(
async () =>
await loadToolsCached({
workspace: $workspaceStore!,
path: tool.value.resource_path,
path: resourcePath,
refreshCount
}),
{ loadInit: false, clearValueOnRefresh: false }
{ loadInit: false }
)
let selectedResource = usePromise(
async () =>
resourcePath && $workspaceStore
? await ResourceService.getResource({
workspace: $workspaceStore,
path: resourcePath
})
: undefined,
{ loadInit: false }
)
let toolOptions = $derived(safeSelectItems((tools.value ?? []).map((t) => t.name)))
let resourcePath = $derived(tool.value.resource_path)
let error = $derived(tools.error?.body?.message || tools.error?.message)
let error = $derived(getErrorMessage(tools.error))
let selectedMcpResourceValue = $derived(getMcpResourceValue(selectedResource.value?.value))
let selectedMcpHasToken = $derived(
typeof selectedMcpResourceValue?.token === 'string' &&
selectedMcpResourceValue.token.trim().length > 0
)
let selectedMcpUrlOnly = $derived(Boolean(selectedMcpResourceValue?.url && !selectedMcpHasToken))
let selectedMcpServerUrl = $derived(selectedMcpResourceValue?.url)
let selectedMcpResourceName = $derived(
selectedMcpResourceValue?.name ||
getFallbackMcpResourceName(resourcePath, selectedMcpServerUrl)
)
let selectedMcpUnauthorized = $derived(
Boolean(selectedMcpUrlOnly && isUnauthorizedError(tools.error))
)
let connectionEstablished = $derived(
Boolean(resourcePath?.length > 0 && tools.status !== 'loading' && !tools.error && tools.value)
)
let oauthDiscoveryPending = $derived(
selectedMcpUnauthorized &&
(oauthDiscoveryStatus === 'none' || oauthDiscoveryStatus === 'discovering')
)
let showConnecting = $derived(
Boolean(
resourcePath?.length > 0 &&
!connectionEstablished &&
(tools.status === 'loading' || selectedResource.status === 'loading' || oauthDiscoveryPending)
)
)
let showOAuthFlow = $derived(
selectedMcpUnauthorized && oauthDiscoveryStatus !== 'unsupported'
)
let connectionError = $derived(
resourcePath?.length > 0 &&
!connectionEstablished &&
!showConnecting &&
(!selectedMcpUnauthorized || oauthDiscoveryStatus === 'unsupported')
? error
: undefined
)
$effect(() => {
resourcePath
@@ -65,6 +179,34 @@
})
})
$effect(() => {
resourcePath
$workspaceStore
untrack(() => {
if (resourcePath?.length > 0 && $workspaceStore) {
selectedResource.refresh()
} else {
selectedResource.clear()
}
})
})
$effect(() => {
const currentPath = resourcePath
untrack(() => {
if (currentPath !== lastResourcePath) {
lastResourcePath = currentPath
oauthDiscoveryStatus = 'none'
}
})
})
$effect(() => {
if (!selectedMcpUnauthorized && oauthDiscoveryStatus !== 'none') {
oauthDiscoveryStatus = 'none'
}
})
$effect(() => {
if (!tool.value.include_tools) {
tool.value.include_tools = []
@@ -80,11 +222,17 @@
}
})
async function handleOAuthConnected(resourcePath: string, resourceName: string) {
function handleOAuthDiscoveryStatus(status: Exclude<OAuthDiscoveryStatus, 'none'>) {
oauthDiscoveryStatus = status
}
async function handleOAuthConnected(connectedResourcePath: string, resourceName: string) {
await resourcePicker?.refreshResources()
tool.value.resource_path = resourcePath
tool.value.resource_path = connectedResourcePath
tool.summary = `MCP: ${resourceName}`
showOAuthForm = false
oauthDiscoveryStatus = 'none'
selectedResource.refresh()
refreshCount += 1
}
</script>
@@ -109,20 +257,29 @@
</Label>
</div>
{#if !resourcePath}
{#if !showOAuthForm}
<Button size="xs" color="light" onClick={() => (showOAuthForm = true)}>
Connect with OAuth
</Button>
{:else}
<McpOAuthConnect
onConnected={handleOAuthConnected}
onCancel={() => (showOAuthForm = false)}
/>
{/if}
{#if showConnecting}
<div class="text-xs text-secondary italic">connecting to mcp server...</div>
{/if}
{#if resourcePath?.length > 0}
{#if showOAuthFlow}
{#key `${resourcePath ?? ''}:${selectedMcpServerUrl ?? ''}`}
<McpOAuthConnect
onConnected={handleOAuthConnected}
onDiscoveryStatus={handleOAuthDiscoveryStatus}
initialServerUrl={selectedMcpServerUrl}
initialResourceName={selectedMcpResourceName}
initialResourcePath={resourcePath}
updateExistingResource={true}
hideUntilDiscovered={true}
/>
{/key}
{/if}
{#if connectionError}
<div class="text-xs text-red-600 dark:text-red-400">{`Failed to connect to MCP server: ${connectionError}`}</div>
{/if}
{#if connectionEstablished}
<div class="w-full">
<Label label="Summary">
<input
@@ -147,24 +304,12 @@
</Button>
{/snippet}
<div class="w-full flex flex-col gap-2">
{#if error}
<div class="text-xs text-red-600 dark:text-red-400 mb-4"
>{`Failed to load tools from MCP server: ${error}`}</div
>
{:else if tools.status === 'loading'}
<div class="max-h-48 overflow-y-auto border rounded p-2 bg-surface-secondary">
<div class="text-xs text-secondary italic">Loading tools...</div>
</div>
{:else if (tools.value ?? []).length === 0 && !error}
<div class="max-h-48 overflow-y-auto border rounded p-2 bg-surface-secondary">
<div class="text-xs text-secondary italic">
No tools loaded yet. Click "Refresh Tools" to fetch tools from the MCP server.
</div>
</div>
{:else if (tools.value ?? []).length > 0}
<div class="max-h-48 overflow-y-auto border rounded p-2 bg-surface-secondary">
<div class="max-h-48 overflow-y-auto border rounded p-2 bg-surface-secondary">
{#if (tools.value ?? []).length === 0}
<div class="text-xs text-secondary italic">No tools returned by this MCP server.</div>
{:else}
<div class="flex flex-col gap-1">
{#each tools.value ?? [] as mcpTool}
{#each tools.value ?? [] as mcpTool (mcpTool.name)}
<div class="text-xs">
<span class="font-semibold">{mcpTool.name}</span>
{#if mcpTool.description}
@@ -173,8 +318,8 @@
</div>
{/each}
</div>
</div>
{/if}
{/if}
</div>
</div>
</Section>