No need for memento hack (#234450)

MSAL node made `clearCache` synchronous 🎉 so we can safely depend on it for clearing the cache.

> Context: The default behavior of MSAL's internal cache is that it is a union with what's in the persistant cache (secret storage) but what _we_ want is that secret storage is the source of truth, so every time we receive an update to secret storage, we clear the in-memory cache to get the data from the persistant cache.

Also bumps msal-node-extensions while we're at it.
This commit is contained in:
Tyler James Leonhardt
2024-11-22 13:35:49 -08:00
committed by GitHub
parent 83857a6ee1
commit f6dd987698
3 changed files with 21 additions and 45 deletions

View File

@@ -45,14 +45,6 @@ export class CachedPublicClientApplication implements ICachedPublicClientApplica
};
private readonly _isBrokerAvailable = this._config.broker?.nativeBrokerPlugin?.isBrokerAvailable ?? false;
/**
* We keep track of the last time an account was removed so we can recreate the PCA if we detect that an account was removed.
* This is due to MSAL-node not providing a way to detect when an account is removed from the cache. An internal issue has been
* filed to track this. If MSAL-node ever provides a way to detect this or handle this better in the Persistant Cache Plugin,
* we can remove this logic.
*/
private _lastCreated: Date;
//#region Events
private readonly _onDidAccountsChangeEmitter = new EventEmitter<{ added: AccountInfo[]; changed: AccountInfo[]; deleted: AccountInfo[] }>;
@@ -71,8 +63,9 @@ export class CachedPublicClientApplication implements ICachedPublicClientApplica
private readonly _secretStorage: SecretStorage,
private readonly _logger: LogOutputChannel
) {
// 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);
this._lastCreated = new Date();
this._disposable = Disposable.from(
this._registerOnSecretStorageChanged(),
this._onDidAccountsChangeEmitter,
@@ -147,7 +140,6 @@ export class CachedPublicClientApplication implements ICachedPublicClientApplica
}
removeAccount(account: AccountInfo): Promise<void> {
this._globalMemento.update(`lastRemoval:${this._clientId}:${this._authority}`, new Date());
if (this._isBrokerAvailable) {
return this._accountAccess.setAllowedAccess(account, false);
}
@@ -185,14 +177,8 @@ export class CachedPublicClientApplication implements ICachedPublicClientApplica
private async _update() {
const before = this._accounts;
this._logger.debug(`[update] [${this._clientId}] [${this._authority}] CachedPublicClientApplication update before: ${before.length}`);
// Dates are stored as strings in the memento
const lastRemovalDate = this._globalMemento.get<string>(`lastRemoval:${this._clientId}:${this._authority}`);
if (lastRemovalDate && this._lastCreated && Date.parse(lastRemovalDate) > this._lastCreated.getTime()) {
this._logger.debug(`[update] [${this._clientId}] [${this._authority}] CachedPublicClientApplication removal detected... recreating PCA...`);
this._pca = new PublicClientApplication(this._config);
this._lastCreated = new Date();
}
// Clear in-memory cache so we know we're getting account data from the SecretStorage
this._pca.clearCache();
let after = await this._pca.getAllAccounts();
if (this._isBrokerAvailable) {
after = after.filter(a => this._accountAccess.isAllowedAccess(a));