Merge pull request #1538 from AzureAD/server-request-parameters-popul…

…ate-null Add tests proving bug with ServerRequestParameters.populateQueryParams when passed null
AzureAD · Apr 24, 2020 · b3bcbe5 · b3bcbe5
2 parents ac663a8 + 407aa0d
commit b3bcbe5
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 7 deletions.
diff --git a/lib/msal-core/src/ServerRequestParameters.ts b/lib/msal-core/src/ServerRequestParameters.ts
@@ -82,7 +82,7 @@ export class ServerRequestParameters {
      * @param request
      * @param serverAuthenticationRequest
      */
-    populateQueryParams(account: Account, request: AuthenticationParameters, adalIdTokenObject?: object, silentCall?: boolean): void {
+    populateQueryParams(account: Account, request: AuthenticationParameters|null, adalIdTokenObject?: object, silentCall?: boolean): void {
         let queryParameters: StringDict = {};
 
         if (request) {
@@ -113,7 +113,7 @@ export class ServerRequestParameters {
         queryParameters = this.addHintParameters(account, queryParameters);
 
         // sanity check for developer passed extraQueryParameters
-        const eQParams: StringDict = request.extraQueryParameters;
+        const eQParams: StringDict|null = request ? request.extraQueryParameters : null;
 
         // Populate the extraQueryParameters to be sent to the server
         this.queryParameters = ServerRequestParameters.generateQueryParametersString(queryParameters);
@@ -245,8 +245,8 @@ export class ServerRequestParameters {
      * Utility to generate a QueryParameterString from a Key-Value mapping of extraQueryParameters passed
      * @param extraQueryParameters
      */
-    static generateQueryParametersString(queryParameters: StringDict, silentCall?: boolean): string {
-        let paramsString: string = null;
+    static generateQueryParametersString(queryParameters?: StringDict, silentCall?: boolean): string|null {
+        let paramsString: string|null = null;
 
         if (queryParameters) {
             Object.keys(queryParameters).forEach((key: string) => {

diff --git a/lib/msal-core/test/ServerRequestParameters.spec.ts b/lib/msal-core/test/ServerRequestParameters.spec.ts
@@ -1,13 +1,15 @@
 import { expect } from "chai";
 import { ServerRequestParameters } from "../src/ServerRequestParameters";
-import { Authority, ClientConfigurationError } from "../src";
+import { Authority, ClientConfigurationError, Account } from "../src";
 import { AuthorityFactory } from "../src/authority/AuthorityFactory";
 import { UrlUtils } from "../src/utils/UrlUtils";
-import { TEST_CONFIG, TEST_RESPONSE_TYPE, TEST_URIS } from "./TestConstants";
+import { TEST_CONFIG, TEST_RESPONSE_TYPE, TEST_URIS, TEST_TOKENS, TEST_DATA_CLIENT_INFO } from "./TestConstants";
 import { ClientConfigurationErrorMessage } from "../src/error/ClientConfigurationError";
 import { AuthenticationParameters } from "../src/AuthenticationParameters";
 import { RequestUtils } from "../src/utils/RequestUtils";
 import sinon from "sinon";
+import { IdToken } from "../src/IdToken";
+import { ClientInfo } from "../src/ClientInfo";
 
 describe("ServerRequestParameters.ts Class", function () {
 
@@ -129,7 +131,7 @@ describe("ServerRequestParameters.ts Class", function () {
         });
     });
 
-    describe("Query Parameters", function () {
+    describe("generateQueryParametersString", function () {
 
         it("test hints populated using queryParameters", function () {
             const eQParams = {domain_hint: "MyDomain.com", locale: "en-us"};
@@ -145,5 +147,38 @@ describe("ServerRequestParameters.ts Class", function () {
             expect(extraQueryParameters).to.include("locale");
         });
 
+        it("properly handles null", () => {
+            const extraQueryParamaters = ServerRequestParameters.generateQueryParametersString(null);
+            expect(extraQueryParamaters).to.be.null;
+        });
+
+    });
+
+    describe("populateQueryParams", () => {
+        const idToken: IdToken = new IdToken(TEST_TOKENS.IDTOKEN_V2);
+        const clientInfo: ClientInfo = new ClientInfo(TEST_DATA_CLIENT_INFO.TEST_RAW_CLIENT_INFO);
+
+        it("populates parameters", () => {
+            const serverRequestParameters = new ServerRequestParameters(AuthorityFactory.CreateInstance("https://login.microsoftonline.com/common/", this.validateAuthority), "client-id", "toke", "redirect-uri", [ "user.read" ], "state", "correlationid");
+
+            serverRequestParameters.populateQueryParams(Account.createAccount(idToken, clientInfo), {
+                scopes: [ "user.read" ],
+                extraQueryParameters: {
+                    key: "value"
+                }
+            });
+
+            expect(serverRequestParameters.queryParameters).to.equal("login_hint=AbeLi%40microsoft.com");
+            expect(serverRequestParameters.extraQueryParameters).to.equal("key=value");
+        });
+
+        it("populates parameters (null request)", () => {
+            const serverRequestParameters = new ServerRequestParameters(AuthorityFactory.CreateInstance("https://login.microsoftonline.com/common/", this.validateAuthority), "client-id", "toke", "redirect-uri", [ "user.read" ], "state", "correlationid");
+
+            serverRequestParameters.populateQueryParams(Account.createAccount(idToken, clientInfo), null);
+
+            expect(serverRequestParameters.queryParameters).to.equal("login_hint=AbeLi%40microsoft.com");
+            expect(serverRequestParameters.extraQueryParameters).to.equal(null);
+        });
     });
 });