From f7a254ce3b49b9030d627d11e86f56217f8a2212 Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Fri, 21 Jun 2019 14:02:35 -0700 Subject: [PATCH] Added better reporting and deletion for eqParams we are excluding --- lib/msal-core/src/Constants.ts | 5 ++ lib/msal-core/src/UserAgentApplication.ts | 10 ++-- .../test/UserAgentApplication.spec.ts | 47 +++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/lib/msal-core/src/Constants.ts b/lib/msal-core/src/Constants.ts index a8100ac349..db6905e901 100644 --- a/lib/msal-core/src/Constants.ts +++ b/lib/msal-core/src/Constants.ts @@ -107,6 +107,11 @@ export const SSOTypes = { DOMAIN_REQ: "domain_req" }; +export const BlacklistedEQParams = [ + SSOTypes.SID, + SSOTypes.LOGIN_HINT +]; + /** * we considered making this "enum" in the request instead of string, however it looks like the allowed list of * prompt values kept changing over past couple of years. There are some undocumented prompt values for some diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 85105f326a..893622bb88 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -7,7 +7,7 @@ import { AccessTokenValue } from "./AccessTokenValue"; import { ServerRequestParameters } from "./ServerRequestParameters"; import { Authority } from "./Authority"; import { ClientInfo } from "./ClientInfo"; -import { Constants, SSOTypes, PromptState } from "./Constants"; +import { Constants, SSOTypes, PromptState, BlacklistedEQParams } from "./Constants"; import { IdToken } from "./IdToken"; import { Logger } from "./Logger"; import { Storage } from "./Storage"; @@ -2585,8 +2585,12 @@ export class UserAgentApplication { this.logger.warning("Removed duplicate claims from extraQueryParameters. Please use either the claimsRequest field OR pass as extraQueryParameter - not both."); delete eQParams[Constants.claims]; } - delete eQParams[SSOTypes.SID]; - delete eQParams[SSOTypes.LOGIN_HINT]; + BlacklistedEQParams.forEach(param => { + if (eQParams[param]) { + this.logger.warning("Removed duplicate " + param + " from extraQueryParameters. Please use the " + param + " field in request object."); + delete eQParams[param]; + } + }); return eQParams; } diff --git a/lib/msal-core/test/UserAgentApplication.spec.ts b/lib/msal-core/test/UserAgentApplication.spec.ts index 9f18608bd6..03b4989661 100644 --- a/lib/msal-core/test/UserAgentApplication.spec.ts +++ b/lib/msal-core/test/UserAgentApplication.spec.ts @@ -44,6 +44,10 @@ describe("UserAgentApplication.ts Class", function () { const validAuthority = DEFAULT_INSTANCE + TENANT; const alternateValidAuthority = ALTERNATE_INSTANCE + TENANT; + // Test SSO params + const TEST_LOGIN_HINT = "test@test.com"; + const TEST_SID = "1234-5678"; + // Test state params const TEST_USER_STATE_NUM = "1234"; const TEST_USER_STATE_URL = "https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-implicit-grant-flow/scope1"; @@ -345,6 +349,49 @@ describe("UserAgentApplication.ts Class", function () { msal.loginRedirect(tokenRequest); }); + it('removes login_hint from request.extraQueryParameters', (done) => { + let tokenRequestWithoutLoginHint: AuthenticationParameters = { + extraQueryParameters: { + login_hint: JSON.stringify(TEST_LOGIN_HINT) + } + }; + sinon.stub(window.location, "replace").callsFake(function (url) { + expect(url).to.include(DEFAULT_INSTANCE + TENANT + '/oauth2/v2.0/authorize?response_type=id_token&scope=openid%20profile'); + expect(url).to.include('&client_id=' + MSAL_CLIENT_ID); + expect(url).to.include('&redirect_uri=' + encodeURIComponent(msal.getRedirectUri())); + expect(url).to.include('&state'); + expect(url).to.include('&client_info=1'); + expect(url).to.not.include('&login_hint='); + expect(url).to.not.include(encodeURIComponent(tokenRequestWithoutLoginHint.extraQueryParameters[SSOTypes.LOGIN_HINT])); + done(); + }); + msal.handleRedirectCallback(tokenReceivedCallback, errorReceivedCallback); + expect(msal.getRedirectUri()).to.be.equal(TEST_REDIR_URI); + msal.loginRedirect(tokenRequestWithoutLoginHint); + }); + + it('removes sid from request.extraQueryParameters', (done) => { + let tokenRequestWithoutLoginHint: AuthenticationParameters = { + extraQueryParameters: { + sid: JSON.stringify(TEST_SID) + } + }; + sinon.stub(window.location, "replace").callsFake(function (url) { + expect(url).to.include(DEFAULT_INSTANCE + TENANT + '/oauth2/v2.0/authorize?response_type=id_token&scope=openid%20profile'); + expect(url).to.include('&client_id=' + MSAL_CLIENT_ID); + expect(url).to.include('&redirect_uri=' + encodeURIComponent(msal.getRedirectUri())); + expect(url).to.include('&state'); + expect(url).to.include('&client_info=1'); + expect(url).to.not.include('&sid='); + expect(url).to.not.include(encodeURIComponent(tokenRequestWithoutLoginHint.extraQueryParameters[SSOTypes.SID])); + + done(); + }); + msal.handleRedirectCallback(tokenReceivedCallback, errorReceivedCallback); + expect(msal.getRedirectUri()).to.be.equal(TEST_REDIR_URI); + msal.loginRedirect(tokenRequestWithoutLoginHint); + }); + it('navigates user to redirectURI passed in constructor config', (done) => { sinon.stub(window.location, "replace").callsFake(function (url) { expect(url).to.include(DEFAULT_INSTANCE + TENANT + '/oauth2/v2.0/authorize?response_type=id_token&scope=openid%20profile');