Skip to content

Commit

Permalink
Merge pull request #1490 from AzureAD/handleRedirectCallback-update-2.0
Browse files Browse the repository at this point in the history 
handleRedirectCallback Updates for MSAL 2.0
  • Loading branch information
Prithvi Kanherkar authored Apr 23, 2020
2 parents 4ef6263 + 7ae1b05 commit 9274fac
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 102 deletions.
4 changes: 2 additions & 2 deletions lib/msal-browser/src/app/Configuration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ export type Configuration = {
const DEFAULT_AUTH_OPTIONS: BrowserAuthOptions = {
clientId: "",
authority: null,
redirectUri: () => BrowserUtils.getDefaultRedirectUri(),
postLogoutRedirectUri: () => BrowserUtils.getDefaultRedirectUri(),
redirectUri: () => BrowserUtils.getCurrentUri(),
postLogoutRedirectUri: () => BrowserUtils.getCurrentUri(),
navigateToLoginRequestUrl: true
};

Expand Down
120 changes: 60 additions & 60 deletions lib/msal-browser/src/app/PublicClientApplication.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ export class PublicClientApplication {
// auth functions imported from @azure/msal-common module
private authModule: AuthorizationCodeModule;

// callback for error/token response
private authCallback: AuthCallback = null;

// Crypto interface implementation
private browserCrypto: CryptoOps;

Expand All @@ -39,6 +36,9 @@ export class PublicClientApplication {
// Network interface implementation
private networkClient: INetworkModule;

// Response promise
private tokenExchangePromise: Promise<TokenResponse>;

/**
* @constructor
* Constructor for the PublicClientApplication used to instantiate the PublicClientApplication object
Expand Down Expand Up @@ -88,13 +88,15 @@ export class PublicClientApplication {
networkInterface: this.networkClient,
storageInterface: this.browserStorage
});

// Check for hash and save response promise
this.tokenExchangePromise = this.handleRedirectResponse();
}

// #region Redirect Flow

/**
* Set the callback functions for the redirect flow to send back the success or error object, and process
* any redirect-related data.
* Process any redirect-related data and send back the success or error object.
* IMPORTANT: Please do not use this function when using the popup APIs, as it may break the response handling
* in the main window.
*
Expand All @@ -109,14 +111,14 @@ export class PublicClientApplication {
throw BrowserConfigurationAuthError.createInvalidCallbackObjectError(authCallback);
}

// Set the callback object.
this.authCallback = authCallback;

// Check if we need to navigate, otherwise handle hash
try {
await this.handleRedirectResponse();
const tokenResponse = await this.tokenExchangePromise;
if (tokenResponse) {
authCallback(null, tokenResponse);
}
} catch (err) {
this.authCallback(err);
authCallback(err);
}
}

Expand All @@ -125,23 +127,38 @@ export class PublicClientApplication {
* - if true, performs logic to cache and navigate
* - if false, handles hash string and parses response
*/
private async handleRedirectResponse(): Promise<void> {
private async handleRedirectResponse(): Promise<TokenResponse> {
// Get current location hash from window or cache.
const { location: { hash } } = window;
const cachedHash = this.browserStorage.getItem(TemporaryCacheKeys.URL_HASH);
const isResponseHash = UrlString.hashContainsKnownProperties(hash);

const loginRequestUrl = this.browserStorage.getItem(TemporaryCacheKeys.ORIGIN_URI);
const currentUrl = BrowserUtils.getCurrentUri();
if (loginRequestUrl === currentUrl) {
// We don't need to navigate - check for hash and prepare to process
if (isResponseHash) {
BrowserUtils.clearHash();
return this.handleHash(hash);
} else {
// Loaded page with no valid hash - pass in the value retrieved from cache, or null/empty string
return this.handleHash(cachedHash);
}
}

if (this.config.auth.navigateToLoginRequestUrl && isResponseHash && !BrowserUtils.isInIframe()) {
// Returned from authority using redirect - need to perform navigation before processing response
this.browserStorage.setItem(TemporaryCacheKeys.URL_HASH, hash);
const loginRequestUrl = this.browserStorage.getItem(TemporaryCacheKeys.ORIGIN_URI);

if (StringUtils.isEmpty(loginRequestUrl) || loginRequestUrl === "null") {
// Redirect to home page if login request url is null (real null or the string null)
this.authModule.logger.warning("Unable to get valid login request url from cache, redirecting to home page");
BrowserUtils.navigateWindow("/", true);
} else {
// Navigate to target url
BrowserUtils.navigateWindow(loginRequestUrl, true);
}
return;
return null;
}

if (!isResponseHash) {
Expand All @@ -154,23 +171,25 @@ export class PublicClientApplication {
BrowserUtils.clearHash();
return this.handleHash(hash);
}

return null;
}

/**
* Checks if hash exists and handles in window. Otherwise, cancel any current requests and continue.
* @param responseHash
* @param interactionHandler
*/
private async handleHash(responseHash: string): Promise<void> {
private async handleHash(responseHash: string): Promise<TokenResponse> {
const interactionHandler = new RedirectHandler(this.authModule, this.browserStorage);
if (!StringUtils.isEmpty(responseHash)) {
// Hash contains known properties - handle and return in callback
const tokenResponse = await interactionHandler.handleCodeResponse(responseHash);
this.authCallback(null, tokenResponse);
} else {
// There is no hash - assume we are in clean state and clear any current request data.
this.cleanRequest();
return interactionHandler.handleCodeResponse(responseHash);
}

// There is no hash - assume we are in clean state and clear any current request data.
this.cleanRequest();
return null;
}

/**
Expand All @@ -179,19 +198,8 @@ export class PublicClientApplication {
* @param {@link (AuthenticationParameters:type)}
*/
loginRedirect(request: AuthenticationParameters): void {
// block the reload if it occurred inside a hidden iframe
BrowserUtils.blockReloadInHiddenIframes();

// Check if callback has been set. If not, handleRedirectCallbacks wasn't called correctly.
if (!this.authCallback) {
throw BrowserConfigurationAuthError.createRedirectCallbacksNotSetError();
}

// Check if interaction is in progress. Throw error in callback and return if true.
if (this.interactionInProgress()) {
this.authCallback(BrowserAuthError.createInteractionInProgressError());
return;
}
// Preflight request
this.preflightRequest();

try {
// Create redirect interaction handler.
Expand All @@ -216,19 +224,8 @@ export class PublicClientApplication {
* To acquire only idToken, please pass clientId as the only scope in the Authentication Parameters
*/
acquireTokenRedirect(request: AuthenticationParameters): void {
// block the reload if it occurred inside a hidden iframe
BrowserUtils.blockReloadInHiddenIframes();

// Check if callback has been set. If not, handleRedirectCallbacks wasn't called correctly.
if (!this.authCallback) {
throw BrowserConfigurationAuthError.createRedirectCallbacksNotSetError();
}

// Check if interaction is in progress. Throw error in callback and return if true.
if (this.interactionInProgress()) {
this.authCallback(BrowserAuthError.createInteractionInProgressError());
return;
}
// Preflight request
this.preflightRequest();

try {
// Create redirect interaction handler.
Expand Down Expand Up @@ -257,13 +254,8 @@ export class PublicClientApplication {
* @returns {Promise.<TokenResponse>} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object
*/
async loginPopup(request: AuthenticationParameters): Promise<TokenResponse> {
// block the reload if it occurred inside a hidden iframe
BrowserUtils.blockReloadInHiddenIframes();

// Check if interaction is in progress. Throw error if true.
if (this.interactionInProgress()) {
throw BrowserAuthError.createInteractionInProgressError();
}
// Preflight request
this.preflightRequest();

// Create login url, which will by default append the client id scope to the call.
const navigateUrl = await this.authModule.createLoginUrl(request);
Expand All @@ -280,13 +272,8 @@ export class PublicClientApplication {
* @returns {Promise.<TokenResponse>} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object
*/
async acquireTokenPopup(request: AuthenticationParameters): Promise<TokenResponse> {
// block the reload if it occurred inside a hidden iframe
BrowserUtils.blockReloadInHiddenIframes();

// Check if interaction is in progress. Throw error if true.
if (this.interactionInProgress()) {
throw BrowserAuthError.createInteractionInProgressError();
}
// Preflight request
this.preflightRequest();

// Create acquire token url.
const navigateUrl = await this.authModule.createAcquireTokenUrl(request);
Expand Down Expand Up @@ -481,13 +468,26 @@ export class PublicClientApplication {
// #region Helpers

/**
* Helper to check whether interaction is in progress
* Helper to check whether interaction is in progress.
*/
private interactionInProgress(): boolean {
// Check whether value in cache is present and equal to expected value
return this.browserStorage.getItem(BrowserConstants.INTERACTION_STATUS_KEY) === BrowserConstants.INTERACTION_IN_PROGRESS_VALUE;
}

/**
* Helper to validate app environment before making a request.
*/
private preflightRequest(): void {
// block the reload if it occurred inside a hidden iframe
BrowserUtils.blockReloadInHiddenIframes();

// Check if interaction is in progress. Throw error if true.
if (this.interactionInProgress()) {
throw BrowserAuthError.createInteractionInProgressError();
}
}

/**
* Helper to remove interaction status and remove tempoarary request data.
*/
Expand Down
2 changes: 1 addition & 1 deletion lib/msal-browser/src/interaction_handler/RedirectHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export class RedirectHandler extends InteractionHandler {
// Navigate if valid URL
if (!StringUtils.isEmpty(requestUrl)) {
// Set interaction status in the library.
this.browserStorage.setItem(TemporaryCacheKeys.ORIGIN_URI, window.location.href);
this.browserStorage.setItem(TemporaryCacheKeys.ORIGIN_URI, BrowserUtils.getCurrentUri());
this.browserStorage.setItem(BrowserConstants.INTERACTION_STATUS_KEY, BrowserConstants.INTERACTION_IN_PROGRESS_VALUE);
this.authModule.logger.infoPii("Navigate to:" + requestUrl);
const isIframedApp = BrowserUtils.isInIframe();
Expand Down
2 changes: 1 addition & 1 deletion lib/msal-browser/src/utils/BrowserUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export class BrowserUtils {
/**
* Returns current window URL as redirect uri
*/
static getDefaultRedirectUri(): string {
static getCurrentUri(): string {
return window.location.href.split("?")[0].split("#")[0];
}

Expand Down
56 changes: 20 additions & 36 deletions lib/msal-browser/test/app/PublicClientApplication.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,22 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
expect(pca).to.be.not.null;
expect(pca instanceof PublicClientApplication).to.be.true;
});

it("navigates and caches hash if navigateToLoginRequestUri is true", () => {
window.location.hash = TEST_HASHES.TEST_SUCCESS_CODE_HASH;
window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, TEST_URIS.TEST_REDIR_URI);
sinon.stub(BrowserUtils, "getCurrentUri").returns("notAUri");
sinon.stub(BrowserUtils, "navigateWindow").callsFake((urlNavigate: string, noHistory?: boolean) => {
expect(noHistory).to.be.true;
expect(urlNavigate).to.be.eq(TEST_URIS.TEST_REDIR_URI);
});
pca = new PublicClientApplication({
auth: {
clientId: TEST_CONFIG.MSAL_CLIENT_ID
}
});
expect(window.sessionStorage.getItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.URL_HASH}`)).to.be.eq(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
});
});

describe("Redirect Flow Unit tests", () => {
Expand All @@ -75,22 +91,6 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
expect(window.sessionStorage.length).to.be.eq(0);
});

it("navigates and caches hash if navigateToLoginRequestUri is true", () => {
window.location.hash = TEST_HASHES.TEST_SUCCESS_CODE_HASH;
window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, TEST_URIS.TEST_REDIR_URI);
sinon.stub(BrowserUtils, "navigateWindow").callsFake((urlNavigate: string, noHistory?: boolean) => {
expect(noHistory).to.be.true;
expect(urlNavigate).to.be.eq(TEST_URIS.TEST_REDIR_URI);
});
pca = new PublicClientApplication({
auth: {
clientId: TEST_CONFIG.MSAL_CLIENT_ID
}
});
pca.handleRedirectCallback(authCallback);
expect(window.sessionStorage.getItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.URL_HASH}`)).to.be.eq(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
});

it("gets hash from cache and processes response", async () => {
const b64Encode = new Base64Encode();
window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, TEST_URIS.TEST_REDIR_URI);
Expand Down Expand Up @@ -277,18 +277,10 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {

describe("loginRedirect", () => {

it("loginRedirect throws an error if authCallback is not set", () => {
expect(() => pca.loginRedirect({})).to.throw(BrowserConfigurationAuthErrorMessage.noRedirectCallbacksSet.desc);
expect(() => pca.loginRedirect({})).to.throw(BrowserConfigurationAuthError);
});

it("loginRedirect throws an error if interaction is currently in progress", async () => {
await pca.handleRedirectCallback((authErr: AuthError, response: AuthResponse) => {
expect(authErr instanceof BrowserAuthError).to.be.true;
expect(authErr.errorMessage).to.be.eq(BrowserAuthErrorMessage.interactionInProgress.desc);
});
window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${BrowserConstants.INTERACTION_STATUS_KEY}`, BrowserConstants.INTERACTION_IN_PROGRESS_VALUE);
pca.loginRedirect({});
expect(() => pca.loginRedirect({})).to.throw(BrowserAuthErrorMessage.interactionInProgress.desc);
expect(() => pca.loginRedirect({})).to.throw(BrowserAuthError);
});

it("loginRedirect navigates to created login url", async () => {
Expand Down Expand Up @@ -325,18 +317,10 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {

describe("acquireTokenRedirect", () => {

it("acquireTokenRedirect throws an error if authCallback is not set", () => {
expect(() => pca.acquireTokenRedirect({})).to.throw(BrowserConfigurationAuthErrorMessage.noRedirectCallbacksSet.desc);
expect(() => pca.acquireTokenRedirect({})).to.throw(BrowserConfigurationAuthError);
});

it("acquireTokenRedirect throws an error if interaction is currently in progress", async () => {
await pca.handleRedirectCallback((authErr: AuthError, response: AuthResponse) => {
expect(authErr instanceof BrowserAuthError).to.be.true;
expect(authErr.errorMessage).to.be.eq(BrowserAuthErrorMessage.interactionInProgress.desc);
});
window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${BrowserConstants.INTERACTION_STATUS_KEY}`, BrowserConstants.INTERACTION_IN_PROGRESS_VALUE);
pca.acquireTokenRedirect({});
expect(() => pca.acquireTokenRedirect({})).to.throw(BrowserAuthErrorMessage.interactionInProgress.desc);
expect(() => pca.acquireTokenRedirect({})).to.throw(BrowserAuthError);
});

it("acquireTokenRedirect navigates to created login url", async () => {
Expand Down
4 changes: 2 additions & 2 deletions lib/msal-browser/test/utils/BrowserUtils.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ describe("BrowserUtils.ts Function Unit Tests", () => {
expect(BrowserUtils.isInIframe()).to.be.true;
});

it("getDefaultRedirectUri returns current location uri of browser", () => {
expect(BrowserUtils.getDefaultRedirectUri()).to.be.eq(TEST_URIS.TEST_REDIR_URI);
it("getCurrentUri() returns current location uri of browser", () => {
expect(BrowserUtils.getCurrentUri()).to.be.eq(TEST_URIS.TEST_REDIR_URI);
});

it("getBrowserNetworkClient() returns fetch client if available", () => {
Expand Down

0 comments on commit 9274fac

Please sign in to comment.