diff --git a/extensions/microsoft-authentication/src/AADHelper.ts b/extensions/microsoft-authentication/src/AADHelper.ts index d7e92e104fb..9a64fc59c38 100644 --- a/extensions/microsoft-authentication/src/AADHelper.ts +++ b/extensions/microsoft-authentication/src/AADHelper.ts @@ -5,7 +5,7 @@ import * as vscode from 'vscode'; import * as path from 'path'; -import { isSupportedEnvironment } from './utils'; +import { IntervalTimer, isSupportedEnvironment } from './utils'; import { generateCodeChallenge, generateCodeVerifier, randomUUID } from './cryptoUtils'; import { BetterTokenStorage, IDidChangeInOtherWindowEvent } from './betterSecretStorage'; import { LoopbackAuthServer } from './node/authServer'; @@ -84,7 +84,7 @@ export const REFRESH_NETWORK_FAILURE = 'Network failure'; export class AzureActiveDirectoryService { // For details on why this is set to 2/3... see https://github.com/microsoft/vscode/issues/133201#issuecomment-966668197 - private static REFRESH_TIMEOUT_MODIFIER = 1000 * 2 / 3; + private static REFRESH_TIMEOUT_MODIFIER = 1000 * 2 / 384; private static POLLING_CONSTANT = 1000 * 60 * 30; private _tokens: IToken[] = []; private _refreshTimeouts: Map = new Map(); @@ -96,6 +96,9 @@ export class AzureActiveDirectoryService { private _codeExchangePromises = new Map>(); private _codeVerfifiers = new Map(); + // Used to keep track of tokens that we need to store but can't because we aren't the focused window. + private _pendingTokensToStore: Map = new Map(); + constructor( private readonly _logger: vscode.LogOutputChannel, _context: vscode.ExtensionContext, @@ -105,6 +108,15 @@ export class AzureActiveDirectoryService { private readonly _env: Environment ) { _context.subscriptions.push(this._tokenStorage.onDidChangeInOtherWindow((e) => this.checkForUpdates(e))); + _context.subscriptions.push(vscode.window.onDidChangeWindowState(async (e) => e.focused && await this.storePendingTokens())); + + // In the event that a window isn't focused for a long time, we should still try to store the tokens at some point. + const timer = new IntervalTimer(); + timer.cancelAndSet( + () => !vscode.window.state.focused && this.storePendingTokens(), + // 5 hours + random extra 0-30 seconds so that each window doesn't try to store at the same time + (18000000) + Math.floor(Math.random() * 30000)); + _context.subscriptions.push(timer); } public async initialize(): Promise { @@ -113,7 +125,7 @@ export class AzureActiveDirectoryService { this._logger.info(`Got ${sessions.length} stored sessions`); const refreshes = sessions.map(async session => { - this._logger.trace(`Read the following stored session with scopes: ${session.scope}`); + this._logger.trace(`Read the following stored session '${session.id}' with scopes: ${session.scope}`); const scopes = session.scope.split(' '); const scopeData: IScopeData = { scopes, @@ -268,7 +280,10 @@ export class AzureActiveDirectoryService { } this._logger.info(`Got ${matchingTokens.length} sessions for scopes: ${modifiedScopesStr}`); - return Promise.all(matchingTokens.map(token => this.convertToSession(token, scopeData))); + const results = await Promise.allSettled(matchingTokens.map(token => this.convertToSession(token, scopeData))); + return results + .filter(result => result.status === 'fulfilled') + .map(result => (result as PromiseFulfilledResult).value); } public async createSession(scopes: string[]): Promise { @@ -554,7 +569,7 @@ export class AzureActiveDirectoryService { if (token.accessToken && (!token.expiresAt || token.expiresAt > Date.now())) { token.expiresAt ? this._logger.info(`Token available from cache (for scopes ${token.scope}), expires in ${token.expiresAt - Date.now()} milliseconds`) - : this._logger.info('Token available from cache (for scopes ${token.scope})'); + : this._logger.info(`Token available from cache (for scopes ${token.scope})`); return { id: token.sessionId, accessToken: token.accessToken, @@ -599,7 +614,7 @@ export class AzureActiveDirectoryService { } private async doRefreshToken(refreshToken: string, scopeData: IScopeData, sessionId?: string): Promise { - this._logger.info(`Refreshing token for scopes: ${scopeData.scopeStr}`); + this._logger.info(`Refreshing token '${sessionId ?? 'new'}' for scopes: ${scopeData.scopeStr}`); const postData = new URLSearchParams({ refresh_token: refreshToken, client_id: scopeData.clientId, @@ -813,7 +828,7 @@ export class AzureActiveDirectoryService { //#region storage operations private setToken(token: IToken, scopeData: IScopeData): void { - this._logger.info(`Setting token for scopes: ${scopeData.scopeStr}`); + this._logger.info(`Setting token '${token.sessionId}' for scopes: ${scopeData.scopeStr}`); const existingTokenIndex = this._tokens.findIndex(t => t.sessionId === token.sessionId); if (existingTokenIndex > -1) { @@ -823,41 +838,18 @@ export class AzureActiveDirectoryService { } // Don't await because setting the token is only useful for any new windows that open. - this.storeToken(token, scopeData); + void this.storeToken(token, scopeData); } private async storeToken(token: IToken, scopeData: IScopeData): Promise { if (!vscode.window.state.focused) { - const shouldStore = await new Promise((resolve, _) => { - // To handle the case where the window is not focused for a long time. We want to store the token - // at some point so that the next time they _do_ interact with VS Code, they don't have to sign in again. - const timer = setTimeout( - () => resolve(true), - // 5 hours + random extra 0-30 seconds so that each window doesn't try to store at the same time - (18000000) + Math.floor(Math.random() * 30000) - ); - const dispose = vscode.Disposable.from( - vscode.window.onDidChangeWindowState(e => { - if (e.focused) { - resolve(true); - dispose.dispose(); - clearTimeout(timer); - } - }), - this._tokenStorage.onDidChangeInOtherWindow(e => { - if (e.updated.includes(token.sessionId)) { - resolve(false); - dispose.dispose(); - clearTimeout(timer); - } - }) - ); - }); - - if (!shouldStore) { - this._logger.info(`Not storing token for scopes ${scopeData.scopeStr} because it was added in another window`); - return; + if (this._pendingTokensToStore.has(token.sessionId)) { + this._logger.info(`Window is not focused, replacing token to be stored with id '${token.sessionId}' for scopes: ${scopeData.scopeStr}`); + } else { + this._logger.info(`Window is not focused, pending storage of token '${token.sessionId}' for scopes: ${scopeData.scopeStr}`); } + this._pendingTokensToStore.set(token.sessionId, token); + return; } await this._tokenStorage.store(token.sessionId, { @@ -867,7 +859,31 @@ export class AzureActiveDirectoryService { account: token.account, endpoint: this._env.activeDirectoryEndpointUrl, }); - this._logger.info(`Stored token for scopes: ${scopeData.scopeStr}`); + this._logger.info(`Stored token '${token.sessionId}' for scopes: ${scopeData.scopeStr}`); + } + + private async storePendingTokens(): Promise { + if (this._pendingTokensToStore.size === 0) { + this._logger.info('No pending tokens to store'); + return; + } + + const tokens = [...this._pendingTokensToStore.values()]; + this._pendingTokensToStore.clear(); + + this._logger.info(`Storing ${tokens.length} pending tokens...`); + await Promise.allSettled(tokens.map(async token => { + this._logger.info(`Storing pending token '${token.sessionId}' for scopes: ${token.scope}`); + await this._tokenStorage.store(token.sessionId, { + id: token.sessionId, + refreshToken: token.refreshToken, + scope: token.scope, + account: token.account, + endpoint: this._env.activeDirectoryEndpointUrl, + }); + this._logger.info(`Stored pending token '${token.sessionId}' for scopes: ${token.scope}`); + })); + this._logger.info('Done storing pending tokens'); } private async checkForUpdates(e: IDidChangeInOtherWindowEvent): Promise { @@ -925,6 +941,13 @@ export class AzureActiveDirectoryService { // because access tokens are not stored in Secret Storage due to their short lifespan. This new refresh token // is not useful in this window because we really only care about the lifetime of the _access_ token which we // are already managing (see usages of `setSessionTimeout`). + // However, in order to minimize the amount of times we store tokens, if a token was stored via another window, + // we cancel any pending token storage operations. + for (const sessionId of e.updated) { + if (this._pendingTokensToStore.delete(sessionId)) { + this._logger.info(`Cancelled pending token storage for session '${sessionId}'`); + } + } } private sessionMatchesEndpoint(session: IStoredSession): boolean { diff --git a/extensions/microsoft-authentication/src/utils.ts b/extensions/microsoft-authentication/src/utils.ts index 7382cc2f4f2..c97092493fc 100644 --- a/extensions/microsoft-authentication/src/utils.ts +++ b/extensions/microsoft-authentication/src/utils.ts @@ -2,7 +2,7 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { env, UIKind, Uri } from 'vscode'; +import { Disposable, env, UIKind, Uri } from 'vscode'; const LOCALHOST_ADDRESSES = ['localhost', '127.0.0.1', '0:0:0:0:0:0:0:1', '::1']; function isLocalhost(uri: Uri): boolean { @@ -35,3 +35,27 @@ export function isSupportedEnvironment(uri: Uri): boolean { /(?:^|\.)github\.localhost$/.test(uri.authority) ); } + +export class IntervalTimer extends Disposable { + + private _token: any; + + constructor() { + super(() => this.cancel()); + this._token = -1; + } + + cancel(): void { + if (this._token !== -1) { + clearInterval(this._token); + this._token = -1; + } + } + + cancelAndSet(runner: () => void, interval: number): void { + this.cancel(); + this._token = setInterval(() => { + runner(); + }, interval); + } +}