Skip to content

Add static OAuth client settings for MCP servers #2129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SoccerField24x7 wants to merge 7 commits into
base: main
Choose a base branch
from
+1,003 −100
Open

Add static OAuth client settings for MCP servers #2129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
382678b
feat(mcp): add static OAuth client configuration (#2096)
SoccerField24x7 Oct 21, 2026
9bcf502
fix(mcp): preserve SDK OAuth resource (#2096)
SoccerField24x7 Oct 21, 2026
b75d677
Merge branch 'main' into enhancement/2096-static-mcp-oauth
SoccerField24x7 Oct 21, 2026
cfdd03d
fix(mcp): keep OAuth requests consistent (#2096)
SoccerField24x7 Oct 21, 2026
3589f5f
fix(mcp): persist OAuth request constraints (#2096)
SoccerField24x7 Oct 21, 2026
ec652a7
test(mcp): cover OAuth creation constraints (#2096)
SoccerField24x7 Oct 21, 2026
8cb827e
fix(mcp): close OAuth lifecycle gaps (#2096)
SoccerField24x7 Oct 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions src/eslint-suppressions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1481,7 +1481,7 @@
},
"services/mcp/McpHub.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 8
"count": 7
}
},
"services/mcp/McpOAuthClientProvider.ts": {
Expand All @@ -1501,19 +1501,14 @@
},
"services/mcp/__tests__/McpOAuthClientProvider.spec.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 20
"count": 18
}
},
"services/mcp/__tests__/SecretStorageService.spec.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 1
}
},
"services/mcp/utils/__tests__/callbackServer.spec.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 4
}
},
"services/mcp/utils/__tests__/oauth.spec.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 9
Expand Down
65 changes: 49 additions & 16 deletions src/services/mcp/McpHub.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
const sseFieldsErrorMessage =
"For 'sse' type servers, you must provide a 'url' field and can optionally include 'headers'"
const streamableHttpFieldsErrorMessage =
"For 'streamable-http' type servers, you must provide a 'url' field and can optionally include 'headers'"
"For 'streamable-http' type servers, you must provide a 'url' field and can optionally include 'headers' and 'oauth'"

Check warning on line 87 in src/services/mcp/McpHub.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory 

src/services/mcp/McpHub.ts:87: Survived StringLiteral mutant (replacement: ""). See the job summary for the complete list and resolution guidance.
const mixedFieldsErrorMessage =
"Cannot mix 'stdio' and ('sse' or 'streamable-http') fields. For 'stdio' use 'command', 'args', and 'env'. For 'sse'/'streamable-http' use 'url' and 'headers'"
const missingFieldsErrorMessage =
Expand All @@ -103,6 +103,7 @@
// Ensure no SSE fields are present
url: z.undefined().optional(),
headers: z.undefined().optional(),
oauth: z.undefined().optional(),
})
.transform((data) => ({
...data,
Expand All @@ -114,6 +115,7 @@
type: z.enum(["sse"]).optional(),
url: z.string().url("URL must be a valid URL format"),
headers: z.record(z.string(), z.string()).optional(),
oauth: z.undefined().optional(),
// Ensure no stdio fields are present
command: z.undefined().optional(),
args: z.undefined().optional(),
Expand All @@ -129,6 +131,14 @@
type: z.enum(["streamable-http"]).optional(),
url: z.string().url("URL must be a valid URL format"),
headers: z.record(z.string(), z.string()).optional(),
oauth: z
.object({

Check warning on line 135 in src/services/mcp/McpHub.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory 

src/services/mcp/McpHub.ts:135: Survived ObjectLiteral mutant (replacement: {}). See the job summary for the complete list and resolution guidance.
clientId: z.string().min(1, "OAuth clientId cannot be empty"),

Check warning on line 136 in src/services/mcp/McpHub.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory 

src/services/mcp/McpHub.ts:136: 2 mutation test gaps; example: Survived MethodExpression mutant (replacement: z.string().max(1, "OAuth clientId cannot be empty")). See the job summary for the complete list and resolution guidance.
scope: z.string().optional(),
redirectUri: z.string().url("OAuth redirectUri must be a valid URL").optional(),

Check warning on line 138 in src/services/mcp/McpHub.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory 

src/services/mcp/McpHub.ts:138: Survived StringLiteral mutant (replacement: ""). See the job summary for the complete list and resolution guidance.
resource: z.union([z.literal("auto"), z.literal(false), z.string().url()]).optional(),

Check warning on line 139 in src/services/mcp/McpHub.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory 

src/services/mcp/McpHub.ts:139: 3 mutation test gaps; example: Survived ArrayDeclaration mutant (replacement: []). See the job summary for the complete list and resolution guidance.
})
.optional(),
// Ensure no stdio fields are present
command: z.undefined().optional(),
args: z.undefined().optional(),
Expand Down Expand Up @@ -713,6 +723,7 @@
// Set up file watchers for enabled servers
this.setupFileWatcher(name, config, source)

let pendingAuthProvider: McpOAuthClientProvider | undefined
try {
const client = new Client(
{
Expand Down Expand Up @@ -821,19 +832,26 @@
// 1. SecretStorage has OAuth data → known OAuth server → discover
// 2. In-memory negative cache says non-OAuth → skip discovery
// 3. Unknown server (first connection) → discover once, cache result
const hasOAuthData = await this.secretStorage.hasOAuthData(configInjected.url)
const oauthClientId = configInjected.oauth?.clientId

Check warning on line 835 in src/services/mcp/McpHub.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory 

src/services/mcp/McpHub.ts:835: Survived OptionalChaining mutant (replacement: configInjected.oauth.clientId). See the job summary for the complete list and resolution guidance.
const hasOAuthData = await this.secretStorage.hasOAuthData(configInjected.url, oauthClientId)
const knownNonOAuth = McpOAuthClientProvider.isKnownNonOAuth(configInjected.url)
const skipDiscovery = !hasOAuthData && knownNonOAuth
const skipDiscovery = !oauthClientId && !hasOAuthData && knownNonOAuth

Check warning on line 838 in src/services/mcp/McpHub.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory 

src/services/mcp/McpHub.ts:838: 6 mutation test gaps; example: NoCoverage BooleanLiteral mutant (replacement: hasOAuthData). See the job summary for the complete list and resolution guidance.

const authProvider = await McpOAuthClientProvider.create(configInjected.url, this.secretStorage, name, {
skipDiscovery,
...configInjected.oauth,
})
pendingAuthProvider = authProvider
if (this.isDisposed) {
await authProvider.close()
return
}

// Pre-register the OAuth client so the SDK can skip its own
// registration step (broken for path-prefixed issuers — see
// utils/oauth.ts for upstream issue links).
// Skip when discovery was skipped — there's no metadata to register with.
if (!skipDiscovery) {
if (!skipDiscovery && !oauthClientId) {
try {
await authProvider.registerClientIfNeeded()
} catch {
Expand Down Expand Up @@ -979,6 +997,7 @@
authProvider: streamableHttpAuthProvider,
}
this.connections.push(connection)
pendingAuthProvider = undefined

// Connect (this will automatically start the transport)
try {
Expand Down Expand Up @@ -1033,6 +1052,7 @@
connection.server.resources = await this.fetchResourcesList(name, source)
connection.server.resourceTemplates = await this.fetchResourceTemplatesList(name, source)
} catch (error) {
await pendingAuthProvider?.close()
// Update status with error
const connection = this.findConnection(name, source)
if (connection) {
Expand Down Expand Up @@ -1066,7 +1086,8 @@
transport: StreamableHTTPClientTransport,
connection: ConnectedMcpConnection,
): Promise<void> {
const serverUrl = "url" in config ? (config as any).url : undefined
const serverUrl = "url" in config ? config.url : undefined
const oauthClientId = config.type === "streamable-http" ? config.oauth?.clientId : undefined
if (!serverUrl || !this.secretStorage) {
return
}
Expand All @@ -1078,21 +1099,27 @@
// Note: onDidChange fires on both writes AND deletes — the re-read below is
// authoritative; we only reconnect if it returns a valid (non-expired) token.
let tokenChangedDuringRead = false
const unsubscribeCrossWindow = this.secretStorage.onDidChange(serverUrl, () => {
tokenChangedDuringRead = true
})
const unsubscribeCrossWindow = this.secretStorage.onDidChange(
serverUrl,
() => {
tokenChangedDuringRead = true
},
oauthClientId,
)

// Check if another window already saved valid tokens.
// Re-read after registering the watcher in case a write landed in the gap.
const existing = await this.secretStorage.getOAuthData(serverUrl)
const existing = await this.secretStorage.getOAuthData(serverUrl, oauthClientId)
if (this.isDisposed) {
unsubscribeCrossWindow()
await authProvider.close()
return
}
// If the first read missed but the watcher fired during it, re-read — a write
// may have landed between subscription and read completion.
const tokenToUse =
existing ?? (tokenChangedDuringRead ? await this.secretStorage.getOAuthData(serverUrl) : undefined)
existing ??
(tokenChangedDuringRead ? await this.secretStorage.getOAuthData(serverUrl, oauthClientId) : undefined)
unsubscribeCrossWindow()

if (!this.isDisposed && tokenToUse && Date.now() < tokenToUse.expires_at - TOKEN_EXPIRY_BUFFER_MS) {
Expand Down Expand Up @@ -1122,6 +1149,7 @@
connection,
progress,
cancellationToken,
oauthClientId,
),
)
}
Expand All @@ -1136,6 +1164,7 @@
connection: ConnectedMcpConnection,
progress: vscode.Progress<{ message?: string; increment?: number }>,
cancellationToken: vscode.CancellationToken,
oauthClientId?: string,
): Promise<void> {
const watcherKey = `${name}:${source}`

Expand Down Expand Up @@ -1170,7 +1199,7 @@
return
}
try {
const data = await this.secretStorage?.getOAuthData(serverUrl)
const data = await this.secretStorage?.getOAuthData(serverUrl, oauthClientId)
// Re-check after the async yield — cancellation or completion may have
// fired while getOAuthData was in flight.
if (disposed || this.isDisposed) return
Expand All @@ -1188,9 +1217,13 @@
}
}

const unsubscribe = this.secretStorage!.onDidChange(serverUrl, () => {
void onTokensChanged()
})
const unsubscribe = this.secretStorage!.onDidChange(
serverUrl,
() => {
void onTokensChanged()
},
oauthClientId,
)

// --- Timeout ---
const timeoutHandle = setTimeout(() => {
Expand Down Expand Up @@ -1273,7 +1306,7 @@
}

// Guard: another window may have authed while the toast was open
const tokens = await this.secretStorage!.getOAuthData(serverUrl)
const tokens = await this.secretStorage!.getOAuthData(serverUrl, oauthClientId)
if (disposed) return
if (tokens && Date.now() < tokens.expires_at - TOKEN_EXPIRY_BUFFER_MS) {
cleanup(cancellationDisposable)
Expand Down Expand Up @@ -1757,7 +1790,7 @@

// Clear OAuth tokens for streamable-http servers on restart
if (validatedConfig.type === "streamable-http" && this.secretStorage) {
await this.secretStorage.deleteOAuthData(validatedConfig.url)
await this.secretStorage.deleteOAuthData(validatedConfig.url, validatedConfig.oauth?.clientId)
}

// Try to connect again using validated config
Expand Down
Loading
Loading