Merge pull request #831 from AzureAD/error-check-request

Error check request
AzureAD · Jul 16, 2019 · ce4f045 · ce4f045
2 parents 2d3c72b + 5d5e779
commit ce4f045
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 4 deletions.
diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts
@@ -429,6 +429,10 @@ export class UserAgentApplication {
    * To renew idToken, please pass clientId as the only scope in the Authentication Parameters
    */
   acquireTokenRedirect(request: AuthenticationParameters): void {
+    if (!request) {
+      throw ClientConfigurationError.createEmptyRequestError();
+    }
+
     // Throw error if callbacks are not set before redirect
     if (!this.redirectCallbacksSet) {
       throw ClientConfigurationError.createRedirectCallbacksNotSetError();
@@ -662,6 +666,9 @@ export class UserAgentApplication {
    * @returns {Promise.<AuthResponse>} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object
    */
   acquireTokenPopup(request: AuthenticationParameters): Promise<AuthResponse> {
+    if (!request) {
+      throw ClientConfigurationError.createEmptyRequestError();
+    }
     return new Promise<AuthResponse>((resolve, reject) => {
       // Validate and filter scopes (the validate function will throw if validation fails)
       this.validateInputScope(request.scopes, true);
@@ -888,6 +895,9 @@ export class UserAgentApplication {
    */
   @resolveTokenOnlyIfOutOfIframe
   acquireTokenSilent(request: AuthenticationParameters): Promise<AuthResponse> {
+    if (!request) {
+      throw ClientConfigurationError.createEmptyRequestError();
+    }
     return new Promise<AuthResponse>((resolve, reject) => {
 
       // Validate and filter scopes (the validate function will throw if validation fails)
@@ -935,9 +945,7 @@ export class UserAgentApplication {
       let authErr: AuthError;
       let cacheResultResponse;
 
-      const forceCacheSkip = request ? request.forceRefresh : false;
-
-      if (!userContainedClaims && !forceCacheSkip) {
+      if (!userContainedClaims && !request.forceRefresh) {
         try {
           cacheResultResponse = this.getCachedToken(serverAuthenticationRequest, account);
         } catch (e) {
@@ -960,7 +968,7 @@ export class UserAgentApplication {
       else {
         if (userContainedClaims) {
           this.logger.verbose("Skipped cache lookup since claims were given.");
-        } else if (forceCacheSkip) {
+        } else if (request.forceRefresh) {
           this.logger.verbose("Skipped cache lookup since request.forceRefresh option was set to true");
         } else {
           this.logger.verbose("Token is not in cache for scope:" + scope);

diff --git a/lib/msal-core/src/error/ClientConfigurationError.ts b/lib/msal-core/src/error/ClientConfigurationError.ts
@@ -70,6 +70,10 @@ export const ClientConfigurationErrorMessage = {
     claimsRequestParsingError: {
         code: "claims_request_parsing_error",
         desc: "Could not parse the given claims request object."
+    },
+    emptyRequestError: {
+        code: "empty_request_error",
+        desc: "Request object is required."
     }
 };
 
@@ -137,4 +141,9 @@ export class ClientConfigurationError extends ClientAuthError {
         return new ClientConfigurationError(ClientConfigurationErrorMessage.claimsRequestParsingError.code,
             `${ClientConfigurationErrorMessage.claimsRequestParsingError.desc} Given value: ${claimsRequestParseError}`);
     }
+
+    static createEmptyRequestError(): ClientConfigurationError {
+        const { code, desc } = ClientConfigurationErrorMessage.emptyRequestError;
+        return new ClientConfigurationError(code, desc);
+    }
 }
diff --git a/lib/msal-core/test/UserAgentApplication.spec.ts b/lib/msal-core/test/UserAgentApplication.spec.ts
@@ -489,6 +489,17 @@ describe("UserAgentApplication.ts Class", function () {
             expect(authErr.stack).to.include("UserAgentApplication.spec.js");
             done();
         });
+
+        it("throws an error if configured with a null request", () => {
+            let correctError;
+            try {
+                msal.acquireTokenRedirect();
+            } catch (e) {
+                expect(e).to.be.instanceOf(ClientConfigurationError);
+                correctError = true;
+            }
+            expect(correctError).to.be.true;
+        });
     });
 
     describe("Different Callback Signatures", function () {
@@ -1298,6 +1309,17 @@ describe("UserAgentApplication.ts Class", function () {
             expect(acquireTokenPromise instanceof Promise).to.be.true;
             acquireTokenPromise.catch(error => {});
         });
+
+        it("throws an error if configured with a null request", () => {
+            let correctError;
+            try {
+                msal.acquireTokenPopup();
+            } catch (e) {
+                expect(e).to.be.instanceOf(ClientConfigurationError);
+                correctError = true;
+            }
+            expect(correctError).to.be.true;
+        });
     });
 
     describe("Silent Flow", function () {
@@ -1325,5 +1347,16 @@ describe("UserAgentApplication.ts Class", function () {
             acquireTokenSilentPromise.catch(error => {});
         });
 
+        it("throws an error if configured with a null request", () => {
+            let correctError;
+            try {
+                msal.acquireTokenSilent();
+            } catch (e) {
+                expect(e).to.be.instanceOf(ClientConfigurationError);
+                correctError = true;
+            }
+            expect(correctError).to.be.true;
+        });
+
     });
 });