diff --git a/.changeset/adr-0105-d8-delegated-admin-org-role.md b/.changeset/adr-0105-d8-delegated-admin-org-role.md index 6edc86f5c5..91d0aa4f5c 100644 --- a/.changeset/adr-0105-d8-delegated-admin-org-role.md +++ b/.changeset/adr-0105-d8-delegated-admin-org-role.md @@ -14,8 +14,8 @@ subtree. That gate is implemented, unit-proven and reachable — but no principa could reach it in a state where it did anything: - better-auth grants `invitation: ["create"]` to `owner` and `admin` only - (`memberAc` holds `invitation: []`, and roles registered through - `additionalOrgRoles` inherited that empty statement); + (`memberAc` holds `invitation: []`, which every other registered role + inherits); - under a wall-enforcing posture, owners and admins are auto-elevated to `organization_admin` (`auto-org-admin-grant.ts`), which carries the wildcard `modifyAllRecords` that makes `isTenantAdmin()` true — and the gate diff --git a/.changeset/app-org-roles-storable.md b/.changeset/app-org-roles-storable.md deleted file mode 100644 index 1e6296255d..0000000000 --- a/.changeset/app-org-roles-storable.md +++ /dev/null @@ -1,64 +0,0 @@ ---- -'@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/.changeset/auth-org-roles-self-derived.md b/.changeset/auth-org-roles-self-derived.md deleted file mode 100644 index f89eb043c0..0000000000 --- a/.changeset/auth-org-roles-self-derived.md +++ /dev/null @@ -1,31 +0,0 @@ ---- -'@objectstack/plugin-auth': minor -'@objectstack/plugin-dev': patch -'@objectstack/verify': patch -'@objectstack/cli': patch ---- - -feat(auth): AuthPlugin derives app-declared organization roles itself — hosts pass nothing (#3723 follow-up, cloud#897) - -Five hosts boot `AuthPlugin` from a stack, and per-host `additionalOrgRoles` -wiring proved to be the defect pattern: three of them (the verify harness, -`DevPlugin`, cloud's `ArtifactKernelFactory`) at some point forgot it, and the -failure is silent — app-declared roles are simply absent. One host (cloud) -mounts `AuthPlugin` before the app metadata even exists, so no init-time walk -could ever cover it. - -`AuthPlugin` now derives the roles in its own `kernel:ready` hook — the one -point that fires after all metadata is registered in every host — via the new -`collectRegisteredOrgRoles(engine, metadataService?)` (the late-bound twin of -`collectStackOrgRoles`). Both consumers are updated from the derived union: -better-auth's org-plugin roles map (`applyConfigPatch`; the instance builds -lazily) and the `sys_invitation.role` / `sys_member.role` select options -(re-registration under the same package id — a supported registry path; no -DDL, options are validator/picker metadata). - -`objectstack serve`, the `@objectstack/verify` harness and `DevPlugin` no -longer pass `additionalOrgRoles` — deliberately, so the dogfood invite gate -only stays green if the auto-derivation works. The option remains for roles -declared OUTSIDE stack metadata; explicit entries are unioned with the derived -set. `collectStackOrgRoles` stays exported for hosts that want an init-time -walk of a raw stack object. diff --git a/.changeset/membership-grade-not-capability-channel.md b/.changeset/membership-grade-not-capability-channel.md new file mode 100644 index 0000000000..602c6af374 --- /dev/null +++ b/.changeset/membership-grade-not-capability-channel.md @@ -0,0 +1,77 @@ +--- +'@objectstack/plugin-auth': minor +'@objectstack/platform-objects': minor +'@objectstack/spec': minor +'@objectstack/lint': patch +'@objectstack/cli': patch +'@objectstack/verify': patch +'@objectstack/plugin-dev': patch +'@objectstack/runtime': patch +--- + +feat(auth)!: membership grade is not a capability channel — the `sys_member.role` +vocabulary is closed (ADR-0108, #3723) + +`sys_member.role` answers "what is your standing in this organization". It does +not answer "what may you do" — that is what positions are for. One column was +answering both. + +`resolve-authz-context` projects EVERY value stored in `sys_member.role` into +`current_user.positions`, alongside the rows read from `sys_user_position`. So a +business role handed out through the membership role *was* capability — granted +with none of the position system's controls: no `granted_by`, no ADR-0091 +validity window, no BU-subtree check, no `assignablePermissionSets` allowlist. +That is what ADR-0057 D4 ruled out ("feed the names to better-auth **only** so +invitations are accepted — **never as the authority for RBAC**"), what +ADR-0090 D3's word ban restates (distribution = `position`), and what +ADR-0095 D3 keeps out of the enforcement path. + +The vocabulary is therefore closed to the four framework-owned names: +`owner` / `admin` / `delegated_admin` / `member`. + +**BREAKING — `additionalOrgRoles` is removed** from `AuthManagerOptions` and +`AuthPluginOptions`, together with `plugin-auth/src/org-roles.ts` in full +(`collectStackOrgRoles`, `collectRegisteredOrgRoles`, +`normalizeAdditionalOrgRoles`, `membershipRoleOptions`, +`withMembershipRoleOptions`, `membershipRoleLabel`, `orgRoleNames`, +`MEMBERSHIP_ROLE_OBJECTS`, `OrgRoleDescriptor`, `OrgRoleInput`, +`OrgRoleLogger`) and the `kernel:ready` derivation hook that fed them. From +`@objectstack/spec`, `MEMBERSHIP_ROLE_NAME_PATTERN` and +`MEMBERSHIP_ROLE_NAME_MIN_LENGTH` are removed — they existed only to validate +app-supplied names. A TypeScript error is the intended failure: an option that +is silently ignored is `declared ≠ enforced` one more time. + +FROM → TO: + +```diff +- new AuthPlugin({ additionalOrgRoles: ['sales_rep'] }) ++ new AuthPlugin({ /* nothing — declare `sales_rep` as a position */ }) + +- POST /organization/invite-member { email, role: 'sales_rep' } ++ POST /organization/invite-member { email, role: 'member', ++ businessUnitId, positions: ['sales_rep'] } +``` + +For an existing member, assign the position through `sys_user_position` (the +governed write path). Invitation placement (ADR-0105 D8) is the one-step +admission flow: issuance is authorized against the issuer's `adminScope` by +dry-running `DelegatedAdminGate`, and acceptance writes real +`sys_user_position` rows with a `granted_by` stamp. It reaches **further** than +what it replaces — a delegated admin may use it within their subtree, where the +membership-role route was open to org admins only (the invitation role cap holds +anyone below admin grade to plain `member`). + +An invitation naming an app role now fails at better-auth's door with +`ROLE_NOT_FOUND`, before any row is written. + +This reverses two changesets that were never consumed into a release +(`app-org-roles-storable`, `auth-org-roles-self-derived`), so no published +version ever offered the behaviour; both are removed rather than shipped and +retracted in the same changelog. A pre-existing deployment could only have +stored a custom value by direct DB write. + +Also derived rather than transcribed: `@objectstack/lint`'s `MEMBERSHIP_TIERS` +now reads `BUILTIN_MEMBERSHIP_ROLES` from `@objectstack/spec`. The hand-kept +copy carried `guest`, which the `sys_member.role` select has never offered — an +approver authored as `{ type: 'org_membership_level', value: 'guest' }` +resolved to nobody and the lint whose whole job is to catch that stayed silent. diff --git a/content/docs/permissions/authentication.mdx b/content/docs/permissions/authentication.mdx index 4124fd7ed9..dbf339fdfa 100644 --- a/content/docs/permissions/authentication.mdx +++ b/content/docs/permissions/authentication.mdx @@ -668,39 +668,52 @@ new AuthPlugin({ }) ``` -#### Positions as membership tiers +#### Membership tiers are a closed list `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: +keeps that vocabulary (ADR-0090 D3's single boundary exception). It holds +exactly four values — `owner`, `admin`, `delegated_admin`, `member` — and your +stack cannot add to them. + +That is deliberate (ADR-0108). A membership tier answers *what is your standing +in this organization*, which decides what you can **reach**: `delegated_admin`, +for instance, is the tier that may reach `/organization/invite-member` without +being an org admin. What you may **do** with records is a +[position](/docs/permissions/positions), and every position assignment carries +an audit stamp (`granted_by`), a validity window, and a scope check that a tier +never had. + +To invite someone straight into a position, put it in the invitation's +*placement* rather than its membership tier: ```http POST /api/v1/auth/organization/invite-member -{ "email": "new.hire@example.com", "role": "sales_rep" } +{ + "email": "new.hire@example.com", + "role": "member", + "businessUnitId": "bu_field_sales", + "positions": ["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. +The tier stays `member`; the capability rides in `positions`. Placement is +authorized at issuance against the issuer's `adminScope`, so a delegated admin +can use it inside their own subtree — where the tier route was open to org +admins only. On acceptance the positions become real `sys_user_position` rows. + + +Earlier releases registered every declared position and permission-set name as +a valid membership tier, so naming one directly in the invitation was accepted. +That path is removed: because every value stored in the tier column is +projected into `current_user.positions`, it granted capability with none of the +controls above. Naming a position as an invitation's tier is now refused with +`ROLE_NOT_FOUND`. Use `positions` as shown. + + +One limit still bounds invitations as a provisioning path: **they never hand 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. ### Admin User Management diff --git a/content/docs/permissions/delegated-administration.mdx b/content/docs/permissions/delegated-administration.mdx index 6e9d1364ae..1d61949068 100644 --- a/content/docs/permissions/delegated-administration.mdx +++ b/content/docs/permissions/delegated-administration.mdx @@ -173,10 +173,12 @@ issuance: - an invited role may not outrank the issuer's (`owner` > `admin` > everyone else); - an issuer below admin grade may invite as **`member` only**. Not just "not - admin/owner": an app-registered role projects into `current_user.positions` - and may be bound to permission sets, so it is a capability channel too. A - delegate's channel for capability is the invitation's **placement** intent, - which this gate allowlists position-by-position; + admin/owner": every value stored in `sys_member.role` projects into + `current_user.positions`, so any tier above plain member is a capability + channel too. A delegate's channel for capability is the invitation's + **placement** intent, which this gate allowlists position-by-position — and + since ADR-0108 that is the *only* way capability travels on an invitation, + because the tier vocabulary is closed to four framework-owned names; - an issuer whose membership cannot be read confers nothing above a plain member (fail-closed). diff --git a/content/docs/permissions/positions.mdx b/content/docs/permissions/positions.mdx index 42f5677d22..4781387077 100644 --- a/content/docs/permissions/positions.mdx +++ b/content/docs/permissions/positions.mdx @@ -16,10 +16,12 @@ 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` — 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)). +> internal `sys_member.role` — the org-membership tier, a **closed** list of +> `owner`, `admin`, `delegated_admin`, `member` and nothing else (ADR-0108). A +> tier decides what you can *reach*; a position decides what you may *do*. To +> invite someone straight into a position, use the invitation's `positions` +> placement, not its membership tier — see +> [Authentication](/docs/permissions/authentication#membership-tiers-are-a-closed-list). ```typescript import type { Position } from '@objectstack/spec/identity'; diff --git a/docs/adr/0108-membership-grade-is-not-a-capability-channel.md b/docs/adr/0108-membership-grade-is-not-a-capability-channel.md new file mode 100644 index 0000000000..5762fbc597 --- /dev/null +++ b/docs/adr/0108-membership-grade-is-not-a-capability-channel.md @@ -0,0 +1,222 @@ +# ADR-0108: Membership Grade Is Not a Capability Channel — the `sys_member.role` Vocabulary Is Closed + +**Status**: Accepted (2026-07-28) — implemented in the same PR: the two `role` selects carry +`BUILTIN_MEMBERSHIP_ROLE_OPTIONS` and nothing else, `additionalOrgRoles` and the whole +`plugin-auth/src/org-roles.ts` feed are removed, proven by +`packages/qa/dogfood/test/membership-role-vocabulary.dogfood.test.ts` +**Deciders**: ObjectStack Protocol Architects +**Builds on**: [ADR-0057](./0057-erp-authorization-core-business-units-and-scope-depth.md) **D4** +(decouple RBAC assignment from better-auth — "never as the authority for RBAC"), +[ADR-0090](./0090-permission-model-v2-concept-convergence.md) **D3** (the `role` → `position` rename +and the word ban) and **D12** (administration as a scoped capability), +[ADR-0095](./0095-authz-kernel-tenant-layer-and-posture-ladder.md) **D3** (posture derives from +capabilities, never from roles), [ADR-0105](./0105-group-tenancy-posture-and-first-class-org-scope.md) +**D8** (scope-bounded invitation issuance and placement) +**Consumers**: `@objectstack/spec`, `@objectstack/plugin-auth`, `@objectstack/platform-objects`, +`@objectstack/lint`, `@objectstack/cli`, `@objectstack/verify`, `@objectstack/plugin-dev`, objectui +**Tracking**: framework#3723 · reverses #3747 and #3779 · related #3697, #3722, #3767 + +--- + +## TL;DR + +`sys_member.role` answers **"what is your standing in this organization"**. It does not answer +"what may you do". Those are different questions, and for a while one column tried to answer both. + +The vocabulary is therefore **closed and framework-owned**: `owner` / `admin` / `delegated_admin` / +`member`. An application's own business roles are **positions**, distributed through +`sys_user_position` — and when the need is "this person should arrive already holding one", the +one-step flow is an **invitation carrying placement** (ADR-0105 D8), not a role name. + +This reverses #3747 (app-declared names became storable) and #3779 (they became automatic in every +host). Both were unreleased when this ADR landed. + +--- + +## Context + +### What the code actually did + +`packages/core/src/security/resolve-authz-context.ts` pushes membership roles and position +assignments into **the same array**: + +```ts +for (const m of activeMembers) { + for (const raw of m.role.split(',')) { + const r = mapMembershipRole(raw); + if (!grants.positions.includes(r)) grants.positions.push(r); // ← membership role + } +} +const userPositionRows = await tryFind(ql, 'sys_user_position', { user_id: userId }, 200); +for (const ur of userPositionRows) grants.positions.push(ur.position); // ← the governed path +``` + +Whatever string is stored in `sys_member.role` **is** a position, by another name. So the question +"which names may be stored there" was never cosmetic — it decided what could be granted without +going through the position system's controls. + +The two doors were guarded asymmetrically: + +| Write path | Gate | +| :-- | :-- | +| `sys_user_position` | `DelegatedAdminGate` — BU-subtree anchoring, `assignablePermissionSets` allowlist, strict containment, `granted_by` stamp, ADR-0091 validity window | +| `sys_member.role` | grade checks only — no audit stamp, no validity window, no scope check | + +`AuthManagerOptions.additionalOrgRoles` registered every `position` / `permission` name a stack +declared with better-auth's organization plugin. #3747 then made those names **storable** (the two +`Field.select`s were widened at boot from the same normalized list), and #3779 made the derivation +**automatic in every host** via a `kernel:ready` hook. What began as "so invitations naming them are +not rejected" had become an ungoverned capability channel, on by default, in every deployment. + +### This question already had an answer + +- **ADR-0057 D4** introduced `additionalOrgRoles` with an explicit qualifier: feed the names to + better-auth *"**only** so invitations to those role names are accepted — **never as the authority + for RBAC**"*, while `sys_member.role` *"shrinks to org-administration"*. The qualifier was the + whole point, and the projection above is precisely what voided it. +- **ADR-0090 D3** bans the word outright — *"capability = `permission_set` · distribution = + `position` · hierarchy = `business_unit` · collaboration = `team`. The word 'role' does not exist + here."* — with one documented exception: `sys_member.role` survives **as third-party schema we do + not own**, labelled "organization membership". An exception granted to a column we cannot rename + is not a licence to build a distribution channel on it. +- **ADR-0095 D3** requires that *"no enforcement-time code path may consult the better-auth role + directly"*, demoting `mapMembershipRole` to a provisioning-time concern. + +No ADR ever authorized the widening. It arrived as a bug fix — which is exactly why it needs a +decision record to close. + +### The replacement already shipped + +ADR-0105 D8 landed **scoped invitation placement**: `sys_invitation` carries `business_unit_id` + +`positions`; issuance is authorized by dry-running `DelegatedAdminGate` against the very +`sys_user_position` rows acceptance would write; acceptance applies them idempotently with a +`granted_by` stamp. + +It is a strict superset of what it replaces: + +| | membership-role channel (retired) | placement (ADR-0105 D8) | +| :-- | :-- | :-- | +| Who may issue | org owner/admin only — the invitation role cap holds anyone below admin grade to plain `member` | admins **and delegated admins**, within subtree + allowlist | +| What acceptance writes | a string on `sys_member` | real `sys_user_position` rows | +| Audit / validity | none | `granted_by` + ADR-0091 windows | +| Scope checks | none | subtree, allowlist, strict containment | + +--- + +## Decision + +### D1 — The membership-role vocabulary is closed + +`sys_member.role` and `sys_invitation.role` offer exactly `owner`, `admin`, `delegated_admin`, +`member` — `BUILTIN_MEMBERSHIP_ROLE_OPTIONS` in `@objectstack/spec`, declared statically by the +platform objects. **Nothing widens them at boot.** The closed select is the write-side guardrail +that makes an ungoverned capability grant *unrepresentable*, not a limitation to work around. + +A membership role is a **grade**: it decides what a principal may *reach* (`delegated_admin` reaches +`/organization/invite-member`), never what they may *do* with the records behind it. + +### D2 — App-declared names are not organization roles + +`additionalOrgRoles` is removed from `AuthManagerOptions` and `AuthPluginOptions`, along with +`plugin-auth/src/org-roles.ts` in its entirety (`collectStackOrgRoles`, +`collectRegisteredOrgRoles`, `normalizeAdditionalOrgRoles`, `membershipRoleOptions`, +`withMembershipRoleOptions`) and the `kernel:ready` derivation hook. A stack's `position` / +`permission` metadata is still surfaced and still drives SecurityPlugin — it is simply not fed to +better-auth's role registry. + +An invitation naming an app role now fails at better-auth's door with `ROLE_NOT_FOUND`, before any +row is written. Loud and early beats a 400 at the insert (the #3747 symptom) and beats silent +success storing an ungoverned grant (what #3747 shipped). + +### D3 — Capability at admission time goes through placement + +The migration for every retired use: + +```diff +- POST /organization/invite-member { email, role: 'sales_rep' } ++ POST /organization/invite-member { email, role: 'member', ++ businessUnitId, positions: ['sales_rep'] } +``` + +and for an existing member, a `sys_user_position` row through the governed write path. + +### D4 — Three facts that look like one; do not merge them + +Recorded because "make it one list" is the tempting next refactor, and two thirds of it would be a +modeling error: + +1. **what names exist** — `@objectstack/spec`'s `membership-role.ts`. The one list. Everything else + derives (the platform objects' selects, the lint tier set, objectui's picker). +2. **which names mean administrative authority** — `orgRoleGrade` (invitation cap) and + `auto-org-admin-grant.ts`. A rule, not a list; it lives next to what it guards. +3. **how a name projects into an identity** — `mapMembershipRole`. Also a rule, also local. + +Only (1) is duplication. Unifying (2) and (3) into it would rebuild the conflation this ADR exists +to end. + +--- + +## Consequences + +**Positive.** +- The position system has **one entrance**, and every grant through it carries `granted_by`, a + validity window and a scope check. "Who gave this person this capability, and when does it lapse" + becomes answerable for every capability, which it was not before. +- ADR-0090 D3's naming commandment is true again in the code, not just in prose. +- Three of the five hand-maintained copies of the vocabulary collapse: the platform objects and the + lint tier set now derive from `@objectstack/spec` (objectui's mirror is the remaining follow-up). +- Deriving the lint's `MEMBERSHIP_TIERS` immediately fixed a live bug: the hand-kept copy carried + `guest`, which the select has never offered, so an approver naming it resolved to nobody and the + lint whose job is to catch that stayed silent. + +**Negative / accepted.** +- **Breaking for any host passing `additionalOrgRoles`** — a TypeScript error, deliberately: a + silently-ignored option would be `declared ≠ enforced` (Prime Directive #10) one more time. The + changeset carries the FROM → TO mapping. +- A deployment that adopted app-declared org roles between #3747 and this change must migrate to + positions. Both changesets were unreleased when this landed, so no published version ever offered + the behaviour; a pre-#3747 deployment could only have reached it by direct DB write. +- **Invitations lose one-step business-role assignment for hosts that have not adopted placement.** + The capability is not lost — placement covers it and reaches further — but it is a different call + shape. + +**Neutral.** +- `mapMembershipRole`'s passthrough default is retained. With the vocabulary closed it can only ever + see the four names, but the default keeps a stored legacy value from resolving to `undefined`. + +--- + +## Alternatives considered + +**Open the two selects (`Field.text` / `allowOther`).** Makes app roles "work" by widening the +ungoverned door and drops the write-side guardrail entirely. Simplest and most wrong. + +**Derive the option list from better-auth's registry (keep #3747, make it reliable).** This is what +#3747 did, and it is the trap: it makes five copies consistent, which reads like a fix, while +promoting "membership role = ungoverned position" from an accident into a formal contract. Making a +wrong design reliable is not fixing it. Deriving becomes correct — and cheap — only *after* the +vocabulary is closed, which is D4 above. + +**Lint only (warn when `additionalOrgRoles` is non-empty).** Honest, and it was the original +recommendation before the ADR history surfaced: it moves the failure to authoring time without +changing what the runtime permits. As a first step it needs no decision; as the answer it leaves the +channel open. Superseded by doing the real thing. + +**Keep the channel and supersede ADR-0057 D4 / ADR-0090 D3 with a new ADR.** The honest way to keep +#3747 — a decision recorded, not a doctrine quietly reversed by a patch changeset. Rejected on the +merits: with placement shipped there is no capability the channel uniquely provides, so such an ADR +would argue for a weaker, less auditable duplicate of an existing path. + +--- + +## References + +- Code: `packages/spec/src/identity/membership-role.ts` (the one list), + `packages/core/src/security/resolve-authz-context.ts` (the projection), + `packages/plugins/plugin-security/src/invitation-placement.ts` (ADR-0105 D8), + `packages/plugins/plugin-auth/src/invitation-role-cap.ts` (grade ceiling), + `packages/plugins/plugin-security/src/delegated-admin-gate.ts` (ADR-0090 D12). +- Proof: `packages/qa/dogfood/test/membership-role-vocabulary.dogfood.test.ts`, + `packages/qa/dogfood/test/delegated-admin-invite.dogfood.test.ts`. +- framework#3723 (the finding, revised three times as the cause moved), #3747 / #3779 (reversed + here), #3767 (`sys_member` governed), objectstack-ai/objectui#2891 (console vocabulary mirror). diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index d13f28e3c3..8fde912cac 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -1491,12 +1491,11 @@ export default class Serve extends Command { if (!trustedOrigins.includes(wildcard)) trustedOrigins.push(wildcard); } - // App-declared organization roles (positions / permission sets) - // need no wiring here: AuthPlugin derives them from the registered - // metadata in its own kernel:ready hook (#3723 / cloud#897) — the - // per-host walk was the defect pattern (three of five hosts forgot - // it). `additionalOrgRoles` remains available for roles OUTSIDE - // the stack metadata, which this host has none of. + // [ADR-0108 / #3723] Nothing to wire: the organization-role + // vocabulary is closed (owner/admin/delegated_admin/member). A + // stack's declared `position` / `permission` names are NOT org + // roles — they are positions, assigned through `sys_user_position` + // or an invitation carrying placement (ADR-0105 D8). await kernel.use(new AuthPlugin({ secret, baseUrl, diff --git a/packages/lint/src/validate-approval-approvers.test.ts b/packages/lint/src/validate-approval-approvers.test.ts index 35f7c5d5b2..492dac6e9a 100644 --- a/packages/lint/src/validate-approval-approvers.test.ts +++ b/packages/lint/src/validate-approval-approvers.test.ts @@ -33,16 +33,29 @@ describe('validateApprovalApprovers', () => { expect(validateApprovalApprovers({ flows: [] })).toEqual([]); }); - it('accepts membership tiers for org_membership_level (owner/admin/member/guest)', () => { + it('accepts the closed membership vocabulary (owner/admin/delegated_admin/member)', () => { const findings = validateApprovalApprovers(stackWithApprovers([ { type: 'org_membership_level', value: 'admin' }, { type: 'org_membership_level', value: 'Owner' }, // case-insensitive { type: 'org_membership_level', value: 'member' }, - { type: 'org_membership_level', value: 'guest' }, + { type: 'org_membership_level', value: 'delegated_admin' }, ])); expect(findings).toEqual([]); }); + it('[ADR-0108] flags `guest` — the tier list is derived, and never offered it', () => { + // The hand-kept copy of this list carried `guest`, which the + // `sys_member.role` select has never offered: an approver naming it + // resolved to nobody, and the lint whose whole job is to catch that stayed + // silent. Deriving the set from `@objectstack/spec` fixed it by + // construction. + const findings = validateApprovalApprovers(stackWithApprovers([ + { type: 'org_membership_level', value: 'guest' }, + ])); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER); + }); + it("flags a position name authored as a membership tier (the ADR-0090 D3 hotcrm class)", () => { const findings = validateApprovalApprovers(stackWithApprovers([ { type: 'org_membership_level', value: 'sales_manager' }, diff --git a/packages/lint/src/validate-approval-approvers.ts b/packages/lint/src/validate-approval-approvers.ts index 30ff88fe2b..ffaf625d88 100644 --- a/packages/lint/src/validate-approval-approvers.ts +++ b/packages/lint/src/validate-approval-approvers.ts @@ -45,7 +45,7 @@ import { canonicalApproverType, normalizeDecisionOutputs, } from '@objectstack/spec/automation'; -import { MEMBERSHIP_ROLE_DELEGATED_ADMIN } from '@objectstack/spec'; +import { BUILTIN_MEMBERSHIP_ROLES } from '@objectstack/spec'; import { collectCelRootIdentifiers } from '@objectstack/formula'; export const APPROVAL_APPROVER_NOT_MEMBERSHIP_TIER = 'approval-approver-not-membership-tier'; @@ -101,21 +101,22 @@ export interface ApprovalApproverFinding { type AnyRec = Record; /** - * The better-auth org-membership tiers `sys_member.role` actually stores - * (see `identity/organization.zod.ts` + `mapMembershipRole`). Anything else - * authored as `{ type: 'org_membership_level' }` (or its deprecated `role` - * spelling) is almost certainly a position name. + * The org-membership tiers `sys_member.role` actually stores — DERIVED, not + * transcribed (ADR-0108 / #3723). + * + * The vocabulary is closed and framework-owned, so the one source in + * `@objectstack/spec` is also the only correct list here. A hand-kept copy is + * how this list came to carry `guest`, which the `sys_member.role` select has + * never offered: an approver naming it resolved to nobody, and the lint that + * exists to catch exactly that stayed silent. + * + * Anything outside this set authored as `{ type: 'org_membership_level' }` (or + * its deprecated `role` spelling) is almost certainly a position name. */ -const MEMBERSHIP_TIERS = new Set([ - 'owner', - 'admin', - 'member', - 'guest', - // [ADR-0105 D8 / #3697] A real tier since the framework registers it with the - // org plugin — the delegated issuer grade. It is storable in - // `sys_member.role`, so an approver naming it resolves to people, not nobody. - MEMBERSHIP_ROLE_DELEGATED_ADMIN, -]); +const MEMBERSHIP_TIERS: ReadonlySet = new Set(BUILTIN_MEMBERSHIP_ROLES); + +/** The same list, rendered for diagnostics — so no message can contradict it. */ +const MEMBERSHIP_TIER_LIST = BUILTIN_MEMBERSHIP_ROLES.join('/'); /** Off-spec dialect spellings we can name a canonical fix for. */ const TYPE_FIX: Record = { @@ -263,12 +264,13 @@ export function validateApprovalApprovers(stack: AnyRec): ApprovalApproverFindin path: `${path}.value`, message: `approver { type: '${type}', value: '${value}' } resolves against the better-auth ` + - `org-membership tier (sys_member.role: owner/admin/member) — '${value}' is not a ` + - `membership tier, so this approver matches nobody and the request stalls.`, + `org-membership tier (sys_member.role: ${MEMBERSHIP_TIER_LIST}) — '${value}' is not ` + + `a membership tier, so this approver matches nobody and the request stalls.`, hint: `If '${value}' is an org position, author { type: 'position', value: '${value}' } ` + `(resolved via sys_user_position, ADR-0090 D3). Keep type 'org_membership_level' ` + - `only for membership tiers (owner/admin/member).`, + `only for membership tiers (${MEMBERSHIP_TIER_LIST}) — the vocabulary is closed ` + + `(ADR-0108), so a business role is always a position.`, }); } else if (type in DEPRECATED_APPROVER_TYPES) { const fix = canonicalApproverType(type); diff --git a/packages/platform-objects/src/identity/sys-invitation.object.ts b/packages/platform-objects/src/identity/sys-invitation.object.ts index 108968b553..47b6f84e0b 100644 --- a/packages/platform-objects/src/identity/sys-invitation.object.ts +++ b/packages/platform-objects/src/identity/sys-invitation.object.ts @@ -197,11 +197,12 @@ 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. + // [ADR-0108 / #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. The vocabulary is CLOSED: an app's own + // business roles are positions, carried by this invitation's `positions` + // placement field (ADR-0105 D8), which is authorized against the issuer's + // `adminScope` instead of riding in un-gated on the role name. role: Field.select({ label: 'Role', required: false, diff --git a/packages/platform-objects/src/identity/sys-member.object.ts b/packages/platform-objects/src/identity/sys-member.object.ts index 1fb14cb31c..aee44eb20e 100644 --- a/packages/platform-objects/src/identity/sys-member.object.ts +++ b/packages/platform-objects/src/identity/sys-member.object.ts @@ -166,16 +166,19 @@ 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. + // [ADR-0108 / #3723] The framework's four roles — the WHOLE list. Nothing + // widens it at boot, and an app's business roles do not belong here: this + // column is ORGANIZATION GRADE, and every value in it is projected into + // `current_user.positions`, so a name added here is capability granted + // with none of ADR-0090 D12's controls. Capability = `position` + // (ADR-0090 D3); one-step admission = an invitation carrying placement + // (ADR-0105 D8). // // 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. + // not exempt it), so the closed list is the write-side guardrail, not a + // limitation to work around. `delegated_admin` (ADR-0105 D8 / #3697) is in + // the list because it is a GRADE — what you may reach — not a capability. role: Field.select({ label: 'Role', required: false, diff --git a/packages/plugins/plugin-auth/src/auth-manager.test.ts b/packages/plugins/plugin-auth/src/auth-manager.test.ts index 96cc1ff71f..5d8ae488c8 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.test.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.test.ts @@ -920,43 +920,34 @@ describe('AuthManager', () => { expect(roles.delegated_admin.statements.team).toEqual([]); }); - it('registering roles keeps the built-in owner/admin/member intact', async () => { + it('the custom map keeps the built-in owner/admin/member intact', async () => { // better-auth's `hasPermission` does `{...options.roles || defaultRoles}`, // so a custom map that omits the defaults silently strips `owner`'s // `invitation:create` and 403s every mutation. - const orgPlugin = await bootOrgPlugin({ additionalOrgRoles: ['sales_rep'] }); + const orgPlugin = await bootOrgPlugin(); const roles = orgPlugin._opts.roles; expect(Object.keys(roles)).toEqual( - expect.arrayContaining(['owner', 'admin', 'member', 'delegated_admin', 'sales_rep']), + expect.arrayContaining(['owner', 'admin', 'member', 'delegated_admin']), ); expect(roles.owner.statements.invitation).toContain('create'); expect(roles.admin.statements.invitation).toContain('create'); expect(roles.member.statements.invitation).toEqual([]); - // App roles stay at member level — only `delegated_admin` gains invite. - expect(roles.sales_rep.statements.invitation).toEqual([]); - }); - - it('an app cannot downgrade `delegated_admin` by declaring the same name', async () => { - const orgPlugin = await bootOrgPlugin({ additionalOrgRoles: ['delegated_admin'] }); - 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'], - }); + it('[ADR-0108] the vocabulary is closed — no app-declared role is registered', async () => { + // The retired channel, asserted as retired. `additionalOrgRoles` is gone + // from the options; passing the name anyway (an unmigrated host, or a + // stack whose `position` metadata declares it) must not put it in the + // map. If it did, the name would be storable in `sys_member.role`, and + // `resolve-authz-context` projects that column into + // `current_user.positions` — capability with no `granted_by`, no + // validity window and no subtree/allowlist check (ADR-0090 D12). + const orgPlugin = await bootOrgPlugin({ additionalOrgRoles: ['sales_rep'] } as never); 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'); + expect(Object.keys(roles).sort()).toEqual(['admin', 'delegated_admin', 'member', 'owner']); + expect(roles.sales_rep).toBeUndefined(); }); it('role cap: a delegate inviting an `admin` is REFUSED (the escalation chain)', async () => { diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 21f28b8314..0134cf14c3 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -21,7 +21,6 @@ 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'; @@ -392,33 +391,22 @@ export interface AuthManagerOptions extends Partial { oidcProviders?: OidcProvidersConfig; /** - * Application-specific organization roles to register with Better-Auth's - * organization plugin. Each name becomes a valid role for invitations and - * member assignments without going through Better-Auth's default - * `owner|admin|member` whitelist. + * `additionalOrgRoles` was REMOVED in ADR-0108 (#3723). The organization-role + * vocabulary is closed: `owner` / `admin` / `delegated_admin` / `member`. * - * The ObjectStack SecurityPlugin handles real RBAC enforcement by matching - * these role names against `permission` metadata (PermissionSets / Profiles), - * so Better-Auth only needs to accept them as opaque strings. Each role is - * registered with the minimum access-control privileges (equivalent to - * Better-Auth's `member` role) so it cannot inadvertently grant org-level - * admin capabilities. + * It registered app-declared `position` / `permission` names with + * better-auth so invitations could name them. Because every value stored in + * `sys_member.role` is projected into `current_user.positions`, that made the + * membership role a capability channel with none of ADR-0090 D12's controls. * - * Rarely needed since the kernel:ready self-derivation (#3723 follow-up): - * AuthPlugin reads the registered `position` / `permission` metadata itself - * via `collectRegisteredOrgRoles`, so stack-declared roles arrive with NO - * host wiring. Pass this only for roles that exist OUTSIDE stack metadata - * (the derived set and this list are unioned). - * - * Accepts a bare name, or `{ name, label }` to carry the declaring - * metadata's own display label into the role picker (#3723) — without it the - * 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: '销售经理' }] + * FROM → TO: + * `new AuthPlugin({ additionalOrgRoles: ['sales_rep'] })` + * → declare `sales_rep` as a `position`, then either assign it + * (`sys_user_position`, governed by `DelegatedAdminGate`) or invite with + * placement: `POST /organization/invite-member` + * `{ role: 'member', businessUnitId, positions: ['sales_rep'] }` + * (ADR-0105 D8 — authorized against the issuer's `adminScope`). */ - additionalOrgRoles?: OrgRoleInput[]; /** * Optional outbound email service used by better-auth callbacks @@ -1559,37 +1547,33 @@ export class AuthManager { if (enabled.organization) { await this.addOptionalPlugin(plugins, 'organization', async () => { const { organization } = await import('better-auth/plugins/organization'); - // Build a `roles` map that registers each app-supplied org role - // (e.g. CRM's sales_rep, sales_manager) as a valid Better-Auth role - // so invitations to those roles aren't rejected with ROLE_NOT_FOUND. - // Real RBAC enforcement is handled by ObjectStack's SecurityPlugin, - // which matches the role name against `permission` metadata - // (PermissionSets). Here we register them with minimum org-plugin - // capabilities (same as the built-in `member` role) so they cannot - // inadvertently grant org-level admin powers. + // [ADR-0108 / #3723] The org-role vocabulary is CLOSED — better-auth's + // own `owner`/`admin`/`member` plus `delegated_admin`. App-declared + // `position` / `permission` names are NOT registered here. // - // [ADR-0105 D8 / #3697] The map ALSO registers `delegated_admin` — the - // one role that may reach `/organization/invite-member` without being an - // org admin. Without it D8's scope-bounded issuance gate has no caller: + // They were, until this change: an app role registered with better-auth + // became storable in `sys_member.role`, and every value in that column is + // projected into `current_user.positions` by `resolve-authz-context`. So + // the membership role was a capability channel carrying none of ADR-0090 + // D12's controls — no `granted_by`, no validity window, no subtree or + // allowlist check — which is exactly what ADR-0057 D4 ("never as the + // authority for RBAC") and ADR-0090 D3's word ban forbid. An app that + // wants `sales_rep` declares a POSITION; the one-step admission flow is + // an invitation carrying placement (ADR-0105 D8), which is governed and + // reaches further (a delegated admin may use it; this never could). + // + // [ADR-0105 D8 / #3697] The map registers `delegated_admin` — the one + // role that may reach `/organization/invite-member` without being an org + // admin. Without it D8's scope-bounded issuance gate has no caller: // better-auth grants `invitation:["create"]` to owner/admin only, and // under a wall-enforcing posture those two are auto-elevated to tenant - // admins, for whom the gate short-circuits and narrows nothing. So the - // map is now built unconditionally, not just when an app supplies extras. + // admins, for whom the gate short-circuits and narrows nothing. // // Deliberately `create` WITHOUT `cancel`: better-auth's cancel route // (`crud-invites.mjs`) checks only `invitation:["cancel"]` — no inviterId // 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; - // [#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; @@ -1597,26 +1581,16 @@ export class AuthManager { // (precedence: `||` then spread). When we pass our own `roles`, the // built-in owner/admin/member are silently dropped, so even the org // owner loses `invitation:create` and every mutation 403s. We must - // re-include the defaults alongside our extras — and if the library - // ever stops exporting them, build NOTHING and let better-auth fall - // back to its own defaults rather than ship a map missing `owner`. + // re-include the defaults alongside `delegated_admin` — and if the + // library ever stops exporting them, build NOTHING and let better-auth + // fall back to its own defaults rather than ship a map missing `owner`. if (defaultAc && memberAc && defaultRoles && typeof memberAc.statements === 'object') { const built: Record = { ...defaultRoles }; const stmts = memberAc.statements; - // Registered BEFORE the app's extras so an app that happens to - // declare a same-named permission cannot downgrade it to a plain - // member role (the loop below skips names already present). built[MEMBERSHIP_ROLE_DELEGATED_ADMIN] = defaultAc.newRole({ ...stmts, invitation: ['create'], }); - // 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); - } customOrgRoles = built; } } catch { diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 85dab3857b..71b0cb7dd9 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -32,17 +32,9 @@ import { runSetInitialPassword } from './set-initial-password.js'; import { runRegisterSsoProviderFromForm, runRegisterSamlProviderFromForm, runRequestDomainVerification, runVerifyDomain } from './register-sso-provider.js'; import { runResendVerificationEmail } from './send-verification-email.js'; import { - AUTH_PLUGIN_ID, authIdentityObjects, authPluginManifestHeader, } from './manifest.js'; -import { - MEMBERSHIP_ROLE_OBJECTS, - collectRegisteredOrgRoles, - normalizeAdditionalOrgRoles, - withMembershipRoleOptions, - type OrgRoleInput, -} from './org-roles.js'; /** * Auth Plugin Options @@ -82,14 +74,12 @@ export interface AuthPluginOptions extends Partial { manifestDatasource?: string; /** - * EXTRA organization roles to register with Better-Auth's organization - * plugin, beyond the ones AuthPlugin derives itself: stack-declared - * `position` / `permission` names arrive automatically via the - * kernel:ready self-derivation (#3723 follow-up), so most hosts pass - * nothing. Only roles that exist OUTSIDE stack metadata need this; the - * two sets are unioned. See {@link AuthManagerOptions.additionalOrgRoles}. + * `additionalOrgRoles` was REMOVED in ADR-0108 (#3723) — see + * {@link AuthManagerOptions} for the FROM → TO migration. The membership-role + * vocabulary is closed (`owner` / `admin` / `delegated_admin` / `member`); + * an app's own business roles are POSITIONS, assigned through + * `sys_user_position` or an invitation carrying placement (ADR-0105 D8). */ - additionalOrgRoles?: OrgRoleInput[]; /** * ADR-0081 D1 — single-org default-organization bootstrap. In single-org @@ -210,16 +200,8 @@ 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 @@ -348,14 +330,13 @@ export class AuthPlugin implements Plugin { ...(this.options.manifestDatasource ? { defaultDatasource: this.options.manifestDatasource } : {}), - // [#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-0108 / #3723] Registered as authored: nothing widens the + // `sys_invitation.role` / `sys_member.role` selects at boot. The closed + // four-name vocabulary those objects declare statically + // (`BUILTIN_MEMBERSHIP_ROLE_OPTIONS`) is the whole list, and it is the + // write-side guardrail that keeps an ungoverned capability grant + // unrepresentable. + objects: authIdentityObjects, // 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 @@ -407,74 +388,16 @@ export class AuthPlugin implements Plugin { : {}), }); - // [#3723 / cloud#897] Late-bound organization roles — hosts pass nothing. - // - // Five hosts boot AuthPlugin from a stack; three of them (verify harness, - // DevPlugin, cloud's ArtifactKernelFactory) at some point forgot to wire - // `additionalOrgRoles`, and the failure is silent: app-declared roles are - // simply absent, no error anywhere. Per-host wiring is the defect pattern; - // this hook removes the need for it. cloud is also ORDER-hostile: it - // mounts AuthPlugin before the app metadata exists, so no init-time walk - // could ever cover it — `kernel:ready` is the one point that fires after - // all metadata is registered in every host. - // - // What happens when app roles are found beyond the explicit config: - // - better-auth side: `applyConfigPatch` merges them; the instance is - // built lazily (or reset by the patch), so the org-plugin roles map is - // rebuilt with the full set on next use. - // - select side: `sys_invitation` / `sys_member` are re-registered with - // widened `role` options under the SAME package id — an explicitly - // supported re-registration path (the registry replaces the owner - // contribution and invalidates the merge cache). No DDL: options are - // validator/picker metadata, the column stays TEXT. + // [ADR-0108 / #3723] There is no organization-role derivation any more. // - // Explicit `additionalOrgRoles` remains as override/extra — the union is - // what both consumers see, preserving the one-list invariant. - ctx.hook('kernel:ready', async () => { - try { - // PluginContext has no getServiceAsync — the sync getService is the - // API (it throws for unknown names, hence the guard). At kernel:ready - // the engine is long registered in every real boot; `undefined` only - // in mock/Lite kernels, where the metadata facade below still serves - // the better-auth half and the select half has no engine to update. - const engine = (() => { - try { return ctx.getService?.('objectql') as unknown; } catch { return undefined; } - })(); - const metadataService = (() => { - try { return ctx.getService?.('metadata') as { list?: (t: string) => unknown } | undefined; } - catch { return undefined; } - })(); - if (!engine && !metadataService) return; // mock/Lite boots — nothing to derive from - - const registered = await collectRegisteredOrgRoles(engine, metadataService, ctx.logger); - if (registered.length === 0) return; - - const known = new Set(additionalOrgRoles.map((d) => d.name)); - const discovered = registered.filter((d) => !known.has(d.name)); - if (discovered.length === 0) return; - - const merged = [...additionalOrgRoles, ...discovered]; - this.authManager?.applyConfigPatch({ additionalOrgRoles: merged }); - - const ql = engine as { registerObject?: (s: unknown, pkg?: string, ns?: string) => string } | undefined; - if (typeof ql?.registerObject === 'function') { - const widened = withMembershipRoleOptions(authIdentityObjects, merged) - .filter((o) => (MEMBERSHIP_ROLE_OBJECTS as readonly string[]).includes((o as { name?: string })?.name ?? '')); - for (const schema of widened) { - ql.registerObject(schema, AUTH_PLUGIN_ID, authPluginManifestHeader.namespace); - } - } - ctx.logger.info('[auth] app-declared organization roles derived from registered metadata', { - discovered: discovered.map((d) => d.name), - total: merged.length, - }); - } catch (e) { - // Derivation is additive — a failure leaves the explicit config in - // force, which is exactly the pre-hook behavior. Loud, not fatal. - ctx.logger.warn('[auth] organization-role derivation failed', { error: (e as Error).message }); - } - }); - + // A `kernel:ready` hook used to walk the registered `position` / + // `permission` metadata and widen both the better-auth role map and the two + // `role` selects, so every host got app-declared roles with no wiring. That + // made an UNGOVERNED capability channel automatic in every deployment: a + // name stored in `sys_member.role` is projected into + // `current_user.positions` with none of ADR-0090 D12's controls. Positions + // are the governed channel, and an invitation carrying placement + // (ADR-0105 D8) is the one-step admission flow that replaces it. ctx.logger.info('Auth Plugin initialized successfully'); } diff --git a/packages/plugins/plugin-auth/src/index.ts b/packages/plugins/plugin-auth/src/index.ts index 53c4e2227e..3ca836f65a 100644 --- a/packages/plugins/plugin-auth/src/index.ts +++ b/packages/plugins/plugin-auth/src/index.ts @@ -30,9 +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'; +// [ADR-0108 / #3723] `./org-roles.js` is gone: there is no app-declared +// organization-role vocabulary to collect, normalize or materialize. The four +// framework roles live in `@objectstack/spec` +// (`BUILTIN_MEMBERSHIP_ROLE_OPTIONS`), which the platform objects read +// directly. An app's own business roles are POSITIONS. export type { AuthConfig, AuthProviderConfig, AuthPluginConfig } from '@objectstack/spec/system'; diff --git a/packages/plugins/plugin-auth/src/org-roles.test.ts b/packages/plugins/plugin-auth/src/org-roles.test.ts deleted file mode 100644 index fcdc093520..0000000000 --- a/packages/plugins/plugin-auth/src/org-roles.test.ts +++ /dev/null @@ -1,288 +0,0 @@ -// 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 { - collectRegisteredOrgRoles, - 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'); - }); -}); - -describe('#3723/cloud#897 collectRegisteredOrgRoles — the late-bound twin', () => { - it('reads position + permission items from the engine registry', async () => { - const engine = { - _registry: { - listItems: (type: string) => - type === 'position' - ? [{ content: { name: 'sales_rep', label: '销售代表' } }] - : type === 'permission' - ? [{ name: 'sales_user' }] // bare item (no content wrapper) — both shapes occur - : [], - }, - }; - expect(await collectRegisteredOrgRoles(engine)).toEqual([ - { name: 'sales_rep', label: '销售代表' }, - { name: 'sales_user' }, - ]); - }); - - it('falls back to the metadata-service facade when the registry has nothing', async () => { - const metadataService = { - list: (type: string) => - Promise.resolve(type === 'position' ? [{ name: 'ops', label: 'Operations' }] : []), - }; - expect(await collectRegisteredOrgRoles(undefined, metadataService)).toEqual([ - { name: 'ops', label: 'Operations' }, - ]); - }); - - it('registry wins over the facade per type — no double counting', async () => { - const engine = { _registry: { listItems: (t: string) => (t === 'position' ? [{ name: 'ops' }] : []) } }; - const metadataService = { - list: (t: string) => (t === 'position' ? [{ name: 'ops' }, { name: 'ghost' }] : [{ name: 'sales_user' }]), - }; - // position served by the registry (facade ignored for it); permission - // empty in the registry → facade serves it. - expect(await collectRegisteredOrgRoles(engine, metadataService)).toEqual([ - { name: 'ops' }, - { name: 'sales_user' }, - ]); - }); - - it('normalizes like every other entry point — built-ins and bad names drop', async () => { - const engine = { - _registry: { - listItems: (t: string) => - t === 'position' ? [{ name: 'owner' }, { name: 'bad.name' }, { name: 'ok_role' }] : [], - }, - }; - expect(await collectRegisteredOrgRoles(engine)).toEqual([{ name: 'ok_role' }]); - }); - - it('nothing to read from → empty, never a throw', async () => { - expect(await collectRegisteredOrgRoles(undefined, undefined)).toEqual([]); - expect(await collectRegisteredOrgRoles({}, {})).toEqual([]); - expect( - await collectRegisteredOrgRoles({ _registry: { listItems: () => { throw new Error('boom'); } } }), - ).toEqual([]); - }); -}); diff --git a/packages/plugins/plugin-auth/src/org-roles.ts b/packages/plugins/plugin-auth/src/org-roles.ts deleted file mode 100644 index 1cf6da79bb..0000000000 --- a/packages/plugins/plugin-auth/src/org-roles.ts +++ /dev/null @@ -1,348 +0,0 @@ -// 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); -} - -/** - * Collect the organization roles REGISTERED in the running kernel — the - * late-bound twin of {@link collectStackOrgRoles}. - * - * `collectStackOrgRoles` needs the raw stack object in the host's hand, which - * is exactly why it kept being forgotten: five hosts booted `AuthPlugin` from - * a stack, and three of them (verify harness, DevPlugin, cloud's - * ArtifactKernelFactory) never wired the walk — a capability silently absent, - * no error anywhere. Worse, one host (cloud) mounts `AuthPlugin` BEFORE the - * app metadata even exists, so no init-time walk could ever cover it. - * - * This variant instead reads the `position` / `permission` metadata already - * registered with the engine, at whatever point it is called — `AuthPlugin` - * calls it from its own `kernel:ready` hook, which in every host fires after - * ALL plugins (and therefore all app metadata) are in. Hosts pass nothing. - * - * Dual read, same as `bootstrapDeclaredPositions`: the engine's - * SchemaRegistry (`_registry.listItems`) is populated by `manifest.register` - * in every boot path; the metadata-service facade is the fallback for boots - * where the registry shape differs. - */ -export async function collectRegisteredOrgRoles( - engine: unknown, - metadataService?: { list?: (type: string) => unknown }, - logger?: OrgRoleLogger, -): Promise { - const entries: OrgRoleInput[] = []; - const push = (items: unknown) => { - if (!Array.isArray(items)) return; - for (const raw of items) { - const item = (raw as { content?: unknown })?.content ?? raw; - if (item && typeof (item as { name?: unknown }).name === 'string') { - const { name, label } = item as { name: string; label?: unknown }; - entries.push(typeof label === 'string' ? { name, label } : { name }); - } - } - }; - for (const type of ['position', 'permission']) { - let items: unknown = undefined; - try { - const reg = (engine as { _registry?: { listItems?: (t: string) => unknown } })?._registry; - if (typeof reg?.listItems === 'function') items = reg.listItems(type); - } catch { /* fall through to the facade */ } - if (!Array.isArray(items) || items.length === 0) { - try { - const listed = metadataService?.list?.(type); - items = typeof (listed as Promise)?.then === 'function' ? await listed : listed; - } catch { items = undefined; } - } - push(items); - } - return normalizeAdditionalOrgRoles(entries, logger); -} diff --git a/packages/plugins/plugin-dev/src/dev-plugin.ts b/packages/plugins/plugin-dev/src/dev-plugin.ts index 0074777f1e..5d43a72f53 100644 --- a/packages/plugins/plugin-dev/src/dev-plugin.ts +++ b/packages/plugins/plugin-dev/src/dev-plugin.ts @@ -523,11 +523,10 @@ export class DevPlugin implements Plugin { if (enabled('auth')) { try { const { AuthPlugin } = await import('@objectstack/plugin-auth') as any; - // [#3723] App-declared organization roles need no wiring: AuthPlugin - // derives them from the registered metadata in its own kernel:ready - // hook, so DevPlugin's "equivalent to the full stack" claim holds - // without this host having to remember a parameter (it was one of the - // three hosts that forgot it). + // [ADR-0108 / #3723] Nothing to wire: the organization-role vocabulary + // is closed, so DevPlugin's "equivalent to the full stack" claim holds + // with no parameter to remember. A stack's `position` / `permission` + // names are positions, not org roles. const authPlugin = new AuthPlugin({ secret: this.options.authSecret, baseUrl: this.options.authBaseUrl, 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 deleted file mode 100644 index 8ccf0a737b..0000000000 --- a/packages/qa/dogfood/test/app-org-role-invite.dogfood.test.ts +++ /dev/null @@ -1,207 +0,0 @@ -// 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/qa/dogfood/test/membership-role-vocabulary.dogfood.test.ts b/packages/qa/dogfood/test/membership-role-vocabulary.dogfood.test.ts new file mode 100644 index 0000000000..1882ff3579 --- /dev/null +++ b/packages/qa/dogfood/test/membership-role-vocabulary.dogfood.test.ts @@ -0,0 +1,208 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0108 / #3723] The organization-role vocabulary is CLOSED, and the + * capability it used to smuggle travels the governed path instead. + * + * This file replaces `app-org-role-invite.dogfood.test.ts`, which proved the + * opposite. That proof was real — an app-declared `position` / `permission` + * name could be registered with better-auth, stored in `sys_invitation.role` + * and held on `sys_member.role` — and it was proving the wrong thing: + * + * `resolve-authz-context.ts` projects EVERY value in `sys_member.role` into + * `current_user.positions`. So a business role handed out through the + * membership role WAS capability, granted with none of ADR-0090 D12's + * controls — no `granted_by`, no ADR-0091 validity window, no BU-subtree + * check, no `assignablePermissionSets` allowlist. + * + * That is what ADR-0057 D4 ruled out ("never as the authority for RBAC"), what + * ADR-0090 D3's word ban restates (**distribution = `position`**; `role` is a + * third-party column we tolerate, not a channel we use), and what ADR-0095 D3 + * keeps out of the enforcement path. + * + * Three properties, in the order a reader needs them: + * + * 1. both enforced selects offer the four framework roles and nothing else — + * the closed list is the write-side guardrail; + * 2. an app-declared name is refused AT BETTER-AUTH'S DOOR, leaving no row — + * it fails early and loudly, not at the insert (the #3747 symptom) and not + * silently (the pre-#3747 symptom); + * 3. the same intent — "this person arrives as a `contributor`" — succeeds + * through invitation placement (ADR-0105 D8), which writes a real + * `sys_user_position` row with a `granted_by` stamp. + * + * (3) is why this is a retirement and not a removal: the replacement reaches + * FURTHER than what it replaces. The membership-role channel needed an org + * admin (the invitation role cap holds anyone below admin grade to plain + * `member`); placement is authorized against the issuer's `adminScope`, so a + * delegated admin can use it inside their own subtree. + * + * Why a dogfood and not a unit test: the original bug shipped green unit tests + * twice. Only the real HTTP route drives better-auth's role check AND the + * ObjectQL validator behind it. + */ + +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 }; + +/** The four framework roles — `BUILTIN_MEMBERSHIP_ROLE_OPTIONS`, in order. */ +const CLOSED_VOCABULARY = ['owner', 'admin', 'delegated_admin', 'member']; + +/** A `position` the showcase stack declares. Under the retired channel this was + * a storable membership role; it must now be reachable only as a position. */ +const APP_POSITION = 'contributor'; +/** A `permission` (PermissionSet) the showcase declares — the other route in. */ +const APP_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 ?? []); +} + +describe('ADR-0108: the membership-role vocabulary is closed; capability goes through positions', () => { + let stack: VerifyStack; + let ql: any; + let orgId: string; + let buId: 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 }, + ); + } + + // Placement lands people in a business unit, so the governed path needs one. + const bu = await ql.insert( + 'sys_business_unit', + { name: 'Field Sales', kind: 'department', organization_id: orgId, active: true }, + { context: SYSTEM_CTX }, + ); + buId = String(bu.id); + }, 180_000); + + afterAll(async () => { + await stack?.stop?.(); + }); + + const optionValues = async (object: string) => { + const schema = await ql.getSchema(object); + return ((schema.fields.role.options ?? []) as { value: string }[]).map((o) => o.value); + }; + + it('both enforced selects offer the four framework roles and nothing else', async () => { + // The registered schema is what the write-path validator reads AND what the + // Setup picker renders. Nothing widens it at boot any more: no + // `withMembershipRoleOptions`, no `kernel:ready` re-registration. + expect(await optionValues('sys_invitation')).toEqual(CLOSED_VOCABULARY); + expect(await optionValues('sys_member')).toEqual(CLOSED_VOCABULARY); + }, 30_000); + + it('a stack-declared position is NOT an organization role — refused at the door', async () => { + // The showcase declares `contributor` as a `position`. Under the retired + // channel this returned 200 and stored the name. It must now fail at + // better-auth's role check (ROLE_NOT_FOUND), before any insert. + const email = 'invitee.contributor.3723@example.com'; + const res = await stack.apiAs(ownerToken, 'POST', '/auth/organization/invite-member', { + email, + role: APP_POSITION, + organizationId: orgId, + }); + + expect(res.status, await res.clone().text()).not.toBe(200); + expect((await findRows(ql, 'sys_invitation', { email }, 5)).length).toBe(0); + }, 30_000); + + it('a declared PermissionSet name is refused the same way — both routes closed', async () => { + // `permission` names were the second collection route into the old feed. + const email = 'invitee.manager.3723@example.com'; + const res = await stack.apiAs(ownerToken, 'POST', '/auth/organization/invite-member', { + email, + role: APP_PERMISSION_SET, + organizationId: orgId, + }); + + expect(res.status).not.toBe(200); + expect((await findRows(ql, 'sys_invitation', { email }, 5)).length).toBe(0); + }, 30_000); + + it('the membership row refuses the name too — the second enforced select', async () => { + // The value the old channel ultimately wanted on `sys_member`. The select is + // enforced on write and system context does not exempt it, so the ungoverned + // grant is unrepresentable at the table, not merely unreachable by route. + const [adminUser] = await findRows(ql, 'sys_user', { email: 'admin@objectos.ai' }, 1); + const [membership] = await findRows(ql, 'sys_member', { user_id: adminUser.id }, 1); + + await expect( + ql.update( + 'sys_member', + { id: membership.id, role: APP_POSITION }, + { context: SYSTEM_CTX }, + ), + ).rejects.toThrow(/must be one of|invalid_option/i); + }, 30_000); + + it('the SAME intent succeeds through placement — governed, audited, and further-reaching', async () => { + // "This person arrives as a contributor", expressed the ADR-0105 D8 way: + // grade stays `member`; the capability rides in `positions`, authorized at + // issuance against the issuer's adminScope by dry-running DelegatedAdminGate. + const email = 'invitee.placed.3723@example.com'; + const res = await stack.apiAs(ownerToken, 'POST', '/auth/organization/invite-member', { + email, + role: 'member', + organizationId: orgId, + businessUnitId: buId, + positions: [APP_POSITION], + }); + + expect(res.status, await res.clone().text()).toBe(200); + const [row] = await findRows(ql, 'sys_invitation', { email }, 5); + expect(row).toBeDefined(); + // Grade and capability are now separate columns — which is the whole point. + expect(row.role).toBe('member'); + const positions = + typeof row.positions === 'string' ? JSON.parse(row.positions) : row.positions; + expect(positions).toEqual([APP_POSITION]); + expect(String(row.business_unit_id ?? row.businessUnitId)).toBe(buId); + }, 30_000); + + it('a name no one ever declared is still refused — nothing opened up', async () => { + 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/runtime/src/standalone-stack.test.ts b/packages/runtime/src/standalone-stack.test.ts index b7dcc554ae..ed38342618 100644 --- a/packages/runtime/src/standalone-stack.test.ts +++ b/packages/runtime/src/standalone-stack.test.ts @@ -2,11 +2,12 @@ // // Regression: the artifact-serve path (`objectstack dev`/`serve`/`start` // booting from `dist/objectstack.json`, no host `objectstack.config.ts`) must -// surface the artifact's app-declared RBAC — `permissions[]` and `roles[]` — at -// the top level of the returned stack config. The CLI reads `config.permissions` -// to honour an app-declared default profile (ADR-0056 D7 — `appDefaultPermissionSetName` -// → SecurityPlugin `fallbackPermissionSet`) and reads `roles[]`/`permissions[]` -// to register app org roles. Before this was fixed, `createStandaloneStack` +// surface the artifact's app-declared RBAC — `permissions[]` and `positions[]` +// — at the top level of the returned stack config. The CLI reads +// `config.permissions` to honour an app-declared default profile (ADR-0056 D7 — +// `appDefaultPermissionSetName` → SecurityPlugin `fallbackPermissionSet`); the +// positions are distributed through `sys_user_position`, never as organization +// roles (ADR-0108). Before this was fixed, `createStandaloneStack` // surfaced `objects`/`requires`/`manifest` but dropped `permissions`/`roles`, so // an `isDefault` profile carrying e.g. `readScope: 'unit_and_below'` was silently // ignored under `objectstack dev` and every user fell back to the built-in diff --git a/packages/runtime/src/standalone-stack.ts b/packages/runtime/src/standalone-stack.ts index 23fea77331..f97cb43882 100644 --- a/packages/runtime/src/standalone-stack.ts +++ b/packages/runtime/src/standalone-stack.ts @@ -98,14 +98,17 @@ export interface StandaloneStackResult { manifest?: any; /** * App-declared RBAC metadata, surfaced so the CLI (`serve`/`dev`/`start`) - * can wire it without a host `objectstack.config.ts`. In particular the - * `serve` command reads `permissions[]` to honour an app-declared default - * profile (ADR-0056 D7 — `appDefaultPermissionSetName` → SecurityPlugin - * `fallbackPermissionSet`) and reads both `positions[]` and `permissions[]` to - * register application org roles with Better-Auth. Without these the - * artifact-serve path silently fell back to the built-in `member_default` - * (owner-only), so an `isDefault` profile declared purely in app metadata - * was ignored under `objectstack dev`. + * can wire it without a host `objectstack.config.ts`. The `serve` command + * reads `permissions[]` to honour an app-declared default profile + * (ADR-0056 D7 — `appDefaultPermissionSetName` → SecurityPlugin + * `fallbackPermissionSet`). Without these the artifact-serve path silently + * fell back to the built-in `member_default` (owner-only), so an + * `isDefault` profile declared purely in app metadata was ignored under + * `objectstack dev`. + * + * These are NOT organization roles: the `sys_member.role` vocabulary is + * closed (ADR-0108). A declared position is distributed through + * `sys_user_position` or an invitation's placement (ADR-0105 D8). */ permissions?: any[]; positions?: any[]; diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 13f777e006..acfb19e2e9 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -54,8 +54,6 @@ "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)", @@ -4271,8 +4269,6 @@ "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)", diff --git a/packages/spec/src/identity/index.ts b/packages/spec/src/identity/index.ts index e6f3665bbf..16c4fbe6cb 100644 --- a/packages/spec/src/identity/index.ts +++ b/packages/spec/src/identity/index.ts @@ -7,6 +7,7 @@ export { positionForm } from './position.form'; export * from './organization.zod'; export * from './scim.zod'; export * from './eval-user.zod'; -// #3723 — the one membership-role list, read by both gatekeepers (better-auth's -// role registry and the `sys_invitation`/`sys_member` role selects). +// #3723 / ADR-0108 — the closed membership-role vocabulary, read by both +// gatekeepers (better-auth's role registry and the `sys_invitation` / +// `sys_member` role selects). Organization grade only; capability = position. export * from './membership-role'; diff --git a/packages/spec/src/identity/membership-role.ts b/packages/spec/src/identity/membership-role.ts index d4f79b0e4d..b73d6a8c76 100644 --- a/packages/spec/src/identity/membership-role.ts +++ b/packages/spec/src/identity/membership-role.ts @@ -1,35 +1,60 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * Organization membership roles — the ONE list (#3723). - * - * A membership role name has to satisfy two independent gatekeepers before a - * person can actually hold it: - * - * 1. **better-auth's organization plugin** must know the name, or - * `POST /organization/invite-member` answers `ROLE_NOT_FOUND`. Registration - * happens in `auth-manager.ts` (`AuthManagerOptions.additionalOrgRoles`). - * 2. **`sys_invitation.role` / `sys_member.role`** must list the name as a - * `select` option. Both are ENFORCED on write — better-auth's own inserts - * run through the ordinary ObjectQL validator, system context and all — so - * a value missing from the option list is a role nobody can be stored as. - * - * Those were two hand-maintained lists that nothing kept in step: an app - * declaring a `permission` named `sales_rep` got the role registered with - * better-auth (step 1) and then watched the `sys_invitation` insert fail with - * `role must be one of: owner, admin, member` (step 2). `declared ≠ enforced` - * (Prime Directive #10), one layer below the declaration. - * - * The fix is contract-first (Prime Directive #12): this module is the single - * source both gatekeepers read. The built-ins live here as ordered constants; - * the app-supplied extras are folded in ONCE, by - * `plugin-auth/src/org-roles.ts`, which feeds the better-auth role map and the - * two `select` option lists from the same normalized array. Neither side can - * accept a name the other rejects, because neither side has its own list. - * - * Naming lives here rather than in plugin-auth because the platform objects - * (`@objectstack/platform-objects`) need the built-in options too, and they - * must not depend on the auth plugin. + * Organization membership roles — a CLOSED, framework-owned vocabulary (#3723). + * + * `sys_member.role` answers **"what is your standing in this organization"**. + * It does not answer "what may you do" — that is what positions are for. + * + * ## Why the list is closed + * + * [ADR-0090 D3] "role" is a reserved-forbidden word in this system, with a + * single documented exception: `sys_member.role` is better-auth's own schema, + * which we do not own. It survives *as a third-party column*, projected as + * `org_membership_level`, and its naming commandment is explicit — + * **capability = `permission_set` · distribution = `position` · hierarchy = + * `business_unit` · collaboration = `team`.** Distribution is a position, so a + * membership role is not a distribution channel. + * + * [ADR-0095 D3] No enforcement-time code path may consult the better-auth role + * directly. `mapMembershipRole` normalizes the four names below into their + * canonical built-in identities at PROVISIONING time (`resolve-authz-context`), + * which is a grant-provisioning concern, not an authorization input. + * + * [ADR-0057 D4, carried forward] App-declared names were once fed to + * better-auth's `additionalOrgRoles` so invitations naming them would not be + * rejected — "**never as the authority for RBAC**". That qualifier was the + * whole point, and it did not hold: whatever string lands in `sys_member.role` + * is projected into `current_user.positions`, so an app role stored here WAS + * capability, granted with none of ADR-0090 D12's controls (no `granted_by`, + * no validity window, no subtree or allowlist check). #3747 made those names + * storable and #3779 auto-derived them in every host; ADR-0108 retires the + * channel and this list is closed again. + * + * ## What replaced it + * + * An app that wants `sales_rep` declares a **`position`** and assigns it + * through `sys_user_position` — the governed path, with the full ADR-0090 D12 + * apparatus. At admission time the one-step flow is an **invitation carrying + * placement** (ADR-0105 D8): `business_unit_id` + `positions`, authorized + * against the issuer's `adminScope` by dry-running `DelegatedAdminGate`, and + * applied on acceptance. That path is strictly more capable than the retired + * one — a delegated admin may use it, where the membership role was + * admin-only. + * + * ## Three facts that look like one — do not merge them + * + * A future reader will be tempted to "unify the role lists". Only the first of + * these is a list; the other two are rules that belong near what they govern: + * + * 1. **what names exist** — this module, the one source; + * 2. **which names mean administrative authority** — `orgRoleGrade` + * (invitation cap) and `auto-org-admin-grant.ts`; + * 3. **how a name projects into an identity** — `mapMembershipRole`. + * + * Naming lives in `@objectstack/spec` rather than in plugin-auth because the + * platform objects (`@objectstack/platform-objects`) need the options too, and + * they must not depend on the auth plugin. */ /** better-auth's built-in organization roles. */ @@ -58,13 +83,18 @@ export const MEMBERSHIP_ROLE_MEMBER = 'member'; * * Doubly opt-in, so a default deployment changes not at all: someone must set * the membership role AND grant an adminScope set. + * + * This is the shape EVERY membership role must have (ADR-0108): a grade that + * decides what you can reach, never a bundle of what you may do. */ export const MEMBERSHIP_ROLE_DELEGATED_ADMIN = 'delegated_admin'; /** - * The membership roles the framework itself ships, in display order. Always - * registered — an app supplies roles IN ADDITION to these, never instead of - * them (dropping better-auth's `owner` alone would 403 every org mutation). + * The membership roles — the WHOLE vocabulary, in display order (ADR-0108). + * + * Not "the built-ins, plus whatever an app adds": there is no second source. + * A stack that needs another business role declares a `position`; see the + * module doc for the migration. */ export const BUILTIN_MEMBERSHIP_ROLES = [ MEMBERSHIP_ROLE_OWNER, @@ -76,10 +106,12 @@ export const BUILTIN_MEMBERSHIP_ROLES = [ export type BuiltinMembershipRole = (typeof BUILTIN_MEMBERSHIP_ROLES)[number]; /** - * `select` options for the built-in roles — the static baseline of - * `sys_invitation.role` and `sys_member.role`. App roles are appended at boot - * (see `plugin-auth/src/org-roles.ts`); a deployment whose stack declares none - * sees exactly this list, unchanged. + * `select` options for `sys_invitation.role` and `sys_member.role` — the + * complete option list both objects declare statically. + * + * Nothing widens these at boot any more (ADR-0108). The closed list IS the + * feature: it is the write-side guardrail that makes an ungoverned capability + * grant unrepresentable, and it is the picker's vocabulary. */ export const BUILTIN_MEMBERSHIP_ROLE_OPTIONS: readonly Readonly<{ label: string; value: string }>[] = [ { label: 'Owner', value: MEMBERSHIP_ROLE_OWNER }, @@ -87,19 +119,3 @@ export const BUILTIN_MEMBERSHIP_ROLE_OPTIONS: readonly Readonly<{ label: string; { 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 dca367704e..326be1389e 100644 --- a/packages/spec/src/index.ts +++ b/packages/spec/src/index.ts @@ -173,8 +173,9 @@ export { } 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. +// #3723 / ADR-0108: organization membership roles — the closed, framework-owned +// vocabulary behind better-auth's role registry AND the `sys_invitation` / +// `sys_member` role selects. Capability travels through positions, never here. export { MEMBERSHIP_ROLE_OWNER, MEMBERSHIP_ROLE_ADMIN, @@ -182,7 +183,5 @@ export { 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 8b7cd8440d..ed4e5599e8 100644 --- a/packages/verify/src/harness.ts +++ b/packages/verify/src/harness.ts @@ -182,12 +182,10 @@ export async function bootStack( // create, ADR-0062 federation, ADR-0086 two-doors). The bootstrap itself is // covered by plugin-auth unit tests + browser E2E. // - // [#3723] App-declared organization roles need no wiring: AuthPlugin derives - // them from the registered metadata in its own kernel:ready hook. The - // harness passing NOTHING is deliberate and is itself part of the proof — - // the dogfood invite gate only stays green if the auto-derivation works, - // which is exactly the guarantee per-host wiring could never give (this - // harness was one of the three hosts that forgot it). + // [ADR-0108 / #3723] Nothing to wire: the organization-role vocabulary is + // closed, and a stack's declared `position` / `permission` names are + // positions, not org roles. `membership-role-vocabulary.dogfood.test.ts` + // boots through this harness and asserts exactly that. await kernel.use(new AuthPlugin({ secret: opts.authSecret ?? DEFAULT_AUTH_SECRET, autoDefaultOrganization: false, diff --git a/scripts/role-word-baseline.json b/scripts/role-word-baseline.json index 7be5ed53e5..f7bd9f4b19 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": 5, + "content/docs/permissions/authentication.mdx": 4, "content/docs/permissions/authorization.mdx": 3, "content/docs/permissions/delegated-administration.mdx": 11, "content/docs/permissions/index.mdx": 1,