Skip to content

Commit

Permalink
Addressed failing tests
Browse files Browse the repository at this point in the history
  • Loading branch information
shylasummers committed Jul 7, 2023
1 parent 7c240be commit 1b6e904
Show file tree
Hide file tree
Showing 11 changed files with 38 additions and 37 deletions.
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 @@ -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.
*/
Expand Down
28 changes: 14 additions & 14 deletions lib/msal-browser/src/controllers/StandardController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
ClientConfigurationError,
ProtocolMode,
ServerResponseType,
OIDC_DEFAULT_SCOPES,
} from "@azure/msal-common";
import {
BrowserCacheManager,
Expand Down Expand Up @@ -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();
}

Check failure on line 336 in lib/msal-browser/src/controllers/StandardController.ts

View workflow job for this annotation

GitHub Actions / msal-browser / build-test

Unexpected var, use let or const instead

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);
}
}
}
Expand All @@ -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(
Expand All @@ -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"
Expand Down Expand Up @@ -415,7 +415,7 @@ export class StandardController implements IController {
const redirectClient =
this.createRedirectClient(correlationId);
redirectResponse =
redirectClient.handleRedirectPromise(hash);
redirectClient.handleRedirectPromise(found_hash);
}

response = redirectResponse
Expand Down
19 changes: 9 additions & 10 deletions lib/msal-browser/src/interaction_client/PopupClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Check failure on line 588 in lib/msal-browser/src/interaction_client/PopupClient.ts

View workflow job for this annotation

GitHub Actions / msal-browser / build-test

'href' is never reassigned. Use 'const' instead
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("#"));
Expand Down
2 changes: 1 addition & 1 deletion lib/msal-browser/src/interaction_client/RedirectClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions lib/msal-browser/test/app/PublicClientApplication.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1591,7 +1591,7 @@ describe("PopupClient", () => {
auth: {
clientId: TEST_CONFIG.MSAL_CLIENT_ID,
protocolMode: ProtocolMode.OIDC,
OIDCOptions: { "serverResponseType": [ServerResponseType.QUERY] }
OIDCOptions: { "serverResponseType": "query" }
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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}
}
});

Expand Down
2 changes: 1 addition & 1 deletion lib/msal-common/src/authority/Authority.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1090,4 +1090,4 @@ export class Authority {

return ciamAuthority;
}
}
}
3 changes: 2 additions & 1 deletion lib/msal-common/src/authority/OIDCOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
* Options for the OIDC protocol mode.
*/
export type OIDCOptions = {
serverResponseType?: Array<string>;
serverResponseType?: ServerResponseType;
defaultScopes?: Array<string>;
};

export const ServerResponseType = {
QUERY: "query",
HASH: "hash",
} as const;
export type ServerResponseType = typeof ServerResponseType[keyof typeof ServerResponseType];
4 changes: 2 additions & 2 deletions lib/msal-common/src/client/AuthorizationCodeClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit 1b6e904

Please sign in to comment.