Merge pull request #703 from ssylvia/secure-token

Patch hideToken for browser CORS support
Esri · May 11, 2020 · b978605 · b978605
2 parents 945ed04 + 1c59e9a
commit b978605
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 11 deletions.
diff --git a/packages/arcgis-rest-request/src/request.ts b/packages/arcgis-rest-request/src/request.ts
@@ -288,7 +288,10 @@ export function request(
 
       if (fetchOptions.method === "GET") {
         // Prevents token from being passed in query params when hideToken option is used.
-        if (params.token && options.hideToken) {
+        /* istanbul ignore if - window is always defined in a browser. Test case is covered by Jasmine in node test */
+        if (params.token && options.hideToken && 
+          // Sharing API does not support preflight check required by modern browsers https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request
+          typeof window === 'undefined') {
           requestHeaders["X-Esri-Authorization"] = `Bearer ${params.token}`
           delete params.token;
         }
@@ -299,16 +302,20 @@ export function request(
           queryParams === "" ? url : url + "?" + encodeQueryString(params);
 
         if (
-          options.maxUrlLength &&
-          urlWithQueryString.length > options.maxUrlLength
+          // This would exceed the maximum length for URLs specified by the consumer and requires POST
+          (options.maxUrlLength &&
+          urlWithQueryString.length > options.maxUrlLength) ||
+          // Or if the customer requires the token to be hidden and it has not already been hidden in the header (for browsers)
+          (params.token && options.hideToken)
         ) {
           // the consumer specified a maximum length for URLs
           // and this would exceed it, so use post instead
           fetchOptions.method = "POST";
 
-          // Add token back to body with other params instead of header
+          // If the token was already added as a Auth header, add the token back to body with other params instead of header
           if (token.length && options.hideToken) {
             params.token = token;
+            // Remove existing header that was added before url query length was checked
             delete requestHeaders["X-Esri-Authorization"];
           }
         } else {

diff --git a/packages/arcgis-rest-request/src/utils/IRequestOptions.ts b/packages/arcgis-rest-request/src/utils/IRequestOptions.ts
@@ -24,7 +24,8 @@ export interface IRequestOptions {
   authentication?: IAuthenticationManager;
   /**
    * Prevents the token from being passed in a URL Query param that is saved in browser history.
-   * Instead, the token will be passed in POST request body or through X-Esri-Authorization header
+   * Instead, the token will be passed in POST request body or through X-Esri-Authorization header.
+   * NOTE: This will force POST requests in browsers since auth header is not yet supported by preflight OPTIONS check with CORS.
    */
   hideToken?: boolean;
   /**

diff --git a/packages/arcgis-rest-request/test/request.test.ts b/packages/arcgis-rest-request/test/request.test.ts
@@ -171,7 +171,7 @@ describe("request()", () => {
       });
   });
 
-  it("should use the `authentication` option to authenticate a request and use `X-ESRI_AUTHORIZATION` header", done => {
+  it("should hide token in POST body in browser environments otherwise it should hide token in `X-ESRI_AUTHORIZATION` header in Node", done => {
     fetchMock.once("*", SharingRestInfo);
 
     const MOCK_AUTH = {
@@ -187,11 +187,23 @@ describe("request()", () => {
       hideToken: true
     })
       .then(response => {
-        const [url, options]: [string, RequestInit] = fetchMock.lastCall("*");
-        expect(url).toEqual("https://www.arcgis.com/sharing/rest/info?f=json");
-        expect(options.method).toBe("GET");
-        expect(response).toEqual(SharingRestInfo);
-        expect((options.headers as any)["X-Esri-Authorization"]).toBe("Bearer token");
+        // Test Node path with Jasmine in Node
+        if (typeof window === 'undefined') {
+          const [url, options]: [string, RequestInit] = fetchMock.lastCall("*");
+          expect(url).toEqual("https://www.arcgis.com/sharing/rest/info?f=json");
+          expect(options.method).toBe("GET");
+          expect(response).toEqual(SharingRestInfo);
+          expect((options.headers as any)["X-Esri-Authorization"]).toBe("Bearer token");
+        } else {
+          // Test browser path when run in browser with Karma
+          const [url, options]: [string, RequestInit] = fetchMock.lastCall("*");
+          expect(url).toEqual("https://www.arcgis.com/sharing/rest/info");
+          expect(options.method).toBe("POST");
+          expect(options.body).toContain("f=json");
+          expect(options.body).toContain("token=token");
+          expect((options.headers as any)["X-Esri-Authorization"]).toBe(undefined);
+          expect(response).toEqual(SharingRestInfo)
+        }
         done();
       })
       .catch(e => {