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

Authenticate popup quit working in v1.4.0 #188

Closed
benjaminmillhouse opened this issue Mar 5, 2019 · 21 comments
Closed

Authenticate popup quit working in v1.4.0 #188

benjaminmillhouse opened this issue Mar 5, 2019 · 21 comments
Assignees

Comments

@benjaminmillhouse
Copy link

I inadvertently pulled down 1.4.0 into my application today. After doing so, I noticed that the authenticate() call would no longer trigger a popup window inside of Teams. I have since locked my package into using 1.3.7 and the authenticate() functionality works as expected again.

@ydogandjiev
Copy link
Contributor

ydogandjiev commented Mar 5, 2019

I don't think we've made much changes to the microsoftTeams.authentication.authenticate API in v1.4.0 but we will look in this. @benjaminmillhouse, did you see any errors in the console that might indicate what the issue might be? @sachingupta, are you able to repro this?

@benjaminmillhouse
Copy link
Author

I did not see any errors in the console. I tried to step through the code to see what was happening, but all I could really tell is that authenticate() was getting called, but the popup never launched. I'll see if I can dig in a little bit more for you. For what it's worth, I'm running the developer version of MS Teams. Not sure if that could make any difference or not.

@benjaminmillhouse
Copy link
Author

benjaminmillhouse commented Mar 5, 2019

It looks like maybe .initialize() needs to be passed the parent window in v1.4.0, whereas before it derived it on its own.

image

@benjaminmillhouse
Copy link
Author

Update for you. It looks like the issue is inside of sendMessageRequest() (after authenticate is called). For some reason in the 1.4.0 library the args array is empty on the request passed to .postMessage(). Trying to run down why it got defaulted to an empty array instead of the passed args.

@benjaminmillhouse
Copy link
Author

createMessageRequest() appears to be where the arguments (url, height, width) get dropped and defaulted to an empty array.

@benjaminmillhouse
Copy link
Author

@ydogandjiev @sachingupta
One more update for you all. I think this is as far as I can dig for you. Looks like the issue has to do with the minification. (Not sure if this is a typescript compile thing or what.... did that change at all?) The way that createMessageRequest() was minified in v1.3.7 and v1.4.0 is different, and the difference I think is what's causing the arguments to be dropped. Here are some screenshots of the minified files to illustrate what I'm seeing.

v1.3.7 .min file

function U is sendMessageRequest and function B is createMessageRequest - separate calls
v1 3 7 min file

v1.4.0 .min file - before createMessageRequest executes

function z is sendMessageRequest - notice that createMessageRequest is now an inline function. I'm assuming maybe the argument is getting dropped here because of closure or something like that? maybe even a name collision? This screenshot is before the createMessageRequest call is invoked, the arguments are defined at the calling function level
min file - createmessagerequest - before invoke

v1.4.0 .min file - during createMessageRequest execution

this screenshot is from inside of the createMessageRequest invocation. notice that the passed arguments are now showing as undefined inside of the call.
min file - createmessagerequest - during invoke

@sachingupta
Copy link
Contributor

thanks a lot @benjaminmillhouse, We move to webpack from 1.37 to 1.4.0 that might have change the logic to minification. We are looking into it.

@sachingupta
Copy link
Contributor

Hi @benjaminmillhouse,
I have fix the issue related to inline function creation when creating min js file and release as beta version https://www.npmjs.com/package/@microsoft/teams-js/v/1.4.0-beta.18 . can you please try teams js sdk v1.4.0-beta.18 and let us know if it work fine(It doesn't repo in our test app so i can not verify).

@benjaminmillhouse
Copy link
Author

I'll try to confirm this for you tomorrow.

@benjaminmillhouse
Copy link
Author

Looks like it fixed the issue. My popup authentication flow works again with the beta release.

@sachingupta
Copy link
Contributor

@benjaminmillhouse thanks for confirming. We have release 1.4.1 with this fix. feel free to use https://www.npmjs.com/package/@microsoft/teams-js/v/1.4.1

@sachingupta sachingupta self-assigned this Mar 6, 2019
@JSteunou
Copy link

@sachingupta I'm sorry but I still have this issue on latest ios, teams app, package teams-js v1.4.1

@sachingupta
Copy link
Contributor

@JSteunou, Can you please provide more detail.

  1. Which app are you seeing this issue?
  2. Is there a way to share error message?
    FYI: @ydogandjiev

@JSteunou
Copy link

I see this in my app for teams, dev version with latest teams-js. No error whatsoever just that compare to 1.3.7 the page/popup link opened by authenticate api does not show up anymore

@kafka-yu
Copy link

@sachingupta I'm sorry but I still have this issue on latest ios, teams app, package teams-js v1.4.1

Me too! with 1.4.1 on desktop, nothing will happen after the auth popup window open, it will navigate to auth-start page in OS's browser, and no actions any more

@unckleg
Copy link

unckleg commented Apr 8, 2020

Any updates ? Popup not working on teams for Desktop.

@ydogandjiev
Copy link
Contributor

@kafka-yu, @unckleg, what version of the SDK library are you using?

@unckleg
Copy link

unckleg commented Apr 9, 2020

Hi @ydogandjiev

I'm using: https://statics.teams.microsoft.com/sdk/v1.6.0/js/MicrosoftTeams.min.js

The problem is with Office.context.ui.displayDialogAsync method it should open a popup but instead of that it is opening a new window in browser and communication for auth between parent and child window is not working.

So the Auth flow is broken.

Everything is working on Teams browser.

@benjaminmillhouse
Copy link
Author

@unckleg I'm not sure that the Teams SDK is meant to work outside of Microsoft Teams. For the application I was working on I used a separate login flow for a normal browser vs. the app running inside of Teams. For a normal browser, I ended up using ADAL / MSAL's popup-driven login flow to simulate the same behavior as the Teams SDK.

@unckleg
Copy link

unckleg commented Apr 9, 2020

@ydogandjiev @benjaminmillhouse How do you think to work outside of Microsoft Teams?

We have an Add-In that is part of Teams for both Browser & Desktop.
But for some reason, authentication logic to Microsoft and our oAuth server doesn't work on Teams for Desktop.

Instead of opening popup as it is doing fine on the browser it is opening a new tab and that's where the auth flow breaks because we are falling back to messaging between popup parent and popup child window.

Should I use something specific for opening auth popup with a specific URL in Teams for Desktop?

Also how can I identify is my Add-In code written in React running on Teams for Desktop or Teams for Web?

I tried with calling microsoftTeams.getContext() (Is it a promise ?), anyhow, I tried with both ways:

  • promise way (No succeed)
  • nonpromise way but I'm not able to get the teams context and to get hostClientType. (No succeed).

When I say no succeed the callback I'm passing to getContext is never being invoked.

@benjaminmillhouse
Copy link
Author

I had the same issue with calling microsoftTeams.getContext(). It is not a promise (at least not the version that I am running). I wish that it was. Also, it will not ever resolve the callback outside of Teams (i.e. normal browser). So what I did as a solution was just add ?inTeams=true to the end of my URL for the Teams application. Then I scrape that value at the very top level of my application and execute either my web authentication code or teams authentication code

Here's a few snippets of the code I wrote for the Teams-based login.
Login method - I wrapped it in a Promise to convert the callback to a promise I could consume every where else in the app

  login() {
    microsoftTeams.initialize();
    return new Promise((resolve, reject) => {
      microsoftTeams.authentication.authenticate({
        url: window.location.origin + "/auth/auth-start.html",
        width: 600,
        height: 535,
        successCallback: () => { return resolve(this.initialize()); },
        failureCallback: reject
      });
    });

Then my auth-start.html code block is almost identical to the Teams docs I think
This is placed in a <script> tag for auth-start.html.

    microsoftTeams.initialize();
    // Get the tab context, and use the information to navigate to Azure AD login page
      microsoftTeams.getContext(function (context) {
        // ADAL.js configuration
        let config = {
          // Use the tenant id of the current organization. For guest users, we want an access token for 
          // the tenant we are currently in, not the home tenant of the guest. 
          tenant: context.tid,
          clientId, // This is the Client Id of your application
          redirectUri: window.location.origin + "/auth/auth-end.html",       // This should be in the list of redirect uris for the AAD app
          cacheLocation: "localStorage",
          navigateToLoginRequestUrl: false,

          // Setup extra query parameters for ADAL
          // - openid and profile scope adds profile information to the id_token
          // - login_hint provides the expected user name
          extraQueryParameter: "scope=openid+profile&login_hint=" + encodeURIComponent(context.loginHint),
        };

        // Navigate to the AzureAD login page        
        let authContext = new AuthenticationContext(config);
        authContext.login();
      });

Here is my code for auth-end.html (You need to make sure that your OAuth server is allowed to reply to this URL)

    microsoftTeams.initialize();
      let config = {
        clientId,
        cacheLocation: "localStorage",
        navigateToLoginRequestUrl: false
      };

      let authContext = new AuthenticationContext(config);
      if (authContext.isCallback(window.location.hash)) {
        authContext.handleWindowCallback(window.location.hash);
        // Only call notifySuccess or notifyFailure if this page is in the authentication popup
        microsoftTeams.getContext(context => {
          let user = authContext.getCachedUser();
          if (user) {
            fetch('/configs?tenantId=' + user.profile.tid).then(res => res.json()).then(({resource}) => {
              authContext.acquireToken(resource, (error, token) => {
                if (token) {
                  microsoftTeams.authentication.notifySuccess();
                }
              });
            })
          } else {
            microsoftTeams.authentication.notifyFailure(authContext.getLoginError());
          }
        });
      }

Then for a normal Web App experience (app running directly in the browser) I just enable config.popup on the ADAL config and let it drive everything. Again, I convert the callback behavior into a Promise because that's easier for me to utilize within the rest of the app. I believe MSAL (the replacement for ADAL) is fully promise-driven instead of callbacks, but I have not ported to it yet.

/** @type {AuthenticationContext.Options} */
const config = {
  cacheLocation: 'localStorage',
  clientId: '', // your app's client id
  extraQueryParameter: `scope=openid+profile?login_hint=${encodeURIComponent(user.userName)}`,
  navigateToLoginRequestUrl: false,
  popUp: true,
  redirectUri: window.location.origin + '/auth/auth-end.html',
  tenant: '' // your user's tenant
  postLogoutRedirectUri: window.location.origin
};

  async login() {
    return new Promise((resolve, reject) => {
      authContext.callback = (errDesc, token, err) => {
        if (!token) { return reject(errDesc); }
        return resolve(this.initialize());
      };
      authContext.login();
    });
  }

@ghost ghost locked as resolved and limited conversation to collaborators Sep 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants