Skip to content

Commit e23695d

Browse files
mofojedmofojed
authored
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
1 parent 0830099 commit e23695d

5 files changed

Lines changed: 147 additions & 70 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ export type AuthBootstrapProps = {
2121

2222
/** Core auth plugins that are always loaded */
2323
const CORE_AUTH_PLUGINS = new Map([
24-
['@deephaven/auth-plugins.AuthPluginPsk', AuthPluginPsk],
2524
['@deephaven/auth-plugins.AuthPluginParent', AuthPluginParent],
25+
['@deephaven/auth-plugins.AuthPluginPsk', AuthPluginPsk],
2626
['@deephaven/auth-plugins.AuthPluginAnonymous', AuthPluginAnonymous],
2727
]);
2828

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

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@ import { act, render, screen } from '@testing-library/react';
33
import { ApiContext, ClientContext } from '@deephaven/jsapi-bootstrap';
44
import { dh } from '@deephaven/jsapi-shim';
55
import type { CoreClient, LoginOptions } from '@deephaven/jsapi-types';
6+
import { TestUtils } from '@deephaven/utils';
67
import AuthPluginParent from './AuthPluginParent';
78
import { AuthConfigMap } from './AuthPlugin';
89

910
let mockParentResponse: Promise<LoginOptions>;
1011
jest.mock('@deephaven/jsapi-utils', () => ({
12+
...jest.requireActual('@deephaven/jsapi-utils'),
1113
LOGIN_OPTIONS_REQUEST: 'mock-login-options-request',
1214
requestParentResponse: jest.fn(() => mockParentResponse),
1315
}));
@@ -49,8 +51,29 @@ function renderComponent(
4951

5052
describe('availability tests', () => {
5153
const authHandlers = [];
54+
5255
it('is available when window opener is set', () => {
53-
window.opener = { postMessage: jest.fn() };
56+
const oldWindowOpener = window.opener;
57+
// Can't use a spy because window.opener isn't set by default
58+
// Still using a var to set the old value, in case that behaviour ever changes
59+
window.opener = TestUtils.createMockProxy<Window>({
60+
postMessage: jest.fn(),
61+
});
62+
window.history.pushState(
63+
{},
64+
'Test Title',
65+
`/test.html?authProvider=parent`
66+
);
67+
expect(AuthPluginParent.isAvailable(authHandlers, authConfigMap)).toBe(
68+
true
69+
);
70+
window.opener = oldWindowOpener;
71+
});
72+
73+
it('is available when window parent is set', () => {
74+
const parentSpy = jest.spyOn(window, 'parent', 'get').mockReturnValue(
75+
TestUtils.createMockProxy<Window>({ postMessage: jest.fn() })
76+
);
5477
window.history.pushState(
5578
{},
5679
'Test Title',
@@ -59,12 +82,16 @@ describe('availability tests', () => {
5982
expect(AuthPluginParent.isAvailable(authHandlers, authConfigMap)).toBe(
6083
true
6184
);
85+
parentSpy.mockRestore();
6286
});
63-
it('is not available when window opener not set', () => {
64-
delete window.opener;
87+
88+
it('is not available when window opener and parent are not set', () => {
89+
const oldWindowOpener = window.opener;
90+
window.opener = null;
6591
expect(AuthPluginParent.isAvailable(authHandlers, authConfigMap)).toBe(
6692
false
6793
);
94+
window.opener = oldWindowOpener;
6895
});
6996
});
7097

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import React from 'react';
22
import type { LoginOptions } from '@deephaven/jsapi-types';
33
import {
4+
getWindowParent,
45
LOGIN_OPTIONS_REQUEST,
56
requestParentResponse,
67
} from '@deephaven/jsapi-utils';
@@ -49,7 +50,7 @@ function Component({ children }: AuthPluginProps): JSX.Element {
4950
const AuthPluginParent: AuthPlugin = {
5051
Component,
5152
isAvailable: () =>
52-
window.opener != null && getWindowAuthProvider() === 'parent',
53+
getWindowParent() != null && getWindowAuthProvider() === 'parent',
5354
};
5455

5556
export default AuthPluginParent;

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

Lines changed: 100 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { TestUtils } from '@deephaven/utils';
12
import {
23
makeMessage,
34
makeResponse,
@@ -7,76 +8,113 @@ import {
78

89
it('Throws an exception if called on a window without parent', async () => {
910
await expect(requestParentResponse('request')).rejects.toThrow(
10-
'window.opener is null, unable to send request.'
11+
'window parent is null, unable to send request.'
1112
);
1213
});
1314

14-
describe('requestParentResponse', () => {
15-
let addListenerSpy: jest.SpyInstance;
16-
let removeListenerSpy: jest.SpyInstance;
17-
let listenerCallback;
18-
let messageId;
19-
const mockPostMessage = jest.fn((data: Message<unknown>) => {
20-
messageId = data.id;
21-
});
15+
/**
16+
* Set up the mock for window.parent or window.opener, and return a cleanup function.
17+
* @param type Whether to mock window.parent or window.opener
18+
* @param mockPostMessage The mock postMessage function to use
19+
* @returns Cleanup function
20+
*/
21+
function setupWindowParentMock(
22+
type: string,
23+
mockPostMessage: jest.Mock
24+
): () => void {
25+
if (type !== 'parent' && type !== 'opener') {
26+
throw new Error(`Invalid type ${type}`);
27+
}
28+
if (type === 'parent') {
29+
const windowParentSpy = jest.spyOn(window, 'parent', 'get').mockReturnValue(
30+
TestUtils.createMockProxy<Window>({
31+
postMessage: mockPostMessage,
32+
})
33+
);
34+
return () => {
35+
windowParentSpy.mockRestore();
36+
};
37+
}
38+
2239
const originalWindowOpener = window.opener;
23-
beforeEach(() => {
24-
addListenerSpy = jest
25-
.spyOn(window, 'addEventListener')
26-
.mockImplementation((event, cb) => {
27-
listenerCallback = cb;
28-
});
29-
removeListenerSpy = jest.spyOn(window, 'removeEventListener');
30-
window.opener = { postMessage: mockPostMessage };
31-
});
32-
afterEach(() => {
33-
addListenerSpy.mockRestore();
34-
removeListenerSpy.mockRestore();
35-
mockPostMessage.mockClear();
40+
window.opener = { postMessage: mockPostMessage };
41+
return () => {
3642
window.opener = originalWindowOpener;
37-
messageId = undefined;
38-
});
43+
};
44+
}
3945

40-
it('Posts message to parent and subscribes to response', async () => {
41-
requestParentResponse('request');
42-
expect(mockPostMessage).toHaveBeenCalledWith(
43-
expect.objectContaining(makeMessage('request', messageId)),
44-
'*'
45-
);
46-
expect(addListenerSpy).toHaveBeenCalledWith(
47-
'message',
48-
expect.any(Function)
49-
);
50-
});
46+
describe.each([['parent'], ['opener']])(
47+
`requestParentResponse with %s`,
48+
type => {
49+
let parentCleanup: () => void;
50+
let addListenerSpy: jest.SpyInstance;
51+
let removeListenerSpy: jest.SpyInstance;
52+
let listenerCallback;
53+
let messageId;
54+
const mockPostMessage = jest.fn((data: Message<unknown>) => {
55+
messageId = data.id;
56+
});
57+
beforeEach(() => {
58+
addListenerSpy = jest
59+
.spyOn(window, 'addEventListener')
60+
.mockImplementation((event, cb) => {
61+
listenerCallback = cb;
62+
});
63+
removeListenerSpy = jest.spyOn(window, 'removeEventListener');
64+
parentCleanup = setupWindowParentMock(type, mockPostMessage);
65+
});
66+
afterEach(() => {
67+
addListenerSpy.mockRestore();
68+
removeListenerSpy.mockRestore();
69+
mockPostMessage.mockClear();
70+
parentCleanup();
71+
messageId = undefined;
72+
});
5173

52-
it('Resolves with the payload from the parent window response and unsubscribes', async () => {
53-
const PAYLOAD = 'PAYLOAD';
54-
const promise = requestParentResponse('request');
55-
listenerCallback({
56-
data: makeResponse(messageId, PAYLOAD),
74+
it('Posts message to parent and subscribes to response', async () => {
75+
requestParentResponse('request');
76+
expect(mockPostMessage).toHaveBeenCalledWith(
77+
expect.objectContaining(makeMessage('request', messageId)),
78+
'*'
79+
);
80+
expect(addListenerSpy).toHaveBeenCalledWith(
81+
'message',
82+
expect.any(Function)
83+
);
5784
});
58-
const result = await promise;
59-
expect(result).toBe(PAYLOAD);
60-
expect(removeListenerSpy).toHaveBeenCalledWith('message', listenerCallback);
61-
});
6285

63-
it('Ignores unrelated response, rejects on timeout', async () => {
64-
jest.useFakeTimers();
65-
const promise = requestParentResponse('request');
66-
listenerCallback({
67-
data: makeMessage('wrong-id'),
86+
it('Resolves with the payload from the parent window response and unsubscribes', async () => {
87+
const PAYLOAD = 'PAYLOAD';
88+
const promise = requestParentResponse('request');
89+
listenerCallback({
90+
data: makeResponse(messageId, PAYLOAD),
91+
});
92+
const result = await promise;
93+
expect(result).toBe(PAYLOAD);
94+
expect(removeListenerSpy).toHaveBeenCalledWith(
95+
'message',
96+
listenerCallback
97+
);
6898
});
69-
jest.runOnlyPendingTimers();
70-
await expect(promise).rejects.toThrow('Request timed out');
71-
jest.useRealTimers();
72-
});
7399

74-
it('Times out if no response', async () => {
75-
jest.useFakeTimers();
76-
const promise = requestParentResponse('request');
77-
jest.runOnlyPendingTimers();
78-
expect(removeListenerSpy).toHaveBeenCalled();
79-
await expect(promise).rejects.toThrow('Request timed out');
80-
jest.useRealTimers();
81-
});
82-
});
100+
it('Ignores unrelated response, rejects on timeout', async () => {
101+
jest.useFakeTimers();
102+
const promise = requestParentResponse('request');
103+
listenerCallback({
104+
data: makeMessage('wrong-id'),
105+
});
106+
jest.runOnlyPendingTimers();
107+
await expect(promise).rejects.toThrow('Request timed out');
108+
jest.useRealTimers();
109+
});
110+
111+
it('Times out if no response', async () => {
112+
jest.useFakeTimers();
113+
const promise = requestParentResponse('request');
114+
jest.runOnlyPendingTimers();
115+
expect(removeListenerSpy).toHaveBeenCalled();
116+
await expect(promise).rejects.toThrow('Request timed out');
117+
jest.useRealTimers();
118+
});
119+
}
120+
);

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,16 @@ export function makeResponse<T>(messageId: string, payload: T): Response<T> {
9595
return { id: messageId, payload };
9696
}
9797

98+
export function getWindowParent(): Window | null {
99+
if (window.opener != null) {
100+
return window.opener;
101+
}
102+
if (window.parent != null && window.parent !== window) {
103+
return window.parent;
104+
}
105+
return null;
106+
}
107+
98108
/**
99109
* Request data from the parent window and wait for response
100110
* @param request Request message to send to the parent window
@@ -105,8 +115,9 @@ export async function requestParentResponse(
105115
request: string,
106116
timeout = 30000
107117
): Promise<unknown> {
108-
if (window.opener == null) {
109-
throw new Error('window.opener is null, unable to send request.');
118+
const parent = getWindowParent();
119+
if (parent == null) {
120+
throw new Error('window parent is null, unable to send request.');
110121
}
111122
return new Promise((resolve, reject) => {
112123
let timeoutId: number;
@@ -131,6 +142,6 @@ export async function requestParentResponse(
131142
window.removeEventListener('message', listener);
132143
reject(new TimeoutError('Request timed out'));
133144
}, timeout);
134-
window.opener.postMessage(makeMessage(request, id), '*');
145+
parent.postMessage(makeMessage(request, id), '*');
135146
});
136147
}

0 commit comments

Comments
 (0)
⚡