enhance: move local auth owner creation to dedicated route /admin/setup - #7943
Conversation
a96aa5d to
073828b
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Route-loop and onboarding regressions can block setup, while configuration failure can leave the owner form unusable.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a dedicated /admin/setup experience for bootstrap users configuring local authentication and the initial owner.
Changes:
- Adds the setup route and integration tests.
- Extracts reusable splash, owner prompt, and initial-user components.
- Updates redirects, responsive styling, and shared splash-state handling.
File summaries
| File | Description |
|---|---|
ui/user/src/routes/login/local/+page.svelte |
Improves mobile sizing. |
ui/user/src/routes/identity-access/AuthProvidersView.svelte |
Uses the shared owner prompt. |
ui/user/src/routes/admin/setup/page.svelte.spec.ts |
Tests the setup flow. |
ui/user/src/routes/admin/setup/+page.ts |
Loads and guards setup data. |
ui/user/src/routes/admin/setup/+page.svelte |
Implements dedicated setup UI. |
ui/user/src/routes/admin/+page.ts |
Redirects bootstrap users to setup. |
ui/user/src/routes/admin/+page.svelte |
Reuses splash-state constant. |
ui/user/src/lib/services/admin/operations.ts |
Adds request options to EULA loading. |
ui/user/src/lib/constants.ts |
Defines the splash storage key. |
ui/user/src/lib/components/navbar/Profile.svelte |
Reuses splash-state constant. |
ui/user/src/lib/components/admin/SetupSplashDialog.svelte |
Routes bootstrap onboarding to setup. |
ui/user/src/lib/components/admin/SetupSplashContent.svelte |
Extracts shared welcome content. |
ui/user/src/lib/components/admin/OwnerSetupPrompt.svelte |
Extracts owner sign-in prompt. |
ui/user/src/lib/components/admin/LocalAuthInitialUserForm.svelte |
Extracts initial-owner form. |
ui/user/src/lib/components/admin/LocalAuthConfigure.svelte |
Uses the extracted owner form. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical configuration and redirect-loop issues, plus setup-state recovery issues, block approval.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
ui/user/src/lib/components/admin/LocalAuthInitialUserForm.svelte:98
- When
ensureConfigured()returnsfalse, this early return bypasses the create-user catch and the onlysaving = falseassignment. A configuration error therefore leaves the form permanently disabled, so the bootstrap user cannot retry after fixing the provider configuration; resetsavingbefore returning (or wrap the whole submission in afinally).
if (!(await ensureConfigured())) {
return;
}
ui/user/src/routes/admin/setup/+page.svelte:86
startOwnerTempLogin()cancels the previous temporary login before requesting a new URL, but this state is not cleared until the request succeeds. After closing the Manage Local Users dialog—or if re-initiation fails—the page can still render the canceled/failed old link, so clicking Sign in uses an invalid token. ClearsetupTempLoginUrlbefore starting the cancellation/re-initiation flow.
async function prepareSignIn() {
if (!localProvider) return;
try {
const result = await startOwnerTempLogin(localProvider);
explicitOwners = result.explicitOwners;
setupTempLoginUrl = result.redirectUrl;
- Files reviewed: 15/15 changed files
- Comments generated: 3
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
b786d8f to
320ef57
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Critical setup redirect issues and moderate local-auth state/configuration issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (4)
Previously missed (1) — in code that hasn't changed since the last review.
ui/user/src/routes/admin/setup/+page.svelte:89
- After
startOwnerTempLoginfails,stepstays'signin'andsetupTempLoginUrlis never reset or retried. On the initial visit this leavesOwnerSetupPromptshowing its indefinite spinner; after managing users,cancelTempLogin()may already have invalidated the old URL, which remains displayed. The bootstrap user is stuck until a full reload. Clear the URL and render a retryable error/action (or return to the owner step) when preparation fails.
ui/user/src/lib/components/admin/LocalAuthInitialUserForm.svelte:40
- This component is also mounted when the route finds a configured local provider with zero users (for example, after an interrupted setup or deleting the last account), but this unconditionally submits
OBOT_AUTH_PROVIDER_EMAIL_DOMAINS=*. That overwrites an existing allowed-domain policy and broadens which local accounts can be created. Only auto-configure whenprovideris not already configured; otherwise preserve the existing configuration.
onMount(() => {
configurePromise = autoConfigure();
ui/user/src/routes/admin/setup/+page.svelte:192
- This modal is used after local auth is already configured, but its configuration values are hard-coded to
*. If the user opens the owner-account manager, goes back from Users to Configuration, and saves,LocalAuthConfiguresends this wildcard toconfigureAuthProviderand replaces the existing allowed-domain policy. Load/reveal the current provider values before opening this modal, or do not expose the configuration step, instead of supplying a wildcard.
values={{ OBOT_AUTH_PROVIDER_EMAIL_DOMAINS: '*' }}
ui/user/src/routes/admin/setup/+page.svelte:83
- After the sign-in prompt has a URL,
handleLocalAuthClosecallsprepareSignInagain when the owner account is changed. This function leaves the old one-time URL insetupTempLoginUrlwhile cancellation and initiation are in flight (and if initiation fails), so the prompt can show the new email while its link points to the canceled/old token. Clear the URL before starting a new preparation so the link is not clickable until it belongs to the current account.
if (!localProvider) return;
try {
- Files reviewed: 15/15 changed files
- Comments generated: 2
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
There was a problem hiding this comment.
🟡 Changes recommended
Five unresolved moderate review findings remain.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (4)
ui/user/src/lib/components/admin/LocalAuthInitialUserForm.svelte:41
- When
/admin/setupis revisited after local auth has already been configured but before its first user exists,provider.configuredis true, yet mounting this form still callsautoConfigure()withOBOT_AUTH_PROVIDER_EMAIL_DOMAINS: '*'. That silently replaces any existing domain allow-list and broadens who can create local accounts. Only auto-configure an unconfigured provider, or preserve the existing configuration here.
onMount(() => {
configurePromise = autoConfigure();
});
ui/user/src/routes/admin/+page.ts:16
- This now sends every bootstrap session to
/admin/setup, including a force-enabled bootstrap session after an owner already exists. In that stateBootstrap.Enabledis true butSetupEnabledis false (seepkg/bootstrap/bootstrap_test.go:124-129), so the setup endpoints used by the new page return 404 and the page is left on its sign-in spinner. Preserve the normal admin/auth-provider redirect or gate this redirect onsetupEnabled.
hasVMCPs?: boolean,
ui/user/src/routes/admin/setup/+page.svelte:145
- This route is only rendered after
+page.tsconfirms the current profile is the bootstrap user, butunauthorizedhides theProfilemenu. Unlike the previous Auth Providers flow, the bootstrap user therefore has no UI action to log out or abandon setup, and/adminjust redirects back here. Render the authenticated navbar (or add an explicit bootstrap logout action) so setup can be exited.
<Navbar class="dark:bg-gray-990 sticky top-0 left-0 z-30 w-full" unauthorized />
ui/user/src/routes/admin/setup/+page.svelte:192
- This management dialog is also opened for an already configured local provider, but it is initialized with a hard-coded
*. Saving the Configuration step without changing anything can therefore overwrite a previously restricted email-domain allow-list. Load/reveal the provider's current values before opening this dialog instead of supplying this default.
values={{ OBOT_AUTH_PROVIDER_EMAIL_DOMAINS: '*' }}
- Files reviewed: 18/18 changed files
- Comments generated: 1
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
2c233d8 to
8b5dab4
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved bootstrap routing, dashboard navigation, setup logout, and local-provider domain-preservation issues remain.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
ui/user/src/lib/components/admin/LocalAuthInitialUserForm.svelte:48
- This form is also selected when the local provider is already configured but has no users. In that case the unconditional auto-configuration posts
OBOT_AUTH_PROVIDER_EMAIL_DOMAINS=*, replacing any narrower domain restriction whenever setup is revisited. Skip configuration for an already configured provider (or preserve its existing settings) so entering owner setup cannot broaden who may authenticate.
async function autoConfigure(): Promise<boolean> {
configuring = true;
configError = undefined;
try {
const err = await onConfigure({ [DOMAINS_KEY]: '*' });
if (err) {
ui/user/src/routes/admin/setup/+page.svelte:145
- This route is rendered for an authenticated bootstrap user, but
unauthorizedsuppresses the Profile menu inNavbar, including the existing bootstrap logout action. The setup flow has no other logout control, so a user who needs to abandon or switch the setup session is trapped on this route; render the authorized navbar here.
<Navbar class="dark:bg-gray-990 sticky top-0 left-0 z-30 w-full" unauthorized />
ui/user/src/routes/admin/setup/+page.svelte:192
- The regular local-user management dialog is given a hard-coded
*value instead of the provider's current configuration. If an existing local provider has restricted email domains and the user returns to the config step, continuing from this dialog overwrites that restriction with*; load/reveal the current provider credentials before opening the dialog and pass those values here.
values={{ OBOT_AUTH_PROVIDER_EMAIL_DOMAINS: '*' }}
- Files reviewed: 19/19 changed files
- Comments generated: 2
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved issues affect Dashboard role gating, setup redirects, auth-domain preservation, setup completion handling, and sign-out access.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (9)
ui/user/src/lib/components/Layout.svelte:228
hasAdminAccess()is true for auditors as well as admins, so a basic user with the AUDITOR role still gets the Dashboard route. The updatedLayout.svelte.spec.tscase for[Group.USER, Group.AUDITOR]expects no/dashboardlink; use a dashboard-specific privilege check instead of the admin-access predicate.
...(isAtLeastPoweruser || profile.current.hasAdminAccess?.()
? [
ui/user/src/lib/components/admin/LocalAuthInitialUserForm.svelte:47
- This form is now rendered by
/admin/setupwhen the local provider is already configured but has no users, yet mounting it always savesOBOT_AUTH_PROVIDER_EMAIL_DOMAINS=*. That silently broadens an existing local-auth domain restriction before the owner enters an email and leaves the provider accepting any domain; only auto-configure an unconfigured provider.
const err = await onConfigure({ [DOMAINS_KEY]: '*' });
ui/user/src/lib/components/admin/LocalAuthInitialUserForm.svelte:115
- If user creation succeeds but the follow-up
listLocalAuthUsers()call fails, this setsinitialUserErrorand then still invokesonCreatedfromfinally. Both callers close or advance the flow on that callback, so the read failure is hidden and setup proceeds with unverified state; only complete after a successful refresh, or make the created response authoritative and handle the refresh failure without closing.
} catch (err) {
initialUserError = errorMessage(err, 'Failed to list local auth users.');
userCount = 1;
} finally {
onCreated?.(userCount, email);
ui/user/src/routes/+page.ts:27
- Because
hasAdminAccess()is also true for auditors, an auditor entering the root route is redirected to/dashboard, where the new role policy says they should be sent to/vmcps. Keep this redirect aligned with the dashboard guard and sidebar by checking the dashboard-specific role instead.
const isAtLeastPoweruser =
profile?.groups.includes(Group.POWERUSER) || profile?.hasAdminAccess?.();
throw redirect(302, isAtLeastPoweruser ? '/dashboard' : '/vmcps');
ui/user/src/routes/admin/+page.ts:54
bootstrapStatusis only populated in thegetProfileerror branch. After bootstrap login,getProfilesucceeds, so this redirect receivesundefinedforisSetupEnabledand sends the bootstrap user to/dashboardor/vmcpsinstead of the new/admin/setupflow. Fetch the bootstrap status for an authenticated bootstrap profile before choosing this redirect.
getAdminRedirectPath(profile, vmcps.length > 0, bootstrapStatus?.setupEnabled)
);
ui/user/src/routes/admin/+page.ts:29
hasAdminAccess()includes auditors, so an auditor entering/adminis redirected to/dashboardeven though the updated layout test explicitly hides Dashboard for auditors. This helper should use the dashboard-specific privilege check rather than admin-page access.
const isAtLeastPoweruser =
profile?.groups.includes(Group.POWERUSER) || profile?.hasAdminAccess?.();
return isAtLeastPoweruser ? '/dashboard' : '/vmcps';
ui/user/src/routes/admin/setup/+page.svelte:193
- The full local-auth management dialog is given a hard-coded wildcard value here.
LocalAuthConfigure.resetDialogState()usesvalues.OBOT_AUTH_PROVIDER_EMAIL_DOMAINSas the editable configuration, so opening Configuration and saving can replace an existing restricted domain list with*. Load the provider's current credentials before opening this dialog instead of supplying a wildcard for an already configured provider.
values={{ OBOT_AUTH_PROVIDER_EMAIL_DOMAINS: '*' }}
onConfigure={handleLocalAuthConfigure}
ui/user/src/routes/admin/setup/+page.svelte:145
- This route is only reachable for an authenticated bootstrap user, but
unauthorizedhides the Profile menu and there is no other sign-out/leave control on the page. That prevents the bootstrap user from using the existingbootstrapLogoutflow to leave setup and return later (the page even documents that revisit case). Render the profile control or add an explicit sign-out action.
<Navbar class="dark:bg-gray-990 sticky top-0 left-0 z-30 w-full" unauthorized />
ui/user/src/routes/dashboard/+page.ts:10
- This guard uses
hasAdminAccess(), which includes auditors, so an auditor can load the Dashboard directly instead of being redirected to/vmcps. That contradicts the new navigation expectation for[Group.USER, Group.AUDITOR]; use the same dashboard-specific privilege check as the sidebar.
const isAtLeastPoweruser =
profile?.groups.includes(Group.POWERUSER) || profile?.hasAdminAccess?.();
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate review findings remain.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (4)
ui/user/src/lib/components/admin/LocalAuthInitialUserForm.svelte:116
- This callback is invoked from
finallyeven whenlistLocalAuthUsers()fails. The setup-page callback immediately replaces this form with the sign-in step, so the list error is hidden and setup proceeds using the synthetic user instead of allowing the user to retry or see the failure. InvokeonCreatedonly after the user-list request succeeds (or explicitly handle the failed refresh).
} finally {
onCreated?.(userCount, email);
saving = false;
ui/user/src/routes/admin/+page.ts:54
bootstrapStatusis only populated in thegetProfileerror branch above, so a normal bootstrap session reaches this call withisSetupEnabledundefined. Because bootstrap profiles also have admin/owner access,getAdminRedirectPaththen skips/admin/setupand sends the user to/vmcps?new=trueor/dashboard, making the new setup route unreachable after bootstrap login. Fetch the bootstrap status for authenticated bootstrap profiles before choosing this redirect.
throw redirect(
307,
getAdminRedirectPath(profile, vmcps.length > 0, bootstrapStatus?.setupEnabled)
);
ui/user/src/routes/admin/setup/+page.svelte:70
- This catches every cancellation failure, but
initiateTempLoginimmediately rejects with 409 whenever a temporary user is still cached. If cancellation fails for any reason other than the known 404/no-cache case, the route can remain on the loading state with no usable sign-in link; only the 404 should be ignored and other errors should be propagated.
} catch {
// Best-effort. 404 means nothing to cancel; other failures should not block setup.
}
ui/user/src/routes/admin/setup/+page.svelte:128
- Clear
setupTempLoginUrlbefore this awaited user-list request. Once the dialog closes, the old link remains rendered during this await;prepareSignIn()will later cancel that token, so a click in the interim follows an invalidated setup link.
localUsers = await AdminService.listLocalAuthUsers();
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved bootstrap-fetch behavior and an unused import that causes lint failure require fixes.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
ui/user/src/lib/components/admin/LocalAuthConfigure.svelte:936
LocalAuthConfigurestill passesrequired={!!showInitialAuthProvider}for the legacy/bootstrap provider setup path, but this removes the required-title branch. That dialog will now showSet Up Localin its title bar while the extracted form saysSet Up Owner Account; retain the owner-specific title whenrequiredis true.
Set Up {provider?.name}
- Files reviewed: 20/20 changed files
- Comments generated: 2
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
2c39d45 to
40711e7
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Critical lint and multiple moderate routing, role-access, and onboarding issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (9)
ui/user/src/lib/components/Layout.svelte:227
Role.Groups()emitsGroupPowerUserPlusseparately fromGroupPowerUser(seeapiclient/types/user.go:137-141), so this new navigation condition hides Dashboard from Power User Plus-only profiles even though that role is above a standard user. IncludeGroup.POWERUSER_PLUSin the condition.
...(isAtLeastPoweruser || profile.current.hasAdminAccess?.()
ui/user/src/lib/components/navbar/Profile.svelte:324
Role.Groups()emitsGroupPowerUserPlusseparately fromGroupPowerUser(seeapiclient/types/user.go:137-141), so a Power User Plus-only profile gets an/vmcpslink here instead of the Dashboard. IncludeGroup.POWERUSER_PLUSin this privilege check.
profile.current.groups.includes(Group.POWERUSER) || profile.current.hasAdminAccess?.()
ui/user/src/routes/+page.ts:29
Role.Groups()emitsGroupPowerUserPlusseparately fromGroupPowerUser(seeapiclient/types/user.go:137-141), so a Power User Plus-only profile is treated as a standard user here and the post-login landing page becomes/vmcpsinstead of/dashboard. IncludeGroup.POWERUSER_PLUSin this privilege check.
const isAtLeastPoweruser =
profile?.groups.includes(Group.POWERUSER) || profile?.hasAdminAccess?.();
throw redirect(302, isAtLeastPoweruser ? '/dashboard' : '/vmcps');
ui/user/src/routes/admin/+page.ts:21
Role.Groups()emitsGroupPowerUserPlusseparately fromGroupPowerUser(seeapiclient/types/user.go:137-141), so a Power User Plus-only profile is treated as a standard user and is sent to/vmcpsrather than the Dashboard after visiting/admin. IncludeGroup.POWERUSER_PLUSin this privilege check.
const isAtLeastPoweruser =
profile?.groups.includes(Group.POWERUSER) || profile?.hasAdminAccess?.();
ui/user/src/routes/admin/setup/+page.svelte:101
- After this splash is continued,
SetupSplashContentmarksSEEN_SPLASH_DIALOG_KEY; when the new owner signs in, the bootstrap session is gone andSetupSplashDialogwill not run its existing model-provider handoff. If no model provider is configured, this flow now goes straight to owner sign-in and then/vmcps//dashboard, leaving the required/models?view=model-providersonboarding unreachable. Preserve that continuation across the owner handoff or route to it before marking setup complete.
if (localUsers.length > 0) {
step = 'signin';
await prepareSignIn();
return;
}
ui/user/src/routes/admin/setup/+page.svelte:89
- If
listExplicitRoleEmailsorinitiateTempLoginfails, this catch only appends a notification and leavessetupTempLoginUrlempty.OwnerSetupPromptthen renders only its loading spinner while the page remains on the sign-in step, with no retry or recovery path without reloading; expose an error/retry state or return to a retryable step.
} catch (err) {
errors.append(err);
}
ui/user/src/routes/admin/setup/+page.ts:16
- This new loader gates a privileged route and chooses among setup, Auth Providers, and Dashboard redirects, but the added spec only renders the component and never exercises these branches. Add loader tests for non-bootstrap/setup-disabled sessions, a non-local configured provider, and local setup so authorization and redirect regressions are caught.
if (!profile.isBootstrapUser?.() || !bootstrapStatus?.setupEnabled) {
throw redirect(307, '/dashboard');
}
ui/user/src/routes/dashboard/+page.ts:10
Role.Groups()emitsGroupPowerUserPlusseparately fromGroupPowerUser(seeapiclient/types/user.go:137-141), so a Power User Plus-only profile is treated as a standard user here and is redirected to/vmcpsinstead of being allowed to use Dashboard. IncludeGroup.POWERUSER_PLUSin this privilege check.
const isAtLeastPoweruser =
profile?.groups.includes(Group.POWERUSER) || profile?.hasAdminAccess?.();
ui/user/src/routes/identity-access/AuthProvidersView.svelte:713
- This only moves the markup;
prepareOwnerSetupabove still runs for bootstrap users on this page and openssetupSignInDialogfor a configured local provider. Visiting/identity-access?view=auth-providersafter local setup therefore still shows the owner-setup modal, contrary to making local owner setup a standalone/admin/setupexperience. Skip the local branch of that effect or redirect bootstrap setup sessions to the new route.
<OwnerSetupPrompt
{isLocalSetup}
localUserEmail={setupLocalUserEmail}
{explicitOwners}
provider={configuringAuthProvider}
tempLoginUrl={setupTempLoginUrl}
onManageLocalUsers={isLocalSetup ? handleManageLocalUsers : undefined}
showTitle={false}
/>
- Files reviewed: 20/20 changed files
- Comments generated: 2
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
276882b to
051c812
Compare
051c812 to
94f8b29
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Four unresolved moderate findings affect bootstrap-status handling and local setup routing.
Review details
Suppressed comments (4)
ui/user/src/routes/+page.ts:19
- This uses the bootstrap
enabledflag, but the backend intentionally reports that flag as true whenOBOT_SERVER_FORCE_ENABLE_BOOTSTRAPis set even after setup is complete, whilesetupEnabledremains false. In that recovery mode a bootstrap login is sent to/admin/setup, which immediately redirects to/dashboardand also bypasses anyrdtarget; this handoff should be gated by the setup-specific flag instead.
if (profile?.isBootstrapUser?.() && bootstrapStatus?.setupEnabled) {
throw redirect(302, '/admin/setup');
ui/user/src/routes/+page.ts:15
- This now makes every logged-in user wait for the bootstrap-status endpoint before honoring a normal redirect. That endpoint performs configured-provider and owner lookups and can return an error, so an unrelated bootstrap-status/database failure would make the root route fail for ordinary users even though they never use bootstrap setup. Only fetch this status for unauthenticated sessions or a logged-in bootstrap user.
const bootstrapStatus = await UserService.getBootstrapStatus();
ui/user/src/routes/admin/+page.ts:35
- The bootstrap status is only needed here for an unauthenticated bootstrap-login page or for deciding where a bootstrap session goes, but this now fetches it for ordinary logged-in users who visit
/adminand should simply receive the existing unauthorized view. Because the status calculation performs provider/owner lookups and can fail, an unrelated backend failure can turn that unauthorized page into a route error; avoid this request for non-bootstrap profiles.
const bootstrapStatus = await UserService.getBootstrapStatus();
ui/user/src/routes/identity-access/AuthProvidersView.svelte:712
- This component is still rendered inside the
ResponsiveDialogon the Auth Providers page.prepareOwnerSetupopens that dialog for bootstrap sessions whose active provider is local, so a bootstrap user who visits/identity-accessstill gets the old local owner setup overlay instead of the new/admin/setupexperience. Redirect local setup to/admin/setupor suppress this dialog for local providers.
<OwnerSetupPrompt
{isLocalSetup}
localUserEmail={setupLocalUserEmail}
{explicitOwners}
provider={configuringAuthProvider}
tempLoginUrl={setupTempLoginUrl}
onManageLocalUsers={isLocalSetup ? handleManageLocalUsers : undefined}
showTitle={false}
- Files reviewed: 20/20 changed files
- Comments generated: 0 new
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved moderate issues remain in setup failure recovery, bootstrap status lookups, and required-flow title handling.
Review details
Suppressed comments (6)
ui/user/src/lib/components/admin/LocalAuthConfigure.svelte:936
- The
requiredonboarding dialog still uses this component (for example,AuthProvidersViewpassesrequired={!!showInitialAuthProvider}), but this unconditional label changes that flow from “Set Up Owner Account” to “Set Up Local” and contradicts the existing required-flow assertions. Preserve the required-specific title here.
Set Up {provider?.name}
ui/user/src/lib/components/admin/OwnerSetupPrompt.svelte:84
- On the dedicated setup route this component is rendered while
prepareSignIn()is still pending. IflistExplicitRoleEmails()orinitiateTempLogin()fails,+page.sveltecatches the error but leavestempLoginUrlempty, so this branch shows a spinner forever with no retry or actionable control. Add an error/retry state for the sign-in preparation failure so bootstrap setup can recover without a full reload.
{#if tempLoginUrl}
<a class="btn btn-secondary w-full" href={tempLoginUrl} rel="external">
{#if provider?.icon}
<img
class="bg-base-100 h-6 w-6 rounded-full p-1 dark:bg-gray-600"
src={provider.icon}
alt={provider.name}
/>
{/if}
<span class="text-center text-sm font-light">
{#if isLocalSetup && localUserEmail}
Sign in as {localUserEmail}
{:else}
Continue with {provider?.name}
{/if}
</span>
</a>
{:else}
<div class="w-full justify-center items-center flex">
<Loading class="size-4" />
</div>
ui/user/src/routes/+page.ts:15
- This now fetches bootstrap status for every logged-in user before the normal home redirect. A transient failure in the bootstrap/auth-provider lookup can therefore make
/fail for standard users, and every such load also pays for a bootstrap database query even though only anonymous or bootstrap sessions need this decision. Fetch it conditionally for those sessions (or otherwise tolerate the failure).
const bootstrapStatus = await UserService.getBootstrapStatus();
ui/user/src/routes/admin/+page.ts:35
- This unconditional status request also makes
/admindepend on the bootstrap subsystem for every authenticated administrator. The redirect only needssetupEnabledfor a bootstrap profile; a failure here now prevents ordinary admins from reaching the admin page, unlike the previous flow. Make the request conditional onprofile?.isBootstrapUser?.()(the page component already fetches status for the unauthenticated login view).
const bootstrapStatus = await UserService.getBootstrapStatus();
ui/user/src/routes/admin/setup/+page.svelte:89
- If
listExplicitRoleEmailsorinitiateTempLoginfails, this catch only appends a global error and leavessetupTempLoginUrlempty.OwnerSetupPromptthen stays on its loading spinner with no retry or actionable error, so bootstrap setup cannot continue without reloading the page; expose the failed state and provide a retry/error action.
} catch (err) {
errors.append(err);
}
ui/user/src/routes/admin/setup/+page.svelte:83
- After the user opens the manage-users dialog,
setupTempLoginUrlcan still contain the previous link.prepareSignIncancels that link before requesting a replacement, but on any subsequent failure the catch only records a notification, so the page keeps rendering a link that was just canceled. Clear the URL before starting the request so a failed refresh cannot expose a stale sign-in action.
async function prepareSignIn() {
if (!localProvider) return;
try {
- Files reviewed: 20/20 changed files
- Comments generated: 0 new
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
Addresses #7942
This addresses so that, on bootstrap login, local auth owner setup is its own separate route/experience rather than being dialogs overlayed over the Auth Providers page.
Also includes following asks: Hide Dashboard from standard users and redirect them to
/vmcpsinstead of/dashboard