Skip to content

Commit 05f96ee

Browse files
hotlongclaude
andcommitted
fix(core): drop a session's unbacked organization claim under a walled posture (#15409)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 6a3cc13 commit 05f96ee

1 file changed

Lines changed: 152 additions & 1 deletion

File tree

‎packages/core/src/security/resolve-authz-context.ts‎

Lines changed: 152 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,15 @@ import { isRowActive } from './row-active.js';
7676
/** The transport-agnostic authorization envelope produced from a request. */
7777
export interface ResolvedAuthzContext {
7878
userId?: string;
79+
/**
80+
* The ACTIVE organization this request operates in.
81+
*
82+
* ⚠️ [#15409] For a session principal under a wall-enforcing posture this is
83+
* a VETTED value, never the stored `activeOrganizationId` as read: a claim
84+
* that is not in {@link accessible_org_ids} is dropped and the context
85+
* resolves with no active organization at all. Absent here is the fail-closed
86+
* state, not a missing lookup — Layer 0 denies on it.
87+
*/
7988
tenantId?: string;
8089
email?: string;
8190
accessToken?: string;
@@ -210,6 +219,57 @@ function warnApiKeyRefusal(details: {
210219
);
211220
}
212221

222+
/**
223+
* [#15409 — maintainer ruling 2026-09-05, option B] Say a DROPPED session
224+
* organization claim out loud, on the SERVER side, where the drop is decided —
225+
* the session mirror of {@link warnApiKeyRefusal} (#15256 / 2A).
226+
*
227+
* ## Why the operator needs this and the caller must not get it
228+
*
229+
* The drop is deliberately INVISIBLE on the wire: the principal stays
230+
* authenticated, no status code moves, no response field is added and no reason
231+
* travels. What the user sees is a session with no active organization — Layer
232+
* 0's existing fail-closed branch (`!organizationId ⇒ RLS_DENY_FILTER` in
233+
* `plugin-security/src/tenant-layer.ts`) — which is indistinguishable from
234+
* never having selected one. That is right for the caller and useless for the
235+
* operator, who otherwise has no way to learn that an offboarded member's live
236+
* session was still naming the organization they left.
237+
*
238+
* ## What may appear here
239+
*
240+
* The `sys_session` ROW id, the owner, the claimed organization, the reason.
241+
* ⛔ NEVER `sys_session.token` — that column's own field comment records a
242+
* replay-proven impersonation (a leaked token is `Authorization: Bearer` for
243+
* that user), so it is the one value this line must never carry. The row `id`
244+
* is a separate column and is derived from neither the token nor its hash.
245+
*
246+
* ## Volume
247+
*
248+
* One line per request that drops a claim — bounded by real sessions whose
249+
* membership ended, not by traffic. An anonymous or bogus cookie never reaches
250+
* here (no `userId`, no claim), so a prober writes nothing. Deliberately not
251+
* rate-limited: the burst — every request of a removed member's still-live
252+
* session, for up to the session's remaining lifetime — is exactly what the
253+
* operator needs to see.
254+
*
255+
* `console.warn` and not an injected logger, for the same reason the API-key
256+
* line is: this resolver is kernel-agnostic and takes no host wiring.
257+
*/
258+
function warnSessionOrganizationClaimDropped(details: {
259+
reason: 'organization_membership_ended';
260+
sessionId?: string;
261+
userId?: string;
262+
organizationId?: string;
263+
}): void {
264+
const { reason, sessionId, userId, organizationId } = details;
265+
console.warn(
266+
`[security] Session organization claim dropped (${reason}): `
267+
+ `session=${sessionId ?? '<unknown>'} principal=${userId ?? '<unknown>'} `
268+
+ `organization=${organizationId ?? '<none>'}. `
269+
+ 'The session stays authenticated with NO active organization — the wire is unchanged.',
270+
);
271+
}
272+
213273
async function tryFind(
214274
ql: any,
215275
object: string,
@@ -324,6 +384,12 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise<Res
324384

325385
let userId: string | undefined;
326386
let tenantId: string | undefined;
387+
/**
388+
* [#15409] The `sys_session` ROW id, kept only so a dropped organization
389+
* claim can name the session it was dropped from. ⛔ Never
390+
* `sessionData.session.token` — see {@link warnSessionOrganizationClaimDropped}.
391+
*/
392+
let sessionId: string | undefined;
327393

328394
// 1. API key (explicit opt-in via header) takes precedence over session.
329395
const admission = await resolveApiKeyAdmission(ql, headers, input.nowMs, input.tenancyPosture);
@@ -359,6 +425,10 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise<Res
359425
const sessionData = await input.getSession(headers);
360426
userId = sessionData?.user?.id ?? sessionData?.session?.userId;
361427
tenantId = tenantId ?? sessionData?.session?.activeOrganizationId;
428+
// [#15409] Identity of the session, for the dropped-claim log line below.
429+
// ⛔ The ROW id, never `session.token` — the token is the credential.
430+
const rawSessionId = sessionData?.session?.id;
431+
sessionId = typeof rawSessionId === 'string' && rawSessionId ? rawSessionId : undefined;
362432
ctx.accessToken = sessionData?.session?.token ?? ctx.accessToken;
363433
if (sessionData?.user?.email) ctx.email = String(sessionData.user.email);
364434
} catch {
@@ -379,7 +449,7 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise<Res
379449
// `sys_member` / `sys_user_position` / `sys_*_permission_set`, so a non-HTTP
380450
// surface that already knows the user id (a `runAs:'user'` automation run,
381451
// #3356) can build the SAME envelope without re-implementing any of it.
382-
const grants = await resolveUserAuthzGrants(ql, userId, {
452+
let grants = await resolveUserAuthzGrants(ql, userId, {
383453
tenantId,
384454
nowMs: input.nowMs,
385455
seedPermissions: ctx.permissions,
@@ -432,6 +502,87 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise<Res
432502
}
433503
}
434504

505+
// ── [#15409 — maintainer ruling 2026-09-05, option B] The SESSION arm ─────
506+
//
507+
// The block above asks "is this stamped organization still backed by a
508+
// membership?" and, until this card, asked it ONLY of an API key. A browser
509+
// session's `activeOrganizationId` reached `ctx.tenantId` unread: measured on
510+
// a live `isolated` boot with the real cloud-private `Organizations` plugin,
511+
// a session whose owner had been removed through better-auth's OWN
512+
// `/organization/remove-member` (driven by the org owner, 200, the
513+
// `sys_member` row really deleted) went on READING that organization's rows
514+
// and WRITING into it — the written row read back out of the sqlite file
515+
// carrying the left organization and the removed user as author, for up to
516+
// the session's remaining lifetime (7 days by default).
517+
//
518+
// The resolver already HELD the fact: `get-session` reported positions
519+
// `["user","org_member"]` while the membership was intact and `["user"]` on
520+
// the very next request — off the same `sys_member` read that builds
521+
// `accessible_org_ids`. Nothing was missing but this comparison.
522+
//
523+
// ⚠️ Why the claim is DROPPED and the principal is NOT refused (option B,
524+
// ruled; ⛔ not option A, which is the API-key arm's shape):
525+
//
526+
// An API key IS its organization binding, so refusing the whole credential
527+
// is right there (#15256, decision 1A). A session is a PERSON, who may hold
528+
// legitimate memberships elsewhere. Refusing the principal would sign them
529+
// out of every organization because one stored claim went stale; dropping
530+
// the claim leaves them signed in, with no active organization, free to
531+
// switch to one they are actually in.
532+
//
533+
// ⛔ No second refusal mechanism is added: with no active organization, Layer
534+
// 0 is ALREADY fail-closed — `!input.organizationId ⇒ RLS_DENY_FILTER` in
535+
// `plugin-security/src/tenant-layer.ts`'s `isolated` branch, and an empty
536+
// `accessible_org_ids` denies under `group`. This uses the branch that exists.
537+
//
538+
// ⛔ And no session revocation: revoking on a membership change is an EVENT
539+
// trigger, covering exactly the removal paths someone remembered to wire — a
540+
// direct row delete, a seed replay, a bulk or control-plane operation fails
541+
// it OPEN, silently. This test runs on EVERY request, at the point the
542+
// decision is made, so it covers every removal path by construction. Session
543+
// revocation is filed separately, as the courtesy an admin expects when they
544+
// click "Remove member", never as the wall.
545+
//
546+
// Free, like its API-key sibling: `resolveUserAuthzGrants` has just read
547+
// `sys_member` for this user, so the test itself is a set membership check on
548+
// data in hand. The RE-resolution below is not free, but it is reached only
549+
// by a request that presented an unbacked claim — never on a healthy one.
550+
//
551+
// Re-resolved rather than hand-edited on purpose: "resolves with NO active
552+
// organization" is an existing, well-defined state (a session that has not
553+
// selected one), and the honest way to reach it is to ask the same resolver
554+
// for it. Editing `ctx.tenantId` alone would leave the envelope's other
555+
// tenant-scoped derivations computed under the claim that was just rejected —
556+
// `org_user_ids`, the fellow-org peer list Layer 1 scopes identity tables
557+
// with, would still enumerate the members of the organization the user left.
558+
//
559+
// Session-only by construction: an admitted API key sets `userId`, which
560+
// makes the session branch above unreachable, so a non-empty `tenantId` here
561+
// with no `keyPrincipal` can only have come from the session claim.
562+
if (
563+
!keyPrincipal
564+
&& tenantId
565+
&& input.tenancyPosture
566+
&& postureEnforcesWall(input.tenancyPosture)
567+
&& !grants.accessible_org_ids.includes(tenantId)
568+
) {
569+
// [#15256 / 2A, mirrored] The one decision point where the drop is decided.
570+
warnSessionOrganizationClaimDropped({
571+
reason: 'organization_membership_ended',
572+
sessionId,
573+
userId,
574+
organizationId: tenantId,
575+
});
576+
tenantId = undefined;
577+
delete ctx.tenantId;
578+
grants = await resolveUserAuthzGrants(ql, userId, {
579+
tenantId,
580+
nowMs: input.nowMs,
581+
seedPermissions: ctx.permissions,
582+
seedEmail: ctx.email,
583+
});
584+
}
585+
435586
ctx.positions = grants.positions;
436587
ctx.permissions = grants.permissions;
437588
ctx.systemPermissions = grants.systemPermissions;

0 commit comments

Comments
 (0)