fix(auth): improve query and error handling when completing sign in

- Add handling for when there is an empty hash
- Add utility function to more easily handle query params

AFFECTS PACKAGES:
@esri/arcgis-rest-auth
@esri/arcgis-rest-request

packages/arcgis-rest-auth/src/UserSession.ts

@@ -20,7 +20,8 @@ import {
   IAuthenticationManager,
   ITokenRequestOptions,
   cleanUrl,
-  encodeQueryString
+  encodeQueryString,
+  decodeQueryString
 } from "@esri/arcgis-rest-request";
 import { IUser } from "@esri/arcgis-rest-types";
 import { generateToken } from "./generate-token";
@@ -408,29 +409,26 @@ export class UserSession implements IAuthenticationManager {
       });
     }
 
-    const match = win.location.href.match(
-      /access_token=(.+)&expires_in=(.+)&username=([^&]+)/
-    );
+    const params = decodeQueryString(win.location.hash);
 
-    if (!match) {
-      const errorMatch = win.location.href.match(
-        /error=(.+)&error_description=(.+)/
-      );
+    if (!params.access_token) {
+      let error;
+      let errorMessage = "Unknown error";
 
-      const error = errorMatch[1];
-      const errorMessage = decodeURIComponent(errorMatch[2]);
+      if (params.error) {
+        error = params.error;
+        errorMessage = params.error_description;
+      }
 
       return completeSignIn({ error, errorMessage });
     }
 
-    const token = match[1];
+    const token = params.access_token;
     const expires = new Date(
-      Date.now() + parseInt(match[2], 10) * 1000 - 60 * 1000
+      Date.now() + parseInt(params.expires_in, 10) * 1000 - 60 * 1000
     );
-    const username = decodeURIComponent(match[3]);
-    const ssl =
-      win.location.href.indexOf("&ssl=true") > -1 ||
-      win.location.href.indexOf("#ssl=true") > -1;
+    const username = params.username;
+    const ssl = params.ssl === "true";
 
     return completeSignIn(undefined, {
       token,

packages/arcgis-rest-auth/test/UserSession.test.ts

@@ -893,8 +893,8 @@ describe("UserSession", () => {
     it("should return a new user session if it cannot find a valid parent", () => {
       const MockWindow = {
         location: {
-          href:
-            "https://example-app.com/redirect-uri#access_token=token&expires_in=1209600&username=c%40sey&ssl=true&persist=true"
+          hash:
+            "#access_token=token&expires_in=1209600&username=c%40sey&ssl=true&persist=true"
         },
         get parent() {
           return this;
@@ -918,8 +918,8 @@ describe("UserSession", () => {
     it("should return a new user session with ssl as false when callback hash does not have ssl parameter", () => {
       const MockWindow = {
         location: {
-          href:
-            "https://example-app.com/redirect-uri#access_token=token&expires_in=1209600&username=c%40sey&persist=true"
+          hash:
+            "#access_token=token&expires_in=1209600&username=c%40sey&persist=true"
         },
         get parent() {
           return this;
@@ -958,8 +958,8 @@ describe("UserSession", () => {
           done();
         },
         location: {
-          href:
-            "https://example-app.com/redirect-uri#access_token=token&expires_in=1209600&username=c%40sey&ssl=true"
+          hash:
+            "#access_token=token&expires_in=1209600&username=c%40sey&ssl=true"
         }
       };
 
@@ -992,8 +992,8 @@ describe("UserSession", () => {
           done();
         },
         location: {
-          href:
-            "https://example-app.com/redirect-uri#access_token=token&expires_in=1209600&username=c%40sey&ssl=true"
+          hash:
+            "#access_token=token&expires_in=1209600&username=c%40sey&ssl=true"
         }
       };
 
@@ -1026,8 +1026,8 @@ describe("UserSession", () => {
           done();
         },
         location: {
-          href:
-            "https://example-app.com/redirect-uri#access_token=token&expires_in=1209600&username=c%40sey&ssl=true"
+          hash:
+            "#access_token=token&expires_in=1209600&username=c%40sey&ssl=true"
         }
       };
 
@@ -1043,8 +1043,7 @@ describe("UserSession", () => {
     it("should throw an error from the authorization window", () => {
       const MockWindow = {
         location: {
-          href:
-            "https://example-app.com/redirect-uri#error=Invalid_Signin&error_description=Invalid_Signin"
+          hash: "#error=Invalid_Signin&error_description=Invalid_Signin"
         },
         get parent() {
           return this;
@@ -1073,8 +1072,7 @@ describe("UserSession", () => {
 
       const MockWindow = {
         location: {
-          href:
-            "https://example-app.com/redirect-uri#error=Invalid_Signin&error_description=Invalid_Signin"
+          hash: "#error=Invalid_Signin&error_description=Invalid_Signin"
         },
         get opener() {
           return MockParent;
@@ -1093,6 +1091,27 @@ describe("UserSession", () => {
     });
   });
 
+  it("should throw an unknown error if the url has no error or access_token", () => {
+    const MockWindow = {
+      location: {
+        hash: ""
+      },
+      get opener() {
+        return this;
+      }
+    };
+
+    expect(function() {
+      UserSession.completeOAuth2(
+        {
+          clientId: "clientId",
+          redirectUri: "https://example-app.com/redirect-uri"
+        },
+        MockWindow
+      );
+    }).toThrowError(ArcGISRequestError, "Unknown error");
+  });
+
   describe(".authorize()", () => {
     it("should redirect the request to the authorization page", done => {
       const spy = jasmine.createSpy("spy");

packages/arcgis-rest-request/src/index.ts

@@ -7,6 +7,7 @@ export * from "./utils/ArcGISRequestError";
 export * from "./utils/clean-url";
 export * from "./utils/encode-form-data";
 export * from "./utils/encode-query-string";
+export * from "./utils/decode-query-string";
 export * from "./utils/ErrorTypes";
 export * from "./utils/GrantTypes";
 export * from "./utils/HTTPMethods";

packages/arcgis-rest-request/src/utils/decode-query-string.ts

@@ -0,0 +1,27 @@
+/* Copyright (c) 2017-2020 Environmental Systems Research Institute, Inc.
+ * Apache-2.0 */
+
+export function decodeParam(param: string): { key: string; value: string } {
+  const [key, value] = param.split("=");
+  return { key: decodeURIComponent(key), value: decodeURIComponent(value) };
+}
+
+/**
+ * Decodes the passed query string as an object.
+ *
+ * @param query A string to be decoded.
+ * @returns A decoded query param object.
+ */
+export function decodeQueryString(query: string): { [key: string]: string } {
+  return query
+    .replace(/^#/, "")
+    .split("&")
+    .reduce(
+      (acc, entry) => {
+        const { key, value } = decodeParam(entry);
+        acc[key] = value;
+        return acc;
+      },
+      {} as any
+    );
+}