Support concurrent usage of multiple GitHub accounts for child extensions (#317821)

src/vs/workbench/services/authentication/browser/authenticationExtensionsService.ts

@@ -162,31 +162,48 @@ export class AuthenticationExtensionsService extends Disposable implements IAuth
 
 	updateAccountPreference(extensionId: string, providerId: string, account: AuthenticationSessionAccount): void {
 		const realExtensionId = ExtensionIdentifier.toKey(extensionId);
-		const parentExtensionId = this._inheritAuthAccountPreferenceChildToParent[realExtensionId] ?? realExtensionId;
-		const key = this._getKey(parentExtensionId, providerId);
+		const key = this._getKey(realExtensionId, providerId);
 
 		// Store the preference in the workspace and application storage. This allows new workspaces to
 		// have a preference set already to limit the number of prompts that are shown... but also allows
 		// a specific workspace to override the global preference.
 		this.storageService.store(key, account.label, StorageScope.WORKSPACE, StorageTarget.MACHINE);
 		this.storageService.store(key, account.label, StorageScope.APPLICATION, StorageTarget.MACHINE);
 
-		const childrenExtensions = this._inheritAuthAccountPreferenceParentToChildren[parentExtensionId];
-		const extensionIds = childrenExtensions ? [parentExtensionId, ...childrenExtensions] : [parentExtensionId];
+		const isChild = !!this._inheritAuthAccountPreferenceChildToParent[realExtensionId];
+		let extensionIds: string[];
+		if (isChild) {
+			extensionIds = [realExtensionId];
+		} else {
+			const childrenExtensions = this._inheritAuthAccountPreferenceParentToChildren[realExtensionId];
+			extensionIds = childrenExtensions ? [realExtensionId, ...childrenExtensions] : [realExtensionId];
+		}
 		this._onDidAccountPreferenceChange.fire({ extensionIds, providerId });
 	}
 
 	getAccountPreference(extensionId: string, providerId: string): string | undefined {
 		const realExtensionId = ExtensionIdentifier.toKey(extensionId);
-		const key = this._getKey(this._inheritAuthAccountPreferenceChildToParent[realExtensionId] ?? realExtensionId, providerId);
+		const specificKey = this._getKey(realExtensionId, providerId);
 
-		// If a preference is set in the workspace, use that. Otherwise, use the global preference.
-		return this.storageService.get(key, StorageScope.WORKSPACE) ?? this.storageService.get(key, StorageScope.APPLICATION);
+		// If a preference is set for the specific extension, use that.
+		const specificPreference = this.storageService.get(specificKey, StorageScope.WORKSPACE) ?? this.storageService.get(specificKey, StorageScope.APPLICATION);
+		if (specificPreference !== undefined) {
+			return specificPreference;
+		}
+
+		// Otherwise, fall back to the parent extension's preference if it exists.
+		const parentExtensionId = this._inheritAuthAccountPreferenceChildToParent[realExtensionId];
+		if (parentExtensionId) {
+			const parentKey = this._getKey(parentExtensionId, providerId);
+			return this.storageService.get(parentKey, StorageScope.WORKSPACE) ?? this.storageService.get(parentKey, StorageScope.APPLICATION);
+		}
+
+		return undefined;
 	}
 
 	removeAccountPreference(extensionId: string, providerId: string): void {
 		const realExtensionId = ExtensionIdentifier.toKey(extensionId);
-		const key = this._getKey(this._inheritAuthAccountPreferenceChildToParent[realExtensionId] ?? realExtensionId, providerId);
+		const key = this._getKey(realExtensionId, providerId);
 
 		// This won't affect any other workspaces that have a preference set, but it will remove the preference
 		// for this workspace and the global preference. This is only paired with a call to updateSessionPreference...

src/vs/workbench/services/authentication/browser/authenticationMcpService.ts

@@ -219,29 +219,46 @@ export class AuthenticationMcpService extends Disposable implements IAuthenticat
 	//#region Account/Session Preference
 
 	updateAccountPreference(mcpServerId: string, providerId: string, account: AuthenticationSessionAccount): void {
-		const parentMcpServerId = this._inheritAuthAccountPreferenceChildToParent[mcpServerId] ?? mcpServerId;
-		const key = this._getKey(parentMcpServerId, providerId);
+		const key = this._getKey(mcpServerId, providerId);
 
 		// Store the preference in the workspace and application storage. This allows new workspaces to
 		// have a preference set already to limit the number of prompts that are shown... but also allows
 		// a specific workspace to override the global preference.
 		this.storageService.store(key, account.label, StorageScope.WORKSPACE, StorageTarget.MACHINE);
 		this.storageService.store(key, account.label, StorageScope.APPLICATION, StorageTarget.MACHINE);
 
-		const childrenMcpServers = this._inheritAuthAccountPreferenceParentToChildren[parentMcpServerId];
-		const mcpServerIds = childrenMcpServers ? [parentMcpServerId, ...childrenMcpServers] : [parentMcpServerId];
+		const isChild = !!this._inheritAuthAccountPreferenceChildToParent[mcpServerId];
+		let mcpServerIds: string[];
+		if (isChild) {
+			mcpServerIds = [mcpServerId];
+		} else {
+			const childrenMcpServers = this._inheritAuthAccountPreferenceParentToChildren[mcpServerId];
+			mcpServerIds = childrenMcpServers ? [mcpServerId, ...childrenMcpServers] : [mcpServerId];
+		}
 		this._onDidAccountPreferenceChange.fire({ mcpServerIds, providerId });
 	}
 
 	getAccountPreference(mcpServerId: string, providerId: string): string | undefined {
-		const key = this._getKey(this._inheritAuthAccountPreferenceChildToParent[mcpServerId] ?? mcpServerId, providerId);
+		const specificKey = this._getKey(mcpServerId, providerId);
 
-		// If a preference is set in the workspace, use that. Otherwise, use the global preference.
-		return this.storageService.get(key, StorageScope.WORKSPACE) ?? this.storageService.get(key, StorageScope.APPLICATION);
+		// If a preference is set for the specific MCP server, use that.
+		const specificPreference = this.storageService.get(specificKey, StorageScope.WORKSPACE) ?? this.storageService.get(specificKey, StorageScope.APPLICATION);
+		if (specificPreference !== undefined) {
+			return specificPreference;
+		}
+
+		// Otherwise, fall back to the parent MCP server's preference if it exists.
+		const parentMcpServerId = this._inheritAuthAccountPreferenceChildToParent[mcpServerId];
+		if (parentMcpServerId) {
+			const parentKey = this._getKey(parentMcpServerId, providerId);
+			return this.storageService.get(parentKey, StorageScope.WORKSPACE) ?? this.storageService.get(parentKey, StorageScope.APPLICATION);
+		}
+
+		return undefined;
 	}
 
 	removeAccountPreference(mcpServerId: string, providerId: string): void {
-		const key = this._getKey(this._inheritAuthAccountPreferenceChildToParent[mcpServerId] ?? mcpServerId, providerId);
+		const key = this._getKey(mcpServerId, providerId);
 
 		// This won't affect any other workspaces that have a preference set, but it will remove the preference
 		// for this workspace and the global preference. This is only paired with a call to updateSessionPreference...

src/vs/workbench/services/authentication/test/browser/authenticationExtensionsService.test.ts

@@ -0,0 +1,116 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import assert from 'assert';
+import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
+import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js';
+import { IStorageService } from '../../../../../platform/storage/common/storage.js';
+import { TestStorageService, TestActivityService } from '../../../../test/common/workbenchTestServices.js';
+import { IProductService } from '../../../../../platform/product/common/productService.js';
+import { IActivityService } from '../../../activity/common/activity.js';
+import { IDialogService } from '../../../../../platform/dialogs/common/dialogs.js';
+import { TestDialogService } from '../../../../../platform/dialogs/test/common/testDialogService.js';
+import { IQuickInputService } from '../../../../../platform/quickinput/common/quickInput.js';
+import { TestQuickInputService } from '../../../../test/browser/workbenchTestServices.js';
+import { IAuthenticationService } from '../../common/authentication.js';
+import { TestAuthenticationService, TestAccessService } from './authenticationQueryServiceMocks.js';
+import { IAuthenticationUsageService } from '../../browser/authenticationUsageService.js';
+import { IAuthenticationAccessService } from '../../browser/authenticationAccessService.js';
+import { AuthenticationExtensionsService } from '../../browser/authenticationExtensionsService.js';
+
+class TestAuthUsageService implements IAuthenticationUsageService {
+	readonly _serviceBrand: undefined;
+	async initializeExtensionUsageCache(): Promise<void> { }
+	async extensionUsesAuth(extensionId: string): Promise<boolean> { return false; }
+	readAccountUsages(providerId: string, accountName: string): any[] { return []; }
+	removeAccountUsage(providerId: string, accountName: string): void { }
+	addAccountUsage(providerId: string, accountName: string, scopes: ReadonlyArray<string>, extensionId: string, extensionName: string): void { }
+}
+
+suite('AuthenticationExtensionsService - Hierarchical Preferences', () => {
+	const disposables = ensureNoDisposablesAreLeakedInTestSuite();
+
+	let extensionsService: AuthenticationExtensionsService;
+	let storageService: TestStorageService;
+
+	setup(() => {
+		const instantiationService = disposables.add(new TestInstantiationService());
+
+		storageService = disposables.add(new TestStorageService());
+		instantiationService.stub(IStorageService, storageService);
+
+		instantiationService.stub(IActivityService, disposables.add(new TestActivityService()));
+		instantiationService.stub(IDialogService, new TestDialogService());
+		instantiationService.stub(IQuickInputService, new TestQuickInputService());
+
+		const mockProductService: Partial<IProductService> = {
+			inheritAuthAccountPreference: {
+				'parent-ext': ['child-ext-1', 'child-ext-2']
+			}
+		};
+		instantiationService.stub(IProductService, mockProductService);
+
+		instantiationService.stub(IAuthenticationService, disposables.add(new TestAuthenticationService()));
+		instantiationService.stub(IAuthenticationUsageService, new TestAuthUsageService());
+		instantiationService.stub(IAuthenticationAccessService, disposables.add(new TestAccessService()));
+
+		extensionsService = disposables.add(instantiationService.createInstance(AuthenticationExtensionsService));
+	});
+
+	test('default inheritance: child inherits parent preference', () => {
+		// Set preference on parent
+		extensionsService.updateAccountPreference('parent-ext', 'github', { id: 'user-a', label: 'account-a' });
+
+		// Child should inherit parent's preference
+		assert.strictEqual(extensionsService.getAccountPreference('child-ext-1', 'github'), 'account-a');
+		assert.strictEqual(extensionsService.getAccountPreference('child-ext-2', 'github'), 'account-a');
+		assert.strictEqual(extensionsService.getAccountPreference('parent-ext', 'github'), 'account-a');
+	});
+
+	test('explicit override: child can use different preference than parent', () => {
+		// Set preference on parent (account-a)
+		extensionsService.updateAccountPreference('parent-ext', 'github', { id: 'user-a', label: 'account-a' });
+
+		// Set explicit preference on child-ext-1 (account-b)
+		extensionsService.updateAccountPreference('child-ext-1', 'github', { id: 'user-b', label: 'account-b' });
+
+		// child-ext-1 should use its own override
+		assert.strictEqual(extensionsService.getAccountPreference('child-ext-1', 'github'), 'account-b');
+
+		// parent-ext and child-ext-2 should still use parent's preference (account-a)
+		assert.strictEqual(extensionsService.getAccountPreference('parent-ext', 'github'), 'account-a');
+		assert.strictEqual(extensionsService.getAccountPreference('child-ext-2', 'github'), 'account-a');
+	});
+
+	test('explicit override: removing child preference falls back to parent preference', () => {
+		// Set preference on parent (account-a) and child (account-b)
+		extensionsService.updateAccountPreference('parent-ext', 'github', { id: 'user-a', label: 'account-a' });
+		extensionsService.updateAccountPreference('child-ext-1', 'github', { id: 'user-b', label: 'account-b' });
+
+		assert.strictEqual(extensionsService.getAccountPreference('child-ext-1', 'github'), 'account-b');
+
+		// Remove child preference
+		extensionsService.removeAccountPreference('child-ext-1', 'github');
+
+		// child-ext-1 should fall back to parent preference
+		assert.strictEqual(extensionsService.getAccountPreference('child-ext-1', 'github'), 'account-a');
+	});
+
+	test('removing parent preference does not remove child preference override', () => {
+		// Set preference on parent (account-a) and child (account-b)
+		extensionsService.updateAccountPreference('parent-ext', 'github', { id: 'user-a', label: 'account-a' });
+		extensionsService.updateAccountPreference('child-ext-1', 'github', { id: 'user-b', label: 'account-b' });
+
+		// Remove parent preference
+		extensionsService.removeAccountPreference('parent-ext', 'github');
+
+		// child-ext-1 should still have its preference override (account-b)
+		assert.strictEqual(extensionsService.getAccountPreference('child-ext-1', 'github'), 'account-b');
+
+		// parent-ext and child-ext-2 should have no preference
+		assert.strictEqual(extensionsService.getAccountPreference('parent-ext', 'github'), undefined);
+		assert.strictEqual(extensionsService.getAccountPreference('child-ext-2', 'github'), undefined);
+	});
+});

src/vs/workbench/services/authentication/test/browser/authenticationMcpService.test.ts

@@ -0,0 +1,116 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import assert from 'assert';
+import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
+import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js';
+import { IStorageService } from '../../../../../platform/storage/common/storage.js';
+import { TestStorageService, TestActivityService } from '../../../../test/common/workbenchTestServices.js';
+import { IProductService } from '../../../../../platform/product/common/productService.js';
+import { IActivityService } from '../../../activity/common/activity.js';
+import { IDialogService } from '../../../../../platform/dialogs/common/dialogs.js';
+import { TestDialogService } from '../../../../../platform/dialogs/test/common/testDialogService.js';
+import { IQuickInputService } from '../../../../../platform/quickinput/common/quickInput.js';
+import { TestQuickInputService } from '../../../../test/browser/workbenchTestServices.js';
+import { IAuthenticationService } from '../../common/authentication.js';
+import { TestAuthenticationService, TestMcpAccessService } from './authenticationQueryServiceMocks.js';
+import { IAuthenticationMcpUsageService } from '../../browser/authenticationMcpUsageService.js';
+import { IAuthenticationMcpAccessService } from '../../browser/authenticationMcpAccessService.js';
+import { AuthenticationMcpService } from '../../browser/authenticationMcpService.js';
+
+class TestMcpUsageService implements IAuthenticationMcpUsageService {
+	readonly _serviceBrand: undefined;
+	async initializeUsageCache(): Promise<void> { }
+	async hasUsedAuth(mcpServerId: string): Promise<boolean> { return false; }
+	readAccountUsages(providerId: string, accountName: string): any[] { return []; }
+	removeAccountUsage(providerId: string, accountName: string): void { }
+	addAccountUsage(providerId: string, accountName: string, scopes: ReadonlyArray<string>, mcpServerId: string, mcpServerName: string): void { }
+}
+
+suite('AuthenticationMcpService - Hierarchical Preferences', () => {
+	const disposables = ensureNoDisposablesAreLeakedInTestSuite();
+
+	let mcpService: AuthenticationMcpService;
+	let storageService: TestStorageService;
+
+	setup(() => {
+		const instantiationService = disposables.add(new TestInstantiationService());
+
+		storageService = disposables.add(new TestStorageService());
+		instantiationService.stub(IStorageService, storageService);
+
+		instantiationService.stub(IActivityService, disposables.add(new TestActivityService()));
+		instantiationService.stub(IDialogService, new TestDialogService());
+		instantiationService.stub(IQuickInputService, new TestQuickInputService());
+
+		const mockProductService: Partial<IProductService> = {
+			inheritAuthAccountPreference: {
+				'parent-mcp': ['child-mcp-1', 'child-mcp-2']
+			}
+		};
+		instantiationService.stub(IProductService, mockProductService);
+
+		instantiationService.stub(IAuthenticationService, disposables.add(new TestAuthenticationService()));
+		instantiationService.stub(IAuthenticationMcpUsageService, new TestMcpUsageService());
+		instantiationService.stub(IAuthenticationMcpAccessService, disposables.add(new TestMcpAccessService()));
+
+		mcpService = disposables.add(instantiationService.createInstance(AuthenticationMcpService));
+	});
+
+	test('default inheritance: child inherits parent preference', () => {
+		// Set preference on parent
+		mcpService.updateAccountPreference('parent-mcp', 'github', { id: 'user-a', label: 'account-a' });
+
+		// Child should inherit parent's preference
+		assert.strictEqual(mcpService.getAccountPreference('child-mcp-1', 'github'), 'account-a');
+		assert.strictEqual(mcpService.getAccountPreference('child-mcp-2', 'github'), 'account-a');
+		assert.strictEqual(mcpService.getAccountPreference('parent-mcp', 'github'), 'account-a');
+	});
+
+	test('explicit override: child can use different preference than parent', () => {
+		// Set preference on parent (account-a)
+		mcpService.updateAccountPreference('parent-mcp', 'github', { id: 'user-a', label: 'account-a' });
+
+		// Set explicit preference on child-mcp-1 (account-b)
+		mcpService.updateAccountPreference('child-mcp-1', 'github', { id: 'user-b', label: 'account-b' });
+
+		// child-mcp-1 should use its own override
+		assert.strictEqual(mcpService.getAccountPreference('child-mcp-1', 'github'), 'account-b');
+
+		// parent-mcp and child-mcp-2 should still use parent's preference (account-a)
+		assert.strictEqual(mcpService.getAccountPreference('parent-mcp', 'github'), 'account-a');
+		assert.strictEqual(mcpService.getAccountPreference('child-mcp-2', 'github'), 'account-a');
+	});
+
+	test('explicit override: removing child preference falls back to parent preference', () => {
+		// Set preference on parent (account-a) and child (account-b)
+		mcpService.updateAccountPreference('parent-mcp', 'github', { id: 'user-a', label: 'account-a' });
+		mcpService.updateAccountPreference('child-mcp-1', 'github', { id: 'user-b', label: 'account-b' });
+
+		assert.strictEqual(mcpService.getAccountPreference('child-mcp-1', 'github'), 'account-b');
+
+		// Remove child preference
+		mcpService.removeAccountPreference('child-mcp-1', 'github');
+
+		// child-mcp-1 should fall back to parent preference
+		assert.strictEqual(mcpService.getAccountPreference('child-mcp-1', 'github'), 'account-a');
+	});
+
+	test('removing parent preference does not remove child preference override', () => {
+		// Set preference on parent (account-a) and child (account-b)
+		mcpService.updateAccountPreference('parent-mcp', 'github', { id: 'user-a', label: 'account-a' });
+		mcpService.updateAccountPreference('child-mcp-1', 'github', { id: 'user-b', label: 'account-b' });
+
+		// Remove parent preference
+		mcpService.removeAccountPreference('parent-mcp', 'github');
+
+		// child-mcp-1 should still have its preference override (account-b)
+		assert.strictEqual(mcpService.getAccountPreference('child-mcp-1', 'github'), 'account-b');
+
+		// parent-mcp and child-mcp-2 should have no preference
+		assert.strictEqual(mcpService.getAccountPreference('parent-mcp', 'github'), undefined);
+		assert.strictEqual(mcpService.getAccountPreference('child-mcp-2', 'github'), undefined);
+	});
+});