diff --git a/extensions/microsoft-authentication/src/common/scopeData.ts b/extensions/microsoft-authentication/src/common/scopeData.ts index 17549f5db1b..a5aee65f3c9 100644 --- a/extensions/microsoft-authentication/src/common/scopeData.ts +++ b/extensions/microsoft-authentication/src/common/scopeData.ts @@ -50,14 +50,14 @@ export class ScopeData { */ readonly claims?: string; - constructor(readonly originalScopes: readonly string[] = [], claims?: string, authorizationServer?: Uri) { + constructor(readonly originalScopes: readonly string[] = [], claims?: string, authorizationServer?: Uri, clientId?: string) { const modifiedScopes = [...originalScopes]; modifiedScopes.sort(); this.allScopes = modifiedScopes; this.scopeStr = modifiedScopes.join(' '); this.claims = claims; this.scopesToSend = this.getScopesToSend(modifiedScopes); - this.clientId = this.getClientId(this.allScopes); + this.clientId = clientId?.trim() || this.getClientId(this.allScopes); this.tenant = this.getTenant(this.allScopes, authorizationServer); this.tenantId = this.getTenantId(this.tenant); } diff --git a/extensions/microsoft-authentication/src/common/test/scopeData.test.ts b/extensions/microsoft-authentication/src/common/test/scopeData.test.ts index 3dc9d95aa14..2529a236bd5 100644 --- a/extensions/microsoft-authentication/src/common/test/scopeData.test.ts +++ b/extensions/microsoft-authentication/src/common/test/scopeData.test.ts @@ -108,4 +108,27 @@ suite('ScopeData', () => { const scopeData = new ScopeData(['custom_scope'], undefined, authorizationServer); assert.strictEqual(scopeData.tenant, 'tenant123'); }); + + test('should use explicit clientId when provided', () => { + const scopeData = new ScopeData(['custom_scope'], undefined, undefined, 'my-custom-client-id'); + assert.strictEqual(scopeData.clientId, 'my-custom-client-id'); + }); + + test('should use explicit clientId over VSCODE_CLIENT_ID scope', () => { + const scopeData = new ScopeData(['custom_scope', 'VSCODE_CLIENT_ID:scope_id'], undefined, undefined, 'override-id'); + assert.strictEqual(scopeData.clientId, 'override-id'); + }); + + test('should ignore empty or whitespace-only clientId and fall back to default', () => { + const empty = new ScopeData(['custom_scope'], undefined, undefined, ''); + assert.strictEqual(empty.clientId, 'aebc6443-996d-45c2-90f0-388ff96faa56'); + + const whitespace = new ScopeData(['custom_scope'], undefined, undefined, ' '); + assert.strictEqual(whitespace.clientId, 'aebc6443-996d-45c2-90f0-388ff96faa56'); + }); + + test('should ignore empty clientId and fall back to VSCODE_CLIENT_ID scope', () => { + const scopeData = new ScopeData(['custom_scope', 'VSCODE_CLIENT_ID:scope_id'], undefined, undefined, ''); + assert.strictEqual(scopeData.clientId, 'scope_id'); + }); }); diff --git a/extensions/microsoft-authentication/src/node/authProvider.ts b/extensions/microsoft-authentication/src/node/authProvider.ts index 95921b9e3a3..0a5b742cba6 100644 --- a/extensions/microsoft-authentication/src/node/authProvider.ts +++ b/extensions/microsoft-authentication/src/node/authProvider.ts @@ -170,7 +170,7 @@ export class MsalAuthProvider implements AuthenticationProvider { async getSessions(scopes: string[] | undefined, options: AuthenticationGetSessionOptions = {}): Promise { const askingForAll = scopes === undefined; - const scopeData = new ScopeData(scopes, undefined, options?.authorizationServer); + const scopeData = new ScopeData(scopes, undefined, options?.authorizationServer, options?.clientId); // Do NOT use `scopes` beyond this place in the code. Use `scopeData` instead. this._logger.info('[getSessions]', askingForAll ? '[all]' : `[${scopeData.scopeStr}]`, 'starting'); @@ -200,7 +200,7 @@ export class MsalAuthProvider implements AuthenticationProvider { } async createSession(scopes: readonly string[], options: AuthenticationProviderSessionOptions): Promise { - const scopeData = new ScopeData(scopes, undefined, options.authorizationServer); + const scopeData = new ScopeData(scopes, undefined, options.authorizationServer, options.clientId); // Do NOT use `scopes` beyond this place in the code. Use `scopeData` instead. this._logger.info('[createSession]', `[${scopeData.scopeStr}]`, 'starting'); @@ -315,7 +315,7 @@ export class MsalAuthProvider implements AuthenticationProvider { if (!claims) { throw new Error('No claims found in authentication challenges'); } - const scopeData = new ScopeData(scopes, claims, options?.authorizationServer); + const scopeData = new ScopeData(scopes, claims, options?.authorizationServer, options?.clientId); this._logger.info('[getSessionsFromChallenges]', `[${scopeData.scopeStr}]`, 'with claims:', scopeData.claims); const cachedPca = await this._publicClientManager.getOrCreate(scopeData.clientId); @@ -338,7 +338,7 @@ export class MsalAuthProvider implements AuthenticationProvider { // Use scopes if available, otherwise fall back to default scopes const effectiveScopes = scopes.length > 0 ? scopes : ['https://graph.microsoft.com/User.Read']; - const scopeData = new ScopeData(effectiveScopes, claims, options.authorizationServer); + const scopeData = new ScopeData(effectiveScopes, claims, options.authorizationServer, options.clientId); this._logger.info('[createSessionFromChallenges]', `[${scopeData.scopeStr}]`, 'starting with claims:', claims); const cachedPca = await this._publicClientManager.getOrCreate(scopeData.clientId); diff --git a/src/vs/platform/mcp/common/mcpPlatformTypes.ts b/src/vs/platform/mcp/common/mcpPlatformTypes.ts index dc4fb38172e..7445f62f0ee 100644 --- a/src/vs/platform/mcp/common/mcpPlatformTypes.ts +++ b/src/vs/platform/mcp/common/mcpPlatformTypes.ts @@ -61,10 +61,15 @@ export interface IMcpStdioServerConfiguration extends ICommonMcpServerConfigurat readonly dev?: IMcpDevModeConfig; } +export interface IMcpRemoteServerOAuthConfiguration { + readonly clientId?: string; +} + export interface IMcpRemoteServerConfiguration extends ICommonMcpServerConfiguration { readonly type: McpServerType.REMOTE; readonly url: string; readonly headers?: Record; + readonly oauth?: IMcpRemoteServerOAuthConfiguration; readonly dev?: IMcpDevModeConfig; } diff --git a/src/vs/workbench/api/browser/mainThreadAuthentication.ts b/src/vs/workbench/api/browser/mainThreadAuthentication.ts index bfd8397bf73..2028933f6b0 100644 --- a/src/vs/workbench/api/browser/mainThreadAuthentication.ts +++ b/src/vs/workbench/api/browser/mainThreadAuthentication.ts @@ -154,12 +154,12 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu this._register(authenticationService.registerAuthenticationProviderHostDelegate({ // Prefer Node.js extension hosts when they're available. No CORS issues etc. priority: extHostContext.extensionHostKind === ExtensionHostKind.LocalWebWorker ? 0 : 1, - create: async (authorizationServer, serverMetadata, resource) => { + create: async (authorizationServer, serverMetadata, resource, overrideClientId) => { // Auth Provider Id is a combination of the authorization server and the resource, if provided. const authProviderId = resource ? `${authorizationServer.toString(true)} ${resource.resource}` : authorizationServer.toString(true); const clientDetails = await this.dynamicAuthProviderStorageService.getClientRegistration(authProviderId); - let clientId = clientDetails?.clientId; - const clientSecret = clientDetails?.clientSecret; + let clientId = overrideClientId ?? clientDetails?.clientId; + const clientSecret = overrideClientId ? undefined : clientDetails?.clientSecret; let initialTokens: (IAuthorizationTokenResponse & { created_at: number })[] | undefined = undefined; if (clientId) { initialTokens = await this.dynamicAuthProviderStorageService.getSessionsForDynamicAuthProvider(authProviderId, clientId); diff --git a/src/vs/workbench/api/browser/mainThreadMcp.ts b/src/vs/workbench/api/browser/mainThreadMcp.ts index 7521c370ee9..a254d1c97be 100644 --- a/src/vs/workbench/api/browser/mainThreadMcp.ts +++ b/src/vs/workbench/api/browser/mainThreadMcp.ts @@ -223,10 +223,10 @@ export class MainThreadMcp extends Disposable implements MainThreadMcpShape { if (!server) { return undefined; } - return this._getSessionForProvider(id, server, providerId, scopes, undefined, options.errorOnUserInteraction); + return this._getSessionForProvider(id, server, providerId, scopes, undefined, options.errorOnUserInteraction, options.clientId); } - async $getTokenFromServerMetadata(id: number, authDetails: IMcpAuthenticationDetails, { errorOnUserInteraction, forceNewRegistration }: IMcpAuthenticationOptions = {}): Promise { + async $getTokenFromServerMetadata(id: number, authDetails: IMcpAuthenticationDetails, { errorOnUserInteraction, forceNewRegistration, clientId }: IMcpAuthenticationOptions = {}): Promise { const server = this._serverDefinitions.get(id); if (!server) { return undefined; @@ -246,14 +246,14 @@ export class MainThreadMcp extends Disposable implements MainThreadMcpShape { } if (!providerId) { - const provider = await this._authenticationService.createDynamicAuthenticationProvider(authorizationServer, authDetails.authorizationServerMetadata, authDetails.resourceMetadata); + const provider = await this._authenticationService.createDynamicAuthenticationProvider(authorizationServer, authDetails.authorizationServerMetadata, authDetails.resourceMetadata, authDetails.clientId); if (!provider) { return undefined; } providerId = provider.id; } - return this._getSessionForProvider(id, server, providerId, resolvedScopes, authorizationServer, errorOnUserInteraction); + return this._getSessionForProvider(id, server, providerId, resolvedScopes, authorizationServer, errorOnUserInteraction, clientId ?? authDetails.clientId); } private async _getSessionForProvider( @@ -262,9 +262,10 @@ export class MainThreadMcp extends Disposable implements MainThreadMcpShape { providerId: string, scopes: string[], authorizationServer?: URI, - errorOnUserInteraction: boolean = false + errorOnUserInteraction: boolean = false, + clientId?: string, ): Promise { - const sessions = await this._authenticationService.getSessions(providerId, scopes, { authorizationServer }, true); + const sessions = await this._authenticationService.getSessions(providerId, scopes, { authorizationServer, clientId }, true); const accountNamePreference = this.authenticationMcpServersService.getAccountPreference(server.id, providerId); let matchingAccountPreferenceSession: AuthenticationSession | undefined; if (accountNamePreference) { @@ -316,7 +317,8 @@ export class MainThreadMcp extends Disposable implements MainThreadMcpShape { { activateImmediate: true, account: accountToCreate, - authorizationServer + authorizationServer, + clientId }); } while ( accountToCreate diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 9d837bc351c..53e9780fa66 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -3510,11 +3510,13 @@ export interface IMcpAuthenticationDetails { authorizationServerMetadata: IAuthorizationServerMetadata; resourceMetadata: IAuthorizationProtectedResourceMetadata | undefined; scopes: string[] | undefined; + clientId?: string; } export interface IMcpAuthenticationOptions { errorOnUserInteraction?: boolean; forceNewRegistration?: boolean; + clientId?: string; } export const enum IAuthResourceMetadataSource { diff --git a/src/vs/workbench/api/common/extHostMcp.ts b/src/vs/workbench/api/common/extHostMcp.ts index f7436ea4590..fddb22fd157 100644 --- a/src/vs/workbench/api/common/extHostMcp.ts +++ b/src/vs/workbench/api/common/extHostMcp.ts @@ -705,7 +705,8 @@ export class McpHTTPHandle extends Disposable { authorizationServer: this._authMetadata.authorizationServer.toJSON(), authorizationServerMetadata: this._authMetadata.serverMetadata, resourceMetadata: this._authMetadata.resourceMetadata, - scopes: this._authMetadata.scopes + scopes: this._authMetadata.scopes, + clientId: this._launch.oauth?.clientId, }; const token = await this._proxy.$getTokenFromServerMetadata( this._id, @@ -734,7 +735,8 @@ export class McpHTTPHandle extends Disposable { this._launch.authentication.scopes, { errorOnUserInteraction, - forceNewRegistration: options?.forceNewRegistration + forceNewRegistration: options?.forceNewRegistration, + clientId: this._launch.oauth?.clientId, } ); if (token) { diff --git a/src/vs/workbench/contrib/mcp/common/discovery/installedMcpServersDiscovery.ts b/src/vs/workbench/contrib/mcp/common/discovery/installedMcpServersDiscovery.ts index c3d16718ad3..3208a2541f8 100644 --- a/src/vs/workbench/contrib/mcp/common/discovery/installedMcpServersDiscovery.ts +++ b/src/vs/workbench/contrib/mcp/common/discovery/installedMcpServersDiscovery.ts @@ -89,6 +89,7 @@ export class InstalledMcpServersDiscovery extends Disposable implements IMcpDisc type: McpServerTransportType.HTTP, uri: URI.parse(config.url), headers: Object.entries(config.headers || {}), + oauth: config.oauth, } : { type: McpServerTransportType.Stdio, command: config.command, diff --git a/src/vs/workbench/contrib/mcp/common/discovery/pluginMcpDiscovery.ts b/src/vs/workbench/contrib/mcp/common/discovery/pluginMcpDiscovery.ts index 3a3bfb38f88..522a1be3ec6 100644 --- a/src/vs/workbench/contrib/mcp/common/discovery/pluginMcpDiscovery.ts +++ b/src/vs/workbench/contrib/mcp/common/discovery/pluginMcpDiscovery.ts @@ -120,6 +120,7 @@ export class PluginMcpDiscovery extends Disposable implements IMcpDiscovery { type: McpServerTransportType.HTTP, uri: URI.parse(config.url), headers: Object.entries(config.headers ?? {}), + oauth: config.oauth, }; } catch { return undefined; diff --git a/src/vs/workbench/contrib/mcp/common/mcpConfiguration.ts b/src/vs/workbench/contrib/mcp/common/mcpConfiguration.ts index fce88654401..8b1559c8f84 100644 --- a/src/vs/workbench/contrib/mcp/common/mcpConfiguration.ts +++ b/src/vs/workbench/contrib/mcp/common/mcpConfiguration.ts @@ -265,6 +265,19 @@ export const mcpServerSchema: IJSONSchema = { description: localize('app.mcp.json.headers', "Additional headers sent to the server."), additionalProperties: { type: 'string' }, }, + oauth: { + type: 'object', + description: localize('app.mcp.json.oauth', "OAuth configuration for authenticating with the server."), + additionalProperties: false, + minProperties: 1, + properties: { + clientId: { + type: 'string', + minLength: 1, + description: localize('app.mcp.json.oauth.clientId', "The OAuth client ID to use when authenticating with the server.") + } + } + }, ...mcpDevModeProps(false), } }, diff --git a/src/vs/workbench/contrib/mcp/common/mcpTypes.ts b/src/vs/workbench/contrib/mcp/common/mcpTypes.ts index 4877aa5d706..35b4dacf823 100644 --- a/src/vs/workbench/contrib/mcp/common/mcpTypes.ts +++ b/src/vs/workbench/contrib/mcp/common/mcpTypes.ts @@ -534,6 +534,10 @@ export interface McpServerTransportHTTPAuthentication { readonly scopes: string[]; } +export interface McpServerTransportHTTPOAuth { + readonly clientId?: string; +} + /** * MCP server launched on the command line which communicated over SSE or Streamable HTTP. * https://spec.modelcontextprotocol.io/specification/2024-11-05/basic/transports/#http-with-sse @@ -543,6 +547,11 @@ export interface McpServerTransportHTTP { readonly type: McpServerTransportType.HTTP; readonly uri: URI; readonly headers: [string, string][]; + readonly oauth?: McpServerTransportHTTPOAuth; + /** + * @deprecated this was originally used for step-auth auth but a different approach was used instead + * so it's effectively dead code. + */ readonly authentication?: McpServerTransportHTTPAuthentication; } @@ -552,7 +561,7 @@ export type McpServerLaunch = export namespace McpServerLaunch { export type Serialized = - | { type: McpServerTransportType.HTTP; uri: UriComponents; headers: [string, string][]; authentication?: McpServerTransportHTTPAuthentication } + | { type: McpServerTransportType.HTTP; uri: UriComponents; headers: [string, string][]; oauth?: McpServerTransportHTTPOAuth; authentication?: McpServerTransportHTTPAuthentication } | { type: McpServerTransportType.Stdio; cwd: string | undefined; command: string; args: readonly string[]; env: Record; envFile: string | undefined; sandbox: IMcpSandboxConfiguration | undefined }; export function toSerialized(launch: McpServerLaunch): McpServerLaunch.Serialized { @@ -562,7 +571,7 @@ export namespace McpServerLaunch { export function fromSerialized(launch: McpServerLaunch.Serialized): McpServerLaunch { switch (launch.type) { case McpServerTransportType.HTTP: - return { type: launch.type, uri: URI.revive(launch.uri), headers: launch.headers, authentication: launch.authentication }; + return { type: launch.type, uri: URI.revive(launch.uri), headers: launch.headers, oauth: launch.oauth, authentication: launch.authentication }; case McpServerTransportType.Stdio: return { type: launch.type, diff --git a/src/vs/workbench/services/authentication/browser/authenticationService.ts b/src/vs/workbench/services/authentication/browser/authenticationService.ts index 2966703acd3..2dbd1b54778 100644 --- a/src/vs/workbench/services/authentication/browser/authenticationService.ts +++ b/src/vs/workbench/services/authentication/browser/authenticationService.ts @@ -364,13 +364,13 @@ export class AuthenticationService extends Disposable implements IAuthentication return undefined; } - async createDynamicAuthenticationProvider(authorizationServer: URI, serverMetadata: IAuthorizationServerMetadata, resource: IAuthorizationProtectedResourceMetadata | undefined): Promise { + async createDynamicAuthenticationProvider(authorizationServer: URI, serverMetadata: IAuthorizationServerMetadata, resource: IAuthorizationProtectedResourceMetadata | undefined, clientId?: string): Promise { const delegate = this._delegates[0]; if (!delegate) { this._logService.error('No authentication provider host delegate found'); return undefined; } - const providerId = await delegate.create(authorizationServer, serverMetadata, resource); + const providerId = await delegate.create(authorizationServer, serverMetadata, resource, clientId); const provider = this._authenticationProviders.get(providerId); if (provider) { this._logService.debug(`Created dynamic authentication provider: ${providerId}`); diff --git a/src/vs/workbench/services/authentication/common/authentication.ts b/src/vs/workbench/services/authentication/common/authentication.ts index 6d88e15d1b0..42ac842860f 100644 --- a/src/vs/workbench/services/authentication/common/authentication.ts +++ b/src/vs/workbench/services/authentication/common/authentication.ts @@ -142,7 +142,7 @@ export interface AllowedExtension { export interface IAuthenticationProviderHostDelegate { /** Priority for this delegate, delegates are tested in descending priority order */ readonly priority: number; - create(authorizationServer: URI, serverMetadata: IAuthorizationServerMetadata, resource: IAuthorizationProtectedResourceMetadata | undefined): Promise; + create(authorizationServer: URI, serverMetadata: IAuthorizationServerMetadata, resource: IAuthorizationProtectedResourceMetadata | undefined, clientId?: string): Promise; } export const IAuthenticationService = createDecorator('IAuthenticationService'); @@ -271,7 +271,7 @@ export interface IAuthenticationService { * Creates a dynamic authentication provider for the given server metadata * @param serverMetadata The metadata for the server that is being authenticated against */ - createDynamicAuthenticationProvider(authorizationServer: URI, serverMetadata: IAuthorizationServerMetadata, resourceMetadata: IAuthorizationProtectedResourceMetadata | undefined): Promise; + createDynamicAuthenticationProvider(authorizationServer: URI, serverMetadata: IAuthorizationServerMetadata, resourceMetadata: IAuthorizationProtectedResourceMetadata | undefined, clientId?: string): Promise; } export function isAuthenticationSession(thing: unknown): thing is AuthenticationSession { diff --git a/src/vscode-dts/vscode.proposed.authIssuers.d.ts b/src/vscode-dts/vscode.proposed.authIssuers.d.ts index 70b3094f7e2..3cb9ca9663d 100644 --- a/src/vscode-dts/vscode.proposed.authIssuers.d.ts +++ b/src/vscode-dts/vscode.proposed.authIssuers.d.ts @@ -18,6 +18,12 @@ declare module 'vscode' { * authenticate the user. This is only used when a provider has `supportedAuthorizationServers` set */ authorizationServer?: Uri; + + /** + * When specified, the authentication provider will use the provided client ID for the OAuth flow + * instead of its default client ID. + */ + clientId?: string; } export interface AuthenticationGetSessionOptions { @@ -26,5 +32,11 @@ declare module 'vscode' { * authenticate the user. This is only used when a provider has `supportedAuthorizationServers` set */ authorizationServer?: Uri; + + /** + * When specified, the authentication provider will use the provided client ID for the OAuth flow + * instead of its default client ID. + */ + clientId?: string; } }