fix(plugin-auth)!: under local sign-in off, the last identity provider is the last way in; the direct sign-in door keeps ?redirect=

.changeset/33358-local-sign-in-off-floor.md

@@ -0,0 +1,19 @@
+---
+'@objectstack/plugin-auth': patch
+---
+
+fix(auth)!: under direct sign-in's local sign-in path `off`, the last identity provider is the last way in; the door's landings on the sign-in page keep its `?redirect=`
+
+Clause-②: yes (narrowing) — narrowed: with direct sign-in on and its local sign-in path `off` (`directSignIn.localSignIn: 'off'`, or `OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH=off`), `POST /api/v1/auth/sso/delete-provider` on the last identity provider answers `409 DELETE_RESTRICTED` and deletes nothing while email and password is switched on, where it deleted the row; email and password, and phone sign-in, no longer count as a way in there, because the sign-in page does not offer them. Widened: module exports (not on the package entry) `pageOffersLocalSignIn`, `pageOffersEmailPassword` and `carriedRedirect`, an optional `localSignIn` on `SignInMethodConfigView.features`, and an `asThePageOffers` option on `otherSignInMethods`. The refusal under `off` answers with a new message, the copy key `enterpriseSsoLocalOff`, in all 11 locales.
+Metadata-protocol: none
+Skills-docs: none — no skill under `skills/**` teaches direct sign-in, the provider delete or the boot report; `content/docs/permissions/sso.mdx` gains the rule in this change.
+
+<!-- adr-0087: not-required (no-migration-prescription) No schema key, accepted value or metadata type is added, removed, renamed or re-shaped. The moved answer is a runtime refusal at one auth door under a deployment switch, so there is nothing stored for a migration to rewrite and no tombstone. -->
+
+**BREAKING** for a deployment running direct sign-in with the local sign-in path `off`: Setup's **Delete SSO Provider** on the last identity provider is refused `409 DELETE_RESTRICTED` (nothing deleted) unless another way in the sign-in page offers remains (another identity provider row, a social or OIDC provider with a client ID and secret, or a host sign-in handoff), even with email and password switched on, and even with phone sign-in switched on. Before, the delete went through and the sign-in page then showed "No sign-in method is available". A boot in that state now reports at error under `no_sign_in_method_at_boot`.
+
+Unchanged: the local sign-in path `hidden` (the default) and direct sign-in off, where email and password and phone sign-in count as before; the server's own password sign-in, which `off` never changed.
+
+What to do: nothing, unless you delete the last identity provider under `off`. Register another identity provider first, or set `OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH=hidden` (with email and password sign-in on) and restart.
+
+Also fixed: the direct sign-in door hands the identity provider a sign-in page error address that keeps the page's `?redirect=`, and its own landing on the sign-in page (no provider to start, a refused or rate-limited start) keeps it too. A `?redirect=` that is not a same-origin path is dropped, by the return path's own rule.

content/docs/permissions/sso.mdx

@@ -758,6 +758,17 @@ OAuth client's hand-off and when no provider can be started. It changes the
 form only: `POST /api/v1/auth/sign-in/email` still accepts a password, and the
 `/register` and `/forgot-password` pages are not changed by it.
 
+Because the form then offers no email and password and no phone sign-in, two
+checks that keep a way to sign in do not count them under `off`, even while
+they are switched on: Setup's **Delete SSO Provider** on the last identity
+provider is refused `409 DELETE_RESTRICTED` with nothing deleted, and a boot
+with no identity provider reports at **error** under the grep token
+`no_sign_in_method_at_boot`. Recover by setting
+`OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH=hidden`, and
+`OS_AUTH_EMAIL_PASSWORD_ENABLED=true` too when email and password sign-in is
+off, then restarting. The auth settings save still counts email and password
+under `off`; that check is still open.
+
 **What a visit does.** The Console sign-in page reads `features.directSignIn`
 from `GET /api/v1/auth/config`. That is `true` only while a provider can be
 started, and only when this server mounted the door: a setting turned on
@@ -772,7 +783,10 @@ no session itself.
 `//host`, a backslash, a control character (a tab or a newline included), or
 any of these behind percent-encoding is dropped, and the sign-in returns to
 the Console home page. The query takes `callbackURL` only, once. Anything else
-is refused **400 `VALIDATION_ERROR`**.
+is refused **400 `VALIDATION_ERROR`**. When the sign-in lands back on the
+sign-in page instead (the provider's error, or a start the door could not
+make), the page's own `?redirect=` comes with it, under the same same-origin
+rule.
 
 **No loops.** The page shows itself, with a **Continue with single sign-on**
 button (and the reason, where there is one), instead of starting again when:

packages/plugins/plugin-auth/src/direct-sign-in.test.ts

@@ -276,6 +276,46 @@ describe('runDirectSignIn: the start door', () => {
     expect((host.logger!.warn as any).mock.calls[0][1]).toMatchObject({ reason: 'rate_limited' });
   });
 
+  // The page hands the door its own path and query; its `?redirect=` survives every landing back on `/login`.
+  const PAGE_WITH_REDIRECT = `?callbackURL=${encodeURIComponent('/_console/login?redirect=%2Fapps%2Fcrm%3Fview%3Dall&direct=returned')}`;
+
+  it('hands the vendor a sign-in page error address that keeps the page\'s ?redirect=', async () => {
+    const { host, dispatched } = fakeHost();
+    await runDirectSignIn(door(PAGE_WITH_REDIRECT), host);
+    expect(dispatched[0].body.errorCallbackURL).toBe(`${ORIGIN}/_console/login?redirect=%2Fapps%2Fcrm%3Fview%3Dall`);
+    expect(new URL(dispatched[0].body.errorCallbackURL).searchParams.get('redirect')).toBe('/apps/crm?view=all');
+  });
+
+  it('lands a rate-limited start on the sign-in page with the page\'s ?redirect= kept', async () => {
+    const { host } = fakeHost({ dispatch: async () => new Response('{"message":"Too many requests"}', { status: 429 }) });
+    const res = await runDirectSignIn(door(PAGE_WITH_REDIRECT), host);
+    expect(res.status).toBe(302);
+    const location = new URL(res.headers.get('location')!);
+    expect(`${location.origin}${location.pathname}`).toBe(`${ORIGIN}/_console/login`);
+    expect(location.searchParams.get('error')).toBe(DIRECT_SIGN_IN_RATE_LIMITED);
+    expect(location.searchParams.get('redirect')).toBe('/apps/crm?view=all');
+  });
+
+  it('CONTROL: with no ?redirect= on the page, both landings are the bare sign-in page as before', async () => {
+    const plain = `?callbackURL=${encodeURIComponent('/_console/login?direct=returned')}`;
+    const { host, dispatched } = fakeHost();
+    await runDirectSignIn(door(plain), host);
+    expect(dispatched[0].body.errorCallbackURL).toBe(`${ORIGIN}/_console/login`);
+    const limited = fakeHost({ dispatch: async () => new Response('{}', { status: 429 }) });
+    const res = await runDirectSignIn(door(plain), limited.host);
+    expect(res.headers.get('location')).toBe(`${ORIGIN}/_console/login?error=${DIRECT_SIGN_IN_RATE_LIMITED}`);
+  });
+
+  it.each(FOREIGN_RETURN_PATHS)('drops the foreign ?redirect= %j, the return path\'s own rule, and still lands on the page', async (foreign) => {
+    const page = `/_console/login?redirect=${encodeURIComponent(foreign)}&direct=returned`;
+    const { host, dispatched } = fakeHost();
+    await runDirectSignIn(door(`?callbackURL=${encodeURIComponent(page)}`), host);
+    expect(dispatched[0].body.errorCallbackURL).toBe(`${ORIGIN}/_console/login`);
+    const limited = fakeHost({ dispatch: async () => new Response('{}', { status: 429 }) });
+    const res = await runDirectSignIn(door(`?callbackURL=${encodeURIComponent(page)}`), limited.host);
+    expect(res.headers.get('location')).toBe(`${ORIGIN}/_console/login?error=${DIRECT_SIGN_IN_RATE_LIMITED}`);
+  });
+
   it('lands on the sign-in page with an error, never a retry, when no provider resolves or the start is refused', async () => {
     const cases: Array<Partial<DirectSignInHost>> = [
       { chooseProvider: async () => ({ miss: 'several_providers' }) },

packages/plugins/plugin-auth/src/direct-sign-in.ts

@@ -211,6 +211,33 @@ export function keptReturnPath(raw: unknown): string | undefined {
   return raw;
 }
 
+/**
+ * The `redirect` the sign-in page was opened with, read from the kept return
+ * path (the page hands the door its own path and query), so a landing back on
+ * the page (the door's own error, or the identity provider's) still knows where
+ * the person was going. Carried only when it is sent once and is itself a kept
+ * return path ({@link keptReturnPath}); otherwise none.
+ */
+export function carriedRedirect(kept: string | undefined): string | undefined {
+  if (kept === undefined) return undefined;
+  let values: string[];
+  try {
+    values = new URL(kept, 'https://return-path.invalid').searchParams.getAll('redirect');
+  } catch {
+    return undefined;
+  }
+  return values.length === 1 ? keptReturnPath(values[0]) : undefined;
+}
+
+/** The Console's `/login`, with the carried `redirect` and the `error` code, when there are any. */
+function signInPageUrl(host: DirectSignInHost, redirect: string | undefined, error?: string): string {
+  const params = new URLSearchParams();
+  if (error !== undefined) params.set('error', error);
+  if (redirect !== undefined) params.set('redirect', redirect);
+  const query = params.toString();
+  return `${host.consolePageUrl('/login')}${query ? `?${query}` : ''}`;
+}
+
 /** Which vendor start a provider takes. */
 export type DirectSignInProvider =
   | { kind: 'oauth'; providerId: string }
@@ -343,6 +370,8 @@ function queryOf(url: URL): Record<string, string[]> {
  *   Console's `/login?error=direct_sign_in_unavailable`, and one `warn` line;
  *   a start the vendor's rate limit refused (`429`) ⇒ the same, with
  *   `error=direct_sign_in_rate_limited` and the reason `rate_limited`.
+ * - Every landing on `/login`, the door's and the vendor's error return
+ *   address alike, carries the page's `?redirect=` ({@link carriedRedirect}).
  *
  * The caller mounts the door only while the setting is on; a call with it off
  * answers `404 RESOURCE_NOT_FOUND` like an unmounted route.
@@ -369,13 +398,18 @@ export async function runDirectSignIn(request: Request, host: DirectSignInHost):
     });
   }
 
+  const requested = url.searchParams.get('callbackURL');
+  const kept = keptReturnPath(requested);
+  // Every landing on the sign-in page keeps the page's own `?redirect=`.
+  const redirectTo = carriedRedirect(kept);
+
   const unavailable = (reason: DirectSignInMiss, providerId?: string): Response => {
     const text = `[auth] Direct sign-in not started: ${reason}. The sign-in page is shown instead.`;
     const meta = { entry: 'direct_sign_in', reason, ...(providerId ? { providerId } : {}) };
     if (host.logger) host.logger.warn(text, meta);
     else console.warn(text, meta);
     const code = reason === 'rate_limited' ? DIRECT_SIGN_IN_RATE_LIMITED : DIRECT_SIGN_IN_UNAVAILABLE;
-    return redirect(`${host.consolePageUrl('/login')}?error=${code}`);
+    return redirect(signInPageUrl(host, redirectTo, code));
   };
 
   let chosen: DirectSignInProvider | { miss: DirectSignInProviderMiss };
@@ -386,8 +420,6 @@ export async function runDirectSignIn(request: Request, host: DirectSignInHost):
   }
   if ('miss' in chosen) return unavailable(chosen.miss);
 
-  const requested = url.searchParams.get('callbackURL');
-  const kept = keptReturnPath(requested);
   if (requested !== null && kept === undefined) {
     host.logger?.debug?.('[auth] Direct sign-in: the return path is not a same-origin path and was dropped.', {
       entry: 'direct_sign_in',
@@ -397,7 +429,7 @@ export async function runDirectSignIn(request: Request, host: DirectSignInHost):
   const callbackURL = kept !== undefined ? `${origin}${kept}` : host.consolePageUrl('/');
   // Defence in depth: whatever was kept, the sign-in returns to this origin.
   if (new URL(callbackURL).origin !== new URL(origin).origin) return unavailable('start_refused');
-  const errorCallbackURL = host.consolePageUrl('/login');
+  const errorCallbackURL = signInPageUrl(host, redirectTo);
 
   const body = chosen.kind === 'oauth'
     ? { provider: chosen.providerId, callbackURL, errorCallbackURL, disableRedirect: true }

packages/plugins/plugin-auth/src/sign-in-method-floor.test.ts

@@ -470,6 +470,9 @@ const PASSWORD = 'S3cure!Passw0rd-30300';
 const SYS = { isSystem: true } as const;
 
 const ENV_KEYS_30300 = [
+  'OS_AUTH_DIRECT_SIGN_IN_ENABLED',
+  'OS_AUTH_DIRECT_SIGN_IN_PROVIDER',
+  'OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH',
   'OS_AUTH_EMAIL_PASSWORD_ENABLED',
   'OS_AUTH_GOOGLE_ENABLED',
   'GOOGLE_CLIENT_ID',
@@ -550,8 +553,8 @@ describe('the last way to sign in is kept, and a boot without one is said (#3030
   // ── Door 1 ─────────────────────────────────────────────────────────────────
 
   /** A signed-in administrator who owns `providers`, on a deployment with enterprise SSO switched on. */
-  async function adminWithProviders(providers: string[]) {
-    const env = await boot({ plugins: { sso: true } });
+  async function adminWithProviders(providers: string[], authOptions: Record<string, unknown> = {}) {
+    const env = await boot({ plugins: { sso: true }, ...authOptions });
     const { ql, manager } = env;
     const signedUp = await manager.handleRequest(
       new Request(`${AUTH}/sign-up/email`, {
@@ -651,8 +654,55 @@ describe('the last way to sign in is kept, and a boot without one is said (#3030
     expect(await providerIds()).toEqual(['okta']);
   }, 30_000);
 
+  // The local sign-in path behind direct sign-in: under `off` the sign-in page
+  // draws no email and password, so it is no way in there, on or not.
+
+  it('under the local sign-in path off, deleting the last provider is refused with email and password on: 409, nothing deleted', async () => {
+    const { manager, remove, providerIds } = await adminWithProviders(['okta'], { directSignIn: { localSignIn: 'off' } });
+    expect(manager.getPublicConfig().emailPassword.enabled).toBe(true);
+    expect(manager.getPublicConfig().features.localSignIn).toBe('off');
+
+    const res = await remove('okta');
+    const body = await res.json();
+    expect(res.status, JSON.stringify(body)).toBe(409);
+    expect(body).toMatchObject({
+      code: 'DELETE_RESTRICTED',
+      message: SIGN_IN_FLOOR_COPY.en.enterpriseSsoLocalOff,
+      lastSignInMethod: 'enterprise_sso',
+    });
+    expect(await providerIds()).toEqual(['okta']);
+  }, 30_000);
+
+  it('CONTROL: with the local sign-in path hidden (the page offers email and password), deleting the last provider goes through as before', async () => {
+    const { manager, remove, providerIds } = await adminWithProviders(['okta'], { directSignIn: { localSignIn: 'hidden' } });
+    expect(manager.getPublicConfig().features.localSignIn).toBe('hidden');
+
+    const res = await remove('okta');
+    expect(res.status, await res.clone().text()).toBe(200);
+    expect(await providerIds()).toEqual([]);
+  }, 30_000);
+
   // ── Door 2 ─────────────────────────────────────────────────────────────────
 
+  it('a boot under the local sign-in path off with no provider is said at error, email and password on or not', async () => {
+    const { manager, reports } = await boot({ directSignIn: { localSignIn: 'off' } });
+    expect(manager.getPublicConfig().emailPassword.enabled).toBe(true);
+    expect(reports()).toHaveLength(1);
+    expect(reports()[0]).toContain('OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH=hidden');
+  }, 30_000);
+
+  it('a boot under the local sign-in path off with email and password off too names both settings to recover', async () => {
+    const { manager, reports } = await boot({ emailAndPassword: { enabled: false }, directSignIn: { localSignIn: 'off' } });
+    expect(manager.getPublicConfig().emailPassword.enabled).toBe(false);
+    expect(reports()).toHaveLength(1);
+    expect(reports()[0]).toContain('OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH=hidden and OS_AUTH_EMAIL_PASSWORD_ENABLED=true');
+  }, 30_000);
+
+  it('CONTROL: the same boot with the local sign-in path hidden says nothing', async () => {
+    const { reports } = await boot({ directSignIn: { localSignIn: 'hidden' } });
+    expect(reports()).toEqual([]);
+  }, 30_000);
+
   it('a redeploy that drops the last provider while email and password is off is said at error, and still boots', async () => {
     const { manager, reports } = await boot({ emailAndPassword: { enabled: false } });
 
@@ -746,6 +796,43 @@ describe('the floor judges a provider delete and a boot by what it can read (#30
     expect(await resolveNoSignInMethodReport(deps({ hostSignInHandoff: true }))).toBeNull();
   });
 
+  it('counts the local ways in only as the sign-in page offers them: under localSignIn off, neither email and password nor phone sign-in', async () => {
+    const LOCAL_OFF: SignInMethodConfigView = {
+      emailPassword: { enabled: true },
+      socialProviders: [],
+      features: { sso: true, phoneNumber: true, localSignIn: 'off' },
+    };
+    await expect(ssoProviderDeleteRefusal(deps({ config: LOCAL_OFF, rows: [{ provider_id: 'okta' }] }), 'okta')).resolves.toEqual({
+      status: 'CONFLICT',
+      code: 'DELETE_RESTRICTED',
+      message: SIGN_IN_FLOOR_COPY.en.enterpriseSsoLocalOff,
+      lastSignInMethod: 'enterprise_sso',
+    });
+    expect(await resolveNoSignInMethodReport(deps({ config: LOCAL_OFF, rows: [] }))).toContain('OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH=hidden');
+    // CONTROL: the same deployment with the path hidden counts both as before.
+    const HIDDEN: SignInMethodConfigView = { ...LOCAL_OFF, features: { ...LOCAL_OFF.features, localSignIn: 'hidden' } };
+    await expect(ssoProviderDeleteRefusal(deps({ config: HIDDEN, rows: [{ provider_id: 'okta' }] }), 'okta')).resolves.toBeUndefined();
+    expect(await resolveNoSignInMethodReport(deps({ config: HIDDEN, rows: [] }))).toBeNull();
+    const PHONE_ONLY: SignInMethodConfigView = { ...HIDDEN, emailPassword: { enabled: false } };
+    await expect(ssoProviderDeleteRefusal(deps({ config: PHONE_ONLY, rows: [{ provider_id: 'okta' }] }), 'okta')).resolves.toBeUndefined();
+    // Phone sign-in alone under off, email and password off too: the page draws neither, so the delete is refused,
+    // and the remedy is the local-path one (turning email and password back on alone draws nothing under off).
+    const PHONE_ONLY_OFF: SignInMethodConfigView = { ...PHONE_ONLY, features: { ...PHONE_ONLY.features, localSignIn: 'off' } };
+    await expect(ssoProviderDeleteRefusal(deps({ config: PHONE_ONLY_OFF, rows: [{ provider_id: 'okta' }] }), 'okta')).resolves.toMatchObject({
+      code: 'DELETE_RESTRICTED',
+      message: SIGN_IN_FLOOR_COPY.en.enterpriseSsoLocalOff,
+    });
+    const report = await resolveNoSignInMethodReport(deps({ config: PHONE_ONLY_OFF, rows: [] }));
+    expect(report).toContain('OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH=hidden and OS_AUTH_EMAIL_PASSWORD_ENABLED=true');
+    expect(report).not.toContain('no phone-number sign-in');
+    // CONTROL: email and password off with the page drawing local sign-in keeps the email-off copy and report.
+    const EMAIL_OFF_HIDDEN: SignInMethodConfigView = { ...OFF, features: { sso: true, localSignIn: 'hidden' } };
+    await expect(ssoProviderDeleteRefusal(deps({ config: EMAIL_OFF_HIDDEN, rows: [{ provider_id: 'okta' }] }), 'okta')).resolves.toMatchObject({
+      message: SIGN_IN_FLOOR_COPY.en.enterpriseSso,
+    });
+    expect(await resolveNoSignInMethodReport(deps({ config: EMAIL_OFF_HIDDEN, rows: [] }))).toContain('OS_AUTH_EMAIL_PASSWORD_ENABLED=true and restart');
+  });
+
   it('says it once at error (warn on a sink without error), and never throws', async () => {
     const error = vi.fn();
     const warn = vi.fn();