From d4a912db6b2812040d3b2fef472f9133feff5d84 Mon Sep 17 00:00:00 2001 From: Trevor Harris Date: Thu, 6 Jun 2024 15:59:37 -0700 Subject: [PATCH 1/7] Initial scaffolding for disabling message proxying --- .../teams-js/src/internal/communication.ts | 39 ++++++++++++------- packages/teams-js/src/internal/globalVars.ts | 1 + packages/teams-js/src/internal/handlers.ts | 5 ++- packages/teams-js/src/public/app.ts | 10 +++++ .../teams-js/test/private/privateAPIs.spec.ts | 33 +++++++++++++++- 5 files changed, 69 insertions(+), 19 deletions(-) diff --git a/packages/teams-js/src/internal/communication.ts b/packages/teams-js/src/internal/communication.ts index efe3b5ba91..4bc2580e27 100644 --- a/packages/teams-js/src/internal/communication.ts +++ b/packages/teams-js/src/internal/communication.ts @@ -5,7 +5,7 @@ import { ApiName, ApiVersionNumber, getApiVersionTag } from '../internal/telemetry'; import { FrameContexts } from '../public/constants'; import { SdkError } from '../public/interfaces'; -import { latestRuntimeApiVersion } from '../public/runtime'; +import { latestRuntimeApiVersion, runtime } from '../public/runtime'; import { version } from '../public/version'; import { GlobalVars } from './globalVars'; import { callHandler } from './handlers'; @@ -752,11 +752,15 @@ function isPartialResponse(evt: DOMMessageEvent): boolean { return evt.data.isPartialResponse === true; } +const handleChildMessageLogger = communicationLogger.extend('handleChildMessage'); + /** * @internal * Limited to Microsoft-internal use */ function handleChildMessage(evt: DOMMessageEvent): void { + const logger = handleChildMessageLogger; + if ('id' in evt.data && 'func' in evt.data) { // Try to delegate the request to the proper handler, if defined const message = deserializeMessageRequest(evt.data as SerializedMessageRequest); @@ -766,20 +770,25 @@ function handleChildMessage(evt: DOMMessageEvent): void { // @ts-ignore sendMessageResponseToChild(message.id, message.uuid, Array.isArray(result) ? result : [result]); } else { - // No handler, proxy to parent - sendMessageToParent( - getApiVersionTag(ApiVersionNumber.V_2, ApiName.Tasks_StartTask), - message.func, - message.args, - (...args: any[]): void => { - if (Communication.childWindow) { - const isPartialResponse = args.pop(); - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - sendMessageResponseToChild(message.id, message.uuid, args, isPartialResponse); - } - }, - ); + if (GlobalVars.allowMessageProxy) { + // No handler, proxy to parent + sendMessageToParent( + getApiVersionTag(ApiVersionNumber.V_2, ApiName.Tasks_StartTask), + message.func, + message.args, + (...args: any[]): void => { + if (Communication.childWindow) { + const isPartialResponse = args.pop(); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + sendMessageResponseToChild(message.id, message.uuid, args, isPartialResponse); + runtime; + } + }, + ); + } else { + logger('Message from child not allowed to proxy: %o', message); + } } } } diff --git a/packages/teams-js/src/internal/globalVars.ts b/packages/teams-js/src/internal/globalVars.ts index da630650e6..5c2abf02fd 100644 --- a/packages/teams-js/src/internal/globalVars.ts +++ b/packages/teams-js/src/internal/globalVars.ts @@ -9,4 +9,5 @@ export class GlobalVars { public static hostClientType: string | undefined = undefined; public static clientSupportedSDKVersion: string; public static printCapabilityEnabled = false; + public static allowMessageProxy = false; } diff --git a/packages/teams-js/src/internal/handlers.ts b/packages/teams-js/src/internal/handlers.ts index e8e1d8935b..c6dfa8d3e0 100644 --- a/packages/teams-js/src/internal/handlers.ts +++ b/packages/teams-js/src/internal/handlers.ts @@ -6,6 +6,7 @@ import { LoadContext, ResumeContext } from '../public/interfaces'; import { pages } from '../public/pages'; import { runtime } from '../public/runtime'; import { Communication, sendMessageEventToChild, sendMessageToParent } from './communication'; +import { GlobalVars } from './globalVars'; import { ensureInitialized } from './internalAPIs'; import { getLogger } from './telemetry'; import { isNullOrUndefined } from './typeCheckUtilities'; @@ -86,11 +87,11 @@ export function callHandler(name: string, args?: unknown[]): [true, unknown] | [ callHandlerLogger('Invoking the registered handler for message %s with arguments %o', name, args); const result = handler.apply(this, args); return [true, result]; - } else if (Communication.childWindow) { + } else if (Communication.childWindow && GlobalVars.allowMessageProxy) { sendMessageEventToChild(name, args); return [false, undefined]; } else { - callHandlerLogger('Handler for action message %s not found.', name); + callHandlerLogger('Handler for action message %s not found or message proxy-ing disabled.', name); return [false, undefined]; } } diff --git a/packages/teams-js/src/public/app.ts b/packages/teams-js/src/public/app.ts index 8efd2fa1e9..351d7d75e0 100644 --- a/packages/teams-js/src/public/app.ts +++ b/packages/teams-js/src/public/app.ts @@ -921,6 +921,16 @@ export namespace app { return openLinkHelper(getApiVersionTag(appTelemetryVersionNumber, ApiName.App_OpenLink), deepLink); } + /** + * @hidden + * Undocumented function used to re-enable message proxy-ing for applications that request one. + * + * By default, message proxy-ing is disabled for all applications. + */ + export function setAllowMessageProxy(value: boolean = false): void { + GlobalVars.allowMessageProxy = value; + } + /** * A namespace for enabling the suspension or delayed termination of an app when the user navigates away. * When an app registers for the registerBeforeSuspendOrTerminateHandler, it chooses to delay termination. diff --git a/packages/teams-js/test/private/privateAPIs.spec.ts b/packages/teams-js/test/private/privateAPIs.spec.ts index 886e4aab6c..4ed201d5b6 100644 --- a/packages/teams-js/test/private/privateAPIs.spec.ts +++ b/packages/teams-js/test/private/privateAPIs.spec.ts @@ -402,7 +402,8 @@ describe('AppSDK-privateAPIs', () => { expect(utils.childMessages.length).toBe(1); }); - it('should properly pass partial responses to nested child frames ', async () => { + it('should properly pass partial responses to nested child frames if message proxy-ing is allowed', async () => { + app.setAllowMessageProxy(true); utils.initializeAsFrameless(['https://www.example.com']); // Simulate recieving a child message as a frameless window @@ -434,9 +435,11 @@ describe('AppSDK-privateAPIs', () => { expect(utils.childMessages.length).toBe(3); const thirdChildMessage = utils.childMessages[2]; expect(thirdChildMessage.isPartialResponse).toBeFalsy(); + app.setAllowMessageProxy(false); }); - it('Proxy messages to child window', async () => { + it('Proxy messages to child window if message proxy-ing is allowed', async () => { + app.setAllowMessageProxy(true); await utils.initializeWithContext('content', null, ['https://teams.microsoft.com']); await utils.processMessage({ origin: 'https://outlook.office.com', @@ -453,8 +456,34 @@ describe('AppSDK-privateAPIs', () => { expect(utils.childMessages.length).toBe(1); const childMessage = utils.findMessageInChildByFunc('backButtonClick'); expect(childMessage).not.toBeNull(); + app.setAllowMessageProxy(false); }); + it('Prevent proxy-ing of messages to child window by default', async () => { + await utils.initializeWithContext('content', null, ['https://teams.microsoft.com']); + await utils.processMessage({ + origin: 'https://outlook.office.com', + source: utils.childWindow, + data: { + id: 100, + func: 'backButtonClick', + args: [], + } as MessageResponse, + } as MessageEvent); + + const message = utils.findMessageByFunc('backButtonClick'); + expect(message).toBeNull(); + expect(utils.childMessages.length).toBe(0); + }); + + // Make a second test here + // look into graham change: should I / can I just pull this out and not test it? + // there seems to be a circle in handleChildMessage / callHandler when proxy-ing is turned on: + // if the parent has no handler for a message it receives in callHandler it will just pass that to its child, + // which is the origin of the event in the first place. Yuck. + // Also look at other uses of handleChildMessage in the codebase to make sure no other proxying going on + // verify authentication of messages in handleChildMessage + describe('sendCustomMessage', () => { it('should successfully pass message and provided arguments', async () => { await utils.initializeWithContext('content'); From b23b1947e7d03a28fd01779e6d7ec9efadc1025e Mon Sep 17 00:00:00 2001 From: Trevor Harris Date: Thu, 13 Jun 2024 16:06:23 -0700 Subject: [PATCH 2/7] Remove event proxying entirely --- packages/teams-js/src/internal/handlers.ts | 6 +- .../teams-js/test/private/privateAPIs.spec.ts | 82 +++++++++---------- 2 files changed, 39 insertions(+), 49 deletions(-) diff --git a/packages/teams-js/src/internal/handlers.ts b/packages/teams-js/src/internal/handlers.ts index c6dfa8d3e0..826b2f1c9e 100644 --- a/packages/teams-js/src/internal/handlers.ts +++ b/packages/teams-js/src/internal/handlers.ts @@ -6,7 +6,6 @@ import { LoadContext, ResumeContext } from '../public/interfaces'; import { pages } from '../public/pages'; import { runtime } from '../public/runtime'; import { Communication, sendMessageEventToChild, sendMessageToParent } from './communication'; -import { GlobalVars } from './globalVars'; import { ensureInitialized } from './internalAPIs'; import { getLogger } from './telemetry'; import { isNullOrUndefined } from './typeCheckUtilities'; @@ -87,11 +86,8 @@ export function callHandler(name: string, args?: unknown[]): [true, unknown] | [ callHandlerLogger('Invoking the registered handler for message %s with arguments %o', name, args); const result = handler.apply(this, args); return [true, result]; - } else if (Communication.childWindow && GlobalVars.allowMessageProxy) { - sendMessageEventToChild(name, args); - return [false, undefined]; } else { - callHandlerLogger('Handler for action message %s not found or message proxy-ing disabled.', name); + callHandlerLogger('Handler for action message %s not found.', name); return [false, undefined]; } } diff --git a/packages/teams-js/test/private/privateAPIs.spec.ts b/packages/teams-js/test/private/privateAPIs.spec.ts index 4ed201d5b6..9e97e09d8f 100644 --- a/packages/teams-js/test/private/privateAPIs.spec.ts +++ b/packages/teams-js/test/private/privateAPIs.spec.ts @@ -402,44 +402,27 @@ describe('AppSDK-privateAPIs', () => { expect(utils.childMessages.length).toBe(1); }); - it('should properly pass partial responses to nested child frames if message proxy-ing is allowed', async () => { + it('Proxy messages from child window to parent if message proxy-ing is allowed', async () => { app.setAllowMessageProxy(true); - utils.initializeAsFrameless(['https://www.example.com']); - - // Simulate recieving a child message as a frameless window + await utils.initializeWithContext('content', null, ['https://teams.microsoft.com']); await utils.processMessage({ - origin: 'https://www.example.com', + origin: 'https://outlook.office.com', source: utils.childWindow, data: { id: 100, - func: 'testPartialFunc1', - args: ['testArgs'], + func: 'backButtonClick', + args: [], } as MessageResponse, } as MessageEvent); - // Send a partial response back - const parentMessage = utils.findMessageByFunc('testPartialFunc1'); - await utils.respondToNativeMessage(parentMessage, true, {}); - - // The child window should properly receive the partial response plus - // the original event - expect(utils.childMessages.length).toBe(2); - const secondChildMessage = utils.childMessages[1]; - expect(utils.childMessages[0].func).toBe('testPartialFunc1'); - expect(secondChildMessage.isPartialResponse).toBeTruthy(); - - // Pass the final response (non partial) - await utils.respondToNativeMessage(parentMessage, false, {}); - - // The child window should properly receive the non-partial response - expect(utils.childMessages.length).toBe(3); - const thirdChildMessage = utils.childMessages[2]; - expect(thirdChildMessage.isPartialResponse).toBeFalsy(); + const message = utils.findMessageByFunc('backButtonClick'); + expect(message).not.toBeNull(); + expect(utils.messages.length).toBe(2); + expect(message).not.toBeNull(); app.setAllowMessageProxy(false); }); - it('Proxy messages to child window if message proxy-ing is allowed', async () => { - app.setAllowMessageProxy(true); + it('Do not proxy messages from child window to parent by default', async () => { await utils.initializeWithContext('content', null, ['https://teams.microsoft.com']); await utils.processMessage({ origin: 'https://outlook.office.com', @@ -452,35 +435,46 @@ describe('AppSDK-privateAPIs', () => { } as MessageEvent); const message = utils.findMessageByFunc('backButtonClick'); - expect(message).not.toBeNull(); - expect(utils.childMessages.length).toBe(1); - const childMessage = utils.findMessageInChildByFunc('backButtonClick'); - expect(childMessage).not.toBeNull(); - app.setAllowMessageProxy(false); + expect(message).toBeNull(); + expect(utils.messages.length).toBe(1); }); - it('Prevent proxy-ing of messages to child window by default', async () => { - await utils.initializeWithContext('content', null, ['https://teams.microsoft.com']); + it('should properly pass partial responses to nested child frames if message proxy-ing is allowed', async () => { + app.setAllowMessageProxy(true); + utils.initializeAsFrameless(['https://www.example.com']); + + // Simulate receiving a child message as a frameless window await utils.processMessage({ - origin: 'https://outlook.office.com', + origin: 'https://www.example.com', source: utils.childWindow, data: { id: 100, - func: 'backButtonClick', - args: [], + func: 'testPartialFunc1', + args: ['testArgs'], } as MessageResponse, } as MessageEvent); - const message = utils.findMessageByFunc('backButtonClick'); - expect(message).toBeNull(); - expect(utils.childMessages.length).toBe(0); + // Send a partial response back + const parentMessage = utils.findMessageByFunc('testPartialFunc1'); + await utils.respondToNativeMessage(parentMessage, true, {}); + + // The child window should properly receive the partial response plus + // the original event + expect(utils.childMessages.length).toBe(1); + const firstChildMessage = utils.childMessages[0]; + expect(firstChildMessage.isPartialResponse).toBeTruthy(); + + // Pass the final response (non partial) + await utils.respondToNativeMessage(parentMessage, false, {}); + + // The child window should properly receive the non-partial response + expect(utils.childMessages.length).toBe(2); + const secondChildMessage = utils.childMessages[1]; + expect(secondChildMessage.isPartialResponse).toBeFalsy(); + app.setAllowMessageProxy(false); }); - // Make a second test here // look into graham change: should I / can I just pull this out and not test it? - // there seems to be a circle in handleChildMessage / callHandler when proxy-ing is turned on: - // if the parent has no handler for a message it receives in callHandler it will just pass that to its child, - // which is the origin of the event in the first place. Yuck. // Also look at other uses of handleChildMessage in the codebase to make sure no other proxying going on // verify authentication of messages in handleChildMessage From b332327156b2d0853e2fc316ae3c48973fcb249f Mon Sep 17 00:00:00 2001 From: Trevor Harris Date: Thu, 13 Jun 2024 16:54:40 -0700 Subject: [PATCH 3/7] Remove more event proxying --- packages/teams-js/src/internal/handlers.ts | 12 +-- packages/teams-js/src/public/pages.ts | 13 +--- .../teams-js/test/private/privateAPIs.spec.ts | 1 - packages/teams-js/test/public/pages.spec.ts | 76 ------------------- 4 files changed, 3 insertions(+), 99 deletions(-) diff --git a/packages/teams-js/src/internal/handlers.ts b/packages/teams-js/src/internal/handlers.ts index 826b2f1c9e..b23cf16028 100644 --- a/packages/teams-js/src/internal/handlers.ts +++ b/packages/teams-js/src/internal/handlers.ts @@ -245,17 +245,9 @@ async function handleBeforeUnload(): Promise { if (HandlersPrivate.beforeSuspendOrTerminateHandler) { await HandlersPrivate.beforeSuspendOrTerminateHandler(); - if (Communication.childWindow) { - sendMessageEventToChild('beforeUnload'); - } else { - readyToUnload(); - } + readyToUnload(); } else if (!HandlersPrivate.beforeUnloadHandler || !HandlersPrivate.beforeUnloadHandler(readyToUnload)) { - if (Communication.childWindow) { - sendMessageEventToChild('beforeUnload'); - } else { - readyToUnload(); - } + readyToUnload(); } } diff --git a/packages/teams-js/src/public/pages.ts b/packages/teams-js/src/public/pages.ts index 53b8afb800..380557c64a 100644 --- a/packages/teams-js/src/public/pages.ts +++ b/packages/teams-js/src/public/pages.ts @@ -1,10 +1,8 @@ import { - Communication, sendAndHandleSdkError, sendAndHandleStatusAndReason, sendAndHandleStatusAndReasonWithDefaultError, sendAndUnwrap, - sendMessageEventToChild, sendMessageToParent, } from '../internal/communication'; import { registerHandler, registerHandlerHelper } from '../internal/handlers'; @@ -600,8 +598,6 @@ export namespace pages { const saveEventType = new SaveEventImpl(result); if (saveHandler) { saveHandler(saveEventType); - } else if (Communication.childWindow) { - sendMessageEventToChild('settings.save', [result]); } else { // If no handler is registered, we assume success. saveEventType.notifySuccess(); @@ -710,8 +706,6 @@ export namespace pages { const removeEventType = new RemoveEventImpl(); if (removeHandler) { removeHandler(removeEventType); - } else if (Communication.childWindow) { - sendMessageEventToChild('settings.remove', []); } else { // If no handler is registered, we assume success. removeEventType.notifySuccess(); @@ -843,12 +837,7 @@ export namespace pages { function handleBackButtonPress(): void { if (!backButtonPressHandler || !backButtonPressHandler()) { - if (Communication.childWindow) { - // If the current window did not handle it let the child window - sendMessageEventToChild('backButtonPress', []); - } else { - navigateBack(); - } + navigateBack(); } } diff --git a/packages/teams-js/test/private/privateAPIs.spec.ts b/packages/teams-js/test/private/privateAPIs.spec.ts index 9e97e09d8f..74b0cb4686 100644 --- a/packages/teams-js/test/private/privateAPIs.spec.ts +++ b/packages/teams-js/test/private/privateAPIs.spec.ts @@ -474,7 +474,6 @@ describe('AppSDK-privateAPIs', () => { app.setAllowMessageProxy(false); }); - // look into graham change: should I / can I just pull this out and not test it? // Also look at other uses of handleChildMessage in the codebase to make sure no other proxying going on // verify authentication of messages in handleChildMessage diff --git a/packages/teams-js/test/public/pages.spec.ts b/packages/teams-js/test/public/pages.spec.ts index 286b49b2d1..d310793a0a 100644 --- a/packages/teams-js/test/public/pages.spec.ts +++ b/packages/teams-js/test/public/pages.spec.ts @@ -1,7 +1,6 @@ import { errorLibraryNotInitialized } from '../../src/internal/constants'; import { GlobalVars } from '../../src/internal/globalVars'; import { DOMMessageEvent } from '../../src/internal/interfaces'; -import { MessageResponse } from '../../src/internal/messageObjects'; import { getGenericOnCompleteHandler } from '../../src/internal/utils'; import { app } from '../../src/public/app'; import { errorNotSupportedOnPlatform, FrameContexts } from '../../src/public/constants'; @@ -1262,44 +1261,6 @@ describe('Testing pages module', () => { const message = utils.findMessageByFunc('settings.save.success'); validateRequestWithoutArguments(message, 'settings.save.success'); }); - - it('pages.config.registerOnSaveHandler should proxy to childWindow if no handler in top window', async () => { - await utils.initializeWithContext(context, null, ['https://teams.microsoft.com']); - pages.config.registerOnSaveHandler(undefined); - await utils.processMessage({ - origin: 'https://outlook.office365.com', - source: utils.childWindow, - data: { - id: 100, - func: 'settings.save', - args: [], - } as MessageResponse, - } as MessageEvent); - expect(utils.childMessages.length).toBe(1); - const childMessage = utils.findMessageInChildByFunc('settings.save'); - expect(childMessage).not.toBeNull(); - }); - - it('pages.config.registerOnSaveHandler should not proxy to childWindow if handler in top window', async () => { - await utils.initializeWithContext(context, null, ['https://teams.microsoft.com']); - let handlerCalled = false; - pages.config.registerOnSaveHandler((saveEvent) => { - saveEvent.notifySuccess(); - handlerCalled = true; - }); - expect(handlerCalled).toBe(false); - await utils.processMessage({ - origin: 'https://outlook.office365.com', - source: utils.childWindow, - data: { - id: 100, - func: 'settings.save', - args: [], - } as MessageResponse, - } as MessageEvent); - expect(handlerCalled).toBe(true); - expect(utils.childMessages.length).toBe(0); - }); } else { it(`pages.config.registerOnSaveHandler does not allow calls from ${context} context`, async () => { await utils.initializeWithContext(context); @@ -1363,43 +1324,6 @@ describe('Testing pages module', () => { expect(handlerCalled).toBeTruthy(); }); - it('pages.config.registerOnRemoveHandler should proxy to childWindow if no handler in top window', async () => { - await utils.initializeWithContext(context, null, ['https://teams.microsoft.com']); - pages.config.registerOnRemoveHandler(undefined); - await utils.processMessage({ - origin: 'https://outlook.office365.com', - source: utils.childWindow, - data: { - id: 100, - func: 'settings.remove', - args: [], - } as MessageResponse, - } as MessageEvent); - expect(utils.childMessages.length).toBe(1); - const childMessage = utils.findMessageInChildByFunc('settings.remove'); - expect(childMessage).not.toBeNull(); - }); - - it('pages.config.registerOnRemoveHandler should not proxy to childWindow if handler in top window', async () => { - await utils.initializeWithContext(context, null, ['https://teams.microsoft.com']); - let handlerCalled = false; - pages.config.registerOnRemoveHandler(() => { - handlerCalled = true; - }); - expect(handlerCalled).toBe(false); - await utils.processMessage({ - origin: 'https://outlook.office365.com', - source: utils.childWindow, - data: { - id: 100, - func: 'settings.remove', - args: [], - } as MessageResponse, - } as MessageEvent); - expect(handlerCalled).toBe(true); - expect(utils.childMessages.length).toBe(0); - }); - it(`pages.config.registerOnRemoveHandler should successfully notify success from the registered remove handler when initialized with ${context} context`, async () => { await utils.initializeWithContext(context); From fd64430e9b1f8ef92714de70cc1a3592dd76c5f9 Mon Sep 17 00:00:00 2001 From: Trevor Harris Date: Fri, 14 Jun 2024 15:05:28 -0700 Subject: [PATCH 4/7] Disable more event proxying in the default case --- packages/teams-js/src/internal/handlers.ts | 7 ++--- .../teams-js/test/private/privateAPIs.spec.ts | 26 ++++++++++++++++--- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/packages/teams-js/src/internal/handlers.ts b/packages/teams-js/src/internal/handlers.ts index b23cf16028..12f6438b9a 100644 --- a/packages/teams-js/src/internal/handlers.ts +++ b/packages/teams-js/src/internal/handlers.ts @@ -6,6 +6,7 @@ import { LoadContext, ResumeContext } from '../public/interfaces'; import { pages } from '../public/pages'; import { runtime } from '../public/runtime'; import { Communication, sendMessageEventToChild, sendMessageToParent } from './communication'; +import { GlobalVars } from './globalVars'; import { ensureInitialized } from './internalAPIs'; import { getLogger } from './telemetry'; import { isNullOrUndefined } from './typeCheckUtilities'; @@ -174,7 +175,7 @@ export function handleThemeChange(theme: string): void { HandlersPrivate.themeChangeHandler(theme); } - if (Communication.childWindow) { + if (Communication.childWindow && GlobalVars.allowMessageProxy) { sendMessageEventToChild('themeChange', [theme]); } } @@ -198,12 +199,12 @@ function handleLoad(loadContext: LoadContext): void { const resumeContext = convertToResumeContext(loadContext); if (HandlersPrivate.resumeHandler) { HandlersPrivate.resumeHandler(resumeContext); - if (Communication.childWindow) { + if (Communication.childWindow && GlobalVars.allowMessageProxy) { sendMessageEventToChild('load', [resumeContext]); } } else if (HandlersPrivate.loadHandler) { HandlersPrivate.loadHandler(loadContext); - if (Communication.childWindow) { + if (Communication.childWindow && GlobalVars.allowMessageProxy) { sendMessageEventToChild('load', [loadContext]); } } diff --git a/packages/teams-js/test/private/privateAPIs.spec.ts b/packages/teams-js/test/private/privateAPIs.spec.ts index 74b0cb4686..04e6d083f4 100644 --- a/packages/teams-js/test/private/privateAPIs.spec.ts +++ b/packages/teams-js/test/private/privateAPIs.spec.ts @@ -1,3 +1,4 @@ +import { GlobalVars } from '../../src/internal/globalVars'; import { MessageRequest, MessageResponse } from '../../src/internal/messageObjects'; import { UserSettingTypes, ViewerActionTypes } from '../../src/private/interfaces'; import { @@ -385,9 +386,10 @@ describe('AppSDK-privateAPIs', () => { }); it('should treat messages to frameless windows as coming from the child', async () => { + GlobalVars.allowMessageProxy = true; utils.initializeAsFrameless(['https://www.example.com']); - // Simulate recieving a child message as a frameless window + // Simulate receiving a child message as a frameless window await utils.processMessage({ origin: 'https://www.example.com', source: utils.childWindow, @@ -400,6 +402,25 @@ describe('AppSDK-privateAPIs', () => { // The frameless window should send a response back to the child window expect(utils.childMessages.length).toBe(1); + GlobalVars.allowMessageProxy = false; + }); + + it('post messages to frameless windows should be ignored if message proxy-ing is disabled', async () => { + utils.initializeAsFrameless(['https://www.example.com']); + + // Simulate receiving a child message as a frameless window + await utils.processMessage({ + origin: 'https://www.example.com', + source: utils.childWindow, + data: { + id: 0, + func: 'themeChange', + args: ['testTheme'], + } as MessageResponse, + } as MessageEvent); + + // The message should have been ignored because message proxy-ing is disabled + expect(utils.childMessages.length).toBe(0); }); it('Proxy messages from child window to parent if message proxy-ing is allowed', async () => { @@ -474,8 +495,7 @@ describe('AppSDK-privateAPIs', () => { app.setAllowMessageProxy(false); }); - // Also look at other uses of handleChildMessage in the codebase to make sure no other proxying going on - // verify authentication of messages in handleChildMessage + // Also look at other uses of handleChildMessage in the codebase to make sure no other proxy-ing going on describe('sendCustomMessage', () => { it('should successfully pass message and provided arguments', async () => { From a056a8cee75ed91d76a35803edc4f938d8c3f63c Mon Sep 17 00:00:00 2001 From: Trevor Harris Date: Fri, 14 Jun 2024 15:13:03 -0700 Subject: [PATCH 5/7] Allow message proxying while the auth window is open in the web --- packages/teams-js/src/internal/communication.ts | 2 +- packages/teams-js/src/internal/globalVars.ts | 1 + packages/teams-js/src/internal/handlers.ts | 6 +++--- packages/teams-js/src/public/authentication.ts | 4 ++++ 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/teams-js/src/internal/communication.ts b/packages/teams-js/src/internal/communication.ts index 4bc2580e27..de7fca1357 100644 --- a/packages/teams-js/src/internal/communication.ts +++ b/packages/teams-js/src/internal/communication.ts @@ -770,7 +770,7 @@ function handleChildMessage(evt: DOMMessageEvent): void { // @ts-ignore sendMessageResponseToChild(message.id, message.uuid, Array.isArray(result) ? result : [result]); } else { - if (GlobalVars.allowMessageProxy) { + if (GlobalVars.allowMessageProxy || GlobalVars.webAuthWindowOpen) { // No handler, proxy to parent sendMessageToParent( getApiVersionTag(ApiVersionNumber.V_2, ApiName.Tasks_StartTask), diff --git a/packages/teams-js/src/internal/globalVars.ts b/packages/teams-js/src/internal/globalVars.ts index 5c2abf02fd..51db86feae 100644 --- a/packages/teams-js/src/internal/globalVars.ts +++ b/packages/teams-js/src/internal/globalVars.ts @@ -10,4 +10,5 @@ export class GlobalVars { public static clientSupportedSDKVersion: string; public static printCapabilityEnabled = false; public static allowMessageProxy = false; + public static webAuthWindowOpen = false; } diff --git a/packages/teams-js/src/internal/handlers.ts b/packages/teams-js/src/internal/handlers.ts index 12f6438b9a..746067f1da 100644 --- a/packages/teams-js/src/internal/handlers.ts +++ b/packages/teams-js/src/internal/handlers.ts @@ -175,7 +175,7 @@ export function handleThemeChange(theme: string): void { HandlersPrivate.themeChangeHandler(theme); } - if (Communication.childWindow && GlobalVars.allowMessageProxy) { + if (Communication.childWindow && (GlobalVars.allowMessageProxy || GlobalVars.webAuthWindowOpen)) { sendMessageEventToChild('themeChange', [theme]); } } @@ -199,12 +199,12 @@ function handleLoad(loadContext: LoadContext): void { const resumeContext = convertToResumeContext(loadContext); if (HandlersPrivate.resumeHandler) { HandlersPrivate.resumeHandler(resumeContext); - if (Communication.childWindow && GlobalVars.allowMessageProxy) { + if (Communication.childWindow && (GlobalVars.allowMessageProxy || GlobalVars.webAuthWindowOpen)) { sendMessageEventToChild('load', [resumeContext]); } } else if (HandlersPrivate.loadHandler) { HandlersPrivate.loadHandler(loadContext); - if (Communication.childWindow && GlobalVars.allowMessageProxy) { + if (Communication.childWindow && (GlobalVars.allowMessageProxy || GlobalVars.webAuthWindowOpen)) { sendMessageEventToChild('load', [loadContext]); } } diff --git a/packages/teams-js/src/public/authentication.ts b/packages/teams-js/src/public/authentication.ts index 715cfef4d4..c0a8ccf244 100644 --- a/packages/teams-js/src/public/authentication.ts +++ b/packages/teams-js/src/public/authentication.ts @@ -328,6 +328,7 @@ export namespace authentication { } finally { Communication.childWindow = null; Communication.childOrigin = null; + GlobalVars.webAuthWindowOpen = false; } } @@ -342,6 +343,7 @@ export namespace authentication { function openAuthenticationWindow(authenticateParameters: AuthenticateParameters): void { // Close the previously opened window if we have one closeAuthenticationWindow(); + // Start with a sensible default size let width = authenticateParameters.width || 600; let height = authenticateParameters.height || 400; @@ -364,6 +366,8 @@ export namespace authentication { : Communication.currentWindow.screenY; left += Communication.currentWindow.outerWidth / 2 - width / 2; top += Communication.currentWindow.outerHeight / 2 - height / 2; + + GlobalVars.webAuthWindowOpen = true; // Open a child window with a desired set of standard browser features Communication.childWindow = Communication.currentWindow.open( fullyQualifiedURL.href, From c96bd89f0f3238d38dc01382c0c9a5ca4da17e24 Mon Sep 17 00:00:00 2001 From: Trevor Harris Date: Fri, 14 Jun 2024 16:08:21 -0700 Subject: [PATCH 6/7] Unit test validating the auth window variable is set correctly --- packages/teams-js/src/public/clipboard.ts | 4 ++- .../test/public/authentication.spec.ts | 36 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/packages/teams-js/src/public/clipboard.ts b/packages/teams-js/src/public/clipboard.ts index e1dc7cc557..11f5503da6 100644 --- a/packages/teams-js/src/public/clipboard.ts +++ b/packages/teams-js/src/public/clipboard.ts @@ -103,7 +103,9 @@ export namespace clipboard { if (GlobalVars.isFramelessWindow) { return ensureInitialized(runtime) && runtime.supports.clipboard ? true : false; } else { - return ensureInitialized(runtime) && navigator && navigator.clipboard && runtime.supports.clipboard ? true : false; + return ensureInitialized(runtime) && navigator && navigator.clipboard && runtime.supports.clipboard + ? true + : false; } } } diff --git a/packages/teams-js/test/public/authentication.spec.ts b/packages/teams-js/test/public/authentication.spec.ts index f686e8ce38..026c1d25ed 100644 --- a/packages/teams-js/test/public/authentication.spec.ts +++ b/packages/teams-js/test/public/authentication.spec.ts @@ -114,10 +114,12 @@ describe('Testing authentication capability', () => { beforeEach(() => { // For *almost* all of these tests we want setInterval to be a no-op, so we set it to immediately return 0 utils.mockWindow.setInterval = (handler: Function, timeout: number): number => 0; + GlobalVars.webAuthWindowOpen = false; }); afterEach(() => { // After each test we reset setInterval to its normal value utils.mockWindow.setInterval = (handler: Function, timeout: number): number => setInterval(handler, timeout); + GlobalVars.webAuthWindowOpen = false; }); it('authentication.authenticate should not allow calls before initialization', () => { const authenticationParams: authentication.AuthenticatePopUpParameters = { @@ -178,7 +180,41 @@ describe('Testing authentication capability', () => { authentication.authenticate(authenticationParams); expect(windowOpenCalled).toBe(true); }); + //------------------- + it(`authentication.authenticate proxy-ing`, async () => { + expect.assertions(3); + await utils.initializeWithContext(context); + + jest.spyOn(utils.mockWindow, 'open').mockImplementation((url, name, specsInput): Window => { + const specs: string = specsInput as string; + return utils.childWindow as Window; + }); + jest.spyOn(utils.mockWindow, 'close').mockImplementation((): void => {}); + + const authenticationParams: authentication.AuthenticatePopUpParameters = { + url: 'https://someurl/', + width: 100, + height: 200, + }; + expect(GlobalVars.webAuthWindowOpen).toBe(false); + const authenticatePromise = authentication.authenticate(authenticationParams); + expect(GlobalVars.webAuthWindowOpen).toBe(true); + + utils.processMessage({ + origin: utils.tabOrigin, + source: utils.childWindow, + data: { + id: 10000000, + func: 'authentication.authenticate.success', + args: [mockResult], + }, + } as MessageEvent); + + await authenticatePromise; + expect(GlobalVars.webAuthWindowOpen).toBe(false); + }); + //------------------- it(`authentication.authenticate should cancel the flow when the auth window gets closed before notifySuccess/notifyFailure are called in legacy flow from ${context} context`, async () => { expect.assertions(5); await utils.initializeWithContext(context); From 4515444130edfa498480b83628943376ee09db12 Mon Sep 17 00:00:00 2001 From: Trevor Harris Date: Fri, 14 Jun 2024 17:05:07 -0700 Subject: [PATCH 7/7] Adding unit tests for authentication window proxying --- ...-0aef066c-3765-45df-b2fb-37ca52cef8d6.json | 7 +++ .../test/public/authentication.spec.ts | 48 +++++++++++++++++-- 2 files changed, 50 insertions(+), 5 deletions(-) create mode 100644 change/@microsoft-teams-js-0aef066c-3765-45df-b2fb-37ca52cef8d6.json diff --git a/change/@microsoft-teams-js-0aef066c-3765-45df-b2fb-37ca52cef8d6.json b/change/@microsoft-teams-js-0aef066c-3765-45df-b2fb-37ca52cef8d6.json new file mode 100644 index 0000000000..03fe7cc46c --- /dev/null +++ b/change/@microsoft-teams-js-0aef066c-3765-45df-b2fb-37ca52cef8d6.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Disabling message proxying from child windows and frames by default", + "packageName": "@microsoft/teams-js", + "email": "trharris@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/teams-js/test/public/authentication.spec.ts b/packages/teams-js/test/public/authentication.spec.ts index 026c1d25ed..23833e8b48 100644 --- a/packages/teams-js/test/public/authentication.spec.ts +++ b/packages/teams-js/test/public/authentication.spec.ts @@ -180,9 +180,9 @@ describe('Testing authentication capability', () => { authentication.authenticate(authenticationParams); expect(windowOpenCalled).toBe(true); }); - //------------------- - it(`authentication.authenticate proxy-ing`, async () => { - expect.assertions(3); + + it(`Message proxy-ing works while the auth window is open`, async () => { + expect.assertions(6); await utils.initializeWithContext(context); jest.spyOn(utils.mockWindow, 'open').mockImplementation((url, name, specsInput): Window => { @@ -201,11 +201,26 @@ describe('Testing authentication capability', () => { const authenticatePromise = authentication.authenticate(authenticationParams); expect(GlobalVars.webAuthWindowOpen).toBe(true); + expect(utils.messages.length).toBe(3); + utils + .processMessage({ + origin: utils.tabOrigin, + source: utils.childWindow, + data: { + id: 10000000, + func: 'getContext', + }, + } as MessageEvent) + .then(() => { + expect(utils.messages.length).toBe(4); + expect(utils.findMessageByFunc('getContext')).not.toBeNull(); + }); + utils.processMessage({ origin: utils.tabOrigin, source: utils.childWindow, data: { - id: 10000000, + id: 10000001, func: 'authentication.authenticate.success', args: [mockResult], }, @@ -214,7 +229,30 @@ describe('Testing authentication capability', () => { await authenticatePromise; expect(GlobalVars.webAuthWindowOpen).toBe(false); }); - //------------------- + + it(`Message proxy-ing does not work if the auth window is not open`, async () => { + expect.assertions(5); + await utils.initializeWithContext(context); + + expect(GlobalVars.webAuthWindowOpen).toBe(false); + expect(GlobalVars.allowMessageProxy).toBe(false); + + expect(utils.messages.length).toBe(1); + utils + .processMessage({ + origin: utils.tabOrigin, + source: utils.childWindow, + data: { + id: 10000000, + func: 'getContext', + }, + } as MessageEvent) + .then(() => { + expect(utils.messages.length).toBe(1); + expect(utils.findMessageByFunc('getContext')).toBeNull(); + }); + }); + it(`authentication.authenticate should cancel the flow when the auth window gets closed before notifySuccess/notifyFailure are called in legacy flow from ${context} context`, async () => { expect.assertions(5); await utils.initializeWithContext(context);