Skip to content

Commit

Permalink
Nested App Auth fixes (#6672)
Browse files Browse the repository at this point in the history
Some minor fixes from initial NAA fix including:

- expiresOn AuthenticationResult contained the wrong value.
- expires_in should be used with timestamp before request was made to
server, to account for time deltas between servers.
- authenticationScheme was returning empty
- AccountInfo was not returning idTokenClaims, and should use id token
for properties as a fallback.
- Added authority parameter for server to host to return authority used
in the request.
- Avoid making call to getActiveAccount if host does not support it.
- Merge client capabilities and claims before sending request to host.
- uniqueId should be localAccountId to match implementation of existing
msal-browser.
- Removes toNaaSilentTokenRequest which had the same behavior as
existing toNaaTokenRequest.
- Handle state parameter locally in MSAL.js
  • Loading branch information
codexeon committed Nov 10, 2023
1 parent 9225d42 commit 6ac9a88
Show file tree
Hide file tree
Showing 13 changed files with 112 additions and 104 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Nested App Auth minor fixes (#6672)",
"packageName": "@azure/msal-browser",
"email": "dasau@microsoft.com",
"dependentChangeType": "patch"
}
11 changes: 8 additions & 3 deletions lib/msal-browser/src/controllers/NestedAppAuthController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
DEFAULT_CRYPTO_IMPLEMENTATION,
PerformanceEvents,
AccountFilter,
TimeUtils,
} from "@azure/msal-common";
import { ITokenCache } from "../cache/ITokenCache";
import { BrowserConfiguration } from "../config/Configuration";
Expand Down Expand Up @@ -134,13 +135,15 @@ export class NestedAppAuthController implements IController {
try {
const naaRequest =
this.nestedAppAuthAdapter.toNaaTokenRequest(request);
const reqTimestamp = TimeUtils.nowSeconds();
const response = await this.bridgeProxy.getTokenInteractive(
naaRequest
);
const result: AuthenticationResult =
this.nestedAppAuthAdapter.fromNaaTokenResponse(
naaRequest,
response
response,
reqTimestamp
);

this.operatingContext.setActiveAccount(result.account);
Expand Down Expand Up @@ -204,13 +207,15 @@ export class NestedAppAuthController implements IController {

try {
const naaRequest =
this.nestedAppAuthAdapter.toNaaSilentTokenRequest(request);
this.nestedAppAuthAdapter.toNaaTokenRequest(request);
const reqTimestamp = TimeUtils.nowSeconds();
const response = await this.bridgeProxy.getTokenSilent(naaRequest);

const result: AuthenticationResult =
this.nestedAppAuthAdapter.fromNaaTokenResponse(
naaRequest,
response
response,
reqTimestamp
);

this.operatingContext.setActiveAccount(result.account);
Expand Down
7 changes: 3 additions & 4 deletions lib/msal-browser/src/naa/AccountInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@
*/

export type AccountInfo = {
homeAccountId: string;
homeAccountId?: string;
environment: string;
tenantId: string;
tenantId?: string;
username: string;
localAccountId: string;
localAccountId?: string;
name?: string;
idToken?: string; // idTokenClaims can be parsed from idToken in MSAL.js
platformBrokerId?: string; // Used by WAM previous called nativeAccountId
idTokenClaims?: object;
client_info?: string;
};
8 changes: 4 additions & 4 deletions lib/msal-browser/src/naa/BridgeError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import { BridgeStatusCode } from "./BridgeStatusCode";

export type BridgeError = {
status: BridgeStatusCode;
code: string; // auth_flow_last_error such as invalid_grant
subError: string; // server_suberror_code such as consent_required
description: string;
properties: object; // additional telemetry info
code?: string; // auth_flow_last_error such as invalid_grant
subError?: string; // server_suberror_code such as consent_required
description?: string;
properties?: object; // additional telemetry info
};

export function isBridgeError(error: unknown): error is BridgeError {
Expand Down
8 changes: 6 additions & 2 deletions lib/msal-browser/src/naa/BridgeProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export class BridgeProxy implements IBridgeProxy {
static crypto: Crypto;
sdkName: string;
sdkVersion: string;
capabilities: BridgeCapabilities;
capabilities?: BridgeCapabilities;

/**
* initializeNestedAppAuthBridge - Initializes the bridge to the host app
Expand Down Expand Up @@ -162,6 +162,10 @@ export class BridgeProxy implements IBridgeProxy {
return this.sendRequest<AccountInfo>("GetActiveAccount", undefined);
}

public getHostCapabilities(): BridgeCapabilities | null {
return this.capabilities ?? null;
}

/**
* A method used to send a request to the bridge
* @param request A token request
Expand Down Expand Up @@ -206,7 +210,7 @@ export class BridgeProxy implements IBridgeProxy {
private constructor(
sdkName: string,
sdkVersion: string,
capabilities: BridgeCapabilities
capabilities?: BridgeCapabilities
) {
this.sdkName = sdkName;
this.sdkVersion = sdkVersion;
Expand Down
2 changes: 2 additions & 0 deletions lib/msal-browser/src/naa/IBridgeProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
AccountByLocalIdRequest,
AccountByUsernameRequest,
} from "./AccountRequests";
import { BridgeCapabilities } from "./BridgeCapabilities";
import { TokenRequest } from "./TokenRequest";
import { TokenResponse } from "./TokenResponse";

Expand All @@ -22,4 +23,5 @@ export interface IBridgeProxy {
| AccountByUsernameRequest
): Promise<AccountInfo>;
getActiveAccount(): Promise<AccountInfo>;
getHostCapabilities(): BridgeCapabilities | null;
}
2 changes: 1 addition & 1 deletion lib/msal-browser/src/naa/InitializeBridgeResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { BridgeCapabilities } from "./BridgeCapabilities";

export interface InitializeBridgeResponse {
capabilities: BridgeCapabilities;
capabilities?: BridgeCapabilities;
sdkName: string;
sdkVersion: string;
}
2 changes: 0 additions & 2 deletions lib/msal-browser/src/naa/TokenRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ export type TokenRequest = {
authority?: string;
scope: string;
correlationId: string;
prompt?: string; // Prompt used to identify interactive request
nonce?: string;
claims?: string;
state?: string;
Expand All @@ -18,7 +17,6 @@ export type TokenRequest = {
authenticationScheme?: string;
shrClaims?: string;
shrNonce?: string;
clientCapabilities?: string[]; // CP1 for CAE support
resourceRequestMethod?: string;
resourceRequestUri?: string;
extendedExpiryToken?: boolean;
Expand Down
3 changes: 1 addition & 2 deletions lib/msal-browser/src/naa/TokenResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@ import { AccountInfo } from "./AccountInfo";
export type TokenResponse = {
access_token: string;
account: AccountInfo;
client_info: string;
expires_in: number;
id_token: string;
properties: TokenResponseProperties | null;
scope: string;
state: string;
shr?: string; // token binding enabled at native layer it is the access token, not the signing keys
extendedLifetimeToken?: boolean;
authority?: string;
};

export type TokenResponseProperties = {
Expand Down
152 changes: 72 additions & 80 deletions lib/msal-browser/src/naa/mapping/NestedAppAuthAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,18 @@ import {
ClientConfigurationError,
InteractionRequiredAuthError,
ServerError,
TimeUtils,
ICrypto,
Logger,
AuthToken,
TokenClaims,
ClientAuthErrorCodes,
AuthenticationScheme,
RequestParameterBuilder,
StringUtils,
createClientAuthError,
} from "@azure/msal-common";
import { isBridgeError } from "../BridgeError";
import { BridgeStatusCode } from "../BridgeStatusCode";
import { SilentRequest } from "../../request/SilentRequest";
import { AuthenticationResult } from "../../response/AuthenticationResult";
import {} from "../../error/BrowserAuthErrorCodes";

Expand All @@ -46,41 +48,6 @@ export class NestedAppAuthAdapter {
this.logger = logger;
}

public toNaaSilentTokenRequest(request: SilentRequest): TokenRequest {
let extraParams: Map<string, string>;
if (request.extraQueryParameters === undefined) {
extraParams = new Map<string, string>();
} else {
extraParams = new Map<string, string>(
Object.entries(request.extraQueryParameters)
);
}
/**
* Need to get information about the client to populate request correctly
* For example: client id, client capabilities
*/
const tokenRequest: TokenRequest = {
userObjectId: request.account?.homeAccountId,
clientId: this.clientId,
authority: request.authority,
scope: request.scopes.join(" "),
correlationId:
request.correlationId !== undefined
? request.correlationId
: this.crypto.createNewGuid(),
prompt: request.prompt !== undefined ? request.prompt : "",
claims: request.claims !== undefined ? request.claims : "",
authenticationScheme:
request.authenticationScheme !== undefined
? request.authenticationScheme
: "",
clientCapabilities: this.clientCapabilities,
extraParameters: extraParams,
};

return tokenRequest;
}

public toNaaTokenRequest(
request: PopupRequest | RedirectRequest
): TokenRequest {
Expand All @@ -93,6 +60,11 @@ export class NestedAppAuthAdapter {
);
}

const requestBuilder = new RequestParameterBuilder();
const claims = requestBuilder.addClientCapabilitiesToClaims(
request.claims,
this.clientCapabilities
);
const tokenRequest: TokenRequest = {
userObjectId: request.account?.homeAccountId,
clientId: this.clientId,
Expand All @@ -101,16 +73,12 @@ export class NestedAppAuthAdapter {
correlationId:
request.correlationId !== undefined
? request.correlationId
: "",
prompt: request.prompt !== undefined ? request.prompt : "",
nonce: request.nonce !== undefined ? request.nonce : "",
claims: request.claims !== undefined ? request.claims : "",
state: request.state !== undefined ? request.state : "",
: this.crypto.createNewGuid(),
nonce: request.nonce,
claims: !StringUtils.isEmptyObj(claims) ? claims : undefined,
state: request.state,
authenticationScheme:
request.authenticationScheme !== undefined
? request.authenticationScheme
: "",
clientCapabilities: undefined,
request.authenticationScheme || AuthenticationScheme.BEARER,
extraParameters: extraParams,
};

Expand All @@ -119,36 +87,41 @@ export class NestedAppAuthAdapter {

public fromNaaTokenResponse(
request: TokenRequest,
response: TokenResponse
response: TokenResponse,
reqTimestamp: number
): AuthenticationResult {
if (!response.id_token || !response.access_token) {
throw createClientAuthError(ClientAuthErrorCodes.nullOrEmptyToken);
}

const expiresOn = new Date(
TimeUtils.nowSeconds() + (response.expires_in || 0) * 1000
(reqTimestamp + (response.expires_in || 0)) * 1000
);
const idTokenClaims = AuthToken.extractTokenClaims(
response.id_token,
this.crypto.base64Decode
);
const account = this.fromNaaAccountInfo(
response.account,
idTokenClaims
);

const account = this.fromNaaAccountInfo(response.account);

const authenticationResult: AuthenticationResult = {
authority: response.account.environment,
uniqueId: response.account.homeAccountId,
tenantId: response.account.tenantId,
authority: response.authority || account.environment,
uniqueId: account.localAccountId,
tenantId: account.tenantId,
scopes: response.scope.split(" "),
account: this.fromNaaAccountInfo(response.account),
account,
idToken: response.id_token !== undefined ? response.id_token : "",
idTokenClaims:
account.idTokenClaims !== undefined
? account.idTokenClaims
: {},
idTokenClaims,
accessToken: response.access_token,
fromCache: true,
expiresOn: expiresOn,
tokenType:
request.authenticationScheme !== undefined
? request.authenticationScheme
: "Bearer",
request.authenticationScheme || AuthenticationScheme.BEARER,
correlationId: request.correlationId,
requestId: "",
extExpiresOn: expiresOn,
state: response.state,
state: request.state,
};

return authenticationResult;
Expand Down Expand Up @@ -176,26 +149,41 @@ export class NestedAppAuthAdapter {
* authorityType?: string;
* };
*/
public fromNaaAccountInfo(fromAccount: NaaAccountInfo): MsalAccountInfo {
let tokenClaims: TokenClaims | undefined;
if (fromAccount.idToken !== undefined) {
tokenClaims = AuthToken.extractTokenClaims(
fromAccount.idToken,
this.crypto.base64Decode
);
} else {
tokenClaims = undefined;
}
public fromNaaAccountInfo(
fromAccount: NaaAccountInfo,
idTokenClaims?: TokenClaims
): MsalAccountInfo {
const effectiveIdTokenClaims =
idTokenClaims || (fromAccount.idTokenClaims as TokenClaims);

const localAccountId =
fromAccount.localAccountId ||
effectiveIdTokenClaims?.oid ||
effectiveIdTokenClaims?.sub ||
"";

const tenantId =
fromAccount.tenantId || effectiveIdTokenClaims?.tid || "";

const homeAccountId =
fromAccount.homeAccountId || `${localAccountId}.${tenantId}`;

const username =
fromAccount.username ||
effectiveIdTokenClaims?.preferred_username ||
"";

const name = fromAccount.name || effectiveIdTokenClaims?.name;

const account: MsalAccountInfo = {
homeAccountId: fromAccount.homeAccountId,
homeAccountId,
environment: fromAccount.environment,
tenantId: fromAccount.tenantId,
username: fromAccount.username,
localAccountId: fromAccount.localAccountId,
name: fromAccount.name,
tenantId,
username,
localAccountId,
name,
idToken: fromAccount.idToken,
idTokenClaims: tokenClaims,
idTokenClaims: effectiveIdTokenClaims,
};

return account;
Expand Down Expand Up @@ -233,7 +221,11 @@ export class NestedAppAuthAdapter {
ClientAuthErrorCodes.nestedAppAuthBridgeDisabled
);
case BridgeStatusCode.NESTED_APP_AUTH_UNAVAILABLE:
return new ClientAuthError(error.code, error.description);
return new ClientAuthError(
error.code ||
ClientAuthErrorCodes.nestedAppAuthBridgeDisabled,
error.description
);
case BridgeStatusCode.TRANSIENT_ERROR:
case BridgeStatusCode.PERSISTENT_ERROR:
return new ServerError(error.code, error.description);
Expand Down

0 comments on commit 6ac9a88

Please sign in to comment.