Skip to content

[msal-node] Cache Lookup - 1: Logical keys for cache entities #1624

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

Merged
merged 34 commits into from
Jun 4, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
446258d
Edit tests to match the new key generation
sameerag May 11, 2020
8b86a3d
update test case desc
sameerag May 11, 2020
82e11c8
Add accountKey changes
sameerag May 11, 2020
4ff2f5f
Merge branch 'msal-node-cache-fileops-async' into msal-node-cache-keys
sameerag May 11, 2020
6948e41
Merge branch 'msal-node-cache-fileops-async' into msal-node-cache-keys
sameerag May 11, 2020
ddee5c4
Merge branch 'msal-node-cache-fileops-async' into msal-node-cache-keys
sameerag May 11, 2020
a456a67
Merge branch 'msal-node-cache-fileops-async' into msal-node-cache-keys
sameerag May 11, 2020
398802f
Merge branch 'msal-node-cache-fileops-async' into msal-node-cache-keys
sameerag May 11, 2020
5863af0
Cache lookup helpers
sameerag May 14, 2020
c3d30dd
Name Change for some APIs
sameerag May 14, 2020
6f9565b
Add helper functions in UnifiedCacheManager
sameerag May 14, 2020
4ddae07
Fix linter issues
sameerag May 14, 2020
2b0afe1
fix linter issues
sameerag May 14, 2020
953e638
Merge branch 'msal-node-cache-lookup-interface' into msal-node-cache-…
sameerag May 14, 2020
7ca83ab
fix linter issues
sameerag May 14, 2020
87ce032
Changing the filter interface for users to be able to pass optional p…
sameerag May 15, 2020
06ecbaf
Merge branch 'msal-node-cache-lookup-interface' into msal-node-cache-…
sameerag May 15, 2020
d109982
fixing compile issues
sameerag May 15, 2020
7369e51
Response Changes
sameerag May 19, 2020
9649a91
fixing linter issues
sameerag May 20, 2020
703b6c1
Merge branch 'msal-node-cache-lookup-interface' into msal-node-cache-…
sameerag May 20, 2020
e19fdad
fix linter issues
sameerag May 20, 2020
5b66edc
Merge branch 'dev' into msal-node-cache-keys
sameerag May 27, 2020
87fa687
Merge branch 'dev' into msal-node-cache-lookup-interface
sameerag May 27, 2020
1660b16
Merge branch 'msal-node-cache-keys' into msal-node-cache-lookup-inter…
sameerag May 27, 2020
403e81a
Merge branch 'msal-node-cache-lookup-interface' into msal-node-cache-…
sameerag May 27, 2020
bdd0162
Merge branch 'dev' into msal-node-cache-keys
sameerag Jun 3, 2020
036ac45
fixing linter errors
sameerag Jun 3, 2020
ef1f729
Merge pull request #1680 from AzureAD/msal-node-cache-response
sameerag Jun 3, 2020
2da03d9
Pushing changes for cacheType and some other oprtimizations! Not final.
sameerag Jun 3, 2020
533a907
Merge branch 'msal-node-cache-lookup-interface' of https://github.com…
sameerag Jun 3, 2020
4dcd69d
Merge branch 'dev' into msal-node-cache-keys
sameerag Jun 3, 2020
3da2e57
Cache save/delete/lookup moved to platform specific implementation
sameerag Jun 4, 2020
4a4dc16
Merge pull request #1655 from AzureAD/msal-node-cache-lookup-interface
sameerag Jun 4, 2020
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
8 changes: 4 additions & 4 deletions lib/msal-common/src/unifiedCache/UnifiedCacheManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,17 @@ export class UnifiedCacheManager {
idToken: IdTokenEntity,
refreshToken: RefreshTokenEntity
): void {
this.inMemoryCache.accessTokens[accessToken.generateAccessTokenEntityKey()] = accessToken;
this.inMemoryCache.idTokens[idToken.generateIdTokenEntityKey()] = idToken;
this.inMemoryCache.refreshTokens[refreshToken.generateRefreshTokenEntityKey()] = refreshToken;
this.inMemoryCache.accessTokens[accessToken.generateCredentialKey()] = accessToken;
this.inMemoryCache.idTokens[idToken.generateCredentialKey()] = idToken;
this.inMemoryCache.refreshTokens[refreshToken.generateCredentialKey()] = refreshToken;
}

/**
* append account to the in memory cache
* @param account
*/
addAccountEntity(account: AccountEntity): void {
const accKey = account.generateAccountEntityKey();
const accKey = account.generateAccountKey();
if (!this.inMemoryCache.accounts[accKey]) {
this.inMemoryCache.accounts[accKey] = account;
}
Expand Down
20 changes: 2 additions & 18 deletions lib/msal-common/src/unifiedCache/entities/AccessTokenEntity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { Credential } from "./Credential";
import { Separators } from "../../utils/Constants";
import { CredentialType } from "../../utils/Constants";
import { AuthenticationResult } from "../../response/AuthenticationResult";

/**
Expand All @@ -20,22 +20,6 @@ export class AccessTokenEntity extends Credential {
keyId?: string; // for POP and SSH tokenTypes
tokenType?: string;

/**
* Generate Account Cache Key as per the schema: <home_account_id>-<environment>-<realm*>
*/
public generateAccessTokenEntityKey(): string {
const accessTokenKeyArray: Array<string> = [
this.homeAccountId,
this.environment,
this.credentialType,
this.clientId,
this.realm,
this.target
];

return accessTokenKeyArray.join(Separators.CACHE_KEY_SEPARATOR).toLowerCase();
}

/**
* Create AccessTokenEntity
* @param homeAccountId
Expand All @@ -52,7 +36,7 @@ export class AccessTokenEntity extends Credential {
const atEntity: AccessTokenEntity = new AccessTokenEntity();

atEntity.homeAccountId = homeAccountId;
atEntity.credentialType = "AccessToken";
atEntity.credentialType = CredentialType.ACCESS_TOKEN;
atEntity.secret = authenticationResult.accessToken;

const date = new Date();
Expand Down
26 changes: 20 additions & 6 deletions lib/msal-common/src/unifiedCache/entities/AccountEntity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,31 @@ export class AccountEntity {
lastModificationTime?: string;
lastModificationApp?: string;

/**
* Generate Account Id key component as per the schema: <home_account_id>-<environment>
*/
generateAccountId(): string {
const accountId: Array<string> = [this.homeAccountId, this.environment];
return accountId.join(Separators.CACHE_KEY_SEPARATOR).toLowerCase();
}

/**
* Generate Account Id key component as per the schema: <home_account_id>-<environment>
*/
generateRealm(): string {
return (this.realm || "").toLowerCase();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think empty space is used in multiple places, consider adding it to the constants file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for EMPTY_STRING constant

}

/**
* Generate Account Cache Key as per the schema: <home_account_id>-<environment>-<realm*>
*/
public generateAccountEntityKey(): string {
const accountCacheKeyArray: Array<string> = [
this.homeAccountId,
this.environment,
this.realm
public generateAccountKey(): string {
const accountKey = [
this.generateAccountId(),
this.generateRealm()
];

return accountCacheKeyArray.join(Separators.CACHE_KEY_SEPARATOR).toLowerCase();
return accountKey.join(Separators.CACHE_KEY_SEPARATOR).toLowerCase();
}

/**
Expand Down
53 changes: 51 additions & 2 deletions lib/msal-common/src/unifiedCache/entities/Credential.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,62 @@
* Licensed under the MIT License.
*/

import { Separators, CredentialType } from "../../utils/Constants";

/**
* Base type for credentials to be stored in the cache: eg: ACCESS_TOKEN, ID_TOKEN etc
*/
export class Credential {
homeAccountId: string;
environment: string;
credentialType: string;
credentialType: CredentialType;
clientId: string;
secret: string;
};
familyId?: string;
realm?: string;
target?: string;

/**
* Generate Account Id key component as per the schema: <home_account_id>-<environment>
*/
generateAccountId(): string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these be used as utils for cache lookup (i.e. generate keys for looking up a specific cred)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So generating accountId for an account object instance should be separate from generating it from a set of values. I will try coming up with common code as needed, for now I added static helpers in CacheHelper.ts in a later PR to address this. Point noted though.

const accountId: Array<string> = [this.homeAccountId, this.environment];
return accountId.join(Separators.CACHE_KEY_SEPARATOR).toLowerCase();
}

/**
* Generate Credential Id key component as per the schema: <credential_type>-<client_id>-<realm>
*/
generateCredentialId(): string {
const clientOrFamilyId = CredentialType.REFRESH_TOKEN
? this.familyId || this.clientId
: this.clientId;
const credentialId: Array<string> = [
this.credentialType,
clientOrFamilyId,
this.realm || "",
];

return credentialId.join(Separators.CACHE_KEY_SEPARATOR).toLowerCase();
}

/**
* Generate target key component as per schema: <target>
*/
generateTarget(): string {
return (this.target || "").toLowerCase();
}

/**
* generates credential key
*/
generateCredentialKey(): string {
const credentialKey = [
this.generateAccountId(),
this.generateCredentialId(),
this.generateTarget(),
];

return credentialKey.join(Separators.CACHE_KEY_SEPARATOR).toLowerCase();
}
}
20 changes: 2 additions & 18 deletions lib/msal-common/src/unifiedCache/entities/IdTokenEntity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { Credential } from "./Credential";
import { Separators } from "../../utils/Constants";
import { CredentialType } from "../../utils/Constants";
import { AuthenticationResult } from "../../response/AuthenticationResult";

/**
Expand All @@ -13,22 +13,6 @@ import { AuthenticationResult } from "../../response/AuthenticationResult";
export class IdTokenEntity extends Credential {
realm: string;

/**
* Generate Account Cache Key as per the schema: <home_account_id>-<environment>-<realm*>
*/
generateIdTokenEntityKey(): string {
const idTokenKeyArray: Array<string> = [
this.homeAccountId,
this.environment,
this.credentialType,
this.clientId,
this.realm,
"" // target
];

return idTokenKeyArray.join(Separators.CACHE_KEY_SEPARATOR).toLowerCase();
}

/**
* Create IdTokenEntity
* @param homeAccountId
Expand All @@ -44,7 +28,7 @@ export class IdTokenEntity extends Credential {
): IdTokenEntity {
const idTokenEntity = new IdTokenEntity();

idTokenEntity.credentialType = "IdToken";
idTokenEntity.credentialType = CredentialType.ID_TOKEN;
idTokenEntity.homeAccountId = homeAccountId;
idTokenEntity.environment = environment;
idTokenEntity.clientId = clientId;
Expand Down
24 changes: 2 additions & 22 deletions lib/msal-common/src/unifiedCache/entities/RefreshTokenEntity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { Credential } from "./Credential";
import { Separators } from "../../utils/Constants";
import { CredentialType } from "../../utils/Constants";
import { AuthenticationResult } from "../../response/AuthenticationResult";

/**
Expand All @@ -13,26 +13,6 @@ import { AuthenticationResult } from "../../response/AuthenticationResult";
export class RefreshTokenEntity extends Credential {
familyId?: string;

/**
* Generate Account Cache Key as per the schema: <home_account_id>-<environment>-<realm*>
*/
generateRefreshTokenEntityKey(): string {
const refreshTokenKeyArray: Array<string> = [
this.homeAccountId,
this.environment,
this.credentialType
];

// append familyId if populted, else fallback to clientId
refreshTokenKeyArray.push(this.familyId || this.clientId);

// realm and target - empty string "" for REFRESH_TOKEN type; target (scopes) is added only if it is resource specific refresh token
refreshTokenKeyArray.push("");
refreshTokenKeyArray.push("");

return refreshTokenKeyArray.join(Separators.CACHE_KEY_SEPARATOR).toLowerCase();
}

/**
* Create RefreshTokenEntity
* @param homeAccountId
Expand All @@ -50,7 +30,7 @@ export class RefreshTokenEntity extends Credential {
const rtEntity = new RefreshTokenEntity();

rtEntity.clientId = clientId;
rtEntity.credentialType = "RefreshToken";
rtEntity.credentialType = CredentialType.REFRESH_TOKEN;
rtEntity.environment = environment;
rtEntity.homeAccountId = homeAccountId;
rtEntity.secret = refreshToken;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ describe("UnifiedCacheManager test cases", () => {

// create mock AccessToken
const atOne = mockCache.createMockATOne();
const atOneKey = atOne.generateAccessTokenEntityKey();
const atOneKey = atOne.generateCredentialKey();
const atTwo = mockCache.createMockATTwo();
const atTwoKey = atTwo.generateAccessTokenEntityKey();
const atTwoKey = atTwo.generateCredentialKey();

expect(Object.keys(unifiedCacheManager.getCacheInMemory().accessTokens).length).to.equal(2);
expect(unifiedCacheManager.getCacheInMemory().accessTokens[atOneKey]).to.eql(atOne);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ describe("AccessTokenEntity.ts Unit Tests", () => {
expect(at instanceof AccessTokenEntity);
});

it("Create an AccessTokenCacheEntity entity", () => {
it("Generate AccessTokenEntity key", () => {
let at = mockCache.createMockATOne();
expect(at.generateAccessTokenEntityKey()).to.eql(
expect(at.generateCredentialKey()).to.eql(
"uid.utid-login.microsoftonline.com-accesstoken-mock_client_id-microsoft-scope1 scope2 scope3"
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe("AccountEntity.ts Unit Tests", () => {
it("generate an AccountEntityKey", () => {
let ac = new AccountEntity();
Object.assign(ac, mockAccountEntity);
expect(ac.generateAccountEntityKey()).to.eql(
expect(ac.generateAccountKey()).to.eql(
"uid.utid-login.microsoftonline.com-microsoft"
);
});
Expand Down Expand Up @@ -81,7 +81,7 @@ describe("AccountEntity.ts Unit Tests", () => {
cryptoInterface
);

expect(acc.generateAccountEntityKey()).to.eql(
expect(acc.generateAccountKey()).to.eql(
"uid.utid-login.microsoftonline.com-microsoft"
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe("IdTokenEntity.ts Unit Tests", () => {
it("Create an IdTokenEntity", () => {
let idT = new IdTokenEntity();
Object.assign(idT, mockIdTokenEntity);
expect(idT.generateIdTokenEntityKey()).to.eql(
expect(idT.generateCredentialKey()).to.eql(
"uid.utid-login.microsoftonline.com-idtoken-mock_client_id-microsoft-"
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ describe("RefreshTokenEntity.ts Unit Tests", () => {
it("Create a RefreshTokenEntity", () => {
let rt = new RefreshTokenEntity();
Object.assign(rt, mockRefreshTokenEntity);
expect(rt.generateRefreshTokenEntityKey()).to.eql(
expect(rt.generateCredentialKey()).to.eql(
"uid.utid-login.microsoftonline.com-refreshtoken-mock_client_id--"
);
});

it("Create a RefreshTokenEntity with familyId", () => {
let rt = new RefreshTokenEntity();
Object.assign(rt, mockRefreshTokenEntityWithFamilyId);
expect(rt.generateRefreshTokenEntityKey()).to.eql(
expect(rt.generateCredentialKey()).to.eql(
"uid.utid-login.microsoftonline.com-refreshtoken-1--"
);
});
Expand Down
12 changes: 6 additions & 6 deletions lib/msal-common/test/unifiedCache/entities/cacheConstants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,17 +129,17 @@ export class mockCache {

export const MockCache = {
atOne: mockCache.createMockATOne(),
atOneKey: mockCache.createMockATOne().generateAccessTokenEntityKey(),
atOneKey: mockCache.createMockATOne().generateCredentialKey(),
atTwo: mockCache.createMockATTwo(),
atTwoKey: mockCache.createMockATTwo().generateAccessTokenEntityKey(),
atTwoKey: mockCache.createMockATTwo().generateCredentialKey(),
idT: mockCache.createMockIdT(),
idTKey: mockCache.createMockIdT().generateIdTokenEntityKey(),
idTKey: mockCache.createMockIdT().generateCredentialKey(),
rt: mockCache.createMockRT(),
rtKey: mockCache.createMockRT().generateRefreshTokenEntityKey(),
rtKey: mockCache.createMockRT().generateCredentialKey(),
rtF: mockCache.createMockRTWithFamilyId(),
rtFKey: mockCache.createMockRTWithFamilyId().generateRefreshTokenEntityKey(),
rtFKey: mockCache.createMockRTWithFamilyId().generateCredentialKey(),
acc: mockCache.createMockAcc(),
accKey: mockCache.createMockAcc().generateAccountEntityKey(),
accKey: mockCache.createMockAcc().generateAccountKey(),
amdt: mockCache.createMockAmdt(),
amdtKey: mockCache.createMockAmdt().generateAppMetaDataEntityKey()
}