From 7064ec8ba48cf413d3c5bb1ad3e77fec27d429ed Mon Sep 17 00:00:00 2001 From: os-warren Date: Fri, 21 Aug 2026 08:33:06 +0000 Subject: [PATCH] fix(auth): teamId on organization/add-member has no active-team fallback (#10532) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two source comments asserted that better-auth's `organization/add-member` defaults BOTH `organizationId` and `teamId` to the caller's active org/team. Measured on the installed better-auth 1.7.1 (`dist/plugins/organization/routes/crud-members.mjs`, inside `addMember`), only the organization half is true: const orgId = ctx.body.organizationId || session?.session.activeOrganizationId; const teamId = "teamId" in ctx.body ? ctx.body.teamId : void 0; `activeOrganizationId` is read 8 times in that module; `activeTeamId`, 0 times (same grep, so the zero is a measurement and not a broken search). Corrected at the origin (`sys_member`'s `add_member` action metadata) and at the citation that named it as authority (`organization-add-member.ts`, which cited it as the justification for forwarding request headers — forwarding buys the organization default only). No runtime behaviour changes and nothing was ever misled at runtime: the `add_member` action's `params` list carries no `teamId`, so the claim was never exercised. It was a wrong citation, not a live defect. The asymmetry that #10050's docs now publish is held by a new pin, `organization-add-member-team-fallback.test.ts`, which reads the fact out of the INSTALLED vendor artifact rather than out of our own comments, with a positive control on `activeOrganizationId` for every zero-hit leg. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- ...-member-team-id-no-active-team-fallback.md | 35 ++++ .../src/identity/sys-member.object.ts | 14 +- ...anization-add-member-team-fallback.test.ts | 172 ++++++++++++++++++ .../src/organization-add-member.ts | 31 +++- 4 files changed, 244 insertions(+), 8 deletions(-) create mode 100644 .changeset/add-member-team-id-no-active-team-fallback.md create mode 100644 packages/plugins/plugin-auth/src/organization-add-member-team-fallback.test.ts diff --git a/.changeset/add-member-team-id-no-active-team-fallback.md b/.changeset/add-member-team-id-no-active-team-fallback.md new file mode 100644 index 0000000000..c1b7ff49eb --- /dev/null +++ b/.changeset/add-member-team-id-no-active-team-fallback.md @@ -0,0 +1,35 @@ +--- +"@objectstack/platform-objects": patch +"@objectstack/plugin-auth": patch +--- + +Correct a false vendor claim in the `organization/add-member` source comments: +`teamId` has **no** active-team fallback (#10532). Two comments — the +`sys_member` `add_member` action metadata (the origin) and the +`organization-add-member.ts` module header that cited it as authority — stated +that "organizationId/teamId default to the caller's active org/team when +omitted". Measured on the installed better-auth 1.7.1 +(`dist/plugins/organization/routes/crud-members.mjs`, inside `addMember`), only +the organization half is true: + +```js +const orgId = ctx.body.organizationId || session?.session.activeOrganizationId; +const teamId = "teamId" in ctx.body ? ctx.body.teamId : void 0; +``` + +`activeOrganizationId` is read 8 times in that module; `activeTeamId`, never. An +omitted `teamId` therefore stays `undefined` and the member joins no team — every +`if (teamId)` branch (team lookup, `TEAM_NOT_FOUND`, per-team limit) is skipped. + +No runtime behaviour changes, and no deployment was ever misled: the `add_member` +action's `params` list carries no `teamId`, so the toolbar never sent one and the +claim was never exercised. What the comment did mislead was the next reader of +the mount, which cited it as the justification for forwarding request headers — +forwarding buys the organization default only. Forwarding `teamId` itself remains +correct: pass it and it works. + +The asymmetry the docs now publish is held by a new pin, +`organization-add-member-team-fallback.test.ts`, which reads the fact out of the +installed vendor artifact (not out of our own comments) so that a future +better-auth bump *adding* an active-team fallback reddens instead of silently +putting the docs out of date. diff --git a/packages/platform-objects/src/identity/sys-member.object.ts b/packages/platform-objects/src/identity/sys-member.object.ts index fdb79af0f9..b422653810 100644 --- a/packages/platform-objects/src/identity/sys-member.object.ts +++ b/packages/platform-objects/src/identity/sys-member.object.ts @@ -43,8 +43,18 @@ export const SysMember = ObjectSchema.create({ // Admin-only: directly attach an existing user to the active org, // bypassing the invite-accept flow. Better-auth: // `organization/add-member { userId, role, organizationId?, teamId? }`. - // organizationId/teamId default to the caller's active org/team when - // omitted, so we leave them as optional params. + // The two optional fields are NOT symmetric. `organizationId` defaults + // to the caller's active organization when omitted; `teamId` has no such + // fallback — omit it and the member simply joins no team. Measured on the + // installed better-auth 1.7.1 + // (`dist/plugins/organization/routes/crud-members.mjs`, `addMember`): + // `ctx.body.organizationId || session?.session.activeOrganizationId` + // against `"teamId" in ctx.body ? ctx.body.teamId : void 0`, and no + // `activeTeamId` read anywhere in that file. So `organizationId` is + // carried as an optional param below and `teamId` is not a param at all + // — this action never sends it, so it never exercises the team half. + // Pinned against a vendor bump that ADDS the fallback by + // plugin-auth's `organization-add-member-team-fallback.test.ts`. name: 'add_member', label: 'Add Member', icon: 'user-plus', diff --git a/packages/plugins/plugin-auth/src/organization-add-member-team-fallback.test.ts b/packages/plugins/plugin-auth/src/organization-add-member-team-fallback.test.ts new file mode 100644 index 0000000000..f0e2ac2c48 --- /dev/null +++ b/packages/plugins/plugin-auth/src/organization-add-member-team-fallback.test.ts @@ -0,0 +1,172 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#10532] `organization/add-member` — `organizationId` falls back to the + * caller's active organization; `teamId` does NOT. + * + * ## What this pin watches, and why it reads the VENDOR + * + * Three of our source comments used to assert the symmetric version of that + * sentence ("organizationId/teamId default to the caller's active org/team when + * omitted"). better-auth 1.7.1 implements only the org half. The comments are + * corrected, and the docs written for #10050 now state the asymmetry to users + * — so the standing risk is no longer a wrong comment but a **vendor bump that + * ADDS an active-team fallback**, which would make our (now correct) docs wrong + * with nothing watching. + * + * Therefore this pin asserts a fact about the INSTALLED vendor artifact, read + * out of `node_modules` at test time. It deliberately does NOT assert anything + * about our own comment text or our own constants: an assertion whose both + * sides derive from files in this repo proves only that we wrote what we wrote, + * and could not fail for the reason it exists. + * + * ## The zero-hit discipline + * + * "There is no active-team fallback" is a NEGATIVE finding, and a negative + * finding from a detector nobody proved is worthless — a typo'd matcher reports + * the same clean absence as a real one. So every negative leg below is paired + * with a POSITIVE control that runs the SAME matcher over a neighbouring fact + * known to be present: + * + * - `SESSION_ACTIVE_FALLBACK` must MATCH the `orgId` binding (which really + * does fall back) before its non-match on the `teamId` binding counts; + * - `activeOrganizationId` must occur in the file before the zero count for + * `activeTeam*` counts. + * + * Measured 2026-08-21 on the installed better-auth 1.7.1, + * `dist/plugins/organization/routes/crud-members.mjs`, inside `addMember`: + * + * const orgId = ctx.body.organizationId || session?.session.activeOrganizationId; + * const teamId = "teamId" in ctx.body ? ctx.body.teamId : void 0; + * + * `activeOrganizationId` occurs 8 times in that file; `activeTeamId`, 0. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync, existsSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { dirname, join } from 'node:path'; + +/** + * Locate this package by walking up from the CWD — the idiom + * `member-role-canonical.test.ts` / `rate-limit-storage-isolation.test.ts` + * established here and state the reason for: plugin-auth is CJS-typed (no + * `"type": "module"`, it publishes `dist/index.js` as CommonJS), so under + * `module: NodeNext` `import.meta` is a TS1470 in this package however well it + * runs under vitest. + */ +function findUp(predicate: (dir: string) => boolean, what: string): string { + let dir = process.cwd(); + for (;;) { + if (predicate(dir)) return dir; + const parent = dirname(dir); + if (parent === dir) throw new Error(`could not locate ${what}`); + dir = parent; + } +} + +const PKG = findUp((dir) => { + const manifest = join(dir, 'package.json'); + if (!existsSync(manifest)) return false; + const { name } = JSON.parse(readFileSync(manifest, 'utf8')) as { name?: string }; + return name === '@objectstack/plugin-auth'; +}, 'the @objectstack/plugin-auth package root'); + +// Resolved from THIS package, so the file read is the better-auth this package +// is pinned to — not whatever a hoist happens to put at the repo root. The read +// goes through `node_modules` at test time; there is no checked-in copy of the +// vendor text for it to fall back to, which is what the ablation in the PR body +// demonstrates (mutate the file under `node_modules` and this suite reddens). +const require_ = createRequire(join(PKG, 'probe.js')); +/** `…/better-auth/dist/index.mjs` → `…/better-auth/dist`. */ +const VENDOR_DIST = dirname(require_.resolve('better-auth')); +const CRUD_MEMBERS = join(VENDOR_DIST, 'plugins', 'organization', 'routes', 'crud-members.mjs'); +const VENDOR_SOURCE = readFileSync(CRUD_MEMBERS, 'utf8'); + +/** + * The whole top-level `const = …` declaration, up to the next top-level + * `const` (declarations inside the body are indented, so a line-start `const` + * marks the next sibling). Throws when the declaration is gone — the drift + * tripwire: a vendor upgrade that restructures the module fails loudly instead + * of leaving a pin that matches nothing and passes. + */ +function topLevelDecl(source: string, name: string): string { + const start = source.indexOf(`\nconst ${name} = `); + if (start < 0) { + throw new Error( + `[#10532] could not find the top-level \`${name}\` declaration in ${CRUD_MEMBERS}. ` + + `better-auth restructured this module — re-read \`addMember\` and re-decide whether ` + + `\`teamId\` still has no active-team fallback (our comments and the ` + + `content/docs/permissions/authentication.mdx "Attaching an existing user to an ` + + `organization" section both state that it does not).`, + ); + } + const rest = source.slice(start + 1); + const next = rest.indexOf('\nconst '); + return next < 0 ? rest : rest.slice(0, next); +} + +/** + * The initialiser of a single-line `const = …;` binding inside a slice, + * returned as vendor BYTES. Throws when absent, for the same reason as above. + */ +function bindingInitializer(slice: string, name: string): string { + const match = new RegExp(`\\n\\s*const ${name} = ([^;\\n]+);`).exec(slice); + if (!match) { + throw new Error( + `[#10532] could not find the \`const ${name} = …;\` binding inside \`addMember\` in ` + + `${CRUD_MEMBERS}. better-auth rewrote the handler — re-read it and re-decide whether ` + + `\`teamId\` still has no active-team fallback.`, + ); + } + return match[1]!; +} + +/** + * "This binding falls back to something on the caller's session." One matcher, + * used on both bindings, so the negative leg is only ever read after the same + * matcher has been shown to fire on the positive one. + */ +const SESSION_ACTIVE_FALLBACK = /session[\s\S]*\bactive[A-Z]\w*/; + +const ADD_MEMBER = topLevelDecl(VENDOR_SOURCE, 'addMember'); + +const occurrences = (needle: RegExp): number => + VENDOR_SOURCE.match(new RegExp(needle.source, 'g'))?.length ?? 0; + +describe('[#10532] better-auth `addMember` — the org/team fallback asymmetry', () => { + it('CONTROL: the `orgId` binding really does fall back to the session active org', () => { + // Positive control for BOTH instruments used below: it proves the vendor + // file was located and sliced, and that `SESSION_ACTIVE_FALLBACK` fires on + // a binding that carries a session fallback. Without this leg passing, the + // non-match in the next test is not evidence of anything. + const orgId = bindingInitializer(ADD_MEMBER, 'orgId'); + expect(orgId).toMatch(SESSION_ACTIVE_FALLBACK); + expect(orgId).toContain('activeOrganizationId'); + }); + + it('the `teamId` binding has NO fallback — it reads the request body and nothing else', () => { + const teamId = bindingInitializer(ADD_MEMBER, 'teamId'); + // Same matcher that just fired on `orgId`. + expect(teamId).not.toMatch(SESSION_ACTIVE_FALLBACK); + // …and positively: the only source of the value is the request body. + expect(teamId).toContain('ctx.body'); + expect(teamId).not.toContain('session'); + }); + + it('no active-team read anywhere in the module (with the neighbouring positive control)', () => { + // The control first: if this ever hits 0 the grep itself is broken and the + // zero below means nothing. + expect(occurrences(/activeOrganizationId/)).toBeGreaterThan(0); + expect(occurrences(/active[Tt]eam\w*/)).toBe(0); + }); + + it('the `if (teamId)` branches stay skipped when teamId is absent — no re-resolution later', () => { + // The remaining way the vendor could acquire an active team is to + // re-resolve it after the binding. Same control shape: the guard we expect + // to be there is asserted present, then the re-resolution is asserted + // absent across the whole handler. + expect(ADD_MEMBER).toMatch(/if \(teamId\)/); + expect(ADD_MEMBER).not.toMatch(/teamId\s*(?:\|\||\?\?)=?/); + }); +}); diff --git a/packages/plugins/plugin-auth/src/organization-add-member.ts b/packages/plugins/plugin-auth/src/organization-add-member.ts index f575530161..313238f82a 100644 --- a/packages/plugins/plugin-auth/src/organization-add-member.ts +++ b/packages/plugins/plugin-auth/src/organization-add-member.ts @@ -42,13 +42,32 @@ * capability error (501 when the organization plugin is off) → body * validation (400) → the vendor's own verdicts, forwarded verbatim. * - * ── organizationId default ────────────────────────────────────────────────── + * ── organizationId defaults; teamId does NOT ──────────────────────────────── * - * The request headers are forwarded into `auth.api.addMember`, so an omitted - * `organizationId` defaults to the CALLER's active organization — the - * behaviour the `sys_member` action metadata documents ("organizationId/teamId - * default to the caller's active org/team when omitted"). An admin with no - * active organization gets the vendor's 400 `NO_ACTIVE_ORGANIZATION`. + * The two optional fields are NOT symmetric. `organizationId` defaults to the + * caller's active organization when omitted; `teamId` has no such fallback — + * omit it and the member simply joins no team. Measured on the installed + * better-auth 1.7.1 (`dist/plugins/organization/routes/crud-members.mjs`, + * inside `addMember`'s handler): + * + * const orgId = ctx.body.organizationId || session?.session.activeOrganizationId; + * const teamId = "teamId" in ctx.body ? ctx.body.teamId : void 0; + * + * and no `activeTeamId` read anywhere in that file (`activeOrganizationId` + * hits 8 times in the same grep, which is what proves the search works). + * + * So what header forwarding buys is the ORG default only: an admin with no + * active organization gets the vendor's 400 `NO_ACTIVE_ORGANIZATION`, while an + * omitted `teamId` stays `undefined` and every `if (teamId)` branch in the + * vendor handler (team lookup, `TEAM_NOT_FOUND`, per-team limit) is skipped. + * Forwarding `teamId` itself is still correct — pass it and it works. + * + * The `sys_member` `add_member` action metadata used to assert the symmetric + * version of this sentence; it was a wrong citation rather than a live defect, + * because that action's `params` list carries no `teamId` and so never sent + * one. Corrected at the origin in the same change, and pinned against a vendor + * bump that ADDS an active-team fallback by + * `organization-add-member-team-fallback.test.ts`. */ import { mapAuthApiError, type EndpointResult } from './admin-user-endpoints.js';