From e9b11df4f21db1c398790203fa074b067faff01c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 01:37:40 +0000 Subject: [PATCH 1/6] fix(auth): app-declared org roles are storable, not just registerable (#3723) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `additionalOrgRoles` registered every `permission` / `position` name a stack declared with better-auth's organization plugin, so `POST /organization/invite-member { role: 'sales_rep' }` passed the role check — and then the write failed, because `sys_invitation.role` and `sys_member.role` were closed selects listing `owner|admin|member` only: ValidationError: role must be one of: owner, admin, member { field: 'role', code: 'invalid_option' } A select is enforced on write and better-auth's own inserts are not exempt (they run through the ordinary ObjectQL validator, after the security middleware), so every deployment declaring role names was registering roles that could be requested and never stored — `declared ≠ enforced` one layer below the declaration. Contract-first fix: one list, materialized into both consumers. - `spec/identity/membership-role.ts` holds the built-in roles + their select options. `MEMBERSHIP_ROLE_DELEGATED_ADMIN` moves here from `eval-user.zod` (package-level export path unchanged). - `plugin-auth/org-roles.ts` owns the derivation: `normalizeAdditionalOrgRoles` is the single normalizer, and its output feeds better-auth's role map AND the two select option lists (`withMembershipRoleOptions`, stamped onto the manifest at registration — copy-on-write, the shared definitions are never mutated). Neither side keeps a list of its own. - `sys_invitation` / `sys_member` declare the built-in baseline only. - `collectStackOrgRoles` is the one producer-side walk; `objectstack serve` and the `@objectstack/verify` harness both use it. The harness passed no roles at all, which is why the surface that drives the real HTTP route was blind to this class of bug. A declared name that is not a valid machine name is now refused on BOTH sides with a boot warning, rather than registered and then stored mangled: `Field.select` strips characters outside `[a-z0-9_]`, so `showcase.export_data` would be accepted as itself and written as `showcaseexport_data` — the same mismatch with extra steps. Verified: `app-org-role-invite.dogfood.test.ts` drives the real invite route on the showcase stack and fails with the exact reported ValidationError when the materialization step is removed. --- .changeset/app-org-roles-storable.md | 54 ++++ packages/cli/src/commands/serve.ts | 38 +-- .../src/identity/sys-invitation.object.ts | 20 +- .../src/identity/sys-member.object.ts | 30 +-- .../plugin-auth/src/auth-manager.test.ts | 17 ++ .../plugins/plugin-auth/src/auth-manager.ts | 11 +- .../plugins/plugin-auth/src/auth-plugin.ts | 18 +- packages/plugins/plugin-auth/src/index.ts | 5 + .../plugin-auth/src/invitation-role-cap.ts | 14 +- .../plugins/plugin-auth/src/org-roles.test.ts | 189 ++++++++++++++ packages/plugins/plugin-auth/src/org-roles.ts | 242 ++++++++++++++++++ .../test/app-org-role-invite.dogfood.test.ts | 194 ++++++++++++++ packages/spec/src/identity/eval-user.zod.ts | 24 -- packages/spec/src/identity/index.ts | 3 + packages/spec/src/identity/membership-role.ts | 105 ++++++++ packages/spec/src/index.ts | 15 +- packages/verify/src/harness.ts | 14 +- 17 files changed, 902 insertions(+), 91 deletions(-) create mode 100644 .changeset/app-org-roles-storable.md create mode 100644 packages/plugins/plugin-auth/src/org-roles.test.ts create mode 100644 packages/plugins/plugin-auth/src/org-roles.ts create mode 100644 packages/qa/dogfood/test/app-org-role-invite.dogfood.test.ts create mode 100644 packages/spec/src/identity/membership-role.ts diff --git a/.changeset/app-org-roles-storable.md b/.changeset/app-org-roles-storable.md new file mode 100644 index 0000000000..9d55fe4c8a --- /dev/null +++ b/.changeset/app-org-roles-storable.md @@ -0,0 +1,54 @@ +--- +'@objectstack/platform-objects': patch +'@objectstack/plugin-auth': patch +'@objectstack/verify': patch +'@objectstack/spec': patch +'@objectstack/cli': patch +--- + +fix(auth): app-declared organization roles are now storable, not just registerable (#3723) + +`AuthManagerOptions.additionalOrgRoles` registered every `permission` / +`position` name a stack declared with better-auth's organization plugin, so +`POST /organization/invite-member { role: 'sales_rep' }` passed the role check — +and then the write failed, because `sys_invitation.role` and `sys_member.role` +were closed selects listing `owner|admin|member` only: + +``` +ValidationError: role must be one of: owner, admin, member + { field: 'role', code: 'invalid_option' } +``` + +A select is enforced on write and better-auth's own inserts are not exempt (they +run through the ordinary ObjectQL validator), so any stack declaring role names +was registering roles that could be requested and never stored. + +Both gatekeepers now read one list. `normalizeAdditionalOrgRoles` is the single +normalizer; its output feeds better-auth's role map **and** the two `select` +option lists, so neither side can accept a name the other rejects. The built-in +roles (`owner`, `admin`, `delegated_admin`, `member`) live in +`@objectstack/spec` as `BUILTIN_MEMBERSHIP_ROLE_OPTIONS`, which is all the +platform objects declare statically — app roles are appended at boot. + +New exports: + +- `@objectstack/spec` — `MEMBERSHIP_ROLE_{OWNER,ADMIN,MEMBER,DELEGATED_ADMIN}`, + `BUILTIN_MEMBERSHIP_ROLES`, `BUILTIN_MEMBERSHIP_ROLE_OPTIONS`, + `MEMBERSHIP_ROLE_NAME_PATTERN`, `MEMBERSHIP_ROLE_NAME_MIN_LENGTH` + (`MEMBERSHIP_ROLE_DELEGATED_ADMIN` moved from `identity/eval-user.zod` to + `identity/membership-role`; the package-level export path is unchanged). +- `@objectstack/plugin-auth` — `collectStackOrgRoles`, + `normalizeAdditionalOrgRoles`, `membershipRoleOptions`, + `withMembershipRoleOptions`. + +Hosts that boot `AuthPlugin` from a loaded stack should derive +`additionalOrgRoles` with `collectStackOrgRoles(stack)` rather than walking the +stack themselves — `objectstack serve` and the `@objectstack/verify` harness now +both do (the harness previously passed none, which is why a dogfood proof could +boot a stack whose declared roles better-auth had never heard of). + +Behaviour change worth noting: a declared role name that is not a valid machine +name (`/^[a-z][a-z0-9_]*$/`, min 2 chars) is no longer registered at all, with a +boot warning. `Field.select` strips characters outside `[a-z0-9_]`, so such a +name would be registered verbatim and stored mangled — the same mismatch with +extra steps. Every name that passes `SnakeCaseIdentifierSchema` is unaffected. diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index bf2be31696..1f4d4fb96f 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -1388,7 +1388,7 @@ export default class Serve extends Command { if (!hasAuthPlugin && tierEnabled('auth')) { try { const authPkg = '@objectstack/plugin-auth'; - const { AuthPlugin } = await import(/* webpackIgnore: true */ authPkg); + const { AuthPlugin, collectStackOrgRoles } = 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. @@ -1485,40 +1485,20 @@ export default class Serve extends Command { // 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`). - // better-auth boundary: its API keeps the word "roles" - // (ADR-0090 D3 exception). Sources: - // - top-level `positions[]` (flat distribution groups) - // - `permissions[]` PermissionSet names - // Real RBAC enforcement is still owned by SecurityPlugin. - const additionalOrgRoles = new Set(); - try { - const stackAny: any = config ?? {}; - const collect = (arr: any) => { - if (!Array.isArray(arr)) return; - for (const r of arr) { - const n = typeof r === 'string' ? r : (r && typeof r.name === 'string' ? r.name : null); - if (n && n !== 'owner' && n !== 'admin' && n !== 'member') additionalOrgRoles.add(n); - } - }; - collect(stackAny.positions); - if (Array.isArray(stackAny.permissions)) { - for (const p of stackAny.permissions) { - if (p && typeof p.name === 'string') { - if (p.name !== 'owner' && p.name !== 'admin' && p.name !== 'member') additionalOrgRoles.add(p.name); - } - } - } - } catch { - // best-effort - } + // 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); await kernel.use(new AuthPlugin({ secret, baseUrl, socialProviders: Object.keys(socialProviders).length > 0 ? socialProviders : undefined, trustedOrigins: trustedOrigins.length ? trustedOrigins : undefined, - ...(additionalOrgRoles.size > 0 ? { additionalOrgRoles: Array.from(additionalOrgRoles) } : {}), + ...(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/platform-objects/src/identity/sys-invitation.object.ts b/packages/platform-objects/src/identity/sys-invitation.object.ts index 7b138a3f79..108968b553 100644 --- a/packages/platform-objects/src/identity/sys-invitation.object.ts +++ b/packages/platform-objects/src/identity/sys-invitation.object.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { BUILTIN_MEMBERSHIP_ROLE_OPTIONS, MEMBERSHIP_ROLE_MEMBER } from '@objectstack/spec/identity'; /** * sys_invitation — System Invitation Object @@ -196,22 +197,17 @@ export const SysInvitation = ObjectSchema.create({ description: 'Email address of the invited user', }), + // [#3723] Same list as `sys_member.role`, from the same constant — this is + // the value that lands there on acceptance, so the two can never be allowed + // to drift. App-declared organization roles are appended at boot by + // plugin-auth's `withMembershipRoleOptions`, from the same normalized array + // that registers them with better-auth; do not hand-add one here. role: Field.select({ label: 'Role', required: false, description: 'Role to assign upon acceptance', - options: [ - { label: 'Owner', value: 'owner' }, - { label: 'Admin', value: 'admin' }, - // [ADR-0105 D8 / #3697] Kept in step with `sys_member.role` — this is - // the value that lands there on acceptance, and inviting is how a - // delegate gets provisioned in the first place. Both selects are - // enforced on write, so a role missing from either one is a role that - // cannot be handed out. - { label: 'Delegated Admin', value: 'delegated_admin' }, - { label: 'Member', value: 'member' }, - ], - defaultValue: 'member', + options: [...BUILTIN_MEMBERSHIP_ROLE_OPTIONS], + defaultValue: MEMBERSHIP_ROLE_MEMBER, }), status: Field.select(['pending', 'accepted', 'rejected', 'expired', 'canceled'], { diff --git a/packages/platform-objects/src/identity/sys-member.object.ts b/packages/platform-objects/src/identity/sys-member.object.ts index edcc7fe8fd..1fb14cb31c 100644 --- a/packages/platform-objects/src/identity/sys-member.object.ts +++ b/packages/platform-objects/src/identity/sys-member.object.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { BUILTIN_MEMBERSHIP_ROLE_OPTIONS, MEMBERSHIP_ROLE_MEMBER } from '@objectstack/spec/identity'; /** * sys_member — System Member Object @@ -165,27 +166,22 @@ export const SysMember = ObjectSchema.create({ required: true, }), + // [#3723] The framework's built-in roles ONLY. App-declared organization + // roles are appended at boot by plugin-auth's `withMembershipRoleOptions`, + // from the SAME normalized array that registers them with better-auth — + // adding one by hand here re-creates the two-lists-that-must-agree bug. + // + // This select is ENFORCED on write: better-auth's own accept-invitation + // membership insert is validated like any other row (system context does + // not exempt it), so a role better-auth accepts and this list omits is a + // role nobody can hold. `delegated_admin` (ADR-0105 D8 / #3697) is in the + // built-in list for exactly that reason. role: Field.select({ label: 'Role', required: false, description: 'Member role within the organization', - options: [ - { label: 'Owner', value: 'owner' }, - { label: 'Admin', value: 'admin' }, - // [ADR-0105 D8 / #3697] The delegated issuer grade — may reach - // `/organization/invite-member` WITHOUT being an org admin, which is - // what finally gives D8's scope-bounded issuance gate a caller. It - // carries no ObjectStack authority by itself: placement authority - // comes from a separately-granted `adminScope`, and the invitation - // role cap holds it to inviting plain members. - // - // Listed here because this select is ENFORCED on write: better-auth's - // own accept-invitation membership insert is validated like any other - // row, so a role missing from this list is a role nobody can hold. - { label: 'Delegated Admin', value: 'delegated_admin' }, - { label: 'Member', value: 'member' }, - ], - defaultValue: 'member', + options: [...BUILTIN_MEMBERSHIP_ROLE_OPTIONS], + defaultValue: MEMBERSHIP_ROLE_MEMBER, }), }, diff --git a/packages/plugins/plugin-auth/src/auth-manager.test.ts b/packages/plugins/plugin-auth/src/auth-manager.test.ts index b390c5dece..96cc1ff71f 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.test.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.test.ts @@ -942,6 +942,23 @@ describe('AuthManager', () => { expect(orgPlugin._opts.roles.delegated_admin.statements.invitation).toEqual(['create']); }); + it('#3723: a role name the write path cannot store is not registered either', async () => { + // `Field.select` strips the dot, so `showcase.export_data` would be + // registered with better-auth verbatim and stored as + // `showcaseexport_data` — the two lists agreeing on the name and + // disagreeing on the value is the original bug with extra steps. + // Refusing it here makes the invitation fail at the door + // (`ROLE_NOT_FOUND`) instead of at the `sys_invitation` insert. + const orgPlugin = await bootOrgPlugin({ + additionalOrgRoles: ['showcase.export_data', 'sales_rep'], + }); + + const roles = orgPlugin._opts.roles; + expect(Object.keys(roles)).toContain('sales_rep'); + expect(Object.keys(roles)).not.toContain('showcase.export_data'); + expect(Object.keys(roles)).not.toContain('showcaseexport_data'); + }); + it('role cap: a delegate inviting an `admin` is REFUSED (the escalation chain)', async () => { // delegate invites admin → sys_member(role='admin') → auto-org-admin-grant // → organization_admin → wildcard modifyAllRecords → isTenantAdmin(). diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index d8b95eb61f..9cfed87e8b 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -21,6 +21,7 @@ import { import { MCP_OAUTH_SCOPES } from '@objectstack/spec/ai'; import { createObjectQLAdapterFactory, withSystemReadContext } from './objectql-adapter.js'; import { invitationRoleCapFailure, isPlainMemberInvitation } from './invitation-role-cap.js'; +import { normalizeAdditionalOrgRoles } from './org-roles.js'; import { isPlaceholderEmail } from './placeholder-email.js'; import { reconcileMembership, type MembershipPolicy } from './reconcile-membership.js'; import type { TenancyService } from './tenancy-service.js'; @@ -1571,7 +1572,15 @@ export class AuthManager { // attribution — so the permission would mean "cancel anyone's pending // invitation in the org". Attributed cancel needs its own guard first. let customOrgRoles: Record | undefined; - const extra = this.config.additionalOrgRoles ?? []; + // [#3723] The SAME normalized array `AuthPlugin` stamps onto the + // `sys_invitation.role` / `sys_member.role` selects. Registering a name + // the write path cannot store is the bug this closes, so a name that + // cannot round-trip through `Field.select` is refused HERE too — the + // invitation then fails at better-auth's door (`ROLE_NOT_FOUND`) rather + // than at the insert. + // (Idempotent: `AuthPlugin` normalizes before constructing the manager, + // so this pass only warns for a caller wiring `AuthManager` directly.) + const extra = normalizeAdditionalOrgRoles(this.config.additionalOrgRoles, this.config.logger); try { const accessMod = await import('better-auth/plugins/organization/access'); const { defaultAc, memberAc, defaultRoles } = accessMod as any; diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index f8e3bf30ac..286f5f65ff 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -35,6 +35,7 @@ import { authIdentityObjects, authPluginManifestHeader, } from './manifest.js'; +import { normalizeAdditionalOrgRoles, withMembershipRoleOptions } from './org-roles.js'; /** * Auth Plugin Options @@ -200,8 +201,16 @@ export class AuthPlugin implements Plugin { ctx.logger.warn('No data engine service found - auth will use in-memory storage'); } + // [#3723] Normalize the app-declared organization roles ONCE, here: the + // same array feeds better-auth's role registry (via AuthManager) and the + // `sys_invitation.role` / `sys_member.role` selects (via the manifest + // below). Two derivations from one caller-supplied list is exactly the + // drift that made a registered role unstorable. + const additionalOrgRoles = normalizeAdditionalOrgRoles(this.options.additionalOrgRoles, ctx.logger); + const authConfig: AuthManagerOptions & AuthPluginOptions = { ...this.options, + additionalOrgRoles, dataEngine, logger: ctx.logger, // ADR-0093 D2/D3 — the membership reconciler consults the tenancy service @@ -330,7 +339,14 @@ export class AuthPlugin implements Plugin { ...(this.options.manifestDatasource ? { defaultDatasource: this.options.manifestDatasource } : {}), - objects: authIdentityObjects, + // [#3723] App-declared organization roles widen the `sys_invitation.role` + // / `sys_member.role` selects, from the SAME normalized array that + // registers them with better-auth (`AuthManager`'s org-plugin roles map). + // Without this the two lists drift and a registered role is one + // better-auth accepts and the write path rejects — the registration is + // half a feature. Copy-on-write: `authIdentityObjects` is a shared + // module-level array, never mutated. + objects: withMembershipRoleOptions(authIdentityObjects, additionalOrgRoles), // ADR-0048 — Setup/Studio/Account apps (and the Setup nav contributions) // moved to their own one-app packages (@objectstack/{setup,studio,account}), // each registering under its own package id so /apps/ resolves diff --git a/packages/plugins/plugin-auth/src/index.ts b/packages/plugins/plugin-auth/src/index.ts index b2744d7a8b..53c4e2227e 100644 --- a/packages/plugins/plugin-auth/src/index.ts +++ b/packages/plugins/plugin-auth/src/index.ts @@ -30,4 +30,9 @@ export * from './auth-schema-config.js'; // compose the reconciler into their own hooks; embeddings query tenancy mode). export * from './reconcile-membership.js'; export * from './tenancy-service.js'; +// #3723 — organization roles. Exported because every host that boots AuthPlugin +// from a loaded stack (`objectstack serve`, the @objectstack/verify harness, the +// cloud per-project kernel) must derive `additionalOrgRoles` the SAME way; a +// second copy of the walk is how the harness came to boot without app roles. +export * from './org-roles.js'; export type { AuthConfig, AuthProviderConfig, AuthPluginConfig } from '@objectstack/spec/system'; diff --git a/packages/plugins/plugin-auth/src/invitation-role-cap.ts b/packages/plugins/plugin-auth/src/invitation-role-cap.ts index ca8a0122ff..46233ae367 100644 --- a/packages/plugins/plugin-auth/src/invitation-role-cap.ts +++ b/packages/plugins/plugin-auth/src/invitation-role-cap.ts @@ -43,10 +43,16 @@ * invitation is refused. */ -/** better-auth's built-in organization roles. */ -export const MEMBERSHIP_ROLE_OWNER = 'owner'; -export const MEMBERSHIP_ROLE_ADMIN = 'admin'; -export const MEMBERSHIP_ROLE_MEMBER = 'member'; +// better-auth's built-in organization roles — from the one membership-role +// list (#3723), the same constants the `sys_invitation.role` / `sys_member.role` +// selects are built from. Re-exported to keep this module's existing surface. +import { + MEMBERSHIP_ROLE_OWNER, + MEMBERSHIP_ROLE_ADMIN, + MEMBERSHIP_ROLE_MEMBER, +} from '@objectstack/spec/identity'; + +export { MEMBERSHIP_ROLE_OWNER, MEMBERSHIP_ROLE_ADMIN, MEMBERSHIP_ROLE_MEMBER }; /** Grade ladder. Anything unrecognized — `member`, `delegated_admin`, an * app-registered role — grades as an ordinary member; only better-auth's two diff --git a/packages/plugins/plugin-auth/src/org-roles.test.ts b/packages/plugins/plugin-auth/src/org-roles.test.ts new file mode 100644 index 0000000000..a928a809a3 --- /dev/null +++ b/packages/plugins/plugin-auth/src/org-roles.test.ts @@ -0,0 +1,189 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#3723] The two gatekeepers read one list. + * + * The bug: `additionalOrgRoles` registered a role with better-auth that the + * `sys_invitation.role` / `sys_member.role` selects then rejected on write — + * two hand-maintained lists nothing kept in step. These tests pin the + * invariant that replaced them: whatever `normalizeAdditionalOrgRoles` yields + * is EXACTLY what both sides see, and neither side has a list of its own. + */ + +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 { + collectStackOrgRoles, + membershipRoleLabel, + membershipRoleOptions, + normalizeAdditionalOrgRoles, + withMembershipRoleOptions, +} from './org-roles.js'; + +const values = (options: readonly { value: string }[]) => options.map((o) => o.value); +const roleOptionsOf = (object: unknown): { label: string; value: string }[] => + ((object as { fields: Record }).fields + .role.options ?? []); + +describe('#3723 normalizeAdditionalOrgRoles — the one normalizer', () => { + it('keeps valid app role names, in declaration order', () => { + expect(normalizeAdditionalOrgRoles(['sales_rep', 'sales_manager', 'service_agent'])).toEqual([ + 'sales_rep', + 'sales_manager', + 'service_agent', + ]); + }); + + it('drops the built-ins — an app may not redefine owner/admin/member/delegated_admin', () => { + // Silently downgrading `owner` to a plain-member role would strip its + // `invitation:create` and 403 every org mutation. + expect( + normalizeAdditionalOrgRoles(['owner', 'admin', 'member', 'delegated_admin', 'sales_rep']), + ).toEqual(['sales_rep']); + }); + + it('de-duplicates and trims', () => { + expect(normalizeAdditionalOrgRoles([' sales_rep ', 'sales_rep', ''])).toEqual(['sales_rep']); + }); + + it('ignores non-strings and a non-array input', () => { + expect(normalizeAdditionalOrgRoles([null, 42, {}, 'sales_rep'] as unknown[])).toEqual(['sales_rep']); + expect(normalizeAdditionalOrgRoles(undefined)).toEqual([]); + expect(normalizeAdditionalOrgRoles(null)).toEqual([]); + }); + + it('REFUSES a name that cannot round-trip through Field.select — loudly', () => { + // `Field.select` lowercases values and strips everything outside + // [a-z0-9_], so `showcase.export_data` would be registered with + // better-auth verbatim and stored as `showcaseexport_data`: the two lists + // agreeing on the name and disagreeing on the value is the same bug. + const warn = vi.fn(); + expect( + normalizeAdditionalOrgRoles( + ['showcase.export_data', 'Sales Rep', '_leading', 'x', 'sales_rep'], + { warn }, + ), + ).toEqual(['sales_rep']); + expect(warn).toHaveBeenCalledTimes(4); + expect(warn.mock.calls.map((c) => String(c[0])).join('\n')).toContain('showcase.export_data'); + }); + + it('is idempotent — AuthPlugin normalizes, AuthManager re-normalizes, same array', () => { + const once = normalizeAdditionalOrgRoles(['sales_rep', 'owner', 'bad.name']); + expect(normalizeAdditionalOrgRoles(once)).toEqual(once); + }); +}); + +describe('#3723 membershipRoleOptions — the option list both selects get', () => { + it('is the built-in baseline when the stack declares nothing', () => { + expect(membershipRoleOptions()).toEqual([...BUILTIN_MEMBERSHIP_ROLE_OPTIONS]); + }); + + it('appends app roles after the built-ins, with humanized labels', () => { + const options = membershipRoleOptions(['sales_rep']); + expect(values(options)).toEqual(['owner', 'admin', 'delegated_admin', 'member', 'sales_rep']); + expect(options.at(-1)).toEqual({ label: 'Sales Rep', value: 'sales_rep' }); + }); + + it('never duplicates a built-in', () => { + expect(values(membershipRoleOptions(['member', 'sales_rep']))).toEqual([ + 'owner', + 'admin', + 'delegated_admin', + 'member', + 'sales_rep', + ]); + }); + + it('humanizes labels without ever changing the stored value', () => { + expect(membershipRoleLabel('sales_rep')).toBe('Sales Rep'); + expect(membershipRoleLabel('ops')).toBe('Ops'); + expect(membershipRoleOptions(['field_ops_delegate']).at(-1)).toEqual({ + label: 'Field Ops Delegate', + value: 'field_ops_delegate', + }); + }); +}); + +describe('#3723 withMembershipRoleOptions — materializing onto the platform objects', () => { + const objects = [SysUser, SysMember, SysInvitation]; + + it('widens BOTH role selects — fixing one leaves the other rejecting the same value', () => { + // The lesson from #3722: `sys_member.role` alone still left + // `sys_invitation.role` refusing the invitation at issuance. + const [, member, invitation] = withMembershipRoleOptions(objects, ['sales_rep']); + expect(values(roleOptionsOf(member))).toContain('sales_rep'); + expect(values(roleOptionsOf(invitation))).toContain('sales_rep'); + expect(values(roleOptionsOf(member))).toEqual(values(roleOptionsOf(invitation))); + }); + + it('leaves unrelated objects untouched, by identity', () => { + const [user] = withMembershipRoleOptions(objects, ['sales_rep']); + expect(user).toBe(SysUser); + }); + + it('is copy-on-write — the shared module-level definitions are never mutated', () => { + // `authIdentityObjects` is a process-wide singleton also used by the + // compile-time `objectstack.config.ts`; two kernels booted with different + // app roles in one test run must not see each other's options. + withMembershipRoleOptions(objects, ['sales_rep']); + expect(values(roleOptionsOf(SysMember))).toEqual(values(BUILTIN_MEMBERSHIP_ROLE_OPTIONS)); + expect(values(roleOptionsOf(SysInvitation))).toEqual(values(BUILTIN_MEMBERSHIP_ROLE_OPTIONS)); + + const a = withMembershipRoleOptions(objects, ['role_a']); + const b = withMembershipRoleOptions(objects, ['role_b']); + expect(values(roleOptionsOf(a[1]))).toContain('role_a'); + expect(values(roleOptionsOf(a[1]))).not.toContain('role_b'); + expect(values(roleOptionsOf(b[1]))).toContain('role_b'); + }); + + it('no app roles → the built-in baseline, unchanged', () => { + const [, member] = withMembershipRoleOptions(objects, []); + expect(values(roleOptionsOf(member))).toEqual(values(BUILTIN_MEMBERSHIP_ROLE_OPTIONS)); + }); + + it('the static definitions carry the built-ins ONLY — extras come from here', () => { + // A drift guard: hand-adding a role to either object file would re-create + // the two-lists problem from the other direction (an option better-auth + // never registered, so the picker offers a role that 400s at the door). + expect(values(roleOptionsOf(SysMember))).toEqual(values(BUILTIN_MEMBERSHIP_ROLE_OPTIONS)); + expect(values(roleOptionsOf(SysInvitation))).toEqual(values(BUILTIN_MEMBERSHIP_ROLE_OPTIONS)); + }); +}); + +describe('#3723 collectStackOrgRoles — one walk for every host', () => { + it('collects position and permission names from a loaded stack', () => { + const roles = collectStackOrgRoles({ + positions: [{ name: 'sales_rep' }, { name: 'sales_manager' }], + permissions: [{ name: 'sales_user' }, { name: 'guest_portal' }], + }); + expect(roles).toEqual(['sales_rep', 'sales_manager', 'sales_user', 'guest_portal']); + }); + + it('accepts bare strings as well as named entries', () => { + expect(collectStackOrgRoles({ positions: ['sales_rep', { name: 'ops' }] })).toEqual([ + 'sales_rep', + 'ops', + ]); + }); + + it('normalizes on the way out — the built-ins never leak into the extras', () => { + expect( + collectStackOrgRoles({ positions: [{ name: 'member' }, { name: 'owner' }, { name: 'ops' }] }), + ).toEqual(['ops']); + }); + + it('a stack with no role metadata yields nothing (and does not throw)', () => { + expect(collectStackOrgRoles({})).toEqual([]); + expect(collectStackOrgRoles(undefined)).toEqual([]); + expect(collectStackOrgRoles({ positions: 'not-an-array', permissions: 7 })).toEqual([]); + }); + + it('feeds the option list directly — collection and materialization agree', () => { + const stack = { permissions: [{ name: 'sales_user' }] }; + const roles = collectStackOrgRoles(stack); + const [, member] = withMembershipRoleOptions([SysUser, SysMember], roles); + expect(values(roleOptionsOf(member))).toContain('sales_user'); + }); +}); diff --git a/packages/plugins/plugin-auth/src/org-roles.ts b/packages/plugins/plugin-auth/src/org-roles.ts new file mode 100644 index 0000000000..a412b2efd0 --- /dev/null +++ b/packages/plugins/plugin-auth/src/org-roles.ts @@ -0,0 +1,242 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#3723] App-declared organization roles — registered once, honoured by both + * gatekeepers. + * + * ## The bug this closes + * + * `AuthManagerOptions.additionalOrgRoles` registers app-supplied roles with + * better-auth's organization plugin so an invitation to `sales_rep` isn't + * rejected with `ROLE_NOT_FOUND`. better-auth then accepted the role — and the + * platform objects rejected it: + * + * ``` + * POST /organization/invite-member { role: 'sales_rep' } → passes better-auth + * insert sys_invitation { role: 'sales_rep' } → ValidationError: + * role must be one of: owner, admin, member + * ``` + * + * `sys_invitation.role` and `sys_member.role` are closed `select`s, and a + * select is ENFORCED on write. better-auth's own inserts are not exempt — its + * writes run through the ordinary ObjectQL validator, which sits after the + * security middleware, so `isSystem` context buys nothing. Every deployment + * whose stack declared `permission` / `position` names was registering roles + * that could be requested and never stored: `declared ≠ enforced` (Prime + * Directive #10), one layer below the declaration. + * + * ## The shape of the fix (Prime Directive #12 — fix the producer) + * + * Not "open the two fields to free text" (that would make better-auth's + * registry the only authority and drop both the picker's option list and the + * write-side guardrail), and not "lint that the two lists agree" (that + * surfaces the mismatch without making app roles work). Instead: **one list, + * materialized into both consumers**. + * + * - {@link normalizeAdditionalOrgRoles} is the one normalizer. Everything + * downstream consumes its output, never a raw caller-supplied array. + * - {@link membershipRoleOptions} turns that array into the `select` options. + * - {@link withMembershipRoleOptions} stamps those options onto + * `sys_invitation` / `sys_member` as `AuthPlugin` registers its manifest. + * - `auth-manager.ts` builds better-auth's `roles` map from the SAME + * normalized array. + * + * Neither side keeps a list of its own, so neither can accept a name the other + * rejects. The static options in `@objectstack/platform-objects` are the + * built-in baseline (`BUILTIN_MEMBERSHIP_ROLE_OPTIONS`) and nothing else. + * + * ## Why names are filtered rather than passed through + * + * `Field.select` lowercases option values and strips everything outside + * `[a-z0-9_]`. A stack-declared name like `showcase.export_data` would be + * registered with better-auth verbatim and stored as `showcaseexport_data` — + * the two lists would agree on the name and still disagree on the value, which + * is the same bug with extra steps. So a name that cannot round-trip is + * dropped from BOTH sides with a warning: better-auth never learns it, so the + * invitation fails loudly at the door (`ROLE_NOT_FOUND`) instead of at the + * insert. Every `permission` / `position` name that passes its own + * `SnakeCaseIdentifierSchema` already satisfies this, so a spec-compliant + * stack loses nothing. + * + * ## What registering a role does NOT grant + * + * A registered role is an opaque string to better-auth — it gets the built-in + * `member` access-control statements, never org-admin capability. It does + * project into `current_user.positions` via `mapMembershipRole`, so a + * `sys_position_permission_set` binding of the same name resolves permission + * sets; that is the intended channel (it is why apps declare these roles), and + * it is capped on the issuing side by `invitation-role-cap.ts` — an issuer + * below admin grade may invite as plain `member` only. + */ + +import { + BUILTIN_MEMBERSHIP_ROLES, + BUILTIN_MEMBERSHIP_ROLE_OPTIONS, + MEMBERSHIP_ROLE_NAME_MIN_LENGTH, + MEMBERSHIP_ROLE_NAME_PATTERN, +} from '@objectstack/spec/identity'; + +/** The two better-auth-managed objects whose `role` select is enforced on write. */ +export const MEMBERSHIP_ROLE_OBJECTS = ['sys_invitation', 'sys_member'] as const; + +/** Field carrying the membership role on each of {@link MEMBERSHIP_ROLE_OBJECTS}. */ +const ROLE_FIELD = 'role'; + +export interface MembershipRoleOption { + label: string; + value: string; +} + +/** Minimal logger surface — `ctx.logger`, `console`, or nothing at all. */ +export interface OrgRoleLogger { + warn?(message: string): void; +} + +const BUILTIN_SET: ReadonlySet = new Set(BUILTIN_MEMBERSHIP_ROLES); + +/** `sales_rep` → `Sales Rep`. Display only; the value is always the raw name. */ +export function membershipRoleLabel(name: string): string { + return name + .split('_') + .filter((word) => word.length > 0) + .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) + .join(' '); +} + +/** + * The one normalizer for app-supplied organization roles. + * + * Trims, drops non-strings, drops the built-ins (an app cannot redefine + * `owner`/`admin`/`member`/`delegated_admin` — silently downgrading `owner` to + * a plain member role would 403 every org mutation), de-duplicates, and + * refuses names that cannot survive `Field.select`'s value normalization. + * Order is preserved so the resulting picker is stable across boots. + * + * @param logger optional sink for the "dropped an unusable role name" warning + */ +export function normalizeAdditionalOrgRoles( + input: readonly unknown[] | undefined | null, + logger?: OrgRoleLogger, +): string[] { + if (!Array.isArray(input)) return []; + const out: string[] = []; + const seen = new Set(); + for (const raw of input) { + if (typeof raw !== 'string') continue; + const name = raw.trim(); + if (!name) continue; + if (BUILTIN_SET.has(name)) continue; + if (seen.has(name)) continue; + if (name.length < MEMBERSHIP_ROLE_NAME_MIN_LENGTH || !MEMBERSHIP_ROLE_NAME_PATTERN.test(name)) { + // Loud, not silent: the name is refused on BOTH sides, so an invitation + // to it fails at better-auth's door rather than at the insert — but the + // operator still needs to know their stack declared something unusable. + logger?.warn?.( + `[auth] organization role '${name}' is not a valid machine name ` + + `(lowercase snake_case, /^[a-z][a-z0-9_]*$/) — not registered. ` + + `A name outside that shape cannot be stored on sys_invitation.role / ` + + `sys_member.role, so registering it would accept invitations that can never be written (#3723).`, + ); + continue; + } + seen.add(name); + out.push(name); + } + return out; +} + +/** + * `select` options for the built-in roles plus `additional` — the exact value + * set that must be storable on `sys_invitation.role` / `sys_member.role`. + * + * Pass an ALREADY-normalized array (the output of + * {@link normalizeAdditionalOrgRoles}); normalizing again here is harmless but + * the point of the single normalizer is that callers share one result. + */ +export function membershipRoleOptions(additional: readonly string[] = []): MembershipRoleOption[] { + return [ + ...BUILTIN_MEMBERSHIP_ROLE_OPTIONS.map((o) => ({ ...o })), + ...additional + .filter((name) => !BUILTIN_SET.has(name)) + .map((name) => ({ label: membershipRoleLabel(name), value: name })), + ]; +} + +/** + * Return `objects` with the membership-role selects widened to the full role + * set — the materialization step that keeps the option lists in step with + * better-auth's registry. + * + * Copy-on-write: the input definitions are module-level singletons shared by + * every kernel in the process (and by the compile-time + * `objectstack.config.ts`), so only the touched object / `fields` map / `role` + * field are cloned. Two kernels booted with different app roles in one test + * run must not see each other's options. + * + * A no-op when the stack declares no extra roles — the overwhelmingly common + * case gets the identical array back. + */ +export function withMembershipRoleOptions( + objects: readonly T[], + additional: readonly string[] = [], +): T[] { + const extras = additional.filter((name) => !BUILTIN_SET.has(name)); + if (extras.length === 0) return [...objects]; + + const options = membershipRoleOptions(extras); + return objects.map((object) => { + const def = object as unknown as { name?: unknown; fields?: Record }; + if (typeof def?.name !== 'string') return object; + if (!(MEMBERSHIP_ROLE_OBJECTS as readonly string[]).includes(def.name)) return object; + const field = def.fields?.[ROLE_FIELD] as { type?: unknown } | undefined; + // Defensive: if the field ever stops being a select, widening its options + // would be meaningless — leave it exactly as authored rather than invent a + // shape the validator does not read. + if (!field || (field.type !== 'select' && field.type !== 'radio')) return object; + return { + ...(object as object), + fields: { + ...def.fields, + [ROLE_FIELD]: { ...field, options: options.map((o) => ({ ...o })) }, + }, + } as T; + }); +} + +/** + * Collect the organization roles a loaded stack declares. + * + * The producer side of the same one list: `objectstack serve`, the + * `@objectstack/verify` boot harness and any embedder all derive + * `additionalOrgRoles` HERE rather than each re-implementing the walk — the + * harness not doing so is why #3722's unit tests passed while the real HTTP + * route failed. + * + * Sources (better-auth boundary keeps the word "roles"; ADR-0090 D3 exception): + * - top-level `positions[]` — flat distribution groups + * - top-level `permissions[]` — PermissionSet / Profile names + * + * Real RBAC enforcement stays with SecurityPlugin; better-auth only needs to + * accept the names as opaque strings. + */ +export function collectStackOrgRoles(stack: unknown, logger?: OrgRoleLogger): string[] { + const names: unknown[] = []; + const collect = (arr: unknown) => { + if (!Array.isArray(arr)) return; + for (const entry of arr) { + if (typeof entry === 'string') names.push(entry); + else if (entry && typeof (entry as { name?: unknown }).name === 'string') { + names.push((entry as { name: string }).name); + } + } + }; + try { + const config = (stack ?? {}) as { positions?: unknown; permissions?: unknown }; + collect(config.positions); + collect(config.permissions); + } catch { + // Best-effort: a malformed stack must not stop the server from booting. + return []; + } + return normalizeAdditionalOrgRoles(names, logger); +} diff --git a/packages/qa/dogfood/test/app-org-role-invite.dogfood.test.ts b/packages/qa/dogfood/test/app-org-role-invite.dogfood.test.ts new file mode 100644 index 0000000000..09961c8288 --- /dev/null +++ b/packages/qa/dogfood/test/app-org-role-invite.dogfood.test.ts @@ -0,0 +1,194 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#3723] An app-declared organization role is usable end to end — proven over + * the real HTTP route, because no unit test could have caught this. + * + * The bug: `additionalOrgRoles` registered every `permission` / `position` name + * a stack declared with better-auth's organization plugin, so + * `POST /organization/invite-member { role: 'contributor' }` passed the role + * check — and then the `sys_invitation` insert failed, because + * `sys_invitation.role` and `sys_member.role` were closed selects listing only + * `owner|admin|member`. Two lists that had to agree, with nothing keeping them + * in step: `declared ≠ enforced` (Prime Directive #10) one layer below the + * declaration. + * + * ``` + * ValidationError: role must be one of: owner, admin, member + * { field: 'role', code: 'invalid_option', options: ['owner','admin','member'] } + * ``` + * + * The fix folds both consumers into one normalized list (see + * `plugin-auth/src/org-roles.ts`). This file is the gate on the whole chain: + * the showcase stack declares `contributor` as a `position` and + * `showcase_manager` as a `permission`, both of which travel + * stack → `collectStackOrgRoles` → better-auth registry AND the two selects. + * + * Why here and not a unit test: #3722's unit tests proved the roles map was + * built correctly and still shipped an unusable role — the failure only + * appears when the real route drives a real insert through the ObjectQL + * validator. Twice, in fact, once per object, which is why both are asserted. + * + * Harness note (also #3723): `bootStack` now derives `additionalOrgRoles` from + * the stack exactly as `objectstack serve` does. Before that it passed none, so + * a dogfood proof booted a stack whose declared roles better-auth had never + * heard of — the one surface that drives the real route was blind to this + * class of bug. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import showcaseStack from '@objectstack/example-showcase'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; + +const SYSTEM_CTX = { isSystem: true }; + +/** A `position` the showcase stack declares — an app role by the position route. */ +const APP_ROLE_POSITION = 'contributor'; +/** A `permission` (PermissionSet) the showcase stack declares — the other route. */ +const APP_ROLE_PERMISSION_SET = 'showcase_manager'; + +async function findRows(ql: any, object: string, where: any, limit = 50): Promise { + const rows = await ql.find(object, { where, limit }, { context: SYSTEM_CTX }); + return Array.isArray(rows) ? rows : (rows?.records ?? []); +} + +async function waitForMembership(ql: any, userId: string): Promise { + for (let i = 0; i < 40; i++) { + const rows = await findRows(ql, 'sys_member', { user_id: userId }, 5); + if (rows.length > 0) return rows[0]; + await new Promise((r) => setTimeout(r, 250)); + } + throw new Error(`no sys_member row appeared for ${userId}`); +} + +describe('#3723: an app-declared org role can actually be invited and held', () => { + let stack: VerifyStack; + let ql: any; + let orgId: string; + let ownerToken: string; + + beforeAll(async () => { + stack = await bootStack(showcaseStack, {}); + ownerToken = await stack.signIn(); // the seeded dev admin + ql = await stack.kernel.getServiceAsync('objectql'); + + // `bootStack` disables the default-org bootstrap, so mint the org the way + // the bootstrap would (system context — the only writer better-auth-managed + // tables accept, ADR-0092) and bind the dev admin as its owner. + const org = await ql.insert( + 'sys_organization', + { name: 'Default Organization', slug: 'default' }, + { context: SYSTEM_CTX }, + ); + orgId = String(org.id); + + const [adminUser] = await findRows(ql, 'sys_user', { email: 'admin@objectos.ai' }, 1); + const adminMembers = await findRows(ql, 'sys_member', { user_id: adminUser.id }, 5); + if (adminMembers.length > 0) { + await ql.update( + 'sys_member', + { id: adminMembers[0].id, organization_id: orgId, role: 'owner' }, + { context: SYSTEM_CTX }, + ); + } else { + await ql.insert( + 'sys_member', + { user_id: adminUser.id, organization_id: orgId, role: 'owner' }, + { context: SYSTEM_CTX }, + ); + } + }, 180_000); + + afterAll(async () => { + await stack?.stop?.(); + }); + + it('both role selects offer the stack-declared roles — one list, two consumers', async () => { + // The registered schema is what the write-path validator reads AND what the + // Setup picker renders, so this is both halves of "the option list agrees + // with better-auth's registry". + const optionValues = async (object: string) => { + const schema = await ql.getSchema(object); + return (schema.fields.role.options ?? []).map((o: any) => o.value); + }; + + const invitationRoles = await optionValues('sys_invitation'); + const memberRoles = await optionValues('sys_member'); + + // The built-ins are never dropped — better-auth's `hasPermission` spreads + // our map over its defaults, so losing `owner` would 403 every mutation. + expect(invitationRoles).toEqual(expect.arrayContaining(['owner', 'admin', 'member'])); + // …and the app's own roles are there, from both declaration routes. + expect(invitationRoles).toContain(APP_ROLE_POSITION); + expect(invitationRoles).toContain(APP_ROLE_PERMISSION_SET); + // Fixing one object and not the other is exactly how #3722 failed twice. + expect(memberRoles).toEqual(invitationRoles); + }, 30_000); + + it('inviting with an app-declared role SUCCEEDS — this is the reported failure', async () => { + // Before the fix: 200 from better-auth's role check, then the sys_invitation + // insert threw `role must be one of: owner, admin, member`. + const email = 'invitee.contributor.3723@example.com'; + const res = await stack.apiAs(ownerToken, 'POST', '/auth/organization/invite-member', { + email, + role: APP_ROLE_POSITION, + organizationId: orgId, + }); + + expect(res.status, await res.clone().text()).toBe(200); + const rows = await findRows(ql, 'sys_invitation', { email }, 5); + expect(rows.length).toBe(1); + expect(rows[0].role).toBe(APP_ROLE_POSITION); + }, 30_000); + + it('a PermissionSet name works too — both collection routes reach the same list', async () => { + const email = 'invitee.manager.3723@example.com'; + const res = await stack.apiAs(ownerToken, 'POST', '/auth/organization/invite-member', { + email, + role: APP_ROLE_PERMISSION_SET, + organizationId: orgId, + }); + + expect(res.status, await res.clone().text()).toBe(200); + const rows = await findRows(ql, 'sys_invitation', { email }, 5); + expect(rows[0].role).toBe(APP_ROLE_PERMISSION_SET); + }, 30_000); + + it('the membership row accepts the same role — the second enforced select', async () => { + // The value the invitation lands on `sys_member` as. Written here through + // ObjectQL rather than by accepting the invitation because the signup + // reconciler has already bound this user to the org (better-auth refuses a + // second membership), but it is the SAME validator on the same enforced + // select that better-auth's acceptance insert goes through — system context + // does not exempt it, which is the whole reason the bug existed. + const token = await stack.signUp('holder.3723@example.com', 'Holder!Pass123', 'Holder 3723'); + expect(token).toBeTruthy(); + const [user] = await findRows(ql, 'sys_user', { email: 'holder.3723@example.com' }, 1); + const membership = await waitForMembership(ql, String(user.id)); + + await ql.update( + 'sys_member', + { id: membership.id, organization_id: orgId, role: APP_ROLE_POSITION }, + { context: SYSTEM_CTX }, + ); + + const [updated] = await findRows(ql, 'sys_member', { id: membership.id }, 1); + expect(updated.role).toBe(APP_ROLE_POSITION); + }, 60_000); + + it('a role the stack never declared is still refused — the fields did not just open up', async () => { + // Option 1 in the issue (make both fields free text) would have made this + // pass. It must not: the write-side guardrail and the picker's option list + // are the reason to keep a closed select, and better-auth's registry is + // still the gate at the door. + const email = 'invitee.undeclared.3723@example.com'; + const res = await stack.apiAs(ownerToken, 'POST', '/auth/organization/invite-member', { + email, + role: 'not_a_declared_role', + organizationId: orgId, + }); + + expect(res.status).not.toBe(200); + expect((await findRows(ql, 'sys_invitation', { email }, 5)).length).toBe(0); + }, 30_000); +}); diff --git a/packages/spec/src/identity/eval-user.zod.ts b/packages/spec/src/identity/eval-user.zod.ts index 69318fecb3..7a1656e1dd 100644 --- a/packages/spec/src/identity/eval-user.zod.ts +++ b/packages/spec/src/identity/eval-user.zod.ts @@ -102,30 +102,6 @@ export const BUILTIN_IDENTITY_METADATA: Record[] = [ + { label: 'Owner', value: MEMBERSHIP_ROLE_OWNER }, + { label: 'Admin', value: MEMBERSHIP_ROLE_ADMIN }, + { label: 'Delegated Admin', value: MEMBERSHIP_ROLE_DELEGATED_ADMIN }, + { label: 'Member', value: MEMBERSHIP_ROLE_MEMBER }, +] as const; + +/** + * The shape a role name must have to survive BOTH gatekeepers unchanged. + * + * Identical to `SnakeCaseIdentifierSchema`'s pattern, which every `permission` + * / `position` name already satisfies — and deliberately so: `Field.select` + * lowercases option values and strips anything outside `[a-z0-9_]`, so a name + * like `showcase.export_data` would be registered with better-auth verbatim + * and stored as `showcaseexport_data`. The two lists would agree on the *name* + * and still disagree on the *value*. Names that cannot round-trip are refused + * by `normalizeAdditionalOrgRoles` on both sides rather than half-accepted. + */ +export const MEMBERSHIP_ROLE_NAME_PATTERN = /^[a-z][a-z0-9_]*$/; + +/** Minimum length, matching `SnakeCaseIdentifierSchema`. */ +export const MEMBERSHIP_ROLE_NAME_MIN_LENGTH = 2; diff --git a/packages/spec/src/index.ts b/packages/spec/src/index.ts index 38469789b9..dca367704e 100644 --- a/packages/spec/src/index.ts +++ b/packages/spec/src/index.ts @@ -166,10 +166,23 @@ export { BUILTIN_IDENTITY_ORG_OWNER, BUILTIN_IDENTITY_ORG_ADMIN, BUILTIN_IDENTITY_ORG_MEMBER, - MEMBERSHIP_ROLE_DELEGATED_ADMIN, ADMIN_FULL_ACCESS, ORGANIZATION_ADMIN, ORGANIZATION_ADMIN_NO_BYPASS, ORGANIZATION_ADMIN_GRANTS, } from './identity/eval-user.zod'; export type { EvalUser, EvalUserInput, BuiltinIdentityName } from './identity/eval-user.zod'; + +// #3723: organization membership roles — the ONE list read by better-auth's +// role registry AND the `sys_invitation`/`sys_member` role selects. +export { + MEMBERSHIP_ROLE_OWNER, + MEMBERSHIP_ROLE_ADMIN, + MEMBERSHIP_ROLE_MEMBER, + MEMBERSHIP_ROLE_DELEGATED_ADMIN, + BUILTIN_MEMBERSHIP_ROLES, + BUILTIN_MEMBERSHIP_ROLE_OPTIONS, + MEMBERSHIP_ROLE_NAME_PATTERN, + MEMBERSHIP_ROLE_NAME_MIN_LENGTH, +} from './identity/membership-role'; +export type { BuiltinMembershipRole } from './identity/membership-role'; diff --git a/packages/verify/src/harness.ts b/packages/verify/src/harness.ts index f3d04cc153..72056ffeb9 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 } from '@objectstack/plugin-auth'; +import { AuthPlugin, collectStackOrgRoles } from '@objectstack/plugin-auth'; import { SecurityPlugin } from '@objectstack/plugin-security'; import { SharingServicePlugin } from '@objectstack/plugin-sharing'; import { SettingsServicePlugin, LocalCryptoProvider } from '@objectstack/service-settings'; @@ -181,7 +181,17 @@ export async function bootStack( // single-tenant baseline these dogfood proofs assert (ADR-0057 identity // create, ADR-0062 federation, ADR-0086 two-doors). The bootstrap itself is // covered by plugin-auth unit tests + browser E2E. - await kernel.use(new AuthPlugin({ secret: opts.authSecret ?? DEFAULT_AUTH_SECRET, autoDefaultOrganization: false })); + // + // [#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. + await kernel.use(new AuthPlugin({ + secret: opts.authSecret ?? DEFAULT_AUTH_SECRET, + autoDefaultOrganization: false, + additionalOrgRoles: collectStackOrgRoles(config), + })); // ADR-0062 — datasource connection service (registers 'datasource-connection'), // mirroring `objectstack dev`/serve. Without it, AppPlugin's declared-datasource From 4111bdffe668eec63fdf515efc418df7bb9b1fcd Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 01:40:27 +0000 Subject: [PATCH 2/6] docs(permissions): positions are membership roles too (#3723) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The positions page said `sys_member.role` was the "org-membership tier: owner/admin/member". That parenthetical was already one value short (`delegated_admin`, #3697) and this branch makes it materially wrong: every declared position / permission-set name is now a valid `sys_member.role` value, so someone can be invited straight into a position. Documents that path where the names are declared, with the two limits that bound it — the invitation role cap (an issuer below admin grade invites plain members only; a delegate's channel for capability is placement, not the membership role) and the machine-name requirement. --- content/docs/permissions/positions.mdx | 36 +++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/content/docs/permissions/positions.mdx b/content/docs/permissions/positions.mdx index 102537787d..eaa90a290d 100644 --- a/content/docs/permissions/positions.mdx +++ b/content/docs/permissions/positions.mdx @@ -16,7 +16,11 @@ of their own** — they only decide *who gets which sets*. > security identifiers and labels). The vocabulary is now unambiguous: > **permission set** = capability, **position** = distribution, > **business unit** = hierarchy. The single exception is better-auth's -> internal `sys_member.role` (org-membership tier: owner/admin/member). +> internal `sys_member.role` (org-membership tier: `owner`, `admin`, +> `delegated_admin`, `member`). A position name you declare here is also +> registered as a valid `sys_member.role` value, so someone can be invited +> directly into the position — see [Positions as membership +> roles](#positions-as-membership-roles). ```typescript import type { Position } from '@objectstack/spec/identity'; @@ -86,6 +90,36 @@ is no singular `current_user.position` at runtime). Framework-seeded names: Test membership in RLS/CEL with `'org_admin' in current_user.positions`. +## Positions as membership roles + +Every position and permission-set name your stack declares is registered as a +valid organization role, so an invitation can name it directly: + +```http +POST /api/v1/auth/organization/invite-member +{ "email": "new.hire@example.com", "role": "sales_rep" } +``` + +On acceptance the value lands in `sys_member.role`, and the membership +projection passes an unrecognized value straight through as a position name — +so the invitee holds `sales_rep` in `current_user.positions` and resolves +whatever permission sets are bound to it. Both `sys_invitation.role` and +`sys_member.role` are pickers built from that same registered set, so the +Setup UI offers your names alongside the built-ins. + +Two limits are worth knowing before you use this as a provisioning path: + +- **It is not a way to hand out more authority than you have.** An issuer + below `admin` grade may invite only as plain `member`, and no invitation may + ever confer a role above the issuer's own. A delegated admin grants + capability through the invitation's *placement* (business unit + positions), + which is authorized against their `adminScope` — not through the membership + role, which nothing scopes. +- **Only spec-compliant names are registered.** A declared name that is not + lowercase snake_case (`/^[a-z][a-z0-9_]*$/`, min 2 characters) is skipped + with a boot warning, and an invitation naming it is refused with + `ROLE_NOT_FOUND`. + ## Audience anchors: `everyone` and `guest` (ADR-0090 D5/D9) Two more built-in positions are **implicit** — they are never stored as From a39cb6c27348337559b3af7a4cd9794cdc78c2a7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 01:44:11 +0000 Subject: [PATCH 3/6] docs(permissions): move the membership-tier note to the better-auth boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit put an "app roles" section on the positions page, which tripped `check:role-word` — the ADR-0090 D3 ratchet — by adding 11 uses of the reserved word to the one page whose thesis is that positions are NOT roles. The ratchet was right; raising its baseline there would have spent exactly what D3 bought. `positions.mdx` keeps only the factual correction to the tier list (the parenthetical was already missing `delegated_admin`, and app-declared names now belong in it), phrased to leave the file's word count unchanged at 6. The substance moves to `authentication.mdx` under Organizations — the better-auth boundary page, where `sys_member.role` is the sanctioned exception. Baseline for that file ratcheted 2 → 5: two `sys_member.role` mentions and the literal `"role"` JSON field in the invite sample. --- content/docs/permissions/authentication.mdx | 34 ++++++++++++++++++ content/docs/permissions/positions.mdx | 39 +++------------------ scripts/role-word-baseline.json | 2 +- 3 files changed, 39 insertions(+), 36 deletions(-) diff --git a/content/docs/permissions/authentication.mdx b/content/docs/permissions/authentication.mdx index 68a48ef78a..4124fd7ed9 100644 --- a/content/docs/permissions/authentication.mdx +++ b/content/docs/permissions/authentication.mdx @@ -668,6 +668,40 @@ new AuthPlugin({ }) ``` +#### Positions as membership tiers + +`sys_member.role` is better-auth's own column and the one place ObjectStack +keeps that vocabulary (ADR-0090 D3's single boundary exception). Beyond the +four built-in tiers — `owner`, `admin`, `delegated_admin`, `member` — every +[position](/docs/permissions/positions) and +[permission set](/docs/permissions/permission-sets) your stack declares is +registered as a valid value, so an invitation can name one directly: + +```http +POST /api/v1/auth/organization/invite-member +{ "email": "new.hire@example.com", "role": "sales_rep" } +``` + +On acceptance the value lands in `sys_member.role`, and the membership +projection passes an unrecognized value through as a position name — so the +invitee holds `sales_rep` in `current_user.positions` and resolves whatever +permission sets are bound to it. Both `sys_invitation` and `sys_member` build +their pickers from that same registered set, so the Setup UI offers your names +alongside the built-ins. + +Two limits bound this as a provisioning path: + +- **It never hands out more authority than the issuer has.** An issuer below + `admin` grade may invite as plain `member` only, and no invitation may confer + a tier above the issuer's own. A delegated admin grants capability through + the invitation's *placement* (business unit + positions), authorized against + their `adminScope` — not through the membership tier, which nothing scopes. +- **Only spec-compliant names are registered.** A declared name that is not + lowercase snake_case (`/^[a-z][a-z0-9_]*$/`, minimum 2 characters) is skipped + with a boot warning, and an invitation naming it is refused with + `ROLE_NOT_FOUND` — better-auth never learns a name the write path could not + store. + ### Admin User Management With `plugins: { admin: true }` (forced on when SCIM is enabled), platform diff --git a/content/docs/permissions/positions.mdx b/content/docs/permissions/positions.mdx index eaa90a290d..42f5677d22 100644 --- a/content/docs/permissions/positions.mdx +++ b/content/docs/permissions/positions.mdx @@ -16,11 +16,10 @@ of their own** — they only decide *who gets which sets*. > security identifiers and labels). The vocabulary is now unambiguous: > **permission set** = capability, **position** = distribution, > **business unit** = hierarchy. The single exception is better-auth's -> internal `sys_member.role` (org-membership tier: `owner`, `admin`, -> `delegated_admin`, `member`). A position name you declare here is also -> registered as a valid `sys_member.role` value, so someone can be invited -> directly into the position — see [Positions as membership -> roles](#positions-as-membership-roles). +> internal `sys_member.role` — the org-membership tier (`owner`, `admin`, +> `delegated_admin`, `member`, **plus every position name your stack +> declares**, so a person can be invited straight into a position; see +> [Authentication](/docs/permissions/authentication#positions-as-membership-tiers)). ```typescript import type { Position } from '@objectstack/spec/identity'; @@ -90,36 +89,6 @@ is no singular `current_user.position` at runtime). Framework-seeded names: Test membership in RLS/CEL with `'org_admin' in current_user.positions`. -## Positions as membership roles - -Every position and permission-set name your stack declares is registered as a -valid organization role, so an invitation can name it directly: - -```http -POST /api/v1/auth/organization/invite-member -{ "email": "new.hire@example.com", "role": "sales_rep" } -``` - -On acceptance the value lands in `sys_member.role`, and the membership -projection passes an unrecognized value straight through as a position name — -so the invitee holds `sales_rep` in `current_user.positions` and resolves -whatever permission sets are bound to it. Both `sys_invitation.role` and -`sys_member.role` are pickers built from that same registered set, so the -Setup UI offers your names alongside the built-ins. - -Two limits are worth knowing before you use this as a provisioning path: - -- **It is not a way to hand out more authority than you have.** An issuer - below `admin` grade may invite only as plain `member`, and no invitation may - ever confer a role above the issuer's own. A delegated admin grants - capability through the invitation's *placement* (business unit + positions), - which is authorized against their `adminScope` — not through the membership - role, which nothing scopes. -- **Only spec-compliant names are registered.** A declared name that is not - lowercase snake_case (`/^[a-z][a-z0-9_]*$/`, min 2 characters) is skipped - with a boot warning, and an invitation naming it is refused with - `ROLE_NOT_FOUND`. - ## Audience anchors: `everyone` and `guest` (ADR-0090 D5/D9) Two more built-in positions are **implicit** — they are never stored as diff --git a/scripts/role-word-baseline.json b/scripts/role-word-baseline.json index 8a6ce08440..7be5ed53e5 100644 --- a/scripts/role-word-baseline.json +++ b/scripts/role-word-baseline.json @@ -13,7 +13,7 @@ "content/docs/kernel/contracts/data-engine.mdx": 1, "content/docs/kernel/events.mdx": 1, "content/docs/kernel/services-checklist.mdx": 4, - "content/docs/permissions/authentication.mdx": 2, + "content/docs/permissions/authentication.mdx": 5, "content/docs/permissions/authorization.mdx": 3, "content/docs/permissions/delegated-administration.mdx": 11, "content/docs/permissions/index.mdx": 1, From cb8434eceb1120c9bafb5e82af48e5ad3b12ddad Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 01:56:06 +0000 Subject: [PATCH 4/6] chore(spec): regenerate api-surface snapshot for the membership-role exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0059's public-API ratchet: an ADDED export still requires regenerating, so every change to the spec's third-party surface is deliberate. 16 additions across `.` and `./identity`, zero removals or narrowed signatures — and `MEMBERSHIP_ROLE_DELEGATED_ADMIN` stays present on both entry points, which is the snapshot confirming its move from `identity/eval-user.zod` to `identity/membership-role` is invisible to consumers. --- packages/spec/api-surface.json | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 37adc1b522..ce27360c34 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -11,7 +11,10 @@ "BUILTIN_IDENTITY_ORG_MEMBER (const)", "BUILTIN_IDENTITY_ORG_OWNER (const)", "BUILTIN_IDENTITY_PLATFORM_ADMIN (const)", + "BUILTIN_MEMBERSHIP_ROLES (const)", + "BUILTIN_MEMBERSHIP_ROLE_OPTIONS (const)", "BuiltinIdentityName (type)", + "BuiltinMembershipRole (type)", "CONVERSIONS_BY_MAJOR (const)", "CONVERSION_CONFLICT_CODE (const)", "CONVERSION_NOTICE_CODE (const)", @@ -48,7 +51,12 @@ "F (const)", "GUEST_POSITION (const)", "MAP_SUPPORTED_FIELDS (const)", + "MEMBERSHIP_ROLE_ADMIN (const)", "MEMBERSHIP_ROLE_DELEGATED_ADMIN (const)", + "MEMBERSHIP_ROLE_MEMBER (const)", + "MEMBERSHIP_ROLE_NAME_MIN_LENGTH (const)", + "MEMBERSHIP_ROLE_NAME_PATTERN (const)", + "MEMBERSHIP_ROLE_OWNER (const)", "METADATA_ALIASES (const)", "MIGRATIONS_BY_MAJOR (const)", "MIGRATION_MAJORS (const)", @@ -4235,7 +4243,10 @@ "BUILTIN_IDENTITY_ORG_MEMBER (const)", "BUILTIN_IDENTITY_ORG_OWNER (const)", "BUILTIN_IDENTITY_PLATFORM_ADMIN (const)", + "BUILTIN_MEMBERSHIP_ROLES (const)", + "BUILTIN_MEMBERSHIP_ROLE_OPTIONS (const)", "BuiltinIdentityName (type)", + "BuiltinMembershipRole (type)", "EVERYONE_POSITION (const)", "EvalUser (type)", "EvalUserInput (type)", @@ -4244,7 +4255,12 @@ "Invitation (type)", "InvitationSchema (const)", "InvitationStatus (type)", + "MEMBERSHIP_ROLE_ADMIN (const)", "MEMBERSHIP_ROLE_DELEGATED_ADMIN (const)", + "MEMBERSHIP_ROLE_MEMBER (const)", + "MEMBERSHIP_ROLE_NAME_MIN_LENGTH (const)", + "MEMBERSHIP_ROLE_NAME_PATTERN (const)", + "MEMBERSHIP_ROLE_OWNER (const)", "Member (type)", "MemberSchema (const)", "ORGANIZATION_ADMIN (const)", From 072f8a28091f8d4238aede35c241cfbd4b8fa410 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 02:12:31 +0000 Subject: [PATCH 5/6] fix(dev): DevPlugin derives org roles from its stack too (#3723) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fourth host that boots AuthPlugin from a loaded stack, and the last one still walking past `additionalOrgRoles`. Unlike the pre-fix `serve` path this was not a mismatch — passing nothing left better-auth and the selects agreeing on the built-ins — but DevPlugin documents itself as equivalent to assembling the full stack by hand, and that equivalence quietly excluded every app-declared organization role. Now uses the same `collectStackOrgRoles(this.options.stack)` as `serve` and the verify harness. Guarded on the export being present: plugin-auth is a dynamic optional import here, so an older copy on disk yields none rather than throwing. --- packages/plugins/plugin-dev/src/dev-plugin.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-dev/src/dev-plugin.ts b/packages/plugins/plugin-dev/src/dev-plugin.ts index 850c1a1c9c..9e5dc63152 100644 --- a/packages/plugins/plugin-dev/src/dev-plugin.ts +++ b/packages/plugins/plugin-dev/src/dev-plugin.ts @@ -522,10 +522,21 @@ export class DevPlugin implements Plugin { let authMounted = false; if (enabled('auth')) { try { - const { AuthPlugin } = await import('@objectstack/plugin-auth') as any; + 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 = new AuthPlugin({ secret: this.options.authSecret, baseUrl: this.options.authBaseUrl, + ...(additionalOrgRoles.length > 0 ? { additionalOrgRoles } : {}), }); this.childPlugins.push(authPlugin); authMounted = true; From d404ae555ebc6f8ae372d25be3a2c4897fa410d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 02:28:19 +0000 Subject: [PATCH 6/6] fix(auth): the role picker shows the declared label, not a title-cased name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deriving the option label by title-casing the machine name made a THIRD source of truth for one string: a position declaring `{ name: 'exec', label: 'Executive' }` rendered as "Exec" in the picker, contradicting the very metadata it came from. Worse for a non-English stack — a position labelled `销售代表` came out as "Sales Rep". Same one-list principle as the role set itself, applied to how it is displayed: `collectStackOrgRoles` carries each entry's declared label through, and `additionalOrgRoles` accepts `{ name, label }` alongside a bare name (a widening — `string[]` callers are unaffected). Title-casing survives only as the fallback for a host that had no label to offer. Presentation only: better-auth is handed names via `orgRoleNames`, and the stored value is always the name. Pinned in the dogfood gate against a real difference — showcase's `exec` declares `Executive`, which title-casing could never produce. --- .changeset/app-org-roles-storable.md | 16 +++- .../plugins/plugin-auth/src/auth-manager.ts | 20 ++-- .../plugins/plugin-auth/src/auth-plugin.ts | 4 +- .../plugins/plugin-auth/src/org-roles.test.ts | 78 +++++++++++---- packages/plugins/plugin-auth/src/org-roles.ts | 94 ++++++++++++++----- .../test/app-org-role-invite.dogfood.test.ts | 13 +++ 6 files changed, 173 insertions(+), 52 deletions(-) diff --git a/.changeset/app-org-roles-storable.md b/.changeset/app-org-roles-storable.md index 9d55fe4c8a..1e6296255d 100644 --- a/.changeset/app-org-roles-storable.md +++ b/.changeset/app-org-roles-storable.md @@ -43,9 +43,19 @@ New exports: Hosts that boot `AuthPlugin` from a loaded stack should derive `additionalOrgRoles` with `collectStackOrgRoles(stack)` rather than walking the -stack themselves — `objectstack serve` and the `@objectstack/verify` harness now -both do (the harness previously passed none, which is why a dogfood proof could -boot a stack whose declared roles better-auth had never heard of). +stack themselves — `objectstack serve`, the `@objectstack/verify` harness and +`DevPlugin` now all do. The harness previously passed none, which is why a +dogfood proof could boot a stack whose declared roles better-auth had never +heard of; `DevPlugin` documents itself as equivalent to the full stack and +silently excluded app roles from that equivalence. + +`additionalOrgRoles` accepts `{ name, label }` alongside a bare name, and +`collectStackOrgRoles` now returns those descriptors. The label is what the +declaring `position` / `permission` metadata already says, so the role picker +shows `Executive` for a position declared as such instead of title-casing the +machine name into `Exec` — a third source of truth for one string. Presentation +only: better-auth sees just the name, and the stored value is always the name. +Passing `string[]` keeps working unchanged. Behaviour change worth noting: a declared role name that is not a valid machine name (`/^[a-z][a-z0-9_]*$/`, min 2 chars) is no longer registered at all, with a diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 9cfed87e8b..ea51467888 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -21,7 +21,7 @@ import { import { MCP_OAUTH_SCOPES } from '@objectstack/spec/ai'; import { createObjectQLAdapterFactory, withSystemReadContext } from './objectql-adapter.js'; import { invitationRoleCapFailure, isPlainMemberInvitation } from './invitation-role-cap.js'; -import { normalizeAdditionalOrgRoles } from './org-roles.js'; +import { normalizeAdditionalOrgRoles, orgRoleNames, type OrgRoleInput } from './org-roles.js'; import { isPlaceholderEmail } from './placeholder-email.js'; import { reconcileMembership, type MembershipPolicy } from './reconcile-membership.js'; import type { TenancyService } from './tenancy-service.js'; @@ -404,12 +404,18 @@ export interface AuthManagerOptions extends Partial { * Better-Auth's `member` role) so it cannot inadvertently grant org-level * admin capabilities. * - * Typical source: the union of `permission` metadata names that have - * declared names, collected from the loaded stack at CLI boot. + * Typical source: `collectStackOrgRoles(stack)` — the one walk every host + * shares (`objectstack serve`, the verify harness, DevPlugin). * - * @example ['sales_rep', 'sales_manager', 'service_agent'] + * Accepts a bare name, or `{ name, label }` to carry the declaring + * metadata's own display label into the role picker (#3723) — without it the + * picker would title-case the machine name and contradict a position that + * already says `销售代表`. The label is presentation only: better-auth sees + * just the name, and the stored value is always the name. + * + * @example ['sales_rep', { name: 'sales_manager', label: '销售经理' }] */ - additionalOrgRoles?: string[]; + additionalOrgRoles?: OrgRoleInput[]; /** * Optional outbound email service used by better-auth callbacks @@ -1601,7 +1607,9 @@ export class AuthManager { ...stmts, invitation: ['create'], }); - for (const name of extra) { + // Names only — a descriptor's `label` is presentation for the role + // picker and means nothing to better-auth. + for (const name of orgRoleNames(extra)) { if (!name) continue; if (built[name]) continue; built[name] = defaultAc.newRole(stmts); diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 286f5f65ff..b5b5f76cc9 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -35,7 +35,7 @@ import { authIdentityObjects, authPluginManifestHeader, } from './manifest.js'; -import { normalizeAdditionalOrgRoles, withMembershipRoleOptions } from './org-roles.js'; +import { normalizeAdditionalOrgRoles, withMembershipRoleOptions, type OrgRoleInput } from './org-roles.js'; /** * Auth Plugin Options @@ -80,7 +80,7 @@ export interface AuthPluginOptions extends Partial { * ROLE_NOT_FOUND. Forwarded as-is to AuthManager. See * {@link AuthManagerOptions.additionalOrgRoles} for details. */ - additionalOrgRoles?: string[]; + additionalOrgRoles?: OrgRoleInput[]; /** * ADR-0081 D1 — single-org default-organization bootstrap. In single-org diff --git a/packages/plugins/plugin-auth/src/org-roles.test.ts b/packages/plugins/plugin-auth/src/org-roles.test.ts index a928a809a3..6a40fcaaac 100644 --- a/packages/plugins/plugin-auth/src/org-roles.test.ts +++ b/packages/plugins/plugin-auth/src/org-roles.test.ts @@ -29,30 +29,54 @@ const roleOptionsOf = (object: unknown): { label: string; value: string }[] => describe('#3723 normalizeAdditionalOrgRoles — the one normalizer', () => { it('keeps valid app role names, in declaration order', () => { expect(normalizeAdditionalOrgRoles(['sales_rep', 'sales_manager', 'service_agent'])).toEqual([ - 'sales_rep', - 'sales_manager', - 'service_agent', + { name: 'sales_rep' }, + { name: 'sales_manager' }, + { name: 'service_agent' }, ]); }); + it('carries a declared label through, and tolerates entries without one', () => { + expect( + normalizeAdditionalOrgRoles([{ name: 'sales_rep', label: '销售代表' }, 'ops']), + ).toEqual([{ name: 'sales_rep', label: '销售代表' }, { name: 'ops' }]); + }); + + it('drops a blank label rather than storing an empty picker entry', () => { + expect(normalizeAdditionalOrgRoles([{ name: 'ops', label: ' ' }])).toEqual([{ name: 'ops' }]); + }); + it('drops the built-ins — an app may not redefine owner/admin/member/delegated_admin', () => { // Silently downgrading `owner` to a plain-member role would strip its // `invitation:create` and 403 every org mutation. expect( normalizeAdditionalOrgRoles(['owner', 'admin', 'member', 'delegated_admin', 'sales_rep']), - ).toEqual(['sales_rep']); + ).toEqual([{ name: 'sales_rep' }]); }); it('de-duplicates and trims', () => { - expect(normalizeAdditionalOrgRoles([' sales_rep ', 'sales_rep', ''])).toEqual(['sales_rep']); + expect(normalizeAdditionalOrgRoles([' sales_rep ', 'sales_rep', ''])).toEqual([ + { name: 'sales_rep' }, + ]); }); - it('ignores non-strings and a non-array input', () => { - expect(normalizeAdditionalOrgRoles([null, 42, {}, 'sales_rep'] as unknown[])).toEqual(['sales_rep']); + it('ignores unusable entries and a non-array input', () => { + expect(normalizeAdditionalOrgRoles([null, 42, {}, { name: 7 }, 'sales_rep'] as unknown[])).toEqual([ + { name: 'sales_rep' }, + ]); expect(normalizeAdditionalOrgRoles(undefined)).toEqual([]); expect(normalizeAdditionalOrgRoles(null)).toEqual([]); }); + it('a blank label never masks the built-in drop or the name filter', () => { + expect( + normalizeAdditionalOrgRoles([ + { name: 'owner', label: 'Boss' }, + { name: 'bad.name', label: 'Bad' }, + { name: 'ok_role', label: 'OK' }, + ]), + ).toEqual([{ name: 'ok_role', label: 'OK' }]); + }); + it('REFUSES a name that cannot round-trip through Field.select — loudly', () => { // `Field.select` lowercases values and strips everything outside // [a-z0-9_], so `showcase.export_data` would be registered with @@ -64,7 +88,7 @@ describe('#3723 normalizeAdditionalOrgRoles — the one normalizer', () => { ['showcase.export_data', 'Sales Rep', '_leading', 'x', 'sales_rep'], { warn }, ), - ).toEqual(['sales_rep']); + ).toEqual([{ name: 'sales_rep' }]); expect(warn).toHaveBeenCalledTimes(4); expect(warn.mock.calls.map((c) => String(c[0])).join('\n')).toContain('showcase.export_data'); }); @@ -81,13 +105,13 @@ describe('#3723 membershipRoleOptions — the option list both selects get', () }); it('appends app roles after the built-ins, with humanized labels', () => { - const options = membershipRoleOptions(['sales_rep']); + const options = membershipRoleOptions([{ name: 'sales_rep' }]); expect(values(options)).toEqual(['owner', 'admin', 'delegated_admin', 'member', 'sales_rep']); expect(options.at(-1)).toEqual({ label: 'Sales Rep', value: 'sales_rep' }); }); it('never duplicates a built-in', () => { - expect(values(membershipRoleOptions(['member', 'sales_rep']))).toEqual([ + expect(values(membershipRoleOptions([{ name: 'member' }, { name: 'sales_rep' }]))).toEqual([ 'owner', 'admin', 'delegated_admin', @@ -99,11 +123,20 @@ describe('#3723 membershipRoleOptions — the option list both selects get', () it('humanizes labels without ever changing the stored value', () => { expect(membershipRoleLabel('sales_rep')).toBe('Sales Rep'); expect(membershipRoleLabel('ops')).toBe('Ops'); - expect(membershipRoleOptions(['field_ops_delegate']).at(-1)).toEqual({ + expect(membershipRoleOptions([{ name: 'field_ops_delegate' }]).at(-1)).toEqual({ label: 'Field Ops Delegate', value: 'field_ops_delegate', }); }); + + it('a declared label WINS over the title-cased machine name', () => { + // The whole point: a position that says `销售代表` must not be rendered as + // "Sales Rep" by a picker deriving its own label from the machine name. + expect(membershipRoleOptions([{ name: 'sales_rep', label: '销售代表' }]).at(-1)).toEqual({ + label: '销售代表', + value: 'sales_rep', + }); + }); }); describe('#3723 withMembershipRoleOptions — materializing onto the platform objects', () => { @@ -112,14 +145,14 @@ describe('#3723 withMembershipRoleOptions — materializing onto the platform ob it('widens BOTH role selects — fixing one leaves the other rejecting the same value', () => { // The lesson from #3722: `sys_member.role` alone still left // `sys_invitation.role` refusing the invitation at issuance. - const [, member, invitation] = withMembershipRoleOptions(objects, ['sales_rep']); + const [, member, invitation] = withMembershipRoleOptions(objects, [{ name: 'sales_rep' }]); expect(values(roleOptionsOf(member))).toContain('sales_rep'); expect(values(roleOptionsOf(invitation))).toContain('sales_rep'); expect(values(roleOptionsOf(member))).toEqual(values(roleOptionsOf(invitation))); }); it('leaves unrelated objects untouched, by identity', () => { - const [user] = withMembershipRoleOptions(objects, ['sales_rep']); + const [user] = withMembershipRoleOptions(objects, [{ name: 'sales_rep' }]); expect(user).toBe(SysUser); }); @@ -127,12 +160,12 @@ describe('#3723 withMembershipRoleOptions — materializing onto the platform ob // `authIdentityObjects` is a process-wide singleton also used by the // compile-time `objectstack.config.ts`; two kernels booted with different // app roles in one test run must not see each other's options. - withMembershipRoleOptions(objects, ['sales_rep']); + withMembershipRoleOptions(objects, [{ name: 'sales_rep' }]); expect(values(roleOptionsOf(SysMember))).toEqual(values(BUILTIN_MEMBERSHIP_ROLE_OPTIONS)); expect(values(roleOptionsOf(SysInvitation))).toEqual(values(BUILTIN_MEMBERSHIP_ROLE_OPTIONS)); - const a = withMembershipRoleOptions(objects, ['role_a']); - const b = withMembershipRoleOptions(objects, ['role_b']); + const a = withMembershipRoleOptions(objects, [{ name: 'role_a' }]); + const b = withMembershipRoleOptions(objects, [{ name: 'role_b' }]); expect(values(roleOptionsOf(a[1]))).toContain('role_a'); expect(values(roleOptionsOf(a[1]))).not.toContain('role_b'); expect(values(roleOptionsOf(b[1]))).toContain('role_b'); @@ -158,20 +191,25 @@ describe('#3723 collectStackOrgRoles — one walk for every host', () => { positions: [{ name: 'sales_rep' }, { name: 'sales_manager' }], permissions: [{ name: 'sales_user' }, { name: 'guest_portal' }], }); - expect(roles).toEqual(['sales_rep', 'sales_manager', 'sales_user', 'guest_portal']); + expect(roles.map((r) => r.name)).toEqual([ + 'sales_rep', + 'sales_manager', + 'sales_user', + 'guest_portal', + ]); }); it('accepts bare strings as well as named entries', () => { expect(collectStackOrgRoles({ positions: ['sales_rep', { name: 'ops' }] })).toEqual([ - 'sales_rep', - 'ops', + { name: 'sales_rep' }, + { name: 'ops' }, ]); }); it('normalizes on the way out — the built-ins never leak into the extras', () => { expect( collectStackOrgRoles({ positions: [{ name: 'member' }, { name: 'owner' }, { name: 'ops' }] }), - ).toEqual(['ops']); + ).toEqual([{ name: 'ops' }]); }); it('a stack with no role metadata yields nothing (and does not throw)', () => { diff --git a/packages/plugins/plugin-auth/src/org-roles.ts b/packages/plugins/plugin-auth/src/org-roles.ts index a412b2efd0..e5d7f534c1 100644 --- a/packages/plugins/plugin-auth/src/org-roles.ts +++ b/packages/plugins/plugin-auth/src/org-roles.ts @@ -87,6 +87,28 @@ export interface MembershipRoleOption { value: string; } +/** + * An app-declared organization role: the machine name better-auth registers, + * plus the display label its `position` / `permission` metadata already + * declared. + * + * The label rides along because the alternative is a THIRD source of truth for + * the same string. A position declares `{ name: 'sales_rep', label: '销售代表' }`; + * deriving the picker's label by title-casing the machine name instead would + * render "Sales Rep" next to the very metadata that says otherwise. Same + * one-list principle as the role set itself, applied to how it is displayed. + * + * Purely presentational: better-auth only ever sees `name`, and the stored + * value is always `name`. + */ +export interface OrgRoleDescriptor { + name: string; + label?: string; +} + +/** What a host may hand to `additionalOrgRoles` — a bare name, or a name + label. */ +export type OrgRoleInput = string | OrgRoleDescriptor; + /** Minimal logger surface — `ctx.logger`, `console`, or nothing at all. */ export interface OrgRoleLogger { warn?(message: string): void; @@ -94,7 +116,11 @@ export interface OrgRoleLogger { const BUILTIN_SET: ReadonlySet = new Set(BUILTIN_MEMBERSHIP_ROLES); -/** `sales_rep` → `Sales Rep`. Display only; the value is always the raw name. */ +/** + * `sales_rep` → `Sales Rep`. The LAST-RESORT label, used only when the + * declaring metadata carried none — a stack that declared a label always wins. + * Display only; the value is always the raw name. + */ export function membershipRoleLabel(name: string): string { return name .split('_') @@ -103,27 +129,43 @@ export function membershipRoleLabel(name: string): string { .join(' '); } +/** The machine names of `descriptors` — what better-auth's role map is keyed by. */ +export function orgRoleNames(descriptors: readonly OrgRoleDescriptor[]): string[] { + return descriptors.map((d) => d.name); +} + /** * The one normalizer for app-supplied organization roles. * - * Trims, drops non-strings, drops the built-ins (an app cannot redefine - * `owner`/`admin`/`member`/`delegated_admin` — silently downgrading `owner` to - * a plain member role would 403 every org mutation), de-duplicates, and - * refuses names that cannot survive `Field.select`'s value normalization. + * Trims, drops entries with no usable name, drops the built-ins (an app cannot + * redefine `owner`/`admin`/`member`/`delegated_admin` — silently downgrading + * `owner` to a plain member role would 403 every org mutation), de-duplicates, + * and refuses names that cannot survive `Field.select`'s value normalization. * Order is preserved so the resulting picker is stable across boots. * + * Accepts a bare name or a {@link OrgRoleDescriptor}; always returns + * descriptors, so every consumer downstream gets the same shape whether or not + * the host had a label to offer. + * * @param logger optional sink for the "dropped an unusable role name" warning */ export function normalizeAdditionalOrgRoles( input: readonly unknown[] | undefined | null, logger?: OrgRoleLogger, -): string[] { +): OrgRoleDescriptor[] { if (!Array.isArray(input)) return []; - const out: string[] = []; + const out: OrgRoleDescriptor[] = []; const seen = new Set(); for (const raw of input) { - if (typeof raw !== 'string') continue; - const name = raw.trim(); + const entry: OrgRoleDescriptor | null = + typeof raw === 'string' + ? { name: raw } + : raw && typeof raw === 'object' && typeof (raw as OrgRoleDescriptor).name === 'string' + ? (raw as OrgRoleDescriptor) + : null; + if (!entry) continue; + const name = entry.name.trim(); + const label = typeof entry.label === 'string' && entry.label.trim() ? entry.label.trim() : undefined; if (!name) continue; if (BUILTIN_SET.has(name)) continue; if (seen.has(name)) continue; @@ -140,7 +182,7 @@ export function normalizeAdditionalOrgRoles( continue; } seen.add(name); - out.push(name); + out.push(label ? { name, label } : { name }); } return out; } @@ -153,12 +195,16 @@ export function normalizeAdditionalOrgRoles( * {@link normalizeAdditionalOrgRoles}); normalizing again here is harmless but * the point of the single normalizer is that callers share one result. */ -export function membershipRoleOptions(additional: readonly string[] = []): MembershipRoleOption[] { +export function membershipRoleOptions( + additional: readonly OrgRoleDescriptor[] = [], +): MembershipRoleOption[] { return [ ...BUILTIN_MEMBERSHIP_ROLE_OPTIONS.map((o) => ({ ...o })), ...additional - .filter((name) => !BUILTIN_SET.has(name)) - .map((name) => ({ label: membershipRoleLabel(name), value: name })), + .filter((d) => !BUILTIN_SET.has(d.name)) + // The declaring metadata's own label wins; title-casing the machine name + // is the fallback for a host that had none. + .map((d) => ({ label: d.label ?? membershipRoleLabel(d.name), value: d.name })), ]; } @@ -178,9 +224,9 @@ export function membershipRoleOptions(additional: readonly string[] = []): Membe */ export function withMembershipRoleOptions( objects: readonly T[], - additional: readonly string[] = [], + additional: readonly OrgRoleDescriptor[] = [], ): T[] { - const extras = additional.filter((name) => !BUILTIN_SET.has(name)); + const extras = additional.filter((d) => !BUILTIN_SET.has(d.name)); if (extras.length === 0) return [...objects]; const options = membershipRoleOptions(extras); @@ -218,15 +264,21 @@ export function withMembershipRoleOptions( * * Real RBAC enforcement stays with SecurityPlugin; better-auth only needs to * accept the names as opaque strings. + * + * Each entry's declared `label` is carried through, so the role picker shows + * what the position/permission metadata already says (`销售代表`) rather than a + * title-cased machine name (`Sales Rep`) contradicting it. */ -export function collectStackOrgRoles(stack: unknown, logger?: OrgRoleLogger): string[] { - const names: unknown[] = []; +export function collectStackOrgRoles(stack: unknown, logger?: OrgRoleLogger): OrgRoleDescriptor[] { + const entries: OrgRoleInput[] = []; const collect = (arr: unknown) => { if (!Array.isArray(arr)) return; for (const entry of arr) { - if (typeof entry === 'string') names.push(entry); - else if (entry && typeof (entry as { name?: unknown }).name === 'string') { - names.push((entry as { name: string }).name); + if (typeof entry === 'string') { + entries.push(entry); + } else if (entry && typeof (entry as { name?: unknown }).name === 'string') { + const { name, label } = entry as { name: string; label?: unknown }; + entries.push(typeof label === 'string' ? { name, label } : { name }); } } }; @@ -238,5 +290,5 @@ export function collectStackOrgRoles(stack: unknown, logger?: OrgRoleLogger): st // Best-effort: a malformed stack must not stop the server from booting. return []; } - return normalizeAdditionalOrgRoles(names, logger); + return normalizeAdditionalOrgRoles(entries, logger); } diff --git a/packages/qa/dogfood/test/app-org-role-invite.dogfood.test.ts b/packages/qa/dogfood/test/app-org-role-invite.dogfood.test.ts index 09961c8288..8ccf0a737b 100644 --- a/packages/qa/dogfood/test/app-org-role-invite.dogfood.test.ts +++ b/packages/qa/dogfood/test/app-org-role-invite.dogfood.test.ts @@ -125,6 +125,19 @@ describe('#3723: an app-declared org role can actually be invited and held', () expect(memberRoles).toEqual(invitationRoles); }, 30_000); + it('the picker shows the DECLARED label, not a title-cased machine name', async () => { + // The showcase position `exec` declares `label: 'Executive'`. Deriving the + // option label from the machine name would render "Exec" — a third source + // of truth for a string the position metadata already owns. Same one-list + // principle as the role set itself, applied to how it is displayed. + const schema = await ql.getSchema('sys_member'); + const options = (schema.fields.role.options ?? []) as { label: string; value: string }[]; + const exec = options.find((o) => o.value === 'exec'); + expect(exec, `no 'exec' option among ${options.map((o) => o.value).join(', ')}`).toBeDefined(); + expect(exec!.label).toBe('Executive'); + expect(exec!.label).not.toBe('Exec'); + }, 30_000); + it('inviting with an app-declared role SUCCEEDS — this is the reported failure', async () => { // Before the fix: 200 from better-auth's role check, then the sys_invitation // insert threw `role must be one of: owner, admin, member`.