Removing polling from auth extensions, fixes #107480

extensions/github-authentication/src/common/keychain.ts

@@ -84,6 +84,12 @@ export class Keychain {
 			return Promise.resolve(undefined);
 		}
 	}
+
+	onDidChangePassword(listener: () => void) {
+		vscode.authentication.onDidChangePassword(_ => {
+			listener();
+		});
+	}
 }
 
 export const keychain = new Keychain();

extensions/github-authentication/src/github.ts

@@ -33,52 +33,48 @@ export class GitHubAuthenticationProvider {
 			// Ignore, network request failed
 		}
 
-		this.pollForChange();
+		keychain.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

@@ -140,7 +140,7 @@ export class AzureActiveDirectoryService {
 			}
 		}
 
-		this.pollForChange();
+		keychain.onDidChangePassword(() => this.checkForUpdates);
 	}
 
 	private parseStoredData(data: string): IStoredSession[] {
@@ -160,67 +160,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> {

extensions/microsoft-authentication/src/keychain.ts

@@ -101,6 +101,12 @@ export class Keychain {
 			return Promise.resolve(null);
 		}
 	}
+
+	onDidChangePassword(listener: () => void) {
+		vscode.authentication.onDidChangePassword(_ => {
+			listener();
+		});
+	}
 }
 
 export const keychain = new Keychain();

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

@@ -163,6 +163,11 @@ declare module 'vscode' {
 		 * @param key The key the password was stored under.
 		 */
 		export function deletePassword(key: string): Thenable<void>;
+
+		/**
+		 * Fires when a password is set or deleted.
+		 */
+		export const onDidChangePassword: Event<void>;
 	}
 
 	//#endregion

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

@@ -248,6 +248,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

@@ -233,6 +233,9 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
 			},
 			deletePassword(key: string): Thenable<void> {
 				return extHostAuthentication.deletePassword(extension, key);
+			},
+			get onDidChangePassword(): Event<void> {
+				return extHostAuthentication.onDidChangePassword;
 			}
 		};
 

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

@@ -1082,6 +1082,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,10 @@ 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);
 	}
@@ -204,6 +208,11 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape {
 		return Promise.resolve();
 	}
 
+	$onDidChangePassword(): Promise<void> {
+		this._onDidChangePassword.fire();
+		return Promise.resolve();
+	}
+
 	getPassword(requestingExtension: IExtensionDescription, key: string): Promise<string | undefined> {
 		const extensionId = ExtensionIdentifier.toKey(requestingExtension.identifier);
 		return this._proxy.$getPassword(extensionId, key);

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();
+	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;
+	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; }