From 691eaea3bdc320ca911a8dd32fb23c25717461bb Mon Sep 17 00:00:00 2001 From: Tyler James Leonhardt Date: Wed, 8 Jan 2025 10:11:01 -0800 Subject: [PATCH] Remove old code, simplify properties (#237512) --- .../src/common/async.ts | 137 +----------------- .../src/node/cachedPublicClientApplication.ts | 79 +++------- 2 files changed, 25 insertions(+), 191 deletions(-) diff --git a/extensions/microsoft-authentication/src/common/async.ts b/extensions/microsoft-authentication/src/common/async.ts index 9eebbb24f65..3555bb09502 100644 --- a/extensions/microsoft-authentication/src/common/async.ts +++ b/extensions/microsoft-authentication/src/common/async.ts @@ -5,11 +5,6 @@ import { CancellationError, CancellationToken, Disposable, Event } from 'vscode'; -/** - * Can be passed into the Delayed to defer using a microtask - */ -export const MicrotaskDelay = Symbol('MicrotaskDelay'); - export class SequencerByKey { private promiseMap = new Map>(); @@ -57,7 +52,7 @@ export class IntervalTimer extends Disposable { * Returns a promise that rejects with an {@CancellationError} as soon as the passed token is cancelled. * @see {@link raceCancellation} */ -export function raceCancellationError(promise: Promise, token: CancellationToken): Promise { +function raceCancellationError(promise: Promise, token: CancellationToken): Promise { return new Promise((resolve, reject) => { const ref = token.onCancellationRequested(() => { ref.dispose(); @@ -67,7 +62,7 @@ export function raceCancellationError(promise: Promise, token: Cancellatio }); } -export function raceTimeoutError(promise: Promise, timeout: number): Promise { +function raceTimeoutError(promise: Promise, timeout: number): Promise { return new Promise((resolve, reject) => { const ref = setTimeout(() => { reject(new CancellationError()); @@ -80,138 +75,12 @@ export function raceCancellationAndTimeoutError(promise: Promise, token: C return raceCancellationError(raceTimeoutError(promise, timeout), token); } -interface IScheduledLater extends Disposable { - isTriggered(): boolean; -} - -const timeoutDeferred = (timeout: number, fn: () => void): IScheduledLater => { - let scheduled = true; - const handle = setTimeout(() => { - scheduled = false; - fn(); - }, timeout); - return { - isTriggered: () => scheduled, - dispose: () => { - clearTimeout(handle); - scheduled = false; - }, - }; -}; - -const microtaskDeferred = (fn: () => void): IScheduledLater => { - let scheduled = true; - queueMicrotask(() => { - if (scheduled) { - scheduled = false; - fn(); - } - }); - - return { - isTriggered: () => scheduled, - dispose: () => { scheduled = false; }, - }; -}; - -/** - * A helper to delay (debounce) execution of a task that is being requested often. - * - * Following the throttler, now imagine the mail man wants to optimize the number of - * trips proactively. The trip itself can be long, so he decides not to make the trip - * as soon as a letter is submitted. Instead he waits a while, in case more - * letters are submitted. After said waiting period, if no letters were submitted, he - * decides to make the trip. Imagine that N more letters were submitted after the first - * one, all within a short period of time between each other. Even though N+1 - * submissions occurred, only 1 delivery was made. - * - * The delayer offers this behavior via the trigger() method, into which both the task - * to be executed and the waiting period (delay) must be passed in as arguments. Following - * the example: - * - * const delayer = new Delayer(WAITING_PERIOD); - * const letters = []; - * - * function letterReceived(l) { - * letters.push(l); - * delayer.trigger(() => { return makeTheTrip(); }); - * } - */ -export class Delayer implements Disposable { - - private deferred: IScheduledLater | null; - private completionPromise: Promise | null; - private doResolve: ((value?: any | Promise) => void) | null; - private doReject: ((err: any) => void) | null; - private task: (() => T | Promise) | null; - - constructor(public defaultDelay: number | typeof MicrotaskDelay) { - this.deferred = null; - this.completionPromise = null; - this.doResolve = null; - this.doReject = null; - this.task = null; - } - - trigger(task: () => T | Promise, delay = this.defaultDelay): Promise { - this.task = task; - this.cancelTimeout(); - - if (!this.completionPromise) { - this.completionPromise = new Promise((resolve, reject) => { - this.doResolve = resolve; - this.doReject = reject; - }).then(() => { - this.completionPromise = null; - this.doResolve = null; - if (this.task) { - const task = this.task; - this.task = null; - return task(); - } - return undefined; - }); - } - - const fn = () => { - this.deferred = null; - this.doResolve?.(null); - }; - - this.deferred = delay === MicrotaskDelay ? microtaskDeferred(fn) : timeoutDeferred(delay, fn); - - return this.completionPromise; - } - - isTriggered(): boolean { - return !!this.deferred?.isTriggered(); - } - - cancel(): void { - this.cancelTimeout(); - - if (this.completionPromise) { - this.doReject?.(new CancellationError()); - this.completionPromise = null; - } - } - - private cancelTimeout(): void { - this.deferred?.dispose(); - this.deferred = null; - } - - dispose(): void { - this.cancel(); - } -} - /** * Given an event, returns another event which only fires once. * * @param event The event source for the new event. */ -export function once(event: Event): Event { +function once(event: Event): Event { return (listener, thisArgs = null, disposables?) => { // we need this, in case the event fires during the listener call let didFire = false; diff --git a/extensions/microsoft-authentication/src/node/cachedPublicClientApplication.ts b/extensions/microsoft-authentication/src/node/cachedPublicClientApplication.ts index 616b3e1120d..7ebf7a1630c 100644 --- a/extensions/microsoft-authentication/src/node/cachedPublicClientApplication.ts +++ b/extensions/microsoft-authentication/src/node/cachedPublicClientApplication.ts @@ -6,44 +6,29 @@ import { PublicClientApplication, AccountInfo, Configuration, SilentFlowRequest, AuthenticationResult, InteractiveRequest, LogLevel, RefreshTokenRequest } from '@azure/msal-node'; import { NativeBrokerPlugin } from '@azure/msal-node-extensions'; import { Disposable, Memento, SecretStorage, LogOutputChannel, window, ProgressLocation, l10n, EventEmitter } from 'vscode'; -import { Delayer, raceCancellationAndTimeoutError } from '../common/async'; +import { raceCancellationAndTimeoutError } from '../common/async'; import { SecretStorageCachePlugin } from '../common/cachePlugin'; import { MsalLoggerOptions } from '../common/loggerOptions'; import { ICachedPublicClientApplication } from '../common/publicClientCache'; import { ScopedAccountAccess } from '../common/accountAccess'; export class CachedPublicClientApplication implements ICachedPublicClientApplication { + // Core properties private _pca: PublicClientApplication; - private _sequencer = new Sequencer(); - // private readonly _refreshDelayer = new DelayerByKey(); - private _accounts: AccountInfo[] = []; + private _sequencer = new Sequencer(); private readonly _disposable: Disposable; - private readonly _loggerOptions = new MsalLoggerOptions(this._logger); + // Cache properties private readonly _secretStorageCachePlugin = new SecretStorageCachePlugin( this._secretStorage, // Include the prefix as a differentiator to other secrets `pca:${JSON.stringify({ clientId: this._clientId, authority: this._authority })}` ); + + // Broker properties private readonly _accountAccess = new ScopedAccountAccess(this._secretStorage, this._cloudName, this._clientId, this._authority); - private readonly _config: Configuration = { - auth: { clientId: this._clientId, authority: this._authority }, - system: { - loggerOptions: { - correlationId: `${this._clientId}] [${this._authority}`, - loggerCallback: (level, message, containsPii) => this._loggerOptions.loggerCallback(level, message, containsPii), - logLevel: LogLevel.Trace - } - }, - broker: { - nativeBrokerPlugin: new NativeBrokerPlugin() - }, - cache: { - cachePlugin: this._secretStorageCachePlugin - } - }; - private readonly _isBrokerAvailable = this._config.broker?.nativeBrokerPlugin?.isBrokerAvailable ?? false; + private readonly _isBrokerAvailable: boolean; //#region Events @@ -65,7 +50,21 @@ export class CachedPublicClientApplication implements ICachedPublicClientApplica ) { // TODO:@TylerLeonhardt clean up old use of memento. Remove this in an iteration this._globalMemento.update(`lastRemoval:${this._clientId}:${this._authority}`, undefined); - this._pca = new PublicClientApplication(this._config); + const loggerOptions = new MsalLoggerOptions(_logger); + const nativeBrokerPlugin = new NativeBrokerPlugin(); + this._isBrokerAvailable = nativeBrokerPlugin.isBrokerAvailable ?? false; + this._pca = new PublicClientApplication({ + auth: { clientId: _clientId, authority: _authority }, + system: { + loggerOptions: { + correlationId: `${_clientId}] [${_authority}`, + loggerCallback: (level, message, containsPii) => loggerOptions.loggerCallback(level, message, containsPii), + logLevel: LogLevel.Trace + } + }, + broker: { nativeBrokerPlugin }, + cache: { cachePlugin: this._secretStorageCachePlugin } + }); this._disposable = Disposable.from( this._registerOnSecretStorageChanged(), this._onDidAccountsChangeEmitter, @@ -117,7 +116,6 @@ export class CachedPublicClientApplication implements ICachedPublicClientApplica } } - // this._setupRefresh(result); if (result.account && !result.fromCache && this._verifyIfUsingBroker(result)) { this._logger.debug(`[acquireTokenSilent] [${this._clientId}] [${this._authority}] [${request.scopes.join(' ')}] [${request.account.username}] firing event due to change`); this._onDidAccountsChangeEmitter.fire({ added: [], changed: [result.account], deleted: [] }); @@ -139,7 +137,6 @@ export class CachedPublicClientApplication implements ICachedPublicClientApplica 1000 * 60 * 5 )) ); - // this._setupRefresh(result); if (this._isBrokerAvailable) { await this._accountAccess.setAllowedAccess(result.account!, true); } @@ -226,24 +223,6 @@ export class CachedPublicClientApplication implements ICachedPublicClientApplica } this._logger.debug(`[update] [${this._clientId}] [${this._authority}] CachedPublicClientApplication update complete`); } - - // private _setupRefresh(result: AuthenticationResult) { - // const on = result.refreshOn || result.expiresOn; - // if (!result.account || !on) { - // return; - // } - - // const account = result.account; - // const scopes = result.scopes; - // const timeToRefresh = on.getTime() - Date.now() - 5 * 60 * 1000; // 5 minutes before expiry - // const key = JSON.stringify({ accountId: account.homeAccountId, scopes }); - // this._logger.debug(`[_setupRefresh] [${this._clientId}] [${this._authority}] [${scopes.join(' ')}] [${account.username}] timeToRefresh: ${timeToRefresh}`); - // this._refreshDelayer.trigger( - // key, - // () => this.acquireTokenSilent({ account, scopes, redirectUri: 'https://vscode.dev/redirect', forceRefresh: true }), - // timeToRefresh > 0 ? timeToRefresh : 0 - // ); - // } } export class Sequencer { @@ -254,17 +233,3 @@ export class Sequencer { return this.current = this.current.then(() => promiseTask(), () => promiseTask()); } } - -// class DelayerByKey { -// private _delayers = new Map>(); - -// trigger(key: string, fn: () => Promise, delay: number): Promise { -// let delayer = this._delayers.get(key); -// if (!delayer) { -// delayer = new Delayer(delay); -// this._delayers.set(key, delayer); -// } - -// return delayer.trigger(fn, delay); -// } -// }