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

Add tests proving bug with ServerRequestParameters.populateQueryParams when passed null #1538

Merged
merged 6 commits into from
Apr 24, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions lib/msal-core/src/ServerRequestParameters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
tnorling marked this conversation as resolved.
Show resolved Hide resolved
let queryParameters: StringDict = {};

if (request) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) => {
Expand Down
41 changes: 38 additions & 3 deletions lib/msal-core/test/ServerRequestParameters.spec.ts
Original file line number Diff line number Diff line change
@@ -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 () {

Expand Down Expand Up @@ -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"};
Expand All @@ -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);
});
});
});