diff --git a/change/@azure-msal-node-b01d5409-72eb-4ca1-aa58-d4985ba4b459.json b/change/@azure-msal-node-b01d5409-72eb-4ca1-aa58-d4985ba4b459.json new file mode 100644 index 0000000000..48e5cfb00c --- /dev/null +++ b/change/@azure-msal-node-b01d5409-72eb-4ca1-aa58-d4985ba4b459.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Fixes bug where getAllAccounts always writes to the cache", + "packageName": "@azure/msal-node", + "email": "shylasummers@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/lib/msal-node/src/cache/TokenCache.ts b/lib/msal-node/src/cache/TokenCache.ts index 22268ca122..db13872253 100644 --- a/lib/msal-node/src/cache/TokenCache.ts +++ b/lib/msal-node/src/cache/TokenCache.ts @@ -124,7 +124,7 @@ export class TokenCache implements ISerializableTokenCache, ITokenCache { let cacheContext; try { if (this.persistence) { - cacheContext = new TokenCacheContext(this, true); + cacheContext = new TokenCacheContext(this, false); await this.persistence.beforeCacheAccess(cacheContext); } return this.storage.getAllAccounts(); diff --git a/lib/msal-node/test/cache/TokenCache.spec.ts b/lib/msal-node/test/cache/TokenCache.spec.ts index 1adc370e74..2ca8234056 100644 --- a/lib/msal-node/test/cache/TokenCache.spec.ts +++ b/lib/msal-node/test/cache/TokenCache.spec.ts @@ -7,7 +7,7 @@ import { } from "@azure/msal-common"; import { NodeStorage } from "../../src/cache/NodeStorage"; import { TokenCache } from "../../src/cache/TokenCache"; -import { promises as fs } from "fs"; +import { existsSync, watch, promises, FSWatcher } from "fs"; import { version, name } from "../../package.json"; import { DEFAULT_CRYPTO_IMPLEMENTATION, @@ -24,6 +24,7 @@ const msalCommon: MSALCommonModule = jest.requireActual( describe("TokenCache tests", () => { let logger: Logger; let storage: NodeStorage; + let watcher: FSWatcher; beforeEach(() => { const loggerOptions = { @@ -50,6 +51,12 @@ describe("TokenCache tests", () => { jest.restoreAllMocks(); }); + afterEach(() => { + if (watcher) { + watcher.close(); + } + }); + it("Constructor tests builds default token cache", async () => { const tokenCache = new TokenCache(storage, logger); expect(tokenCache).toBeInstanceOf(TokenCache); @@ -115,7 +122,7 @@ describe("TokenCache tests", () => { it("TokenCache beforeCacheAccess and afterCacheAccess", async () => { const beforeCacheAccess = async (context: TokenCacheContext) => { context.tokenCache.deserialize( - await fs.readFile( + await promises.readFile( "./test/cache/cache-test-files/cache-unrecognized-entities.json", "utf-8" ) @@ -123,7 +130,7 @@ describe("TokenCache tests", () => { }; const cachePath = "./test/cache/cache-test-files/temp-cache.json"; const afterCacheAccess = async (context: TokenCacheContext) => { - await fs.writeFile(cachePath, context.tokenCache.serialize()); + await promises.writeFile(cachePath, context.tokenCache.serialize()); }; const cachePlugin: ICachePlugin = { @@ -153,7 +160,7 @@ describe("TokenCache tests", () => { // try and clean up try { - await fs.unlink(cachePath); + await promises.unlink(cachePath); } catch (err) { const errnoException = err as NodeJS.ErrnoException; if (errnoException.code == "ENOENT") { @@ -164,6 +171,50 @@ describe("TokenCache tests", () => { } }); + it("getAllAccounts doesn't write to cache", async () => { + const cachePath = + "./test/cache/cache-test-files/cache-unrecognized-entities.json"; + if (existsSync(cachePath)) { + watcher = watch(cachePath, (eventType: string) => { + if (eventType === "change") { + throw new Error("test cache changed"); + } + }); + } else { + throw new Error("error in watching test cache"); + } + + const beforeCacheAccess = jest.fn( + async (context: TokenCacheContext) => { + if (context.hasChanged == true) { + throw new Error("hasChanged should be false"); + } + return promises.readFile(cachePath, "utf-8").then((data) => { + context.tokenCache.deserialize(data); + }); + } + ); + + const afterCacheAccess = jest.fn(async (context: TokenCacheContext) => { + if (context.hasChanged == true) { + throw new Error("hasChanged should be false"); + } + return Promise.resolve(); + }); + + const cachePlugin: ICachePlugin = { + beforeCacheAccess, + afterCacheAccess, + }; + + const tokenCache = new TokenCache(storage, logger, cachePlugin); + + const accounts = await tokenCache.getAllAccounts(); + expect(accounts.length).toBe(1); + expect(beforeCacheAccess).toHaveBeenCalled(); + expect(afterCacheAccess).toHaveBeenCalled(); + }); + it("should return an empty KV store if TokenCache is empty", () => { const tokenCache = new TokenCache(storage, logger);