Skip to content

fix(auth): resolve the id-shaped platform-admin question in one place#11686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-sam merged 1 commit into from
Aug 24, 2026
Merged

fix(auth): resolve the id-shaped platform-admin question in one place #11686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/one-id-shaped-platform-admin-judge.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
---
"@objectstack/core": minor
"@objectstack/plugin-auth": patch
---

**Security:** the "is this user id a platform admin?" question is now asked in exactly one place, and the two copies that answered it differently are gone (#10348, #10949).

ADR-0068 D2 defines platform standing as one thing — an unscoped `admin_full_access` grant, held now. `core/security/resolve-authz-context.ts` is the declared authority for authorization derivation and its header states that every entry point must resolve through it and never re-read the grant tables itself. `plugin-auth`'s `auth-manager.ts` did exactly that twice: once inside the `customSession` callback, and once in the predicate that authorizes `/sso/register` and, through the impersonation oracle, `/admin/impersonate-user`. Both copies are deleted. Both callers — and the session payload — now ask `hasPlatformAdminStanding(engine, userId)`, a projection of `resolveUserAuthzGrants` exported from `@objectstack/core`, so a platform-admin verdict is derived in one place for the whole platform.

**What that changes, and it is a tightening on all three counts.** The deleted copies applied neither the ADR-0091 validity window nor the ADR-0049 `active` check, and resolved `admin_full_access` by matching a name over a page of the permission-set catalogue. The authority applies both checks before any derivation and resolves the set by id. So:

- an **expired** platform-admin grant no longer authorizes `/sso/register` or `/admin/impersonate-user`, and no longer appears in the session payload;
- a **deactivated** `admin_full_access` permission set no longer confers platform standing anywhere — the deactivation dialog's promise now holds on these gates too;
- an environment holding **more permission sets than a single catalogue page** can no longer lose the `admin_full_access` row and demote every platform admin at once.

**One behaviour widens, and it was ruled deliberately** (maintainer, 2026-08-24). The `customSession` copy read without a system identity while the other read with one. The single authority reads as system, so on a strictly org-scoped deployment the session payload stops under-reporting platform admin — the fail-closed drift between the payload and the gates ends. Open-core composition is unaffected: the two reads reached identical rows there already.

**The org boundary is unchanged and now pinned at both gates.** An org owner, an org admin, a `TENANT_ADMIN`-posture principal and an org-scoped `admin_full_access` grant are all refused — the `PLATFORM_ADMIN` rung derives from the unscoped capability grant alone. The predicate takes an engine and a user id and nothing else: it deliberately does not accept the resolver's caller-supplied seeds, so no part of a request can supply part of its own verdict.

Population queries are a different kind and are untouched: `ensure-default-organization.ts` asks *which* user is the platform admin, which a per-user predicate cannot express.
4 changes: 4 additions & 0 deletions packages/core/src/security/index.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -88,6 +88,10 @@ export {
export {
resolveAuthzContext,
resolveUserAuthzGrants,
// [#10348] The ONE id-shaped platform-admin predicate (ADR-0068 D2). Every
// surface that only knows a user id asks this instead of re-reading
// `sys_*_permission_set` — the prohibition resolve-authz-context.ts states.
hasPlatformAdminStanding,
resolveLocalizationContext,
type ResolvedAuthzContext,
type ResolveAuthzInput,
Expand Down
56 changes: 56 additions & 0 deletions packages/core/src/security/resolve-authz-context.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -627,6 +627,62 @@ export async function resolveUserAuthzGrants(
return grants;
}

/**
* hasPlatformAdminStanding — the ID-SHAPED platform-admin question, asked in
* exactly one place.
*
* ADR-0068 D2 defines PLATFORM standing as one thing: an UNSCOPED
* (`organization_id = null`) `sys_user_permission_set` grant on the
* `admin_full_access` set, held **now**. A surface that only knows a user id —
* a session-payload derivation, a platform-operator route gate, an
* impersonation oracle — asks here, so it never has to re-read the grant tables
* itself, which is the prohibition this module's header states.
*
* It is a PROJECTION of {@link resolveUserAuthzGrants}, never a second
* derivation: the answer is the `PLATFORM_ADMIN` rung of the posture ladder,
* and that rung is derived from the unscoped-grant evidence and nothing else.
* Everything that governs those grants therefore applies here by construction
* and cannot drift from it — the ADR-0091 validity window (§6), the ADR-0049
* `active` flag on the catalogue row (§6b), the system-identity read, and the
* resolution of `admin_full_access` BY ID rather than by scanning a page of the
* catalogue. Each of those was missing from a hand-written copy of this
* predicate; none of them can be missing from a projection.
*
* ⛔ Read the RUNG — never `positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN)`.
* The positions list is wider on purpose: an ADR-0057 D4 `sys_user_position`
* row may spell that very name, and a platform-RBAC assignment is not the D2
* capability grant. The two readings genuinely differ, so the narrow one is the
* one that gets a name here.
*
* ⛔ The options are deliberately NOT {@link ResolveUserAuthzGrantsOptions}.
* That type carries caller-supplied seeds (`seedEmail`, `seedPermissions`) for
* transports that already resolved part of a principal; an authorization
* predicate that accepted them would let a caller supply part of its own
* verdict. Clock injection is the only thing a caller may pass, so this
* function's answer is a function of `(ql, userId)` and the stored rows alone.
*
* ⚠️ This is the PER-USER predicate. The POPULATION question ("which user is
* the platform admin?" — `ensure-default-organization.ts`) is a different kind
* and is deliberately not expressible through it; do not widen this to serve
* it.
*
* Fail-CLOSED: an empty id, a missing engine, or any unreadable lookup answers
* `false`. This backs security gates, and an unverifiable actor never passes.
*/
export async function hasPlatformAdminStanding(
ql: any,
userId: string,
opts: { nowMs?: number } = {},
): Promise<boolean> {
if (!ql || typeof userId !== 'string' || userId.length === 0) return false;
try {
const grants = await resolveUserAuthzGrants(ql, userId, { nowMs: opts.nowMs });
return grants.posture === 'PLATFORM_ADMIN';
} catch {
return false;
}
}

// ── Localization (ADR-0053 Phase 2) ─────────────────────────────────────────

function isValidTimeZone(tz: string): boolean {
Expand Down
113 changes: 51 additions & 62 deletions packages/plugins/plugin-auth/src/auth-manager.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,11 @@ import type {
OidcProvidersConfig,
} from '@objectstack/spec/system';
import type { IDataEngine } from '@objectstack/core';
// [#10348] The ONE id-shaped platform-admin predicate (ADR-0068 D2).
// `auth-manager` used to re-derive that standing itself, in two spellings
// that had drifted from the declared authority and from each other; both
// now ask the authority. Nothing in this file reads the grant tables.
import { hasPlatformAdminStanding } from '@objectstack/core';
import type { IEmailService, ISmsService } from '@objectstack/spec/contracts';
import {
readEnvWithDeprecation,
Expand DownExpand Up@@ -3030,7 +3035,7 @@ export class AuthManager {
// `roles: string[]` array (ADR-0068 D1/D2): the stored `user.role` scalar
// split on commas, PLUS the active membership mapped to canonical
// `org_owner`/`org_admin`/`org_member`, PLUS `platform_admin` when the user
// holds the admin_full_access permission set. `user.isPlatformAdmin` is a
// resolves as a platform admin (ADR-0068 D2). `user.isPlatformAdmin` is a
// derived alias of `'platform_admin' in positions`.
//
// IMPORTANT: `user.role` is NOT overwritten anymore — consumers must gate
Expand All@@ -3043,9 +3048,10 @@ export class AuthManager {
// Better-auth's `sys_user` table doesn't carry a `role` column. We derive
// it from two sources:
//
// 1. **Platform admin** — a `sys_user_permission_set` row that points at
// the `admin_full_access` permission set with `organization_id = null`
// (seeded by `bootstrapPlatformAdmin`).
// 1. **Platform admin** — the ADR-0068 D2 standing, resolved through
// `core/security/resolve-authz-context.ts` (the single authority for
// authorization derivation) and never re-read here. See
// `isPlatformAdminUserId` below.
// 2. **Organization admin** — a `sys_member` row in the user's *active*
// organization (`session.activeOrganizationId`) with role `owner` or
// `admin`. Org owners/admins are entitled to manage org-scoped
Expand All@@ -3061,29 +3067,6 @@ export class AuthManager {
plugins.push(customSession(async ({ user, session }) => {
if (!user?.id) return { user, session };

const isPlatformAdmin = async (): Promise<boolean> => {
try {
const links = await dataEngine.find('sys_user_permission_set', {
where: { user_id: user.id },
limit: 50,
});
const platformLinks = (Array.isArray(links) ? links : []).filter(
(l: any) => !l.organization_id,
);
if (platformLinks.length === 0) return false;
const sets = await dataEngine.find('sys_permission_set', { limit: 50 });
const adminSet = (Array.isArray(sets) ? sets : []).find(
(r: any) => r.name === 'admin_full_access',
);
if (!adminSet) return false;
return platformLinks.some(
(l: any) => l.permission_set_id === adminSet.id,
);
} catch {
return false;
}
};

// ADR-0068 D2 — surface CANONICAL org_* role names (not a boolean flag):
// a membership owner/admin/member maps to org_owner/org_admin/org_member.
const activeOrgRoles = async (): Promise<string[]> => {
Expand DownExpand Up@@ -3112,7 +3095,10 @@ export class AuthManager {
// positions[] (identity names + position names), with NO singular
// overwrite. isPlatformAdmin is a DERIVED alias of
// `'platform_admin' in positions`, retained for back-compat clients.
const platformAdmin = await isPlatformAdmin();
// [#10348] Asked through the ONE authority, exactly as `/sso/register`
// and `/admin/impersonate-user` ask it — so the session payload can no
// longer disagree with the gates about who a platform admin is.
const platformAdmin = await this.isPlatformAdminUserId(user.id);
const orgRoles = await activeOrgRoles();
const storedRole = typeof (user as any).role === 'string' ? (user as any).role : '';
const positions = Array.from(new Set([
Expand DownExpand Up@@ -4503,48 +4489,51 @@ export class AuthManager {
}

/**
* ADR-0068 D2, asked on its own: is `userId` a PLATFORM admin — a
* `sys_user_permission_set` row pointing at the `admin_full_access`
* permission set with `organization_id = null` (seeded by
* `bootstrapPlatformAdmin`)?
* ADR-0068 D2, asked on its own: is `userId` a PLATFORM admin?
*
* ⭐ [#10348] This is NOT a judge. It is this manager's engine binding over
* `hasPlatformAdminStanding` — the one place in the tree that turns a user id
* into that boolean, in `core/security/resolve-authz-context.ts`, whose header
* states that every entry point must resolve authorization through it and
* never re-read `sys_*_permission_set` itself. This file used to do exactly
* what that forbids, twice: here, and again inside the `customSession`
* callback. Both copies are gone; both callers land here, and this method
* performs no derivation of its own. Adding one back is the regression.
*
* Deliberately does NOT admit organization owners/admins. Platform-admin
* routes must not be reachable by whoever happens to own an org (ADR-0068).
* [#10009] This replaced an `isOrgOrPlatformAdmin` predicate that admitted
* both; once `/sso/register` stopped asking the org question, that wider
* predicate had no caller left and was removed rather than parked.
* The three call sites are `/sso/register`'s ADR-0024 before-hook, the
* `/admin/impersonate-user` oracle (both the caller and the protected-target
* question), and the `customSession` payload — which is why the payload can
* no longer report a different answer from the gates.
*
* ⛔ The legacy `user.role === 'admin'` scalar is NOT consulted here. This
* asks the permission-set question only — the channel ADR-0068 D2 keeps.
* [#10949] What the copies had drifted away from, and what the authority
* applies: the ADR-0091 validity window on the grant, the ADR-0049 `active`
* flag on the catalogue row, and a lookup of `admin_full_access` BY ID rather
* than by name over a page of the catalogue. The first two mean an expired or
* deactivated grant no longer authorizes anything; the third means an
* environment with more permission sets than that page held can no longer
* demote every platform admin at once.
*
* Reads through `withSystemReadContext` so the lookups are not themselves
* RLS-scoped to the acting — possibly non-privileged — user, and fails CLOSED
* (returns false) on any lookup error: this backs a security gate, and an
* unverifiable actor must never pass.
* Deliberately does NOT admit organization owners/admins — the authority
* derives the `PLATFORM_ADMIN` rung from the UNSCOPED capability grant alone,
* so an org owner/admin (and a TENANT_ADMIN-posture principal) is refused.
* [#10009] That boundary was established when `/sso/register` stopped asking
* the org question; it is pinned, at both gates, in
* `platform-admin-standing.consolidation.test.ts`.
*
* ⛔ The legacy `user.role === 'admin'` scalar is NOT consulted. ⛔ Nor is any
* caller-supplied seed: the predicate takes `(engine, userId)` and reads the
* stored rows, so nothing about a request can supply part of its own verdict.
*
* The authority reads with system identity, so the lookups are not themselves
* RLS-scoped to the acting — possibly non-privileged — user. Fails CLOSED
* (returns false) on an empty id, a missing engine, or any lookup error: this
* backs security gates, and an unverifiable actor must never pass.
*/
private async isPlatformAdminUserId(userId: string): Promise<boolean> {
if (!userId) return false;
const engine = this.getDataEngine();
if (!engine) return false;
try {
const sys = withSystemReadContext(engine);
const links = await sys.find('sys_user_permission_set', {
where: { user_id: userId },
limit: 50,
});
const platformLinks = (Array.isArray(links) ? links : []).filter(
(l: any) => !l.organization_id,
);
if (platformLinks.length === 0) return false;
const sets = await sys.find('sys_permission_set', { limit: 50 });
const adminSet = (Array.isArray(sets) ? sets : []).find(
(r: any) => r.name === 'admin_full_access',
);
if (!adminSet) return false;
return platformLinks.some((l: any) => l.permission_set_id === adminSet.id);
} catch {
return false;
}
return hasPlatformAdminStanding(engine, userId);
}

/**
Expand Down
Loading
Loading