Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle case where account entities is empty in after cache access #7329

Open
wants to merge 19 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
9272c19
fix: handle case where account entities is empty in after cache access
LuccaRebelloToledo Sep 20, 2024
a143b90
refactor: improve clarity and maintainability of after cache access
LuccaRebelloToledo Sep 20, 2024
0ccc126
Merge branch 'dev' into fix/distributed-cache-plugin
LuccaRebelloToledo Sep 21, 2024
17ee935
Merge branch 'dev' into fix/distributed-cache-plugin
LuccaRebelloToledo Sep 26, 2024
38e133c
Merge branch 'dev' into fix/distributed-cache-plugin
LuccaRebelloToledo Sep 27, 2024
899c103
Merge branch 'dev' into fix/distributed-cache-plugin
LuccaRebelloToledo Oct 11, 2024
b2a5eb0
Merge branch 'dev' into fix/distributed-cache-plugin
LuccaRebelloToledo Oct 25, 2024
65a930d
refactor: keep afterCacheAccess more simple
LuccaRebelloToledo Oct 25, 2024
6abca33
test: add unit test to verify account removal from token cache, ensur…
LuccaRebelloToledo Oct 25, 2024
020be27
Merge branch 'dev' into fix/distributed-cache-plugin
LuccaRebelloToledo Oct 30, 2024
cf7c07f
Merge branch 'dev' into fix/distributed-cache-plugin
LuccaRebelloToledo Nov 3, 2024
0b7b4ac
Merge branch 'dev' into fix/distributed-cache-plugin
LuccaRebelloToledo Nov 5, 2024
c6433fe
Merge branch 'dev' into fix/distributed-cache-plugin
LuccaRebelloToledo Nov 6, 2024
23a706f
Merge branch 'dev' into fix/distributed-cache-plugin
LuccaRebelloToledo Nov 8, 2024
fa81761
Merge branch 'dev' into fix/distributed-cache-plugin
LuccaRebelloToledo Nov 13, 2024
ecf0bf5
chore: remove unnecessary empty line
LuccaRebelloToledo Nov 13, 2024
b984bda
Change files
LuccaRebelloToledo Nov 13, 2024
147631e
test: add temporary comment to trigger workflow
LuccaRebelloToledo Nov 13, 2024
b2e4a57
chore: remove temporary comment
LuccaRebelloToledo Nov 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "removeAccount method now clears client cache by retrieving partitionKey when accountEntities are empty (#7329)",
"packageName": "@azure/msal-node",
"email": "[email protected]",
"dependentChangeType": "patch"
}
15 changes: 9 additions & 6 deletions lib/msal-node/src/cache/distributed/DistributedCachePlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,20 @@ export class DistributedCachePlugin implements ICachePlugin {
AccountEntity.isAccountEntity(value as object)
);

let partitionKey: string;
if (accountEntities.length > 0) {
const accountEntity = accountEntities[0] as AccountEntity;
const partitionKey = await this.partitionManager.extractKey(
partitionKey = await this.partitionManager.extractKey(
accountEntity
);

await this.client.set(
partitionKey,
cacheContext.tokenCache.serialize()
);
Robbie-Microsoft marked this conversation as resolved.
Show resolved Hide resolved
} else {
partitionKey = await this.partitionManager.getKey();
}

await this.client.set(
partitionKey,
cacheContext.tokenCache.serialize()
);
}
}
}
30 changes: 27 additions & 3 deletions lib/msal-node/test/cache/cacheConstants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ import {
CacheHelpers,
} from "@azure/msal-common";

export const MOCK_PARTITION_KEY = "mock_partition_key";
export const MOCK_CACHE_STRING = "mock_cache_string";

// mock tokens
export const mockAccessTokenEntity_1: AccessTokenEntity = {
homeAccountId: "uid.utid",
Expand Down Expand Up @@ -140,3 +137,30 @@ export const MockCache = {
amdt: mockCache.createMockAmdt(),
amdtKey: CacheHelpers.generateAppMetadataKey(mockCache.createMockAmdt()),
};

// mock cache storage
export const MOCK_PARTITION_KEY = MockCache.acc.homeAccountId;
export const MOCK_CACHE_STORAGE = {
[MOCK_PARTITION_KEY]: {
Account: {
[`${MOCK_PARTITION_KEY}-login.windows.net`]: mockAccountEntity,
},
IdToken: {
[`${MOCK_PARTITION_KEY}-login.windows.net-idtoken`]:
mockIdTokenEntity,
},
AccessToken: {
[`${MOCK_PARTITION_KEY}-login.windows.net-accesstoken`]:
mockAccessTokenEntity_1,
},
RefreshToken: {
[`${MOCK_PARTITION_KEY}-login.windows.net-refreshtoken`]:
mockRefreshTokenEntity,
},
AppMetadata: {
[`${MOCK_PARTITION_KEY}-login.windows.net-appmetadata`]:
mockAppMetaDataEntity,
},
},
};
export const MOCK_CACHE_STRING = () => JSON.stringify(MOCK_CACHE_STORAGE);
Original file line number Diff line number Diff line change
@@ -1,32 +1,50 @@
import { DistributedCachePlugin } from "../../../src/cache/distributed/DistributedCachePlugin";
import { DistributedCachePlugin } from "../../../src/cache/distributed/DistributedCachePlugin.js";
import {
AccountEntity,
ICachePlugin,
TokenCacheContext,
} from "@azure/msal-common";
import { TokenCache } from "../../../src/cache/TokenCache";
import { TokenCache } from "../../../src/cache/TokenCache.js";
import {
MockCache,
MOCK_CACHE_STRING,
MOCK_PARTITION_KEY,
} from "../cacheConstants";
import { IPartitionManager } from "../../../src/cache/distributed/IPartitionManager";
import { ICacheClient } from "../../../src/cache/distributed/ICacheClient";
MOCK_CACHE_STORAGE,
} from "../cacheConstants.js";
import { IPartitionManager } from "../../../src/cache/distributed/IPartitionManager.js";
import { ICacheClient } from "../../../src/cache/distributed/ICacheClient.js";

describe("Distributed Cache Plugin Tests for msal-node", () => {
let distributedCachePluginInstance: ICachePlugin;
let cacheHasChanged = true;
const tokenCache = {
serialize: jest
.fn()
.mockImplementation((): string => MOCK_CACHE_STRING),
serialize: jest.fn().mockImplementation((): string => {
cacheHasChanged = false;
return MOCK_CACHE_STRING();
}),
deserialize: jest.fn(),
getKVStore: jest.fn().mockImplementation(() => ({
[MockCache.idTKey]: MockCache.idT,
[MockCache.accKey]: MockCache.acc,
})),
getAllAccounts: jest
.fn()
.mockImplementation(async () => [MockCache.acc]),
removeAccount: jest.fn().mockImplementation(async () => {
const cacheStorage = MOCK_CACHE_STORAGE;

(cacheStorage[MOCK_PARTITION_KEY].Account as any) = {};
(cacheStorage[MOCK_PARTITION_KEY].IdToken as any) = {};
(cacheStorage[MOCK_PARTITION_KEY].AccessToken as any) = {};
(cacheStorage[MOCK_PARTITION_KEY].RefreshToken as any) = {};
(cacheStorage[MOCK_PARTITION_KEY].AppMetadata as any) = {};

cacheHasChanged = true;
}),
hasChanged: jest.fn().mockImplementation(() => cacheHasChanged),
} as unknown as TokenCache;
const tokenCacheContext = {
cacheHasChanged: true,
cacheHasChanged,
tokenCache,
} as unknown as TokenCacheContext;
const partitionManager = {
Expand All @@ -46,7 +64,7 @@ describe("Distributed Cache Plugin Tests for msal-node", () => {
get: jest
.fn()
.mockImplementation(
async (_: string): Promise<string> => MOCK_CACHE_STRING
async (_: string): Promise<string> => MOCK_CACHE_STRING()
),
set: jest
.fn()
Expand Down Expand Up @@ -75,7 +93,9 @@ describe("Distributed Cache Plugin Tests for msal-node", () => {
// Confirm the intended effects
expect(partitionManager.getKey).toHaveBeenCalled();
expect(cacheClient.get).toHaveBeenCalledWith(MOCK_PARTITION_KEY);
expect(tokenCache.deserialize).toHaveBeenCalledWith(MOCK_CACHE_STRING);
expect(tokenCache.deserialize).toHaveBeenCalledWith(
MOCK_CACHE_STRING()
);
});

it("properly handles afterCacheAccess", async () => {
Expand All @@ -90,7 +110,29 @@ describe("Distributed Cache Plugin Tests for msal-node", () => {
expect(tokenCache.serialize).toHaveBeenCalled();
expect(cacheClient.set).toHaveBeenCalledWith(
MockCache.acc.homeAccountId,
MOCK_CACHE_STRING
MOCK_CACHE_STRING()
);
});

it("removes the specified account from the cache", async () => {
const accounts = await tokenCache.getAllAccounts();
await tokenCache.removeAccount(accounts[0]);
expect(tokenCache.hasChanged()).toEqual(true);

const tokenCacheAfterSerialization = JSON.parse(tokenCache.serialize());

expect(tokenCache.hasChanged()).toEqual(false);
expect(
tokenCacheAfterSerialization[MOCK_PARTITION_KEY].Account
).toEqual({});
expect(
tokenCacheAfterSerialization[MOCK_PARTITION_KEY].RefreshToken
).toEqual({});
expect(
tokenCacheAfterSerialization[MOCK_PARTITION_KEY].AccessToken
).toEqual({});
expect(
tokenCacheAfterSerialization[MOCK_PARTITION_KEY].IdToken
).toEqual({});
});
});
Loading