From 5bab6ca96bf107a634ce844f3d0fa20d0f000ea0 Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Wed, 22 May 2024 10:17:56 +0200 Subject: [PATCH 1/4] feat: if basic auth, delete the stored token --- src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts | 8 ++++++-- 1 file changed, 6 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 c689e1e66a..10aec07725 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 @@ -173,9 +173,13 @@ export class UmbAppElement extends UmbLitElement { // Try to initialise the auth flow and get the runtime status try { - // If the runtime level is "install" we should clear any cached tokens + // If the runtime level is "install" or ?status=false is set, we should clear any cached tokens // else we should try and set the auth status - if (this.#serverConnection.getStatus() === RuntimeLevelModel.INSTALL) { + const searchParams = new URLSearchParams(window.location.search); + if ( + (searchParams.has('status') && searchParams.get('status') === 'false') || + this.#serverConnection.getStatus() === RuntimeLevelModel.INSTALL + ) { await this.#authContext.clearTokenStorage(); } else { await this.#setAuthStatus(); From 5c5b0e35d95982fe636748810290f2d6a268dc1b Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Wed, 22 May 2024 10:18:12 +0200 Subject: [PATCH 2/4] feat: accept a ?returnPath that overrides the stored path --- .../src/apps/app/app-auth.controller.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Web.UI.Client/src/apps/app/app-auth.controller.ts b/src/Umbraco.Web.UI.Client/src/apps/app/app-auth.controller.ts index 13a928e69c..fbf49db6ce 100644 --- a/src/Umbraco.Web.UI.Client/src/apps/app/app-auth.controller.ts +++ b/src/Umbraco.Web.UI.Client/src/apps/app/app-auth.controller.ts @@ -71,7 +71,13 @@ export class UmbAppAuthController extends UmbControllerBase { } // Save the current state - sessionStorage.setItem(UMB_STORAGE_REDIRECT_URL, window.location.href); + let currentUrl = window.location.href; + const searchParams = new URLSearchParams(window.location.search); + if (searchParams.has('returnPath')) { + currentUrl = decodeURIComponent(searchParams.get('returnPath') || currentUrl); + } + const safeUrl = new URL(currentUrl, window.location.origin); + sessionStorage.setItem(UMB_STORAGE_REDIRECT_URL, safeUrl.toString()); // Figure out which providers are available const availableProviders = await firstValueFrom(this.#authContext.getAuthProviders(umbExtensionsRegistry)); From 44471d89d4a8a943ae0f9ab6fe9ecf8dc43a355c Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Wed, 22 May 2024 11:43:54 +0200 Subject: [PATCH 3/4] feat: add protection and figure out if the redirect url is inside or outside backoffice --- src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts | 10 +++++++++- 1 file changed, 9 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 10aec07725..5a4fcc7112 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 @@ -93,7 +93,15 @@ export class UmbAppElement extends UmbLitElement { sessionStorage.removeItem(UMB_STORAGE_REDIRECT_URL); currentRoute = savedRoute.endsWith('logout') ? currentRoute : savedRoute; } - history.replaceState(null, '', currentRoute); + + const url = new URL(currentRoute); + const isLocalRoute = url.origin === window.location.origin && url.pathname.startsWith(this.backofficePath); + + if (isLocalRoute) { + history.replaceState(null, '', url.pathname + url.search + url.hash); + } else { + window.location.href = url.toString(); + } }); } From 8411b9573469f4da5a7de9fad0039cdc76477c6f Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Wed, 22 May 2024 13:51:35 +0200 Subject: [PATCH 4/4] move logic out into functions and add tests for those --- .../src/apps/app/app-auth.controller.ts | 11 +--- .../src/apps/app/app.element.ts | 20 +++---- .../src/packages/core/utils/index.ts | 2 + .../path/ensure-local-path.function.test.ts | 26 +++++++++ .../utils/path/ensure-local-path.function.ts | 10 ++++ .../utils/path/stored-path.function.test.ts | 56 +++++++++++++++++++ .../core/utils/path/stored-path.function.ts | 30 ++++++++++ 7 files changed, 134 insertions(+), 21 deletions(-) create mode 100644 src/Umbraco.Web.UI.Client/src/packages/core/utils/path/ensure-local-path.function.test.ts create mode 100644 src/Umbraco.Web.UI.Client/src/packages/core/utils/path/ensure-local-path.function.ts create mode 100644 src/Umbraco.Web.UI.Client/src/packages/core/utils/path/stored-path.function.test.ts create mode 100644 src/Umbraco.Web.UI.Client/src/packages/core/utils/path/stored-path.function.ts diff --git a/src/Umbraco.Web.UI.Client/src/apps/app/app-auth.controller.ts b/src/Umbraco.Web.UI.Client/src/apps/app/app-auth.controller.ts index fbf49db6ce..b893575d90 100644 --- a/src/Umbraco.Web.UI.Client/src/apps/app/app-auth.controller.ts +++ b/src/Umbraco.Web.UI.Client/src/apps/app/app-auth.controller.ts @@ -1,14 +1,10 @@ -import { - UMB_AUTH_CONTEXT, - UMB_MODAL_APP_AUTH, - UMB_STORAGE_REDIRECT_URL, - type UmbUserLoginState, -} from '@umbraco-cms/backoffice/auth'; +import { UMB_AUTH_CONTEXT, UMB_MODAL_APP_AUTH, type UmbUserLoginState } from '@umbraco-cms/backoffice/auth'; import { UmbControllerBase } from '@umbraco-cms/backoffice/class-api'; import type { UmbControllerHost } from '@umbraco-cms/backoffice/controller-api'; import { firstValueFrom } from '@umbraco-cms/backoffice/external/rxjs'; import { umbExtensionsRegistry } from '@umbraco-cms/backoffice/extension-registry'; import { UMB_MODAL_MANAGER_CONTEXT } from '@umbraco-cms/backoffice/modal'; +import { setStoredPath } from '@umbraco-cms/backoffice/utils'; export class UmbAppAuthController extends UmbControllerBase { #authContext?: typeof UMB_AUTH_CONTEXT.TYPE; @@ -76,8 +72,7 @@ export class UmbAppAuthController extends UmbControllerBase { if (searchParams.has('returnPath')) { currentUrl = decodeURIComponent(searchParams.get('returnPath') || currentUrl); } - const safeUrl = new URL(currentUrl, window.location.origin); - sessionStorage.setItem(UMB_STORAGE_REDIRECT_URL, safeUrl.toString()); + setStoredPath(currentUrl); // Figure out which providers are available const availableProviders = await firstValueFrom(this.#authContext.getAuthProviders(umbExtensionsRegistry)); 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 5a4fcc7112..2fae365b92 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 @@ -4,7 +4,7 @@ import { UmbAppContext } from './app.context.js'; import { UmbServerConnection } from './server-connection.js'; import { UmbAppAuthController } from './app-auth.controller.js'; import type { UMB_AUTH_CONTEXT } from '@umbraco-cms/backoffice/auth'; -import { UMB_STORAGE_REDIRECT_URL, UmbAuthContext } from '@umbraco-cms/backoffice/auth'; +import { UmbAuthContext } from '@umbraco-cms/backoffice/auth'; import { css, html, customElement, property } from '@umbraco-cms/backoffice/external/lit'; import { UUIIconRegistryEssential } from '@umbraco-cms/backoffice/external/uui'; import { UmbLitElement } from '@umbraco-cms/backoffice/lit-element'; @@ -18,6 +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'; @customElement('umb-app') export class UmbAppElement extends UmbLitElement { @@ -87,20 +88,13 @@ export class UmbAppElement extends UmbLitElement { this.observe(this.#authContext.authorizationSignal, () => { // Redirect to the saved state or root - let currentRoute = ''; - const savedRoute = sessionStorage.getItem(UMB_STORAGE_REDIRECT_URL); - if (savedRoute) { - sessionStorage.removeItem(UMB_STORAGE_REDIRECT_URL); - currentRoute = savedRoute.endsWith('logout') ? currentRoute : savedRoute; - } + const url = retrieveStoredPath(); + const isBackofficePath = url?.pathname.startsWith(this.backofficePath) ?? true; - const url = new URL(currentRoute); - const isLocalRoute = url.origin === window.location.origin && url.pathname.startsWith(this.backofficePath); - - if (isLocalRoute) { - history.replaceState(null, '', url.pathname + url.search + url.hash); + if (isBackofficePath) { + history.replaceState(null, '', url?.toString() ?? ''); } else { - window.location.href = url.toString(); + window.location.href = url?.toString() ?? this.backofficePath; } }); } 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 59f441f2ba..27a30b45fc 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 @@ -5,10 +5,12 @@ export * from './get-processed-image-url.function.js'; export * from './math/math.js'; 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/path-decode.function.js'; export * from './path/path-encode.function.js'; export * from './path/path-folder-name.function.js'; +export * from './path/stored-path.function.js'; export * from './path/umbraco-path.function.js'; export * from './path/url-pattern-to-string.function.js'; export * from './selection-manager/selection.manager.js'; diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/ensure-local-path.function.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/ensure-local-path.function.test.ts new file mode 100644 index 0000000000..9e6874d9fe --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/ensure-local-path.function.test.ts @@ -0,0 +1,26 @@ +import { expect } from '@open-wc/testing'; +import { ensureLocalPath } from './ensure-local-path.function.js'; + +describe('ensureLocalPath', () => { + it('should return the same URL if it is a local URL', () => { + const localUrl = new URL('/test', window.location.origin); + expect(ensureLocalPath(localUrl).href).to.eq(localUrl.href); + }); + + it('should return the fallback URL if the input URL is not a local URL', () => { + const nonLocalUrl = new URL('https://example.com/test'); + const fallbackUrl = new URL('http://localhost/fallback'); + expect(ensureLocalPath(nonLocalUrl, fallbackUrl).href).to.eq(fallbackUrl.href); + }); + + it('should return the same URL if it is a local path', () => { + const localPath = '/test'; + expect(ensureLocalPath(localPath).pathname).to.eq(localPath); + }); + + it('should return the fallback URL if the input path is not a local path', () => { + const nonLocalPath = 'https://example.com/test'; + const fallbackUrl = new URL('http://localhost/fallback'); + expect(ensureLocalPath(nonLocalPath, fallbackUrl).href).to.eq(fallbackUrl.href); + }); +}); diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/ensure-local-path.function.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/ensure-local-path.function.ts new file mode 100644 index 0000000000..6862d689cb --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/ensure-local-path.function.ts @@ -0,0 +1,10 @@ +/** + * Ensure that the path is a local path. + */ +export function ensureLocalPath(path: URL | string, fallbackPath?: URL | string): URL { + const url = new URL(path, window.location.origin); + if (url.origin === window.location.origin) { + return url; + } + return fallbackPath ? new URL(fallbackPath) : new URL(window.location.origin); +} diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/stored-path.function.test.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/stored-path.function.test.ts new file mode 100644 index 0000000000..33f26b6efd --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/stored-path.function.test.ts @@ -0,0 +1,56 @@ +import { expect } from '@open-wc/testing'; +import { retrieveStoredPath, setStoredPath } from './stored-path.function.js'; +import { UMB_STORAGE_REDIRECT_URL } from '@umbraco-cms/backoffice/auth'; + +describe('retrieveStoredPath', () => { + beforeEach(() => { + sessionStorage.clear(); + }); + + it('should return a null if no path is stored', () => { + expect(retrieveStoredPath()).to.be.null; + }); + + it('should return the stored path if a path is stored', () => { + const testSafeUrl = new URL('/test', window.location.origin); + setStoredPath(testSafeUrl.toString()); + expect(retrieveStoredPath()?.toString()).to.eq(testSafeUrl.toString()); + }); + + it('should remove the stored path after it is retrieved', () => { + setStoredPath('/test'); + retrieveStoredPath(); + expect(sessionStorage.getItem(UMB_STORAGE_REDIRECT_URL)).to.be.null; + }); + + it('should return null if the stored path ends with "logout"', () => { + setStoredPath('/logout'); + expect(retrieveStoredPath()).to.be.null; + }); + + it('should not be possible to trick it with a fake URL', () => { + setStoredPath('//www.google.com'); + expect(retrieveStoredPath()).to.be.null; + + // also test setting it directly in sessionStorage (this will return the current path instead of the fake path) + sessionStorage.setItem(UMB_STORAGE_REDIRECT_URL, '//www.google.com'); + expect(retrieveStoredPath()?.pathname).to.eq(window.location.pathname); + }); +}); + +describe('setStoredPath', () => { + beforeEach(() => { + sessionStorage.clear(); + }); + + it('should store a local path', () => { + const testSafeUrl = new URL('/test', window.location.origin); + setStoredPath(testSafeUrl.toString()); + expect(sessionStorage.getItem(UMB_STORAGE_REDIRECT_URL)).to.eq(testSafeUrl.toString()); + }); + + it('should not store a non-local path', () => { + setStoredPath('https://example.com/test'); + expect(sessionStorage.getItem(UMB_STORAGE_REDIRECT_URL)).to.be.null; + }); +}); diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/stored-path.function.ts b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/stored-path.function.ts new file mode 100644 index 0000000000..12c35df508 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/packages/core/utils/path/stored-path.function.ts @@ -0,0 +1,30 @@ +import { ensureLocalPath } from './ensure-local-path.function.js'; +import { UMB_STORAGE_REDIRECT_URL } from '@umbraco-cms/backoffice/auth'; + +/** + * Retrieve the stored path from the session storage. + * @remark This is used to redirect the user to the correct page after login. + */ +export function retrieveStoredPath(): URL | null { + let currentRoute = ''; + const savedRoute = sessionStorage.getItem(UMB_STORAGE_REDIRECT_URL); + if (savedRoute) { + sessionStorage.removeItem(UMB_STORAGE_REDIRECT_URL); + currentRoute = savedRoute.endsWith('logout') ? currentRoute : savedRoute; + } + + return currentRoute ? ensureLocalPath(currentRoute) : null; +} + +/** + * Store the path in the session storage. + * @remark This is used to redirect the user to the correct page after login. + * @remark The path must be a local path, otherwise it is not stored. + */ +export function setStoredPath(path: string): void { + const url = new URL(path, window.location.origin); + if (url.origin !== window.location.origin) { + return; + } + sessionStorage.setItem(UMB_STORAGE_REDIRECT_URL, url.toString()); +}