From b23906a6b6bd6c7c321da744ce9c7d995605e9ff Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Tue, 26 Aug 2025 16:48:46 +0200 Subject: [PATCH] V16: Unwarranted redirect after auth (#19935) * fix: uses isAuthorized to check if user is logged in before terminating the observer * feat: adds new function to redirect to stored path * fix: always redirect to stored path even on failure the user may have landed up on the page by mistake * Revert "fix: always redirect to stored path even on failure" This reverts commit 0c0cc0253c175ae8910e0cd26fc6df238374e6b2. * fix: sends back the result * fix: waits for the initial authorization request to come back before listening to the authorization signal (and then only listen once for it) also check if the request was null, which means we can safely redirect the user * docs: clarify what happens * chore: converts the promise code to async/await pattern * fix: tokenResponse should validate its internal object state * feat: allows function to force a window redirect * fix: checks if the user happens to already be authorized, because then we do not need a new code check * Update src/Umbraco.Web.UI.Client/src/packages/core/utils/path/stored-path.function.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../src/apps/app/app.element.ts | 49 ++++++++++++------- .../src/authorization_request_handler.ts | 3 +- .../src/packages/core/auth/auth-flow.ts | 2 +- .../core/utils/path/stored-path.function.ts | 17 +++++++ 4 files changed, 50 insertions(+), 21 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 ff655336d5..7a17520579 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 @@ -19,7 +19,7 @@ import { umbExtensionsRegistry, } from '@umbraco-cms/backoffice/extension-registry'; import { filter, first, firstValueFrom } from '@umbraco-cms/backoffice/external/rxjs'; -import { hasOwnOpener, retrieveStoredPath } from '@umbraco-cms/backoffice/utils'; +import { hasOwnOpener, redirectToStoredPath } from '@umbraco-cms/backoffice/utils'; import { UmbApiInterceptorController } from '@umbraco-cms/backoffice/resources'; import { umbHttpClient } from '@umbraco-cms/backoffice/http-client'; @@ -61,7 +61,7 @@ export class UmbAppElement extends UmbLitElement { { path: 'oauth_complete', component: () => import('./app-oauth.element.js'), - setup: (component) => { + setup: async (component) => { if (!this.#authContext) { (component as UmbAppOauthElement).failure = true; console.error('[Fatal] Auth context is not available'); @@ -76,26 +76,37 @@ export class UmbAppElement extends UmbLitElement { return; } - // If we are in the main window (i.e. no opener), we should redirect to the root after the authorization request is completed. - // 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 (!hasOwnOpener(this.backofficePath)) { - this.observe(this.#authContext.authorizationSignal, () => { - // Redirect to the saved state or root - const url = retrieveStoredPath(); - const isBackofficePath = url?.pathname.startsWith(this.backofficePath) ?? true; - - if (isBackofficePath) { - history.replaceState(null, '', url?.toString() ?? ''); - } else { - window.location.href = url?.toString() ?? this.backofficePath; - } - }); + // Check that we are not already authorized + if (this.#authContext.getIsAuthorized()) { + redirectToStoredPath(this.backofficePath, true); + return; } // Complete the authorization request, which will send the authorization signal - this.#authContext.completeAuthorizationRequest().catch(() => undefined); + try { + const result = await this.#authContext.completeAuthorizationRequest(); + + if (result === null) { + // If the result is null, it could mean that no new token was required, so we can redirect the user + // This could happen if the user is already authorized or accidentally enters the oauth_complete url + redirectToStoredPath(this.backofficePath, true); + return; + } + + // If we are in the main window (i.e. no opener), we should redirect to the root after the authorization request is completed. + // 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 their previous path (or root). + if (hasOwnOpener(this.backofficePath)) return; + + // Listen for the first authorization signal after the initial authorization request + await firstValueFrom(this.#authContext.authorizationSignal); + // When it hits, we should redirect the user to the backoffice + redirectToStoredPath(this.backofficePath); + } catch { + (component as UmbAppOauthElement).failure = true; + console.error('[Fatal] Authorization request failed'); + } }, }, { diff --git a/src/Umbraco.Web.UI.Client/src/external/openid/src/authorization_request_handler.ts b/src/Umbraco.Web.UI.Client/src/external/openid/src/authorization_request_handler.ts index 90eb72b171..d77d8a0e59 100644 --- a/src/Umbraco.Web.UI.Client/src/external/openid/src/authorization_request_handler.ts +++ b/src/Umbraco.Web.UI.Client/src/external/openid/src/authorization_request_handler.ts @@ -118,7 +118,7 @@ export abstract class AuthorizationRequestHandler { /** * Completes the authorization request if necessary & when possible. */ - completeAuthorizationRequestIfPossible(): Promise { + completeAuthorizationRequestIfPossible(): Promise { // call complete authorization if possible to see there might // be a response that needs to be delivered. log(`Checking to see if there is an authorization response to be delivered.`); @@ -133,6 +133,7 @@ export abstract class AuthorizationRequestHandler { if (result && this.notifier) { this.notifier.onAuthorizationComplete(result.request, result.response, result.error); } + return result; }); } diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/auth/auth-flow.ts b/src/Umbraco.Web.UI.Client/src/packages/core/auth/auth-flow.ts index 8c10231db6..0e02546423 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/auth/auth-flow.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/auth/auth-flow.ts @@ -236,7 +236,7 @@ export class UmbAuthFlow { * @returns true if the user is logged in, false otherwise. */ isAuthorized(): boolean { - return !!this.#tokenResponse; + return !!this.#tokenResponse.getValue(); } /** 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 index e9196ab324..24d280c861 100644 --- 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 @@ -30,3 +30,20 @@ export function setStoredPath(path: string): void { } sessionStorage.setItem(UMB_STORAGE_REDIRECT_URL, url.toString()); } + +/** + * Redirect the user to the stored path or the base path if not available. + * If the basePath matches the start of the stored path, the browser will replace the state instead of redirecting. + * @param {string} basePath - The base path to redirect to if no stored path is available. + * @param {boolean} [force=false] - If true, will redirect using Location + */ +export function redirectToStoredPath(basePath: string, force = false): void { + const url = retrieveStoredPath(); + const isBackofficePath = url?.pathname.startsWith(basePath) ?? false; + + if (isBackofficePath && !force) { + history.replaceState(null, '', url?.toString() ?? ''); + } else { + window.location.href = url?.toString() ?? basePath; + } +}