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 3b5c9b14495d..711233754bb1 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 @@ -260,7 +260,7 @@ export class UmbAppElement extends UmbLitElement { // Register public extensions (login extensions) await new UmbServerExtensionRegistrator(this, umbExtensionsRegistry).registerPublicExtensions(); - new UmbAppEntryPointExtensionInitializer(this, umbExtensionsRegistry); + const entryPointInitializer = new UmbAppEntryPointExtensionInitializer(this, umbExtensionsRegistry); // Try to initialise the auth flow and get the runtime status try { @@ -276,6 +276,12 @@ export class UmbAppElement extends UmbLitElement { await this.#setAuthStatus(); } + // The login screen decides which auth provider to use from the registered + // `authProvider` extensions. App-entry-points may register or unregister those during + // their async onInit, so wait for them to settle before routing — otherwise on a slow + // connection the decision races and falls back to the local login. + await this.observe(entryPointInitializer.loaded).asPromise(); + // Initialise the router this.#redirect(); } catch (error) { diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/initializers/extension-initializer-base.test.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/initializers/extension-initializer-base.test.ts new file mode 100644 index 000000000000..c6460d722579 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/initializers/extension-initializer-base.test.ts @@ -0,0 +1,134 @@ +import type { ManifestBase } from '../types/index.js'; +import { UmbExtensionRegistry } from '../registry/extension.registry.js'; +import { loadManifestPlainJs } from '../functions/load-manifest-plain-js.function.js'; +import { UmbExtensionInitializerBase } from './extension-initializer-base.js'; +import { UmbObserver } from '../../observable-api/observer.js'; +import { expect, fixture } from '@open-wc/testing'; +import { UmbControllerHostElementMixin } from '@umbraco-cms/backoffice/controller-api'; +import type { UmbControllerHostElement } from '@umbraco-cms/backoffice/controller-api'; +import { customElement, html } from '@umbraco-cms/backoffice/external/lit'; + +@customElement('umb-test-initializer-base-host') +// eslint-disable-next-line @typescript-eslint/no-unused-vars +class UmbTestInitializerBaseHostElement extends UmbControllerHostElementMixin(HTMLElement) {} + +async function wait(ms: number) { + await new Promise((r) => setTimeout(r, ms)); +} + +// Factory for a concrete initializer over the 'test' manifest type. The base constructor's +// `observe` callback fires synchronously during `super()` — before any subclass field would +// initialise — so the record of instantiated aliases is a closed-over array created up front +// rather than instance state. +function createTestInitializer(host: UmbControllerHostElement, registry: UmbExtensionRegistry) { + const instantiated: string[] = []; + class UmbTestInitializer extends UmbExtensionInitializerBase<'test'> { + constructor() { + super(host, registry as never, 'test'); + } + async instantiateExtension(manifest: ManifestBase & { js?: unknown }): Promise { + if (manifest.js) { + await loadManifestPlainJs(manifest.js as never); + } + instantiated.push(manifest.alias); + } + unloadExtension(manifest: ManifestBase): void { + const index = instantiated.indexOf(manifest.alias); + if (index !== -1) instantiated.splice(index, 1); + } + } + return { initializer: new UmbTestInitializer(), instantiated }; +} + +describe('UmbExtensionInitializerBase — loaded signal', () => { + let hostElement: UmbControllerHostElement; + + beforeEach(async () => { + hostElement = await fixture(html``); + }); + + // Regression for the v17.4+ external-login race (introduced in #22522). + // + // A default Umbraco install registers ZERO app-entry-point extensions. The boot sequence + // awaits the app-entry-point initializer's `loaded` before deciding which login provider + // to use. If `loaded` never resolves when there are no matching extensions, that await + // hangs forever — which is precisely why the await was removed, leaving externally + // registered auth providers un-awaited and the login flow racing on slow connections. + // + // So: an initializer for a type with zero matching extensions MUST still resolve `loaded`. + it('resolves `loaded` even when no extensions of the type are registered', async () => { + const extensionRegistry = new UmbExtensionRegistry(); + const { initializer } = createTestInitializer(hostElement, extensionRegistry); + + const outcome = await Promise.race([ + new UmbObserver(initializer.loaded).asPromise().then(() => 'resolved'), + wait(1000).then(() => 'timeout'), + ]); + + expect(outcome, '`loaded` must resolve for an initializer with zero matching extensions').to.equal('resolved'); + }); + + // Regression for the late-loading race that the external-login bug is built on. + // + // This simulates an extension that registers AFTER the initial load and whose + // instantiation is slow (the app-entry-point case: its onInit registers an auth provider + // after an async module load). A consumer that awaits `loaded` must not be told "loaded" + // until that late, slow extension has actually finished instantiating — otherwise it makes + // its decision (e.g. which login provider to redirect to) against a stale registry. + it('does not report `loaded` until a late-registered, slow extension has finished instantiating', async () => { + const extensionRegistry = new UmbExtensionRegistry(); + + // Initial, fast extension — load settles to `true`. + extensionRegistry.register({ type: 'test', name: 'a', alias: 'Umb.Test.A' } as never); + const { initializer, instantiated } = createTestInitializer(hostElement, extensionRegistry); + await new UmbObserver(initializer.loaded).asPromise(); + expect(instantiated, 'initial extension instantiated').to.eql(['Umb.Test.A']); + + // A late, slow extension registers (mirrors an app-entry-point's onInit registering an + // auth provider after an async delay). + extensionRegistry.register({ + type: 'test', + name: 'b-late', + alias: 'Umb.Test.B.Late', + js: () => new Promise((r) => setTimeout(() => r({}), 100)), + } as never); + + // Awaiting `loaded` now must wait for the late extension to finish instantiating. + const lateExtInstantiatedWhenLoaded = await new UmbObserver(initializer.loaded) + .asPromise() + .then(() => instantiated.includes('Umb.Test.B.Late')); + + expect( + lateExtInstantiatedWhenLoaded, + '`loaded` resolved before the late, slow extension finished instantiating', + ).to.be.true; + }); + + // Permission-timing guard (re: the #22522 "user permissions resolved too late" concern). + // + // The backoffice route is gated by `#loadedGuard`, which awaits `bundleInitializer.loaded` + // via `.asPromise()`; the private extensions and user-permission data that load behind that + // gate must not be raced. So the gate must NOT open until the extensions registered before it + // was awaited have actually finished instantiating. This guards against a naive "resolve + // unconditionally" that sets `loaded` before instantiation completes. + // + // Note: user-permission *condition* resolution itself lives in UmbBaseExtensionInitializer + // (see base-extension-initializer.race.test.ts) — a different class this change does not touch. + it('does not open the `loaded` gate until the initially-registered extensions have instantiated', async () => { + const extensionRegistry = new UmbExtensionRegistry(); + extensionRegistry.register({ + type: 'test', + name: 'slow-boot', + alias: 'Umb.Test.SlowBoot', + js: () => new Promise((r) => setTimeout(() => r({}), 100)), + } as never); + + const {initializer, instantiated} = createTestInitializer(hostElement, extensionRegistry); + + const instantiatedWhenGateOpened = await new UmbObserver(initializer.loaded) + .asPromise() + .then(() => instantiated.includes('Umb.Test.SlowBoot')); + + expect(instantiatedWhenGateOpened, '`loaded` opened the gate before the extension instantiated').to.be.true; + }); +}); diff --git a/src/Umbraco.Web.UI.Client/src/libs/extension-api/initializers/extension-initializer-base.ts b/src/Umbraco.Web.UI.Client/src/libs/extension-api/initializers/extension-initializer-base.ts index 131e9ebd16fe..90a248c23033 100644 --- a/src/Umbraco.Web.UI.Client/src/libs/extension-api/initializers/extension-initializer-base.ts +++ b/src/Umbraco.Web.UI.Client/src/libs/extension-api/initializers/extension-initializer-base.ts @@ -20,11 +20,23 @@ export abstract class UmbExtensionInitializerBase< #loaded = new UmbBooleanState(undefined); loaded = this.#loaded.asObservable(); + // Identifies the current processing pass. The observer callback is async, so passes can + // overlap; only the latest pass is allowed to settle `loaded`, so a slower earlier pass + // cannot unblock waiters before the newest set of extensions has finished instantiating. + #loadPass = 0; + constructor(host: UmbElement, extensionRegistry: UmbExtensionRegistry, manifestType: Key) { super(host); this.host = host; this.extensionRegistry = extensionRegistry; this.observe(extensionRegistry.byType(manifestType), async (extensions) => { + const pass = ++this.#loadPass; + + // Re-arm while this pass is in flight so a consumer awaiting `loaded` waits for it to + // finish instead of resolving on a stale `true` from a previous pass. `undefined` + // rather than `false` because `asPromise()` resolves on the first non-undefined value. + this.#loaded.setValue(undefined); + this.#extensionMap.forEach((existingExt) => { if (!extensions.find((b) => b.alias === existingExt.alias)) { this.unloadExtension(existingExt); @@ -32,7 +44,10 @@ export abstract class UmbExtensionInitializerBase< } }); - await Promise.all( + // `allSettled` so a throwing/rejecting `instantiateExtension` cannot leave `loaded` + // stuck at `undefined` and hang a waiter (e.g. the app boot gate). Failures are + // surfaced rather than swallowed. + const results = await Promise.allSettled( extensions.map((extension) => { if (this.#extensionMap.has(extension.alias)) return; this.#extensionMap.set(extension.alias, extension); @@ -40,7 +55,16 @@ export abstract class UmbExtensionInitializerBase< }), ); - if (extensions.length > 0) { + for (const result of results) { + if (result.status === 'rejected') { + console.error('[UmbExtensionInitializer] Failed to instantiate extension', result.reason); + } + } + + // Only the latest pass settles `loaded`. Resolving unconditionally — including for + // zero extensions — so a consumer awaiting `loaded` (the app-entry-point boot gate, + // the bundle guard) never hangs on a default install that registers none of this type. + if (pass === this.#loadPass) { this.#loaded.setValue(true); } }); diff --git a/tests/Umbraco.Tests.AcceptanceTest/playwright.config.ts b/tests/Umbraco.Tests.AcceptanceTest/playwright.config.ts index 5671805a9f13..d9cb55d5a1a3 100644 --- a/tests/Umbraco.Tests.AcceptanceTest/playwright.config.ts +++ b/tests/Umbraco.Tests.AcceptanceTest/playwright.config.ts @@ -95,6 +95,15 @@ export default defineConfig({ ignoreHTTPSErrors: true, } }, + // Unauthenticated: this exercises the login screen itself (a late-registered auth provider). + { + name: 'authProviderLateRegistration', + testMatch: 'AuthProviderLateRegistration/**/*.spec.ts', + use: { + ...devices['Desktop Chrome'], + ignoreHTTPSErrors: true, + } + }, // This project is used to test the install steps, for that we do not need to authenticate. { name: 'unattendedInstallConfig', diff --git a/tests/Umbraco.Tests.AcceptanceTest/tests/AuthProviderLateRegistration/AdditionalSetup/App_Plugins/LateAuthProvider/entry-point.js b/tests/Umbraco.Tests.AcceptanceTest/tests/AuthProviderLateRegistration/AdditionalSetup/App_Plugins/LateAuthProvider/entry-point.js new file mode 100644 index 000000000000..13b1e9830268 --- /dev/null +++ b/tests/Umbraco.Tests.AcceptanceTest/tests/AuthProviderLateRegistration/AdditionalSetup/App_Plugins/LateAuthProvider/entry-point.js @@ -0,0 +1,30 @@ +// Test fixture: an appEntryPoint that registers an external auth provider LATE — i.e. after +// an async delay inside onInit — mirroring a real provider (e.g. Umbraco ID) whose onInit +// registers its authProvider after fetching/initialising on a slow connection. +// +// The backoffice boot must wait for app-entry-points to settle before deciding which login +// provider to use. If it doesn't, the login screen renders before this provider is registered +// and the late provider never appears (the v17.4+ regression). The delay makes that race +// deterministic. + +const LATE_REGISTRATION_DELAY_MS = 1500; + +export const onInit = async (_host, extensionRegistry) => { + await new Promise((resolve) => setTimeout(resolve, LATE_REGISTRATION_DELAY_MS)); + + extensionRegistry.register({ + type: 'authProvider', + alias: 'Test.LateAuthProvider', + name: 'Late External Login', + forProviderName: 'Umbraco.LateTest', + meta: { + label: 'Late External Login', + defaultView: { + icon: 'icon-cloud', + }, + behavior: { + autoRedirect: false, + }, + }, + }); +}; diff --git a/tests/Umbraco.Tests.AcceptanceTest/tests/AuthProviderLateRegistration/AdditionalSetup/App_Plugins/LateAuthProvider/umbraco-package.json b/tests/Umbraco.Tests.AcceptanceTest/tests/AuthProviderLateRegistration/AdditionalSetup/App_Plugins/LateAuthProvider/umbraco-package.json new file mode 100644 index 000000000000..4d220728b2bf --- /dev/null +++ b/tests/Umbraco.Tests.AcceptanceTest/tests/AuthProviderLateRegistration/AdditionalSetup/App_Plugins/LateAuthProvider/umbraco-package.json @@ -0,0 +1,12 @@ +{ + "name": "Late Auth Provider (test)", + "allowPublicAccess": true, + "extensions": [ + { + "type": "appEntryPoint", + "alias": "Test.LateAuthProvider.EntryPoint", + "name": "Late Auth Provider Entry Point", + "js": "/App_Plugins/LateAuthProvider/entry-point.js" + } + ] +} diff --git a/tests/Umbraco.Tests.AcceptanceTest/tests/AuthProviderLateRegistration/LateAuthProvider.spec.ts b/tests/Umbraco.Tests.AcceptanceTest/tests/AuthProviderLateRegistration/LateAuthProvider.spec.ts new file mode 100644 index 000000000000..6959645b9c06 --- /dev/null +++ b/tests/Umbraco.Tests.AcceptanceTest/tests/AuthProviderLateRegistration/LateAuthProvider.spec.ts @@ -0,0 +1,30 @@ +import {test} from '@umbraco/acceptance-test-helpers'; +import {expect} from '@playwright/test'; + +// Regression guard for the v17.4+ external-login race (introduced in #22522). +// +// AdditionalSetup/App_Plugins/LateAuthProvider deploys an appEntryPoint whose onInit, after a +// 1.5s delay, registers an external auth provider ("Late External Login"). This mirrors a real +// provider (e.g. Umbraco ID) that registers its authProvider during an async onInit on a slow +// connection. +// +// The backoffice boot must wait for app-entry-points to settle before rendering the login +// screen. If it does not, the login decision is made before the provider is registered, the +// late provider never appears, and the user is dropped on the local login instead. So: the +// late-registered provider MUST be offered on the login screen. +// +// This test is intentionally brittle (it depends on boot timing) but must remain working — it +// is the only end-to-end guard for the boot-gate behaviour. +test('a late-registered external auth provider is offered on the login screen', async ({umbracoUi}) => { + test.slow(); + + // Act - navigate to the backoffice unauthenticated (the login screen). + await umbracoUi.goToBackOffice(); + + // Assert - the provider registered late by the appEntryPoint is still offered. On the + // buggy boot the login screen renders before this provider exists, so it never appears. + const lateProviderButton = umbracoUi.page + .locator('umb-auth-provider-default') + .getByText('Sign in with Late External Login'); + await expect(lateProviderButton).toBeVisible({timeout: 15000}); +});