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/src/internal/communication.ts b/packages/teams-js/src/internal/communication.ts index efe3b5ba91..de7fca1357 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 || GlobalVars.webAuthWindowOpen) { + // 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..51db86feae 100644 --- a/packages/teams-js/src/internal/globalVars.ts +++ b/packages/teams-js/src/internal/globalVars.ts @@ -9,4 +9,6 @@ export class GlobalVars { public static hostClientType: string | undefined = undefined; 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 e8e1d8935b..746067f1da 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,9 +87,6 @@ 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) { - sendMessageEventToChild(name, args); - return [false, undefined]; } else { callHandlerLogger('Handler for action message %s not found.', name); return [false, undefined]; @@ -177,7 +175,7 @@ export function handleThemeChange(theme: string): void { HandlersPrivate.themeChangeHandler(theme); } - if (Communication.childWindow) { + if (Communication.childWindow && (GlobalVars.allowMessageProxy || GlobalVars.webAuthWindowOpen)) { sendMessageEventToChild('themeChange', [theme]); } } @@ -201,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 || GlobalVars.webAuthWindowOpen)) { sendMessageEventToChild('load', [resumeContext]); } } else if (HandlersPrivate.loadHandler) { HandlersPrivate.loadHandler(loadContext); - if (Communication.childWindow) { + if (Communication.childWindow && (GlobalVars.allowMessageProxy || GlobalVars.webAuthWindowOpen)) { sendMessageEventToChild('load', [loadContext]); } } @@ -248,17 +246,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/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/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, 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/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 886e4aab6c..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,43 +402,48 @@ 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('should properly pass partial responses to nested child frames ', async () => { + it('post messages to frameless windows should be ignored if message proxy-ing is disabled', async () => { 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, data: { - id: 100, - func: 'testPartialFunc1', - args: ['testArgs'], + id: 0, + func: 'themeChange', + args: ['testTheme'], } 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(); + // The message should have been ignored because message proxy-ing is disabled + expect(utils.childMessages.length).toBe(0); + }); - // Pass the final response (non partial) - await utils.respondToNativeMessage(parentMessage, false, {}); + it('Proxy messages from child window to parent 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', + source: utils.childWindow, + data: { + id: 100, + func: 'backButtonClick', + args: [], + } as MessageResponse, + } as MessageEvent); - // 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', async () => { + 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', @@ -449,12 +456,47 @@ describe('AppSDK-privateAPIs', () => { } as MessageEvent); const message = utils.findMessageByFunc('backButtonClick'); - expect(message).not.toBeNull(); + expect(message).toBeNull(); + expect(utils.messages.length).toBe(1); + }); + + 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://www.example.com', + source: utils.childWindow, + data: { + id: 100, + func: 'testPartialFunc1', + args: ['testArgs'], + } 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(1); - const childMessage = utils.findMessageInChildByFunc('backButtonClick'); - expect(childMessage).not.toBeNull(); + 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); }); + // 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 () => { await utils.initializeWithContext('content'); diff --git a/packages/teams-js/test/public/authentication.spec.ts b/packages/teams-js/test/public/authentication.spec.ts index f686e8ce38..23833e8b48 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 = { @@ -179,6 +181,78 @@ describe('Testing authentication capability', () => { expect(windowOpenCalled).toBe(true); }); + 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 => { + 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); + + 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: 10000001, + func: 'authentication.authenticate.success', + args: [mockResult], + }, + } as MessageEvent); + + 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); 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);