From 12964793cf72a9335b1ca771f4fcdc9038fb4930 Mon Sep 17 00:00:00 2001 From: Marc Villain Date: Mon, 2 Nov 2026 00:22:35 +0100 Subject: [PATCH] fix: list routes answer for permitted orgs only; org lists go through the hooks --- CHANGELOG.md | 9 +++++ package.json | 2 +- src/index.ts | 1 + src/org-paths.ts | 10 +++--- src/permissions.ts | 32 ++++++++++++++++++ src/routes/activities.ts | 17 ++++++---- src/routes/delegations.ts | 7 ++-- src/routes/orgs.ts | 37 ++++++++++++++------- src/routes/permission-profiles.ts | 5 +-- src/routes/roles.ts | 12 ++++--- src/routes/rule-cross-org.ts | 9 ++--- src/routes/rule-role.ts | 7 ++-- src/routes/rule-user.ts | 7 ++-- src/routes/user-crud.ts | 11 ++++--- src/routes/user-org-filter.ts | 10 ++---- src/routes/user-roles.ts | 5 +-- test/permitted-scope.test.mjs | 55 +++++++++++++++++++++++++++++++ 17 files changed, 179 insertions(+), 57 deletions(-) create mode 100644 test/permitted-scope.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index b13efc9..1ed194d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,15 @@ All notable changes to this package will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.2.0] - 2026-11-02 + +### Added +- `permittedScope(db, req, reply, orgIds, activity, view)`: the orgs of a list request the caller may act in, through the hooks; 403 only when none passes. + +### Fixed +- The list routes for users, roles, role settings, role, user and cross-org rules, delegations, permission profiles, user roles and activities checked only the first org of `X-Org-Ids`, then answered for every listed org. A caller allowed in the first org read the others' rows, and a caller refused in the first org got 403 for the whole list. They now answer for the permitted orgs only. +- The unpinned org list, the stored org filter and `readableOrgPaths` decided with `morbac.is_allowed` in SQL, which skips the permission hooks. They now go through them, so an org the hooks refuse is never offered. + ## [1.1.0] - 2026-10-21 ### Added diff --git a/package.json b/package.json index dbb4f0a..544a0ce 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@crudy/pgmorbac-fastify", - "version": "1.1.0", + "version": "1.2.0", "type": "module", "description": "Fastify layer over pgmorbac: permission checks, RLS session context, management routes, PostgREST data proxy, migration loader", "keywords": [ diff --git a/src/index.ts b/src/index.ts index a6ce40e..3f2681f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,6 +8,7 @@ export { hasAnyPermission, isAllowed, permittedOrgs, + permittedScope, type OrgTarget, type OrgSelection, } from './permissions.js'; diff --git a/src/org-paths.ts b/src/org-paths.ts index 8c5e689..03bc72b 100644 --- a/src/org-paths.ts +++ b/src/org-paths.ts @@ -1,4 +1,5 @@ import type { Pool } from 'pg'; +import { permittedOrgs } from './permissions.js'; /** * Wraps a query selecting the visible orgs (id, name, parent_id, metadata) and @@ -26,10 +27,11 @@ export function withOrgPaths(visSql: string): string { * the path start at the topmost org they can read, never above it. */ export async function readableOrgPaths(db: Pool, userId: string, view: string): Promise> { + const { rows: candidates } = await db.query<{ id: string }>( + `SELECT id FROM morbac.orgs WHERE morbac.is_allowed($1::UUID, id, 'read', $2)`, [userId, view]); + const readable = await permittedOrgs(db, userId, candidates.map((c) => c.id), 'read', view); const { rows } = await db.query<{ id: string; path: string[] }>( - withOrgPaths( - `SELECT id, name, parent_id, NULL::jsonb AS metadata - FROM morbac.orgs WHERE morbac.is_allowed($1::UUID, id, 'read', $2)`), - [userId, view]); + withOrgPaths('SELECT id, name, parent_id, NULL::jsonb AS metadata FROM morbac.orgs WHERE id = ANY($1::UUID[])'), + [readable]); return new Map(rows.map((r) => [r.id, r.path])); } diff --git a/src/permissions.ts b/src/permissions.ts index bde3cf4..49320a3 100644 --- a/src/permissions.ts +++ b/src/permissions.ts @@ -304,6 +304,38 @@ export async function hasPermissionAny( return true; } +/** + * The orgs of a list request the caller may read in, or null once a 401 or 403 + * is sent. An unpinned request (no org header) is decided on the no-org + * perimeter and answers []. A pinned one keeps only the orgs that pass, hooks + * included, and is refused only when none does: one refused org neither fails + * the list nor widens it to orgs the caller cannot read. + */ +export async function permittedScope( + db: Pool, + req: FastifyRequest, + reply: FastifyReply, + orgIds: string[], + activity: string, + target: string, +): Promise { + if (orgIds.length === 0) return await hasPermission(db, req, reply, null, activity, target) ? [] : null; + const user = getUser(req); + if (!user) { + reply.code(401).send({ error: 'Authentication required' }); + return null; + } + const apiKeyScope = ((req as unknown) as Record).apiKeyScope as ApiKeyScope | undefined; + const allowed = scopeAllows(apiKeyScope, activity, target) + ? await permittedOrgs(db, user.id, orgIds, activity, target, req) + : []; + if (allowed.length === 0) { + reply.code(403).send({ error: 'Insufficient permissions' }); + return null; + } + return allowed; +} + /** * The one permission decision every helper makes: morbac's answer, narrowed by * the application's permission hooks (`addPermissionHook`). Use it instead of diff --git a/src/routes/activities.ts b/src/routes/activities.ts index 677405c..ef27122 100644 --- a/src/routes/activities.ts +++ b/src/routes/activities.ts @@ -1,6 +1,6 @@ import type { FastifyInstance } from 'fastify'; import type { Pool } from 'pg'; -import { getUser, getOrgScope, hasPermission } from '../permissions.js'; +import { getUser, getOrgScope, permittedScope } from '../permissions.js'; import { unauthorized, badRequest, forbidden } from '../schemas.js'; export function createActivityRoutes(db: Pool) { @@ -31,7 +31,8 @@ export function createActivityRoutes(db: Pool) { if (!user) return reply.code(401).send({ error: 'Authentication required' }); const orgIds = getOrgScope(req, reply, false); if (orgIds === null) return reply; - if (!await hasPermission(db, req, reply, orgIds[0] ?? null, 'read', 'rules')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'rules'); + if (!scope) return reply; const { rows } = await db.query('SELECT name, description FROM morbac.activities ORDER BY name'); return rows; }); @@ -70,7 +71,8 @@ export function createActivityRoutes(db: Pool) { const orgIds = getOrgScope(req, reply, false); if (orgIds === null) return reply; - if (!await hasPermission(db, req, reply, orgIds[0] ?? null, 'read', 'users')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'users'); + if (!scope) return reply; const { name } = req.params as { name: string }; const { rows } = await db.query( @@ -132,7 +134,8 @@ export function createActivityRoutes(db: Pool) { if (!user) return reply.code(401).send({ error: 'Authentication required' }); const orgIds = getOrgScope(req, reply, false); if (orgIds === null) return reply; - if (!await hasPermission(db, req, reply, orgIds[0] ?? null, 'read', 'rules')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'rules'); + if (!scope) return reply; // no_org marks a view whose rows carry no org, so callers can offer the // unattributed grant instead of a tree-scoped rule that could never match. const { rows } = await db.query( @@ -178,7 +181,8 @@ export function createActivityRoutes(db: Pool) { const orgIds = getOrgScope(req, reply, false); if (orgIds === null) return reply; - if (!await hasPermission(db, req, reply, orgIds[0] ?? null, 'read', 'users')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'users'); + if (!scope) return reply; const { name } = req.params as { name: string }; const { rows } = await db.query( @@ -239,7 +243,8 @@ export function createActivityRoutes(db: Pool) { if (!user) return reply.code(401).send({ error: 'Authentication required' }); const orgIds = getOrgScope(req, reply, false); if (orgIds === null) return reply; - if (!await hasPermission(db, req, reply, orgIds[0] ?? null, 'read', 'rules')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'rules'); + if (!scope) return reply; const { rows } = await db.query( 'SELECT activity, view FROM morbac.activity_view_bindings ORDER BY activity, view', ); diff --git a/src/routes/delegations.ts b/src/routes/delegations.ts index 04b22f4..faede2d 100644 --- a/src/routes/delegations.ts +++ b/src/routes/delegations.ts @@ -1,6 +1,6 @@ import type { FastifyInstance } from 'fastify'; import type { Pool } from 'pg'; -import { getUser, getOrgScope, orgScopeExpr, hasPermission, canActOnUser, SYSTEM_ORG_ID } from '../permissions.js'; +import { getUser, getOrgScope, orgScopeExpr, hasPermission, permittedScope, canActOnUser, SYSTEM_ORG_ID } from '../permissions.js'; import { delegationResponse, notFound, forbidden, unauthorized, badRequest } from '../schemas.js'; const SELECT_COLUMNS = ` @@ -30,9 +30,10 @@ export function createDelegationRoutes(db: Pool) { const orgIds = getOrgScope(req, reply, false); if (orgIds === null) return reply; - if (!await hasPermission(db, req, reply, orgIds[0] ?? null, 'read', 'delegations')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'delegations'); + if (!scope) return reply; - const { expr, params } = orgScopeExpr(orgIds, 'd.org_id'); + const { expr, params } = orgScopeExpr(scope, 'd.org_id'); const { rows } = await db.query( `SELECT ${SELECT_COLUMNS} FROM morbac.delegations d diff --git a/src/routes/orgs.ts b/src/routes/orgs.ts index 02969f0..24cdd64 100644 --- a/src/routes/orgs.ts +++ b/src/routes/orgs.ts @@ -3,11 +3,12 @@ import { withMorbacContext } from '../rls-context.js'; import type { Pool } from 'pg'; import type { CheckLimitFn, OrgEventSink } from '../plugin.js'; import { - getUser, getOrgScope, orgScopeExpr, hasPermission, SYSTEM_ORG_ID, + getUser, getOrgScope, orgScopeExpr, hasPermission, permittedScope, permittedOrgs, SYSTEM_ORG_ID, } from '../permissions.js'; import { orgResponse, notFound, forbidden, unauthorized, badRequest } from '../schemas.js'; import { withOrgPaths } from '../org-paths.js'; +import { runPreHooks, runPostHooks } from '../permission-hooks.js'; interface OrgRow { id: string; name: string; parent_id: string | null; metadata?: Record } @@ -40,25 +41,37 @@ export function createOrgRoutes(db: Pool, checkLimit?: CheckLimitFn, emitOrgEven // caller in it even with no read/orgs rule, and the org filter is // what supplies X-Org-Id, so dropping those orgs leaves such a // user with an empty perimeter and an app that does nothing. + // Both ways in still go through the permission hooks, so an org + // the hooks refuse never reaches the filter and every request + // pinned to it fails. + const { rows: candidates } = await db.query<{ id: string; member: boolean }>( + `SELECT o.id, EXISTS (SELECT 1 FROM morbac.user_roles ur WHERE ur.org_id = o.id AND ur.user_id = $1) AS member + FROM morbac.orgs o + WHERE morbac.is_allowed($1::UUID, o.id, 'read', 'orgs') + OR EXISTS (SELECT 1 FROM morbac.user_roles ur WHERE ur.org_id = o.id AND ur.user_id = $1)`, + [user.id], + ); + const readable = new Set(await permittedOrgs(db, user.id, candidates.map((c) => c.id), 'read', 'orgs', req)); + const visible: string[] = []; + for (const c of candidates) { + const check = { userId: user.id, orgId: c.id, activity: 'read', view: 'orgs', req }; + if (readable.has(c.id) || (c.member && await runPreHooks(check, db) && await runPostHooks(check, true, db))) visible.push(c.id); + } const { rows } = await withMorbacContext(db, { userId: user.id }, (client) => client.query( - withOrgPaths( - `SELECT id, name, parent_id, metadata FROM morbac.orgs o - WHERE morbac.is_allowed($1::UUID, o.id, 'read', 'orgs') - OR EXISTS (SELECT 1 FROM morbac.user_roles ur - WHERE ur.org_id = o.id AND ur.user_id = $1)`, - ), - [user.id], + withOrgPaths('SELECT id, name, parent_id, metadata FROM morbac.orgs WHERE id = ANY($1::UUID[])'), + [visible], ), ); return rows; } - if (!await hasPermission(db, req, reply, orgIds[0], 'read', 'orgs')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'orgs'); + if (!scope) return reply; - const isSystemScope = orgIds.includes(SYSTEM_ORG_ID); - const { expr, params } = isSystemScope ? { expr: '', params: [] } : orgScopeExpr(orgIds, 'id'); - const { rows } = await withMorbacContext(db, { userId: user.id, orgId: orgIds[0] }, (client) => + const isSystemScope = scope.includes(SYSTEM_ORG_ID); + const { expr, params } = isSystemScope ? { expr: '', params: [] } : orgScopeExpr(scope, 'id'); + const { rows } = await withMorbacContext(db, { userId: user.id, orgId: scope[0] }, (client) => client.query( withOrgPaths(`SELECT id, name, parent_id, metadata FROM morbac.orgs ${expr ? `WHERE ${expr}` : ''}`), params, diff --git a/src/routes/permission-profiles.ts b/src/routes/permission-profiles.ts index 1990701..1ec24cf 100644 --- a/src/routes/permission-profiles.ts +++ b/src/routes/permission-profiles.ts @@ -1,7 +1,7 @@ import type { FastifyInstance } from 'fastify'; import type { Pool } from 'pg'; import type { CheckLimitFn, RoleEventSink } from '../plugin.js'; -import { getUser, getOrgScope, hasPermission, SYSTEM_ORG_ID } from '../permissions.js'; +import { getUser, getOrgScope, hasPermission, permittedScope, SYSTEM_ORG_ID } from '../permissions.js'; import { roleResponse, notFound, forbidden, unauthorized, badRequest } from '../schemas.js'; import { loadProfile, validateGrantable, applyProfileRules } from './profile-apply.js'; @@ -58,7 +58,8 @@ export function createPermissionProfileRoutes(db: Pool, checkLimit?: CheckLimitF const orgIds = getOrgScope(req, reply, false); if (orgIds === null) return reply; - if (!await hasPermission(db, req, reply, orgIds[0] ?? null, 'read', 'roles')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'roles'); + if (!scope) return reply; const { module_id } = req.query as { module_id?: string }; const { rows } = await db.query( diff --git a/src/routes/roles.ts b/src/routes/roles.ts index 6d71a29..7a14aad 100644 --- a/src/routes/roles.ts +++ b/src/routes/roles.ts @@ -2,7 +2,7 @@ import type { FastifyInstance } from 'fastify'; import type { Pool } from 'pg'; import type { CheckLimitFn, RoleEventSink } from '../plugin.js'; import { - getUser, getOrgScope, orgScopeExpr, hasPermission, SYSTEM_ORG_ID, + getUser, getOrgScope, orgScopeExpr, hasPermission, permittedScope, SYSTEM_ORG_ID, } from '../permissions.js'; import { parseListParams, composeWhere, setContentRange, ListQueryError } from '../list-query.js'; import { roleResponse, notFound, forbidden, unauthorized, badRequest } from '../schemas.js'; @@ -30,9 +30,10 @@ export function createRoleRoutes(db: Pool, checkLimit?: CheckLimitFn, emitRoleEv const orgIds = getOrgScope(req, reply, false); if (orgIds === null) return reply; - if (!await hasPermission(db, req, reply, orgIds[0] ?? null, 'read', 'roles')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'roles'); + if (!scope) return reply; - const { expr, params } = orgScopeExpr(orgIds, 'r.org_id'); + const { expr, params } = orgScopeExpr(scope, 'r.org_id'); let list; try { list = parseListParams(req.query as Record, { @@ -214,9 +215,10 @@ export function createRoleRoutes(db: Pool, checkLimit?: CheckLimitFn, emitRoleEv const orgIds = getOrgScope(req, reply, false); if (orgIds === null) return reply; - if (!await hasPermission(db, req, reply, orgIds[0] ?? null, 'read', 'roles')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'roles'); + if (!scope) return reply; - const { expr, params } = orgScopeExpr(orgIds, 'rs.org_id'); + const { expr, params } = orgScopeExpr(scope, 'rs.org_id'); const { rows } = await db.query( `SELECT rh.senior_role_id, rs.name AS senior_name, rh.junior_role_id, rj.name AS junior_name, diff --git a/src/routes/rule-cross-org.ts b/src/routes/rule-cross-org.ts index 88f4464..1471d23 100644 --- a/src/routes/rule-cross-org.ts +++ b/src/routes/rule-cross-org.ts @@ -1,6 +1,6 @@ import type { FastifyInstance } from 'fastify'; import type { Pool } from 'pg'; -import { getUser, getOrgScope, orgScopeExpr, hasPermission, isActivityViewBound, SYSTEM_ORG_ID } from '../permissions.js'; +import { getUser, getOrgScope, orgScopeExpr, hasPermission, permittedScope, isActivityViewBound, SYSTEM_ORG_ID } from '../permissions.js'; import { crossOrgRuleResponse, notFound, forbidden, unauthorized, badRequest } from '../schemas.js'; export function createCrossOrgRuleRoutes(db: Pool) { @@ -23,10 +23,11 @@ export function createCrossOrgRuleRoutes(db: Pool) { const orgIds = getOrgScope(req, reply, false); if (orgIds === null) return reply; - if (!await hasPermission(db, req, reply, orgIds[0] ?? null, 'read', 'cross_org_rules')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'cross_org_rules'); + if (!scope) return reply; - const { expr: srcExpr, params: srcParams } = orgScopeExpr(orgIds, 'cor.source_org_id'); - const { expr: tgtExpr, params: tgtParams } = orgScopeExpr(orgIds, 'cor.target_org_id', srcParams.length); + const { expr: srcExpr, params: srcParams } = orgScopeExpr(scope, 'cor.source_org_id'); + const { expr: tgtExpr, params: tgtParams } = orgScopeExpr(scope, 'cor.target_org_id', srcParams.length); const where = srcExpr && tgtExpr ? `WHERE (${srcExpr} OR ${tgtExpr})` : srcExpr ? `WHERE ${srcExpr}` : tgtExpr ? `WHERE ${tgtExpr}` diff --git a/src/routes/rule-role.ts b/src/routes/rule-role.ts index 258a2e3..5b2540d 100644 --- a/src/routes/rule-role.ts +++ b/src/routes/rule-role.ts @@ -1,6 +1,6 @@ import type { FastifyInstance } from 'fastify'; import type { Pool } from 'pg'; -import { getUser, getOrgScope, orgScopeExpr, hasPermission, canGrantRule, isActivityViewBound, SYSTEM_ORG_ID } from '../permissions.js'; +import { getUser, getOrgScope, orgScopeExpr, hasPermission, permittedScope, canGrantRule, isActivityViewBound, SYSTEM_ORG_ID } from '../permissions.js'; import { ruleResponse, notFound, forbidden, unauthorized, badRequest } from '../schemas.js'; export function createRoleRuleRoutes(db: Pool) { @@ -23,9 +23,10 @@ export function createRoleRuleRoutes(db: Pool) { const orgIds = getOrgScope(req, reply, false); if (orgIds === null) return reply; - if (!await hasPermission(db, req, reply, orgIds[0] ?? null, 'read', 'rules')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'rules'); + if (!scope) return reply; - const { expr, params } = orgScopeExpr(orgIds, 'r.org_id'); + const { expr, params } = orgScopeExpr(scope, 'r.org_id'); const { rows } = await db.query( `SELECT r.id, r.role_id, r.org_id, o.name AS org_name, ro.name AS role_name, r.activity, r.view, r.modality, r.priority, r.scope, diff --git a/src/routes/rule-user.ts b/src/routes/rule-user.ts index fc3efa6..efb9e45 100644 --- a/src/routes/rule-user.ts +++ b/src/routes/rule-user.ts @@ -1,6 +1,6 @@ import type { FastifyInstance } from 'fastify'; import type { Pool } from 'pg'; -import { getUser, getOrgScope, orgScopeExpr, hasPermission, isActivityViewBound } from '../permissions.js'; +import { getUser, getOrgScope, orgScopeExpr, hasPermission, permittedScope, isActivityViewBound } from '../permissions.js'; import { userRuleResponse, notFound, forbidden, unauthorized, badRequest } from '../schemas.js'; export function createUserRuleRoutes(db: Pool) { @@ -23,9 +23,10 @@ export function createUserRuleRoutes(db: Pool) { const orgIds = getOrgScope(req, reply, false); if (orgIds === null) return reply; - if (!await hasPermission(db, req, reply, orgIds[0] ?? null, 'read', 'user_rules')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'user_rules'); + if (!scope) return reply; - const { expr, params } = orgScopeExpr(orgIds, 'ur.org_id'); + const { expr, params } = orgScopeExpr(scope, 'ur.org_id'); const { rows } = await db.query( `SELECT ur.id, ur.user_id, ur.org_id, COALESCE(u.name, ur.user_id::TEXT) AS user_name, diff --git a/src/routes/user-crud.ts b/src/routes/user-crud.ts index 93a252c..443c5d0 100644 --- a/src/routes/user-crud.ts +++ b/src/routes/user-crud.ts @@ -3,7 +3,7 @@ import type { Pool } from 'pg'; import bcrypt from 'bcryptjs'; import type { CheckLimitFn } from '../plugin.js'; import { - getUser, getOrgScope, orgScopeExpr, hasPermission, canActOnUser, checkApiKeyScope, + getUser, getOrgScope, orgScopeExpr, hasPermission, permittedScope, canActOnUser, checkApiKeyScope, getAccountType, viewForType, isAllowed, SYSTEM_ORG_ID, } from '../permissions.js'; import { userResponse, notFound, forbidden, unauthorized, badRequest } from '../schemas.js'; @@ -30,9 +30,10 @@ export function createUserCrudRoutes(db: Pool, checkLimit?: CheckLimitFn) { const orgIds = getOrgScope(req, reply, false); if (orgIds === null) return reply; - if (!await hasPermission(db, req, reply, orgIds[0] ?? null, 'read', 'users')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'users'); + if (!scope) return reply; - const checkOrg = orgIds[0] ?? null; + const checkOrg = scope[0] ?? null; // A user awaiting placement belongs to no org, so seeing one is the // no-org perimeter (a NULL org), not a claim about every org. Any // grant that reaches it answers: a role rule scoped unattributed or @@ -43,8 +44,8 @@ export function createUserCrudRoutes(db: Pool, checkLimit?: CheckLimitFn) { isAllowed(db, user.id, null, 'read', 'users', req), ]); - const { expr, params } = orgScopeExpr(orgIds, 'ur2.org_id'); - const isSystemScope = orgIds.includes(SYSTEM_ORG_ID); + const { expr, params } = orgScopeExpr(scope, 'ur2.org_id'); + const isSystemScope = scope.includes(SYSTEM_ORG_ID); const placement = canReadUnattached ? ' OR u.needs_placement' : ''; const whereClause = expr && !isSystemScope ? `WHERE (u.id IN (SELECT ur2.user_id FROM morbac.user_roles ur2 WHERE ${expr})${placement} OR u.account_type IN ('system', 'service'))` diff --git a/src/routes/user-org-filter.ts b/src/routes/user-org-filter.ts index 37493d6..63d22e3 100644 --- a/src/routes/user-org-filter.ts +++ b/src/routes/user-org-filter.ts @@ -1,6 +1,6 @@ import type { FastifyInstance, FastifyReply } from 'fastify'; import type { Pool } from 'pg'; -import { getUser, canActOnUser, checkApiKeyScope, getAccountType, viewForType } from '../permissions.js'; +import { getUser, canActOnUser, permittedOrgs, checkApiKeyScope, getAccountType, viewForType } from '../permissions.js'; import { notFound, forbidden, unauthorized, badRequest } from '../schemas.js'; const MAX_ORG_FILTER = 100; @@ -21,12 +21,8 @@ const orgFilterResponse = { * direction can be used to probe orgs outside the caller's reach. */ async function visibleOrgIds(db: Pool, userId: string, ids: string[]): Promise { if (ids.length === 0) return []; - const { rows } = await db.query<{ id: string }>( - `SELECT o.id FROM morbac.orgs o - WHERE o.id = ANY($2::UUID[]) AND morbac.is_allowed($1::UUID, o.id, 'read', 'orgs')`, - [userId, ids], - ); - return rows.map((r) => r.id); + const { rows } = await db.query<{ id: string }>('SELECT id FROM morbac.orgs WHERE id = ANY($1::UUID[])', [ids]); + return permittedOrgs(db, userId, rows.map((r) => r.id), 'read', 'orgs'); } async function storedOrgFilter(db: Pool, userId: string): Promise { diff --git a/src/routes/user-roles.ts b/src/routes/user-roles.ts index c0a6dfd..5d292c9 100644 --- a/src/routes/user-roles.ts +++ b/src/routes/user-roles.ts @@ -1,6 +1,6 @@ import type { FastifyInstance } from 'fastify'; import type { Pool } from 'pg'; -import { getUser, getOrgScope, hasPermission, getAccountType } from '../permissions.js'; +import { getUser, getOrgScope, hasPermission, permittedScope, getAccountType } from '../permissions.js'; import { notFound, forbidden, unauthorized, badRequest } from '../schemas.js'; import { callerControlsRoleOrg } from './role-authority.js'; @@ -99,7 +99,8 @@ export function createUserRoleRoutes(db: Pool) { const orgIds = getOrgScope(req, reply, false); if (orgIds === null) return reply; - if (!await hasPermission(db, req, reply, orgIds[0] ?? null, 'read', 'users')) return reply; + const scope = await permittedScope(db, req, reply, orgIds, 'read', 'users'); + if (!scope) return reply; const { id } = req.params as { id: string }; const exists = await db.query('SELECT 1 FROM app.users WHERE id = $1', [id]); diff --git a/test/permitted-scope.test.mjs b/test/permitted-scope.test.mjs new file mode 100644 index 0000000..4422782 --- /dev/null +++ b/test/permitted-scope.test.mjs @@ -0,0 +1,55 @@ +import { test, beforeEach } from 'node:test'; +import assert from 'node:assert/strict'; +import { addPermissionHook, permittedScope } from '../dist/index.js'; + +const A = '00000000-0000-4000-8000-00000000000a'; +const B = '00000000-0000-4000-8000-00000000000b'; + +function fakeDb(allowed) { + return { + async query(sql, params) { + if (sql.includes('unnest')) return { rows: params[1].filter((o) => allowed.has(o)).map((o) => ({ o })) }; + if (sql.includes('morbac.is_allowed')) return { rows: [{ allowed: allowed.has(params[1]) }] }; + throw new Error(`unexpected query ${sql}`); + }, + }; +} + +function fakeReply() { + return { status: 200, code(c) { this.status = c; return this; }, send() { return this; } }; +} + +const req = { user: { id: 'u' } }; +let removers = []; +beforeEach(() => { removers.forEach((r) => r()); removers = []; }); + +test('a pinned list keeps only the orgs the caller may read', async () => { + const reply = fakeReply(); + assert.deepEqual(await permittedScope(fakeDb(new Set([B])), req, reply, [A, B], 'read', 'users'), [B]); + assert.equal(reply.status, 200); +}); + +test('a refused first org no longer fails the list', async () => { + removers.push(addPermissionHook('pre', (c) => c.orgId !== A)); + assert.deepEqual(await permittedScope(fakeDb(new Set([A, B])), req, fakeReply(), [A, B], 'read', 'users'), [B]); +}); + +test('a list with no permitted org is refused', async () => { + const reply = fakeReply(); + assert.equal(await permittedScope(fakeDb(new Set()), req, reply, [A, B], 'read', 'users'), null); + assert.equal(reply.status, 403); +}); + +test('an unpinned list is decided on the no-org perimeter', async () => { + assert.deepEqual(await permittedScope(fakeDb(new Set([null])), req, fakeReply(), [], 'read', 'users'), []); + const reply = fakeReply(); + assert.equal(await permittedScope(fakeDb(new Set()), req, reply, [], 'read', 'users'), null); + assert.equal(reply.status, 403); +}); + +test('an API key outside its scope gets nothing', async () => { + const reply = fakeReply(); + const keyed = { ...req, apiKeyScope: [{ activity: 'read', view: 'roles' }] }; + assert.equal(await permittedScope(fakeDb(new Set([A])), keyed, reply, [A], 'read', 'users'), null); + assert.equal(reply.status, 403); +});