Skip to content

Commit

Permalink
Merge pull request #3755 from AzureAD/persist-active-account
Browse files Browse the repository at this point in the history
Persist active account
  • Loading branch information
tnorling committed Jun 29, 2021
2 parents f0ee29d + 44dc694 commit a48ad51
Show file tree
Hide file tree
Showing 27 changed files with 281 additions and 82 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "Remove note from FAQ about active accounts not persisting",
"packageName": "@azure/msal-angular",
"email": "thomas.norling@microsoft.com",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Persist active account #3755",
"packageName": "@azure/msal-browser",
"email": "thomas.norling@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Add constant for active account cache key #3755",
"packageName": "@azure/msal-common",
"email": "thomas.norling@microsoft.com",
"dependentChangeType": "patch"
}
2 changes: 1 addition & 1 deletion lib/msal-angular/docs/FAQ.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ We recommend setting the active account:
- After any action that may change the account, especially if your app uses multiple accounts. See [here](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/samples/msal-angular-v2-samples/angular11-sample-app/src/app/home/home.component.ts#L23) for an example of setting the account after a successful login.
- On initial page load. Wait until all interactions are complete (by subscribing to the `inProgress$` observable and filtering for `InteractionStatus.None`), check if there is an active account, and if there is none, set the active account. This could be the first account retrieved by `getAllAccounts()`, or other account selection logic required by your app. See [here](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/samples/msal-angular-v2-samples/angular11-sample-app) for an example of checking and setting the active account on page load.

**Note:** Currently, active accounts are for each page load and do not persist. While this is an enhancement we are looking to make, we recommend that you set the active account for each page load.
**Note:** Prior to `@azure/msal-browser@2.15.0` active account did not persist across page loads. If you are using `@azure/msal-browser@2.14.2` or earlier we recommend that you set the active account for each page load. In version 2.15.0 and above the active account will be cached in the cache location specified in your MSAL config and retrieved each time `getActiveAccount` is called.

Our [Angular 11](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/samples/msal-angular-v2-samples/angular11-sample-app) sample demonstrates basic usage. Your app may require more complicated logic to choose accounts.

Expand Down
20 changes: 2 additions & 18 deletions lib/msal-browser/src/app/ClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ export abstract class ClientApplication {
// Flag to indicate if in browser environment
protected isBrowserEnvironment: boolean;

// Sets the account to use if no account info is given
private activeLocalAccountId: string | null;

// Set the SKU and Version for wrapper library if applicable
private wrapperSKU: string | undefined;
private wrapperVer: string | undefined;
Expand Down Expand Up @@ -95,8 +92,6 @@ export abstract class ClientApplication {
// Set the configuration.
this.config = buildConfiguration(configuration, this.isBrowserEnvironment);

this.activeLocalAccountId = null;

// Initialize logger
this.logger = new Logger(this.config.system.loggerOptions, name, version);

Expand Down Expand Up @@ -918,25 +913,14 @@ export abstract class ClientApplication {
* @param account
*/
setActiveAccount(account: AccountInfo | null): void {
if (account) {
this.logger.verbose("setActiveAccount: Active account set");
this.activeLocalAccountId = account.localAccountId;
} else {
this.logger.verbose("setActiveAccount: No account passed, active account not set");
this.activeLocalAccountId = null;
}
this.browserStorage.setActiveAccount(account);
}

/**
* Gets the currently active account
*/
getActiveAccount(): AccountInfo | null {
if (!this.activeLocalAccountId) {
this.logger.verbose("getActiveAccount: No active account");
return null;
}

return this.getAccountByLocalId(this.activeLocalAccountId);
return this.browserStorage.getActiveAccount();
}

// #endregion
Expand Down
58 changes: 58 additions & 0 deletions lib/msal-browser/src/cache/BrowserCacheManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,64 @@ export class BrowserCacheManager extends CacheManager {
this.internalStorage.setItem(key, JSON.stringify(entity));
}

/**
* Gets the active account
*/
getActiveAccount(): AccountInfo | null {
const activeAccountIdKey = this.generateCacheKey(PersistentCacheKeys.ACTIVE_ACCOUNT);
const activeAccountId = this.browserStorage.getItem(activeAccountIdKey);
if (!activeAccountId) {
return null;
}
return this.getAccountInfoByFilter({localAccountId: activeAccountId})[0] || null;
}

/**
* Sets the active account's localAccountId in cache
* @param account
*/
setActiveAccount(account: AccountInfo | null): void {
const activeAccountIdKey = this.generateCacheKey(PersistentCacheKeys.ACTIVE_ACCOUNT);
if (account) {
this.logger.verbose("setActiveAccount: Active account set");
this.browserStorage.setItem(activeAccountIdKey, account.localAccountId);
} else {
this.logger.verbose("setActiveAccount: No account passed, active account not set");
this.browserStorage.removeItem(activeAccountIdKey);
}
}

/**
* Gets a list of accounts that match all of the filters provided
* @param account
*/
getAccountInfoByFilter(accountFilter: Partial<Omit<AccountInfo, "idTokenClaims"|"name">>): AccountInfo[] {
const allAccounts = this.getAllAccounts();
return allAccounts.filter((accountObj) => {
if (accountFilter.username && accountFilter.username.toLowerCase() !== accountObj.username.toLowerCase()) {
return false;
}

if (accountFilter.homeAccountId && accountFilter.homeAccountId !== accountObj.homeAccountId) {
return false;
}

if (accountFilter.localAccountId && accountFilter.localAccountId !== accountObj.localAccountId) {
return false;
}

if (accountFilter.tenantId && accountFilter.tenantId !== accountObj.tenantId) {
return false;
}

if (accountFilter.environment && accountFilter.environment !== accountObj.environment) {
return false;
}

return true;
});
}

/**
* fetch throttling entity from the platform cache
* @param throttlingCacheKey
Expand Down
73 changes: 29 additions & 44 deletions lib/msal-browser/test/app/PublicClientApplication.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
describe("Constructor tests", () => {

it("passes null check", (done) => {
expect(pca).not.toBeNull;
expect(pca).not.toBe(null);
expect(pca instanceof PublicClientApplication).toBeTruthy();
done();
});
Expand All @@ -78,7 +78,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
sinon.stub(pca, <any>"interactionInProgress").returns(false);
window.location.hash = TEST_HASHES.TEST_SUCCESS_CODE_HASH_REDIRECT;
window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, TEST_URIS.TEST_ALTERNATE_REDIR_URI);
expect(await pca.handleRedirectPromise()).toBeNull;
expect(await pca.handleRedirectPromise()).toBe(null);
}
);

Expand Down Expand Up @@ -424,7 +424,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
const tokenResponse1 = await promise1;
const tokenResponse2 = await promise2;
const tokenResponse3 = await pca.handleRedirectPromise("testHash");
expect(tokenResponse3).toBeNull();
expect(tokenResponse3).toBe(null);
const tokenResponse4 = await pca.handleRedirectPromise();

if (!tokenResponse1 || !tokenResponse2) {
Expand Down Expand Up @@ -689,7 +689,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {

const tokenResponse = await pca.handleRedirectPromise();
if (!tokenResponse) {
expect(tokenResponse).not.toBeNull();
expect(tokenResponse).not.toBe(null);
throw new Error("Token Response is null!"); // Throw to resolve Typescript complaints below
}
expect(callbackCalled).toBeTruthy();
Expand Down Expand Up @@ -2550,13 +2550,13 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {

it("getAccountByUsername returns null if account doesn't exist", () => {
const account = pca.getAccountByUsername("this-email-doesnt-exist@microsoft.com");
expect(account).toBeNull;
expect(account).toBe(null);
});

it("getAccountByUsername returns null if passed username is null", () => {
// @ts-ignore
const account = pca.getAccountByUsername(null);
expect(account).toBeNull;
expect(account).toBe(null);
});

it("getAccountByHomeId returns account specified", () => {
Expand All @@ -2566,13 +2566,13 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {

it("getAccountByHomeId returns null if passed id doesn't exist", () => {
const account = pca.getAccountByHomeId("this-id-doesnt-exist");
expect(account).toBeNull;
expect(account).toBe(null);
});

it("getAccountByHomeId returns null if passed id is null", () => {
// @ts-ignore
const account = pca.getAccountByHomeId(null);
expect(account).toBeNull;
expect(account).toBe(null);
});
});

Expand Down Expand Up @@ -2609,36 +2609,29 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {

it("active account is initialized as null", () => {
// Public client should initialze with active account set to null.
expect(pca.getActiveAccount()).toBeNull;
expect(pca.getActiveAccount()).toBe(null);
});

it(
"setActiveAccount() sets the active account local id value correctly",
() => {
expect((pca as any).activeLocalAccountId).toBeNull;
it("setActiveAccount() sets the active account local id value correctly", () => {
expect(pca.getActiveAccount()).toBe(null);
pca.setActiveAccount(testAccountInfo1);
expect((pca as any).activeLocalAccountId).toEqual(testAccountInfo1.localAccountId);
}
);
expect(pca.getActiveAccount()).toEqual(testAccountInfo1);
});

it(
"getActiveAccount looks up the current account values and returns them()",
() => {
it("getActiveAccount looks up the current account values and returns them", () => {
pca.setActiveAccount(testAccountInfo1);
const activeAccount1 = pca.getActiveAccount();
expect(activeAccount1).toEqual(testAccountInfo1);

const newName = "Ben Franklin";
window.sessionStorage.clear();
testAccountInfo1.name = newName;
testAccount1.name = newName;
const cacheKey = AccountEntity.generateAccountCacheKey(testAccountInfo1);
window.sessionStorage.setItem(cacheKey, JSON.stringify(testAccount1));

const activeAccount2 = pca.getActiveAccount();
expect(activeAccount2).toEqual(testAccountInfo1);
}
);
});

describe("activeAccount logout", () => {
const testAccountInfo2: AccountInfo = {
Expand Down Expand Up @@ -2666,26 +2659,22 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
});

it("Clears active account on logoutRedirect with no account", async () => {
expect((pca as any).activeLocalAccountId).toEqual(testAccountInfo1.localAccountId);
expect(pca.getActiveAccount()).toEqual(testAccountInfo1);
await pca.logoutRedirect();
expect(pca.getActiveAccount()).toBeNull;
expect(pca.getActiveAccount()).toBe(null);
});

it(
"Clears active account on logoutRedirect when the given account info matches",
async () => {
expect((pca as any).activeLocalAccountId).toEqual(testAccountInfo1.localAccountId);
it("Clears active account on logoutRedirect when the given account info matches", async () => {
expect(pca.getActiveAccount()).toEqual(testAccountInfo1);
await pca.logoutRedirect({
account: testAccountInfo1
});
expect(pca.getActiveAccount()).toBeNull;
expect(pca.getActiveAccount()).toBe(null);
}
);

it(
"Does not clear active account on logoutRedirect if given account object does not match",
async () => {
expect((pca as any).activeLocalAccountId).toEqual(testAccountInfo1.localAccountId);
it("Does not clear active account on logoutRedirect if given account object does not match", async () => {
expect(pca.getActiveAccount()).toEqual(testAccountInfo1);
await pca.logoutRedirect({
account: testAccountInfo2
});
Expand All @@ -2694,26 +2683,22 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
);

it("Clears active account on logoutPopup with no account", async () => {
expect((pca as any).activeLocalAccountId).toEqual(testAccountInfo1.localAccountId);
expect(pca.getActiveAccount()).toEqual(testAccountInfo1);
await pca.logoutPopup();
expect(pca.getActiveAccount()).toBeNull;
expect(pca.getActiveAccount()).toBe(null);
});

it(
"Clears active account on logoutPopup when the given account info matches",
async () => {
expect((pca as any).activeLocalAccountId).toEqual(testAccountInfo1.localAccountId);
it("Clears active account on logoutPopup when the given account info matches", async () => {
expect(pca.getActiveAccount()).toEqual(testAccountInfo1);
await pca.logoutPopup({
account: testAccountInfo1
});
expect(pca.getActiveAccount()).toBeNull;
expect(pca.getActiveAccount()).toBe(null);
}
);

it(
"Does not clear active account on logoutPopup if given account object does not match",
async () => {
expect((pca as any).activeLocalAccountId).toEqual(testAccountInfo1.localAccountId);
it("Does not clear active account on logoutPopup if given account object does not match", async () => {
expect(pca.getActiveAccount()).toEqual(testAccountInfo1);
await pca.logoutPopup({
account: testAccountInfo2
});
Expand Down
Loading

0 comments on commit a48ad51

Please sign in to comment.