Updating to msal.js 1.3 and fixing teams provider redirect loop issue (#381)

nmetulev · shweaver-MSFT · web-flow · commit 9447c8ed6ccd · 2020-04-30T08:59:53.000-07:00
* updated to msal 1.3.beta and fixed teams provider auth issue

* added support for sign out

* minor improvement to auth.html

* Update to msal 1.3.0

Co-authored-by: Shane Weaver &lt;shweaver@microsoft.com&gt;
diff --git a/auth.html b/auth.html
@@ -1,10 +1,16 @@
 <html>
   <head>
     <script src="https://unpkg.com/@microsoft/teams-js/dist/MicrosoftTeams.min.js" crossorigin="anonymous"></script>
+    <!-- <script src="./dist/bundle/mgt-loader.js"></script>
+    <script>
+      mgt.TeamsProvider.handleAuth();
+    </script> -->
     <script type="module">
       import { TeamsProvider } from './dist/es6/providers/TeamsProvider.js';
       TeamsProvider.handleAuth();
     </script>
   </head>
-  <body></body>
+  <body>
+    Signing You In...
+  </body>
 </html>
diff --git a/package.json b/package.json
@@ -62,7 +62,7 @@
     "@microsoft/microsoft-graph-types": "^1.12.0",
     "@microsoft/microsoft-graph-types-beta": "github:microsoftgraph/msgraph-typescript-typings#beta",
     "lit-element": "^2.3.1",
-    "msal": "1.1.3",
+    "msal": "^1.3.0",
     "office-ui-fabric-core": "11.0.0"
   },
   "devDependencies": {
diff --git a/src/providers/MsalProvider.ts b/src/providers/MsalProvider.ts
@@ -6,7 +6,7 @@
  */
 
 import { AuthenticationProviderOptions } from '@microsoft/microsoft-graph-client/lib/es/IAuthenticationProviderOptions';
-import { createFromProvider, Graph } from '../Graph';
+import { createFromProvider } from '../Graph';
 import { IProvider, LoginType, ProviderState } from './IProvider';
 
 import { AuthenticationParameters, AuthError, AuthResponse, Configuration, UserAgentApplication } from 'msal';
@@ -116,7 +116,7 @@ export class MsalProvider extends IProvider {
   }
 
   /**
-   * simplified form of single sign-on (SSO)
+   * attempts to sign in user silently
    *
    * @returns
    * @memberof MsalProvider
@@ -137,7 +137,7 @@ export class MsalProvider extends IProvider {
   }
 
   /**
-   * login auth Promise, Redirects request, and sets Provider state to SignedIn if response is recieved
+   * sign in user
    *
    * @param {AuthenticationParameters} [authenticationParameters]
    * @returns {Promise<void>}
@@ -159,7 +159,7 @@ export class MsalProvider extends IProvider {
   }
 
   /**
-   * logout auth Promise, sets Provider state to SignedOut
+   * sign out user
    *
    * @returns {Promise<void>}
    * @memberof MsalProvider
@@ -168,8 +168,9 @@ export class MsalProvider extends IProvider {
     this._userAgentApplication.logout();
     this.setState(ProviderState.SignedOut);
   }
+
   /**
-   * recieves acess token Promise
+   * returns an access token for scopes
    *
    * @param {AuthenticationProviderOptions} options
    * @returns {Promise<string>}
@@ -210,6 +211,7 @@ export class MsalProvider extends IProvider {
     }
     throw null;
   }
+
   /**
    * sets scopes
    *
@@ -221,7 +223,7 @@ export class MsalProvider extends IProvider {
   }
 
   /**
-   * if login runs into error, require user interaction
+   * checks if error indicates a user interaction is required
    *
    * @protected
    * @param {*} error
@@ -296,6 +298,7 @@ export class MsalProvider extends IProvider {
       sessionStorage.setItem(this.sessionStorageDeniedScopesKey, JSON.stringify(deniedScopes));
     }
   }
+
   /**
    * gets deniedScopes from sessionStorage
    *
@@ -307,6 +310,7 @@ export class MsalProvider extends IProvider {
     const scopesStr = sessionStorage.getItem(this.sessionStorageDeniedScopesKey);
     return scopesStr ? JSON.parse(scopesStr) : null;
   }
+
   /**
    * if scopes are denied
    *
@@ -331,14 +335,6 @@ export class MsalProvider extends IProvider {
     this._loginType = typeof config.loginType !== 'undefined' ? config.loginType : LoginType.Redirect;
     this._loginHint = config.loginHint;
 
-    const tokenReceivedCallbackFunction = ((response: AuthResponse) => {
-      this.tokenReceivedCallback(response);
-    }).bind(this);
-
-    const errorReceivedCallbackFunction = ((authError: AuthError, accountState: string) => {
-      this.errorReceivedCallback(authError, status);
-    }).bind(this);
-
     if (config.clientId) {
       const msalConfig: Configuration = config.options || { auth: { clientId: config.clientId } };
 
@@ -358,7 +354,10 @@ export class MsalProvider extends IProvider {
       this.clientId = config.clientId;
 
       this._userAgentApplication = new UserAgentApplication(msalConfig);
-      this._userAgentApplication.handleRedirectCallback(tokenReceivedCallbackFunction, errorReceivedCallbackFunction);
+      this._userAgentApplication.handleRedirectCallback(
+        response => this.tokenReceivedCallback(response),
+        (error, state) => this.errorReceivedCallback(error, state)
+      );
     } else {
       throw new Error('clientId must be provided');
     }
diff --git a/src/providers/TeamsProvider.ts b/src/providers/TeamsProvider.ts
@@ -143,17 +143,15 @@ export class TeamsProvider extends MsalProvider {
 
     teams.initialize();
 
-    // msal checks for the window.opener.msal to check if this is a popup authentication
-    // and gets a false positive since teams opens a popup for the authentication.
-    // in reality, we are doing a redirect authentication and need to act as if this is the
-    // window initiating the authentication
-    if (window.opener) {
-      window.opener.msal = null;
+    // if we were signing out before, then we are done
+    if (sessionStorage.getItem(this._sessionStorageLogoutInProgress)) {
+      teams.authentication.notifySuccess();
     }
 
     const url = new URL(window.location.href);
+    const isSignOut = url.searchParams.get('signout');
 
-    const paramsString = sessionStorage.getItem(this._sessionStorageParametersKey);
+    const paramsString = localStorage.getItem(this._localStorageParametersKey);
     let authParams: AuthParams;
 
     if (paramsString) {
@@ -162,14 +160,6 @@ export class TeamsProvider extends MsalProvider {
       authParams = {};
     }
 
-    if (!authParams.clientId) {
-      authParams.clientId = url.searchParams.get('clientId');
-      authParams.scopes = url.searchParams.get('scopes');
-      authParams.loginHint = url.searchParams.get('loginHint');
-
-      sessionStorage.setItem(this._sessionStorageParametersKey, JSON.stringify(authParams));
-    }
-
     if (!authParams.clientId) {
       teams.authentication.notifyFailure('no clientId provided');
       return;
@@ -181,8 +171,7 @@ export class TeamsProvider extends MsalProvider {
       clientId: authParams.clientId,
       options: {
         auth: {
-          clientId: authParams.clientId,
-          redirectUri: url.protocol + '//' + url.host + url.pathname
+          clientId: authParams.clientId
         },
         system: {
           loadFrameTimeout: 10000
@@ -200,17 +189,30 @@ export class TeamsProvider extends MsalProvider {
       // how do we handle when user can't sign in
       // change to promise and return status
       if (provider.state === ProviderState.SignedOut) {
-        provider.login({
-          loginHint: authParams.loginHint,
-          scopes: scopes || provider.scopes
-        });
+        if (isSignOut) {
+          teams.authentication.notifySuccess();
+          return;
+        }
+
+        // make sure we are calling login only once
+        if (!sessionStorage.getItem(this._sessionStorageLoginInProgress)) {
+          sessionStorage.setItem(this._sessionStorageLoginInProgress, 'true');
+          provider.login({
+            loginHint: authParams.loginHint,
+            scopes: scopes || provider.scopes
+          });
+        }
       } else if (provider.state === ProviderState.SignedIn) {
+        if (isSignOut) {
+          sessionStorage.setItem(this._sessionStorageLogoutInProgress, 'true');
+          await provider.logout();
+          return;
+        }
+
         try {
           const accessToken = await provider.getAccessTokenForScopes(...provider.scopes);
-          sessionStorage.removeItem(this._sessionStorageParametersKey);
           teams.authentication.notifySuccess(accessToken);
         } catch (e) {
-          sessionStorage.removeItem(this._sessionStorageParametersKey);
           teams.authentication.notifyFailure(e);
         }
       }
@@ -220,15 +222,9 @@ export class TeamsProvider extends MsalProvider {
     handleProviderState();
   }
 
-  private static _sessionStorageParametersKey = 'msg-teamsprovider-auth-parameters';
-
-  /**
-   * Scopes used for authentication
-   *
-   * @type {string[]}
-   * @memberof TeamsProvider
-   */
-  public scopes: string[];
+  private static _localStorageParametersKey = 'msg-teamsprovider-auth-parameters';
+  private static _sessionStorageLoginInProgress = 'msg-teamsprovider-login-in-progress';
+  private static _sessionStorageLogoutInProgress = 'msg-teamsprovider-logout-in-progress';
 
   private teamsContext;
   private _authPopupUrl: string;
@@ -261,24 +257,54 @@ export class TeamsProvider extends MsalProvider {
       teams.getContext(context => {
         this.teamsContext = context;
 
-        const url = new URL(this._authPopupUrl, new URL(window.location.href));
-        url.searchParams.append('clientId', this.clientId);
+        const authParams: AuthParams = {
+          clientId: this.clientId,
+          loginHint: context.loginHint,
+          scopes: this.scopes.join(',')
+        };
 
-        if (context.loginHint) {
-          url.searchParams.append('loginHint', context.loginHint);
-        }
+        localStorage.setItem(TeamsProvider._localStorageParametersKey, JSON.stringify(authParams));
 
-        if (this.scopes) {
-          url.searchParams.append('scopes', this.scopes.join(','));
-        }
+        const url = new URL(this._authPopupUrl, new URL(window.location.href));
 
         teams.authentication.authenticate({
           failureCallback: reason => {
             this.setState(ProviderState.SignedOut);
             reject();
           },
           successCallback: result => {
-            this.setState(ProviderState.SignedIn);
+            this.trySilentSignIn();
+            resolve();
+          },
+          url: url.href
+        });
+      });
+    });
+  }
+
+  /**
+   * sign out user
+   *
+   * @returns {Promise<void>}
+   * @memberof MsalProvider
+   */
+  public async logout(): Promise<void> {
+    const teams = TeamsHelper.microsoftTeamsLib;
+
+    return new Promise((resolve, reject) => {
+      teams.getContext(context => {
+        this.teamsContext = context;
+
+        const url = new URL(this._authPopupUrl, new URL(window.location.href));
+        url.searchParams.append('signout', 'true');
+
+        teams.authentication.authenticate({
+          failureCallback: reason => {
+            this.trySilentSignIn();
+            reject();
+          },
+          successCallback: result => {
+            this.trySilentSignIn();
             resolve();
           },
           url: url.href
@@ -295,8 +321,9 @@ export class TeamsProvider extends MsalProvider {
    * @memberof TeamsProvider
    */
   public async getAccessToken(options: AuthenticationProviderOptions): Promise<string> {
-    if (!this.teamsContext) {
+    if (!this.teamsContext && TeamsHelper.microsoftTeamsLib) {
       const teams = TeamsHelper.microsoftTeamsLib;
+      teams.initialize();
       this.teamsContext = await teams.getContext();
     }
 
@@ -309,17 +336,10 @@ export class TeamsProvider extends MsalProvider {
       accessTokenRequest.loginHint = this.teamsContext.loginHint;
     }
 
-    const currentParent = window.parent;
-    if (document.referrer.startsWith('https://teams.microsoft.com/')) {
-      (window as any).parent = window;
-    }
-
     try {
       const response = await this._userAgentApplication.acquireTokenSilent(accessTokenRequest);
-      (window as any).parent = currentParent;
       return response.accessToken;
     } catch (e) {
-      (window as any).parent = currentParent;
       if (this.requiresInteraction(e)) {
         // nothing we can do now until we can do incremental consent
         return null;

Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@`
`6`	`6`	`*/`
`7`	`7`
`8`	`8`	`import { AuthenticationProviderOptions } from '@microsoft/microsoft-graph-client/lib/es/IAuthenticationProviderOptions';`
`9`		`-import { createFromProvider, Graph } from '../Graph';`
	`9`	`+import { createFromProvider } from '../Graph';`
`10`	`10`	`import { IProvider, LoginType, ProviderState } from './IProvider';`
`11`	`11`
`12`	`12`	`import { AuthenticationParameters, AuthError, AuthResponse, Configuration, UserAgentApplication } from 'msal';`
`@@ -116,7 +116,7 @@ export class MsalProvider extends IProvider {`
`116`	`116`	`}`
`117`	`117`
`118`	`118`	`/**`
`119`		`- * simplified form of single sign-on (SSO)`
	`119`	`+ * attempts to sign in user silently`
`120`	`120`	`*`
`121`	`121`	`* @returns`
`122`	`122`	`* @memberof MsalProvider`
`@@ -137,7 +137,7 @@ export class MsalProvider extends IProvider {`
`137`	`137`	`}`
`138`	`138`
`139`	`139`	`/**`
`140`		`- * login auth Promise, Redirects request, and sets Provider state to SignedIn if response is recieved`
	`140`	`+ * sign in user`
`141`	`141`	`*`
`142`	`142`	`* @param {AuthenticationParameters} [authenticationParameters]`
`143`	`143`	`* @returns {Promise<void>}`
`@@ -159,7 +159,7 @@ export class MsalProvider extends IProvider {`
`159`	`159`	`}`
`160`	`160`
`161`	`161`	`/**`
`162`		`- * logout auth Promise, sets Provider state to SignedOut`
	`162`	`+ * sign out user`
`163`	`163`	`*`
`164`	`164`	`* @returns {Promise<void>}`
`165`	`165`	`* @memberof MsalProvider`
`@@ -168,8 +168,9 @@ export class MsalProvider extends IProvider {`
`168`	`168`	`this._userAgentApplication.logout();`
`169`	`169`	`this.setState(ProviderState.SignedOut);`
`170`	`170`	`}`
	`171`	`+`
`171`	`172`	`/**`
`172`		`- * recieves acess token Promise`
	`173`	`+ * returns an access token for scopes`
`173`	`174`	`*`
`174`	`175`	`* @param {AuthenticationProviderOptions} options`
`175`	`176`	`* @returns {Promise<string>}`
`@@ -210,6 +211,7 @@ export class MsalProvider extends IProvider {`
`210`	`211`	`}`
`211`	`212`	`throw null;`
`212`	`213`	`}`
	`214`	`+`
`213`	`215`	`/**`
`214`	`216`	`* sets scopes`
`215`	`217`	`*`
`@@ -221,7 +223,7 @@ export class MsalProvider extends IProvider {`
`221`	`223`	`}`
`222`	`224`
`223`	`225`	`/**`
`224`		`- * if login runs into error, require user interaction`
	`226`	`+ * checks if error indicates a user interaction is required`
`225`	`227`	`*`
`226`	`228`	`* @protected`
`227`	`229`	`* @param {*} error`
`@@ -296,6 +298,7 @@ export class MsalProvider extends IProvider {`
`296`	`298`	`sessionStorage.setItem(this.sessionStorageDeniedScopesKey, JSON.stringify(deniedScopes));`
`297`	`299`	`}`
`298`	`300`	`}`
	`301`	`+`
`299`	`302`	`/**`
`300`	`303`	`* gets deniedScopes from sessionStorage`
`301`	`304`	`*`
`@@ -307,6 +310,7 @@ export class MsalProvider extends IProvider {`
`307`	`310`	`const scopesStr = sessionStorage.getItem(this.sessionStorageDeniedScopesKey);`
`308`	`311`	`return scopesStr ? JSON.parse(scopesStr) : null;`
`309`	`312`	`}`
	`313`	`+`
`310`	`314`	`/**`
`311`	`315`	`* if scopes are denied`
`312`	`316`	`*`
`@@ -331,14 +335,6 @@ export class MsalProvider extends IProvider {`
`331`	`335`	`this._loginType = typeof config.loginType !== 'undefined' ? config.loginType : LoginType.Redirect;`
`332`	`336`	`this._loginHint = config.loginHint;`
`333`	`337`
`334`		`- const tokenReceivedCallbackFunction = ((response: AuthResponse) => {`
`335`		`- this.tokenReceivedCallback(response);`
`336`		`- }).bind(this);`
`337`		`-`
`338`		`- const errorReceivedCallbackFunction = ((authError: AuthError, accountState: string) => {`
`339`		`- this.errorReceivedCallback(authError, status);`
`340`		`- }).bind(this);`
`341`		`-`
`342`	`338`	`if (config.clientId) {`
`343`	`339`	`const msalConfig: Configuration = config.options \|\| { auth: { clientId: config.clientId } };`
`344`	`340`
`@@ -358,7 +354,10 @@ export class MsalProvider extends IProvider {`
`358`	`354`	`this.clientId = config.clientId;`
`359`	`355`
`360`	`356`	`this._userAgentApplication = new UserAgentApplication(msalConfig);`
`361`		`- this._userAgentApplication.handleRedirectCallback(tokenReceivedCallbackFunction, errorReceivedCallbackFunction);`
	`357`	`+ this._userAgentApplication.handleRedirectCallback(`
	`358`	`+ response => this.tokenReceivedCallback(response),`
	`359`	`+ (error, state) => this.errorReceivedCallback(error, state)`
	`360`	`+ );`
`362`	`361`	`} else {`
`363`	`362`	`throw new Error('clientId must be provided');`
`364`	`363`	`}`