Skip to content

External login: wait for app-entry-points before the login provider decision#23167

Merged
leekelleher merged 3 commits into
release/17.5.0from
v17/bugfix/external-login-entrypoint-race
Jun 19, 2026
Merged

External login: wait for app-entry-points before the login provider decision#23167
leekelleher merged 3 commits into
release/17.5.0from
v17/bugfix/external-login-entrypoint-race

Conversation

@iOvergaard

Copy link
Copy Markdown
Contributor

What

The backoffice boot stopped waiting for appEntryPoint extensions to settle before deciding which auth provider to use — a regression introduced in #22522 (the block-permissions PR, which also reworked the extension-initializer lifecycle). On a slow connection an externally registered authProvider (e.g. Umbraco ID) registered inside an app-entry-point's async onInit isn't present yet when the login screen renders, so the login decision is made against a stale registry and the user is dropped on the local login instead of being redirected to the external provider.

Reported from Cloud: intermittent on 3G/incognito, reproducible from v17.4.0 onward (and present on v18 RCs).

Root cause

#22522 changed UmbExtensionInitializerBase.#loaded from a one-shot ReplaySubject to UmbBooleanState(undefined) and removed the await that gated the boot on the app-entry-point initializer. Two problems followed:

  1. loaded only became true if (extensions.length > 0), so for a type with zero registered extensions (a default install has no app-entry-points) it never resolved — which is why the boot gate could not simply be re-added (it would hang).
  2. With the gate gone, nothing waited for app-entry-points to finish their async onInit before the login provider decision.

Fix

  • extension-initializer-base.tsloaded re-arms to undefined while a processing pass is in flight and resolves to true unconditionally (including zero extensions). .asPromise() skips undefined, so this gates correctly and never hangs.
  • app.element.ts — restore the awaited boot gate before routing: await this.observe(entryPointInitializer.loaded).asPromise().

Tests

  • Unit (extension-initializer-base.test.ts): loaded must resolve with zero extensions (the hang), and must not report loaded until a late-registered, slow extension has finished instantiating (the race). Both fail on the unfixed code, pass with the fix.
  • Acceptance (AuthProviderLateRegistration/): deploys an appEntryPoint that registers an authProvider 1.5s after boot and asserts it is offered on the login screen. Verified RED on the unfixed client build and GREEN on the fixed build against a live instance.

Non-regression

The tests #22522 introduced/reworked all pass with this change: block language-access controller, read-only + read-only-variant guards, and the extension-initializer condition-race tests (64 tests), plus the full extension-api suite (174).

🤖 Generated with Claude Code

…ecision

The backoffice boot stopped waiting for app-entry-point extensions to settle
before deciding which auth provider to use (regression introduced in #22522).
On a slow connection an externally registered authProvider (e.g. Umbraco ID)
is not registered yet when the login screen renders, so the user is dropped on
the local login instead of being redirected to the external provider.

- extension-initializer-base: `loaded` re-arms to `undefined` while a pass is in
  flight and resolves to `true` unconditionally (including zero extensions), so
  `.asPromise()` gates correctly and never hangs on a default install (which has
  no app-entry-points) — the reason the await was removed in the first place.
- app.element: restore the awaited boot gate before routing.

Tests:
- Unit test for the `loaded` signal contract (zero extensions resolves; a late,
  slow extension is awaited).
- Playwright acceptance test that deploys an app-entry-point registering an
  authProvider after a delay and asserts it is offered on the login screen.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 19, 2026 09:10
@claude

claude Bot commented Jun 19, 2026

Copy link
Copy Markdown

Claude finished @iOvergaard's task in 8m 54s —— View job


PR Review Complete

  • Load review skill and standards
  • Gather context (diff, changed files)
  • Read area-specific documentation
  • Review changed files
  • Post findings
  • Apply labels (area/frontend, category/ux, category/ui)

Review posted as a new comment. Verdict: Approved — one narrow theoretical suggestion about concurrent async callback ordering (not a blocker for the specific fix).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a regression in the backoffice boot sequence where the login provider decision could occur before appEntryPoint extensions (and their async onInit) finished, causing external auth providers to be missed on slow connections.

Changes:

  • Adjust UmbExtensionInitializerBase.loaded to re-arm while processing and to resolve even when zero extensions exist.
  • Restore an awaited boot gate in UmbAppElement to wait for app-entry-points to settle before routing.
  • Add unit + acceptance coverage for late auth provider registration and wire it into Playwright projects.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/Umbraco.Tests.AcceptanceTest/tests/AuthProviderLateRegistration/LateAuthProvider.spec.ts Adds E2E regression test ensuring a late-registered auth provider appears on the login screen.
tests/Umbraco.Tests.AcceptanceTest/tests/AuthProviderLateRegistration/AdditionalSetup/App_Plugins/LateAuthProvider/umbraco-package.json Adds a public package fixture that contributes an appEntryPoint for the test.
tests/Umbraco.Tests.AcceptanceTest/tests/AuthProviderLateRegistration/AdditionalSetup/App_Plugins/LateAuthProvider/entry-point.js Implements delayed authProvider registration inside onInit to reproduce the race.
tests/Umbraco.Tests.AcceptanceTest/playwright.config.ts Adds a new Playwright project for the unauthenticated late-registration login test.
src/Umbraco.Web.UI.Client/src/libs/extension-api/initializers/extension-initializer-base.ts Updates loaded semantics to avoid hangs (zero extensions) and to re-arm while a pass is running.
src/Umbraco.Web.UI.Client/src/libs/extension-api/initializers/extension-initializer-base.test.ts Adds unit tests for loaded resolving with zero extensions and waiting for late/slow instantiation.
src/Umbraco.Web.UI.Client/src/apps/app/app.element.ts Waits for app-entry-points to finish initializing before routing/login provider decision.

Comment thread tests/Umbraco.Tests.AcceptanceTest/playwright.config.ts
@claude

claude Bot commented Jun 19, 2026

Copy link
Copy Markdown

PR Review

Target: origin/release/17.5.0 · Based on commit: 102e4aa80b11f9bc96ea47ead373cca5ea0402c6

Restores the app-entry-point boot gate removed in #22522, fixing the race where externally registered authProvider extensions were absent when the login screen decided which provider to use. The two-part fix — unconditional loaded resolution including zero-extension case, plus re-arming loaded to undefined during each processing pass — correctly handles both the zero-extension hang and the stale-true resolution.

  • Modified public API: UmbExtensionInitializerBase.loaded behavioral change — observable now cycles undefined → true on each extension processing pass (previously only emitted once and only when extensions.length > 0). Type is unchanged: Observable<boolean | undefined>.
  • Affected implementations (outside this PR): Two existing asPromise() consumers of .loaded — both in app.element.ts — handle the re-arm semantics correctly and are unaffected.

Suggestions

  • extension-initializer-base.ts:27–51: The re-arm prevents a stale true from an earlier pass tricking consumers — but only if the newer pass completes before the older one does. If two passes overlap (the registry emits a new extension list while a previous Promise.all is still in flight), the older pass completing fires setValue(true) while the newer pass is still processing, potentially resolving a consumer's asPromise() prematurely. This cannot happen in the specific boot scenario being fixed (one stable batch from registerPublicExtensions()), so it is a narrow theoretical edge case — not a blocker, just worth noting if appEntryPoint extensions are ever registered dynamically during boot.

Approved

This looks good to be merged as-is. Root cause analysis is thorough, both unit regression tests cover the exact failure modes (zero-extension hang and late-extension race), and the acceptance test provides an end-to-end guard documented as verified red-on-unfixed / green-on-fixed. Recommend a manual sanity check on the login flow before merging.

@claude claude Bot added area/frontend category/ux User experience category/ui User interface labels Jun 19, 2026
iOvergaard and others added 2 commits June 19, 2026 11:19
Add a test asserting the collection initializer's `loaded` does not open the
gate (`#loadedGuard` awaits it via `.asPromise()`, fronting private-extension
and user-permission loading) until the initially-registered extensions have
instantiated. Addresses the #22522 "user permissions resolved too late" concern
in writing; user-permission condition resolution itself lives in
UmbBaseExtensionInitializer (covered by base-extension-initializer.race.test.ts)
and is untouched by this change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…review)

Address PR review feedback:
- extension-initializer-base: only the latest processing pass settles `loaded`
  (monotonic pass id), so a slow earlier pass can't unblock waiters early when
  the async observer overlaps passes; and use `Promise.allSettled` so a throwing
  `instantiateExtension` can't leave `loaded` stuck at `undefined` (hanging the
  boot gate) — failures are logged rather than swallowed.
- playwright.config: narrow the project glob to `**/*.spec.ts` so Playwright
  doesn't try to load the App_Plugins `entry-point.js` ESM fixture as a test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@iOvergaard

Copy link
Copy Markdown
Contributor Author

Addressed the review suggestions in c566dd0:

  1. Playwright glob — narrowed testMatch to AuthProviderLateRegistration/**/*.spec.ts so the entry-point.js ESM fixture under AdditionalSetup isn't picked up as a test file. Matches the other AdditionalSetup-bearing projects.
  2. Concurrent passesloaded now only settles for the latest processing pass (monotonic pass id), so a slow earlier pass can't unblock waiters before the newest set of extensions has finished instantiating.
  3. Throwing instantiateExtension — switched to Promise.allSettled so a rejection can't leave loaded stuck at undefined (which would hang the boot gate); failures are logged rather than swallowed. Mirrors how #loadedGuard already handles its awaits.

Still green: 175 extension-api unit tests, and the AuthProviderLateRegistration acceptance spec passes (and confirmed RED on the unfixed client build).

@leekelleher leekelleher self-requested a review June 19, 2026 10:39

@leekelleher leekelleher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested out, works as described. 🚀

@leekelleher leekelleher merged commit 9f63341 into release/17.5.0 Jun 19, 2026
25 checks passed
@leekelleher leekelleher deleted the v17/bugfix/external-login-entrypoint-race branch June 19, 2026 11:11
iOvergaard added a commit that referenced this pull request Jun 19, 2026
…ecision (#23167)

* External login: wait for app-entry-points before the login provider decision

The backoffice boot stopped waiting for app-entry-point extensions to settle
before deciding which auth provider to use (regression introduced in #22522).
On a slow connection an externally registered authProvider (e.g. Umbraco ID)
is not registered yet when the login screen renders, so the user is dropped on
the local login instead of being redirected to the external provider.

- extension-initializer-base: `loaded` re-arms to `undefined` while a pass is in
  flight and resolves to `true` unconditionally (including zero extensions), so
  `.asPromise()` gates correctly and never hangs on a default install (which has
  no app-entry-points) — the reason the await was removed in the first place.
- app.element: restore the awaited boot gate before routing.

Tests:
- Unit test for the `loaded` signal contract (zero extensions resolves; a late,
  slow extension is awaited).
- Playwright acceptance test that deploys an app-entry-point registering an
  authProvider after a delay and asserts it is offered on the login screen.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(backoffice): guard the loaded-gate timing for permission loading

Add a test asserting the collection initializer's `loaded` does not open the
gate (`#loadedGuard` awaits it via `.asPromise()`, fronting private-extension
and user-permission loading) until the initially-registered extensions have
instantiated. Addresses the #22522 "user permissions resolved too late" concern
in writing; user-permission condition resolution itself lives in
UmbBaseExtensionInitializer (covered by base-extension-initializer.race.test.ts)
and is untouched by this change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(backoffice): harden loaded signal + narrow acceptance test glob (review)

Address PR review feedback:
- extension-initializer-base: only the latest processing pass settles `loaded`
  (monotonic pass id), so a slow earlier pass can't unblock waiters early when
  the async observer overlaps passes; and use `Promise.allSettled` so a throwing
  `instantiateExtension` can't leave `loaded` stuck at `undefined` (hanging the
  boot gate) — failures are logged rather than swallowed.
- playwright.config: narrow the project glob to `**/*.spec.ts` so Playwright
  doesn't try to load the App_Plugins `entry-point.js` ESM fixture as a test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
iOvergaard added a commit that referenced this pull request Jun 19, 2026
The `loaded`-signal test (added in #23167) built its host with
`UmbControllerHostElementMixin(HTMLElement)`, mirroring the older
`UmbBaseExtensionInitializer` tests. But `UmbExtensionInitializerBase`
requires a full `UmbElement` host, so the test failed `tsc` (TS2345)
under the root tsconfig. The product build excludes `*.test.ts`, so it
slipped through CI but breaks `npm run compile`/the editor.

Use `UmbElementMixin(HTMLElement)`, matching what production callers pass
(app/backoffice/preview elements are all UmbElements).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants