From 3b435169045adc65c168f8c775732f0f236d5d4e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 10:20:59 +0000 Subject: [PATCH 1/3] fix(plugin-auth): a cleared auth setting restores the deployment-declared value in the same pass (WIP) Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CZPLJKSBMtARXMHc4W --- .../plugins/plugin-auth/src/auth-plugin.ts | 162 +++++++++++++++--- .../plugin-auth/src/auth-settings-declared.ts | 140 +++++++++++++++ 2 files changed, 275 insertions(+), 27 deletions(-) create mode 100644 packages/plugins/plugin-auth/src/auth-settings-declared.ts diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 6ef278f3a58..009916643e6 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -124,6 +124,13 @@ import { import { cloudManagedRefusal } from './cloud-managed.js'; import { registerCopyInvitationLinkAction } from './invitation-link-action.js'; import { allowedIpRangesWritePredicate, parseAllowedIpRanges } from './allowed-ip-ranges.js'; +import { + AUTH_SETTING_RESTORE, + captureDeclaredAuthOptions, + copyDeclared, + restoreGroupKeys, + type DeclaredAuthOptions, +} from './auth-settings-declared.js'; import { FORWARDED_CLIENT_ADDRESS_HEADERS } from './client-address-forwarding.js'; import { runAdminBanUser, @@ -547,14 +554,19 @@ export class AuthPlugin implements Plugin { private allowListUndeterminedWarned = false; private allowListUntrustedHeaderWarned = false; /** - * [#30203] The allow-list the deployment itself declared (plugin options or - * an `auth:configure` handler), taken before any setting is applied: what - * the running list returns to when the stored `allowed_ip_ranges` is cleared - * or restored to its default. + * [#30203, #30312] What the deployment itself declared (plugin options and + * `auth:configure` handlers) for every option an `auth.*` setting can drive, + * taken before any setting is applied: what the running value returns to when + * the stored key is cleared or restored to its default. + */ + private declaredAuthOptions: DeclaredAuthOptions = {}; + /** + * [#30203, #30312] The `auth.*` keys whose stored value the running config + * was last given (or whose restore the manager refused): a key leaving this + * set by resolving `source: 'default'` hands its value back to the + * declaration. Boot never restores, so an unset key costs nothing. */ - private declaredAllowedIpRanges: string[] | undefined; - /** [#30203] Whether the running allow-list is the one a stored setting gave it. */ - private allowedIpRangesFromSettings = false; + private authSettingsFromStore = new Set(); private configuredSocialProviders: SocialProviderConfig | undefined; /** Whether each provider declared at boot holds a credential pair (the sign-in floor's count). */ private bootProviderCredentials: ReadonlyMap = new Map(); @@ -794,7 +806,7 @@ export class AuthPlugin implements Plugin { logger: ctx.logger, }); - this.declaredAllowedIpRanges = authConfig.allowedIpRanges ? [...authConfig.allowedIpRanges] : undefined; + this.declaredAuthOptions = captureDeclaredAuthOptions(authConfig); // Initialize auth manager with data engine this.authManager = new AuthManager(authConfig); // ADR-0092 D6 — remember the storage better-auth will actually use so the @@ -2120,6 +2132,18 @@ export class AuthPlugin implements Plugin { } const isExplicit = (key: string) => (sources[key] ?? 'default') !== 'default'; + // [#30312] A key the running config took from the store that now + // resolves `source: 'default'` (cleared, or the namespace restored to + // its defaults) hands its value back to the deployment's declaration in + // this same pass, through the piece a save of it travels. Without this, + // every branch below skipped it and the last stored value ruled until a + // restart while the console showed the default. See auth-settings-declared.ts. + const declared = this.declaredAuthOptions; + const restoring = (key: string) => !isExplicit(key) && this.authSettingsFromStore.has(key); + const groupRestoring = (group: 'rateLimit' | 'google' | 'audience'): string[] => { + const keys = restoreGroupKeys(group); + return keys.some(isExplicit) ? [] : keys.filter(restoring); + }; const asBoolean = (value: unknown, fallback: boolean): boolean => { if (typeof value === 'boolean') return value; if (typeof value === 'string') return value.toLowerCase() !== 'false'; @@ -2328,9 +2352,6 @@ export class AuthPlugin implements Plugin { const d = asPositiveInt(values.session_refresh_days); if (d !== undefined) session.updateAge = d * 86_400; } - if (Object.keys(session).length > 0) { - patch.session = session as AuthManagerOptions['session']; - } // Session controls (ADR-0069 D4) — idle / absolute / concurrent. 0 = off; // non-negative reader so an explicit 0 disables. @@ -2352,16 +2373,10 @@ export class AuthPlugin implements Plugin { } // Network gating (ADR-0069 D5) — parse the CIDR/IP textarea into a list. - // [#30203] A cleared or restored-default value resolves with source - // `default`. It hands the gate back to the deployment's own declaration - // at once, rather than leaving the last stored list in force until a - // restart while the console shows the field empty. + // [#30203] A cleared value hands the gate back to the deployment's own + // declaration at once (the restore arms below). if (isExplicit('allowed_ip_ranges')) { patch.allowedIpRanges = parseAllowedIpRanges(values.allowed_ip_ranges); - this.allowedIpRangesFromSettings = true; - } else if (this.allowedIpRangesFromSettings) { - patch.allowedIpRanges = this.declaredAllowedIpRanges ? [...this.declaredAllowedIpRanges] : []; - this.allowedIpRangesFromSettings = false; } // Anti-abuse (ADR-0069 D2) — account lockout (custom, per-identity) @@ -2436,11 +2451,80 @@ export class AuthPlugin implements Plugin { : undefined; } - const audienceKeys = [ - 'audience_posture', - 'audience_allowed_email_domains', - 'audience_self_registration_permission_set', - ]; + // [#30312] The restore arms: each key that left the store goes back to + // its declared value through the same piece a save of it rides, so a + // refusable block is judged on a restore exactly as on a save. + const restoredInPatch: string[] = []; + let sessionRestoring = false; + for (const [key, arm] of Object.entries(AUTH_SETTING_RESTORE)) { + if (!restoring(key)) continue; + switch (arm.via) { + case 'patch': + (patch as Record)[arm.field] = copyDeclared(declared[arm.field]); + restoredInPatch.push(key); + break; + case 'emailAndPassword': + // The declared verification flag is the deployment's own + // declaration, as it was at boot (#20389). + emailAndPasswordField( + key, + { [arm.field]: declared.emailAndPassword?.[arm.field] }, + arm.field === 'requireEmailVerification' ? { requireEmailVerificationFrom: 'deployment' } : undefined, + ); + break; + case 'plugins': + refusable.push({ + key, + patch: { + plugins: { + [arm.field]: (declared.plugins as Record | undefined)?.[arm.field], + } as AuthManagerOptions['plugins'], + }, + }); + break; + case 'session': + sessionRestoring = true; + restoredInPatch.push(key); + break; + case 'mfaRequired': + // As a save of the declared value: on is one piece with its + // enrollment path; off leaves enrolled factors verifiable. + if (declared.mfaRequired === true) { + refusable.push({ + key, + patch: { mfaRequired: true, plugins: { twoFactor: true } as AuthManagerOptions['plugins'] }, + }); + } else { + patch.mfaRequired = declared.mfaRequired; + restoredInPatch.push(key); + } + break; + case 'group': + break; + } + } + // Session lifetime rides one `session` block, merged over the declared + // one: a stored field overrides its declared twin, the rest stand. + if (Object.keys(session).length > 0 || sessionRestoring) { + const merged = { ...(declared.session ?? {}), ...session }; + patch.session = (Object.keys(merged).length > 0 ? merged : copyDeclared(declared.session)) as + AuthManagerOptions['session']; + } + const rateLimitRestoring = groupRestoring('rateLimit'); + if (rateLimitRestoring.length > 0) { + patch.rateLimit = copyDeclared(declared.rateLimit); + restoredInPatch.push(...rateLimitRestoring); + } + const googleRestoring = groupRestoring('google'); + if (googleRestoring.length > 0) { + const configured = this.configuredSocialProviders ? { ...this.configuredSocialProviders } : undefined; + patch.socialProviders = configured && Object.keys(configured).length > 0 ? configured : undefined; + restoredInPatch.push(...googleRestoring); + } + // Keys whose restore the manager refused: still owed on the next pass. + const owed: string[] = []; + + const audienceKeys = restoreGroupKeys('audience'); // [#20412] Each piece is applied in its own `try`, so a refusal is // contained to the piece it names. `error`, not `warn` (AGENTS.md, @@ -2456,6 +2540,7 @@ export class AuthPlugin implements Plugin { try { authManager.applyConfigPatch(piece, options); } catch (refusal: any) { + owed.push(...keys.filter(restoring)); const named = keys.map((key) => `auth.${key}`).join(', '); ctx.logger.error( `[auth] auth settings REFUSED (${named}) — the standing runtime value keeps ruling while the settings ` + @@ -2467,9 +2552,12 @@ export class AuthPlugin implements Plugin { if (Object.keys(patch).length > 0) { const pieceKeys = new Set(refusable.map((piece) => piece.key)); applyPiece( - Object.keys(values).filter( - (key) => isExplicit(key) && !pieceKeys.has(key) && !audienceKeys.includes(key), - ), + [ + ...Object.keys(values).filter( + (key) => isExplicit(key) && !pieceKeys.has(key) && !audienceKeys.includes(key), + ), + ...restoredInPatch, + ], patch, ); } @@ -2559,7 +2647,27 @@ export class AuthPlugin implements Plugin { ); } } + } else { + // [#30312] No audience key stored any more: the declared audience + // comes back whole, judged by the same merged-result validation. + const audienceRestoring = groupRestoring('audience'); + if (audienceRestoring.length > 0) { + try { + authManager.applyConfigPatch({ audience: copyDeclared(declared.audience) }); + } catch (audienceErr: any) { + owed.push(...audienceRestoring); + ctx.logger.error( + `[auth] audience settings REFUSED — the standing posture ('${authManager.getAudience().posture}') keeps ruling. ` + + String(audienceErr?.message ?? audienceErr), + ); + } + } } + + this.authSettingsFromStore = new Set([ + ...Object.keys(AUTH_SETTING_RESTORE).filter(isExplicit), + ...owed, + ]); } catch (err: any) { // [#20412] A refusal no longer reaches here — each piece above // contains its own. What does is a pass that failed as a whole, the diff --git a/packages/plugins/plugin-auth/src/auth-settings-declared.ts b/packages/plugins/plugin-auth/src/auth-settings-declared.ts new file mode 100644 index 00000000000..9ea08e97e85 --- /dev/null +++ b/packages/plugins/plugin-auth/src/auth-settings-declared.ts @@ -0,0 +1,140 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#30312] What a cleared `auth.*` setting hands the running config back to. + * + * A stored `auth.*` value rules the manager while it is stored. When it is + * cleared (the console sends `null`) or the namespace is restored to its + * defaults, the key resolves `source: 'default'`, and the running value returns + * in the same pass to what the deployment itself declared: the plugin options + * and `auth:configure` handlers, as the boot pass handed them to the manager, + * and the manager's own built-in default where they declared nothing. The + * manifest default is a console default and never rules here. + * + * Two pieces live here, both data: + * + * - {@link captureDeclaredAuthOptions} takes the declared values once, in + * `init()`, before any setting is applied. + * - {@link AUTH_SETTING_RESTORE} names, for every key the settings pass + * applies, the piece its declared value goes back through. A key whose + * stored value lands in a block the manager can refuse (`emailAndPassword`, + * `plugins`, `audience`) restores through that same block, so the manager + * judges a restored value exactly as it judges a saved one. Keys that rule + * one runtime value together (the rate-limit pair, the Google trio, the + * audience trio) restore as a group, and only once none of them is stored. + */ + +import type { AuthManagerOptions } from './auth-manager.js'; + +/** The manager options a stored `auth.*` setting can drive. */ +const DECLARED_FIELDS = [ + 'emailAndPassword', + 'membershipPolicy', + 'plugins', + 'passwordRequireComplexity', + 'passwordMinClasses', + 'passwordHistoryCount', + 'passwordExpiryDays', + 'mfaRequired', + 'mfaGracePeriodDays', + 'apiKeyDefaultExpiryDays', + 'apiKeyMaxExpiryDays', + 'apiKeyInactivityDays', + 'apiKeyPolicyStartedAt', + 'session', + 'sessionIdleTimeoutMinutes', + 'sessionAbsoluteMaxHours', + 'maxConcurrentSessions', + 'allowedIpRanges', + 'lockoutThreshold', + 'lockoutDurationMinutes', + 'rateLimit', + 'audience', +] as const satisfies ReadonlyArray; + +type DeclaredField = (typeof DECLARED_FIELDS)[number]; + +/** The deployment's own values for {@link DECLARED_FIELDS}, taken before any setting applies. */ +export type DeclaredAuthOptions = Readonly>; + +/** A fresh copy one level deep, so the manager never shares an object with the snapshot. */ +export function copyDeclared(value: T): T { + if (Array.isArray(value)) return [...value] as T; + if (value !== null && typeof value === 'object') return { ...(value as object) } as T; + return value; +} + +/** + * The declared values, as the boot pass hands them to the manager: call it in + * `init()` after `auth:configure`, on the config the manager is built from. + */ +export function captureDeclaredAuthOptions(config: AuthManagerOptions): DeclaredAuthOptions { + const declared: Record = {}; + for (const field of DECLARED_FIELDS) declared[field] = copyDeclared(config[field]); + return declared as DeclaredAuthOptions; +} + +type EmailAndPasswordField = keyof NonNullable; + +/** The piece a key's declared value goes back through. */ +export type AuthSettingRestore = + /** A top-level option the manager does not judge: rides the pass's plain patch. */ + | { readonly via: 'patch'; readonly field: DeclaredField } + /** A field of `emailAndPassword`: its own refusable piece, judged by `assertAudienceConfig`. */ + | { readonly via: 'emailAndPassword'; readonly field: EmailAndPasswordField } + /** A flag of `plugins`: its own refusable piece, judged by `assertScimAdminCoherence`. */ + | { readonly via: 'plugins'; readonly field: 'passwordRejectBreached' } + /** A field of `session`, merged over the declared `session` object. */ + | { readonly via: 'session'; readonly field: 'expiresIn' | 'updateAge' } + /** Enforced MFA: restored exactly as a save of the declared value would apply. */ + | { readonly via: 'mfaRequired' } + /** One runtime value ruled by several keys: restored whole once none of them is stored. */ + | { readonly via: 'group'; readonly group: 'rateLimit' | 'google' | 'audience' }; + +/** + * Every key the auth settings pass applies, and how its declared value comes + * back. `auth-settings-clear.pin.test.ts` holds this table to the shipped auth + * manifest: a manifest key missing here is a clear that would not take effect. + */ +export const AUTH_SETTING_RESTORE: Readonly> = { + email_password_enabled: { via: 'emailAndPassword', field: 'enabled' }, + signup_enabled: { via: 'emailAndPassword', field: 'disableSignUp' }, + require_email_verification: { via: 'emailAndPassword', field: 'requireEmailVerification' }, + password_min_length: { via: 'emailAndPassword', field: 'minPasswordLength' }, + password_max_length: { via: 'emailAndPassword', field: 'maxPasswordLength' }, + membership_policy: { via: 'patch', field: 'membershipPolicy' }, + password_reject_breached: { via: 'plugins', field: 'passwordRejectBreached' }, + password_require_complexity: { via: 'patch', field: 'passwordRequireComplexity' }, + password_min_classes: { via: 'patch', field: 'passwordMinClasses' }, + password_history_count: { via: 'patch', field: 'passwordHistoryCount' }, + password_expiry_days: { via: 'patch', field: 'passwordExpiryDays' }, + mfa_required: { via: 'mfaRequired' }, + mfa_grace_period_days: { via: 'patch', field: 'mfaGracePeriodDays' }, + api_key_default_expiry_days: { via: 'patch', field: 'apiKeyDefaultExpiryDays' }, + api_key_max_expiry_days: { via: 'patch', field: 'apiKeyMaxExpiryDays' }, + api_key_inactivity_days: { via: 'patch', field: 'apiKeyInactivityDays' }, + api_key_policy_started_at: { via: 'patch', field: 'apiKeyPolicyStartedAt' }, + session_expiry_days: { via: 'session', field: 'expiresIn' }, + session_refresh_days: { via: 'session', field: 'updateAge' }, + session_idle_timeout_minutes: { via: 'patch', field: 'sessionIdleTimeoutMinutes' }, + session_absolute_max_hours: { via: 'patch', field: 'sessionAbsoluteMaxHours' }, + max_concurrent_sessions_per_user: { via: 'patch', field: 'maxConcurrentSessions' }, + allowed_ip_ranges: { via: 'patch', field: 'allowedIpRanges' }, + lockout_threshold: { via: 'patch', field: 'lockoutThreshold' }, + lockout_duration_minutes: { via: 'patch', field: 'lockoutDurationMinutes' }, + rate_limit_max: { via: 'group', group: 'rateLimit' }, + rate_limit_window_seconds: { via: 'group', group: 'rateLimit' }, + google_enabled: { via: 'group', group: 'google' }, + google_client_id: { via: 'group', group: 'google' }, + google_client_secret: { via: 'group', group: 'google' }, + audience_posture: { via: 'group', group: 'audience' }, + audience_allowed_email_domains: { via: 'group', group: 'audience' }, + audience_self_registration_permission_set: { via: 'group', group: 'audience' }, +}; + +/** The keys of one restore group. */ +export function restoreGroupKeys(group: 'rateLimit' | 'google' | 'audience'): string[] { + return Object.entries(AUTH_SETTING_RESTORE) + .filter(([, arm]) => arm.via === 'group' && arm.group === group) + .map(([key]) => key); +} From 3c8f92ee1415ebb25133dbc725f106af2ae1d986 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 10:25:09 +0000 Subject: [PATCH 2/3] fix(plugin-auth): restore arms for every auth key, floor judges a cleared toggle at its declaration, pins Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CZPLJKSBMtARXMHc4W --- .../plugins/plugin-auth/src/auth-plugin.ts | 37 +- .../src/auth-settings-clear.pin.test.ts | 356 ++++++++++++++++++ .../plugin-auth/src/auth-settings-declared.ts | 7 +- .../plugin-auth/src/sign-in-method-floor.ts | 16 +- 4 files changed, 395 insertions(+), 21 deletions(-) create mode 100644 packages/plugins/plugin-auth/src/auth-settings-clear.pin.test.ts diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 009916643e6..b86da09c7cb 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -2115,6 +2115,8 @@ export class AuthPlugin implements Plugin { // Read at save time: the index is set in init, after `auth:configure`. providerHasCredentials: (provider) => providerCredentialsFrom(this.bootProviderCredentials)(provider), env: () => ((globalThis as any)?.process?.env ?? {}) as Record, + // [#30312] A cleared toggle returns to the declaration: judge it there. + declaredEmailPasswordEnabled: () => this.declaredAuthOptions.emailAndPassword?.enabled !== false, }, ctx.logger, ); @@ -2140,7 +2142,7 @@ export class AuthPlugin implements Plugin { // restart while the console showed the default. See auth-settings-declared.ts. const declared = this.declaredAuthOptions; const restoring = (key: string) => !isExplicit(key) && this.authSettingsFromStore.has(key); - const groupRestoring = (group: 'rateLimit' | 'google' | 'audience'): string[] => { + const groupRestoring = (group: 'rateLimit' | 'google'): string[] => { const keys = restoreGroupKeys(group); return keys.some(isExplicit) ? [] : keys.filter(restoring); }; @@ -2595,6 +2597,24 @@ export class AuthPlugin implements Plugin { // Missing required siblings are NOT guessed: the patch goes out // without them and `applyConfigPatch` refuses the merged result // (standing config keeps ruling — refusing to OPEN fails closed). + // [#30312] No posture stored any more (cleared, or the namespace + // restored) while an audience key had been: the declared audience comes + // back whole, judged by the same merged-result validation. A sibling + // still stored without a posture is then IGNORED as below. + if (!isExplicit('audience_posture')) { + const audienceRestoring = audienceKeys.filter(restoring); + if (audienceRestoring.length > 0) { + try { + authManager.applyConfigPatch({ audience: copyDeclared(declared.audience) }); + } catch (audienceErr: any) { + owed.push(...audienceRestoring); + ctx.logger.error( + `[auth] audience settings REFUSED — the standing posture ('${authManager.getAudience().posture}') keeps ruling. ` + + String(audienceErr?.message ?? audienceErr), + ); + } + } + } if (audienceKeys.some((key) => isExplicit(key))) { const rawPosture = values.audience_posture; if (!isExplicit('audience_posture')) { @@ -2647,21 +2667,6 @@ export class AuthPlugin implements Plugin { ); } } - } else { - // [#30312] No audience key stored any more: the declared audience - // comes back whole, judged by the same merged-result validation. - const audienceRestoring = groupRestoring('audience'); - if (audienceRestoring.length > 0) { - try { - authManager.applyConfigPatch({ audience: copyDeclared(declared.audience) }); - } catch (audienceErr: any) { - owed.push(...audienceRestoring); - ctx.logger.error( - `[auth] audience settings REFUSED — the standing posture ('${authManager.getAudience().posture}') keeps ruling. ` + - String(audienceErr?.message ?? audienceErr), - ); - } - } } this.authSettingsFromStore = new Set([ diff --git a/packages/plugins/plugin-auth/src/auth-settings-clear.pin.test.ts b/packages/plugins/plugin-auth/src/auth-settings-clear.pin.test.ts new file mode 100644 index 00000000000..b2dd78df591 --- /dev/null +++ b/packages/plugins/plugin-auth/src/auth-settings-clear.pin.test.ts @@ -0,0 +1,356 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#30312] Clearing an `auth.*` setting (the console sends `null`) or restoring + * the namespace's defaults takes effect in the same pass: the key resolves + * `source: 'default'`, and the running value returns to what the deployment + * itself declared (plugin options, `auth:configure`), or to the manager's own + * built-in default where it declared nothing. Measured before the fix on this + * composition: `lockout_threshold` 3 and `max_concurrent_sessions_per_user` 2 + * stayed in force after both were saved `null`, until a restart. + * + * The composition is #30203's: the real `AuthPlugin` bound to a real + * `SettingsService` carrying the shipped auth manifest, with the real settings + * routes in front of it for the save-door cases. One pin per key family + * (`AUTH_SETTING_RESTORE`'s pieces), the namespace reset, the table held to + * the manifest, and the refusable piece the restore made reachable: a cleared + * `email_password_enabled` on a deployment that declared it off is judged by + * the sign-in floor as the off it now applies. + * + * The running values are read off the manager's config (`running()`): most of + * these options have no public reader, and the config is what every consumer + * reads at request time. + * + * `@objectstack/service-settings` resolves to its `src/` here (this package's + * `vitest.config.ts`). + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { IHttpRequest, IHttpResponse, IHttpServer, RouteHandler } from '@objectstack/spec/contracts'; +import { + SettingsService, + builtinSettingsManifests, + registerSettingsRoutes, +} from '@objectstack/service-settings'; +import { AuthPlugin } from './auth-plugin.js'; +import type { AuthManager, AuthManagerOptions } from './auth-manager.js'; +import { AUTH_SETTING_RESTORE } from './auth-settings-declared.js'; + +const ADMIN_CTX = { enforced: true, permissions: ['manage_platform_settings'], userId: 'usr_admin' }; +const authManifest = builtinSettingsManifests.find((m) => m.namespace === 'auth')!; + +class MockHttp implements IHttpServer { + routes = new Map(); + private add(method: string, path: string, handler: RouteHandler) { + this.routes.set(`${method} ${path}`, handler); + } + get(path: string, h: RouteHandler) { this.add('GET', path, h); return this as any; } + post(path: string, h: RouteHandler) { this.add('POST', path, h); return this as any; } + put(path: string, h: RouteHandler) { this.add('PUT', path, h); return this as any; } + delete(path: string, h: RouteHandler) { this.add('DELETE', path, h); return this as any; } + patch(path: string, h: RouteHandler) { this.add('PATCH', path, h); return this as any; } + use() { return this as any; } + listen() { return Promise.resolve(); } + close() { return Promise.resolve(); } + getInstance() { return null; } +} + +interface Rig { + svc: SettingsService; + manager: AuthManager; + save(body: Record, transport?: { headers?: Record; remoteAddress?: string }): + Promise<{ status: number; body: any }>; +} + +async function boot( + opts: { env?: Record; svc?: SettingsService; plugin?: Record } = {}, +): Promise { + const svc = opts.svc ?? new SettingsService({ env: opts.env ?? {} }); + if (!opts.svc) svc.registerManifest(authManifest); + const handlers = new Map Promise>>(); + const ctx: any = { + registerService: vi.fn(), + getService: vi.fn((name: string) => { + if (name === 'manifest') return { register: vi.fn() }; + if (name === 'settings') return svc; + return undefined; + }), + getServices: vi.fn(() => new Map()), + hook: vi.fn((name: string, handler: () => Promise) => { + handlers.set(name, [...(handlers.get(name) ?? []), handler]); + }), + trigger: vi.fn(), + logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }, + getKernel: vi.fn(), + }; + const plugin = new AuthPlugin({ + secret: 'test-secret-at-least-32-chars-long', + baseUrl: 'http://localhost:3000', + ...(opts.plugin ?? {}), + }); + await plugin.init(ctx); + const manager = ctx.registerService.mock.calls.find(([name]: [string]) => name === 'auth')?.[1] as AuthManager; + await plugin.start(ctx); + for (const h of handlers.get('kernel:ready') ?? []) await h(); + + const http = new MockHttp(); + registerSettingsRoutes(http, svc, { contextFromRequest: () => ({ ...ADMIN_CTX }) }); + const save: Rig['save'] = async (body, transport = {}) => { + const out = { status: 200, body: undefined as any }; + const res: IHttpResponse = { + json: vi.fn((data: any) => { out.body = data; }) as any, + send: vi.fn() as any, + status: vi.fn((code: number) => { out.status = code; return res; }) as any, + header: vi.fn(() => res) as any, + }; + const req: IHttpRequest = { + params: { namespace: 'auth' }, + query: {}, + body, + headers: transport.headers ?? {}, + method: 'PUT', + path: '/api/settings/auth', + ...(transport.remoteAddress ? { remoteAddress: transport.remoteAddress } : {}), + }; + await http.routes.get('PUT /api/settings/:namespace')!(req, res); + return out; + }; + return { svc, manager, save }; +} + + +const running = (manager: AuthManager): AuthManagerOptions => (manager as any).config; +const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); +const DAY = 86_400; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('#30312 the restore table covers every key the auth pass applies', () => { + it('names exactly the shipped auth manifest keys', () => { + const manifestKeys = ((authManifest as any).specifiers as Array<{ key?: string }>) + .flatMap((specifier) => (specifier.key ? [specifier.key] : [])) + .sort(); + expect(manifestKeys.length).toBeGreaterThan(30); + expect(Object.keys(AUTH_SETTING_RESTORE).sort()).toEqual(manifestKeys); + }); +}); + +describe('#30312 a cleared plain option returns to the declaration in the same pass', () => { + it('lockout and concurrent sessions: the measured case, with and without a declaration', async () => { + const { svc, manager } = await boot({ plugin: { lockoutThreshold: 7 } }); + expect(running(manager).lockoutThreshold).toBe(7); + await svc.setMany('auth', { lockout_threshold: 3, max_concurrent_sessions_per_user: 2 }, {}); + await vi.waitFor(() => expect(running(manager).maxConcurrentSessions).toBe(2)); + expect(running(manager).lockoutThreshold).toBe(3); + + await svc.setMany('auth', { lockout_threshold: null, max_concurrent_sessions_per_user: null }, {}); + expect((await svc.get('auth', 'lockout_threshold', {})).source).toBe('default'); + await vi.waitFor(() => expect(running(manager).lockoutThreshold).toBe(7)); + expect(running(manager).maxConcurrentSessions).toBeUndefined(); + }); + + it('password policy and API-key lifetime fields', async () => { + const { svc, manager } = await boot({ plugin: { passwordMinClasses: 2, apiKeyMaxExpiryDays: 30 } }); + await svc.setMany('auth', { password_min_classes: 4, api_key_max_expiry_days: 400 }, {}); + await vi.waitFor(() => expect(running(manager).apiKeyMaxExpiryDays).toBe(400)); + expect(running(manager).passwordMinClasses).toBe(4); + + await svc.setMany('auth', { password_min_classes: null, api_key_max_expiry_days: null }, {}); + await vi.waitFor(() => expect(running(manager).apiKeyMaxExpiryDays).toBe(30)); + expect(running(manager).passwordMinClasses).toBe(2); + }); + + it('a key the store never gave a value is left alone by a pass about another key', async () => { + const { svc, manager } = await boot({ plugin: { lockoutThreshold: 7 } }); + const before = running(manager); + await svc.set('auth', 'max_concurrent_sessions_per_user', 2, {}); + await vi.waitFor(() => expect(running(manager).maxConcurrentSessions).toBe(2)); + expect(running(manager).lockoutThreshold).toBe(7); + expect(running(manager).emailAndPassword).toEqual(before.emailAndPassword); + }); +}); + +describe('#30312 refusable pieces restore through the piece a save travels', () => { + it('emailAndPassword fields return to the declared block', async () => { + const { svc, manager } = await boot({ plugin: { emailAndPassword: { enabled: true, minPasswordLength: 12 } } }); + await svc.setMany('auth', { password_min_length: 20, signup_enabled: false }, {}); + await vi.waitFor(() => expect(running(manager).emailAndPassword?.minPasswordLength).toBe(20)); + expect(running(manager).emailAndPassword?.disableSignUp).toBe(true); + + await svc.setMany('auth', { password_min_length: null, signup_enabled: null }, {}); + await vi.waitFor(() => expect(running(manager).emailAndPassword?.minPasswordLength).toBe(12)); + expect(running(manager).emailAndPassword?.disableSignUp).toBeUndefined(); + expect(running(manager).emailAndPassword?.enabled).toBe(true); + }); + + it('breached-password rejection returns to the declared plugin flag', async () => { + const { svc, manager } = await boot(); + await svc.set('auth', 'password_reject_breached', true, {}); + await vi.waitFor(() => expect((running(manager).plugins as any)?.passwordRejectBreached).toBe(true)); + + await svc.set('auth', 'password_reject_breached', null, {}); + await vi.waitFor(() => expect((running(manager).plugins as any)?.passwordRejectBreached).toBeUndefined()); + }); + + it('mfa_required cleared returns to a declared off, and enrolled factors stay verifiable', async () => { + const { svc, manager } = await boot(); + await svc.set('auth', 'mfa_required', true, {}); + await vi.waitFor(() => expect(running(manager).mfaRequired).toBe(true)); + expect((running(manager).plugins as any)?.twoFactor).toBe(true); + + await svc.set('auth', 'mfa_required', null, {}); + await vi.waitFor(() => expect(running(manager).mfaRequired).toBeUndefined()); + // As a save of off: the requirement goes, the enrollment path stays. + expect((running(manager).plugins as any)?.twoFactor).toBe(true); + }); + + it('mfa_required cleared returns to a declared on as one piece with its enrollment path', async () => { + const { svc, manager } = await boot({ plugin: { mfaRequired: true } }); + await svc.set('auth', 'mfa_required', false, {}); + await vi.waitFor(() => expect(running(manager).mfaRequired).toBe(false)); + + await svc.set('auth', 'mfa_required', null, {}); + await vi.waitFor(() => expect(running(manager).mfaRequired).toBe(true)); + expect((running(manager).plugins as any)?.twoFactor).toBe(true); + }); + + it('a cleared email_password_enabled is judged as the off it returns to, and refused with no other way in', async () => { + const { manager, save } = await boot({ + plugin: { + emailAndPassword: { enabled: false }, + socialProviders: { google: { clientId: 'cid', clientSecret: 'secret' } }, + }, + }); + expect((await save({ email_password_enabled: true })).status).toBe(200); + await vi.waitFor(() => expect(running(manager).emailAndPassword?.enabled).toBe(true)); + expect((await save({ google_enabled: false })).status).toBe(200); + await vi.waitFor(() => expect(running(manager).socialProviders?.google).toBeUndefined()); + + const refused = await save({ email_password_enabled: null }); + expect(refused.status).toBe(400); + expect(refused.body.error.code).toBe('SETTINGS_VALIDATION'); + expect(refused.body.error.details.fields).toEqual( + expect.arrayContaining([expect.objectContaining({ field: 'email_password_enabled', code: 'invalid_value' })]), + ); + await settle(); + expect(running(manager).emailAndPassword?.enabled).toBe(true); + }); + + it('the same clear saves and applies when another way in stands (preservation)', async () => { + const { manager, save } = await boot({ + plugin: { + emailAndPassword: { enabled: false }, + socialProviders: { google: { clientId: 'cid', clientSecret: 'secret' } }, + }, + }); + expect((await save({ email_password_enabled: true })).status).toBe(200); + await vi.waitFor(() => expect(running(manager).emailAndPassword?.enabled).toBe(true)); + + expect((await save({ email_password_enabled: null })).status).toBe(200); + await vi.waitFor(() => expect(running(manager).emailAndPassword?.enabled).toBe(false)); + expect(running(manager).socialProviders?.google).toBeDefined(); + }); +}); + +describe('#30312 keys that rule one runtime value together restore together', () => { + it('session lifetime: a stored field overrides its declared twin, a cleared one returns to it', async () => { + const { svc, manager } = await boot({ plugin: { session: { expiresIn: 10 * DAY, updateAge: DAY } } }); + await svc.set('auth', 'session_expiry_days', 3, {}); + await vi.waitFor(() => expect(running(manager).session?.expiresIn).toBe(3 * DAY)); + expect(running(manager).session?.updateAge).toBe(DAY); + + await svc.set('auth', 'session_expiry_days', null, {}); + await vi.waitFor(() => expect(running(manager).session?.expiresIn).toBe(10 * DAY)); + expect(running(manager).session?.updateAge).toBe(DAY); + }); + + it('the rate-limit pair returns to the declaration once neither is stored', async () => { + const { svc, manager } = await boot(); + await svc.setMany('auth', { rate_limit_max: 5, rate_limit_window_seconds: 30 }, {}); + await vi.waitFor(() => expect(running(manager).rateLimit?.max).toBe(5)); + + await svc.set('auth', 'rate_limit_max', null, {}); + await vi.waitFor(() => expect(running(manager).rateLimit?.max).toBe(10)); + expect(running(manager).rateLimit?.window).toBe(30); + + await svc.set('auth', 'rate_limit_window_seconds', null, {}); + await vi.waitFor(() => expect(running(manager).rateLimit).toBeUndefined()); + }); + + it('Google returns to the declared provider once no google key is stored', async () => { + const { svc, manager } = await boot({ + plugin: { socialProviders: { google: { clientId: 'cid', clientSecret: 'secret' } } }, + }); + await svc.set('auth', 'google_enabled', false, {}); + await vi.waitFor(() => expect(running(manager).socialProviders?.google).toBeUndefined()); + + await svc.set('auth', 'google_enabled', null, {}); + await vi.waitFor(() => expect(running(manager).socialProviders?.google?.clientId).toBe('cid')); + }); + + it('the audience returns whole to the declared posture once no audience key is stored', async () => { + const { svc, manager } = await boot(); + await svc.setMany('auth', { + audience_posture: 'email_domain', + audience_allowed_email_domains: 'example.com', + audience_self_registration_permission_set: 'member_default', + }, {}); + await vi.waitFor(() => expect(manager.getAudience().posture).toBe('email_domain')); + + await svc.setMany('auth', { + audience_posture: null, + audience_allowed_email_domains: null, + audience_self_registration_permission_set: null, + }, {}); + await vi.waitFor(() => expect(manager.getAudience().posture).toBe('invite_only')); + }); +}); + +describe('#30312 the audience follows its posture', () => { + it('a cleared posture returns to the declared one while a sibling is still stored', async () => { + const { svc, manager } = await boot(); + await svc.setMany('auth', { + audience_posture: 'email_domain', + audience_allowed_email_domains: 'example.com', + audience_self_registration_permission_set: 'member_default', + }, {}); + await vi.waitFor(() => expect(manager.getAudience().posture).toBe('email_domain')); + + await svc.set('auth', 'audience_posture', null, {}); + await vi.waitFor(() => expect(manager.getAudience().posture).toBe('invite_only')); + }); +}); + +describe('#30312 restoring the namespace defaults restores every family at once', () => { + it('reset hands plain, refusable and grouped keys back to the declaration', async () => { + const { svc, manager } = await boot({ + plugin: { lockoutThreshold: 7, emailAndPassword: { enabled: true, minPasswordLength: 12 } }, + }); + await svc.setMany('auth', { + lockout_threshold: 3, + password_min_length: 20, + session_idle_timeout_minutes: 15, + rate_limit_max: 5, + }, {}); + await vi.waitFor(() => expect(running(manager).rateLimit?.max).toBe(5)); + + await svc.resetNamespace('auth', {}); + await vi.waitFor(() => expect(running(manager).lockoutThreshold).toBe(7)); + expect(running(manager).emailAndPassword?.minPasswordLength).toBe(12); + expect(running(manager).sessionIdleTimeoutMinutes).toBeUndefined(); + expect(running(manager).rateLimit).toBeUndefined(); + }); + + it('an env-pinned key keeps its env value through a reset', async () => { + const { svc, manager } = await boot({ env: { OS_AUTH_LOCKOUT_THRESHOLD: '5' } }); + await vi.waitFor(() => expect(running(manager).lockoutThreshold).toBe(5)); + await svc.set('auth', 'max_concurrent_sessions_per_user', 2, {}); + await vi.waitFor(() => expect(running(manager).maxConcurrentSessions).toBe(2)); + + await svc.resetNamespace('auth', {}); + await vi.waitFor(() => expect(running(manager).maxConcurrentSessions).toBeUndefined()); + expect(running(manager).lockoutThreshold).toBe(5); + }); +}); diff --git a/packages/plugins/plugin-auth/src/auth-settings-declared.ts b/packages/plugins/plugin-auth/src/auth-settings-declared.ts index 9ea08e97e85..83bd2012f0c 100644 --- a/packages/plugins/plugin-auth/src/auth-settings-declared.ts +++ b/packages/plugins/plugin-auth/src/auth-settings-declared.ts @@ -20,8 +20,9 @@ * stored value lands in a block the manager can refuse (`emailAndPassword`, * `plugins`, `audience`) restores through that same block, so the manager * judges a restored value exactly as it judges a saved one. Keys that rule - * one runtime value together (the rate-limit pair, the Google trio, the - * audience trio) restore as a group, and only once none of them is stored. + * one runtime value together restore as a group: the rate-limit pair and + * the Google trio once none of them is stored, the audience trio once its + * posture is not (a sibling stored without a posture is ignored). */ import type { AuthManagerOptions } from './auth-manager.js'; @@ -88,7 +89,7 @@ export type AuthSettingRestore = | { readonly via: 'session'; readonly field: 'expiresIn' | 'updateAge' } /** Enforced MFA: restored exactly as a save of the declared value would apply. */ | { readonly via: 'mfaRequired' } - /** One runtime value ruled by several keys: restored whole once none of them is stored. */ + /** One runtime value ruled by several keys: restored whole (see the module note for when). */ | { readonly via: 'group'; readonly group: 'rateLimit' | 'google' | 'audience' }; /** diff --git a/packages/plugins/plugin-auth/src/sign-in-method-floor.ts b/packages/plugins/plugin-auth/src/sign-in-method-floor.ts index 6b2fcf634a1..5521f21896a 100644 --- a/packages/plugins/plugin-auth/src/sign-in-method-floor.ts +++ b/packages/plugins/plugin-auth/src/sign-in-method-floor.ts @@ -57,7 +57,10 @@ * settings); a key the save sets is read from the batch unless an environment * override is in force for it, which wins over what is saved; a cleared key * reads as its default (on, for both toggles), as the settings service's own - * save-time view does. + * save-time view does — except that a cleared `email_password_enabled` reads + * as the deployment's own declaration, which the settings pass hands the + * running value back to (#30312): a deployment that declared email and + * password off turns it off again with that clear. * * A deployment that stored the dead-end state before this check existed gets * back in with `OS_AUTH_EMAIL_PASSWORD_ENABLED=true`, the settings environment @@ -97,6 +100,12 @@ export interface SignInMethodFloorDeps { */ providerHasCredentials(provider: { id: string; type: 'social' | 'oidc' }): boolean; env(): Record; + /** + * [#30312] Whether the deployment itself declared email and password on (the + * plugin options and `auth:configure`, before any setting): what a cleared + * `email_password_enabled` hands the running value back to. Absent ⇒ on. + */ + declaredEmailPasswordEnabled?(): boolean; } const hasCredentialPair = (entry: unknown): boolean => { @@ -335,7 +344,10 @@ export function signInMethodFloorPredicate(deps: SignInMethodFloorDeps, read: Au const batch = input.batch ?? { [key]: next }; const config = deps.publicConfig(); const emailPasswordAfter = Object.prototype.hasOwnProperty.call(batch, 'email_password_enabled') - ? readToggle(await valueAfter(read, input, batch, 'email_password_enabled'), true) + ? readToggle( + await valueAfter(read, input, batch, 'email_password_enabled'), + deps.declaredEmailPasswordEnabled?.() ?? true, + ) : config.emailPassword?.enabled !== false; if (emailPasswordAfter) return undefined; const googleNow = (config.socialProviders ?? []).some((p) => isSettingsGoogle(p) && providerCounts(deps, p)); From f58be8f5bc3e3e61abc77826c117e3b87b51f66a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 10:35:45 +0000 Subject: [PATCH 3/3] test(plugin-auth): pin that an unstored key is never re-applied; changeset Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Utp6CZPLJKSBMtARXMHc4W --- ...30312-auth-settings-clear-applies-default.md | 17 +++++++++++++++++ .../src/auth-settings-clear.pin.test.ts | 6 ++++-- 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 .changeset/30312-auth-settings-clear-applies-default.md diff --git a/.changeset/30312-auth-settings-clear-applies-default.md b/.changeset/30312-auth-settings-clear-applies-default.md new file mode 100644 index 00000000000..9659915b7b1 --- /dev/null +++ b/.changeset/30312-auth-settings-clear-applies-default.md @@ -0,0 +1,17 @@ +--- +'@objectstack/plugin-auth': patch +--- + +fix(plugin-auth)!: clearing an authentication setting, or restoring the namespace's defaults, takes effect at once (#30312) + +Clause-②: yes (narrowing) — one save an administrator could make before is now refused at the settings save door with the existing `SETTINGS_VALIDATION` / `invalid_value` field error: clearing `auth.email_password_enabled` on a deployment that itself declared email and password off, when nothing else signs anyone in after the save. Before, that clear was saved and changed nothing at runtime; now a clear applies, and that one would lock everyone out. No new error code, settings key or export. +Metadata-protocol: none +Skills-docs: none — no file under `skills/**` teaches the auth settings namespace, its clear or the sign-in floor. + + + +**BREAKING** accept-set narrowing at the auth settings save door (`PUT /api/settings/auth`, `SettingsService.setMany('auth', …)`): one clear that was saved before is refused. + +- **The fix.** A cleared `auth.*` key (the console sends `null`) or a namespace reset resolves `source: 'default'`. Every key except `allowed_ip_ranges` (#30203) then kept its last stored value in force until a restart: lockout threshold, max sessions per user, password policy, session lifetime, MFA, API-key lifetime, rate limits, Google, audience. Now the running value returns in the same pass to what the deployment declared (plugin options, `auth:configure`), or to the built-in default where it declared nothing. A value in a block the auth manager judges (`emailAndPassword`, `plugins`, `audience`) is restored through that same block, so it is judged as a save of it would be. Environment overrides keep ruling. +- **The refused clear.** With email and password declared off by the deployment and turned on in Setup, and no other way in (Google switched off in Setup, no other provider, no SSO, no phone sign-in, no host handoff), clearing "Enable email/password login" now answers `400 SETTINGS_VALIDATION` at `email_password_enabled` (`constraint.lastSignInMethod: email_password`), and nothing is written. Set up another sign-in method first, or save the toggle on explicitly instead of clearing it. +- **Session lifetime.** A stored `session_expiry_days` / `session_refresh_days` now overrides its field of the deployment's declared `session` block instead of replacing the whole block. diff --git a/packages/plugins/plugin-auth/src/auth-settings-clear.pin.test.ts b/packages/plugins/plugin-auth/src/auth-settings-clear.pin.test.ts index b2dd78df591..79652ab0156 100644 --- a/packages/plugins/plugin-auth/src/auth-settings-clear.pin.test.ts +++ b/packages/plugins/plugin-auth/src/auth-settings-clear.pin.test.ts @@ -164,11 +164,13 @@ describe('#30312 a cleared plain option returns to the declaration in the same p it('a key the store never gave a value is left alone by a pass about another key', async () => { const { svc, manager } = await boot({ plugin: { lockoutThreshold: 7 } }); - const before = running(manager); + const applied = vi.spyOn(manager, 'applyConfigPatch'); await svc.set('auth', 'max_concurrent_sessions_per_user', 2, {}); await vi.waitFor(() => expect(running(manager).maxConcurrentSessions).toBe(2)); expect(running(manager).lockoutThreshold).toBe(7); - expect(running(manager).emailAndPassword).toEqual(before.emailAndPassword); + // Only the stored key's option travels: no piece re-applies (and so no + // refusable block re-judges) a declaration nobody cleared. + expect(applied.mock.calls.map(([piece]) => Object.keys(piece))).toEqual([['maxConcurrentSessions']]); }); });