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-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 b5b5f76cc9..85dab3857b 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 @@ -75,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[]; @@ -398,6 +407,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'),