diff --git a/.changeset/app-org-roles-storable.md b/.changeset/app-org-roles-storable.md new file mode 100644 index 0000000000..1e6296255d --- /dev/null +++ b/.changeset/app-org-roles-storable.md @@ -0,0 +1,64 @@ +--- +'@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`, 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 +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/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 102537787d..42f5677d22 100644 --- a/content/docs/permissions/positions.mdx +++ b/content/docs/permissions/positions.mdx @@ -16,7 +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/member). +> 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'; 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..ea51467888 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, 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'; @@ -403,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 @@ -1571,7 +1578,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; @@ -1592,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 f8e3bf30ac..b5b5f76cc9 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, type OrgRoleInput } from './org-roles.js'; /** * Auth Plugin Options @@ -79,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 @@ -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..6a40fcaaac --- /dev/null +++ b/packages/plugins/plugin-auth/src/org-roles.test.ts @@ -0,0 +1,227 @@ +// 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([ + { 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([{ name: 'sales_rep' }]); + }); + + it('de-duplicates and trims', () => { + expect(normalizeAdditionalOrgRoles([' sales_rep ', 'sales_rep', ''])).toEqual([ + { name: '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 + // 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([{ name: '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([{ 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([{ name: 'member' }, { name: '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([{ 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', () => { + 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, [{ 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, [{ name: '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, [{ 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, [{ 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'); + }); + + 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.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([ + { 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([{ name: '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..e5d7f534c1 --- /dev/null +++ b/packages/plugins/plugin-auth/src/org-roles.ts @@ -0,0 +1,294 @@ +// 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; +} + +/** + * 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; +} + +const BUILTIN_SET: ReadonlySet = new Set(BUILTIN_MEMBERSHIP_ROLES); + +/** + * `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('_') + .filter((word) => word.length > 0) + .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) + .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 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, +): OrgRoleDescriptor[] { + if (!Array.isArray(input)) return []; + const out: OrgRoleDescriptor[] = []; + const seen = new Set(); + for (const raw of input) { + 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; + 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(label ? { name, label } : { 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 OrgRoleDescriptor[] = [], +): MembershipRoleOption[] { + return [ + ...BUILTIN_MEMBERSHIP_ROLE_OPTIONS.map((o) => ({ ...o })), + ...additional + .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 })), + ]; +} + +/** + * 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 OrgRoleDescriptor[] = [], +): T[] { + const extras = additional.filter((d) => !BUILTIN_SET.has(d.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. + * + * 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): OrgRoleDescriptor[] { + const entries: OrgRoleInput[] = []; + const collect = (arr: unknown) => { + if (!Array.isArray(arr)) return; + for (const entry of arr) { + 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 }); + } + } + }; + 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(entries, logger); +} 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; 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..8ccf0a737b --- /dev/null +++ b/packages/qa/dogfood/test/app-org-role-invite.dogfood.test.ts @@ -0,0 +1,207 @@ +// 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('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`. + 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/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)", 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 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,