Added better reporting and deletion for eqParams we are excluding

AzureAD · Jun 21, 2019 · f7a254c · f7a254c
1 parent d442d70
commit f7a254c
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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');