fix(connection): disable caching when on an insecure connection (#194)

When the page is served over an insecure connection, some browsers (Firefox) will disable the
CacheStorage API for security reasons and will throw an error when an attempt is made to use it.
Thus, do not even attempt to use CacheStorage on such connections in the first place.

fix #193

Author: schoel-bis

packages/superset-ui-connection/src/callApi/callApi.ts

@@ -28,7 +28,11 @@ export default function callApi({
     signal,
   };
 
-  if (method === 'GET' && CACHE_AVAILABLE) {
+  if (
+    method === 'GET' &&
+    CACHE_AVAILABLE &&
+    (self.location && self.location.protocol) === 'https:'
+  ) {
     return caches.open(CACHE_KEY).then(supersetCache =>
       supersetCache
         .match(url)

packages/superset-ui-connection/test/callApi/callApi.test.ts

@@ -300,83 +300,118 @@ describe('callApi()', () => {
     });
   });
 
-  it('caches requests with ETags', () =>
-    callApi({ url: mockCacheUrl, method: 'GET' }).then(() => {
+  describe('caching', () => {
+    const origLocation = self.location;
+
+    beforeAll(() => {
+      Object.defineProperty(self, 'location', { value: {} });
+    });
+
+    afterAll(() => {
+      Object.defineProperty(self, 'location', { value: origLocation });
+    });
+
+    beforeEach(() => {
+      self.location.protocol = 'https:';
+
+      return caches.delete(constants.CACHE_KEY);
+    });
+
+    it('caches requests with ETags', () =>
+      callApi({ url: mockCacheUrl, method: 'GET' }).then(() => {
+        const calls = fetchMock.calls(mockCacheUrl);
+        expect(calls).toHaveLength(1);
+
+        return caches.open(constants.CACHE_KEY).then(supersetCache =>
+          supersetCache.match(mockCacheUrl).then(cachedResponse => {
+            expect(cachedResponse).toBeDefined();
+
+            return true;
+          }),
+        );
+      }));
+
+    it('will not use cache when running off an insecure connection', () => {
+      self.location.protocol = 'http:';
+
+      return callApi({ url: mockCacheUrl, method: 'GET' }).then(() => {
+        const calls = fetchMock.calls(mockCacheUrl);
+        expect(calls).toHaveLength(1);
+
+        return caches.open(constants.CACHE_KEY).then(supersetCache =>
+          supersetCache.match(mockCacheUrl).then(cachedResponse => {
+            expect(cachedResponse).toBeUndefined();
+
+            return true;
+          }),
+        );
+      });
+    });
+
+    it('works when the Cache API is disabled', async () => {
+      Object.defineProperty(constants, 'CACHE_AVAILABLE', { value: false });
+
+      const firstResponse = await callApi({ url: mockCacheUrl, method: 'GET' });
       const calls = fetchMock.calls(mockCacheUrl);
       expect(calls).toHaveLength(1);
+      const firstBody = await firstResponse.text();
+      expect(firstBody).toEqual('BODY');
+
+      const secondResponse = await callApi({ url: mockCacheUrl, method: 'GET' });
+      const fetchParams = calls[1][1];
+      expect(calls).toHaveLength(2);
+      // second call should not have If-None-Match header
+      expect(fetchParams.headers).toBeUndefined();
+      const secondBody = await secondResponse.text();
+      expect(secondBody).toEqual('BODY');
+
+      Object.defineProperty(constants, 'CACHE_AVAILABLE', { value: true });
+    });
 
-      return caches.open(constants.CACHE_KEY).then(supersetCache =>
-        supersetCache.match(mockCacheUrl).then(cachedResponse => {
-          expect(cachedResponse).toBeDefined();
+    it('sends known ETags in the If-None-Match header', () =>
+      // first call sets the cache
+      callApi({ url: mockCacheUrl, method: 'GET' }).then(() => {
+        const calls = fetchMock.calls(mockCacheUrl);
+        expect(calls).toHaveLength(1);
+
+        // second call sends the Etag in the If-None-Match header
+        return callApi({ url: mockCacheUrl, method: 'GET' }).then(() => {
+          const fetchParams = calls[1][1];
+          const headers = { 'If-None-Match': 'etag' };
+          expect(calls).toHaveLength(2);
+          expect(fetchParams.headers).toEqual(expect.objectContaining(headers));
 
           return true;
-        }),
-      );
-    }));
-
-  it('works when the Cache API is disabled', async () => {
-    Object.defineProperty(constants, 'CACHE_AVAILABLE', { value: false });
-
-    const firstResponse = await callApi({ url: mockCacheUrl, method: 'GET' });
-    const calls = fetchMock.calls(mockCacheUrl);
-    expect(calls).toHaveLength(1);
-    const firstBody = await firstResponse.text();
-    expect(firstBody).toEqual('BODY');
-
-    const secondResponse = await callApi({ url: mockCacheUrl, method: 'GET' });
-    const fetchParams = calls[1][1];
-    expect(calls).toHaveLength(2);
-    // second call should not have If-None-Match header
-    expect(fetchParams.headers).toBeUndefined();
-    const secondBody = await secondResponse.text();
-    expect(secondBody).toEqual('BODY');
-
-    Object.defineProperty(constants, 'CACHE_AVAILABLE', { value: true });
-  });
+        });
+      }));
 
-  it('sends known ETags in the If-None-Match header', () =>
-    // first call sets the cache
-    callApi({ url: mockCacheUrl, method: 'GET' }).then(() => {
+    it('reuses cached responses on 304 status', async () => {
+      // first call sets the cache
+      await callApi({ url: mockCacheUrl, method: 'GET' });
       const calls = fetchMock.calls(mockCacheUrl);
       expect(calls).toHaveLength(1);
+      // second call reuses the cached payload on a 304
+      const mockCachedPayload = { status: 304 };
+      fetchMock.get(mockCacheUrl, mockCachedPayload, { overwriteRoutes: true });
+
+      const secondResponse = await callApi({ url: mockCacheUrl, method: 'GET' });
+      expect(calls).toHaveLength(2);
+      const secondBody = await secondResponse.text();
+      expect(secondBody).toEqual('BODY');
+    });
 
-      // second call sends the Etag in the If-None-Match header
-      return callApi({ url: mockCacheUrl, method: 'GET' }).then(() => {
-        const fetchParams = calls[1][1];
-        const headers = { 'If-None-Match': 'etag' };
-        expect(calls).toHaveLength(2);
-        expect(fetchParams.headers).toEqual(expect.objectContaining(headers));
+    it('throws error when cache fails on 304', () => {
+      // this should never happen, since a 304 is only returned if we have
+      // the cached response and sent the If-None-Match header
+      const mockUncachedUrl = '/mock/uncached/url';
+      const mockCachedPayload = { status: 304 };
+      fetchMock.get(mockUncachedUrl, mockCachedPayload);
 
-        return true;
+      return callApi({ url: mockUncachedUrl, method: 'GET' }).catch(error => {
+        const calls = fetchMock.calls(mockUncachedUrl);
+        expect(calls).toHaveLength(1);
+        expect(error.message).toEqual('Received 304 but no content is cached!');
       });
-    }));
-
-  it('reuses cached responses on 304 status', async () => {
-    // first call sets the cache
-    await callApi({ url: mockCacheUrl, method: 'GET' });
-    const calls = fetchMock.calls(mockCacheUrl);
-    expect(calls).toHaveLength(1);
-    // second call reuses the cached payload on a 304
-    const mockCachedPayload = { status: 304 };
-    fetchMock.get(mockCacheUrl, mockCachedPayload, { overwriteRoutes: true });
-
-    const secondResponse = await callApi({ url: mockCacheUrl, method: 'GET' });
-    expect(calls).toHaveLength(2);
-    const secondBody = await secondResponse.text();
-    expect(secondBody).toEqual('BODY');
-  });
-
-  it('throws error when cache fails on 304', () => {
-    // this should never happen, since a 304 is only returned if we have
-    // the cached response and sent the If-None-Match header
-    const mockUncachedUrl = '/mock/uncached/url';
-    const mockCachedPayload = { status: 304 };
-    fetchMock.get(mockUncachedUrl, mockCachedPayload);
-
-    return callApi({ url: mockUncachedUrl, method: 'GET' }).catch(error => {
-      const calls = fetchMock.calls(mockUncachedUrl);
-      expect(calls).toHaveLength(1);
-      expect(error.message).toEqual('Received 304 but no content is cached!');
     });
   });
 

test/setupTests.ts

@@ -25,6 +25,14 @@ class CacheStorage {
       resolve(new Cache(key));
     });
   }
+  delete(key: string): Promise<boolean> {
+    const wasPresent = key in caches;
+    if (wasPresent) {
+      caches[key] = undefined;
+    }
+
+    return Promise.resolve(wasPresent);
+  }
 };
 
 global.caches = new CacheStorage();