From 8c5c987ae5cfec9191285fcb11854d9dd1e93e94 Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Fri, 24 May 2024 10:16:26 +0200 Subject: [PATCH 1/4] fix: only assume we are in iframe mode if the origin of the opener matches the current origin --- src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts b/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts index 2fae365b92..b614d5acd4 100644 --- a/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts +++ b/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts @@ -77,7 +77,11 @@ export class UmbAppElement extends UmbLitElement { // The authorization request will be completed in the active window (main or popup) and the authorization signal will be sent. // If we are in a popup window, the storage event in UmbAuthContext will catch the signal and close the window. // If we are in the main window, the signal will be caught right here and the user will be redirected to the root. - if (window.opener) { + if ( + window.opener && + window.opener instanceof Window && + window.opener.location.origin === window.location.origin + ) { (component as UmbAppErrorElement).errorMessage = hasCode ? this.localize.term('errors_externalLoginSuccess') : this.localize.term('errors_externalLoginFailed'); From c8d3772635aa5d05662106ea64741d8abe38f4d8 Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Fri, 24 May 2024 10:57:24 +0200 Subject: [PATCH 2/4] fix: make sure the opener came from within the backoffice itself --- src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts b/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts index b614d5acd4..3e4b82e0cf 100644 --- a/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts +++ b/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts @@ -79,8 +79,9 @@ export class UmbAppElement extends UmbLitElement { // If we are in the main window, the signal will be caught right here and the user will be redirected to the root. if ( window.opener && - window.opener instanceof Window && - window.opener.location.origin === window.location.origin + window.opener !== window && + window.opener.location.origin === window.location.origin && + window.opener.location.pathname.startsWith(this.backofficePath) ) { (component as UmbAppErrorElement).errorMessage = hasCode ? this.localize.term('errors_externalLoginSuccess') From a0be30d77bff421feca0e9c4760c2574f315198d Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Fri, 24 May 2024 11:35:30 +0200 Subject: [PATCH 3/4] add a function to test if we have a safe opener --- .../src/packages/core/utils/index.ts | 1 + .../path/has-own-opener.function.test.ts | 64 +++++++++++++++++++ .../utils/path/has-own-opener.function.ts | 35 ++++++++++ 3 files changed, 100 insertions(+) create mode 100644 src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.test.ts create mode 100644 src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.ts diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/index.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/index.ts index 27a30b45fc..f7ec7dd723 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/utils/index.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/index.ts @@ -7,6 +7,7 @@ export * from './object/deep-merge.function.js'; export * from './pagination-manager/pagination.manager.js'; export * from './path/ensure-local-path.function.js'; export * from './path/ensure-path-ends-with-slash.function.js'; +export * from './path/has-own-opener.function.js'; export * from './path/path-decode.function.js'; export * from './path/path-encode.function.js'; export * from './path/path-folder-name.function.js'; diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.test.ts new file mode 100644 index 0000000000..5804cd1964 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.test.ts @@ -0,0 +1,64 @@ +import { expect } from '@open-wc/testing'; +import { hasOwnOpener } from './has-own-opener.function.js'; + +describe('hasOwnOpener', () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let mockWindow: any; + + beforeEach(() => { + mockWindow = { + location: { + origin: 'http://localhost', + pathname: '/test', + }, + }; + }); + + it('should return false if there is no opener', () => { + expect(hasOwnOpener(undefined, mockWindow)).to.be.false; + }); + + it('should return false if the opener is from a different origin', () => { + mockWindow.opener = { + location: { + origin: 'https://example.com', + pathname: '/test', + }, + }; + + expect(hasOwnOpener(undefined, mockWindow)).to.be.false; + }); + + it('should return true if the opener is from the same origin and no pathname is specified', () => { + mockWindow.opener = { + location: { + origin: 'http://localhost', + pathname: '/test', + }, + }; + + expect(hasOwnOpener(undefined, mockWindow)).to.be.true; + }); + + it('should return false if the opener is from the same origin but has a different pathname', () => { + mockWindow.opener = { + location: { + origin: 'http://localhost', + pathname: '/different', + }, + }; + + expect(hasOwnOpener('/test', mockWindow)).to.be.false; + }); + + it('should return true if the opener is from the same origin and has the same pathname', () => { + mockWindow.opener = { + location: { + origin: 'http://localhost', + pathname: '/test', + }, + }; + + expect(hasOwnOpener('/test', mockWindow)).to.be.true; + }); +}); diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.ts new file mode 100644 index 0000000000..a1763d6214 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/has-own-opener.function.ts @@ -0,0 +1,35 @@ +/** + * Check if the current window has an opener window with the same origin and optional pathname. + * This is useful for checking if the current window was opened by another window from within the same application. + * @remark If the current window was opened by another window, the opener window is accessible via `window.opener`. + * @remark There could still be an opener if the opener window is closed or navigated away or if the opener window is not from the same origin, + * but this function will only return `true` if the opener window is accessible and has the same origin and optional pathname. + * + * @param pathname Optional pathname to check if the opener window has the same pathname. + * @param windowLike The window-like object to use for checking the opener. Default is `window`. + * @returns `true` if the current window has an opener window with the same origin and optional pathname, otherwise `false`. + */ +export function hasOwnOpener(pathname?: string, windowLike: Window = globalThis.window): boolean { + try { + const opener = windowLike.opener; + if (!opener) { + return false; + } + + const openerLocation = opener.location; + const currentLocation = windowLike.location; + + if (openerLocation.origin !== currentLocation.origin) { + return false; + } + + if (pathname && openerLocation.pathname !== pathname) { + return false; + } + + return true; + } catch (e) { + // If there is a security error, it means that the opener is from a different origin, so we let it fall through + return false; + } +} From bd17020e1a15eef6cf1f64f957b86077456d6884 Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Fri, 24 May 2024 11:35:51 +0200 Subject: [PATCH 4/4] use the `hasOwnOpener` function to test if the request came from within the backoffice, which lets us know that we should stay on the page --- src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts b/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts index 3e4b82e0cf..2725ec1ab6 100644 --- a/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts +++ b/src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts @@ -18,7 +18,7 @@ import { umbExtensionsRegistry, } from '@umbraco-cms/backoffice/extension-registry'; import { filter, first, firstValueFrom } from '@umbraco-cms/backoffice/external/rxjs'; -import { retrieveStoredPath } from '@umbraco-cms/backoffice/utils'; +import { hasOwnOpener, retrieveStoredPath } from '@umbraco-cms/backoffice/utils'; @customElement('umb-app') export class UmbAppElement extends UmbLitElement { @@ -77,12 +77,7 @@ export class UmbAppElement extends UmbLitElement { // The authorization request will be completed in the active window (main or popup) and the authorization signal will be sent. // If we are in a popup window, the storage event in UmbAuthContext will catch the signal and close the window. // If we are in the main window, the signal will be caught right here and the user will be redirected to the root. - if ( - window.opener && - window.opener !== window && - window.opener.location.origin === window.location.origin && - window.opener.location.pathname.startsWith(this.backofficePath) - ) { + if (hasOwnOpener(this.backofficePath)) { (component as UmbAppErrorElement).errorMessage = hasCode ? this.localize.term('errors_externalLoginSuccess') : this.localize.term('errors_externalLoginFailed');