Skip to content

Commit 6f467ef

Browse files
os-zhuangclaude
andcommitted
fix(security): platform admins see all rows of better-auth-managed identity objects (ADR-0024 / cloud#551)
managedBy:'better-auth' identity tables (sys_oauth_application, sys_account, sys_session, sys_sso_provider, …) are written via better-auth's adapter with no tenant context → organization_id never stamped → member_default's wildcard tenant_isolation RLS hides every row from a platform admin (empty Setup lists). Extend the posture-gated superuser bypass (currently private/tenancyDisabled) to managedBy:'better-auth' objects — ADMIN-ONLY (members never trigger the bypass; their _self carve-outs / tenant_isolation still apply; the flag is NOT used in the wildcard-drop, so no member leak). +2 regression tests (admin bypassed, member tenant-scoped). 145 security tests green; E2E: admin sees all sessions, plain member sees own. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 51bec81 commit 6f467ef

3 files changed

Lines changed: 76 additions & 5 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
Security: platform admins see all rows of better-auth-managed identity objects (ADR-0024 / cloud#551)
6+
7+
Identity tables managed by the auth library (`managedBy: 'better-auth'``sys_oauth_application`, `sys_account`, `sys_session`, `sys_sso_provider`, …) are written by better-auth's own adapter with **no tenant context**, so `organization_id` is never stamped and `member_default`'s wildcard `tenant_isolation` RLS denies every row — a platform admin's Setup list (OAuth Applications, Identity Links, …) renders **empty**.
8+
9+
These objects now get the **same posture-gated superuser bypass** as `private` / `tenancy.enabled:false` objects, so a platform admin's `viewAllRecords` sees all identity rows env-wide. This is **admin-only**: non-admins never trigger the bypass — their `_self` carve-outs / `tenant_isolation` still apply (verified by a regression test that a member stays tenant-scoped), and the flag is deliberately **not** used for the wildcard-policy drop, so it can never leak rows to members.
10+
11+
Fixes the empty-list symptom across all better-auth-managed Setup objects without per-object `tenancy` changes (which would risk the control plane, where some of these objects ARE cross-env-isolated).

packages/plugins/plugin-security/src/security-plugin.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,54 @@ describe('SecurityPlugin', () => {
483483
await expect(harness.run(opCtx)).resolves.toBeDefined();
484484
expect(harness.findOne).not.toHaveBeenCalled();
485485
});
486+
487+
// ADR-0024 / cloud#551 — `managedBy: 'better-auth'` identity tables get the
488+
// SAME posture-gated superuser bypass as private/non-tenant objects (their
489+
// rows are written without a tenant stamp, so the wildcard tenant_isolation
490+
// would otherwise hide every row from a platform admin).
491+
it('ALLOWS the platform admin and BYPASSES wildcard RLS on a better-auth-managed object', async () => {
492+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'admin_full_access' });
493+
const harness = makeMiddlewareCtx({
494+
permissionSets: [adminSet],
495+
objectFields: ['id', 'organization_id', 'provider_id'],
496+
schemaExtra: { managedBy: 'better-auth' }, // NOT private, NOT tenancy-disabled
497+
orgScoping: true,
498+
});
499+
await plugin.init(harness.ctx);
500+
await plugin.start(harness.ctx);
501+
const opCtx: any = {
502+
object: 'task', operation: 'find', ast: { where: undefined },
503+
context: { userId: 'admin', tenantId: 'org-1', roles: ['admin_full_access'], permissions: [] },
504+
};
505+
await expect(harness.run(opCtx)).resolves.toBeDefined();
506+
expect(opCtx.ast.where).toBeUndefined(); // bypassed — admin sees all identity rows
507+
});
508+
509+
it('does NOT bypass for a non-admin on a better-auth-managed object (no leak)', async () => {
510+
const memberWithRls: PermissionSet = {
511+
name: 'member_default', label: 'Member', isProfile: true,
512+
objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } },
513+
rowLevelSecurity: [
514+
{ name: 'tenant_isolation', object: '*', operation: 'all', using: 'organization_id = current_user.organization_id' },
515+
],
516+
} as any;
517+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
518+
const harness = makeMiddlewareCtx({
519+
permissionSets: [memberWithRls],
520+
objectFields: ['id', 'organization_id', 'provider_id'],
521+
schemaExtra: { managedBy: 'better-auth' },
522+
orgScoping: true,
523+
});
524+
await plugin.init(harness.ctx);
525+
await plugin.start(harness.ctx);
526+
const opCtx: any = {
527+
object: 'task', operation: 'find', ast: { where: undefined },
528+
context: { userId: 'u1', tenantId: 'org-1', roles: [], permissions: [] },
529+
};
530+
await harness.run(opCtx);
531+
// The member is still tenant-scoped — the managedBy bypass is admin-only.
532+
expect(opCtx.ast.where).toEqual({ organization_id: 'org-1' });
533+
});
486534
});
487535

488536
// -------------------------------------------------------------------------

packages/plugins/plugin-security/src/security-plugin.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ export class SecurityPlugin implements Plugin {
120120
* `requiredPermissions` capability contract. Populated lazily from the schema;
121121
* cleared on metadata change alongside the other schema-derived caches.
122122
*/
123-
private readonly objectSecurityMetaCache = new Map<string, { isPrivate: boolean; tenancyDisabled: boolean; requiredPermissions: string[]; fieldRequiredPermissions: Record<string, string[]> }>();
123+
private readonly objectSecurityMetaCache = new Map<string, { isPrivate: boolean; tenancyDisabled: boolean; isBetterAuthManaged: boolean; requiredPermissions: string[]; fieldRequiredPermissions: Record<string, string[]> }>();
124124
private dbLoader?: (names: string[]) => Promise<PermissionSet[]>;
125125
private logger: { info?: (...a: any[]) => void; warn?: (...a: any[]) => void; error?: (...a: any[]) => void } = {};
126126

@@ -382,7 +382,7 @@ export class SecurityPlugin implements Plugin {
382382
const secMeta =
383383
permissionSets.length > 0
384384
? await this.getObjectSecurityMeta(opCtx.object)
385-
: { isPrivate: false, tenancyDisabled: false, requiredPermissions: [] as string[], fieldRequiredPermissions: {} as Record<string, string[]> };
385+
: { isPrivate: false, tenancyDisabled: false, isBetterAuthManaged: false, requiredPermissions: [] as string[], fieldRequiredPermissions: {} as Record<string, string[]> };
386386

387387
// 1.5. [ADR-0066 D3] requiredPermissions AND-gate — a capability
388388
// prerequisite checked BEFORE the CRUD grant (ADR §Precedence): a
@@ -981,7 +981,7 @@ export class SecurityPlugin implements Plugin {
981981
// posture gate ensures this never grants cross-tenant visibility on ordinary
982982
// tenant business objects.
983983
const meta = await this.getObjectSecurityMeta(object);
984-
if (meta.isPrivate || meta.tenancyDisabled) {
984+
if (meta.isPrivate || meta.tenancyDisabled || meta.isBetterAuthManaged) {
985985
const isWrite = operation === 'insert' || operation === 'update' || operation === 'delete';
986986
const bypass = isWrite
987987
? this.permissionEvaluator.hasSuperuserWriteBypass(object, permissionSets, { isPrivate: meta.isPrivate })
@@ -1036,7 +1036,7 @@ export class SecurityPlugin implements Plugin {
10361036
// check) on private/platform-global objects.
10371037
const meta = await this.getObjectSecurityMeta(object);
10381038
if (
1039-
(meta.isPrivate || meta.tenancyDisabled) &&
1039+
(meta.isPrivate || meta.tenancyDisabled || meta.isBetterAuthManaged) &&
10401040
this.permissionEvaluator.hasSuperuserWriteBypass(object, permissionSets, { isPrivate: meta.isPrivate })
10411041
) {
10421042
return null;
@@ -1244,7 +1244,7 @@ export class SecurityPlugin implements Plugin {
12441244
*/
12451245
private async getObjectSecurityMeta(
12461246
object: string,
1247-
): Promise<{ isPrivate: boolean; tenancyDisabled: boolean; requiredPermissions: string[]; fieldRequiredPermissions: Record<string, string[]> }> {
1247+
): Promise<{ isPrivate: boolean; tenancyDisabled: boolean; isBetterAuthManaged: boolean; requiredPermissions: string[]; fieldRequiredPermissions: Record<string, string[]> }> {
12481248
const cached = this.objectSecurityMetaCache.get(object);
12491249
if (cached) return cached;
12501250
let obj: any = typeof this.ql?.getSchema === 'function' ? this.ql.getSchema(object) : null;
@@ -1270,6 +1270,18 @@ export class SecurityPlugin implements Plugin {
12701270
isPrivate: (obj as any)?.access?.default === 'private',
12711271
tenancyDisabled:
12721272
(obj as any)?.tenancy?.enabled === false || (obj as any)?.systemFields?.tenant === false,
1273+
// Identity-infrastructure tables managed by the auth library
1274+
// (`managedBy: 'better-auth'`: sys_user, sys_account, sys_session,
1275+
// sys_oauth_application, sys_sso_provider, …). Their rows are written by
1276+
// better-auth's own adapter with no tenant context, so `organization_id`
1277+
// is never stamped and the wildcard `tenant_isolation` RLS denies them —
1278+
// making a platform admin's `viewAllRecords` see an empty list. Treat
1279+
// them like the private/non-tenant posture for the SUPERUSER BYPASS ONLY
1280+
// (so the platform super-admin sees all identity rows env-wide). This does
1281+
// NOT relax member RLS (members never trigger the bypass; their `_self`
1282+
// carve-outs / tenant_isolation still apply) and is NOT used for the
1283+
// wildcard-policy drop below, so it can never leak rows to non-admins.
1284+
isBetterAuthManaged: (obj as any)?.managedBy === 'better-auth',
12731285
requiredPermissions: Array.isArray((obj as any)?.requiredPermissions)
12741286
? (obj as any).requiredPermissions.map(String)
12751287
: [],

0 commit comments

Comments
 (0)