From 1b6e904a0ecd847eaf410f50755fb6ed3b97dcd3 Mon Sep 17 00:00:00 2001 From: shylasummers Date: Fri, 7 Jul 2023 11:09:05 -0700 Subject: [PATCH] Addressed failing tests --- lib/msal-browser/src/config/Configuration.ts | 4 +-- .../src/controllers/StandardController.ts | 28 +++++++++---------- .../src/interaction_client/PopupClient.ts | 19 ++++++------- .../src/interaction_client/RedirectClient.ts | 2 +- .../StandardInteractionClient.ts | 4 +-- .../test/app/PublicClientApplication.spec.ts | 4 +-- .../interaction_client/PopupClient.spec.ts | 2 +- .../StandardInteractionClient.spec.ts | 3 +- lib/msal-common/src/authority/Authority.ts | 2 +- lib/msal-common/src/authority/OIDCOptions.ts | 3 +- .../src/client/AuthorizationCodeClient.ts | 4 +-- 11 files changed, 38 insertions(+), 37 deletions(-) diff --git a/lib/msal-browser/src/config/Configuration.ts b/lib/msal-browser/src/config/Configuration.ts index 880b04c63a..c046e6ca2f 100644 --- a/lib/msal-browser/src/config/Configuration.ts +++ b/lib/msal-browser/src/config/Configuration.ts @@ -75,10 +75,10 @@ export type BrowserAuthOptions = { * Enum that represents the protocol that msal follows. Used for configuring proper endpoints. */ protocolMode?: ProtocolMode; - /** + /** * Enum that configures options for the OIDC protocol mode. */ - OIDCOptions?: OIDCOptions | null; + OIDCOptions?: OIDCOptions | null; /** * Enum that represents the Azure Cloud to use. */ diff --git a/lib/msal-browser/src/controllers/StandardController.ts b/lib/msal-browser/src/controllers/StandardController.ts index 9aec5f8a7c..1f0dfafd83 100644 --- a/lib/msal-browser/src/controllers/StandardController.ts +++ b/lib/msal-browser/src/controllers/StandardController.ts @@ -27,7 +27,6 @@ import { ClientConfigurationError, ProtocolMode, ServerResponseType, - OIDC_DEFAULT_SCOPES, } from "@azure/msal-common"; import { BrowserCacheManager, @@ -331,25 +330,26 @@ export class StandardController implements IController { ); // Throw an error if user has set OIDCOptions without being in OIDC protocol mode - if(this.config.auth.protocolMode != ProtocolMode.OIDC && + if(this.config.auth.protocolMode !== ProtocolMode.OIDC && this.config.auth.OIDCOptions) { throw ClientConfigurationError.createCannotSetOIDCOptionsError(); } - if(this.config.auth.protocolMode == ProtocolMode.OIDC && - this.config.auth.OIDCOptions?.serverResponseType?.includes(ServerResponseType.QUERY) && - !this.config.auth.OIDCOptions?.serverResponseType?.includes(ServerResponseType.HASH)) { - /* - * Check from ?code to make sure we don't get a random query string - * until # since some IDPs add stuff that doesn't concern MSAL after the # + var found_hash = hash; + + if(this.config.auth.protocolMode === ProtocolMode.OIDC && + this.config.auth.OIDCOptions?.serverResponseType === ServerResponseType.QUERY) { + /** + * Check from ?code to make sure we don't get a random query string + * until # since some IDPs add stuff that doesn't concern MSAL after the # */ - let url = window.location.href; + const url = window.location.href; if(url.indexOf("?code") > -1) { if(url.indexOf("#") > -1) { - hash = url.substring(url.indexOf("?code") + 1, url.indexOf("#")); + found_hash = url.substring(url.indexOf("?code") + 1, url.indexOf("#")); } else { - hash = url.substring(url.indexOf("?code") + 1); + found_hash = url.substring(url.indexOf("?code") + 1); } } } @@ -361,7 +361,7 @@ export class StandardController implements IController { * otherwise return the promise from the first invocation. Prevents race conditions when handleRedirectPromise is called * several times concurrently. */ - const redirectResponseKey = hash || Constants.EMPTY_STRING; + const redirectResponseKey = found_hash || Constants.EMPTY_STRING; let response = this.redirectResponse.get(redirectResponseKey); if (typeof response === "undefined") { this.eventHandler.emitEvent( @@ -383,7 +383,7 @@ export class StandardController implements IController { this.nativeExtensionProvider ) && this.nativeExtensionProvider && - !hash + !found_hash ) { this.logger.trace( "handleRedirectPromise - acquiring token from native platform" @@ -415,7 +415,7 @@ export class StandardController implements IController { const redirectClient = this.createRedirectClient(correlationId); redirectResponse = - redirectClient.handleRedirectPromise(hash); + redirectClient.handleRedirectPromise(found_hash); } response = redirectResponse diff --git a/lib/msal-browser/src/interaction_client/PopupClient.ts b/lib/msal-browser/src/interaction_client/PopupClient.ts index 1d293bc347..e8700bacbc 100644 --- a/lib/msal-browser/src/interaction_client/PopupClient.ts +++ b/lib/msal-browser/src/interaction_client/PopupClient.ts @@ -418,7 +418,7 @@ export class PopupClient extends StandardInteractionClient { try { authClient.authority.endSessionEndpoint; } catch { - if (validRequest.account?.homeAccountId && validRequest.postLogoutRedirectUri && authClient.authority.protocolMode == ProtocolMode.OIDC){ + if (validRequest.account?.homeAccountId && validRequest.postLogoutRedirectUri && authClient.authority.protocolMode === ProtocolMode.OIDC){ this.browserStorage.removeAccount(validRequest.account?.homeAccountId); this.browserStorage.removeAccessToken(validRequest.account?.homeAccountId); this.browserStorage.removeIdToken(validRequest.account?.homeAccountId); @@ -585,15 +585,14 @@ export class PopupClient extends StandardInteractionClient { return; } - var href = popupWindow.location.href; - var hash; - if(this.config.auth.protocolMode == ProtocolMode.OIDC && - this.config.auth.OIDCOptions?.serverResponseType?.includes(ServerResponseType.QUERY) && - !this.config.auth.OIDCOptions?.serverResponseType?.includes(ServerResponseType.HASH)) { - /* - * Check from ?code to make sure we don't get a random query string - * until # since some IDPs add stuff that doesn't concern MSAL after the # - */ + let href = popupWindow.location.href; + let hash; + if(this.config.auth.protocolMode === ProtocolMode.OIDC && + this.config.auth.OIDCOptions?.serverResponseType === ServerResponseType.QUERY) { + /** + * Check from ?code to make sure we don't get a random query string + * until # since some IDPs add stuff that doesn't concern MSAL after the # + */ if(href.indexOf("?code") > -1) { if(href.indexOf("#") > -1) { hash = href.substring(href.indexOf("?code") + 1, href.indexOf("#")); diff --git a/lib/msal-browser/src/interaction_client/RedirectClient.ts b/lib/msal-browser/src/interaction_client/RedirectClient.ts index 5494f00d91..c306d2cfc9 100644 --- a/lib/msal-browser/src/interaction_client/RedirectClient.ts +++ b/lib/msal-browser/src/interaction_client/RedirectClient.ts @@ -507,7 +507,7 @@ export class RedirectClient extends StandardInteractionClient { ); this.logger.verbose("Auth code client created"); - if(authClient.authority.protocolMode == ProtocolMode.OIDC) { + if(authClient.authority.protocolMode === ProtocolMode.OIDC) { try { authClient.authority.endSessionEndpoint; } catch { diff --git a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts index 1e1038d811..163f06847f 100644 --- a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts @@ -436,10 +436,10 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { this.correlationId ); - var configResponseMode = null; + let configResponseMode = null; if(!this.config.auth.OIDCOptions || (this.config.auth.OIDCOptions.serverResponseType && - this.config.auth.OIDCOptions.serverResponseType.includes(ServerResponseType.HASH))) { + this.config.auth.OIDCOptions.serverResponseType === ServerResponseType.HASH)) { configResponseMode = ResponseMode.FRAGMENT; } else { diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts index bb1cc9752f..54feb369e7 100644 --- a/lib/msal-browser/test/app/PublicClientApplication.spec.ts +++ b/lib/msal-browser/test/app/PublicClientApplication.spec.ts @@ -808,7 +808,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, protocolMode: ProtocolMode.AAD, - OIDCOptions: { serverResponseType: [ServerResponseType.QUERY] } + OIDCOptions: { serverResponseType: "query" } }, telemetry: { application: { @@ -836,7 +836,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, protocolMode: ProtocolMode.OIDC, - OIDCOptions: { serverResponseType: [ServerResponseType.QUERY] } + OIDCOptions: { serverResponseType: ServerResponseType.QUERY } }, telemetry: { application: { diff --git a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts index f717e04fd2..7b6c23e029 100644 --- a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts @@ -1591,7 +1591,7 @@ describe("PopupClient", () => { auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, protocolMode: ProtocolMode.OIDC, - OIDCOptions: { "serverResponseType": [ServerResponseType.QUERY] } + OIDCOptions: { "serverResponseType": "query" } }, }); diff --git a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts index 2b129a087b..b3a6690b69 100644 --- a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts @@ -11,6 +11,7 @@ import { AzureCloudInstance, Authority, ProtocolMode, + ServerResponseType } from "@azure/msal-common"; import { PublicClientApplication } from "../../src/app/PublicClientApplication"; import { StandardInteractionClient } from "../../src/interaction_client/StandardInteractionClient"; @@ -189,7 +190,7 @@ describe("StandardInteractionClient OIDCOptions Tests", () => { auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, protocolMode: ProtocolMode.OIDC, - OIDCOptions: {serverResponseType: ["query"]} + OIDCOptions: {serverResponseType: ServerResponseType.QUERY} } }); diff --git a/lib/msal-common/src/authority/Authority.ts b/lib/msal-common/src/authority/Authority.ts index b9966ebd1a..14e2890da0 100644 --- a/lib/msal-common/src/authority/Authority.ts +++ b/lib/msal-common/src/authority/Authority.ts @@ -1090,4 +1090,4 @@ export class Authority { return ciamAuthority; } -} \ No newline at end of file +} diff --git a/lib/msal-common/src/authority/OIDCOptions.ts b/lib/msal-common/src/authority/OIDCOptions.ts index 269a183293..1c9c4639fd 100644 --- a/lib/msal-common/src/authority/OIDCOptions.ts +++ b/lib/msal-common/src/authority/OIDCOptions.ts @@ -7,7 +7,7 @@ * Options for the OIDC protocol mode. */ export type OIDCOptions = { - serverResponseType?: Array; + serverResponseType?: ServerResponseType; defaultScopes?: Array; }; @@ -15,3 +15,4 @@ export const ServerResponseType = { QUERY: "query", HASH: "hash", } as const; +export type ServerResponseType = typeof ServerResponseType[keyof typeof ServerResponseType]; diff --git a/lib/msal-common/src/client/AuthorizationCodeClient.ts b/lib/msal-common/src/client/AuthorizationCodeClient.ts index 6f4718bc36..e7bc9c80b8 100644 --- a/lib/msal-common/src/client/AuthorizationCodeClient.ts +++ b/lib/msal-common/src/client/AuthorizationCodeClient.ts @@ -203,8 +203,8 @@ export class AuthorizationCodeClient extends BaseClient { // Deserialize hash fragment response parameters. const hashUrlString = new UrlString(hashFragment); // Deserialize hash fragment response parameters. - var serverParams : ServerAuthorizationCodeResponse; - if(this.config.authOptions.authority.options.OIDCOptions?.serverResponseType?.includes(ServerResponseType.QUERY)) { + let serverParams : ServerAuthorizationCodeResponse; + if(this.config.authOptions.authority.options.OIDCOptions?.serverResponseType === ServerResponseType.QUERY) { serverParams = UrlString.getDeserializedHash(hashFragment); } else{