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

Update current account and active account behavior in NAA apps #7390

Merged
merged 17 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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": "Add support for forceRefresh and set default cache policy in NAA flows",
"packageName": "@azure/msal-browser",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Update current account and active account behavior in NAA apps",
"packageName": "@azure/msal-browser",
"email": "[email protected]",
"dependentChangeType": "patch"
}
5 changes: 4 additions & 1 deletion lib/msal-browser/docs/accounts.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ As of `@azure/[email protected]`, all login hint values can be used to search f
- `username` account property
- `upn` ID token claim


> Note: All attributes above can be passed into the account filter as the `loginHint` property. The account filter will also accept the `username` attribute as `username`, and will yield a more performant search.

#### Using `login_hint` claim
Expand Down Expand Up @@ -158,6 +157,10 @@ function getAccessToken() {

Note: As of version 2.16.0 the active account is stored in the cache location configured on your `PublicClientApplication` instance. If you are using a previous version the active account is stored in-memory and thus must be reset on every page load.

### Nested App Authentication

For NAA applications, we consider `setActiveAccount()` and `getActiveAccount()` as NO-OP APIs. Though we allow users to set and get active accounts, they are actively ignored since the NAA application is always expected to have _one_ account and the account is supplied by the host application with `accountContext`. In the future when multiple accounts are supported across the hubs, we expect this to change.

## Notes

- The current msal-browser default [sample](../../../samples/msal-browser-samples/VanillaJSTestApp2.0) has a working single account scenario.
Expand Down
1 change: 1 addition & 0 deletions lib/msal-browser/docs/initialization.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ Please note the below guidance before opting in for Nested app authentication:
- `supportsNestedAppAuth` in MSAL Browser configuration will be deprecated in the next major version. Please use `createNestablePublicClientApplication` instead.
- `createNestablePublicClientApplication` will fall back to `createStandardPublicClientApplication` if nested app bridge is unavailable or the Hub is not configured to support nested app authentication.
- If an application does not want to be Nested App, it should use `createStandardPublicClientApplication` instead.
- Certain account lookup APIs are not supported in NAA apps, please refer to [active accounts](./accounts.md#active-account-apis).

## Initializing the PublicClientApplication object

Expand Down
7 changes: 6 additions & 1 deletion lib/msal-browser/src/app/PublicClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,12 @@ export async function createNestablePublicClientApplication(

if (nestedAppAuth.isAvailable()) {
const controller = new NestedAppAuthController(nestedAppAuth);
return new PublicClientApplication(configuration, controller);
const nestablePCA = new PublicClientApplication(
configuration,
controller
);
await nestablePCA.initialize();
return nestablePCA;
}

return createStandardPublicClientApplication(configuration);
Expand Down
52 changes: 36 additions & 16 deletions lib/msal-browser/src/controllers/NestedAppAuthController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import {
} from "../cache/BrowserCacheManager.js";
import { ClearCacheRequest } from "../request/ClearCacheRequest.js";
import * as AccountManager from "../cache/AccountManager.js";
import { AccountContext } from "../naa/BridgeAccountContext.js";
import { InitializeApplicationRequest } from "../request/InitializeApplicationRequest.js";
import { createNewGuid } from "../crypto/BrowserCrypto.js";

Expand Down Expand Up @@ -84,6 +85,9 @@ export class NestedAppAuthController implements IController {
// NestedAppAuthAdapter
protected readonly nestedAppAuthAdapter: NestedAppAuthAdapter;

// currentAccount for NAA apps
protected currentAccountContext: AccountContext | null;

constructor(operatingContext: NestedAppOperatingContext) {
this.operatingContext = operatingContext;
const proxy = this.operatingContext.getBridgeProxy();
Expand Down Expand Up @@ -134,15 +138,7 @@ export class NestedAppAuthController implements IController {

// Set the active account if available
const accountContext = this.bridgeProxy.getAccountContext();
if (accountContext) {
const cachedAccount = AccountManager.getAccount(
accountContext,
this.logger,
this.browserStorage
);

AccountManager.setActiveAccount(cachedAccount, this.browserStorage);
}
this.currentAccountContext = accountContext ? accountContext : null;
}

/**
Expand Down Expand Up @@ -229,7 +225,13 @@ export class NestedAppAuthController implements IController {
// cache the tokens in the response
await this.hydrateCache(result, request);

this.browserStorage.setActiveAccount(result.account);
// cache the account context in memory after successful token fetch
this.currentAccountContext = {
homeAccountId: result.account.homeAccountId,
environment: result.account.environment,
tenantId: result.account.tenantId,
};

this.eventHandler.emitEvent(
EventType.ACQUIRE_TOKEN_SUCCESS,
InteractionType.Popup,
Expand Down Expand Up @@ -323,7 +325,13 @@ export class NestedAppAuthController implements IController {
// cache the tokens in the response
await this.hydrateCache(result, request);

this.browserStorage.setActiveAccount(result.account);
// cache the account context in memory after successful token fetch
this.currentAccountContext = {
homeAccountId: result.account.homeAccountId,
environment: result.account.environment,
tenantId: result.account.tenantId,
};

this.eventHandler.emitEvent(
EventType.ACQUIRE_TOKEN_SUCCESS,
InteractionType.Silent,
Expand Down Expand Up @@ -381,8 +389,20 @@ export class NestedAppAuthController implements IController {
return null;
}

// if the request has forceRefresh, we cannot look up in the cache
if (request.forceRefresh) {
this.logger.verbose(
"forceRefresh is set to true, skipping cache lookup"
);
return null;
}

// respect cache lookup policy
let result: AuthenticationResult | null = null;
if (!request.cacheLookupPolicy) {
request.cacheLookupPolicy = CacheLookupPolicy.Default;
}

switch (request.cacheLookupPolicy) {
case CacheLookupPolicy.Default:
case CacheLookupPolicy.AccessToken:
Expand Down Expand Up @@ -433,16 +453,16 @@ export class NestedAppAuthController implements IController {
private async acquireTokenFromCacheInternal(
request: SilentRequest
): Promise<AuthenticationResult | null> {
const accountContext = this.bridgeProxy.getAccountContext();
let currentAccount = null;
// always prioritize the account context from the bridge
const accountContext =
this.bridgeProxy.getAccountContext() || this.currentAccountContext;
let currentAccount: AccountInfo | null = null;
if (accountContext) {
const hubAccount = AccountManager.getAccount(
currentAccount = AccountManager.getAccount(
accountContext,
this.logger,
this.browserStorage
);
// always prioritize for hub account context, the reqirement of `request.account` will be removed soon
currentAccount = hubAccount || request.account;
}

// fall back to brokering if no cached account is found
Expand Down
64 changes: 54 additions & 10 deletions lib/msal-browser/test/controllers/NestedAppAuthController.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,7 @@ import {
import BridgeProxy from "../../src/naa/BridgeProxy.js";
import { NestedAppAuthAdapter } from "../../src/naa/mapping/NestedAppAuthAdapter.js";
import { CryptoOps } from "../../src/crypto/CryptoOps.js";

const cacheConfig = {
temporaryCacheLocation: BrowserCacheLocation.SessionStorage,
cacheLocation: BrowserCacheLocation.SessionStorage,
storeAuthStateInCookie: false,
secureCookies: false,
cacheMigrationEnabled: false,
claimsBasedCachingEnabled: false,
};
import exp from "constants";

function stubProvider(config: Configuration) {
const browserEnvironment = typeof window !== "undefined";
Expand Down Expand Up @@ -133,6 +125,7 @@ describe("NestedAppAuthController.ts Class Unit Tests", () => {
expect(pca instanceof PublicClientApplication).toBeTruthy();
// @ts-ignore
expect(pca.controller).toBeInstanceOf(NestedAppAuthController);
expect(pca.getActiveAccount()).toBeNull();
done();
});
});
Expand Down Expand Up @@ -177,19 +170,25 @@ describe("NestedAppAuthController.ts Class Unit Tests", () => {
);
});

it("acquireTokenSilent calls acquireTokenFromCach with no cache policy set", async () => {
it("acquireTokenSilent calls acquireTokenFromCache with no cache policy set", async () => {
jest.spyOn(
NestedAppAuthController.prototype as any,
"acquireTokenFromCache"
).mockResolvedValue(testTokenResponse);

const setActiveAccountSpy = jest.spyOn(
PublicClientApplication.prototype,
"setActiveAccount"
);

const response = await pca.acquireTokenSilent({
scopes: [NAA_SCOPE],
account: testAccount,
state: "test-state",
});
expect(response?.idToken).not.toBeNull();
expect(response).toEqual(testTokenResponse);
expect(setActiveAccountSpy).toHaveBeenCalledTimes(0);
});

it("acquireTokenSilent looks for cache first if cache policy prefers it", async () => {
Expand All @@ -198,6 +197,19 @@ describe("NestedAppAuthController.ts Class Unit Tests", () => {
"acquireTokenFromCache"
).mockResolvedValue(testTokenResponse);

const activeAccount = {
homeAccountId: NAA_APP_CONSTANTS.altHomeAccountId,
localAccountId: NAA_APP_CONSTANTS.altLocalAccountId,
environment: NAA_APP_CONSTANTS.environment,
tenantId: NAA_APP_CONSTANTS.tenantId,
username: NAA_APP_CONSTANTS.altUsername,
};

jest.spyOn(
PublicClientApplication.prototype as any,
"setActiveAccount"
).mockResolvedValue(activeAccount);

const response = await pca.acquireTokenSilent({
scopes: [NAA_SCOPE],
account: testAccount,
Expand All @@ -206,6 +218,9 @@ describe("NestedAppAuthController.ts Class Unit Tests", () => {
});
expect(response?.idToken).not.toBeNull();
expect(response).toEqual(testTokenResponse);
expect(response.account.localAccountId).toEqual(
NAA_APP_CONSTANTS.localAccountId
);
});

it("acquireTokenSilent sends the request to bridge if cache policy prefers it", async () => {
Expand All @@ -223,9 +238,38 @@ describe("NestedAppAuthController.ts Class Unit Tests", () => {
SILENT_TOKEN_RESPONSE,
0
);

const hydrateCacheSpy = jest.spyOn(
NestedAppAuthController.prototype as any,
"hydrateCache"
);

const response = await pca.acquireTokenSilent(testRequest);

expect(response.accessToken).toEqual(testResponse.accessToken);
expect(hydrateCacheSpy).toHaveBeenCalledTimes(1);
});

it("acquireTokenSilent ignores cache if forceRefresh is on", async () => {
mockBridge.addAuthResultResponse("GetToken", SILENT_TOKEN_RESPONSE);

const testRequest = {
scopes: [NAA_SCOPE],
account: testAccount,
forceRefresh: true,
correlationId: NAA_CORRELATION_ID,
};

const testTokenResponse = nestedAppAuthAdapter.fromNaaTokenResponse(
nestedAppAuthAdapter.toNaaTokenRequest(testRequest),
SILENT_TOKEN_RESPONSE,
0
);

const response = await pca.acquireTokenSilent(testRequest);

expect(response?.idToken).not.toBeNull();
expect(response.accessToken).toEqual(testTokenResponse.accessToken);
});

it("acquireTokenSilent sends the request to bridge if cache misses", async () => {
Expand Down
4 changes: 4 additions & 0 deletions lib/msal-browser/test/naa/BridgeProxyConstants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,14 @@ export const SILENT_TOKEN_RESPONSE: AuthResult = {
export const NAA_APP_CONSTANTS = {
homeAccountId:
"2995ae49-d9dd-409d-8d62-ba969ce58a81.51178b70-16cc-41b5-bef1-ae1808139065",
altHomeAccountId:
"c691463b-b280-4755-8fd1-486f6e9c6f53.73541b02-bd0d-4c53-ad05-b0cf19ab7d40",
localAccountId: "2995ae49-d9dd-409d-8d62-ba969ce58a81",
altLocalAccountId: "c691463b-b280-4755-8fd1-486f6e9c6f53",
environment: "login.microsoftonline.com",
tenantId: "51178b70-16cc-41b5-bef1-ae1808139065",
username: "[email protected]",
altUsername: "sampleacccounto.onmicrosoft.com",
idTokenClaims: {
ver: "2.0",
iss: "https://login.microsoftonline.com/3338040d-6c67-4c5b-b112-36a304b66dad/v2.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,12 @@ export async function run() {
// Config object to be passed to Msal on creation
const msalConfig = {
auth: {
clientId: "a076930c-cfc9-4ebd-9607-7963bccbf666",
authority: "https://login.microsoftonline.com/common",
supportsNestedAppAuth: true
clientId: "58f921c3-84a4-4da8-8544-65acb867aaf4",
authority: "https://login.microsoftonline.com/common",
supportsNestedAppAuth: true
},
cache: {
cacheLocation: 'localStorage'
},
};

Expand Down
Loading