mirror of
https://github.com/microsoft/vscode.git
synced 2026-04-10 17:43:38 -05:00
Add oauth section to mcp.json to allow overriding of client id (#308648)
* Add `oauth` section to mcp.json to allow overriding of client id More properties (like client secret) can come later... but this is the foundation, which is a lot of plumbing... ref https://github.com/microsoft/vscode/issues/257415 * feedback
This commit is contained in:
committed by
GitHub
parent
99d79c60bf
commit
d632fa37d2
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -170,7 +170,7 @@ export class MsalAuthProvider implements AuthenticationProvider {
|
||||
|
||||
async getSessions(scopes: string[] | undefined, options: AuthenticationGetSessionOptions = {}): Promise<AuthenticationSession[]> {
|
||||
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<AuthenticationSession> {
|
||||
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);
|
||||
|
||||
@@ -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<string, string>;
|
||||
readonly oauth?: IMcpRemoteServerOAuthConfiguration;
|
||||
readonly dev?: IMcpDevModeConfig;
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<string | undefined> {
|
||||
async $getTokenFromServerMetadata(id: number, authDetails: IMcpAuthenticationDetails, { errorOnUserInteraction, forceNewRegistration, clientId }: IMcpAuthenticationOptions = {}): Promise<string | undefined> {
|
||||
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<string | undefined> {
|
||||
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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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),
|
||||
}
|
||||
},
|
||||
|
||||
@@ -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<string, string | number | null>; 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,
|
||||
|
||||
@@ -364,13 +364,13 @@ export class AuthenticationService extends Disposable implements IAuthentication
|
||||
return undefined;
|
||||
}
|
||||
|
||||
async createDynamicAuthenticationProvider(authorizationServer: URI, serverMetadata: IAuthorizationServerMetadata, resource: IAuthorizationProtectedResourceMetadata | undefined): Promise<IAuthenticationProvider | undefined> {
|
||||
async createDynamicAuthenticationProvider(authorizationServer: URI, serverMetadata: IAuthorizationServerMetadata, resource: IAuthorizationProtectedResourceMetadata | undefined, clientId?: string): Promise<IAuthenticationProvider | undefined> {
|
||||
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}`);
|
||||
|
||||
@@ -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<string>;
|
||||
create(authorizationServer: URI, serverMetadata: IAuthorizationServerMetadata, resource: IAuthorizationProtectedResourceMetadata | undefined, clientId?: string): Promise<string>;
|
||||
}
|
||||
|
||||
export const IAuthenticationService = createDecorator<IAuthenticationService>('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<IAuthenticationProvider | undefined>;
|
||||
createDynamicAuthenticationProvider(authorizationServer: URI, serverMetadata: IAuthorizationServerMetadata, resourceMetadata: IAuthorizationProtectedResourceMetadata | undefined, clientId?: string): Promise<IAuthenticationProvider | undefined>;
|
||||
}
|
||||
|
||||
export function isAuthenticationSession(thing: unknown): thing is AuthenticationSession {
|
||||
|
||||
12
src/vscode-dts/vscode.proposed.authIssuers.d.ts
vendored
12
src/vscode-dts/vscode.proposed.authIssuers.d.ts
vendored
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user