Skip to content

Commit 011ffac

Browse files
Improve organization of auth state for MCP servers (#251864)
Now they will show in the their own section in the Account Menu AND have an option to Disconnect or Sign out in the MCP Server menu depending on if the account is used by other things.
1 parent 88dceb6 commit 011ffac

File tree

9 files changed

+281
-48
lines changed

9 files changed

+281
-48
lines changed

src/vs/workbench/api/browser/mainThreadAuthentication.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu
225225
}
226226

227227
async $registerDynamicAuthenticationProvider(id: string, label: string, authorizationServer: UriComponents, clientId: string): Promise<void> {
228-
await this.$registerAuthenticationProvider(id, label, false, [authorizationServer]);
228+
await this.$registerAuthenticationProvider(id, label, true, [authorizationServer]);
229229
this.dynamicAuthProviderStorageService.storeClientId(id, URI.revive(authorizationServer).toString(true), clientId, label);
230230
}
231231

src/vs/workbench/api/browser/mainThreadMcp.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,12 @@ export class MainThreadMcp extends Disposable implements MainThreadMcpShape {
165165
if (sessions.length) {
166166
// If we have an existing session preference, use that. If not, we'll return any valid session at the end of this function.
167167
if (matchingAccountPreferenceSession && this.authenticationMCPServerAccessService.isAccessAllowed(providerId, matchingAccountPreferenceSession.account.label, server.id)) {
168+
this._mcpRegistry.setAuthenticationUsage(server.id, providerId);
168169
return matchingAccountPreferenceSession.accessToken;
169170
}
170171
// If we only have one account for a single auth provider, lets just check if it's allowed and return it if it is.
171172
if (!provider.supportsMultipleAccounts && this.authenticationMCPServerAccessService.isAccessAllowed(providerId, sessions[0].account.label, server.id)) {
173+
this._mcpRegistry.setAuthenticationUsage(server.id, providerId);
172174
return sessions[0].accessToken;
173175
}
174176
}
@@ -201,6 +203,7 @@ export class MainThreadMcp extends Disposable implements MainThreadMcpShape {
201203
);
202204
}
203205

206+
this._mcpRegistry.setAuthenticationUsage(server.id, providerId);
204207
this.authenticationMCPServerAccessService.updateAllowedMcpServers(providerId, session.account.label, [{ id: server.id, name: server.label, allowed: true }]);
205208
this.authenticationMcpServersService.updateAccountPreference(server.id, providerId, session.account);
206209
this.authenticationMCPServerUsageService.addAccountUsage(providerId, session.account.label, scopesSupported, server.id, server.label);

src/vs/workbench/browser/parts/globalCompositeBar.ts

Lines changed: 95 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -358,66 +358,118 @@ export class AccountsActivityActionViewItem extends AbstractGlobalActivityAction
358358
protected override async resolveMainMenuActions(accountsMenu: IMenu, disposables: DisposableStore): Promise<IAction[]> {
359359
await super.resolveMainMenuActions(accountsMenu, disposables);
360360

361-
const providers = this.authenticationService.getProviderIds();
361+
const providers = this.authenticationService.getProviderIds().filter(p => !p.startsWith(INTERNAL_AUTH_PROVIDER_PREFIX));
362362
const otherCommands = accountsMenu.getActions();
363363
let menus: IAction[] = [];
364364

365-
for (const providerId of providers) {
366-
if (providerId.startsWith(INTERNAL_AUTH_PROVIDER_PREFIX)) {
367-
continue;
368-
}
369-
if (!this.initialized) {
370-
const noAccountsAvailableAction = disposables.add(new Action('noAccountsAvailable', localize('loading', "Loading..."), undefined, false));
371-
menus.push(noAccountsAvailableAction);
372-
break;
373-
}
374-
const provider = this.authenticationService.getProvider(providerId);
375-
const accounts = this.groupedAccounts.get(providerId);
376-
if (!accounts) {
377-
if (this.problematicProviders.has(providerId)) {
378-
const providerUnavailableAction = disposables.add(new Action('providerUnavailable', localize('authProviderUnavailable', '{0} is currently unavailable', provider.label), undefined, false));
379-
menus.push(providerUnavailableAction);
380-
// try again in the background so that if the failure was intermittent, we can resolve it on the next showing of the menu
381-
try {
382-
await this.addAccountsFromProvider(providerId);
383-
} catch (e) {
384-
this.logService.error(e);
365+
const registeredProviders = providers.filter(providerId => !this.authenticationService.isDynamicAuthenticationProvider(providerId));
366+
const dynamicProviders = providers.filter(providerId => this.authenticationService.isDynamicAuthenticationProvider(providerId));
367+
368+
if (!this.initialized) {
369+
const noAccountsAvailableAction = disposables.add(new Action('noAccountsAvailable', localize('loading', "Loading..."), undefined, false));
370+
menus.push(noAccountsAvailableAction);
371+
} else {
372+
for (const providerId of registeredProviders) {
373+
const provider = this.authenticationService.getProvider(providerId);
374+
const accounts = this.groupedAccounts.get(providerId);
375+
if (!accounts) {
376+
if (this.problematicProviders.has(providerId)) {
377+
const providerUnavailableAction = disposables.add(new Action('providerUnavailable', localize('authProviderUnavailable', '{0} is currently unavailable', provider.label), undefined, false));
378+
menus.push(providerUnavailableAction);
379+
// try again in the background so that if the failure was intermittent, we can resolve it on the next showing of the menu
380+
try {
381+
await this.addAccountsFromProvider(providerId);
382+
} catch (e) {
383+
this.logService.error(e);
384+
}
385385
}
386+
continue;
386387
}
387-
continue;
388+
389+
const canUseMcp = !!provider.authorizationServers?.length;
390+
for (const account of accounts) {
391+
const manageExtensionsAction = toAction({
392+
id: `configureSessions${account.label}`,
393+
label: localize('manageTrustedExtensions', "Manage Trusted Extensions"),
394+
enabled: true,
395+
run: () => this.commandService.executeCommand('_manageTrustedExtensionsForAccount', { providerId, accountLabel: account.label })
396+
});
397+
398+
399+
const providerSubMenuActions: IAction[] = [manageExtensionsAction];
400+
if (canUseMcp) {
401+
const manageMCPAction = toAction({
402+
id: `configureSessions${account.label}`,
403+
label: localize('manageTrustedMCPServers', "Manage Trusted MCP Servers"),
404+
enabled: true,
405+
run: () => this.commandService.executeCommand('_manageTrustedMCPServersForAccount', { providerId, accountLabel: account.label })
406+
});
407+
providerSubMenuActions.push(manageMCPAction);
408+
}
409+
if (account.canSignOut) {
410+
providerSubMenuActions.push(toAction({
411+
id: 'signOut',
412+
label: localize('signOut', "Sign Out"),
413+
enabled: true,
414+
run: () => this.commandService.executeCommand('_signOutOfAccount', { providerId, accountLabel: account.label })
415+
}));
416+
}
417+
418+
const providerSubMenu = new SubmenuAction('activitybar.submenu', `${account.label} (${provider.label})`, providerSubMenuActions);
419+
menus.push(providerSubMenu);
420+
}
421+
}
422+
423+
if (dynamicProviders.length && registeredProviders.length) {
424+
menus.push(new Separator());
388425
}
389426

390-
const canUseMcp = !!provider.authorizationServers?.length;
391-
for (const account of accounts) {
392-
const manageExtensionsAction = toAction({
393-
id: `configureSessions${account.label}`,
394-
label: localize('manageTrustedExtensions', "Manage Trusted Extensions"),
395-
enabled: true,
396-
run: () => this.commandService.executeCommand('_manageTrustedExtensionsForAccount', { providerId, accountLabel: account.label })
397-
});
427+
for (const providerId of dynamicProviders) {
428+
const provider = this.authenticationService.getProvider(providerId);
429+
const accounts = this.groupedAccounts.get(providerId);
430+
if (!accounts) {
431+
if (this.problematicProviders.has(providerId)) {
432+
const providerUnavailableAction = disposables.add(new Action('providerUnavailable', localize('authProviderUnavailable', '{0} is currently unavailable', provider.label), undefined, false));
433+
menus.push(providerUnavailableAction);
434+
// try again in the background so that if the failure was intermittent, we can resolve it on the next showing of the menu
435+
try {
436+
await this.addAccountsFromProvider(providerId);
437+
} catch (e) {
438+
this.logService.error(e);
439+
}
440+
}
441+
continue;
442+
}
398443

444+
for (const account of accounts) {
445+
// TODO@TylerLeonhardt: Is there a nice way to bring this back?
446+
// const manageExtensionsAction = toAction({
447+
// id: `configureSessions${account.label}`,
448+
// label: localize('manageTrustedExtensions', "Manage Trusted Extensions"),
449+
// enabled: true,
450+
// run: () => this.commandService.executeCommand('_manageTrustedExtensionsForAccount', { providerId, accountLabel: account.label })
451+
// });
399452

400-
const providerSubMenuActions: IAction[] = [manageExtensionsAction];
401-
if (canUseMcp) {
453+
const providerSubMenuActions: IAction[] = [];
402454
const manageMCPAction = toAction({
403455
id: `configureSessions${account.label}`,
404456
label: localize('manageTrustedMCPServers', "Manage Trusted MCP Servers"),
405457
enabled: true,
406458
run: () => this.commandService.executeCommand('_manageTrustedMCPServersForAccount', { providerId, accountLabel: account.label })
407459
});
408460
providerSubMenuActions.push(manageMCPAction);
409-
}
410-
if (account.canSignOut) {
411-
providerSubMenuActions.push(toAction({
412-
id: 'signOut',
413-
label: localize('signOut', "Sign Out"),
414-
enabled: true,
415-
run: () => this.commandService.executeCommand('_signOutOfAccount', { providerId, accountLabel: account.label })
416-
}));
417-
}
461+
if (account.canSignOut) {
462+
providerSubMenuActions.push(toAction({
463+
id: 'signOut',
464+
label: localize('signOut', "Sign Out"),
465+
enabled: true,
466+
run: () => this.commandService.executeCommand('_signOutOfAccount', { providerId, accountLabel: account.label })
467+
}));
468+
}
418469

419-
const providerSubMenu = new SubmenuAction('activitybar.submenu', `${account.label} (${provider.label})`, providerSubMenuActions);
420-
menus.push(providerSubMenu);
470+
const providerSubMenu = new SubmenuAction('activitybar.submenu', `${account.label} (${provider.label})`, providerSubMenuActions);
471+
menus.push(providerSubMenu);
472+
}
421473
}
422474
}
423475

src/vs/workbench/contrib/mcp/browser/mcpCommands.ts

Lines changed: 146 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,17 @@ import { ContextKeyExpr } from '../../../../platform/contextkey/common/contextke
2525
import { ExtensionsLocalizedLabel } from '../../../../platform/extensionManagement/common/extensionManagement.js';
2626
import { IInstantiationService, ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js';
2727
import { IMcpGalleryService } from '../../../../platform/mcp/common/mcpManagement.js';
28+
import { IProductService } from '../../../../platform/product/common/productService.js';
2829
import { IQuickInputService, IQuickPickItem, IQuickPickSeparator } from '../../../../platform/quickinput/common/quickInput.js';
2930
import { StorageScope } from '../../../../platform/storage/common/storage.js';
3031
import { spinningLoading } from '../../../../platform/theme/common/iconRegistry.js';
3132
import { IWorkspaceContextService } from '../../../../platform/workspace/common/workspace.js';
3233
import { ActiveEditorContext, ResourceContextKey } from '../../../common/contextkeys.js';
3334
import { IWorkbenchContribution } from '../../../common/contributions.js';
35+
import { IAuthenticationAccessService } from '../../../services/authentication/browser/authenticationAccessService.js';
36+
import { IAuthenticationMcpAccessService } from '../../../services/authentication/browser/authenticationMcpAccessService.js';
37+
import { IAuthenticationMcpService } from '../../../services/authentication/browser/authenticationMcpService.js';
38+
import { IAuthenticationService } from '../../../services/authentication/common/authentication.js';
3439
import { IEditorService } from '../../../services/editor/common/editorService.js';
3540
import { IViewsService } from '../../../services/views/common/viewsService.js';
3641
import { IChatWidgetService } from '../../chat/browser/chat.js';
@@ -142,6 +147,9 @@ export class ListMcpServerCommand extends Action2 {
142147
}
143148
}
144149

150+
interface ActionItem extends IQuickPickItem {
151+
action: 'start' | 'stop' | 'restart' | 'disconnect' | 'signout' | 'showOutput' | 'config' | 'configSampling' | 'samplingLog' | 'resources';
152+
}
145153

146154
export class McpServerOptionsCommand extends Action2 {
147155
constructor() {
@@ -160,6 +168,11 @@ export class McpServerOptionsCommand extends Action2 {
160168
const editorService = accessor.get(IEditorService);
161169
const commandService = accessor.get(ICommandService);
162170
const samplingService = accessor.get(IMcpSamplingService);
171+
const authenticationMcpService = accessor.get(IAuthenticationMcpService);
172+
const authenticationMcpAccessService = accessor.get(IAuthenticationMcpAccessService);
173+
const authenticationExtensionAccessService = accessor.get(IAuthenticationAccessService);
174+
const authenticationService = accessor.get(IAuthenticationService);
175+
const productService = accessor.get(IProductService);
163176
const server = mcpService.servers.get().find(s => s.definition.id === id);
164177
if (!server) {
165178
return;
@@ -168,10 +181,6 @@ export class McpServerOptionsCommand extends Action2 {
168181
const collection = mcpRegistry.collections.get().find(c => c.id === server.collection.id);
169182
const serverDefinition = collection?.serverDefinitions.get().find(s => s.id === server.definition.id);
170183

171-
interface ActionItem extends IQuickPickItem {
172-
action: 'start' | 'stop' | 'restart' | 'showOutput' | 'config' | 'configSampling' | 'samplingLog' | 'resources';
173-
}
174-
175184
const items: (ActionItem | IQuickPickSeparator)[] = [];
176185
const serverState = server.connectionState.get();
177186

@@ -194,6 +203,18 @@ export class McpServerOptionsCommand extends Action2 {
194203
});
195204
}
196205

206+
const item = this._getAuthAction(
207+
mcpRegistry,
208+
authenticationMcpService,
209+
authenticationMcpAccessService,
210+
authenticationExtensionAccessService,
211+
productService,
212+
server.definition.id
213+
);
214+
if (item) {
215+
items.push(item);
216+
}
217+
197218
const configTarget = serverDefinition?.presentation?.origin || collection?.presentation?.origin;
198219
if (configTarget) {
199220
items.push({
@@ -254,6 +275,26 @@ export class McpServerOptionsCommand extends Action2 {
254275
await server.stop();
255276
await server.start({ isFromInteraction: true });
256277
break;
278+
case 'disconnect':
279+
await this._handleAuth(
280+
mcpRegistry,
281+
authenticationMcpService,
282+
authenticationMcpAccessService,
283+
authenticationService,
284+
server,
285+
false
286+
);
287+
break;
288+
case 'signout':
289+
await this._handleAuth(
290+
mcpRegistry,
291+
authenticationMcpService,
292+
authenticationMcpAccessService,
293+
authenticationService,
294+
server,
295+
true
296+
);
297+
break;
257298
case 'showOutput':
258299
server.showOutput();
259300
break;
@@ -278,6 +319,107 @@ export class McpServerOptionsCommand extends Action2 {
278319
assertNever(pick.action);
279320
}
280321
}
322+
323+
private _getAuthAction(
324+
mcpRegistry: IMcpRegistry,
325+
authenticationMcpService: IAuthenticationMcpService,
326+
authenticationMcpAccessService: IAuthenticationMcpAccessService,
327+
authenticationAccessService: IAuthenticationAccessService,
328+
productService: IProductService,
329+
serverId: string
330+
): ActionItem | undefined {
331+
const providerId = mcpRegistry.getAuthenticationUsage(serverId);
332+
if (!providerId) {
333+
return undefined;
334+
}
335+
const preference = authenticationMcpService.getAccountPreference(serverId, providerId);
336+
if (!preference) {
337+
return undefined;
338+
}
339+
if (!authenticationMcpAccessService.isAccessAllowed(providerId, preference, serverId)) {
340+
return undefined;
341+
}
342+
const allowedServers = this._getAllAllowedItems(
343+
authenticationMcpAccessService,
344+
authenticationAccessService,
345+
productService,
346+
providerId,
347+
preference
348+
);
349+
350+
// If there are multiple allowed servers/extensions, other things are using this provider
351+
// so we show a disconnect action, otherwise we show a sign out action.
352+
if (allowedServers.length > 1) {
353+
return {
354+
action: 'disconnect',
355+
label: localize('mcp.disconnect', 'Disconnect Account'),
356+
description: `(${preference})`,
357+
};
358+
}
359+
return {
360+
action: 'signout',
361+
label: localize('mcp.signOut', 'Sign Out'),
362+
description: `(${preference})`
363+
};
364+
}
365+
366+
// TODO@TylerLeonhardt: The fact that this function exists means that these classes could really use some refactoring...
367+
private _getAllAllowedItems(
368+
authenticationMcpAccessService: IAuthenticationMcpAccessService,
369+
authenticationAccessService: IAuthenticationAccessService,
370+
productService: IProductService,
371+
providerId: string,
372+
preference: string
373+
) {
374+
const trustedExtensionAuth = Array.isArray(productService.trustedExtensionAuthAccess) || !productService.trustedExtensionAuthAccess
375+
? []
376+
: productService.trustedExtensionAuthAccess[providerId] ?? [];
377+
const trustedMcpAuth = Array.isArray(productService.trustedMcpAuthAccess) || !productService.trustedMcpAuthAccess
378+
? []
379+
: productService.trustedMcpAuthAccess[providerId] ?? [];
380+
381+
return [
382+
...authenticationMcpAccessService.readAllowedMcpServers(providerId, preference).filter(s => !s.trusted),
383+
...authenticationAccessService.readAllowedExtensions(providerId, preference).filter(e => !e.trusted),
384+
...trustedExtensionAuth,
385+
...trustedMcpAuth
386+
];
387+
}
388+
389+
private async _handleAuth(
390+
mcpRegistry: IMcpRegistry,
391+
authenticationMcpService: IAuthenticationMcpService,
392+
authenticationMcpAccessService: IAuthenticationMcpAccessService,
393+
authenticationService: IAuthenticationService,
394+
server: IMcpServer,
395+
signOut: boolean
396+
) {
397+
const providerId = mcpRegistry.getAuthenticationUsage(server.definition.id);
398+
if (!providerId) {
399+
return;
400+
}
401+
const preference = authenticationMcpService.getAccountPreference(server.definition.id, providerId);
402+
if (!preference) {
403+
return;
404+
}
405+
authenticationMcpAccessService.updateAllowedMcpServers(providerId, preference, [
406+
{
407+
id: server.definition.id,
408+
name: server.definition.label,
409+
allowed: false
410+
}
411+
]);
412+
if (signOut) {
413+
const accounts = await authenticationService.getAccounts(providerId);
414+
const account = accounts.find(a => a.label === preference);
415+
if (account) {
416+
const sessions = await authenticationService.getSessions(providerId, undefined, { account });
417+
for (const session of sessions) {
418+
await authenticationService.removeSession(providerId, session.id);
419+
}
420+
}
421+
}
422+
}
281423
}
282424

283425
export class MCPServerActionRendering extends Disposable implements IWorkbenchContribution {

0 commit comments

Comments
 (0)