Remove polling from auth extensions (#108428)

extensions/github-authentication/src/extension.ts

@@ -16,7 +16,7 @@ export async function activate(context: vscode.ExtensionContext) {
 	context.subscriptions.push(vscode.window.registerUriHandler(uriHandler));
 	const loginService = new GitHubAuthenticationProvider();
 
-	await loginService.initialize();
+	await loginService.initialize(context);
 
 	context.subscriptions.push(vscode.commands.registerCommand('github.provide-token', () => {
 		return loginService.manuallyProvideToken();

extensions/github-authentication/src/github.ts

@@ -26,59 +26,55 @@ export class GitHubAuthenticationProvider {
 	private _sessions: vscode.AuthenticationSession[] = [];
 	private _githubServer = new GitHubServer();
 
-	public async initialize(): Promise<void> {
+	public async initialize(context: vscode.ExtensionContext): Promise<void> {
 		try {
 			this._sessions = await this.readSessions();
 		} catch (e) {
 			// Ignore, network request failed
 		}
 
-		this.pollForChange();
+		context.subscriptions.push(vscode.authentication.onDidChangePassword(() => this.checkForUpdates()));
 	}
 
-	private pollForChange() {
-		setTimeout(async () => {
-			let storedSessions: vscode.AuthenticationSession[];
-			try {
-				storedSessions = await this.readSessions();
-			} catch (e) {
-				// Ignore, network request failed
-				return;
-			}
-
-			const added: string[] = [];
-			const removed: string[] = [];
+	private async checkForUpdates() {
+		let storedSessions: vscode.AuthenticationSession[];
+		try {
+			storedSessions = await this.readSessions();
+		} catch (e) {
+			// Ignore, network request failed
+			return;
+		}
 
-			storedSessions.forEach(session => {
-				const matchesExisting = this._sessions.some(s => s.id === session.id);
-				// Another window added a session to the keychain, add it to our state as well
-				if (!matchesExisting) {
-					Logger.info('Adding session found in keychain');
-					this._sessions.push(session);
-					added.push(session.id);
-				}
-			});
-
-			this._sessions.map(session => {
-				const matchesExisting = storedSessions.some(s => s.id === session.id);
-				// Another window has logged out, remove from our state
-				if (!matchesExisting) {
-					Logger.info('Removing session no longer found in keychain');
-					const sessionIndex = this._sessions.findIndex(s => s.id === session.id);
-					if (sessionIndex > -1) {
-						this._sessions.splice(sessionIndex, 1);
-					}
+		const added: string[] = [];
+		const removed: string[] = [];
 
-					removed.push(session.id);
+		storedSessions.forEach(session => {
+			const matchesExisting = this._sessions.some(s => s.id === session.id);
+			// Another window added a session to the keychain, add it to our state as well
+			if (!matchesExisting) {
+				Logger.info('Adding session found in keychain');
+				this._sessions.push(session);
+				added.push(session.id);
+			}
+		});
+
+		this._sessions.map(session => {
+			const matchesExisting = storedSessions.some(s => s.id === session.id);
+			// Another window has logged out, remove from our state
+			if (!matchesExisting) {
+				Logger.info('Removing session no longer found in keychain');
+				const sessionIndex = this._sessions.findIndex(s => s.id === session.id);
+				if (sessionIndex > -1) {
+					this._sessions.splice(sessionIndex, 1);
 				}
-			});
 
-			if (added.length || removed.length) {
-				onDidChangeSessions.fire({ added, removed, changed: [] });
+				removed.push(session.id);
 			}
+		});
 
-			this.pollForChange();
-		}, 1000 * 30);
+		if (added.length || removed.length) {
+			onDidChangeSessions.fire({ added, removed, changed: [] });
+		}
 	}
 
 	private async readSessions(): Promise<vscode.AuthenticationSession[]> {

extensions/microsoft-authentication/src/AADHelper.ts

@@ -93,10 +93,11 @@ export class AzureActiveDirectoryService {
 	private _tokens: IToken[] = [];
 	private _refreshTimeouts: Map<string, NodeJS.Timeout> = new Map<string, NodeJS.Timeout>();
 	private _uriHandler: UriEventHandler;
+	private _disposables: vscode.Disposable[] = [];
 
 	constructor() {
 		this._uriHandler = new UriEventHandler();
-		vscode.window.registerUriHandler(this._uriHandler);
+		this._disposables.push(vscode.window.registerUriHandler(this._uriHandler));
 	}
 
 	public async initialize(): Promise<void> {
@@ -140,7 +141,7 @@ export class AzureActiveDirectoryService {
 			}
 		}
 
-		this.pollForChange();
+		this._disposables.push(vscode.authentication.onDidChangePassword(() => this.checkForUpdates));
 	}
 
 	private parseStoredData(data: string): IStoredSession[] {
@@ -160,67 +161,63 @@ export class AzureActiveDirectoryService {
 		await keychain.setToken(JSON.stringify(serializedData));
 	}
 
-	private pollForChange() {
-		setTimeout(async () => {
-			const addedIds: string[] = [];
-			let removedIds: string[] = [];
-			const storedData = await keychain.getToken();
-			if (storedData) {
-				try {
-					const sessions = this.parseStoredData(storedData);
-					let promises = sessions.map(async session => {
-						const matchesExisting = this._tokens.some(token => token.scope === session.scope && token.sessionId === session.id);
-						if (!matchesExisting && session.refreshToken) {
-							try {
-								await this.refreshToken(session.refreshToken, session.scope, session.id);
-								addedIds.push(session.id);
-							} catch (e) {
-								if (e.message === REFRESH_NETWORK_FAILURE) {
-									// Ignore, will automatically retry on next poll.
-								} else {
-									await this.logout(session.id);
-								}
+	private async checkForUpdates(): Promise<void> {
+		const addedIds: string[] = [];
+		let removedIds: string[] = [];
+		const storedData = await keychain.getToken();
+		if (storedData) {
+			try {
+				const sessions = this.parseStoredData(storedData);
+				let promises = sessions.map(async session => {
+					const matchesExisting = this._tokens.some(token => token.scope === session.scope && token.sessionId === session.id);
+					if (!matchesExisting && session.refreshToken) {
+						try {
+							await this.refreshToken(session.refreshToken, session.scope, session.id);
+							addedIds.push(session.id);
+						} catch (e) {
+							if (e.message === REFRESH_NETWORK_FAILURE) {
+								// Ignore, will automatically retry on next poll.
+							} else {
+								await this.logout(session.id);
 							}
 						}
-					});
-
-					promises = promises.concat(this._tokens.map(async token => {
-						const matchesExisting = sessions.some(session => token.scope === session.scope && token.sessionId === session.id);
-						if (!matchesExisting) {
-							await this.logout(token.sessionId);
-							removedIds.push(token.sessionId);
-						}
-					}));
+					}
+				});
 
-					await Promise.all(promises);
-				} catch (e) {
-					Logger.error(e.message);
-					// if data is improperly formatted, remove all of it and send change event
-					removedIds = this._tokens.map(token => token.sessionId);
-					this.clearSessions();
-				}
-			} else {
-				if (this._tokens.length) {
-					// Log out all, remove all local data
-					removedIds = this._tokens.map(token => token.sessionId);
-					Logger.info('No stored keychain data, clearing local data');
+				promises = promises.concat(this._tokens.map(async token => {
+					const matchesExisting = sessions.some(session => token.scope === session.scope && token.sessionId === session.id);
+					if (!matchesExisting) {
+						await this.logout(token.sessionId);
+						removedIds.push(token.sessionId);
+					}
+				}));
 
-					this._tokens = [];
+				await Promise.all(promises);
+			} catch (e) {
+				Logger.error(e.message);
+				// if data is improperly formatted, remove all of it and send change event
+				removedIds = this._tokens.map(token => token.sessionId);
+				this.clearSessions();
+			}
+		} else {
+			if (this._tokens.length) {
+				// Log out all, remove all local data
+				removedIds = this._tokens.map(token => token.sessionId);
+				Logger.info('No stored keychain data, clearing local data');
 
-					this._refreshTimeouts.forEach(timeout => {
-						clearTimeout(timeout);
-					});
+				this._tokens = [];
 
-					this._refreshTimeouts.clear();
-				}
-			}
+				this._refreshTimeouts.forEach(timeout => {
+					clearTimeout(timeout);
+				});
 
-			if (addedIds.length || removedIds.length) {
-				onDidChangeSessions.fire({ added: addedIds, removed: removedIds, changed: [] });
+				this._refreshTimeouts.clear();
 			}
+		}
 
-			this.pollForChange();
-		}, 1000 * 30);
+		if (addedIds.length || removedIds.length) {
+			onDidChangeSessions.fire({ added: addedIds, removed: removedIds, changed: [] });
+		}
 	}
 
 	private async convertToSession(token: IToken): Promise<vscode.AuthenticationSession> {
@@ -344,6 +341,11 @@ export class AzureActiveDirectoryService {
 		});
 	}
 
+	public dispose(): void {
+		this._disposables.forEach(disposable => disposable.dispose());
+		this._disposables = [];
+	}
+
 	private getCallbackEnvironment(callbackUri: vscode.Uri): string {
 		if (callbackUri.authority.endsWith('.workspaces.github.com') || callbackUri.authority.endsWith('.github.dev')) {
 			return `${callbackUri.authority},`;

extensions/microsoft-authentication/src/extension.ts

@@ -14,6 +14,7 @@ export async function activate(context: vscode.ExtensionContext) {
 	const telemetryReporter = new TelemetryReporter(name, version, aiKey);
 
 	const loginService = new AzureActiveDirectoryService();
+	context.subscriptions.push(loginService);
 
 	await loginService.initialize();
 

src/vs/platform/native/common/native.ts

@@ -149,4 +149,5 @@ export interface ICommonNativeHostService {
 	deletePassword(service: string, account: string): Promise<boolean>;
 	findPassword(service: string): Promise<string | null>;
 	findCredentials(service: string): Promise<Array<{ account: string, password: string }>>
+	readonly onDidChangePassword: Event<void>;
 }

src/vs/platform/native/electron-main/nativeHostMainService.ts

@@ -83,6 +83,9 @@ export class NativeHostMainService implements INativeHostMainService {
 	private readonly _onColorSchemeChange = new Emitter<IColorScheme>();
 	readonly onColorSchemeChange = this._onColorSchemeChange.event;
 
+	private readonly _onDidChangePassword = new Emitter<void>();
+	readonly onDidChangePassword = this._onDidChangePassword.event;
+
 	//#endregion
 
 	//#region Window
@@ -632,14 +635,18 @@ export class NativeHostMainService implements INativeHostMainService {
 
 	async setPassword(windowId: number | undefined, service: string, account: string, password: string): Promise<void> {
 		const keytar = await import('keytar');
-
-		return keytar.setPassword(service, account, password);
+		await keytar.setPassword(service, account, password);
+		this._onDidChangePassword.fire();
 	}
 
 	async deletePassword(windowId: number | undefined, service: string, account: string): Promise<boolean> {
 		const keytar = await import('keytar');
+		const didDelete = await keytar.deletePassword(service, account);
+		if (didDelete) {
 
-		return keytar.deletePassword(service, account);
+			this._onDidChangePassword.fire();
+		}
+		return didDelete;
 	}
 
 	async findPassword(windowId: number | undefined, service: string): Promise<string | null> {

src/vs/vscode.proposed.d.ts

@@ -143,6 +143,11 @@ declare module 'vscode' {
 		* provider
 		*/
 		export function logout(providerId: string, sessionId: string): Thenable<void>;
+
+		/**
+		 * Fires when a password is set or deleted.
+		 */
+		export const onDidChangePassword: Event<void>;
 	}
 
 	//#endregion

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

@@ -19,6 +19,7 @@ import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteA
 import { fromNow } from 'vs/base/common/date';
 import { ActivationKind, IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
 import { isWeb } from 'vs/base/common/platform';
+import { ICredentialsService } from 'vs/workbench/services/credentials/common/credentials';
 
 const VSO_ALLOWED_EXTENSIONS = ['github.vscode-pull-request-github', 'github.vscode-pull-request-github-insiders', 'vscode.git', 'ms-vsonline.vsonline', 'vscode.github-browser', 'ms-vscode.github-browser'];
 
@@ -220,7 +221,8 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu
 		@IStorageKeysSyncRegistryService private readonly storageKeysSyncRegistryService: IStorageKeysSyncRegistryService,
 		@IRemoteAgentService private readonly remoteAgentService: IRemoteAgentService,
 		@IQuickInputService private readonly quickInputService: IQuickInputService,
-		@IExtensionService private readonly extensionService: IExtensionService
+		@IExtensionService private readonly extensionService: IExtensionService,
+		@ICredentialsService private readonly credentialsService: ICredentialsService
 	) {
 		super();
 		this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostAuthentication);
@@ -242,6 +244,10 @@ export class MainThreadAuthentication extends Disposable implements MainThreadAu
 		this._register(this.authenticationService.onDidChangeDeclaredProviders(e => {
 			this._proxy.$setProviders(e);
 		}));
+
+		this._register(this.credentialsService.onDidChangePassword(_ => {
+			this._proxy.$onDidChangePassword();
+		}));
 	}
 
 	$getProviderIds(): Promise<string[]> {

src/vs/workbench/api/common/extHost.api.impl.ts

@@ -225,6 +225,9 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
 			get onDidChangeSessions(): Event<vscode.AuthenticationSessionsChangeEvent> {
 				return extHostAuthentication.onDidChangeSessions;
 			},
+			get onDidChangePassword(): Event<void> {
+				return extHostAuthentication.onDidChangePassword;
+			}
 		};
 
 		// namespace: commands

src/vs/workbench/api/common/extHost.protocol.ts

@@ -1077,6 +1077,7 @@ export interface ExtHostAuthenticationShape {
 	$onDidChangeAuthenticationSessions(id: string, label: string, event: modes.AuthenticationSessionsChangeEvent): Promise<void>;
 	$onDidChangeAuthenticationProviders(added: modes.AuthenticationProviderInformation[], removed: modes.AuthenticationProviderInformation[]): Promise<void>;
 	$setProviders(providers: modes.AuthenticationProviderInformation[]): Promise<void>;
+	$onDidChangePassword(): Promise<void>;
 }
 
 export interface ExtHostSearchShape {

src/vs/workbench/api/common/extHostAuthentication.ts

@@ -24,6 +24,9 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape {
 	private _onDidChangeSessions = new Emitter<vscode.AuthenticationSessionsChangeEvent>();
 	readonly onDidChangeSessions: Event<vscode.AuthenticationSessionsChangeEvent> = this._onDidChangeSessions.event;
 
+	private _onDidChangePassword = new Emitter<void>();
+	readonly onDidChangePassword: Event<void> = this._onDidChangePassword.event;
+
 	constructor(mainContext: IMainContext) {
 		this._proxy = mainContext.getProxy(MainContext.MainThreadAuthentication);
 	}
@@ -203,4 +206,8 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape {
 		this._onDidChangeAuthenticationProviders.fire({ added, removed });
 		return Promise.resolve();
 	}
+
+	async $onDidChangePassword(): Promise<void> {
+		this._onDidChangePassword.fire();
+	}
 }

src/vs/workbench/services/credentials/browser/credentialsService.ts

@@ -6,11 +6,15 @@
 import { ICredentialsService, ICredentialsProvider } from 'vs/workbench/services/credentials/common/credentials';
 import { registerSingleton } from 'vs/platform/instantiation/common/extensions';
 import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
+import { Emitter } from 'vs/base/common/event';
 
 export class BrowserCredentialsService implements ICredentialsService {
 
 	declare readonly _serviceBrand: undefined;
 
+	private _onDidChangePassword: Emitter<void> = new Emitter();
+	readonly onDidChangePassword = this._onDidChangePassword.event;
+
 	private credentialsProvider: ICredentialsProvider;
 
 	constructor(@IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService) {
@@ -25,12 +29,18 @@ export class BrowserCredentialsService implements ICredentialsService {
 		return this.credentialsProvider.getPassword(service, account);
 	}
 
-	setPassword(service: string, account: string, password: string): Promise<void> {
-		return this.credentialsProvider.setPassword(service, account, password);
+	async setPassword(service: string, account: string, password: string): Promise<void> {
+		await this.credentialsProvider.setPassword(service, account, password);
+		this._onDidChangePassword.fire();
 	}
 
 	deletePassword(service: string, account: string): Promise<boolean> {
-		return this.credentialsProvider.deletePassword(service, account);
+		const didDelete = this.credentialsProvider.deletePassword(service, account);
+		if (didDelete) {
+			this._onDidChangePassword.fire();
+		}
+
+		return didDelete;
 	}
 
 	findPassword(service: string): Promise<string | null> {

src/vs/workbench/services/credentials/common/credentials.ts

@@ -3,6 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { Event } from 'vs/base/common/event';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
 
 export const ICredentialsService = createDecorator<ICredentialsService>('credentialsService');
@@ -17,4 +18,5 @@ export interface ICredentialsProvider {
 
 export interface ICredentialsService extends ICredentialsProvider {
 	readonly _serviceBrand: undefined;
+	readonly onDidChangePassword: Event<void>;
 }

src/vs/workbench/services/credentials/electron-sandbox/credentialsService.ts

@@ -6,12 +6,18 @@
 import { ICredentialsService } from 'vs/workbench/services/credentials/common/credentials';
 import { INativeHostService } from 'vs/platform/native/electron-sandbox/native';
 import { registerSingleton } from 'vs/platform/instantiation/common/extensions';
+import { Emitter } from 'vs/base/common/event';
 
 export class KeytarCredentialsService implements ICredentialsService {
 
 	declare readonly _serviceBrand: undefined;
 
-	constructor(@INativeHostService private readonly nativeHostService: INativeHostService) { }
+	private _onDidChangePassword: Emitter<void> = new Emitter();
+	onDidChangePassword = this._onDidChangePassword.event;
+
+	constructor(@INativeHostService private readonly nativeHostService: INativeHostService) {
+		this.nativeHostService.onDidChangePassword(event => this._onDidChangePassword.fire(event));
+	}
 
 	getPassword(service: string, account: string): Promise<string | null> {
 		return this.nativeHostService.getPassword(service, account);

src/vs/workbench/test/electron-browser/workbenchTestServices.ts

@@ -167,6 +167,7 @@ export class TestNativeHostService implements INativeHostService {
 	onWindowBlur: Event<number> = Event.None;
 	onOSResume: Event<unknown> = Event.None;
 	onColorSchemeChange = Event.None;
+	onDidChangePassword = Event.None;
 
 	windowCount = Promise.resolve(1);
 	getWindowCount(): Promise<number> { return this.windowCount; }