Skip to content

fix(security): resolve the issuer's real grants when authorizing invitation placement (ADR-0105 D8) - #3695

Merged
os-zhuang merged 1 commit into
mainfrom
claude/adr-0105-group-tenancy-posture-5womi2
Jul 27, 2026
Merged

fix(security): resolve the issuer's real grants when authorizing invitation placement (ADR-0105 D8)#3695
os-zhuang merged 1 commit into
mainfrom
claude/adr-0105-group-tenancy-posture-5womi2

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

What's wrong

Scoped-invitation issuance (#3663, ADR-0105 D8) dry-runs DelegatedAdminGate
against the sys_user_position rows the acceptance would write — the right
design, wired wrong.

The gate judges authority from context.positions / context.permissions
(security-plugin.ts resolvePermissionSetsForContext). But
beforeCreateInvitation handed it a hand-built context:

actorContext: { userId: inviter?.id, tenantId: organizationId },

No positions, no permissions. Every issuer therefore resolved to the
additive baseline and nothing else, so the gate refused them all:

[Security] Access denied: 'insert' on 'sys_user_position' requires tenant-level
administration or a delegated adminScope (ADR-0090 D12) — plain CRUD grants on
RBAC tables do not make a permission administrator.

Fail-closed, but dead: only a tenant admin could ever issue a placement,
and a tenant admin is the one caller the feature was not built for. The whole
point of D8 is letting a plant admin invite straight into their own subtree.

Caught by cloud's ee-group-showcase group-posture dogfood — the first thing
to drive the real HTTP path with a real delegate rather than a mocked gate.
My unit tests passed because they asserted the context was forwarded, never
that it carried authority.

The fix

assertIssuable takes actorUserId instead of a caller-built context and
resolves that user's grants itself, through the single authz resolver:

const grants = userId
  ? await resolveUserAuthzGrants(ql, userId, { tenantId: organizationId ?? undefined })
  : null;

resolveUserAuthzGrants is the userId-driven half of resolveAuthzContext
factored out precisely so a surface that knows who the principal is, with no
HTTP request to resolve it from, builds the same envelope from the same
reads (sys_member, sys_user_position, sys_*_permission_set) instead of
re-deriving them. A better-auth hook is exactly that surface.

Why the id and not a context: a hook can honestly supply who is issuing; it
cannot supply what they may do. Pushing resolution behind the service
boundary means the next caller cannot reintroduce this bug by passing a
plausible-looking object.

A principal-less call still reaches the gate with an empty context on purpose —
the gate's own !ctx.userId branch owns that refusal, so the security boundary
keeps exactly one place an issuance can be denied.

Tests

  • Regression (invitation-placement.test.ts): an issuer holding
    plant_admin via sys_user_position now reaches the gate with that
    position on the context
    . This is the assertion the original tests were
    missing — they pinned forwarding, not authority.
  • Principal-less issuance reaches the gate with {} (gate owns the refusal).
  • auth-manager.test.ts updated to expect actorUserId.
plugin-security  29 files  608 passed
plugin-auth      24 files  535 passed
check:authz-resolver ✓   check:nul-bytes ✓   check:doc-authoring ✓   check:role-word ✓

Compatibility

InvitationPlacementService is an internal kernel service (registered by
SecurityPlugin, consumed by plugin-auth's invitation hooks). The signature
change is contained to that seam — no REST/SDK surface moves, no schema change.
Shipped as a patch: the feature never worked for its intended caller, so
there is no working behaviour to preserve.

🤖 Generated with Claude Code

https://claude.ai/code/session_015FebXPaaGrLhGKw1LHPbpL


Generated by Claude Code

…tation placement

Scoped-invitation issuance (ADR-0105 D8) dry-runs `DelegatedAdminGate` against
the `sys_user_position` rows the acceptance would write. The gate reads
authority off `context.positions` / `context.permissions`
(`resolvePermissionSetsForContext`) — but `beforeCreateInvitation` handed it a
hand-built `{ userId, tenantId }`, which carries neither. Every delegated
administrator resolved to the additive baseline alone and was refused with
"requires tenant-level administration or a delegated adminScope".

Fail-closed, but dead: only a tenant admin could issue a placement, which is
the one caller the feature was not for. Caught by cloud's group-posture
dogfood driving the real HTTP path with a real delegate.

`assertIssuable` now takes `actorUserId` and resolves that user's grants
itself via `@objectstack/core` `resolveUserAuthzGrants` — the userId-driven
half of the single authz resolver, producing the same envelope a transport
would have carried, from the same reads. A better-auth hook has no request to
resolve a context from, so the id is what a caller can honestly supply and the
resolution belongs behind the service boundary rather than in front of it.

A principal-less call still reaches the gate with an empty context: the gate
owns that refusal too, so there stays exactly one place an issuance is denied.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015FebXPaaGrLhGKw1LHPbpL
@vercel

vercel Bot commented Jul 27, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Jul 27, 2026 2:43pm

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/m labels Jul 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-auth, @objectstack/plugin-security.

16 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx (via @objectstack/plugin-auth, @objectstack/plugin-security)
  • content/docs/deployment/production-readiness.mdx (via @objectstack/plugin-auth)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/plugin-auth)
  • content/docs/permissions/access-recipes.mdx (via packages/plugins/plugin-security)
  • content/docs/permissions/authentication.mdx (via @objectstack/plugin-auth)
  • content/docs/permissions/authorization.mdx (via @objectstack/plugin-security)
  • content/docs/permissions/explain.mdx (via @objectstack/plugin-security)
  • content/docs/permissions/permissions-matrix.mdx (via packages/plugins/plugin-security)
  • content/docs/permissions/sharing-rules.mdx (via @objectstack/plugin-security)
  • content/docs/permissions/sso.mdx (via @objectstack/plugin-auth)
  • content/docs/plugins/index.mdx (via @objectstack/plugin-auth, @objectstack/plugin-security)
  • content/docs/plugins/packages.mdx (via @objectstack/plugin-auth, @objectstack/plugin-security)
  • content/docs/releases/implementation-status.mdx (via @objectstack/plugin-auth, @objectstack/plugin-security)
  • content/docs/releases/v9.mdx (via @objectstack/plugin-auth)
  • content/docs/ui/audience-based-interfaces.mdx (via packages/plugins/plugin-security)
  • content/docs/ui/dashboards.mdx (via @objectstack/plugin-security)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@os-zhuang
os-zhuang marked this pull request as ready for review July 27, 2026 14:50
@os-zhuang
os-zhuang merged commit 735f850 into main Jul 27, 2026
16 checks passed
@os-zhuang
os-zhuang deleted the claude/adr-0105-group-tenancy-posture-5womi2 branch July 27, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants