diff --git a/.changeset/auth-membership-policy-setting.md b/.changeset/auth-membership-policy-setting.md new file mode 100644 index 0000000000..2f566bc374 --- /dev/null +++ b/.changeset/auth-membership-policy-setting.md @@ -0,0 +1,49 @@ +--- +"@objectstack/plugin-auth": minor +"@objectstack/service-settings": minor +--- + +feat(auth): `membership_policy` is a platform setting, and sign-up and backfill read one source (#5152) + +**What a new user joins is now configurable at runtime.** ADR-0093's +`membershipPolicy` decides whether a freshly created user is auto-bound to the +deployment's default organization (`auto`) or gets membership only from an +explicit act — creating a workspace, accepting an invitation, an admin adding +them, SSO just-in-time provisioning (`invite-only`). Until now it was settable +**only** as an `AuthPlugin` constructor option, and the AuthPlugin a self-hosted +stack gets is injected by the CLI, which passes no such option and has no env +fallback. Every self-hosted deployment therefore ran `auto`, with no way to say +otherwise. `invite-only` was, in practice, unreachable outside a custom host. + +It is now `auth.membership_policy` in the platform settings — a two-value select +(`auto` / `invite-only`, default `auto`) alongside `signup_enabled`, which it +pairs with: one says whether people may self-register, the other says what they +join when they do. Set it in Setup → Authentication → Membership, or pin it +per-deployment with `OS_AUTH_MEMBERSHIP_POLICY`. It applies **without a +restart** — the existing `settings.subscribe('auth', …)` re-application seam +carries it, the same one the password-policy keys ride. + +**No behaviour changes unless you set it.** Only an *explicit* value applies; +the manifest's `auto` default is a UI default and never masks a deployment that +configured the policy in code. A stack that sets nothing keeps today's +auto-binding exactly. + +**Bug fix — the two membership paths read one source.** Sign-up (the reconciler +in better-auth's `user.create.after`) read the AuthManager's live config, while +the ADR-0093 D6 backfill of pre-existing member-less users read the plugin's +**constructor options**. Wiring a setting to the first and not the second would +have produced "sign-up honours the new policy, backfill still runs the old one" +— and the backfill binds in **bulk**, so it is the more dangerous half. Both now +resolve the policy through the new `AuthManager.getMembershipPolicy()`, and the +backfill waits for the settings namespace to bind before its first pass (the two +`kernel:ready` hooks fire in registration order, which was the wrong order). + +**An invalid value is rejected, not coerced.** `PUT /api/settings/auth` refuses +a policy outside the declared option table (`invalid_option`, naming the allowed +set). A value arriving from `OS_AUTH_MEMBERSHIP_POLICY` — which bypasses that +validation — is logged at `error` and **ignored**, leaving the deployment's +current policy in force; it is never silently read as `auto`, because that would +leave an operator believing a wall is up while every sign-up is auto-bound. + +New public API on `@objectstack/plugin-auth`: `AuthManager.getMembershipPolicy()`, +plus `MEMBERSHIP_POLICIES` and `isMembershipPolicy()` from `reconcile-membership`. diff --git a/content/docs/deployment/environment-variables.mdx b/content/docs/deployment/environment-variables.mdx index dafc8c08a4..ad567d94bd 100644 --- a/content/docs/deployment/environment-variables.mdx +++ b/content/docs/deployment/environment-variables.mdx @@ -78,6 +78,7 @@ read at startup unless noted otherwise. Boolean variables accept `true` / `false | `OS_AUTH_EMAIL_PASSWORD_ENABLED` | boolean | settings default | Settings env override for `auth.email_password_enabled`. Controls local email/password login. | | `OS_AUTH_SIGNUP_ENABLED` | boolean | settings default | Settings env override for `auth.signup_enabled`. Takes precedence over UI settings and is preferred over `OS_DISABLE_SIGNUP`. | | `OS_AUTH_REQUIRE_EMAIL_VERIFICATION` | boolean | settings default | Settings env override for `auth.require_email_verification`. | +| `OS_AUTH_MEMBERSHIP_POLICY` | `auto` \| `invite-only` | `auto` | Settings env override for `auth.membership_policy` — what a newly created user joins (ADR-0093 D1). `auto` binds every new user to the deployment's default organization. `invite-only` grants membership solely through an explicit act: creating a workspace, accepting an invitation, an admin adding them, or SSO just-in-time provisioning. Applies to sign-up **and** to the backfill of pre-existing member-less users. An unrecognized value is rejected with an `error` log and **ignored** — the deployment keeps its current policy rather than silently reverting to `auto`. | | `OS_AUTH_GOOGLE_ENABLED` | boolean | settings default | Settings env override for `auth.google_enabled`. Requires Google OAuth credentials from Settings or env. | | `GOOGLE_CLIENT_ID` | string | — | Deployment-level Google OAuth client id for the open-source Google login implementation. | | `GOOGLE_CLIENT_SECRET` | string | — | Deployment-level Google OAuth client secret for the open-source Google login implementation. | diff --git a/content/docs/permissions/authentication.mdx b/content/docs/permissions/authentication.mdx index ae8d9f2655..87eb877e0c 100644 --- a/content/docs/permissions/authentication.mdx +++ b/content/docs/permissions/authentication.mdx @@ -114,6 +114,13 @@ GOOGLE_CLIENT_SECRET=your-google-client-secret # Optional: lock auth settings from env. Env wins over Setup UI values. OS_AUTH_SIGNUP_ENABLED=false OS_AUTH_GOOGLE_ENABLED=true + +# Optional: what a new user joins (ADR-0093 D1). `auto` (default) binds every +# new user to the default organization; `invite-only` grants membership only +# through an explicit act — creating a workspace, accepting an invitation, an +# admin adding them, or SSO just-in-time provisioning. +# Also configurable in Setup → Authentication → Membership. +OS_AUTH_MEMBERSHIP_POLICY=invite-only ``` > **Important**: Never commit `OS_AUTH_SECRET` to version control. Use a strong random string (minimum 32 characters). diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 5b0991b00d..63ad83b07c 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -2661,6 +2661,26 @@ export class AuthManager { } } + /** + * ADR-0093 D1 — the deployment's membership policy **as it stands right now**. + * + * The ONE source both membership paths read (#5152): + * - sign-up: the reconciler composed into `user.create.after` (below); + * - backfill: `AuthPlugin`'s ADR-0093 D6 pass over pre-existing member-less + * users, which used to read the plugin's CONSTRUCTOR options instead. + * + * That split mattered because `this.config` is what {@link applyConfigPatch} + * targets: once `auth.membership_policy` became a platform setting, the + * constructor options stopped being current the moment an admin saved the + * form. Sign-up would honour the new policy while the backfill kept running + * the old one — and the backfill binds in BULK. Read the policy through here, + * never off a captured option, so a settings change reaches both without a + * restart. + */ + getMembershipPolicy(): MembershipPolicy { + return this.config.membershipPolicy ?? 'auto'; + } + /** * Inject (or replace) the outbound email service used by better-auth * callbacks. Safe to call after construction but BEFORE the first @@ -3647,7 +3667,9 @@ export class AuthManager { const membershipReconciler = async (user: any) => { try { await reconcileMembership(this.config.dataEngine, user?.id, { - policy: this.config.membershipPolicy ?? 'auto', + // #5152 — read through the accessor, not `this.config` directly: it is + // the single source the backfill path reads too. + policy: this.getMembershipPolicy(), resolveTargetOrg: async () => { const tenancy = this.config.getTenancy?.(); // Single-org → default org; multi-org → none (invite/JIT own it). diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 43e4764254..842f73a106 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -30,7 +30,12 @@ import { ensureDefaultOrganization } from './ensure-default-organization.js'; import { runAttributedToUser } from './auth-actor-attribution.js'; import type { ResolvedSocialProvider } from './backfill-account-issuer.js'; import { createTenancyService, type TenancyService } from './tenancy-service.js'; -import { backfillMemberships, type MembershipPolicy } from './reconcile-membership.js'; +import { + backfillMemberships, + isMembershipPolicy, + MEMBERSHIP_POLICIES, + type MembershipPolicy, +} from './reconcile-membership.js'; import { registerIdentityWriteGuard, registerManagedUpdateWhitelist, @@ -234,6 +239,11 @@ export class AuthPlugin implements Plugin { // session-snapshot refresh reads through this; undefined = refresh no-ops. private effectiveSecondaryStorage: AuthManagerOptions['secondaryStorage']; + /** + * Memoized `bindAuthSettings()` run — see {@link ensureAuthSettingsBound}. + */ + private authSettingsBinding: Promise | null = null; + constructor(options: AuthPluginOptions = {}) { this.options = { registerRoutes: true, @@ -589,7 +599,7 @@ export class AuthPlugin implements Plugin { // / sendMagicLink) can actually deliver mail. Resolved here on // kernel:ready so EmailServicePlugin has had a chance to register. if (this.authManager) { - await this.bindAuthSettings(ctx); + await this.ensureAuthSettingsBound(ctx); let emailSvc: IEmailService | undefined; try { emailSvc = ctx.getService('email'); } catch { emailSvc = undefined; } @@ -855,11 +865,27 @@ export class AuthPlugin implements Plugin { const runBackfill = (source: string): Promise => { backfillChain = backfillChain.then(async () => { try { + // #5152 — the policy this pass runs under is a SETTING, so bind the + // namespace before reading it. This hook is registered in `init()` + // and therefore fires ahead of the one in `start()` that normally + // binds; without this the first pass of a fresh boot would run the + // pre-settings policy. Idempotent and shared with that hook. + await this.ensureAuthSettingsBound(ctx); const ql = ctx.getService('objectql'); const tenancy = this.tenancy; - if (!ql || !tenancy) return; + // #5152 — the policy is read off the AuthManager, the same object + // the sign-up reconciler reads and the same one `applyConfigPatch` + // targets. It used to be `this.options.membershipPolicy`, a + // constructor option that no settings change can reach: an admin + // switching to `invite-only` stopped sign-up auto-binds while this + // pass kept bulk-binding every member-less user. No `??` fallback + // to the options here on purpose — a second reading of the policy + // is exactly the defect. The manager exists from `init()`, so the + // guard is a precondition, not a degraded mode. + const manager = this.authManager; + if (!ql || !tenancy || !manager) return; const res = await backfillMemberships(ql, { - policy: this.options.membershipPolicy ?? 'auto', + policy: manager.getMembershipPolicy(), resolveTargetOrg: () => tenancy.defaultOrgId(), logger: ctx.logger, }); @@ -987,12 +1013,35 @@ export class AuthPlugin implements Plugin { ctx.logger.info('Auth Plugin started successfully'); } + /** + * Bind the auth settings namespace once, whoever asks first. + * + * Two `kernel:ready` hooks need the settings applied, and they fire in + * REGISTRATION order, which is the opposite of the order they need + * (#5152): the ADR-0093 D6 membership backfill is registered in `init()`, + * the settings binding in `start()`. Left alone, the very first backfill of + * a fresh boot would read the pre-settings policy and bulk-bind every + * pre-existing member-less user on a deployment whose stored setting says + * `invite-only` — the exact failure the setting exists to prevent, on the + * one pass nobody gets to observe before it has happened. + * + * So neither hook owns the binding: both await this, the first one through + * performs it, and the memoized promise keeps `settings.subscribe` from + * being registered twice. + */ + private ensureAuthSettingsBound(ctx: PluginContext): Promise { + this.authSettingsBinding ??= this.bindAuthSettings(ctx); + return this.authSettingsBinding; + } + /** * Bind the small open-source auth settings namespace to better-auth config. * * Only explicit settings values (stored or OS_AUTH_* env overrides) affect * runtime config. Manifest defaults are UI defaults and do not mask code or * deployment configuration. + * + * Call through {@link ensureAuthSettingsBound}, never directly. */ private async bindAuthSettings(ctx: PluginContext): Promise { if (!this.authManager) return; @@ -1047,6 +1096,33 @@ export class AuthPlugin implements Plugin { false, ); } + + // ADR-0093 D1 / #5152 — membership policy. `signup_enabled` says whether + // people may self-register; this says what they join when they do, and + // the two are halves of one platform posture, so it rides the same + // settings seam. Only an EXPLICIT value applies: the manifest default + // (`auto`) is a UI default and must not mask a deployment that set the + // policy at construction. + // + // An unrecognised value is REJECTED, never coerced to `auto`. The + // settings service enforces the option table on `setMany`, but an + // `OS_AUTH_MEMBERSHIP_POLICY` env value bypasses that path entirely, and + // silently reading a typo'd `invite_only` as `auto` would leave an + // operator believing the wall is up while every sign-up is auto-bound — + // invisible until someone finds a stranger in their org. `error`, not + // `warn`: nothing looks broken afterwards. + if (isExplicit('membership_policy')) { + const raw = values.membership_policy; + if (isMembershipPolicy(raw)) { + patch.membershipPolicy = raw; + } else { + ctx.logger.error( + `[auth] membership_policy '${String(raw)}' is not a valid policy — IGNORED, the deployment keeps its current policy ` + + `('${this.authManager.getMembershipPolicy()}') and new sign-ups will continue to follow it. ` + + `Set auth.membership_policy (or OS_AUTH_MEMBERSHIP_POLICY) to one of: ${MEMBERSHIP_POLICIES.join(', ')}.`, + ); + } + } // Password policy — better-auth enforces these bounds on sign-up and // password reset. Ignore malformed/non-positive values (keep the default). if (isExplicit('password_min_length')) { diff --git a/packages/plugins/plugin-auth/src/membership-policy-setting.test.ts b/packages/plugins/plugin-auth/src/membership-policy-setting.test.ts new file mode 100644 index 0000000000..4cc6effacf --- /dev/null +++ b/packages/plugins/plugin-auth/src/membership-policy-setting.test.ts @@ -0,0 +1,323 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `auth.membership_policy` — the platform setting, and the ONE source both + * membership paths read (#5152, ADR-0093 D1/D2/D6). + * + * Two defects are pinned here, and they are only worth fixing together: + * + * 1. **No runtime configuration entry.** `membershipPolicy` was settable only + * as an `AuthPlugin` constructor option, and the CLI-injected AuthPlugin + * (`cli/src/commands/serve.ts`) does not pass it — so a self-hosted stack + * could never express `invite-only`, whatever cloud#1012 decided. It now + * rides the `bindAuthSettings()` → `applyConfigPatch()` seam that + * `signup_enabled` and the password-policy keys already ride. + * + * 2. **The two read sites disagreed.** Sign-up read `this.config` (patchable); + * the ADR-0093 D6 backfill read `this.options` (a constructor option no + * settings change can reach). Wiring the setting without unifying them + * yields "sign-up honours the new policy, backfill still runs the old one" + * — and the backfill binds in BULK. + * + * `reconcileMembership` / `backfillMemberships` are spied through to the REAL + * implementations, so the assertions below name the actual outcome + * (`policy-skip` vs `no-target-org`) rather than inferring it from an absence. + * That distinction is the whole point of the acceptance criteria: a walled + * deployment already produces "user has no membership" as a side effect of + * `defaultOrgId()` returning null, so asserting only the absence proves + * nothing about the policy. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import type { PluginContext } from '@objectstack/core'; +import { AuthPlugin } from './auth-plugin.js'; +import { AuthManager } from './auth-manager.js'; + +vi.mock('./reconcile-membership.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + reconcileMembership: vi.fn(actual.reconcileMembership), + backfillMemberships: vi.fn(actual.backfillMemberships), + }; +}); + +import { reconcileMembership, backfillMemberships, MEMBERSHIP_POLICIES } from './reconcile-membership.js'; + +const reconcileSpy = vi.mocked(reconcileMembership); +const backfillSpy = vi.mocked(backfillMemberships); + +const SECRET = 'test-secret-at-least-32-chars-long'; + +type SettingEntry = { value: unknown; source: string }; + +/** In-memory `sys_user` / `sys_member` store with the find/insert surface used. */ +function makeEngine(seed: { users?: Array<{ id: string }>; members?: Array<{ user_id: string }> } = {}) { + const users = [...(seed.users ?? [])]; + const members: Array<{ organization_id: string; user_id: string }> = [ + ...((seed.members ?? []) as any[]), + ]; + return { + _members: members, + insert: vi.fn(async (_object: string, row: any) => { + members.push({ organization_id: row.organization_id, user_id: row.user_id }); + return row; + }), + find: vi.fn(async (object: string, query: any) => { + const where = query?.where ?? {}; + if (object === 'sys_user') return users; + if (object === 'sys_member') { + return members.filter( + (m) => + (where.user_id === undefined || m.user_id === where.user_id) && + (where.organization_id === undefined || m.organization_id === where.organization_id), + ); + } + return []; + }), + findOne: vi.fn(async () => null), + }; +} + +describe('auth.membership_policy — the setting', () => { + let mockContext: PluginContext; + let hookHandlers: Map Promise>>; + const previousSkipBackfill = process.env.OS_SKIP_MEMBERSHIP_BACKFILL; + const previousEnvPolicy = process.env.OS_AUTH_MEMBERSHIP_POLICY; + + const settingsStore: { values: Record } = { values: {} }; + let subscribers: Array<() => void>; + + const makeSettings = () => ({ + getNamespace: vi.fn(async (namespace: string) => + namespace === 'auth' ? { values: settingsStore.values } : { values: {} }, + ), + subscribe: vi.fn((namespace: string, cb: () => void) => { + if (namespace === 'auth') subscribers.push(cb); + }), + }); + + beforeEach(() => { + reconcileSpy.mockClear(); + backfillSpy.mockClear(); + settingsStore.values = {}; + subscribers = []; + delete process.env.OS_SKIP_MEMBERSHIP_BACKFILL; + delete process.env.OS_AUTH_MEMBERSHIP_POLICY; + hookHandlers = new Map(); + mockContext = { + registerService: vi.fn(), + getService: vi.fn((name: string) => { + if (name === 'manifest') return { register: vi.fn() }; + if (name === 'settings') return makeSettings(); + return undefined; + }), + getServices: vi.fn(() => new Map()), + hook: vi.fn((name: string, handler: () => Promise) => { + if (!hookHandlers.has(name)) hookHandlers.set(name, []); + hookHandlers.get(name)!.push(handler); + }), + trigger: vi.fn(), + logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }, + getKernel: vi.fn(), + } as unknown as PluginContext; + }); + + afterEach(() => { + if (previousSkipBackfill === undefined) delete process.env.OS_SKIP_MEMBERSHIP_BACKFILL; + else process.env.OS_SKIP_MEMBERSHIP_BACKFILL = previousSkipBackfill; + if (previousEnvPolicy === undefined) delete process.env.OS_AUTH_MEMBERSHIP_POLICY; + else process.env.OS_AUTH_MEMBERSHIP_POLICY = previousEnvPolicy; + }); + + const fire = async (name: string) => { + for (const h of hookHandlers.get(name) ?? []) await h(); + }; + + /** + * Boot the plugin exactly as a host does (init → start → kernel:ready), with + * `engine` behind both the `data` and `objectql` services and a tenancy stub + * that DOES resolve a default org — so `no-target-org` is off the table and + * any skip that happens can only be the policy. + * + * `kernel:ready` is fired because that is when the settings namespace binds + * AND when the ADR-0093 D6 backfill runs. The two hooks fire in registration + * order — backfill (`init`) first, settings (`start`) second — so this + * harness is also what proves the backfill does not outrun its own policy. + */ + const boot = async (opts: { + settings?: Record; + pluginOptions?: Record; + engine?: ReturnType; + defaultOrgId?: string | null; + } = {}) => { + settingsStore.values = opts.settings ?? {}; + const engine = opts.engine ?? makeEngine(); + const defaultOrgId = vi.fn(async () => opts.defaultOrgId ?? 'org_default'); + (mockContext.getService as any).mockImplementation((name: string) => { + if (name === 'manifest') return { register: vi.fn() }; + if (name === 'settings') return makeSettings(); + if (name === 'data' || name === 'objectql') return engine; + return undefined; + }); + + const plugin = new AuthPlugin({ secret: SECRET, baseUrl: 'http://localhost:3000', ...(opts.pluginOptions ?? {}) }); + await plugin.init(mockContext); + // Substitute the tenancy stub for the real service: `defaultOrgId` is the + // observable that separates `policy-skip` from `no-target-org`. + (plugin as any).tenancy = { defaultOrgId }; + const manager = (mockContext.registerService as any).mock.calls.find( + ([name]: [string]) => name === 'auth', + )?.[1] as AuthManager; + (manager as any).config.getTenancy = () => ({ defaultOrgId }); + await plugin.start(mockContext); + await fire('kernel:ready'); + return { plugin, manager, engine, defaultOrgId }; + }; + + /** The composed better-auth `user.create.after` hook — the sign-up seam. */ + const signUpHook = (manager: AuthManager) => + (manager as any).composeDatabaseHooks(undefined).user.create.after as (u: any) => Promise; + + // ── 1. The setting reaches the runtime ──────────────────────────────────── + + it('an explicit invite-only setting patches the live policy', async () => { + const { manager } = await boot({ + settings: { membership_policy: { value: 'invite-only', source: 'global' } }, + }); + expect(manager.getMembershipPolicy()).toBe('invite-only'); + }); + + it('leaves the default alone when the setting is not explicitly set (no breaking change)', async () => { + // The manifest ships `default: 'auto'`; the settings service reports it at + // source `default`. A UI default must never be mistaken for an operator's + // intent — same rule every sibling key in `bindAuthSettings` follows. + const { manager } = await boot({ + settings: { membership_policy: { value: 'auto', source: 'default' } }, + }); + expect(manager.getMembershipPolicy()).toBe('auto'); + expect((manager as any).config.membershipPolicy).toBeUndefined(); + }); + + it('a manifest default does not mask a deployment that set the policy in code', async () => { + const { manager } = await boot({ + pluginOptions: { membershipPolicy: 'invite-only' }, + settings: { membership_policy: { value: 'auto', source: 'default' } }, + }); + expect(manager.getMembershipPolicy()).toBe('invite-only'); + }); + + it('re-applies on a settings change without a restart', async () => { + const { manager } = await boot({ + settings: { membership_policy: { value: 'auto', source: 'global' } }, + }); + expect(manager.getMembershipPolicy()).toBe('auto'); + + settingsStore.values = { membership_policy: { value: 'invite-only', source: 'global' } }; + expect(subscribers.length).toBeGreaterThan(0); + for (const cb of subscribers) cb(); + await vi.waitFor(() => expect(manager.getMembershipPolicy()).toBe('invite-only')); + }); + + // ── 2. An invalid value is rejected, never coerced ──────────────────────── + + it('rejects an unrecognised value instead of silently falling back to auto', async () => { + // `OS_AUTH_MEMBERSHIP_POLICY` bypasses the settings service's option-table + // validation entirely, so a typo arrives here uncaught. Reading it as + // `auto` would leave an operator believing the wall is up while every + // sign-up is auto-bound. + const { manager } = await boot({ + pluginOptions: { membershipPolicy: 'invite-only' }, + settings: { membership_policy: { value: 'invite_only', source: 'env' } }, + }); + expect(manager.getMembershipPolicy()).toBe('invite-only'); // NOT coerced to 'auto' + const logged = (mockContext.logger.error as any).mock.calls.map((c: any[]) => String(c[0])); + expect(logged.some((m: string) => m.includes('invite_only'))).toBe(true); + expect(logged.some((m: string) => m.includes(MEMBERSHIP_POLICIES.join(', ')))).toBe(true); + }); + + // ── 3. Sign-up: the reason is `policy-skip`, not `no-target-org` ────────── + + it('invite-only makes a new sign-up take the policy-skip path (not no-target-org)', async () => { + const { manager, engine, defaultOrgId } = await boot({ + settings: { membership_policy: { value: 'invite-only', source: 'global' } }, + }); + + await signUpHook(manager)({ id: 'usr_new' }); + + expect(reconcileSpy).toHaveBeenCalledTimes(1); + expect(reconcileSpy.mock.calls[0]![2]!.policy).toBe('invite-only'); + // The literal outcome, not an inference from "no membership exists". + await expect(reconcileSpy.mock.results[0]!.value).resolves.toEqual({ outcome: 'policy-skip' }); + // And the corroborating evidence: `no-target-org` can only be reached by + // ASKING for the target org. Under the policy skip it is never consulted. + expect(defaultOrgId).not.toHaveBeenCalled(); + expect(engine.insert).not.toHaveBeenCalled(); + }); + + it('the unset default still auto-binds a new sign-up (unchanged behaviour)', async () => { + const { manager, engine, defaultOrgId } = await boot(); + + await signUpHook(manager)({ id: 'usr_new' }); + + await expect(reconcileSpy.mock.results[0]!.value).resolves.toEqual({ + outcome: 'bound', + organizationId: 'org_default', + }); + expect(defaultOrgId).toHaveBeenCalled(); + expect(engine.insert).toHaveBeenCalledWith( + 'sys_member', + expect.objectContaining({ organization_id: 'org_default', user_id: 'usr_new' }), + expect.anything(), + ); + }); + + // ── 4. Sign-up and backfill read ONE source ────────────────────────────── + + it('the backfill honours a settings-set policy the constructor never saw', async () => { + // THE regression pin. On `main` the backfill read + // `this.options.membershipPolicy` — a constructor option — so this exact + // deployment (code says `auto`, admin says `invite-only`) bulk-bound every + // pre-existing member-less user while sign-up correctly refused to. + const engine = makeEngine({ users: [{ id: 'usr_legacy_1' }, { id: 'usr_legacy_2' }] }); + const { defaultOrgId } = await boot({ + pluginOptions: { membershipPolicy: 'auto' }, + settings: { membership_policy: { value: 'invite-only', source: 'global' } }, + engine, + }); + + expect(backfillSpy).toHaveBeenCalled(); + expect(backfillSpy.mock.calls[0]![1]!.policy).toBe('invite-only'); + await expect(backfillSpy.mock.results[0]!.value).resolves.toMatchObject({ + bound: 0, + reason: 'policy', + }); + expect(defaultOrgId).not.toHaveBeenCalled(); + expect(engine.insert).not.toHaveBeenCalled(); + }); + + it('the backfill still binds pre-existing member-less users under the unset default', async () => { + const engine = makeEngine({ users: [{ id: 'usr_legacy_1' }] }); + await boot({ engine }); + + await expect(backfillSpy.mock.results[0]!.value).resolves.toMatchObject({ bound: 1 }); + expect(engine._members).toEqual([{ organization_id: 'org_default', user_id: 'usr_legacy_1' }]); + }); + + it('both paths resolve the policy through AuthManager.getMembershipPolicy()', async () => { + // The structural half of the pin: a behaviour test can be satisfied by two + // read sites that happen to agree today. This one fails the moment either + // path goes back to reading a captured copy, whatever value it holds. + const engine = makeEngine({ users: [{ id: 'usr_legacy_1' }] }); + const { manager } = await boot({ engine }); + const spy = vi.spyOn(manager, 'getMembershipPolicy'); + + await signUpHook(manager)({ id: 'usr_new' }); + expect(spy, 'sign-up must read the policy off the AuthManager').toHaveBeenCalled(); + + spy.mockClear(); + // `app:seeded` re-runs the same (idempotent) backfill — #2996. + await fire('app:seeded'); + expect(spy, 'the ADR-0093 D6 backfill must read the SAME source').toHaveBeenCalled(); + }); +}); diff --git a/packages/plugins/plugin-auth/src/reconcile-membership.ts b/packages/plugins/plugin-auth/src/reconcile-membership.ts index 12ee0bdce2..b452843914 100644 --- a/packages/plugins/plugin-auth/src/reconcile-membership.ts +++ b/packages/plugins/plugin-auth/src/reconcile-membership.ts @@ -28,7 +28,25 @@ import { authSystemWriteContext } from './auth-actor-attribution.js'; -export type MembershipPolicy = 'auto' | 'invite-only'; +/** + * The closed vocabulary of deployment membership policies (ADR-0093 D1). + * + * Exported as a runtime value, not just a type, because the policy also + * arrives from the `auth.membership_policy` platform setting — an untyped + * boundary (a stored row, or an `OS_AUTH_MEMBERSHIP_POLICY` env value that + * never passes through the settings service's option-table validation). The + * binding in `auth-plugin.ts` checks against THIS list and rejects anything + * else loudly rather than coercing it, so only these two values can ever + * reach a reconciler. + */ +export const MEMBERSHIP_POLICIES = ['auto', 'invite-only'] as const; + +export type MembershipPolicy = (typeof MEMBERSHIP_POLICIES)[number]; + +/** Type guard over {@link MEMBERSHIP_POLICIES}. */ +export function isMembershipPolicy(value: unknown): value is MembershipPolicy { + return (MEMBERSHIP_POLICIES as readonly string[]).includes(value as string); +} export type ReconcileOutcome = /** Inserted a `sys_member` row binding the user to the target org. */ diff --git a/packages/services/service-settings/src/manifests/auth.manifest.test.ts b/packages/services/service-settings/src/manifests/auth.manifest.test.ts index 32d0f70ab9..060b3416c5 100644 --- a/packages/services/service-settings/src/manifests/auth.manifest.test.ts +++ b/packages/services/service-settings/src/manifests/auth.manifest.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'; import { SettingsManifestSchema } from '@objectstack/spec/system'; +import { SettingsService } from '../settings-service.js'; import { authSettingsManifest } from './auth.manifest.js'; describe('authSettingsManifest', () => { @@ -33,6 +34,49 @@ describe('authSettingsManifest', () => { ]); }); + // #5152 / ADR-0093 D1 — `signup_enabled` says whether people may + // self-register; this says what they join when they do. Without it a + // self-hosted stack could not express `invite-only` at all: the policy was + // an `AuthPlugin` constructor option and the CLI-injected plugin never + // passed one. + it('exposes membership_policy as a closed two-value select (#5152)', () => { + const specs = authSettingsManifest.specifiers as any[]; + const policy = specs.find((s) => s.key === 'membership_policy'); + + expect(policy.type).toBe('select'); + expect(policy.default).toBe('auto'); + // The option table is the enforcement surface, not a front-end + // convention: `SettingsService.setMany` rejects a value outside it + // (`invalid_option`), which is what keeps a script or AI-authored + // bootstrap from writing a policy the reconciler has no branch for. + expect(policy.options.map((o: any) => o.value)).toEqual(['auto', 'invite-only']); + + // Membership is decided on EVERY creation path — SSO just-in-time + // provisioning, admin create-user and bulk import included — so it must + // not be hidden behind the email/password provider the way the password + // keys are. An SSO-only deployment is precisely the one that needs it. + expect(policy.visible).toBeUndefined(); + expect(specs.filter((s) => s.type === 'group').map((s) => s.id)).toContain('membership'); + }); + + it('refuses a membership policy outside the option table at the write API (#5152)', async () => { + const svc = new SettingsService({ env: {} }); + svc.registerManifest(authSettingsManifest as any); + + await expect(svc.setMany('auth', { membership_policy: 'invite_only' })).rejects.toMatchObject({ + code: 'SETTINGS_VALIDATION', + fields: [ + { + field: 'membership_policy', + code: 'invalid_option', + constraint: { allowed: 'auto, invite-only' }, + }, + ], + }); + await expect(svc.setMany('auth', { membership_policy: 'invite-only' })).resolves.toBeDefined(); + expect((await svc.get('auth', 'membership_policy')).value).toBe('invite-only'); + }); + it('exposes password-policy + session number fields with bounds and defaults', () => { const specs = authSettingsManifest.specifiers as any[]; const byKey = (k: string) => specs.find((s) => s.key === k); diff --git a/packages/services/service-settings/src/manifests/auth.manifest.ts b/packages/services/service-settings/src/manifests/auth.manifest.ts index ccef8b9bb9..c90b9939ab 100644 --- a/packages/services/service-settings/src/manifests/auth.manifest.ts +++ b/packages/services/service-settings/src/manifests/auth.manifest.ts @@ -47,6 +47,32 @@ const manifest = { visible: "${data.email_password_enabled !== false}", }, + // ADR-0093 D1 — deliberately its OWN group, not a member of + // `email_password`: membership is decided for EVERY creation path (email + // sign-up, SSO just-in-time provisioning, admin create-user, bulk import), + // so hiding it behind `email_password_enabled` would leave an SSO-only + // deployment unable to configure the one posture it most needs. + { + type: 'group', + id: 'membership', + label: 'Membership', + required: false, + description: 'What a newly created user joins. Pairs with self-service registration above.', + }, + { + type: 'select', + key: 'membership_policy', + label: 'New user membership', + required: false, + default: 'auto', + options: [ + { value: 'auto', label: 'Join the default organization automatically' }, + { value: 'invite-only', label: 'Invitation only — never join automatically' }, + ], + description: + 'Automatic binds every new user to this deployment\'s default organization. Invitation only grants membership solely through an explicit act — creating a workspace, accepting an invitation, being added by an admin, or SSO just-in-time provisioning. Applies to the backfill of pre-existing member-less users too.', + }, + { type: 'group', id: 'password_policy', diff --git a/packages/services/service-settings/src/translations/en.ts b/packages/services/service-settings/src/translations/en.ts index d363f8717d..5c431f0fc1 100644 --- a/packages/services/service-settings/src/translations/en.ts +++ b/packages/services/service-settings/src/translations/en.ts @@ -135,6 +135,10 @@ export const en: TranslationData = { title: 'Email and password', description: 'Control local email/password sign-in and self-service registration.', }, + membership: { + title: 'Membership', + description: 'What a newly created user joins. Pairs with self-service registration above.', + }, password_policy: { title: 'Password policy', description: 'Length bounds enforced by the auth provider on sign-up and password reset.', @@ -153,6 +157,14 @@ export const en: TranslationData = { email_password_enabled: { label: 'Enable email/password login' }, signup_enabled: { label: 'Allow self-service registration' }, require_email_verification: { label: 'Require email verification' }, + membership_policy: { + label: 'New user membership', + help: 'Automatic binds every new user to the default organization. Invitation only grants membership solely through an explicit act — creating a workspace, accepting an invitation, an admin adding them, or SSO just-in-time provisioning.', + options: { + auto: 'Join the default organization automatically', + 'invite-only': 'Invitation only — never join automatically', + }, + }, password_min_length: { label: 'Minimum password length' }, password_max_length: { label: 'Maximum password length', help: 'Guards against denial-of-service via very long password hashing.' }, session_expiry_days: { label: 'Session lifetime (days)', help: 'A session expires this many days after sign-in.' }, diff --git a/packages/services/service-settings/src/translations/es-ES.ts b/packages/services/service-settings/src/translations/es-ES.ts index 7b3e56197d..5e46908ad2 100644 --- a/packages/services/service-settings/src/translations/es-ES.ts +++ b/packages/services/service-settings/src/translations/es-ES.ts @@ -85,6 +85,10 @@ export const esES: TranslationData = { title: 'Correo y contraseña', description: 'Controla el inicio de sesión local con correo/contraseña y el registro de autoservicio.', }, + membership: { + title: 'Pertenencia', + description: 'A qué se une un usuario recién creado. Complementa el registro de autoservicio anterior.', + }, password_policy: { title: 'Política de contraseñas', description: 'Límites de longitud que el proveedor de autenticación exige en el registro y el restablecimiento de contraseña.', @@ -115,6 +119,14 @@ export const esES: TranslationData = { email_password_enabled: { label: 'Habilitar inicio de sesión con correo/contraseña' }, signup_enabled: { label: 'Permitir registro de autoservicio' }, require_email_verification: { label: 'Requerir verificación de correo' }, + membership_policy: { + label: 'Pertenencia de los usuarios nuevos', + help: 'La opción automática vincula cada usuario nuevo a la organización predeterminada del despliegue. «Solo por invitación» concede la pertenencia únicamente mediante un acto explícito: crear un espacio de trabajo, aceptar una invitación, que un administrador lo añada o el aprovisionamiento just-in-time por SSO.', + options: { + auto: 'Unirse automáticamente a la organización predeterminada', + 'invite-only': 'Solo por invitación: nunca se une automáticamente', + }, + }, password_min_length: { label: 'Longitud mínima de contraseña' }, password_max_length: { label: 'Longitud máxima de contraseña', help: 'Un límite superior protege frente a la denegación de servicio por el hasheo de contraseñas muy largas.' }, password_reject_breached: { diff --git a/packages/services/service-settings/src/translations/ja-JP.ts b/packages/services/service-settings/src/translations/ja-JP.ts index 702c616170..e61a0f3d85 100644 --- a/packages/services/service-settings/src/translations/ja-JP.ts +++ b/packages/services/service-settings/src/translations/ja-JP.ts @@ -85,6 +85,10 @@ export const jaJP: TranslationData = { title: 'メールとパスワード', description: 'ローカルのメール/パスワードサインインとセルフサービス登録を制御します。', }, + membership: { + title: 'メンバーシップ', + description: '新しく作成されたユーザーが所属する先。上のセルフサービス登録と対になる設定です。', + }, password_policy: { title: 'パスワードポリシー', description: 'サインアップおよびパスワードリセット時に認証プロバイダーが強制する長さ制限。', @@ -115,6 +119,14 @@ export const jaJP: TranslationData = { email_password_enabled: { label: 'メール/パスワードログインを有効化' }, signup_enabled: { label: 'セルフサービス登録を許可' }, require_email_verification: { label: 'メール確認を必須にする' }, + membership_policy: { + label: '新規ユーザーのメンバーシップ', + help: '「自動」はすべての新規ユーザーをこのデプロイの既定組織に紐付けます。「招待のみ」では、ワークスペースの作成、招待の承諾、管理者による追加、SSO のジャストインタイムプロビジョニングといった明示的な行為によってのみメンバーシップが付与されます。', + options: { + auto: '既定の組織に自動的に参加', + 'invite-only': '招待のみ — 自動では参加しない', + }, + }, password_min_length: { label: 'パスワードの最小文字数' }, password_max_length: { label: 'パスワードの最大文字数', help: '非常に長いパスワードのハッシュ化によるサービス拒否を防ぎます。' }, password_reject_breached: { diff --git a/packages/services/service-settings/src/translations/zh-CN.ts b/packages/services/service-settings/src/translations/zh-CN.ts index 8ebea4d00a..f28fbc8368 100644 --- a/packages/services/service-settings/src/translations/zh-CN.ts +++ b/packages/services/service-settings/src/translations/zh-CN.ts @@ -246,6 +246,10 @@ export const zhCN: TranslationData = { title: '邮箱与密码', description: '控制本地邮箱/密码登录与自助注册。', }, + membership: { + title: '成员归属', + description: '新建用户加入什么。与上方的自助注册成对配置。', + }, password_policy: { title: '密码策略', description: '由认证提供商在注册和重置密码时强制的长度限制。', @@ -275,6 +279,14 @@ export const zhCN: TranslationData = { email_password_enabled: { label: '启用邮箱/密码登录' }, signup_enabled: { label: '允许自助注册' }, require_email_verification: { label: '要求邮箱验证' }, + membership_policy: { + label: '新用户的成员归属', + help: '「自动加入」会把每个新用户绑定到本部署的默认组织;「仅限邀请」只在用户显式行动后才授予成员身份——自行创建工作区、接受邀请、被管理员添加,或由 SSO 即时开通。', + options: { + auto: '自动加入默认组织', + 'invite-only': '仅限邀请——绝不自动加入', + }, + }, password_min_length: { label: '密码最小长度' }, password_max_length: { label: '密码最大长度', help: '防止超长密码哈希导致的拒绝服务。' }, password_reject_breached: {