diff --git a/extensions/github-authentication/src/common/keychain.ts b/extensions/github-authentication/src/common/keychain.ts index 96afff43b09..9753650508e 100644 --- a/extensions/github-authentication/src/common/keychain.ts +++ b/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(); diff --git a/extensions/github-authentication/src/github.ts b/extensions/github-authentication/src/github.ts index b14a1507669..7b9b745f211 100644 --- a/extensions/github-authentication/src/github.ts +++ b/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; + private async checkForUpdates() { + let storedSessions: vscode.AuthenticationSession[]; + try { + storedSessions = await this.readSessions(); + } catch (e) { + // Ignore, network request failed + return; + } + + const added: string[] = []; + const removed: string[] = []; + + 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); } + }); - const added: string[] = []; - const removed: string[] = []; - - 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); } - }); - 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); - } - - removed.push(session.id); - } - }); - - 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 { diff --git a/extensions/microsoft-authentication/src/AADHelper.ts b/extensions/microsoft-authentication/src/AADHelper.ts index b30937f6dd2..7d7dbacf126 100644 --- a/extensions/microsoft-authentication/src/AADHelper.ts +++ b/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 { + 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); - } - })); + 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'); - - this._tokens = []; - - this._refreshTimeouts.forEach(timeout => { - clearTimeout(timeout); - }); - - this._refreshTimeouts.clear(); - } + 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'); - if (addedIds.length || removedIds.length) { - onDidChangeSessions.fire({ added: addedIds, removed: removedIds, changed: [] }); + this._tokens = []; + + this._refreshTimeouts.forEach(timeout => { + clearTimeout(timeout); + }); + + 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 { diff --git a/extensions/microsoft-authentication/src/keychain.ts b/extensions/microsoft-authentication/src/keychain.ts index f0e487760eb..6b13eded95b 100644 --- a/extensions/microsoft-authentication/src/keychain.ts +++ b/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(); diff --git a/src/vs/platform/native/common/native.ts b/src/vs/platform/native/common/native.ts index 9f9507e191f..1b426f046e7 100644 --- a/src/vs/platform/native/common/native.ts +++ b/src/vs/platform/native/common/native.ts @@ -149,4 +149,5 @@ export interface ICommonNativeHostService { deletePassword(service: string, account: string): Promise; findPassword(service: string): Promise; findCredentials(service: string): Promise> + readonly onDidChangePassword: Event; } diff --git a/src/vs/platform/native/electron-main/nativeHostMainService.ts b/src/vs/platform/native/electron-main/nativeHostMainService.ts index 92421779ea5..a1f4042203c 100644 --- a/src/vs/platform/native/electron-main/nativeHostMainService.ts +++ b/src/vs/platform/native/electron-main/nativeHostMainService.ts @@ -83,6 +83,9 @@ export class NativeHostMainService implements INativeHostMainService { private readonly _onColorSchemeChange = new Emitter(); readonly onColorSchemeChange = this._onColorSchemeChange.event; + private readonly _onDidChangePassword = new Emitter(); + 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 { 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 { 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 { diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 2a4a12c8897..a3ad64e0d9d 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/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; + + /** + * Fires when a password is set or deleted. + */ + export const onDidChangePassword: Event; } //#endregion diff --git a/src/vs/workbench/api/browser/mainThreadAuthentication.ts b/src/vs/workbench/api/browser/mainThreadAuthentication.ts index 3abbcc9bbfd..02240e28b7e 100644 --- a/src/vs/workbench/api/browser/mainThreadAuthentication.ts +++ b/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 { diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index cf44f735dca..d4393a332bd 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -233,6 +233,9 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I }, deletePassword(key: string): Thenable { return extHostAuthentication.deletePassword(extension, key); + }, + get onDidChangePassword(): Event { + return extHostAuthentication.onDidChangePassword; } }; diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index c13066d7be0..b7ad89e366b 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -1082,6 +1082,7 @@ export interface ExtHostAuthenticationShape { $onDidChangeAuthenticationSessions(id: string, label: string, event: modes.AuthenticationSessionsChangeEvent): Promise; $onDidChangeAuthenticationProviders(added: modes.AuthenticationProviderInformation[], removed: modes.AuthenticationProviderInformation[]): Promise; $setProviders(providers: modes.AuthenticationProviderInformation[]): Promise; + $onDidChangePassword(): Promise; } export interface ExtHostSearchShape { diff --git a/src/vs/workbench/api/common/extHostAuthentication.ts b/src/vs/workbench/api/common/extHostAuthentication.ts index f9a32ebe9f4..d97c7d4b29f 100644 --- a/src/vs/workbench/api/common/extHostAuthentication.ts +++ b/src/vs/workbench/api/common/extHostAuthentication.ts @@ -24,6 +24,10 @@ export class ExtHostAuthentication implements ExtHostAuthenticationShape { private _onDidChangeSessions = new Emitter(); readonly onDidChangeSessions: Event = this._onDidChangeSessions.event; + + private _onDidChangePassword = new Emitter(); + readonly onDidChangePassword: Event = 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 { + this._onDidChangePassword.fire(); + return Promise.resolve(); + } + getPassword(requestingExtension: IExtensionDescription, key: string): Promise { const extensionId = ExtensionIdentifier.toKey(requestingExtension.identifier); return this._proxy.$getPassword(extensionId, key); diff --git a/src/vs/workbench/services/credentials/browser/credentialsService.ts b/src/vs/workbench/services/credentials/browser/credentialsService.ts index aff3526121e..98e2856f1b9 100644 --- a/src/vs/workbench/services/credentials/browser/credentialsService.ts +++ b/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 = 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 { - return this.credentialsProvider.setPassword(service, account, password); + async setPassword(service: string, account: string, password: string): Promise { + await this.credentialsProvider.setPassword(service, account, password); + this._onDidChangePassword.fire(); } deletePassword(service: string, account: string): Promise { - return this.credentialsProvider.deletePassword(service, account); + const didDelete = this.credentialsProvider.deletePassword(service, account); + if (didDelete) { + this._onDidChangePassword.fire(); + } + + return didDelete; } findPassword(service: string): Promise { diff --git a/src/vs/workbench/services/credentials/common/credentials.ts b/src/vs/workbench/services/credentials/common/credentials.ts index b6dd9404b41..f6a3a2de020 100644 --- a/src/vs/workbench/services/credentials/common/credentials.ts +++ b/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('credentialsService'); @@ -17,4 +18,5 @@ export interface ICredentialsProvider { export interface ICredentialsService extends ICredentialsProvider { readonly _serviceBrand: undefined; + onDidChangePassword: Event; } diff --git a/src/vs/workbench/services/credentials/electron-sandbox/credentialsService.ts b/src/vs/workbench/services/credentials/electron-sandbox/credentialsService.ts index a6e9361b42d..16414f2fc13 100644 --- a/src/vs/workbench/services/credentials/electron-sandbox/credentialsService.ts +++ b/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 = 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 { return this.nativeHostService.getPassword(service, account); diff --git a/src/vs/workbench/test/electron-browser/workbenchTestServices.ts b/src/vs/workbench/test/electron-browser/workbenchTestServices.ts index d3fa18f6d16..e74af7ce96d 100644 --- a/src/vs/workbench/test/electron-browser/workbenchTestServices.ts +++ b/src/vs/workbench/test/electron-browser/workbenchTestServices.ts @@ -167,6 +167,7 @@ export class TestNativeHostService implements INativeHostService { onWindowBlur: Event = Event.None; onOSResume: Event = Event.None; onColorSchemeChange = Event.None; + onDidChangePassword = Event.None; windowCount = Promise.resolve(1); getWindowCount(): Promise { return this.windowCount; }