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 0c0cc02.

* 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 <[email protected]>

---------

Co-authored-by: Copilot <[email protected]>

Author: iOvergaard

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');
+				}
 			},
 		},
 		{

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<void> {
+	completeAuthorizationRequestIfPossible(): Promise<AuthorizationRequestResponse | null> {
 		// 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;
 		});
 	}
 

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();
 	}
 
 	/**

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;
+	}
+}