Fix completeOAuth2 for edge case where window is opened from another …

…window.open call (#652) * Update UserSession.ts * Update popup * Update UserSession.ts * update tests and doc * more small tweaks
Esri · Jan 30, 2020 · 25778e3 · 25778e3
1 parent 0e09a1e
commit 25778e3
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 25 deletions.
diff --git a/packages/arcgis-rest-auth/src/UserSession.ts b/packages/arcgis-rest-auth/src/UserSession.ts
@@ -252,9 +252,9 @@ export interface IUserSessionOptions {
  */
 export class UserSession implements IAuthenticationManager {
   /**
-   * Begins a new browser-based OAuth 2.0 sign in. If `options.popup` is true the
+   * Begins a new browser-based OAuth 2.0 sign in. If `options.popup` is `true` the
    * authentication window will open in a new tab/window otherwise the user will
-   * be redirected to the authorization page in their current tab.
+   * be redirected to the authorization page in their current tab/window.
    *
    * @browserOnly
    */
@@ -339,42 +339,60 @@ export class UserSession implements IAuthenticationManager {
   }
 
   /**
-   * Completes a browser-based OAuth 2.0 sign if `options.popup` is true the user
+   * Completes a browser-based OAuth 2.0  in. If `options.popup` is `true` the user
    * will be returned to the previous window. Otherwise a new `UserSession`
-   * will be returned.
+   * will be returned. You must pass the same values for `options.popup` and
+   * `options.portal` as you used in `beginOAuth2()`.
    *
    * @browserOnly
    */
   /* istanbul ignore next */
   public static completeOAuth2(options: IOAuth2Options, win: any = window) {
-    const { portal, clientId }: IOAuth2Options = {
-      ...{ portal: "https://www.arcgis.com/sharing/rest" },
+    const { portal, clientId, popup }: IOAuth2Options = {
+      ...{ portal: "https://www.arcgis.com/sharing/rest", popup: true },
       ...options
     };
 
     function completeSignIn(error: any, oauthInfo?: IFetchTokenResponse) {
-      if (win.opener && win.opener.parent) {
-        const handlerFn = win.opener[`__ESRI_REST_AUTH_HANDLER_${clientId}`];
-        if (handlerFn) {
-          handlerFn(
-            error ? JSON.stringify(error) : undefined,
-            JSON.stringify(oauthInfo)
-          );
+      try {
+        if (
+          popup &&
+          win.opener &&
+          win.opener.parent &&
+          win.opener.parent[`__ESRI_REST_AUTH_HANDLER_${clientId}`]
+        ) {
+          const handlerFn =
+            win.opener.parent[`__ESRI_REST_AUTH_HANDLER_${clientId}`];
+          if (handlerFn) {
+            handlerFn(
+              error ? JSON.stringify(error) : undefined,
+              JSON.stringify(oauthInfo)
+            );
+          }
+          win.close();
+          return undefined;
         }
-        win.close();
-        return undefined;
-      }
 
-      if (win !== win.parent) {
-        const handlerFn = win.parent[`__ESRI_REST_AUTH_HANDLER_${clientId}`];
-        if (handlerFn) {
-          handlerFn(
-            error ? JSON.stringify(error) : undefined,
-            JSON.stringify(oauthInfo)
-          );
+        if (
+          popup &&
+          win !== win.parent &&
+          win.parent &&
+          win.parent[`__ESRI_REST_AUTH_HANDLER_${clientId}`]
+        ) {
+          const handlerFn = win.parent[`__ESRI_REST_AUTH_HANDLER_${clientId}`];
+          if (handlerFn) {
+            handlerFn(
+              error ? JSON.stringify(error) : undefined,
+              JSON.stringify(oauthInfo)
+            );
+          }
+          win.close();
+          return undefined;
         }
-        win.close();
-        return undefined;
+      } catch (e) {
+        throw new ArcGISAuthError(
+          `Unable to complete authentication. It's possible you specified popup based oAuth2 but no handler from "beginOAuth2()" present. This generally happens because the "popup" option differs between "beginOAuth2()" and "completeOAuth2()".`
+        );
       }
 
       if (error) {

diff --git a/packages/arcgis-rest-auth/test/UserSession.test.ts b/packages/arcgis-rest-auth/test/UserSession.test.ts
@@ -1011,6 +1011,36 @@ describe("UserSession", () => {
         );
       }).toThrowError(ArcGISRequestError, "Invalid_Signin: Invalid_Signin");
     });
+
+    it("should throw an error if the handler or parent window cannot be accessed", () => {
+      const MockParent = {
+        get parent() {
+          throw new Error(
+            "This window isn't where auth started, but was opened from somewhere else via window.open() perhaps from another domain which would cause a cross domain error when read."
+          );
+        }
+      };
+
+      const MockWindow = {
+        location: {
+          href:
+            "https://example-app.com/redirect-uri#error=Invalid_Signin&error_description=Invalid_Signin"
+        },
+        get opener() {
+          return MockParent;
+        }
+      };
+
+      expect(function() {
+        UserSession.completeOAuth2(
+          {
+            clientId: "clientId",
+            redirectUri: "https://example-app.com/redirect-uri"
+          },
+          MockWindow
+        );
+      }).toThrowError(ArcGISAuthError);
+    });
   });
 
   describe(".authorize()", () => {