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][msal-browser] Add support for sub error codes #1533

Merged
merged 19 commits into from
May 7, 2020
Merged
Show file tree
Hide file tree
Changes from 18 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
5 changes: 3 additions & 2 deletions lib/msal-browser/src/app/PublicClientApplication.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 { Account, SPAClient, AuthenticationParameters, INetworkModule, TokenResponse, UrlString, TemporaryCacheKeys, TokenRenewParameters, StringUtils, PromptValue, ServerError } from "@azure/msal-common";
import { Account, SPAClient, AuthenticationParameters, INetworkModule, TokenResponse, UrlString, TemporaryCacheKeys, TokenRenewParameters, StringUtils, PromptValue, ServerError, InteractionRequiredAuthError } from "@azure/msal-common";
import { Configuration, buildConfiguration } from "../config/Configuration";
import { BrowserStorage } from "../cache/BrowserStorage";
import { CryptoOps } from "../crypto/CryptoOps";
Expand Down Expand Up @@ -391,8 +391,9 @@ export class PublicClientApplication {
return await this.authModule.getValidToken(silentRequest);
} catch (e) {
const isServerError = e instanceof ServerError;
const isInteractionRequiredError = e instanceof InteractionRequiredAuthError;
jasonnutter marked this conversation as resolved.
Show resolved Hide resolved
const isInvalidGrantError = (e.errorCode === BrowserConstants.INVALID_GRANT_ERROR);
if (isServerError && isInvalidGrantError) {
if (isServerError && isInvalidGrantError && !isInteractionRequiredError) {
tnorling marked this conversation as resolved.
Show resolved Hide resolved
const tokenRequest: AuthenticationParameters = {
...silentRequest,
prompt: PromptValue.NONE
Expand Down
43 changes: 20 additions & 23 deletions lib/msal-common/src/error/InteractionRequiredAuthError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,42 +9,39 @@ import { ServerError } from "./ServerError";
/**
* InteractionRequiredAuthErrorMessage class containing string constants used by error codes and messages.
*/
export const InteractionRequiredAuthErrorMessage = {
interactionRequired: {
code: "interaction_required"
},
consentRequired: {
code: "consent_required"
},
loginRequired: {
code: "login_required"
}
};
export const InteractionRequiredAuthErrorMessage = [
"interaction_required",
"consent_required",
"login_required"
];

export const InteractionRequiredAuthSubErrorMessage = [
"message_only",
"additional_action",
"basic_action",
"user_password_expired",
"consent_required"
];

/**
* Error thrown when user interaction is required at the auth server.
*/
export class InteractionRequiredAuthError extends ServerError {

constructor(errorCode: string, errorMessage?: string) {
super(errorCode, errorMessage);
constructor(errorCode: string, errorMessage?: string, subError?: string) {
super(errorCode, errorMessage, subError);
this.name = "InteractionRequiredAuthError";

Object.setPrototypeOf(this, InteractionRequiredAuthError.prototype);
}

static isInteractionRequiredError(errorCode: string, errorString: string) : boolean {
const interactionRequiredCodes = [
InteractionRequiredAuthErrorMessage.interactionRequired.code,
InteractionRequiredAuthErrorMessage.consentRequired.code,
InteractionRequiredAuthErrorMessage.loginRequired.code
];

const isInteractionRequiredErrorCode = !StringUtils.isEmpty(errorCode) && interactionRequiredCodes.indexOf(errorCode) > -1;
const isInteractionRequiredErrorDesc = !StringUtils.isEmpty(errorString) && interactionRequiredCodes.some((irErrorCode) => {
static isInteractionRequiredError(errorCode: string, errorString: string, subError?: string) : boolean {
const isInteractionRequiredErrorCode = !StringUtils.isEmpty(errorCode) && InteractionRequiredAuthErrorMessage.indexOf(errorCode) > -1;
const isInteractionRequiredSubError = !StringUtils.isEmpty(subError) && InteractionRequiredAuthSubErrorMessage.indexOf(subError) > -1;
const isInteractionRequiredErrorDesc = !StringUtils.isEmpty(errorString) && InteractionRequiredAuthErrorMessage.some((irErrorCode) => {
return errorString.indexOf(irErrorCode) > -1;
});

return isInteractionRequiredErrorCode || isInteractionRequiredErrorDesc;
return isInteractionRequiredErrorCode || isInteractionRequiredErrorDesc || isInteractionRequiredSubError;
}
}
5 changes: 4 additions & 1 deletion lib/msal-common/src/error/ServerError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ import { AuthError } from "./AuthError";
*/
export class ServerError extends AuthError {

constructor(errorCode: string, errorMessage?: string) {
subError: string;

constructor(errorCode: string, errorMessage?: string, subError?: string) {
super(errorCode, errorMessage);
this.name = "ServerError";
this.subError = subError;

Object.setPrototypeOf(this, ServerError.prototype);
}
Expand Down
14 changes: 9 additions & 5 deletions lib/msal-common/src/response/ResponseHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,12 @@ export class ResponseHandler {
}

// Check for error
if (serverResponseHash.error || serverResponseHash.error_description) {
if (InteractionRequiredAuthError.isInteractionRequiredError(serverResponseHash.error, serverResponseHash.error_description)) {
throw new InteractionRequiredAuthError(serverResponseHash.error, serverResponseHash.error_description);
if (serverResponseHash.error || serverResponseHash.error_description || serverResponseHash.suberror) {
if (InteractionRequiredAuthError.isInteractionRequiredError(serverResponseHash.error, serverResponseHash.error_description, serverResponseHash.suberror)) {
throw new InteractionRequiredAuthError(serverResponseHash.error, serverResponseHash.error_description, serverResponseHash.suberror);
}

throw new ServerError(serverResponseHash.error, serverResponseHash.error_description);
throw new ServerError(serverResponseHash.error, serverResponseHash.error_description, serverResponseHash.suberror);
}

if (serverResponseHash.client_info) {
Expand All @@ -130,7 +130,11 @@ export class ResponseHandler {
*/
public validateServerAuthorizationTokenResponse(serverResponse: ServerAuthorizationTokenResponse): void {
// Check for error
if (serverResponse.error || serverResponse.error_description) {
if (serverResponse.error || serverResponse.error_description || serverResponse.suberror) {
if (InteractionRequiredAuthError.isInteractionRequiredError(serverResponse.error, serverResponse.error_description, serverResponse.suberror)) {
throw new InteractionRequiredAuthError(serverResponse.error, serverResponse.error_description, serverResponse.suberror);
}

const errString = `${serverResponse.error_codes} - [${serverResponse.timestamp}]: ${serverResponse.error_description} - Correlation ID: ${serverResponse.correlation_id} - Trace ID: ${serverResponse.trace_id}`;
throw new ServerError(serverResponse.error, errString);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@ export type ServerAuthorizationCodeResponse = {
state?: string;
error?: string,
error_description?: string;
suberror?: string;
};
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export type ServerAuthorizationTokenResponse = {
error?: string;
error_description?: string;
error_codes?: Array<string>;
suberror?: string;
timestamp?: string;
trace_id?: string;
correlation_id?: string;
Expand Down
28 changes: 18 additions & 10 deletions lib/msal-common/test/error/InteractionRequiredAuthError.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { expect } from "chai";
import { InteractionRequiredAuthError, InteractionRequiredAuthErrorMessage } from "../../src/error/InteractionRequiredAuthError";
import { InteractionRequiredAuthError, InteractionRequiredAuthErrorMessage, InteractionRequiredAuthSubErrorMessage } from "../../src/error/InteractionRequiredAuthError";
import { ServerError } from "../../src/error/ServerError";
import { AuthError } from "../../src/error/AuthError";
import { IdToken } from "../../src/auth/IdToken";


describe("InteractionRequiredAuthError.ts Class Unit Tests", () => {
Expand Down Expand Up @@ -29,24 +30,31 @@ describe("InteractionRequiredAuthError.ts Class Unit Tests", () => {
});

it("Returns expected value for given error code", () => {
expect(InteractionRequiredAuthError.isInteractionRequiredError(InteractionRequiredAuthErrorMessage.interactionRequired.code, "")).to.be.true;
expect(InteractionRequiredAuthError.isInteractionRequiredError(InteractionRequiredAuthErrorMessage.consentRequired.code, "")).to.be.true;
expect(InteractionRequiredAuthError.isInteractionRequiredError(InteractionRequiredAuthErrorMessage.loginRequired.code, "")).to.be.true;
InteractionRequiredAuthErrorMessage.forEach(function (errorCode) {
expect(InteractionRequiredAuthError.isInteractionRequiredError(errorCode, "")).to.be.true;
});
expect(InteractionRequiredAuthError.isInteractionRequiredError("bad_token", "")).to.be.false;
});

it("Returns expected value for given error string", () => {
expect(InteractionRequiredAuthError.isInteractionRequiredError("", `This is a ${InteractionRequiredAuthErrorMessage.interactionRequired.code} error!`)).to.be.true;
expect(InteractionRequiredAuthError.isInteractionRequiredError("", `This is a ${InteractionRequiredAuthErrorMessage.consentRequired.code} error!`)).to.be.true;
expect(InteractionRequiredAuthError.isInteractionRequiredError("", `This is a ${InteractionRequiredAuthErrorMessage.loginRequired.code} error!`)).to.be.true;
InteractionRequiredAuthErrorMessage.forEach(function (errorCode) {
expect(InteractionRequiredAuthError.isInteractionRequiredError("", `This is a ${errorCode} error!`)).to.be.true;
});
expect(InteractionRequiredAuthError.isInteractionRequiredError("", "This is not an interaction required error")).to.be.false;
});

it("Returns expected value for given error code and error string", () => {
expect(InteractionRequiredAuthError.isInteractionRequiredError(InteractionRequiredAuthErrorMessage.interactionRequired.code, `This is a ${InteractionRequiredAuthErrorMessage.interactionRequired.code} error!`)).to.be.true;
expect(InteractionRequiredAuthError.isInteractionRequiredError(InteractionRequiredAuthErrorMessage.consentRequired.code, `This is a ${InteractionRequiredAuthErrorMessage.consentRequired.code} error!`)).to.be.true;
expect(InteractionRequiredAuthError.isInteractionRequiredError(InteractionRequiredAuthErrorMessage.loginRequired.code, `This is a ${InteractionRequiredAuthErrorMessage.loginRequired.code} error!`)).to.be.true;
InteractionRequiredAuthErrorMessage.forEach(function (errorCode) {
expect(InteractionRequiredAuthError.isInteractionRequiredError(errorCode, `This is a ${errorCode} error!`)).to.be.true;
});
expect(InteractionRequiredAuthError.isInteractionRequiredError("bad_token", "This is not an interaction required error")).to.be.false;
});

it("Returns expected value for given sub-error", () => {
InteractionRequiredAuthSubErrorMessage.forEach(function (subErrorCode) {
expect(InteractionRequiredAuthError.isInteractionRequiredError("", "", subErrorCode)).to.be.true;
});
expect(InteractionRequiredAuthError.isInteractionRequiredError("", "", "bad_token")).to.be.false;
});
});
});
35 changes: 32 additions & 3 deletions lib/msal-common/test/response/ResponseHandler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { ServerError } from "../../src/error/ServerError";
import { CodeResponse } from "../../src";
import { ServerAuthorizationTokenResponse } from "../../src/server/ServerAuthorizationTokenResponse";
import { TimeUtils } from "../../src/utils/TimeUtils";
import { InteractionRequiredAuthErrorMessage, InteractionRequiredAuthError } from "../../src/error/InteractionRequiredAuthError";
import { InteractionRequiredAuthErrorMessage, InteractionRequiredAuthError, InteractionRequiredAuthSubErrorMessage } from "../../src/error/InteractionRequiredAuthError";
import { AccessTokenKey } from "../../src/cache/AccessTokenKey";
import { AccessTokenValue } from "../../src/cache/AccessTokenValue";

Expand Down Expand Up @@ -206,8 +206,8 @@ describe("ResponseHandler.ts Class Unit Tests", () => {
});

it("throws InteractionRequiredAuthError if hash contains error parameters", () => {
const TEST_ERROR_CODE: string = InteractionRequiredAuthErrorMessage.interactionRequired.code;
const TEST_ERROR_MSG: string = `This is an ${InteractionRequiredAuthErrorMessage.interactionRequired.code} test error`;
const TEST_ERROR_CODE: string = InteractionRequiredAuthErrorMessage[0];
const TEST_ERROR_MSG: string = `This is an ${InteractionRequiredAuthErrorMessage[0]} test error`;
const testServerParams: ServerAuthorizationCodeResponse = {
error: TEST_ERROR_CODE,
error_description: TEST_ERROR_MSG,
Expand Down Expand Up @@ -283,6 +283,35 @@ describe("ResponseHandler.ts Class Unit Tests", () => {
expect(() => responseHandler.validateServerAuthorizationTokenResponse(testServerParams)).to.throw(testServerParams.error_description);
expect(() => responseHandler.validateServerAuthorizationTokenResponse(testServerParams)).to.throw(ServerError);
});

it("throws InteractionRequiredAuthError if hash contains error parameters", () => {
const TEST_ERROR_CODE: string = InteractionRequiredAuthErrorMessage[0];
const TEST_ERROR_MSG: string = `This is an ${InteractionRequiredAuthErrorMessage[0]} test error`;
const testServerParams: ServerAuthorizationTokenResponse = {
error: TEST_ERROR_CODE,
error_description: TEST_ERROR_MSG
};

const responseHandler = new ResponseHandler(TEST_CONFIG.MSAL_CLIENT_ID, cacheStorage, cacheHelpers, cryptoInterface, logger);
expect(() => responseHandler.validateServerAuthorizationTokenResponse(testServerParams)).to.throw(testServerParams.error_description);
expect(() => responseHandler.validateServerAuthorizationTokenResponse(testServerParams)).to.throw(ServerError);
expect(() => responseHandler.validateServerAuthorizationTokenResponse(testServerParams)).to.throw(InteractionRequiredAuthError);
});

it("throws InteractionRequiredAuthError if hash contains interaction required sub-error", () => {
const TEST_ERROR_CODE: string = InteractionRequiredAuthErrorMessage[0];
const TEST_ERROR_MSG: string = `This is an ${InteractionRequiredAuthErrorMessage[0]} test error`;
const testServerParams: ServerAuthorizationTokenResponse = {
error: "invalid_grant",
error_description: "test error",
suberror: InteractionRequiredAuthSubErrorMessage[0]
};

const responseHandler = new ResponseHandler(TEST_CONFIG.MSAL_CLIENT_ID, cacheStorage, cacheHelpers, cryptoInterface, logger);
expect(() => responseHandler.validateServerAuthorizationTokenResponse(testServerParams)).to.throw(testServerParams.error_description);
expect(() => responseHandler.validateServerAuthorizationTokenResponse(testServerParams)).to.throw(ServerError);
expect(() => responseHandler.validateServerAuthorizationTokenResponse(testServerParams)).to.throw(InteractionRequiredAuthError);
});
});

describe("createTokenResponse()", () => {
Expand Down
23 changes: 9 additions & 14 deletions samples/VanillaJSTestApp2.0/app/default/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,11 @@ function signOut() {
async function getTokenPopup(request) {
return await myMSALObj.acquireTokenSilent(request).catch(async (error) => {
console.log("silent token acquisition fails.");
if (error instanceof msal.AuthenticationRequiredError) {
if (msal.AuthenticationRequiredError.isInteractionRequiredError(error.errorCode, error.errorDesc)) {
// fallback to interaction when silent call fails
console.log("acquiring token using popup");
return myMSALObj.acquireTokenPopup(request).catch(error => {
console.error(error);
});
}
if (error instanceof msal.InteractionRequiredAuthError) {
console.log("acquiring token using popup");
return myMSALObj.acquireTokenPopup(request).catch(error => {
console.error(error);
});
} else {
console.error(error);
}
Expand All @@ -79,12 +76,10 @@ async function getTokenPopup(request) {
async function getTokenRedirect(request) {
return await myMSALObj.acquireTokenSilent(request).catch(async (error) => {
console.log("silent token acquisition fails.");
if (error instanceof AuthenticationRequiredError) {
if (AuthenticationRequiredError.isInteractionRequiredError(error.errorCode, error.errorDesc)) {
// fallback to interaction when silent call fails
console.log("acquiring token using redirect");
myMSALObj.acquireTokenRedirect(request);
}
if (error instanceof msal.InteractionRequiredAuthError) {
// fallback to interaction when silent call fails
console.log("acquiring token using redirect");
myMSALObj.acquireTokenRedirect(request);
} else {
console.error(error);
}
Expand Down
24 changes: 10 additions & 14 deletions samples/VanillaJSTestApp2.0/app/ssoSilent/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ if (myMSALObj.getAccount()) {
}
}).catch(error => {
console.error("Silent Error: " + error);
if (msal.InteractionRequiredAuthError.isInteractionRequiredError(error.errorCode, error.errorDesc)) {
if (error instanceof msal.InteractionRequiredAuthError) {
signIn("loginPopup");
}
});
Expand Down Expand Up @@ -79,13 +79,11 @@ async function getTokenPopup(request) {
return await myMSALObj.acquireTokenSilent(request).catch(async (error) => {
console.log("silent token acquisition fails.");
if (error instanceof msal.InteractionRequiredAuthError) {
if (msal.InteractionRequiredAuthError.isInteractionRequiredError(error.errorCode, error.errorDesc)) {
// fallback to interaction when silent call fails
console.log("acquiring token using popup");
return myMSALObj.acquireTokenPopup(request).catch(error => {
console.error(error);
});
}
// fallback to interaction when silent call fails
console.log("acquiring token using popup");
return myMSALObj.acquireTokenPopup(request).catch(error => {
console.error(error);
});
} else {
console.error(error);
}
Expand All @@ -96,12 +94,10 @@ async function getTokenPopup(request) {
async function getTokenRedirect(request) {
return await myMSALObj.acquireTokenSilent(request).catch(async (error) => {
console.log("silent token acquisition fails.");
if (error instanceof AuthenticationRequiredError) {
if (AuthenticationRequiredError.isInteractionRequiredError(error.errorCode, error.errorDesc)) {
// fallback to interaction when silent call fails
console.log("acquiring token using redirect");
myMSALObj.acquireTokenRedirect(request);
}
if (error instanceof msal.InteractionRequiredAuthError) {
// fallback to interaction when silent call fails
console.log("acquiring token using redirect");
myMSALObj.acquireTokenRedirect(request);
} else {
console.error(error);
}
Expand Down