diff --git a/extensions/azurecore/package.json b/extensions/azurecore/package.json index 32e4b63397..5e71071e90 100644 --- a/extensions/azurecore/package.json +++ b/extensions/azurecore/package.json @@ -364,6 +364,7 @@ "@azure/storage-blob": "^12.6.0", "axios": "^0.27.2", "crypto": "^1.0.1", + "lockfile": "1.0.4", "msal": "^1.4.16", "node-fetch": "^2.6.7", "qs": "^6.9.1", @@ -375,6 +376,7 @@ "@microsoft/azdata-test": "^2.0.3", "@microsoft/vscodetestcover": "^1.2.1", "@types/keytar": "4.4.0", + "@types/lockfile": "^1.0.2", "@types/mocha": "^7.0.2", "@types/node": "^12.20.55", "@types/qs": "^6.9.1", diff --git a/extensions/azurecore/src/account-provider/utils/msalCachePlugin.ts b/extensions/azurecore/src/account-provider/utils/msalCachePlugin.ts index 175eae7768..e7496dedcf 100644 --- a/extensions/azurecore/src/account-provider/utils/msalCachePlugin.ts +++ b/extensions/azurecore/src/account-provider/utils/msalCachePlugin.ts @@ -4,42 +4,62 @@ *--------------------------------------------------------------------------------------------*/ import { ICachePlugin, TokenCacheContext } from '@azure/msal-node'; -import { constants, promises as fsPromises } from 'fs'; +import { promises as fsPromises } from 'fs'; + +import * as lockFile from 'lockfile'; import * as path from 'path'; +import { AccountsClearTokenCacheCommand } from '../../constants'; import { Logger } from '../../utils/Logger'; export class MsalCachePluginProvider { constructor( private readonly _serviceName: string, - private readonly _msalFilePath: string + private readonly _msalFilePath: string, ) { this._msalFilePath = path.join(this._msalFilePath, this._serviceName); this._serviceName = this._serviceName.replace(/-/, '_'); Logger.verbose(`MsalCachePluginProvider: Using cache path ${_msalFilePath} and serviceName ${_serviceName}`); } + private _lockTaken: boolean = false; + + private getLockfilePath(): string { + return this._msalFilePath + '.lock'; + } + public getCachePlugin(): ICachePlugin { + const lockFilePath = this.getLockfilePath(); const beforeCacheAccess = async (cacheContext: TokenCacheContext): Promise => { - let exists = true; + await this.waitAndLock(lockFilePath); try { - await fsPromises.access(this._msalFilePath, constants.R_OK | constants.W_OK); - } catch { - exists = false; - } - if (exists) { + const cache = await fsPromises.readFile(this._msalFilePath, { encoding: 'utf8' }); try { - const cache = await fsPromises.readFile(this._msalFilePath, { encoding: 'utf8' }); cacheContext.tokenCache.deserialize(cache); - Logger.verbose(`MsalCachePlugin: Token read from cache successfully.`); } catch (e) { - Logger.error(`MsalCachePlugin: Failed to read from cache file. ${e}`); + // Handle deserialization error in cache file in case file gets corrupted. + // Clearing cache here will ensure account is marked stale so re-authentication can be triggered. + Logger.verbose(`MsalCachePlugin: Error occurred when trying to read cache file, file contents will be cleared: ${e.message}`); + await fsPromises.writeFile(this._msalFilePath, '', { encoding: 'utf8' }); + } + Logger.verbose(`MsalCachePlugin: Token read from cache successfully.`); + } catch (e) { + if (e.code === 'ENOENT') { + // File doesn't exist, log and continue + Logger.verbose(`MsalCachePlugin: Cache file not found on disk: ${e.code}`); + } + else { + Logger.error(`MsalCachePlugin: Failed to read from cache file: ${e}`); throw e; } + } finally { + lockFile.unlockSync(lockFilePath); + this._lockTaken = false; } - }; + } const afterCacheAccess = async (cacheContext: TokenCacheContext): Promise => { if (cacheContext.cacheHasChanged) { + await this.waitAndLock(lockFilePath); try { const data = cacheContext.tokenCache.serialize(); await fsPromises.writeFile(this._msalFilePath, data, { encoding: 'utf8' }); @@ -47,6 +67,9 @@ export class MsalCachePluginProvider { } catch (e) { Logger.error(`MsalCachePlugin: Failed to write to cache file. ${e}`); throw e; + } finally { + lockFile.unlockSync(lockFilePath); + this._lockTaken = false; } } }; @@ -61,4 +84,36 @@ export class MsalCachePluginProvider { afterCacheAccess, }; } + + private async waitAndLock(lockFilePath: string): Promise { + // Make 500 retry attempts with 100ms wait time between each attempt to allow enough time for the lock to be released. + const retries = 500; + const retryWait = 100; + + // We cannot rely on lockfile.lockSync() to clear stale lockfile, + // so we check if the lockfile exists and if it does, calling unlockSync() will clear it. + if (lockFile.checkSync(lockFilePath) && !this._lockTaken) { + lockFile.unlockSync(lockFilePath); + Logger.verbose(`MsalCachePlugin: Stale lockfile found and has been removed.`); + } + + let retryAttempt = 0; + while (retryAttempt <= retries) { + try { + // Use lockfile.lockSync() to ensure only one process is accessing the cache at a time. + // lockfile.lock() does not wait for async callback promise to resolve. + lockFile.lockSync(lockFilePath); + this._lockTaken = true; + break; + } catch (e) { + if (retryAttempt === retries) { + Logger.error(`MsalCachePlugin: Failed to acquire lock on cache file after ${retries} attempts.`); + throw new Error(`Failed to acquire lock on cache file after ${retries} attempts. Please attempt command: '${AccountsClearTokenCacheCommand}' to clear access token cache.`); + } + retryAttempt++; + Logger.verbose(`MsalCachePlugin: Failed to acquire lock on cache file. Retrying in ${retryWait} ms.`); + await new Promise(resolve => setTimeout(resolve, retryWait)); + } + } + } } diff --git a/extensions/azurecore/yarn.lock b/extensions/azurecore/yarn.lock index d94d09edf5..1972bcef33 100644 --- a/extensions/azurecore/yarn.lock +++ b/extensions/azurecore/yarn.lock @@ -413,6 +413,11 @@ resolved "https://registry.yarnpkg.com/@types/keytar/-/keytar-4.4.0.tgz#ca24e6ee6d0df10c003aafe26e93113b8faf0d8e" integrity sha512-cq/NkUUy6rpWD8n7PweNQQBpw2o0cf5v6fbkUVEpOB9VzzIvyPvSEId1/goIj+MciW2v1Lw5mRimKO01XgE9EA== +"@types/lockfile@^1.0.2": + version "1.0.2" + resolved "https://registry.yarnpkg.com/@types/lockfile/-/lockfile-1.0.2.tgz#3f77e84171a2b7e3198bd5717c7547a54393baf8" + integrity sha512-jD5VbvhfMhaYN4M3qPJuhMVUg3Dfc4tvPvLEAXn6GXbs/ajDFtCQahX37GIE65ipTI3I+hEvNaXS3MYAn9Ce3Q== + "@types/mocha@^7.0.2": version "7.0.2" resolved "https://registry.yarnpkg.com/@types/mocha/-/mocha-7.0.2.tgz#b17f16cf933597e10d6d78eae3251e692ce8b0ce" @@ -1334,6 +1339,13 @@ locate-path@^3.0.0: p-locate "^3.0.0" path-exists "^3.0.0" +lockfile@1.0.4: + version "1.0.4" + resolved "https://registry.yarnpkg.com/lockfile/-/lockfile-1.0.4.tgz#07f819d25ae48f87e538e6578b6964a4981a5609" + integrity sha512-cvbTwETRfsFh4nHsL1eGWapU1XFi5Ot9E85sWAwia7Y7EgB7vfqcZhTKZ+l7hCGxSPoushMv5GKhT5PdLv03WA== + dependencies: + signal-exit "^3.0.2" + lodash.get@^4.4.2: version "4.4.2" resolved "https://registry.yarnpkg.com/lodash.get/-/lodash.get-4.4.2.tgz#2d177f652fa31e939b4438d5341499dfa3825e99" @@ -1774,6 +1786,11 @@ side-channel@^1.0.4: get-intrinsic "^1.0.2" object-inspect "^1.9.0" +signal-exit@^3.0.2: + version "3.0.7" + resolved "https://registry.yarnpkg.com/signal-exit/-/signal-exit-3.0.7.tgz#a9a1767f8af84155114eaabd73f99273c8f59ad9" + integrity sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ== + sinon@^9.0.2: version "9.0.2" resolved "https://registry.yarnpkg.com/sinon/-/sinon-9.0.2.tgz#b9017e24633f4b1c98dfb6e784a5f0509f5fd85d"