From d3c996dc5c1931f6c18e4d42c4f71860b0c91770 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra <13396919+cheenamalhotra@users.noreply.github.com> Date: Thu, 1 Jun 2023 15:49:29 -0700 Subject: [PATCH] Delete both cache files on decryption failure (#23291) --- .../src/account-provider/auths/azureAuth.ts | 7 ++- .../account-provider/azureAccountProvider.ts | 4 +- .../account-provider/utils/msalCachePlugin.ts | 53 +++++++++++++------ 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/extensions/azurecore/src/account-provider/auths/azureAuth.ts b/extensions/azurecore/src/account-provider/auths/azureAuth.ts index 90d175b59f..2a75cd9406 100644 --- a/extensions/azurecore/src/account-provider/auths/azureAuth.ts +++ b/extensions/azurecore/src/account-provider/auths/azureAuth.ts @@ -870,10 +870,15 @@ export abstract class AzureAuth implements vscode.Disposable { protected toBase64UrlEncoding(base64string: string): string { return base64string.replace(/=/g, '').replace(/\+/g, '-').replace(/\//g, '_'); // Need to use base64url encoding } + public async deleteAllCacheMsal(): Promise { this.clientApplication.clearCache(); - await this.msalCacheProvider.clearLocalCache(); + + // unlink both cache files + await this.msalCacheProvider.unlinkMsalCache(); + await this.msalCacheProvider.unlinkLocalCache(); } + public async deleteAllCacheAdal(): Promise { const results = await this.tokenCache.findCredentials(''); diff --git a/extensions/azurecore/src/account-provider/azureAccountProvider.ts b/extensions/azurecore/src/account-provider/azureAccountProvider.ts index eda170212e..6787f20635 100644 --- a/extensions/azurecore/src/account-provider/azureAccountProvider.ts +++ b/extensions/azurecore/src/account-provider/azureAccountProvider.ts @@ -152,7 +152,9 @@ export class AzureAccountProvider implements azdata.AccountProvider, vscode.Disp try { // Fetch cached token from local cache if token is available and valid. let accessToken = await this.msalCacheProvider.getTokenFromLocalCache(account.key.accountId, tenantId, resource); - if (this.isValidToken(accessToken)) { + if (this.isValidToken(accessToken) && + // Ensure MSAL Cache contains user account + (await this.clientApplication.getAllAccounts()).find((accountInfo) => accountInfo.homeAccountId === account.key.accountId)) { return accessToken; } // else fallback to fetching a new token. } catch (e) { diff --git a/extensions/azurecore/src/account-provider/utils/msalCachePlugin.ts b/extensions/azurecore/src/account-provider/utils/msalCachePlugin.ts index 19a8765747..559bf0f31a 100644 --- a/extensions/azurecore/src/account-provider/utils/msalCachePlugin.ts +++ b/extensions/azurecore/src/account-provider/utils/msalCachePlugin.ts @@ -65,7 +65,7 @@ export class MsalCachePluginProvider { public getCachePlugin(): ICachePlugin { const beforeCacheAccess = async (cacheContext: TokenCacheContext): Promise => { try { - const decryptedData = await this.readCache(this._msalCacheConfiguration); + const decryptedData = await this.readCache(this._msalCacheConfiguration, this._localCacheConfiguration); cacheContext.tokenCache.deserialize(decryptedData); } catch (e) { // Handle deserialization error in cache file in case file gets corrupted. @@ -101,7 +101,7 @@ export class MsalCachePluginProvider { * @returns Access Token. */ public async getTokenFromLocalCache(accountId: string, tenantId: string, resource: azdata.AzureResource): Promise { - let cache = JSON.parse(await this.readCache(this._localCacheConfiguration)) as LocalAccountCache; + let cache = JSON.parse(await this.readCache(this._localCacheConfiguration, this._msalCacheConfiguration)) as LocalAccountCache; let token = cache?.tokens?.find(token => ( token.key === accountId && token.tenantId === tenantId && @@ -118,7 +118,7 @@ export class MsalCachePluginProvider { let updateCount = 0; let indexToUpdate = -1; let cache: LocalAccountCache; - cache = JSON.parse(await this.readCache(this._localCacheConfiguration)) as LocalAccountCache; + cache = JSON.parse(await this.readCache(this._localCacheConfiguration, this._msalCacheConfiguration)) as LocalAccountCache; if (cache?.tokens) { cache.tokens.forEach((t, i) => { if (t.key === token.key && t.tenantId === token.tenantId && t.resource === token.resource @@ -157,7 +157,7 @@ export class MsalCachePluginProvider { * @param accountId Account ID */ public async clearAccountFromLocalCache(accountId: string): Promise { - let cache = JSON.parse(await this.readCache(this._localCacheConfiguration)) as LocalAccountCache; + let cache = JSON.parse(await this.readCache(this._localCacheConfiguration, this._msalCacheConfiguration)) as LocalAccountCache; let tokenIndices: number[] = []; if (cache?.tokens) { cache.tokens.forEach((t, i) => { @@ -173,10 +173,17 @@ export class MsalCachePluginProvider { } /** - * Clears local access token cache. + * Deletes Msal access token cache file */ - public async clearLocalCache(): Promise { - await this.writeCache(JSON.stringify({ tokens: [] }), this._localCacheConfiguration); + public async unlinkMsalCache(): Promise { + await fsPromises.unlink(this._msalCacheConfiguration.cacheFilePath); + } + + /** + * Deletes local access token cache file. + */ + public async unlinkLocalCache(): Promise { + await fsPromises.unlink(this._localCacheConfiguration.cacheFilePath); } //#region Private helper methods @@ -194,26 +201,42 @@ export class MsalCachePluginProvider { } } - private async readCache(config: CacheConfiguration): Promise { - config.lockTaken = await this.waitAndLock(config.lockFilePath, config.lockTaken); + /** + * Reads from an encrypted cache file based on currentConfig provided. + * @param currentConfig Currently used cache configuration. + * @param alternateConfig Alternate cache configuration for resetting needs. + * @returns Decrypted data. + */ + private async readCache(currentConfig: CacheConfiguration, alternateConfig: CacheConfiguration): Promise { + currentConfig.lockTaken = await this.waitAndLock(currentConfig.lockFilePath, currentConfig.lockTaken); try { - const cache = await fsPromises.readFile(config.cacheFilePath, { encoding: 'utf8' }); + const cache = await fsPromises.readFile(currentConfig.cacheFilePath, { encoding: 'utf8' }); const decryptedData = await this._fileEncryptionHelper.fileOpener(cache!, true); return decryptedData; } catch (e) { if (e.code === 'ENOENT') { // File doesn't exist, log and continue - Logger.verbose(`MsalCachePlugin: Cache file for '${config.name}' cache not found on disk: ${e.code}`); + Logger.verbose(`MsalCachePlugin: Cache file for '${currentConfig.name}' cache not found on disk: ${e.code}`); } else { Logger.error(`MsalCachePlugin: Failed to read from cache file: ${e}`); - Logger.verbose(`MsalCachePlugin: Error occurred when trying to read cache file, file will be deleted: ${e.message}`); - await fsPromises.unlink(config.cacheFilePath); + Logger.verbose(`MsalCachePlugin: Error occurred when trying to read cache file ${currentConfig.name}, file will be deleted: ${e.message}`); + await fsPromises.unlink(currentConfig.cacheFilePath); + + // Ensure both configurations are not same. + if (currentConfig.name !== alternateConfig.name) { + // Delete alternate cache file as well. + alternateConfig.lockTaken = await this.waitAndLock(alternateConfig.lockFilePath, alternateConfig.lockTaken); + await fsPromises.unlink(alternateConfig.cacheFilePath); + lockFile.unlockSync(alternateConfig.lockFilePath); + alternateConfig.lockTaken = false; + Logger.verbose(`MsalCachePlugin: Cache file for ${alternateConfig.name} cache also deleted.`); + } } return '{}'; // Return empty json string if cache not read. } finally { - lockFile.unlockSync(config.lockFilePath); - config.lockTaken = false; + lockFile.unlockSync(currentConfig.lockFilePath); + currentConfig.lockTaken = false; } }