Skip to content
This repository has been archived by the owner on Sep 21, 2021. It is now read-only.

Fix dialog/authentication issues for Edge + 365 and Desktop versions of Office #144

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

almgong
Copy link

@almgong almgong commented Aug 16, 2019

Fixes #143

This fix is intended to address authentication issues for both Office 365 with Edge and the desktop versions of Office as of the recent 1903 Windows update, paired with the 1907 update of Office.

We found that for the online version of Office apps running on Edge, the taskpane instance of add-ins would believe that it were running in an add-in environment, but the dialog pop-up would determine that it was in a web environment. This mismatch prevents add-ins from communicating with the dialogs they open for authentication, since add-ins would wait for message events, while dialogs would only set an entry in localStorage.

The fix in this PR is to separate the handling of Edge as a special case and to rely solely on localStorage for both the desktop and online versions of Office apps. We decided to do this because we experienced inconsistent message sending behavior on Edge, whereas localStorage had proven to be more stable.

This will allow for finer control over handling of each browser agent
We will continue to use localStorage as a means of communication between windows
@msftclas
Copy link

msftclas commented Aug 16, 2019

CLA assistant check
All CLA requirements met.

@almgong almgong changed the title Vm fix edge support Fix dialog/authentication issues for Edge + 365 and Desktop versions of Office Aug 16, 2019
@Rick-Kirkham Rick-Kirkham requested a review from LouMM August 16, 2019 22:40
@Rick-Kirkham
Copy link
Contributor

@almgong While you are waiting for Louis to look at this, I have a question about this part of your description: "...since add-ins would wait for message events, while dialogs would only set an entry in localStorage"

Are you saying that the messageParent API does not work when Edge is the viewer?

@almgong
Copy link
Author

almgong commented Aug 16, 2019

@Rick-Kirkham For the line that you quoted, I was referring to the the different ways that the parent add-in and the dialog were listening and sending messages to each other, respectively. The library handles these two operations differently depending on the determined host, and it is critical that this resolved value is the same in multiple windows for the dialog to close and communicate data successfully.

On the Edge/Office clients that we used to test this fix, we found that the call to Office.context.ui.messageParent() did not trigger the event listener that is attached here: https://github.com/OfficeDev/office-js-helpers/blob/master/src/helpers/dialog.ts#L94. However, falling back to the communication method with localStorage that was already in place for IE 11 and Edge worked every time for us. So I would agree with the statement that the messageParent API does not consistently work on Edge.

@alaaet
Copy link

alaaet commented Aug 27, 2019

@almgong , I'm facing the same issue, but I use the CDN to load the library into the code, do you know how can I get your fix as a compiled JS file(office.helpers(.min).js)?

@almgong
Copy link
Author

almgong commented Aug 27, 2019

@alaaet One way would be to clone the forked repository, navigate to the relevant branch vmFixEdgeSupport, and then run yarn build. This will create the compiled JS file(s) that you can use, including office.helpers.min.js.

@LouMM LouMM removed their assignment Oct 10, 2019
@LouMM LouMM removed their request for review October 10, 2019 16:37
Comment on lines 149 to 160
private _edgeDialog(): Promise<T> {
return new Promise((resolve, reject) => {
Office.context.ui.displayDialogAsync(this.url, { width: this.size.width$, height: this.size.height$ }, (result: Office.AsyncResult) => {
if (result.status === Office.AsyncResultStatus.Failed) {
reject(new DialogError(result.error.message, result.error));
}
else {
this._pollLocalStorageForToken(resolve, reject);
}
});
});
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there's a bug with this dialog on the Outlook desktop client (tested build 1908 & 1909). I use the fork this PR is based on, and my changes in my fork of the fork(🙄) seems to have fixed it.

The problem seem to be that the dialog isn't properly closed as far as office.js is concerned. It works as expected when using OWA in Edge, but not on the desktop client.

The scenario is the user is authenticated through the popup dialog right. And at some point the token is invalid and offce-helpers tries to open a new popup to authenticate the user again. But it receives an error from office.js with code 12007 "A dialog box is already opened from this host window."

These changes seems to have fixed the problem:

   private _edgeDialog(): Promise<T> {
     return new Promise((resolve, reject) => {
       Office.context.ui.displayDialogAsync(this.url, { width: this.size.width$, height: this.size.height$ }, (result: Office.AsyncResult) => {
+        let dialog = result.value as Office.DialogHandler;
+
         if (result.status === Office.AsyncResultStatus.Failed) {
           reject(new DialogError(result.error.message, result.error));
         }
         else {
-          this._pollLocalStorageForToken(resolve, reject);
+          const onToken = token => {
+            if (dialog) {
+              dialog.close();
+            }
+
+            return resolve(token);
+          };
+
+          this._pollLocalStorageForToken(onToken, reject);
         }
       });
     });

@zliebersbach
Copy link

@lindalu-MSFT Seeing as you were the last committer on this repository, could we please get some information as to the future of this repository?
The last responses from Microsoft in issue #143 don't seem to promising...

@OfficeDev OfficeDev locked as off-topic and limited conversation to collaborators Oct 18, 2019
@OfficeDev OfficeDev deleted a comment from zliebersbach Oct 18, 2019
@OfficeDev OfficeDev deleted a comment from jsaele Oct 18, 2019
almgong added 5 commits March 17, 2021 18:45
The handler for Edge environments used to poll for localStorage, but
due to issues relating to Edge WebView2 rollout, localStorage is no
longer shared between the dialog prompt and add-in pane. This change
essentially reverts the original change from 2019 that updated the
handling to rely on localStorage.

The reason we are doing this is because:

1. The authentication flow is broken until MS resolves this bug:
OfficeDev/office-js#1741

2. messageParent has always been the preferred communication medium
as noted in the DialogApi documentation:
https://docs.microsoft.com/en-us/office/dev/add-ins/develop/dialog-api-in-office-add-ins

3. There was a finding that the dialog needs to be opened with a URL
that matches the opener. With this change, messageParent now appears
to be working correctly.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Office Version 1907 / Windows 1903 - Authentication Redirect
7 participants