Improve async nature of register/unregister auth providers (#251244)

* Improve async nature of register/unregister auth providers

I noticed a race-condition or 2 with registering & unregistering auth providers. This change makes sure order is maintained dispite the async nature.

* real passing tests

* Try tracking disposable state
This commit is contained in:
Tyler James Leonhardt 2025-06-11 20:30:29 -07:00 committed by GitHub
parent 6ec42e43d5
commit 48e4737a88
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 544 additions and 147 deletions

View File

@ -33,4 +33,9 @@ export class TestSecretStorageService implements ISecretStorageService {
clear(): void {
this._storage.clear();
}
dispose(): void {
this._onDidChangeSecretEmitter.dispose();
this._storage.clear();
}
}

View File

@ -80,6 +80,7 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu
private readonly _registrations = this._register(new DisposableMap<string>());
private _sentProviderUsageEvents = new Set<string>();
private _suppressUnregisterEvent = false;
constructor(
extHostContext: IExtHostContext,
@ -101,7 +102,11 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu
this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostAuthentication);
this._register(this.authenticationService.onDidChangeSessions(e => this._proxy.$onDidChangeAuthenticationSessions(e.providerId, e.label)));
this._register(this.authenticationService.onDidUnregisterAuthenticationProvider(e => this._proxy.$onDidUnregisterAuthenticationProvider(e.id)));
this._register(this.authenticationService.onDidUnregisterAuthenticationProvider(e => {
if (!this._suppressUnregisterEvent) {
this._proxy.$onDidUnregisterAuthenticationProvider(e.id);
}
}));
this._register(this.authenticationExtensionsService.onDidChangeAccountPreference(e => {
const providerInfo = this.authenticationService.getProvider(e.providerId);
this._proxy.$onDidChangeAuthenticationSessions(providerInfo.id, providerInfo.label, e.extensionIds);
@ -153,7 +158,13 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu
async $unregisterAuthenticationProvider(id: string): Promise<void> {
this._registrations.deleteAndDispose(id);
this.authenticationService.unregisterAuthenticationProvider(id);
// The ext host side already unregisters the provider, so we can suppress the event here.
this._suppressUnregisterEvent = true;
try {
this.authenticationService.unregisterAuthenticationProvider(id);
} finally {
this._suppressUnregisterEvent = false;
}
}
async $ensureProvider(id: string): Promise<void> {

View File

@ -16,7 +16,7 @@ import { URI, UriComponents } from '../../../base/common/uri.js';
import { fetchDynamicRegistration, getClaimsFromJWT, IAuthorizationJWTClaims, IAuthorizationProtectedResourceMetadata, IAuthorizationServerMetadata, IAuthorizationTokenResponse, isAuthorizationTokenResponse } from '../../../base/common/oauth.js';
import { IExtHostWindow } from './extHostWindow.js';
import { IExtHostInitDataService } from './extHostInitDataService.js';
import { ILogger, ILoggerService } from '../../../platform/log/common/log.js';
import { ILogger, ILoggerService, ILogService } from '../../../platform/log/common/log.js';
import { autorun, derivedOpts, IObservable, ISettableObservable, observableValue } from '../../../base/common/observable.js';
import { stringHash } from '../../../base/common/hash.js';
import { DisposableStore, IDisposable } from '../../../base/common/lifecycle.js';
@ -26,7 +26,7 @@ import { equals as arraysEqual } from '../../../base/common/arrays.js';
import { IExtHostProgress } from './extHostProgress.js';
import { IProgressStep } from '../../../platform/progress/common/progress.js';
import { CancellationError, isCancellationError } from '../../../base/common/errors.js';
import { raceCancellationError } from '../../../base/common/async.js';
import { raceCancellationError, SequencerByKey } from '../../../base/common/async.js';
export interface IExtHostAuthentication extends ExtHostAuthentication { }
export const IExtHostAuthentication = createDecorator<IExtHostAuthentication>('IExtHostAuthentication');
@ -46,6 +46,7 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape {
private _proxy: MainThreadAuthenticationShape;
private _authenticationProviders: Map<string, ProviderWithMetadata> = new Map<string, ProviderWithMetadata>();
private _providerOperations = new SequencerByKey<string>();
private _onDidChangeSessions = new Emitter<vscode.AuthenticationSessionsChangeEvent & { extensionIdFilter?: string[] }>();
private _getSessionTaskSingler = new TaskSingler<vscode.AuthenticationSession | undefined>();
@ -58,7 +59,8 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape {
@IExtHostWindow private readonly _extHostWindow: IExtHostWindow,
@IExtHostUrlsService private readonly _extHostUrls: IExtHostUrlsService,
@IExtHostProgress private readonly _extHostProgress: IExtHostProgress,
@ILoggerService private readonly _extHostLoggerService: ILoggerService
@ILoggerService private readonly _extHostLoggerService: ILoggerService,
@ILogService private readonly _logService: ILogService,
) {
this._proxy = extHostRpc.getProxy(MainContext.MainThreadAuthentication);
}
@ -98,58 +100,67 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape {
return await this._proxy.$getAccounts(providerId);
}
async removeSession(providerId: string, sessionId: string): Promise<void> {
const providerData = this._authenticationProviders.get(providerId);
if (!providerData) {
return this._proxy.$removeSession(providerId, sessionId);
}
return providerData.provider.removeSession(sessionId);
}
registerAuthenticationProvider(id: string, label: string, provider: vscode.AuthenticationProvider, options?: vscode.AuthenticationProviderOptions): vscode.Disposable {
if (this._authenticationProviders.get(id)) {
throw new Error(`An authentication provider with id '${id}' is already registered.`);
}
this._authenticationProviders.set(id, { label, provider, options: options ?? { supportsMultipleAccounts: false } });
const listener = provider.onDidChangeSessions(e => this._proxy.$sendDidChangeSessions(id, e));
this._proxy.$registerAuthenticationProvider(id, label, options?.supportsMultipleAccounts ?? false, options?.supportedAuthorizationServers);
// register
void this._providerOperations.queue(id, async () => {
// This use to be synchronous, but that wasn't an accurate representation because the main thread
// may have unregistered the provider in the meantime. I don't see how this could really be done
// synchronously, so we just say first one wins.
if (this._authenticationProviders.get(id)) {
this._logService.error(`An authentication provider with id '${id}' is already registered. The existing provider will not be replaced.`);
return;
}
const listener = provider.onDidChangeSessions(e => this._proxy.$sendDidChangeSessions(id, e));
this._authenticationProviders.set(id, { label, provider, disposable: listener, options: options ?? { supportsMultipleAccounts: false } });
await this._proxy.$registerAuthenticationProvider(id, label, options?.supportsMultipleAccounts ?? false, options?.supportedAuthorizationServers);
});
// unregister
return new Disposable(() => {
listener.dispose();
this._authenticationProviders.delete(id);
this._proxy.$unregisterAuthenticationProvider(id);
void this._providerOperations.queue(id, async () => {
const providerData = this._authenticationProviders.get(id);
if (providerData) {
providerData.disposable?.dispose();
this._authenticationProviders.delete(id);
await this._proxy.$unregisterAuthenticationProvider(id);
}
});
});
}
async $createSession(providerId: string, scopes: string[], options: vscode.AuthenticationProviderSessionOptions): Promise<vscode.AuthenticationSession> {
const providerData = this._authenticationProviders.get(providerId);
if (providerData) {
options.authorizationServer = URI.revive(options.authorizationServer);
return await providerData.provider.createSession(scopes, options);
}
$createSession(providerId: string, scopes: string[], options: vscode.AuthenticationProviderSessionOptions): Promise<vscode.AuthenticationSession> {
return this._providerOperations.queue(providerId, async () => {
const providerData = this._authenticationProviders.get(providerId);
if (providerData) {
options.authorizationServer = URI.revive(options.authorizationServer);
return await providerData.provider.createSession(scopes, options);
}
throw new Error(`Unable to find authentication provider with handle: ${providerId}`);
throw new Error(`Unable to find authentication provider with handle: ${providerId}`);
});
}
async $removeSession(providerId: string, sessionId: string): Promise<void> {
const providerData = this._authenticationProviders.get(providerId);
if (providerData) {
return await providerData.provider.removeSession(sessionId);
}
$removeSession(providerId: string, sessionId: string): Promise<void> {
return this._providerOperations.queue(providerId, async () => {
const providerData = this._authenticationProviders.get(providerId);
if (providerData) {
return await providerData.provider.removeSession(sessionId);
}
throw new Error(`Unable to find authentication provider with handle: ${providerId}`);
throw new Error(`Unable to find authentication provider with handle: ${providerId}`);
});
}
async $getSessions(providerId: string, scopes: ReadonlyArray<string> | undefined, options: vscode.AuthenticationProviderSessionOptions): Promise<ReadonlyArray<vscode.AuthenticationSession>> {
const providerData = this._authenticationProviders.get(providerId);
if (providerData) {
options.authorizationServer = URI.revive(options.authorizationServer);
return await providerData.provider.getSessions(scopes, options);
}
$getSessions(providerId: string, scopes: ReadonlyArray<string> | undefined, options: vscode.AuthenticationProviderSessionOptions): Promise<ReadonlyArray<vscode.AuthenticationSession>> {
return this._providerOperations.queue(providerId, async () => {
const providerData = this._authenticationProviders.get(providerId);
if (providerData) {
options.authorizationServer = URI.revive(options.authorizationServer);
return await providerData.provider.getSessions(scopes, options);
}
throw new Error(`Unable to find authentication provider with handle: ${providerId}`);
throw new Error(`Unable to find authentication provider with handle: ${providerId}`);
});
}
$onDidChangeAuthenticationSessions(id: string, label: string, extensionIdFilter?: string[]) {
@ -160,18 +171,14 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape {
return Promise.resolve();
}
// Today, this only handles unregistering extensions that have disposables...
// so basiscally just the dynmaic ones. This was done to fix a bug where
// there was a racecondition between this event and re-registering a provider
// with the same id. (https://github.com/microsoft/vscode-copilot/issues/18045)
// This works for now, but should be cleaned up so theres one flow for register/unregister
$onDidUnregisterAuthenticationProvider(id: string): Promise<void> {
const providerData = this._authenticationProviders.get(id);
if (providerData?.disposable) {
providerData.disposable.dispose();
this._authenticationProviders.delete(id);
}
return Promise.resolve();
return this._providerOperations.queue(id, async () => {
const providerData = this._authenticationProviders.get(id);
if (providerData) {
providerData.disposable?.dispose();
this._authenticationProviders.delete(id);
}
});
}
async $registerDynamicAuthProvider(
@ -206,17 +213,22 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape {
this._onDidDynamicAuthProviderTokensChange,
initialTokens || []
);
const disposable = provider.onDidChangeSessions(e => this._proxy.$sendDidChangeSessions(provider.id, e));
this._authenticationProviders.set(
provider.id,
{
label: provider.label,
provider,
disposable: Disposable.from(provider, disposable),
options: { supportsMultipleAccounts: false }
}
);
await this._proxy.$registerDynamicAuthenticationProvider(provider.id, provider.label, provider.authorizationServer, provider.clientId);
// Use the sequencer to ensure dynamic provider registration is serialized
await this._providerOperations.queue(provider.id, async () => {
const disposable = provider.onDidChangeSessions(e => this._proxy.$sendDidChangeSessions(provider.id, e));
this._authenticationProviders.set(
provider.id,
{
label: provider.label,
provider,
disposable: Disposable.from(provider, disposable),
options: { supportsMultipleAccounts: false }
}
);
await this._proxy.$registerDynamicAuthenticationProvider(provider.id, provider.label, provider.authorizationServer, provider.clientId);
});
return provider.id;
}

View File

@ -13,7 +13,7 @@ import { IExtHostRpcService } from '../common/extHostRpcService.js';
import { IExtHostInitDataService } from '../common/extHostInitDataService.js';
import { IExtHostWindow } from '../common/extHostWindow.js';
import { IExtHostUrlsService } from '../common/extHostUrls.js';
import { ILogger, ILoggerService } from '../../../platform/log/common/log.js';
import { ILogger, ILoggerService, ILogService } from '../../../platform/log/common/log.js';
import { MainThreadAuthenticationShape } from '../common/extHost.protocol.js';
import { IAuthorizationServerMetadata, IAuthorizationProtectedResourceMetadata, IAuthorizationTokenResponse, DEFAULT_AUTH_FLOW_PORT, IAuthorizationDeviceResponse, isAuthorizationDeviceResponse, isAuthorizationTokenResponse, IAuthorizationDeviceTokenErrorResponse } from '../../../base/common/oauth.js';
import { Emitter } from '../../../base/common/event.js';
@ -476,8 +476,9 @@ export class NodeExtHostAuthentication extends ExtHostAuthentication implements
extHostUrls: IExtHostUrlsService,
extHostProgress: IExtHostProgress,
extHostLoggerService: ILoggerService,
extHostLogService: ILogService
) {
super(extHostRpc, initData, extHostWindow, extHostUrls, extHostProgress, extHostLoggerService);
super(extHostRpc, initData, extHostWindow, extHostUrls, extHostProgress, extHostLoggerService, extHostLogService);
}
}

View File

@ -4,13 +4,12 @@
*--------------------------------------------------------------------------------------------*/
import assert from 'assert';
import { DisposableStore } from '../../../../base/common/lifecycle.js';
import { IDialogService } from '../../../../platform/dialogs/common/dialogs.js';
import { TestDialogService } from '../../../../platform/dialogs/test/common/testDialogService.js';
import { TestInstantiationService } from '../../../../platform/instantiation/test/common/instantiationServiceMock.js';
import { INotificationService } from '../../../../platform/notification/common/notification.js';
import { TestNotificationService } from '../../../../platform/notification/test/common/testNotificationService.js';
import { IQuickInputHideEvent, IQuickInputService, IQuickPickDidAcceptEvent } from '../../../../platform/quickinput/common/quickInput.js';
import { IQuickInputHideEvent, IQuickInputService, IQuickPickDidAcceptEvent, IQuickPickItem, QuickInputHideReason } from '../../../../platform/quickinput/common/quickInput.js';
import { IStorageService } from '../../../../platform/storage/common/storage.js';
import { ITelemetryService } from '../../../../platform/telemetry/common/telemetry.js';
import { NullTelemetryService } from '../../../../platform/telemetry/common/telemetryUtils.js';
@ -44,27 +43,31 @@ import { TestSecretStorageService } from '../../../../platform/secrets/test/comm
import { IDynamicAuthenticationProviderStorageService } from '../../../services/authentication/common/dynamicAuthenticationProviderStorage.js';
import { DynamicAuthenticationProviderStorageService } from '../../../services/authentication/browser/dynamicAuthenticationProviderStorageService.js';
import { ExtHostProgress } from '../../common/extHostProgress.js';
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js';
import { ServiceCollection } from '../../../../platform/instantiation/common/serviceCollection.js';
import { SyncDescriptor } from '../../../../platform/instantiation/common/descriptors.js';
class AuthQuickPick {
private listener: ((e: IQuickPickDidAcceptEvent) => any) | undefined;
private accept: ((e: IQuickPickDidAcceptEvent) => any) | undefined;
private hide: ((e: IQuickInputHideEvent) => any) | undefined;
public items = [];
public get selectedItems(): string[] {
public get selectedItems(): IQuickPickItem[] {
return this.items;
}
onDidAccept(listener: (e: IQuickPickDidAcceptEvent) => any) {
this.listener = listener;
this.accept = listener;
}
onDidHide(listener: (e: IQuickInputHideEvent) => any) {
this.hide = listener;
}
dispose() {
}
show() {
this.listener!({
inBackground: false
});
this.accept?.({ inBackground: false });
this.hide?.({ reason: QuickInputHideReason.Other });
}
}
class AuthTestQuickInputService extends TestQuickInputService {
@ -111,39 +114,43 @@ class TestAuthProvider implements AuthenticationProvider {
}
suite('ExtHostAuthentication', () => {
let disposables: DisposableStore;
const disposables = ensureNoDisposablesAreLeakedInTestSuite();
let extHostAuthentication: ExtHostAuthentication;
let instantiationService: TestInstantiationService;
let mainInstantiationService: TestInstantiationService;
suiteSetup(async () => {
instantiationService = new TestInstantiationService();
instantiationService.stub(ILogService, new NullLogService());
instantiationService.stub(IDialogService, new TestDialogService({ confirmed: true }));
instantiationService.stub(IStorageService, new TestStorageService());
instantiationService.stub(ISecretStorageService, new TestSecretStorageService());
instantiationService.stub(IDynamicAuthenticationProviderStorageService, instantiationService.createInstance(DynamicAuthenticationProviderStorageService));
instantiationService.stub(IQuickInputService, new AuthTestQuickInputService());
instantiationService.stub(IExtensionService, new TestExtensionService());
setup(async () => {
// services
const services = new ServiceCollection();
services.set(ILogService, new SyncDescriptor(NullLogService));
services.set(IDialogService, new SyncDescriptor(TestDialogService, [{ confirmed: true }]));
services.set(IStorageService, new SyncDescriptor(TestStorageService));
services.set(ISecretStorageService, new SyncDescriptor(TestSecretStorageService));
services.set(IDynamicAuthenticationProviderStorageService, new SyncDescriptor(DynamicAuthenticationProviderStorageService));
services.set(IQuickInputService, new SyncDescriptor(AuthTestQuickInputService));
services.set(IExtensionService, new SyncDescriptor(TestExtensionService));
services.set(IActivityService, new SyncDescriptor(TestActivityService));
services.set(IRemoteAgentService, new SyncDescriptor(TestRemoteAgentService));
services.set(INotificationService, new SyncDescriptor(TestNotificationService));
services.set(IHostService, new SyncDescriptor(TestHostService));
services.set(IUserActivityService, new SyncDescriptor(UserActivityService));
services.set(IAuthenticationAccessService, new SyncDescriptor(AuthenticationAccessService));
services.set(IAuthenticationService, new SyncDescriptor(AuthenticationService));
services.set(IAuthenticationUsageService, new SyncDescriptor(AuthenticationUsageService));
services.set(IAuthenticationExtensionsService, new SyncDescriptor(AuthenticationExtensionsService));
mainInstantiationService = disposables.add(new TestInstantiationService(services, undefined, undefined, true));
instantiationService.stub(IActivityService, new TestActivityService());
instantiationService.stub(IRemoteAgentService, new TestRemoteAgentService());
instantiationService.stub(INotificationService, new TestNotificationService());
instantiationService.stub(IHostService, new TestHostService());
// stubs
// eslint-disable-next-line local/code-no-dangerous-type-assertions
instantiationService.stub(IOpenerService, {} as Partial<IOpenerService>);
instantiationService.stub(IUserActivityService, new UserActivityService(instantiationService));
instantiationService.stub(ITelemetryService, NullTelemetryService);
instantiationService.stub(IBrowserWorkbenchEnvironmentService, TestEnvironmentService);
instantiationService.stub(IProductService, TestProductService);
instantiationService.stub(IAuthenticationAccessService, instantiationService.createInstance(AuthenticationAccessService));
instantiationService.stub(IAuthenticationService, instantiationService.createInstance(AuthenticationService));
instantiationService.stub(IAuthenticationUsageService, instantiationService.createInstance(AuthenticationUsageService));
const rpcProtocol = new TestRPCProtocol();
mainInstantiationService.stub(IOpenerService, {} as Partial<IOpenerService>);
mainInstantiationService.stub(ITelemetryService, NullTelemetryService);
mainInstantiationService.stub(IBrowserWorkbenchEnvironmentService, TestEnvironmentService);
mainInstantiationService.stub(IProductService, TestProductService);
instantiationService.stub(IAuthenticationExtensionsService, instantiationService.createInstance(AuthenticationExtensionsService));
rpcProtocol.set(MainContext.MainThreadAuthentication, instantiationService.createInstance(MainThreadAuthentication, rpcProtocol));
rpcProtocol.set(MainContext.MainThreadWindow, instantiationService.createInstance(MainThreadWindow, rpcProtocol));
const rpcProtocol = disposables.add(new TestRPCProtocol());
rpcProtocol.set(MainContext.MainThreadAuthentication, disposables.add(mainInstantiationService.createInstance(MainThreadAuthentication, rpcProtocol)));
rpcProtocol.set(MainContext.MainThreadWindow, disposables.add(mainInstantiationService.createInstance(MainThreadWindow, rpcProtocol)));
const initData: IExtHostInitDataService = {
environment: {
appUriScheme: 'test',
@ -161,13 +168,10 @@ suite('ExtHostAuthentication', () => {
new ExtHostWindow(initData, rpcProtocol),
new ExtHostUrls(rpcProtocol),
new ExtHostProgress(rpcProtocol),
new TestLoggerService(),
disposables.add(new TestLoggerService()),
new NullLogService()
);
rpcProtocol.set(ExtHostContext.ExtHostAuthentication, extHostAuthentication);
});
setup(async () => {
disposables = new DisposableStore();
disposables.add(extHostAuthentication.registerAuthenticationProvider('test', 'test provider', new TestAuthProvider('test')));
disposables.add(extHostAuthentication.registerAuthenticationProvider(
'test-multiple',
@ -176,14 +180,6 @@ suite('ExtHostAuthentication', () => {
{ supportsMultipleAccounts: true }));
});
suiteTeardown(() => {
instantiationService.dispose();
});
teardown(() => {
disposables.dispose();
});
test('createIfNone - true', async () => {
const scopes = ['foo'];
const session = await extHostAuthentication.getSession(
@ -553,5 +549,215 @@ suite('ExtHostAuthentication', () => {
});
//#endregion
//#region Race Condition and Sequencing Tests
test('concurrent operations on same provider are serialized', async () => {
const provider = new TestAuthProvider('concurrent-test');
const operationOrder: string[] = [];
// Mock the provider methods to track operation order
const originalCreateSession = provider.createSession.bind(provider);
const originalGetSessions = provider.getSessions.bind(provider);
provider.createSession = async (scopes) => {
operationOrder.push(`create-start-${scopes[0]}`);
await new Promise(resolve => setTimeout(resolve, 20)); // Simulate async work
const result = await originalCreateSession(scopes);
operationOrder.push(`create-end-${scopes[0]}`);
return result;
};
provider.getSessions = async (scopes) => {
const scopeKey = scopes ? scopes[0] : 'all';
operationOrder.push(`get-start-${scopeKey}`);
await new Promise(resolve => setTimeout(resolve, 10)); // Simulate async work
const result = await originalGetSessions(scopes);
operationOrder.push(`get-end-${scopeKey}`);
return result;
};
const disposable = extHostAuthentication.registerAuthenticationProvider('concurrent-test', 'Concurrent Test', provider);
disposables.add(disposable);
// Start multiple operations simultaneously on the same provider
const promises = [
extHostAuthentication.getSession(extensionDescription, 'concurrent-test', ['scope1'], { createIfNone: true }),
extHostAuthentication.getSession(extensionDescription, 'concurrent-test', ['scope2'], { createIfNone: true }),
extHostAuthentication.getSession(extensionDescription, 'concurrent-test', ['scope1'], {}) // This should get the existing session
];
await Promise.all(promises);
// Verify that operations were serialized - no overlapping operations
// Build a map of operation starts to their corresponding ends
const operationPairs: Array<{ start: number; end: number; operation: string }> = [];
for (let i = 0; i < operationOrder.length; i++) {
const current = operationOrder[i];
if (current.includes('-start-')) {
const scope = current.split('-start-')[1];
const operationType = current.split('-start-')[0];
const endOperation = `${operationType}-end-${scope}`;
const endIndex = operationOrder.indexOf(endOperation, i + 1);
if (endIndex !== -1) {
operationPairs.push({
start: i,
end: endIndex,
operation: `${operationType}-${scope}`
});
}
}
}
// Verify no operations overlap (serialization)
for (let i = 0; i < operationPairs.length; i++) {
for (let j = i + 1; j < operationPairs.length; j++) {
const op1 = operationPairs[i];
const op2 = operationPairs[j];
// Operations should not overlap - one should completely finish before the other starts
const op1EndsBeforeOp2Starts = op1.end < op2.start;
const op2EndsBeforeOp1Starts = op2.end < op1.start;
assert.ok(op1EndsBeforeOp2Starts || op2EndsBeforeOp1Starts,
`Operations ${op1.operation} and ${op2.operation} should not overlap. ` +
`Op1: ${op1.start}-${op1.end}, Op2: ${op2.start}-${op2.end}. ` +
`Order: [${operationOrder.join(', ')}]`);
}
}
// Verify we have the expected operations
assert.ok(operationOrder.includes('create-start-scope1'), 'Should have created session for scope1');
assert.ok(operationOrder.includes('create-end-scope1'), 'Should have completed creating session for scope1');
assert.ok(operationOrder.includes('create-start-scope2'), 'Should have created session for scope2');
assert.ok(operationOrder.includes('create-end-scope2'), 'Should have completed creating session for scope2');
// The third call should use getSessions to find the existing scope1 session
assert.ok(operationOrder.includes('get-start-scope1'), 'Should have called getSessions for existing scope1 session');
assert.ok(operationOrder.includes('get-end-scope1'), 'Should have completed getSessions for existing scope1 session');
});
test('provider registration and immediate disposal race condition', async () => {
const provider = new TestAuthProvider('race-test');
// Register and immediately dispose
const disposable = extHostAuthentication.registerAuthenticationProvider('race-test', 'Race Test', provider);
disposable.dispose();
// Try to use the provider after disposal - should fail gracefully
try {
await extHostAuthentication.getSession(extensionDescription, 'race-test', ['scope'], { createIfNone: true });
assert.fail('Should have thrown an error for non-existent provider');
} catch (error) {
// Expected - provider should be unavailable
assert.ok(error);
}
});
test('provider re-registration after proper disposal', async () => {
const provider1 = new TestAuthProvider('reregister-test-1');
const provider2 = new TestAuthProvider('reregister-test-2');
// First registration
const disposable1 = extHostAuthentication.registerAuthenticationProvider('reregister-test', 'Provider 1', provider1);
// Create a session with first provider
const session1 = await extHostAuthentication.getSession(extensionDescription, 'reregister-test', ['scope'], { createIfNone: true });
assert.strictEqual(session1?.account.label, 'reregister-test-1');
// Dispose first provider
disposable1.dispose();
// Re-register with different provider
const disposable2 = extHostAuthentication.registerAuthenticationProvider('reregister-test', 'Provider 2', provider2);
disposables.add(disposable2);
// Create session with second provider
const session2 = await extHostAuthentication.getSession(extensionDescription, 'reregister-test', ['scope'], { createIfNone: true });
assert.strictEqual(session2?.account.label, 'reregister-test-2');
assert.notStrictEqual(session1?.accessToken, session2?.accessToken);
});
test('session operations during provider lifecycle changes', async () => {
const provider = new TestAuthProvider('lifecycle-test');
const disposable = extHostAuthentication.registerAuthenticationProvider('lifecycle-test', 'Lifecycle Test', provider);
// Start a session creation
const sessionPromise = extHostAuthentication.getSession(extensionDescription, 'lifecycle-test', ['scope'], { createIfNone: true });
// Don't dispose immediately - let the session creation start
await new Promise(resolve => setTimeout(resolve, 5));
// Dispose the provider while the session creation is likely still in progress
disposable.dispose();
// The session creation should complete successfully even if we dispose during the operation
const session = await sessionPromise;
assert.ok(session);
assert.strictEqual(session.account.label, 'lifecycle-test');
});
test('operations on different providers run concurrently', async () => {
const provider1 = new TestAuthProvider('concurrent-1');
const provider2 = new TestAuthProvider('concurrent-2');
let provider1Started = false;
let provider2Started = false;
let provider1Finished = false;
let provider2Finished = false;
let concurrencyVerified = false;
// Override createSession to track timing
const originalCreate1 = provider1.createSession.bind(provider1);
const originalCreate2 = provider2.createSession.bind(provider2);
provider1.createSession = async (scopes) => {
provider1Started = true;
await new Promise(resolve => setTimeout(resolve, 20));
const result = await originalCreate1(scopes);
provider1Finished = true;
return result;
};
provider2.createSession = async (scopes) => {
provider2Started = true;
// Provider 2 should start before provider 1 finishes (concurrent execution)
if (provider1Started && !provider1Finished) {
concurrencyVerified = true;
}
await new Promise(resolve => setTimeout(resolve, 10));
const result = await originalCreate2(scopes);
provider2Finished = true;
return result;
};
const disposable1 = extHostAuthentication.registerAuthenticationProvider('concurrent-1', 'Concurrent 1', provider1);
const disposable2 = extHostAuthentication.registerAuthenticationProvider('concurrent-2', 'Concurrent 2', provider2);
disposables.add(disposable1);
disposables.add(disposable2);
// Start operations on both providers simultaneously
const [session1, session2] = await Promise.all([
extHostAuthentication.getSession(extensionDescription, 'concurrent-1', ['scope'], { createIfNone: true }),
extHostAuthentication.getSession(extensionDescription, 'concurrent-2', ['scope'], { createIfNone: true })
]);
// Verify both operations completed successfully
assert.ok(session1);
assert.ok(session2);
assert.ok(provider1Started, 'Provider 1 should have started');
assert.ok(provider2Started, 'Provider 2 should have started');
assert.ok(provider1Finished, 'Provider 1 should have finished');
assert.ok(provider2Finished, 'Provider 2 should have finished');
assert.strictEqual(session1.account.label, 'concurrent-1');
assert.strictEqual(session2.account.label, 'concurrent-2');
// Verify that operations ran concurrently (provider 2 started while provider 1 was still running)
assert.ok(concurrencyVerified, 'Operations should have run concurrently - provider 2 should start while provider 1 is still running');
});
//#endregion
});

View File

@ -0,0 +1,151 @@
/*---------------------------------------------------------------------------------------------
* 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 { IDialogService } from '../../../../platform/dialogs/common/dialogs.js';
import { TestDialogService } from '../../../../platform/dialogs/test/common/testDialogService.js';
import { TestInstantiationService } from '../../../../platform/instantiation/test/common/instantiationServiceMock.js';
import { INotificationService } from '../../../../platform/notification/common/notification.js';
import { TestNotificationService } from '../../../../platform/notification/test/common/testNotificationService.js';
import { IQuickInputService } from '../../../../platform/quickinput/common/quickInput.js';
import { IStorageService } from '../../../../platform/storage/common/storage.js';
import { ITelemetryService } from '../../../../platform/telemetry/common/telemetry.js';
import { NullTelemetryService } from '../../../../platform/telemetry/common/telemetryUtils.js';
import { MainThreadAuthentication } from '../../browser/mainThreadAuthentication.js';
import { ExtHostContext, MainContext } from '../../common/extHost.protocol.js';
import { IActivityService } from '../../../services/activity/common/activity.js';
import { AuthenticationService } from '../../../services/authentication/browser/authenticationService.js';
import { IAuthenticationExtensionsService, IAuthenticationService } from '../../../services/authentication/common/authentication.js';
import { IExtensionService } from '../../../services/extensions/common/extensions.js';
import { IRemoteAgentService } from '../../../services/remote/common/remoteAgentService.js';
import { TestRPCProtocol } from '../common/testRPCProtocol.js';
import { TestEnvironmentService, TestHostService, TestQuickInputService, TestRemoteAgentService } from '../../../test/browser/workbenchTestServices.js';
import { TestActivityService, TestExtensionService, TestProductService, TestStorageService } from '../../../test/common/workbenchTestServices.js';
import { IBrowserWorkbenchEnvironmentService } from '../../../services/environment/browser/environmentService.js';
import { IProductService } from '../../../../platform/product/common/productService.js';
import { AuthenticationAccessService, IAuthenticationAccessService } from '../../../services/authentication/browser/authenticationAccessService.js';
import { AuthenticationUsageService, IAuthenticationUsageService } from '../../../services/authentication/browser/authenticationUsageService.js';
import { AuthenticationExtensionsService } from '../../../services/authentication/browser/authenticationExtensionsService.js';
import { ILogService, NullLogService } from '../../../../platform/log/common/log.js';
import { IHostService } from '../../../services/host/browser/host.js';
import { IOpenerService } from '../../../../platform/opener/common/opener.js';
import { IUserActivityService, UserActivityService } from '../../../services/userActivity/common/userActivityService.js';
import { ISecretStorageService } from '../../../../platform/secrets/common/secrets.js';
import { TestSecretStorageService } from '../../../../platform/secrets/test/common/testSecretStorageService.js';
import { IDynamicAuthenticationProviderStorageService } from '../../../services/authentication/common/dynamicAuthenticationProviderStorage.js';
import { DynamicAuthenticationProviderStorageService } from '../../../services/authentication/browser/dynamicAuthenticationProviderStorageService.js';
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js';
import { ServiceCollection } from '../../../../platform/instantiation/common/serviceCollection.js';
import { SyncDescriptor } from '../../../../platform/instantiation/common/descriptors.js';
suite('MainThreadAuthentication', () => {
const disposables = ensureNoDisposablesAreLeakedInTestSuite();
let mainThreadAuthentication: MainThreadAuthentication;
let instantiationService: TestInstantiationService;
let rpcProtocol: TestRPCProtocol;
setup(async () => {
// services
const services = new ServiceCollection();
services.set(ILogService, new SyncDescriptor(NullLogService));
services.set(IDialogService, new SyncDescriptor(TestDialogService, [{ confirmed: true }]));
services.set(IStorageService, new SyncDescriptor(TestStorageService));
services.set(ISecretStorageService, new SyncDescriptor(TestSecretStorageService));
services.set(IDynamicAuthenticationProviderStorageService, new SyncDescriptor(DynamicAuthenticationProviderStorageService));
services.set(IQuickInputService, new SyncDescriptor(TestQuickInputService));
services.set(IExtensionService, new SyncDescriptor(TestExtensionService));
services.set(IActivityService, new SyncDescriptor(TestActivityService));
services.set(IRemoteAgentService, new SyncDescriptor(TestRemoteAgentService));
services.set(INotificationService, new SyncDescriptor(TestNotificationService));
services.set(IHostService, new SyncDescriptor(TestHostService));
services.set(IUserActivityService, new SyncDescriptor(UserActivityService));
services.set(IAuthenticationAccessService, new SyncDescriptor(AuthenticationAccessService));
services.set(IAuthenticationService, new SyncDescriptor(AuthenticationService));
services.set(IAuthenticationUsageService, new SyncDescriptor(AuthenticationUsageService));
services.set(IAuthenticationExtensionsService, new SyncDescriptor(AuthenticationExtensionsService));
instantiationService = disposables.add(new TestInstantiationService(services, undefined, undefined, true));
// stubs
// eslint-disable-next-line local/code-no-dangerous-type-assertions
instantiationService.stub(IOpenerService, {} as Partial<IOpenerService>);
instantiationService.stub(ITelemetryService, NullTelemetryService);
instantiationService.stub(IBrowserWorkbenchEnvironmentService, TestEnvironmentService);
instantiationService.stub(IProductService, TestProductService);
rpcProtocol = disposables.add(new TestRPCProtocol());
mainThreadAuthentication = disposables.add(instantiationService.createInstance(MainThreadAuthentication, rpcProtocol));
rpcProtocol.set(MainContext.MainThreadAuthentication, mainThreadAuthentication);
});
test('provider registration completes without errors', async () => {
// Test basic registration - this should complete without throwing
await mainThreadAuthentication.$registerAuthenticationProvider('test-provider', 'Test Provider', false);
// Test unregistration - this should also complete without throwing
await mainThreadAuthentication.$unregisterAuthenticationProvider('test-provider');
// Success if we reach here without timeout
assert.ok(true, 'Registration and unregistration completed successfully');
});
test('event suppression during explicit unregistration', async () => {
let unregisterEventFired = false;
let eventProviderId: string | undefined;
// Mock the ext host to capture unregister events
const mockExtHost = {
$onDidUnregisterAuthenticationProvider: (id: string) => {
unregisterEventFired = true;
eventProviderId = id;
return Promise.resolve();
},
$getSessions: () => Promise.resolve([]),
$createSession: () => Promise.resolve({} as any),
$removeSession: () => Promise.resolve(),
$onDidChangeAuthenticationSessions: () => Promise.resolve(),
$registerDynamicAuthProvider: () => Promise.resolve('test'),
$onDidChangeDynamicAuthProviderTokens: () => Promise.resolve()
};
rpcProtocol.set(ExtHostContext.ExtHostAuthentication, mockExtHost);
// Register a provider
await mainThreadAuthentication.$registerAuthenticationProvider('test-suppress', 'Test Suppress', false);
// Reset the flag
unregisterEventFired = false;
eventProviderId = undefined;
// Unregister the provider - this should NOT fire the event due to suppression
await mainThreadAuthentication.$unregisterAuthenticationProvider('test-suppress');
// Verify the event was suppressed
assert.strictEqual(unregisterEventFired, false, 'Unregister event should be suppressed during explicit unregistration');
assert.strictEqual(eventProviderId, undefined, 'No provider ID should be captured from suppressed event');
});
test('concurrent provider registrations complete without errors', async () => {
// Register multiple providers simultaneously
const registrationPromises = [
mainThreadAuthentication.$registerAuthenticationProvider('concurrent-1', 'Concurrent 1', false),
mainThreadAuthentication.$registerAuthenticationProvider('concurrent-2', 'Concurrent 2', false),
mainThreadAuthentication.$registerAuthenticationProvider('concurrent-3', 'Concurrent 3', false)
];
await Promise.all(registrationPromises);
// Unregister all providers
const unregistrationPromises = [
mainThreadAuthentication.$unregisterAuthenticationProvider('concurrent-1'),
mainThreadAuthentication.$unregisterAuthenticationProvider('concurrent-2'),
mainThreadAuthentication.$unregisterAuthenticationProvider('concurrent-3')
];
await Promise.all(unregistrationPromises);
// Success if we reach here without timeout
assert.ok(true, 'Concurrent registrations and unregistrations completed successfully');
});
});

View File

@ -146,9 +146,7 @@ export class TestRPCProtocol implements IExtHostContext, IExtHostRpcService {
});
}
public dispose() {
throw new Error('Not implemented!');
}
public dispose() { }
public assertRegistered(identifiers: ProxyIdentifier<any>[]): void {
throw new Error('Not implemented!');

View File

@ -21,6 +21,7 @@ import { ExtensionsRegistry } from '../../extensions/common/extensionsRegistry.j
import { match } from '../../../../base/common/glob.js';
import { URI } from '../../../../base/common/uri.js';
import { IAuthorizationProtectedResourceMetadata, IAuthorizationServerMetadata } from '../../../../base/common/oauth.js';
import { raceTimeout } from '../../../../base/common/async.js';
export function getAuthenticationProviderActivationEvent(id: string): string { return `onAuthenticationRequest:${id}`; }
@ -108,6 +109,8 @@ export class AuthenticationService extends Disposable implements IAuthentication
private readonly _delegates: IAuthenticationProviderHostDelegate[] = [];
private _isDisposable: boolean = false;
constructor(
@IExtensionService private readonly _extensionService: IExtensionService,
@IAuthenticationAccessService authenticationAccessService: IAuthenticationAccessService,
@ -115,7 +118,7 @@ export class AuthenticationService extends Disposable implements IAuthentication
@ILogService private readonly _logService: ILogService
) {
super();
this._register(toDisposable(() => this._isDisposable = true));
this._register(authenticationAccessService.onDidChangeExtensionSessionAccess(e => {
// The access has changed, not the actual session itself but extensions depend on this event firing
// when they have gained access to an account so this fires that event.
@ -264,6 +267,10 @@ export class AuthenticationService extends Disposable implements IAuthentication
}
async getSessions(id: string, scopes?: string[], options?: IAuthenticationGetSessionsOptions, activateImmediate: boolean = false): Promise<ReadonlyArray<AuthenticationSession>> {
if (this._isDisposable) {
return [];
}
const authProvider = this._authenticationProviders.get(id) || await this.tryActivateProvider(id, activateImmediate);
if (authProvider) {
// Check if the authorization server is in the list of supported authorization servers
@ -281,6 +288,10 @@ export class AuthenticationService extends Disposable implements IAuthentication
}
async createSession(id: string, scopes: string[], options?: IAuthenticationCreateSessionOptions): Promise<AuthenticationSession> {
if (this._isDisposable) {
throw new Error('Authentication service is disposed.');
}
const authProvider = this._authenticationProviders.get(id) || await this.tryActivateProvider(id, !!options?.activateImmediate);
if (authProvider) {
return await authProvider.createSession(scopes, {
@ -293,6 +304,10 @@ export class AuthenticationService extends Disposable implements IAuthentication
}
async removeSession(id: string, sessionId: string): Promise<void> {
if (this._isDisposable) {
throw new Error('Authentication service is disposed.');
}
const authProvider = this._authenticationProviders.get(id);
if (authProvider) {
return authProvider.removeSession(sessionId);
@ -361,32 +376,30 @@ export class AuthenticationService extends Disposable implements IAuthentication
return provider;
}
const store = new DisposableStore();
// When activate has completed, the extension has made the call to `registerAuthenticationProvider`.
// However, activate cannot block on this, so the renderer may not have gotten the event yet.
const didRegister: Promise<IAuthenticationProvider> = new Promise((resolve, _) => {
store.add(Event.once(this.onDidRegisterAuthenticationProvider)(e => {
if (e.id === providerId) {
provider = this._authenticationProviders.get(providerId);
if (provider) {
resolve(provider);
} else {
throw new Error(`No authentication provider '${providerId}' is currently registered.`);
}
}
}));
});
const didTimeout: Promise<IAuthenticationProvider> = new Promise((_, reject) => {
const handle = setTimeout(() => {
reject('Timed out waiting for authentication provider to register');
}, 5000);
store.add(toDisposable(() => clearTimeout(handle)));
});
return Promise.race([didRegister, didTimeout]).finally(() => store.dispose());
const store = this._register(new DisposableStore());
try {
const result = await raceTimeout(
Event.toPromise(
Event.filter(
this.onDidRegisterAuthenticationProvider,
e => e.id === providerId,
store
),
store
),
5000
);
if (!result) {
throw new Error(`Timed out waiting for authentication provider '${providerId}' to register.`);
}
provider = this._authenticationProviders.get(result.id);
if (provider) {
return provider;
}
throw new Error(`No authentication provider '${providerId}' is currently registered.`);
} finally {
store.dispose();
}
}
}