Merge pull request #1767 from AzureAD/login-instrumentation

Extend login and logout instrumentation
AzureAD · Jun 15, 2020 · 8c86372 · 8c86372
2 parents b3a7636 + d2468df
commit 8c86372
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 12 deletions.
diff --git a/build/sdl-tasks.yml b/build/sdl-tasks.yml
@@ -10,14 +10,14 @@ steps:
     optionsXS: 1
     optionsHMENABLE: 0
 
-- task: securedevelopmentteam.vss-secure-development-tools.build-task-credscan.CredScan@3
-  displayName: 'Run CredScan3'
-  inputs:
-    scanFolder: './'
-    debugMode: false
+# - task: securedevelopmentteam.vss-secure-development-tools.build-task-credscan.CredScan@3
+#   displayName: 'Run CredScan3'
+#   inputs:
+#     scanFolder: './'
+#     debugMode: false
 
 - task: securedevelopmentteam.vss-secure-development-tools.build-task-postanalysis.PostAnalysis@1
   displayName: 'Post Analysis'
   inputs:
-    CredScan: true
+    CredScan: false
     PoliCheck: true
diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts
@@ -287,33 +287,46 @@ export class UserAgentApplication {
      * @param hash
      */
     public urlContainsHash(hash: string) {
+        this.logger.verbose("UrlContainsHash has been called");
         return UrlUtils.urlContainsHash(hash);
     }
 
     private authResponseHandler(interactionType: InteractionType, response: AuthResponse, resolve?: any) : void {
+        this.logger.verbose("AuthResponseHandler has been called");
+
         if (interactionType === Constants.interactionTypeRedirect) {
+            this.logger.verbose("Interaction type is redirect");
             if (this.errorReceivedCallback) {
+                this.logger.verbose("Two callbacks were provided to handleRedirectCallback, calling success callback with response");
                 this.tokenReceivedCallback(response);
             } else if (this.authResponseCallback) {
+                this.logger.verbose("One callback was provided to handleRedirectCallback, calling authResponseCallback with response");
                 this.authResponseCallback(null, response);
             }
         } else if (interactionType === Constants.interactionTypePopup) {
+            this.logger.verbose("Interaction type is popup, resolving");
             resolve(response);
         } else {
             throw ClientAuthError.createInvalidInteractionTypeError();
         }
     }
 
     private authErrorHandler(interactionType: InteractionType, authErr: AuthError, response: AuthResponse, reject?: any) : void {
+        this.logger.verbose("AuthErrorHandler has been called");
+
         // set interaction_status to complete
         this.cacheStorage.removeItem(TemporaryCacheKeys.INTERACTION_STATUS);
         if (interactionType === Constants.interactionTypeRedirect) {
+            this.logger.verbose("Interaction type is redirect");
             if (this.errorReceivedCallback) {
+                this.logger.verbose("Two callbacks were provided to handleRedirectCallback, calling error callback");
                 this.errorReceivedCallback(authErr, response.accountState);
             } else {
+                this.logger.verbose("One callback was provided to handleRedirectCallback, calling authResponseCallback with error");
                 this.authResponseCallback(authErr, response);
             }
         } else if (interactionType === Constants.interactionTypePopup) {
+            this.logger.verbose("Interaction type is popup, rejecting");
             reject(authErr);
         } else {
             throw ClientAuthError.createInvalidInteractionTypeError();
@@ -326,6 +339,8 @@ export class UserAgentApplication {
      * @param {@link (AuthenticationParameters:type)}
      */
     loginRedirect(userRequest?: AuthenticationParameters): void {
+        this.logger.verbose("LoginRedirect has been called");
+
         // validate request
         const request: AuthenticationParameters = RequestUtils.validateRequest(userRequest, true, this.clientId, Constants.interactionTypeRedirect);
         this.acquireTokenInteractive(Constants.interactionTypeRedirect, true, request,  null, null);
@@ -338,6 +353,8 @@ export class UserAgentApplication {
      * To renew idToken, please pass clientId as the only scope in the Authentication Parameters
      */
     acquireTokenRedirect(userRequest: AuthenticationParameters): void {
+        this.logger.verbose("AcquireTokenRedirect has been called");
+
         // validate request
         const request: AuthenticationParameters = RequestUtils.validateRequest(userRequest, false, this.clientId, Constants.interactionTypeRedirect);
         this.acquireTokenInteractive(Constants.interactionTypeRedirect, false, request, null, null);
@@ -351,6 +368,8 @@ 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
      */
     loginPopup(userRequest?: AuthenticationParameters): Promise<AuthResponse> {
+        this.logger.verbose("LoginPopup has been called");
+
         // validate request
         const request: AuthenticationParameters = RequestUtils.validateRequest(userRequest, true, this.clientId, Constants.interactionTypePopup);
         const apiEvent: ApiEvent = this.telemetryManager.createAndStartApiEvent(request.correlationId, API_EVENT_IDENTIFIER.LoginPopup);
@@ -359,6 +378,7 @@ export class UserAgentApplication {
             this.acquireTokenInteractive(Constants.interactionTypePopup, true, request, resolve, reject);
         })
             .then((resp) => {
+                this.logger.verbose("Successfully logged in");
                 this.telemetryManager.stopAndFlushApiEvent(request.correlationId, apiEvent, true);
                 return resp;
             })
@@ -651,6 +671,8 @@ export class UserAgentApplication {
      * @param request
      */
     ssoSilent(request: AuthenticationParameters): Promise<AuthResponse> {
+        this.logger.verbose("ssoSilent has been called");
+
         // throw an error on an empty request
         if (!request) {
             throw ClientConfigurationError.createEmptyRequestError();
@@ -1019,6 +1041,7 @@ export class UserAgentApplication {
      * Default behaviour is to redirect the user to `window.location.href`.
      */
     logout(correlationId?: string): void {
+        this.logger.verbose("Logout has been called");
         this.logoutAsync(correlationId);
     }
 
@@ -1043,15 +1066,28 @@ export class UserAgentApplication {
 
             const correlationIdParam = `client-request-id=${requestCorrelationId}`;
 
-            const postLogoutQueryParam = this.getPostLogoutRedirectUri()
-                ? `&post_logout_redirect_uri=${encodeURIComponent(this.getPostLogoutRedirectUri())}`
-                : "";
+            let postLogoutQueryParam: string;
+            if (this.getPostLogoutRedirectUri()) {
+                postLogoutQueryParam = `&post_logout_redirect_uri=${encodeURIComponent(this.getPostLogoutRedirectUri())}`;
+                this.logger.verbose("redirectUri found and set");
+            } else {
+                postLogoutQueryParam = "";
+                this.logger.verbose("No redirectUri set for app. postLogoutQueryParam is empty");
+            }
 
-            const urlNavigate = this.authorityInstance.EndSessionEndpoint
-                ? `${this.authorityInstance.EndSessionEndpoint}?${correlationIdParam}${postLogoutQueryParam}`
-                : `${this.authority}oauth2/v2.0/logout?${correlationIdParam}${postLogoutQueryParam}`;
+            let urlNavigate: string;
+            if (this.authorityInstance.EndSessionEndpoint) {
+                urlNavigate = `${this.authorityInstance.EndSessionEndpoint}?${correlationIdParam}${postLogoutQueryParam}`;
+                this.logger.verbose("EndSessionEndpoint found and urlNavigate set");
+                this.logger.verbosePii(`urlNavigate set to: ${this.authorityInstance.EndSessionEndpoint}`);
+            } else {
+                urlNavigate = `${this.authority}oauth2/v2.0/logout?${correlationIdParam}${postLogoutQueryParam}`;
+                this.logger.verbose("No endpoint, urlNavigate set to default");
+            }
 
             this.telemetryManager.stopAndFlushApiEvent(requestCorrelationId, apiEvent, true);
+
+            this.logger.verbose("Navigating window to urlNavigate");
             this.navigateWindow(urlNavigate);
         } catch (error) {
             this.telemetryManager.stopAndFlushApiEvent(requestCorrelationId, apiEvent, false, error.errorCode);
@@ -1064,6 +1100,7 @@ export class UserAgentApplication {
      * @ignore
      */
     protected clearCache(): void {
+        this.logger.verbose("Clearing cache");
         window.renewStates = [];
         const accessTokenItems = this.cacheStorage.getAllAccessTokens(Constants.clientId, Constants.homeAccountIdentifier);
         for (let i = 0; i < accessTokenItems.length; i++) {
@@ -1072,6 +1109,7 @@ export class UserAgentApplication {
         this.cacheStorage.resetCacheItems();
         // state not being sent would mean this call may not be needed; check later
         this.cacheStorage.clearMsalCookie();
+        this.logger.verbose("Cache cleared");
     }
 
     /**
@@ -1081,11 +1119,13 @@ export class UserAgentApplication {
      * @param accessToken
      */
     protected clearCacheForScope(accessToken: string) {
+        this.logger.verbose("Clearing access token from cache");
         const accessTokenItems = this.cacheStorage.getAllAccessTokens(Constants.clientId, Constants.homeAccountIdentifier);
         for (let i = 0; i < accessTokenItems.length; i++) {
             const token = accessTokenItems[i];
             if (token.value.accessToken === accessToken) {
                 this.cacheStorage.removeItem(JSON.stringify(token.key));
+                this.logger.verbosePii(`Access token removed: ${token.key}`);
             }
         }
     }