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

[msal-common]Combine Request module for node and browser components #1682

Merged
merged 18 commits into from
Jun 4, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
261 changes: 175 additions & 86 deletions lib/msal-browser/src/app/PublicClientApplication.ts

Large diffs are not rendered by default.

101 changes: 99 additions & 2 deletions lib/msal-browser/src/cache/BrowserStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/
import { ICacheStorage, Constants, PersistentCacheKeys, TemporaryCacheKeys, InMemoryCache } from "@azure/msal-common";
import { ICacheStorage, Constants, PersistentCacheKeys, InMemoryCache, StringUtils, AuthorizationCodeRequest, ICrypto } from "@azure/msal-common";
import { CacheOptions } from "../config/Configuration";
import { BrowserAuthError } from "../error/BrowserAuthError";
import { BrowserConfigurationAuthError } from "../error/BrowserConfigurationAuthError";
import { BrowserConstants } from "../utils/BrowserConstants";
import { BrowserConstants, TemporaryCacheKeys } from "../utils/BrowserConstants";

// Cookie life calculation (hours * minutes * seconds * ms)
const COOKIE_LIFE_MULTIPLIER = 24 * 60 * 60 * 1000;
Expand Down Expand Up @@ -274,6 +274,103 @@ export class BrowserStorage implements ICacheStorage {
}

/**
* Create authorityKey to cache authority
* @param state
*/
generateAuthorityKey(state: string): string {
return `${TemporaryCacheKeys.AUTHORITY}${Constants.RESOURCE_DELIM}${state}`;
}

/**
* Create Nonce key to cache nonce
* @param state
*/
generateNonceKey(state: string): string {
return `${TemporaryCacheKeys.NONCE_IDTOKEN}${Constants.RESOURCE_DELIM}${state}`;
}

/**
* Sets the cacheKey for and stores the authority information in cache
* @param state
* @param authority
*/
setAuthorityCache(authority: string, state: string): void {
// Cache authorityKey
const authorityKey = this.generateAuthorityKey(state);
this.setItem(authorityKey, authority);
}

/**
* Updates account, authority, and state in cache
* @param serverAuthenticationRequest
* @param account
*/
updateCacheEntries(state: string, nonce: string, authorityInstance: string): void {
// Cache the request state
this.setItem(TemporaryCacheKeys.REQUEST_STATE, state);

// Cache the nonce
this.setItem(this.generateNonceKey(state), nonce);

// Cache authorityKey
this.setAuthorityCache(authorityInstance, state);
}

/**
* Reset all temporary cache items
* @param state
*/
resetRequestCache(state: string): void {
// check state and remove associated cache items
this.getKeys().forEach(key => {
if (!StringUtils.isEmpty(state) && key.indexOf(state) !== -1) {
const splitKey = key.split(Constants.RESOURCE_DELIM);
const keyState = splitKey.length > 1 ? splitKey[splitKey.length-1]: null;
if (keyState === state) {
this.removeItem(key);
}
}
});

// delete generic interactive request parameters
this.removeItem(TemporaryCacheKeys.REQUEST_STATE);
this.removeItem(TemporaryCacheKeys.REQUEST_PARAMS);
this.removeItem(TemporaryCacheKeys.ORIGIN_URI);
}

cleanRequest(): void {
// Interaction is completed - remove interaction status.
this.removeItem(BrowserConstants.INTERACTION_STATUS_KEY);
const cachedState = this.getItem(TemporaryCacheKeys.REQUEST_STATE);
this.resetRequestCache(cachedState || "");
}

cacheCodeRequest(authCodeRequest: AuthorizationCodeRequest, browserCrypto: ICrypto): void {
this.setItem(TemporaryCacheKeys.REQUEST_PARAMS, browserCrypto.base64Encode(JSON.stringify(authCodeRequest)));
}

/**
* Gets the token exchange parameters from the cache. Throws an error if nothing is found.
*/
getCachedRequest(state: string, browserCrypto: ICrypto): AuthorizationCodeRequest {
try {
// Get token request from cache and parse as TokenExchangeParameters.
const encodedTokenRequest = this.getItem(TemporaryCacheKeys.REQUEST_PARAMS);
const parsedRequest = JSON.parse(browserCrypto.base64Decode(encodedTokenRequest)) as AuthorizationCodeRequest;
this.removeItem(TemporaryCacheKeys.REQUEST_PARAMS);
// Get cached authority and use if no authority is cached with request.
if (StringUtils.isEmpty(parsedRequest.authority)) {
const authorityKey: string = this.generateAuthorityKey(state);
const cachedAuthority: string = this.getItem(authorityKey);
parsedRequest.authority = cachedAuthority;
}
return parsedRequest;
} catch (err) {
throw BrowserAuthError.createTokenRequestCacheError(err);
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

+1 for this. All browser specific cache handling now is clearly demarcated between msal-common and msal-browser


/*
* Dummy implementation for interface compat - will change after BrowserCacheMigration
* @param key
* @param value
Expand Down
4 changes: 2 additions & 2 deletions lib/msal-browser/src/config/Configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/
import { SystemOptions, LoggerOptions, INetworkModule, LogLevel, DEFAULT_SYSTEM_OPTIONS } from "@azure/msal-common";
import { SystemOptions, LoggerOptions, INetworkModule, LogLevel, DEFAULT_SYSTEM_OPTIONS, Constants } from "@azure/msal-common";
import { BrowserUtils } from "../utils/BrowserUtils";
import { BrowserConstants } from "../utils/BrowserConstants";

Expand Down Expand Up @@ -64,7 +64,7 @@ export type Configuration = {
// Default auth options for browser
const DEFAULT_AUTH_OPTIONS: BrowserAuthOptions = {
clientId: "",
authority: null,
authority: `${Constants.DEFAULT_AUTHORITY}/`,
knownAuthorities: [],
redirectUri: () => BrowserUtils.getCurrentUri(),
postLogoutRedirectUri: () => BrowserUtils.getCurrentUri(),
Expand Down
15 changes: 14 additions & 1 deletion lib/msal-browser/src/error/BrowserAuthError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ export const BrowserAuthErrorMessage = {
silentPromptValueError: {
code: "silent_prompt_value_error",
desc: "The value given for the prompt value is not valid for silent requests - must be set to 'none'."
}
},
tokenRequestCacheError: {
code: "token_request_cache_error",
desc: "The token request could not be fetched from the cache correctly."
},
};

/**
Expand Down Expand Up @@ -215,4 +219,13 @@ export class BrowserAuthError extends AuthError {
static createSilentPromptValueError(givenPrompt: string): BrowserAuthError {
return new BrowserAuthError(BrowserAuthErrorMessage.silentPromptValueError.code, `${BrowserAuthErrorMessage.silentPromptValueError.desc} Given value: ${givenPrompt}`);
}

/**
* Creates an error thrown when the token request could not be retrieved from the cache
* @param errDetail
*/
static createTokenRequestCacheError(errDetail: string): BrowserAuthError {
return new BrowserAuthError(BrowserAuthErrorMessage.tokenRequestCacheError.code,
`${BrowserAuthErrorMessage.tokenRequestCacheError.desc} Error Detail: ${errDetail}`);
}
}
4 changes: 1 addition & 3 deletions lib/msal-browser/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,8 @@ export type { AuthCallback } from "./types/AuthCallback";
// Common Object Formats
export {
// Request
AuthenticationParameters,
TokenExchangeParameters,
TokenRenewParameters,
// Response
AuthResponse,
TokenResponse,
// Error
InteractionRequiredAuthError,
Expand Down
25 changes: 20 additions & 5 deletions lib/msal-browser/src/interaction_handler/InteractionHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/
import { SPAClient, TokenResponse, StringUtils } from "@azure/msal-common";
import { SPAClient, TokenResponse, StringUtils, AuthorizationCodeRequest, ProtocolUtils } from "@azure/msal-common";
import { BrowserStorage } from "../cache/BrowserStorage";
import { BrowserAuthError } from "../error/BrowserAuthError";
import { TemporaryCacheKeys } from "../utils/BrowserConstants";

/**
* Abstract class which defines operations for a browser interaction handling class.
Expand All @@ -13,6 +14,7 @@ export abstract class InteractionHandler {

protected authModule: SPAClient;
protected browserStorage: BrowserStorage;
protected authCodeRequest: AuthorizationCodeRequest;

constructor(authCodeModule: SPAClient, storageImpl: BrowserStorage) {
this.authModule = authCodeModule;
Expand All @@ -23,7 +25,7 @@ export abstract class InteractionHandler {
* Function to enable user interaction.
* @param requestUrl
*/
abstract initiateAuthRequest(requestUrl: string): Window | Promise<HTMLIFrameElement>;
abstract initiateAuthRequest(requestUrl: string, authCodeRequest: AuthorizationCodeRequest): Window | Promise<HTMLIFrameElement>;

/**
* Function to handle response parameters from hash.
Expand All @@ -35,10 +37,23 @@ export abstract class InteractionHandler {
throw BrowserAuthError.createEmptyHashError(locationHash);
}

// Get cached items
const requestState = this.browserStorage.getItem(TemporaryCacheKeys.REQUEST_STATE);
const cachedNonceKey = this.browserStorage.generateNonceKey(requestState);
const cachedNonce = this.browserStorage.getItem(cachedNonceKey);

// Handle code response.
const codeResponse = this.authModule.handleFragmentResponse(locationHash);

const authCode = this.authModule.handleFragmentResponse(locationHash, requestState);

// Assign code to request
this.authCodeRequest.code = authCode;

// Extract user state.
const userState = ProtocolUtils.getUserRequestState(requestState);

// Acquire token with retrieved code.
return this.authModule.acquireToken(codeResponse);
const tokenResponse = await this.authModule.acquireToken(this.authCodeRequest, userState, cachedNonce);
this.browserStorage.cleanRequest();
return tokenResponse;
}
}
9 changes: 5 additions & 4 deletions lib/msal-browser/src/interaction_handler/PopupHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/
import { UrlString, StringUtils, Constants, SPAClient } from "@azure/msal-common";
import { UrlString, StringUtils, Constants, SPAClient, AuthorizationCodeRequest } from "@azure/msal-common";
import { InteractionHandler } from "./InteractionHandler";
import { BrowserAuthError } from "../error/BrowserAuthError";
import { BrowserConstants } from "../utils/BrowserConstants";
Expand All @@ -27,9 +27,11 @@ export class PopupHandler extends InteractionHandler {
* Opens a popup window with given request Url.
* @param requestUrl
*/
initiateAuthRequest(requestUrl: string): Window {
initiateAuthRequest(requestUrl: string, authCodeRequest: AuthorizationCodeRequest): Window {
// Check that request url is not empty.
if (!StringUtils.isEmpty(requestUrl)) {
// Save auth code request
this.authCodeRequest = authCodeRequest;
// Set interaction status in the library.
this.browserStorage.setItem(BrowserConstants.INTERACTION_STATUS_KEY, BrowserConstants.INTERACTION_IN_PROGRESS_VALUE);
this.authModule.logger.infoPii("Navigate to:" + requestUrl);
Expand Down Expand Up @@ -150,8 +152,7 @@ export class PopupHandler extends InteractionHandler {
* Event callback to unload main window.
*/
unloadWindow(e: Event): void {
this.authModule.cancelRequest();
this.browserStorage.removeItem(BrowserConstants.INTERACTION_STATUS_KEY);
this.browserStorage.cleanRequest();
this.currentWindow.close();
// Guarantees browser unload will happen, so no other errors will be thrown.
delete e["returnValue"];
Expand Down
28 changes: 22 additions & 6 deletions lib/msal-browser/src/interaction_handler/RedirectHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/
import { StringUtils, TemporaryCacheKeys, TokenResponse } from "@azure/msal-common";
import { StringUtils, TokenResponse, AuthorizationCodeRequest, ICrypto, ProtocolUtils } from "@azure/msal-common";
import { InteractionHandler } from "./InteractionHandler";
import { BrowserAuthError } from "../error/BrowserAuthError";
import { BrowserConstants } from "../utils/BrowserConstants";
import { BrowserConstants, TemporaryCacheKeys } from "../utils/BrowserConstants";
import { BrowserUtils } from "../utils/BrowserUtils";

export class RedirectHandler extends InteractionHandler {
Expand All @@ -14,12 +14,13 @@ export class RedirectHandler extends InteractionHandler {
* Redirects window to given URL.
* @param urlNavigate
*/
initiateAuthRequest(requestUrl: string): Window {
initiateAuthRequest(requestUrl: string, authCodeRequest: AuthorizationCodeRequest, browserCrypto?: ICrypto): Window {
// Navigate if valid URL
if (!StringUtils.isEmpty(requestUrl)) {
// Set interaction status in the library.
this.browserStorage.setItem(TemporaryCacheKeys.ORIGIN_URI, BrowserUtils.getCurrentUri());
this.browserStorage.setItem(BrowserConstants.INTERACTION_STATUS_KEY, BrowserConstants.INTERACTION_IN_PROGRESS_VALUE);
this.browserStorage.cacheCodeRequest(authCodeRequest, browserCrypto);
this.authModule.logger.infoPii("Navigate to:" + requestUrl);
const isIframedApp = BrowserUtils.isInIframe();
if (isIframedApp) {
Expand All @@ -41,19 +42,34 @@ export class RedirectHandler extends InteractionHandler {
* Handle authorization code response in the window.
* @param hash
*/
async handleCodeResponse(locationHash: string): Promise<TokenResponse> {
async handleCodeResponse(locationHash: string, browserCrypto?: ICrypto): Promise<TokenResponse> {
// Check that location hash isn't empty.
if (StringUtils.isEmpty(locationHash)) {
throw BrowserAuthError.createEmptyHashError(locationHash);
}

// Interaction is completed - remove interaction status.
this.browserStorage.removeItem(BrowserConstants.INTERACTION_STATUS_KEY);

// Get cached items
const requestState = this.browserStorage.getItem(TemporaryCacheKeys.REQUEST_STATE);
const cachedNonceKey = this.browserStorage.generateNonceKey(requestState);
const cachedNonce = this.browserStorage.getItem(cachedNonceKey);
this.authCodeRequest = this.browserStorage.getCachedRequest(requestState, browserCrypto);

// Handle code response.
const codeResponse = this.authModule.handleFragmentResponse(locationHash);
const authCode = this.authModule.handleFragmentResponse(locationHash, requestState);
this.authCodeRequest.code = authCode;

// Hash was processed successfully - remove from cache
this.browserStorage.removeItem(TemporaryCacheKeys.URL_HASH);

// Extract user state.
const userState = ProtocolUtils.getUserRequestState(requestState);

// Acquire token with retrieved code.
return this.authModule.acquireToken(codeResponse);
const tokenResponse = await this.authModule.acquireToken(this.authCodeRequest, userState, cachedNonce);
Copy link
Member Author

@sameerag sameerag May 27, 2020

Choose a reason for hiding this comment

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

let's discuss nonce, I wonder why this is not applicable to node. I do understand that redirect use case for msal-browser poses unique challenges w.r.t application memory. Lets revisit this.

this.browserStorage.cleanRequest();
return tokenResponse;
}
}
6 changes: 4 additions & 2 deletions lib/msal-browser/src/interaction_handler/SilentHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/
import { UrlString, SPAClient, StringUtils } from "@azure/msal-common";
import { UrlString, SPAClient, StringUtils, AuthorizationCodeRequest } from "@azure/msal-common";
import { InteractionHandler } from "./InteractionHandler";
import { BrowserConstants } from "../utils/BrowserConstants";
import { BrowserAuthError } from "../error/BrowserAuthError";
Expand All @@ -21,12 +21,14 @@ export class SilentHandler extends InteractionHandler {
* @param urlNavigate
* @param userRequestScopes
*/
async initiateAuthRequest(requestUrl: string, userRequestScopes?: string): Promise<HTMLIFrameElement> {
async initiateAuthRequest(requestUrl: string, authCodeRequest: AuthorizationCodeRequest, userRequestScopes?: string): Promise<HTMLIFrameElement> {
if (StringUtils.isEmpty(requestUrl)) {
// Throw error if request URL is empty.
this.authModule.logger.info("Navigate url is empty");
throw BrowserAuthError.createEmptyNavigationUriError();
}
// Save auth code request
this.authCodeRequest = authCodeRequest;
const frameName = userRequestScopes ? `msalTokenFrame${userRequestScopes}` : "msalTokenFrame";
return this.loadFrameTimeout ? await this.loadFrame(requestUrl, frameName) : this.loadFrameSync(requestUrl, frameName);
}
Expand Down
4 changes: 2 additions & 2 deletions lib/msal-browser/src/types/AuthCallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/
import { AuthError, AuthResponse } from "@azure/msal-common";
import { AuthError, TokenResponse } from "@azure/msal-common";

/**
* A type alias for an authResponseCallback function.
* {@link (authResponseCallback:type)}
* @param authErr error created for failure cases
* @param response response containing token strings in success cases, or just state value in error cases
*/
export type AuthCallback = (authErr: AuthError, response?: AuthResponse) => void;
export type AuthCallback = (authErr: AuthError, response?: TokenResponse) => void;
16 changes: 16 additions & 0 deletions lib/msal-browser/src/utils/BrowserConstants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,19 @@ export enum HTTP_REQUEST_TYPE {
GET = "GET",
POST = "POST"
};

/**
* Temporary cache keys for MSAL, deleted after any request.
*/
export enum TemporaryCacheKeys {
AUTHORITY = "authority",
ACQUIRE_TOKEN_ACCOUNT = "acquireToken.account",
SESSION_STATE = "session.state",
REQUEST_STATE = "request.state",
NONCE_IDTOKEN = "nonce.idtoken",
ORIGIN_URI = "request.origin",
RENEW_STATUS = "token.renew.status",
URL_HASH = "urlHash",
REQUEST_PARAMS = "request.params",
SCOPES = "scopes"
}