fix: AuthPluginParent wasn't working when embedded in an iframe (#1383)

- Check for window.opener _or_ window.parent when using AuthPluginParent
- Reorder plugin priority to use AuthPluginParent first if specified
- Tested using an example html page that both embeds and pops open a new
tab
- Fixes #1373

packages/app-utils/src/components/AuthBootstrap.tsx

@@ -21,8 +21,8 @@ export type AuthBootstrapProps = {
 
 /** Core auth plugins that are always loaded */
 const CORE_AUTH_PLUGINS = new Map([
-  ['@deephaven/auth-plugins.AuthPluginPsk', AuthPluginPsk],
   ['@deephaven/auth-plugins.AuthPluginParent', AuthPluginParent],
+  ['@deephaven/auth-plugins.AuthPluginPsk', AuthPluginPsk],
   ['@deephaven/auth-plugins.AuthPluginAnonymous', AuthPluginAnonymous],
 ]);
 

packages/auth-plugins/src/AuthPluginParent.test.tsx

@@ -3,11 +3,13 @@ import { act, render, screen } from '@testing-library/react';
 import { ApiContext, ClientContext } from '@deephaven/jsapi-bootstrap';
 import { dh } from '@deephaven/jsapi-shim';
 import type { CoreClient, LoginOptions } from '@deephaven/jsapi-types';
+import { TestUtils } from '@deephaven/utils';
 import AuthPluginParent from './AuthPluginParent';
 import { AuthConfigMap } from './AuthPlugin';
 
 let mockParentResponse: Promise<LoginOptions>;
 jest.mock('@deephaven/jsapi-utils', () => ({
+  ...jest.requireActual('@deephaven/jsapi-utils'),
   LOGIN_OPTIONS_REQUEST: 'mock-login-options-request',
   requestParentResponse: jest.fn(() => mockParentResponse),
 }));
@@ -49,8 +51,29 @@ function renderComponent(
 
 describe('availability tests', () => {
   const authHandlers = [];
+
   it('is available when window opener is set', () => {
-    window.opener = { postMessage: jest.fn() };
+    const oldWindowOpener = window.opener;
+    // Can't use a spy because window.opener isn't set by default
+    // Still using a var to set the old value, in case that behaviour ever changes
+    window.opener = TestUtils.createMockProxy<Window>({
+      postMessage: jest.fn(),
+    });
+    window.history.pushState(
+      {},
+      'Test Title',
+      `/test.html?authProvider=parent`
+    );
+    expect(AuthPluginParent.isAvailable(authHandlers, authConfigMap)).toBe(
+      true
+    );
+    window.opener = oldWindowOpener;
+  });
+
+  it('is available when window parent is set', () => {
+    const parentSpy = jest.spyOn(window, 'parent', 'get').mockReturnValue(
+      TestUtils.createMockProxy<Window>({ postMessage: jest.fn() })
+    );
     window.history.pushState(
       {},
       'Test Title',
@@ -59,12 +82,16 @@ describe('availability tests', () => {
     expect(AuthPluginParent.isAvailable(authHandlers, authConfigMap)).toBe(
       true
     );
+    parentSpy.mockRestore();
   });
-  it('is not available when window opener not set', () => {
-    delete window.opener;
+
+  it('is not available when window opener and parent are not set', () => {
+    const oldWindowOpener = window.opener;
+    window.opener = null;
     expect(AuthPluginParent.isAvailable(authHandlers, authConfigMap)).toBe(
       false
     );
+    window.opener = oldWindowOpener;
   });
 });
 

packages/auth-plugins/src/AuthPluginParent.tsx

@@ -1,6 +1,7 @@
 import React from 'react';
 import type { LoginOptions } from '@deephaven/jsapi-types';
 import {
+  getWindowParent,
   LOGIN_OPTIONS_REQUEST,
   requestParentResponse,
 } from '@deephaven/jsapi-utils';
@@ -49,7 +50,7 @@ function Component({ children }: AuthPluginProps): JSX.Element {
 const AuthPluginParent: AuthPlugin = {
   Component,
   isAvailable: () =>
-    window.opener != null && getWindowAuthProvider() === 'parent',
+    getWindowParent() != null && getWindowAuthProvider() === 'parent',
 };
 
 export default AuthPluginParent;

packages/jsapi-utils/src/MessageUtils.test.ts

@@ -1,3 +1,4 @@
+import { TestUtils } from '@deephaven/utils';
 import {
   makeMessage,
   makeResponse,
@@ -7,76 +8,113 @@ import {
 
 it('Throws an exception if called on a window without parent', async () => {
   await expect(requestParentResponse('request')).rejects.toThrow(
-    'window.opener is null, unable to send request.'
+    'window parent is null, unable to send request.'
   );
 });
 
-describe('requestParentResponse', () => {
-  let addListenerSpy: jest.SpyInstance;
-  let removeListenerSpy: jest.SpyInstance;
-  let listenerCallback;
-  let messageId;
-  const mockPostMessage = jest.fn((data: Message<unknown>) => {
-    messageId = data.id;
-  });
+/**
+ * Set up the mock for window.parent or window.opener, and return a cleanup function.
+ * @param type Whether to mock window.parent or window.opener
+ * @param mockPostMessage The mock postMessage function to use
+ * @returns Cleanup function
+ */
+function setupWindowParentMock(
+  type: string,
+  mockPostMessage: jest.Mock
+): () => void {
+  if (type !== 'parent' && type !== 'opener') {
+    throw new Error(`Invalid type ${type}`);
+  }
+  if (type === 'parent') {
+    const windowParentSpy = jest.spyOn(window, 'parent', 'get').mockReturnValue(
+      TestUtils.createMockProxy<Window>({
+        postMessage: mockPostMessage,
+      })
+    );
+    return () => {
+      windowParentSpy.mockRestore();
+    };
+  }
+
   const originalWindowOpener = window.opener;
-  beforeEach(() => {
-    addListenerSpy = jest
-      .spyOn(window, 'addEventListener')
-      .mockImplementation((event, cb) => {
-        listenerCallback = cb;
-      });
-    removeListenerSpy = jest.spyOn(window, 'removeEventListener');
-    window.opener = { postMessage: mockPostMessage };
-  });
-  afterEach(() => {
-    addListenerSpy.mockRestore();
-    removeListenerSpy.mockRestore();
-    mockPostMessage.mockClear();
+  window.opener = { postMessage: mockPostMessage };
+  return () => {
     window.opener = originalWindowOpener;
-    messageId = undefined;
-  });
+  };
+}
 
-  it('Posts message to parent and subscribes to response', async () => {
-    requestParentResponse('request');
-    expect(mockPostMessage).toHaveBeenCalledWith(
-      expect.objectContaining(makeMessage('request', messageId)),
-      '*'
-    );
-    expect(addListenerSpy).toHaveBeenCalledWith(
-      'message',
-      expect.any(Function)
-    );
-  });
+describe.each([['parent'], ['opener']])(
+  `requestParentResponse with %s`,
+  type => {
+    let parentCleanup: () => void;
+    let addListenerSpy: jest.SpyInstance;
+    let removeListenerSpy: jest.SpyInstance;
+    let listenerCallback;
+    let messageId;
+    const mockPostMessage = jest.fn((data: Message<unknown>) => {
+      messageId = data.id;
+    });
+    beforeEach(() => {
+      addListenerSpy = jest
+        .spyOn(window, 'addEventListener')
+        .mockImplementation((event, cb) => {
+          listenerCallback = cb;
+        });
+      removeListenerSpy = jest.spyOn(window, 'removeEventListener');
+      parentCleanup = setupWindowParentMock(type, mockPostMessage);
+    });
+    afterEach(() => {
+      addListenerSpy.mockRestore();
+      removeListenerSpy.mockRestore();
+      mockPostMessage.mockClear();
+      parentCleanup();
+      messageId = undefined;
+    });
 
-  it('Resolves with the payload from the parent window response and unsubscribes', async () => {
-    const PAYLOAD = 'PAYLOAD';
-    const promise = requestParentResponse('request');
-    listenerCallback({
-      data: makeResponse(messageId, PAYLOAD),
+    it('Posts message to parent and subscribes to response', async () => {
+      requestParentResponse('request');
+      expect(mockPostMessage).toHaveBeenCalledWith(
+        expect.objectContaining(makeMessage('request', messageId)),
+        '*'
+      );
+      expect(addListenerSpy).toHaveBeenCalledWith(
+        'message',
+        expect.any(Function)
+      );
     });
-    const result = await promise;
-    expect(result).toBe(PAYLOAD);
-    expect(removeListenerSpy).toHaveBeenCalledWith('message', listenerCallback);
-  });
 
-  it('Ignores unrelated response, rejects on timeout', async () => {
-    jest.useFakeTimers();
-    const promise = requestParentResponse('request');
-    listenerCallback({
-      data: makeMessage('wrong-id'),
+    it('Resolves with the payload from the parent window response and unsubscribes', async () => {
+      const PAYLOAD = 'PAYLOAD';
+      const promise = requestParentResponse('request');
+      listenerCallback({
+        data: makeResponse(messageId, PAYLOAD),
+      });
+      const result = await promise;
+      expect(result).toBe(PAYLOAD);
+      expect(removeListenerSpy).toHaveBeenCalledWith(
+        'message',
+        listenerCallback
+      );
     });
-    jest.runOnlyPendingTimers();
-    await expect(promise).rejects.toThrow('Request timed out');
-    jest.useRealTimers();
-  });
 
-  it('Times out if no response', async () => {
-    jest.useFakeTimers();
-    const promise = requestParentResponse('request');
-    jest.runOnlyPendingTimers();
-    expect(removeListenerSpy).toHaveBeenCalled();
-    await expect(promise).rejects.toThrow('Request timed out');
-    jest.useRealTimers();
-  });
-});
+    it('Ignores unrelated response, rejects on timeout', async () => {
+      jest.useFakeTimers();
+      const promise = requestParentResponse('request');
+      listenerCallback({
+        data: makeMessage('wrong-id'),
+      });
+      jest.runOnlyPendingTimers();
+      await expect(promise).rejects.toThrow('Request timed out');
+      jest.useRealTimers();
+    });
+
+    it('Times out if no response', async () => {
+      jest.useFakeTimers();
+      const promise = requestParentResponse('request');
+      jest.runOnlyPendingTimers();
+      expect(removeListenerSpy).toHaveBeenCalled();
+      await expect(promise).rejects.toThrow('Request timed out');
+      jest.useRealTimers();
+    });
+  }
+);

packages/jsapi-utils/src/MessageUtils.ts

@@ -95,6 +95,16 @@ export function makeResponse<T>(messageId: string, payload: T): Response<T> {
   return { id: messageId, payload };
 }
 
+export function getWindowParent(): Window | null {
+  if (window.opener != null) {
+    return window.opener;
+  }
+  if (window.parent != null && window.parent !== window) {
+    return window.parent;
+  }
+  return null;
+}
+
 /**
  * Request data from the parent window and wait for response
  * @param request Request message to send to the parent window
@@ -105,8 +115,9 @@ export async function requestParentResponse(
   request: string,
   timeout = 30000
 ): Promise<unknown> {
-  if (window.opener == null) {
-    throw new Error('window.opener is null, unable to send request.');
+  const parent = getWindowParent();
+  if (parent == null) {
+    throw new Error('window parent is null, unable to send request.');
   }
   return new Promise((resolve, reject) => {
     let timeoutId: number;
@@ -131,6 +142,6 @@ export async function requestParentResponse(
       window.removeEventListener('message', listener);
       reject(new TimeoutError('Request timed out'));
     }, timeout);
-    window.opener.postMessage(makeMessage(request, id), '*');
+    parent.postMessage(makeMessage(request, id), '*');
   });
 }