From 96242ef2d5f79b2a5b337b5cb2d4f67cba82d2b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 04:16:33 +0000 Subject: [PATCH 1/2] =?UTF-8?q?feat(auth):=20AuthPlugin=20derives=20app=20?= =?UTF-8?q?org=20roles=20itself=20=E2=80=94=20hosts=20pass=20nothing=20(#3?= =?UTF-8?q?723,=20cloud#897)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per-host `additionalOrgRoles` wiring was the defect pattern: five hosts boot AuthPlugin from a stack, three of them (verify harness, DevPlugin, cloud's ArtifactKernelFactory) at some point forgot the parameter, and the failure is silent — app-declared roles are simply absent. cloud is also order-hostile: it mounts AuthPlugin before the app metadata exists, so no init-time walk could ever cover it. AuthPlugin now derives the roles in its own kernel:ready hook — the one point that fires after all metadata is registered in every host — via `collectRegisteredOrgRoles` (the late-bound twin of `collectStackOrgRoles`; same dual read as `bootstrapDeclaredPositions`: engine registry first, metadata-service facade as fallback). Both consumers update from the derived union: - better-auth side: `applyConfigPatch` merges the roles; the instance is built lazily (or reset by the patch), so the org-plugin roles map carries the full set on next use. - select side: `sys_invitation` / `sys_member` are re-registered with widened `role` options under the same package id — an explicitly supported registry path (owner contribution replaced, merge cache invalidated). No DDL: options are validator/picker metadata, the column stays TEXT. serve / verify harness / DevPlugin drop their wiring — deliberately, so the dogfood invite gates only stay green if the auto-derivation works. Explicit `additionalOrgRoles` remains as an override for roles outside stack metadata (unioned with the derived set). One bug found the hard way and worth recording: the hook originally acquired the engine via `ctx.getServiceAsync('objectql')` — a method PluginContext does not have. The optional-chain made that silently yield `undefined`, the metadata facade still fed the better-auth half, and the select half was skipped without an error: derivation logged success while invitations kept failing. PluginContext's sync `getService` is the API. Verified: dogfood app-org-role-invite + delegated-admin-invite gates green with ZERO host wiring (10/10); org-roles unit tests 29 (5 new for collectRegisteredOrgRoles); plugin-auth 603; full dogfood 375 passed. --- .changeset/auth-org-roles-self-derived.md | 31 ++++++++ packages/cli/src/commands/serve.ts | 19 ++--- .../plugins/plugin-auth/src/auth-plugin.ts | 77 ++++++++++++++++++- .../plugins/plugin-auth/src/org-roles.test.ts | 61 +++++++++++++++ packages/plugins/plugin-auth/src/org-roles.ts | 54 +++++++++++++ packages/plugins/plugin-dev/src/dev-plugin.ts | 18 ++--- packages/verify/src/harness.ts | 14 ++-- 7 files changed, 242 insertions(+), 32 deletions(-) create mode 100644 .changeset/auth-org-roles-self-derived.md diff --git a/.changeset/auth-org-roles-self-derived.md b/.changeset/auth-org-roles-self-derived.md new file mode 100644 index 0000000000..f89eb043c0 --- /dev/null +++ b/.changeset/auth-org-roles-self-derived.md @@ -0,0 +1,31 @@ +--- +'@objectstack/plugin-auth': minor +'@objectstack/plugin-dev': patch +'@objectstack/verify': patch +'@objectstack/cli': patch +--- + +feat(auth): AuthPlugin derives app-declared organization roles itself — hosts pass nothing (#3723 follow-up, cloud#897) + +Five hosts boot `AuthPlugin` from a stack, and per-host `additionalOrgRoles` +wiring proved to be the defect pattern: three of them (the verify harness, +`DevPlugin`, cloud's `ArtifactKernelFactory`) at some point forgot it, and the +failure is silent — app-declared roles are simply absent. One host (cloud) +mounts `AuthPlugin` before the app metadata even exists, so no init-time walk +could ever cover it. + +`AuthPlugin` now derives the roles in its own `kernel:ready` hook — the one +point that fires after all metadata is registered in every host — via the new +`collectRegisteredOrgRoles(engine, metadataService?)` (the late-bound twin of +`collectStackOrgRoles`). Both consumers are updated from the derived union: +better-auth's org-plugin roles map (`applyConfigPatch`; the instance builds +lazily) and the `sys_invitation.role` / `sys_member.role` select options +(re-registration under the same package id — a supported registry path; no +DDL, options are validator/picker metadata). + +`objectstack serve`, the `@objectstack/verify` harness and `DevPlugin` no +longer pass `additionalOrgRoles` — deliberately, so the dogfood invite gate +only stays green if the auto-derivation works. The option remains for roles +declared OUTSIDE stack metadata; explicit entries are unioned with the derived +set. `collectStackOrgRoles` stays exported for hosts that want an init-time +walk of a raw stack object. diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 536333ba9e..d13f28e3c3 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -1396,7 +1396,7 @@ export default class Serve extends Command { if (!hasAuthPlugin && tierEnabled('auth')) { try { const authPkg = '@objectstack/plugin-auth'; - const { AuthPlugin, collectStackOrgRoles } = await import(/* webpackIgnore: true */ authPkg); + const { AuthPlugin } = await import(/* webpackIgnore: true */ authPkg); // In dev, fall back to a stable local secret so users don't have // to set OS_AUTH_SECRET just to try the login/register flow. @@ -1491,22 +1491,17 @@ export default class Serve extends Command { if (!trustedOrigins.includes(wildcard)) trustedOrigins.push(wildcard); } - // Collect application-defined org roles from the stack so - // Better-Auth's organization plugin accepts invitations to - // those names (otherwise it 400s with `ROLE_NOT_FOUND`) AND the - // `sys_invitation.role` / `sys_member.role` selects accept them on - // write. #3723: the walk lives in plugin-auth so `serve`, the - // `@objectstack/verify` harness and any embedder derive the list - // identically — a second copy of it here is how the harness came to - // boot without app roles while `serve` had them. - const additionalOrgRoles = collectStackOrgRoles(config); - + // App-declared organization roles (positions / permission sets) + // need no wiring here: AuthPlugin derives them from the registered + // metadata in its own kernel:ready hook (#3723 / cloud#897) — the + // per-host walk was the defect pattern (three of five hosts forgot + // it). `additionalOrgRoles` remains available for roles OUTSIDE + // the stack metadata, which this host has none of. await kernel.use(new AuthPlugin({ secret, baseUrl, socialProviders: Object.keys(socialProviders).length > 0 ? socialProviders : undefined, trustedOrigins: trustedOrigins.length ? trustedOrigins : undefined, - ...(additionalOrgRoles.length > 0 ? { additionalOrgRoles } : {}), // Enable the admin plugin by default so the Setup app's // ban/unban/set-password/impersonate/set-role row actions // resolve to real endpoints. The plugin self-gates by role diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index b5b5f76cc9..768647fe0a 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -32,10 +32,17 @@ import { runSetInitialPassword } from './set-initial-password.js'; import { runRegisterSsoProviderFromForm, runRegisterSamlProviderFromForm, runRequestDomainVerification, runVerifyDomain } from './register-sso-provider.js'; import { runResendVerificationEmail } from './send-verification-email.js'; import { + AUTH_PLUGIN_ID, authIdentityObjects, authPluginManifestHeader, } from './manifest.js'; -import { normalizeAdditionalOrgRoles, withMembershipRoleOptions, type OrgRoleInput } from './org-roles.js'; +import { + MEMBERSHIP_ROLE_OBJECTS, + collectRegisteredOrgRoles, + normalizeAdditionalOrgRoles, + withMembershipRoleOptions, + type OrgRoleInput, +} from './org-roles.js'; /** * Auth Plugin Options @@ -398,6 +405,74 @@ export class AuthPlugin implements Plugin { : {}), }); + // [#3723 / cloud#897] Late-bound organization roles — hosts pass nothing. + // + // Five hosts boot AuthPlugin from a stack; three of them (verify harness, + // DevPlugin, cloud's ArtifactKernelFactory) at some point forgot to wire + // `additionalOrgRoles`, and the failure is silent: app-declared roles are + // simply absent, no error anywhere. Per-host wiring is the defect pattern; + // this hook removes the need for it. cloud is also ORDER-hostile: it + // mounts AuthPlugin before the app metadata exists, so no init-time walk + // could ever cover it — `kernel:ready` is the one point that fires after + // all metadata is registered in every host. + // + // What happens when app roles are found beyond the explicit config: + // - better-auth side: `applyConfigPatch` merges them; the instance is + // built lazily (or reset by the patch), so the org-plugin roles map is + // rebuilt with the full set on next use. + // - select side: `sys_invitation` / `sys_member` are re-registered with + // widened `role` options under the SAME package id — an explicitly + // supported re-registration path (the registry replaces the owner + // contribution and invalidates the merge cache). No DDL: options are + // validator/picker metadata, the column stays TEXT. + // + // Explicit `additionalOrgRoles` remains as override/extra — the union is + // what both consumers see, preserving the one-list invariant. + ctx.hook('kernel:ready', async () => { + try { + // PluginContext has no getServiceAsync — the sync getService is the + // API (it throws for unknown names, hence the guard). At kernel:ready + // the engine is long registered in every real boot; `undefined` only + // in mock/Lite kernels, where the metadata facade below still serves + // the better-auth half and the select half has no engine to update. + const engine = (() => { + try { return ctx.getService?.('objectql') as unknown; } catch { return undefined; } + })(); + const metadataService = (() => { + try { return ctx.getService?.('metadata') as { list?: (t: string) => unknown } | undefined; } + catch { return undefined; } + })(); + if (!engine && !metadataService) return; // mock/Lite boots — nothing to derive from + + const registered = await collectRegisteredOrgRoles(engine, metadataService, ctx.logger); + if (registered.length === 0) return; + + const known = new Set(additionalOrgRoles.map((d) => d.name)); + const discovered = registered.filter((d) => !known.has(d.name)); + if (discovered.length === 0) return; + + const merged = [...additionalOrgRoles, ...discovered]; + this.authManager?.applyConfigPatch({ additionalOrgRoles: merged }); + + const ql = engine as { registerObject?: (s: unknown, pkg?: string, ns?: string) => string } | undefined; + if (typeof ql?.registerObject === 'function') { + const widened = withMembershipRoleOptions(authIdentityObjects, merged) + .filter((o) => (MEMBERSHIP_ROLE_OBJECTS as readonly string[]).includes((o as { name?: string })?.name ?? '')); + for (const schema of widened) { + ql.registerObject(schema, AUTH_PLUGIN_ID, authPluginManifestHeader.namespace); + } + } + ctx.logger.info('[auth] app-declared organization roles derived from registered metadata', { + discovered: discovered.map((d) => d.name), + total: merged.length, + }); + } catch (e) { + // Derivation is additive — a failure leaves the explicit config in + // force, which is exactly the pre-hook behavior. Loud, not fatal. + ctx.logger.warn('[auth] organization-role derivation failed', { error: (e as Error).message }); + } + }); + ctx.logger.info('Auth Plugin initialized successfully'); } diff --git a/packages/plugins/plugin-auth/src/org-roles.test.ts b/packages/plugins/plugin-auth/src/org-roles.test.ts index 6a40fcaaac..fcdc093520 100644 --- a/packages/plugins/plugin-auth/src/org-roles.test.ts +++ b/packages/plugins/plugin-auth/src/org-roles.test.ts @@ -14,6 +14,7 @@ import { describe, it, expect, vi } from 'vitest'; import { BUILTIN_MEMBERSHIP_ROLE_OPTIONS } from '@objectstack/spec/identity'; import { SysInvitation, SysMember, SysUser } from '@objectstack/platform-objects/identity'; import { + collectRegisteredOrgRoles, collectStackOrgRoles, membershipRoleLabel, membershipRoleOptions, @@ -225,3 +226,63 @@ describe('#3723 collectStackOrgRoles — one walk for every host', () => { expect(values(roleOptionsOf(member))).toContain('sales_user'); }); }); + +describe('#3723/cloud#897 collectRegisteredOrgRoles — the late-bound twin', () => { + it('reads position + permission items from the engine registry', async () => { + const engine = { + _registry: { + listItems: (type: string) => + type === 'position' + ? [{ content: { name: 'sales_rep', label: '销售代表' } }] + : type === 'permission' + ? [{ name: 'sales_user' }] // bare item (no content wrapper) — both shapes occur + : [], + }, + }; + expect(await collectRegisteredOrgRoles(engine)).toEqual([ + { name: 'sales_rep', label: '销售代表' }, + { name: 'sales_user' }, + ]); + }); + + it('falls back to the metadata-service facade when the registry has nothing', async () => { + const metadataService = { + list: (type: string) => + Promise.resolve(type === 'position' ? [{ name: 'ops', label: 'Operations' }] : []), + }; + expect(await collectRegisteredOrgRoles(undefined, metadataService)).toEqual([ + { name: 'ops', label: 'Operations' }, + ]); + }); + + it('registry wins over the facade per type — no double counting', async () => { + const engine = { _registry: { listItems: (t: string) => (t === 'position' ? [{ name: 'ops' }] : []) } }; + const metadataService = { + list: (t: string) => (t === 'position' ? [{ name: 'ops' }, { name: 'ghost' }] : [{ name: 'sales_user' }]), + }; + // position served by the registry (facade ignored for it); permission + // empty in the registry → facade serves it. + expect(await collectRegisteredOrgRoles(engine, metadataService)).toEqual([ + { name: 'ops' }, + { name: 'sales_user' }, + ]); + }); + + it('normalizes like every other entry point — built-ins and bad names drop', async () => { + const engine = { + _registry: { + listItems: (t: string) => + t === 'position' ? [{ name: 'owner' }, { name: 'bad.name' }, { name: 'ok_role' }] : [], + }, + }; + expect(await collectRegisteredOrgRoles(engine)).toEqual([{ name: 'ok_role' }]); + }); + + it('nothing to read from → empty, never a throw', async () => { + expect(await collectRegisteredOrgRoles(undefined, undefined)).toEqual([]); + expect(await collectRegisteredOrgRoles({}, {})).toEqual([]); + expect( + await collectRegisteredOrgRoles({ _registry: { listItems: () => { throw new Error('boom'); } } }), + ).toEqual([]); + }); +}); diff --git a/packages/plugins/plugin-auth/src/org-roles.ts b/packages/plugins/plugin-auth/src/org-roles.ts index e5d7f534c1..1cf6da79bb 100644 --- a/packages/plugins/plugin-auth/src/org-roles.ts +++ b/packages/plugins/plugin-auth/src/org-roles.ts @@ -292,3 +292,57 @@ export function collectStackOrgRoles(stack: unknown, logger?: OrgRoleLogger): Or } return normalizeAdditionalOrgRoles(entries, logger); } + +/** + * Collect the organization roles REGISTERED in the running kernel — the + * late-bound twin of {@link collectStackOrgRoles}. + * + * `collectStackOrgRoles` needs the raw stack object in the host's hand, which + * is exactly why it kept being forgotten: five hosts booted `AuthPlugin` from + * a stack, and three of them (verify harness, DevPlugin, cloud's + * ArtifactKernelFactory) never wired the walk — a capability silently absent, + * no error anywhere. Worse, one host (cloud) mounts `AuthPlugin` BEFORE the + * app metadata even exists, so no init-time walk could ever cover it. + * + * This variant instead reads the `position` / `permission` metadata already + * registered with the engine, at whatever point it is called — `AuthPlugin` + * calls it from its own `kernel:ready` hook, which in every host fires after + * ALL plugins (and therefore all app metadata) are in. Hosts pass nothing. + * + * Dual read, same as `bootstrapDeclaredPositions`: the engine's + * SchemaRegistry (`_registry.listItems`) is populated by `manifest.register` + * in every boot path; the metadata-service facade is the fallback for boots + * where the registry shape differs. + */ +export async function collectRegisteredOrgRoles( + engine: unknown, + metadataService?: { list?: (type: string) => unknown }, + logger?: OrgRoleLogger, +): Promise { + const entries: OrgRoleInput[] = []; + const push = (items: unknown) => { + if (!Array.isArray(items)) return; + for (const raw of items) { + const item = (raw as { content?: unknown })?.content ?? raw; + if (item && typeof (item as { name?: unknown }).name === 'string') { + const { name, label } = item as { name: string; label?: unknown }; + entries.push(typeof label === 'string' ? { name, label } : { name }); + } + } + }; + for (const type of ['position', 'permission']) { + let items: unknown = undefined; + try { + const reg = (engine as { _registry?: { listItems?: (t: string) => unknown } })?._registry; + if (typeof reg?.listItems === 'function') items = reg.listItems(type); + } catch { /* fall through to the facade */ } + if (!Array.isArray(items) || items.length === 0) { + try { + const listed = metadataService?.list?.(type); + items = typeof (listed as Promise)?.then === 'function' ? await listed : listed; + } catch { items = undefined; } + } + push(items); + } + return normalizeAdditionalOrgRoles(entries, logger); +} diff --git a/packages/plugins/plugin-dev/src/dev-plugin.ts b/packages/plugins/plugin-dev/src/dev-plugin.ts index 9e5dc63152..0074777f1e 100644 --- a/packages/plugins/plugin-dev/src/dev-plugin.ts +++ b/packages/plugins/plugin-dev/src/dev-plugin.ts @@ -522,21 +522,15 @@ export class DevPlugin implements Plugin { let authMounted = false; if (enabled('auth')) { try { - const { AuthPlugin, collectStackOrgRoles } = await import('@objectstack/plugin-auth') as any; - // [#3723] The same stack walk `objectstack serve` and the verify - // harness use. DevPlugin advertises itself as equivalent to the full - // stack, so a stack whose declared positions / permission sets are - // invitable under `serve` must be invitable here too — otherwise - // "equivalent" quietly excludes app-declared organization roles. - // Guarded: an older plugin-auth on disk simply yields none. - const additionalOrgRoles: string[] = - typeof collectStackOrgRoles === 'function' - ? collectStackOrgRoles(this.options.stack, ctx.logger) - : []; + const { AuthPlugin } = await import('@objectstack/plugin-auth') as any; + // [#3723] App-declared organization roles need no wiring: AuthPlugin + // derives them from the registered metadata in its own kernel:ready + // hook, so DevPlugin's "equivalent to the full stack" claim holds + // without this host having to remember a parameter (it was one of the + // three hosts that forgot it). const authPlugin = new AuthPlugin({ secret: this.options.authSecret, baseUrl: this.options.authBaseUrl, - ...(additionalOrgRoles.length > 0 ? { additionalOrgRoles } : {}), }); this.childPlugins.push(authPlugin); authMounted = true; diff --git a/packages/verify/src/harness.ts b/packages/verify/src/harness.ts index 72056ffeb9..8b7cd8440d 100644 --- a/packages/verify/src/harness.ts +++ b/packages/verify/src/harness.ts @@ -25,7 +25,7 @@ import { ObjectQLPlugin } from '@objectstack/objectql'; import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; import { HonoServerPlugin } from '@objectstack/plugin-hono-server'; import { createRestApiPlugin } from '@objectstack/rest'; -import { AuthPlugin, collectStackOrgRoles } from '@objectstack/plugin-auth'; +import { AuthPlugin } from '@objectstack/plugin-auth'; import { SecurityPlugin } from '@objectstack/plugin-security'; import { SharingServicePlugin } from '@objectstack/plugin-sharing'; import { SettingsServicePlugin, LocalCryptoProvider } from '@objectstack/service-settings'; @@ -182,15 +182,15 @@ export async function bootStack( // create, ADR-0062 federation, ADR-0086 two-doors). The bootstrap itself is // covered by plugin-auth unit tests + browser E2E. // - // [#3723] `additionalOrgRoles` IS derived here, from the same - // `collectStackOrgRoles` walk `objectstack serve` uses. The harness used to - // omit it, so a dogfood proof booted a stack whose declared roles better-auth - // had never heard of — the one surface that drives the real HTTP route was - // blind to the exact class of bug it exists to catch. + // [#3723] App-declared organization roles need no wiring: AuthPlugin derives + // them from the registered metadata in its own kernel:ready hook. The + // harness passing NOTHING is deliberate and is itself part of the proof — + // the dogfood invite gate only stays green if the auto-derivation works, + // which is exactly the guarantee per-host wiring could never give (this + // harness was one of the three hosts that forgot it). await kernel.use(new AuthPlugin({ secret: opts.authSecret ?? DEFAULT_AUTH_SECRET, autoDefaultOrganization: false, - additionalOrgRoles: collectStackOrgRoles(config), })); // ADR-0062 — datasource connection service (registers 'datasource-connection'), From 3a11875532ac0187e2fb6dc3378c31faa63a76e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 04:19:05 +0000 Subject: [PATCH 2/2] =?UTF-8?q?docs(auth):=20additionalOrgRoles=20TSDoc=20?= =?UTF-8?q?reflects=20the=20self-derivation=20=E2=80=94=20it=20is=20the=20?= =?UTF-8?q?exception=20now,=20not=20the=20wiring?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both option surfaces still described the parameter as 'the one walk every host shares', naming the three hosts whose wiring this branch just removed. Flagged by re-reading the docs-drift advisory against the actual diff. --- packages/plugins/plugin-auth/src/auth-manager.ts | 7 +++++-- packages/plugins/plugin-auth/src/auth-plugin.ts | 10 ++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index ea51467888..21f28b8314 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -404,8 +404,11 @@ export interface AuthManagerOptions extends Partial { * Better-Auth's `member` role) so it cannot inadvertently grant org-level * admin capabilities. * - * Typical source: `collectStackOrgRoles(stack)` — the one walk every host - * shares (`objectstack serve`, the verify harness, DevPlugin). + * Rarely needed since the kernel:ready self-derivation (#3723 follow-up): + * AuthPlugin reads the registered `position` / `permission` metadata itself + * via `collectRegisteredOrgRoles`, so stack-declared roles arrive with NO + * host wiring. Pass this only for roles that exist OUTSIDE stack metadata + * (the derived set and this list are unioned). * * Accepts a bare name, or `{ name, label }` to carry the declaring * metadata's own display label into the role picker (#3723) — without it the diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 768647fe0a..85dab3857b 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -82,10 +82,12 @@ export interface AuthPluginOptions extends Partial { manifestDatasource?: string; /** - * Application-specific organization roles to register with Better-Auth's - * organization plugin so invitations to those roles aren't rejected with - * ROLE_NOT_FOUND. Forwarded as-is to AuthManager. See - * {@link AuthManagerOptions.additionalOrgRoles} for details. + * EXTRA organization roles to register with Better-Auth's organization + * plugin, beyond the ones AuthPlugin derives itself: stack-declared + * `position` / `permission` names arrive automatically via the + * kernel:ready self-derivation (#3723 follow-up), so most hosts pass + * nothing. Only roles that exist OUTSIDE stack metadata need this; the + * two sets are unioned. See {@link AuthManagerOptions.additionalOrgRoles}. */ additionalOrgRoles?: OrgRoleInput[];