From 663d7d203781b21deaf53c01adcbc81f2fb4913a Mon Sep 17 00:00:00 2001 From: Maksym Pipkun Date: Mon, 14 Sep 2026 14:11:56 +0300 Subject: [PATCH 01/11] feat: centralize access checks in scoped resource API Column-level access rules (backendOnly, editReadonly, showIn, allowModifyWhenNotShowIn*) lived as private helpers inside restApi.ts, so every plugin re-implemented them by hand and each copy dropped a different check. Add adminforth.resource(id).asUser(adminUser, { meta }) and .asSystem({ hooks }) as the single entry point for resource access, backed by one pipeline: - columnAccess.ts owns backendOnly/showIn resolution, record write policy, read stripping, and aggregation column exposure - resourceAccess.ts owns interpretResource, breaking the restApi/connector import cycle - create_record, update_record, delete_record and aggregate go through the scoped API, so the core path and the plugin path cannot drift apart - createResourceRecord/updateResourceRecord/deleteResourceRecord and unscoped resource() calls remain as deprecated aliases with runtime warnings - a denied or failed operation is always visible: reads and delete throw, create and update resolve to { ok, error } Also fixes /aggregate, which resolved allowedActions with a single ListRequest pass and then required allowedActions.show, which that source always sets to false, so the endpoint rejected every request. list and show are now resolved separately. --- adminforth/dataConnectors/baseConnector.ts | 2 +- .../tutorial/03-Customization/11-dataApi.md | 104 +++- .../tutorial/03-Customization/12-security.md | 5 +- adminforth/index.ts | 75 ++- adminforth/modules/columnAccess.ts | 281 +++++++++++ adminforth/modules/operationalResource.ts | 470 +++++++++++++++++- adminforth/modules/resourceAccess.ts | 47 ++ adminforth/modules/restApi.ts | 421 ++++------------ adminforth/modules/utils.ts | 31 +- adminforth/types/Back.ts | 90 +++- .../operational_resource_scope.test.ts | 281 +++++++++++ 11 files changed, 1408 insertions(+), 399 deletions(-) create mode 100644 adminforth/modules/columnAccess.ts create mode 100644 adminforth/modules/resourceAccess.ts create mode 100644 tests/jest_tests/operational_resource_scope.test.ts diff --git a/adminforth/dataConnectors/baseConnector.ts b/adminforth/dataConnectors/baseConnector.ts index 55fbf9f03..f52c5e03a 100644 --- a/adminforth/dataConnectors/baseConnector.ts +++ b/adminforth/dataConnectors/baseConnector.ts @@ -10,7 +10,7 @@ import type { AdminUser } from "../types/Common.js" import { suggestIfTypo } from "../modules/utils.js"; import { decodeRecordId, encodeRecordId, isCompositePrimaryKey, primaryKeyColumnNames, primaryKeyColumns } from "../modules/recordId.js"; -import { interpretResource } from "../modules/restApi.js"; +import { interpretResource } from "../modules/resourceAccess.js"; import { ActionCheckSource, AdminForthDataTypes, AdminForthFilterOperators, AdminForthSortDirections, AllowedActionsEnum } from "../types/Common.js"; import { randomUUID } from "crypto"; import dayjs from "dayjs"; diff --git a/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md b/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md index 37e368f55..3bd099d09 100644 --- a/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md +++ b/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md @@ -29,11 +29,75 @@ const admin = new AdminForth({ }); // get the resource object -await admin.resource('adminuser').get(Filters.EQ('id', '1234')); +await admin.resource('adminuser').asSystem({ hooks: false }).get(Filters.EQ('id', '1234')); ``` Here we will show you how to use the Data API with simple examples. +## Access scope + +Choose an explicit access scope for new code which works with resource data: + +```ts +const users = admin.resource('adminuser'); + +await users.asUser(adminUser, { meta }).create(record); +await users.asSystem({ meta }).create(record); +await users.asSystem({ hooks: false }).create(record); +``` + +`asUser()` enforces the resource ACL and column access rules, validates the +record, and runs lifecycle hooks. `asSystem()` skips ACL and column access but +still validates the record and runs hooks. Pass `{ hooks: false }` for a trusted +connector-level operation which still performs normalization and validation. + +A denied or failed operation is always visible. `get`, `list`, `count`, +`aggregate` and `delete` throw, since their return value carries no room for an +error; `create` and `update` resolve to `{ ok: false, error }`. `delete` returns +`false` when the record simply did not exist: + +```ts +const { ok, error } = await users.asUser(adminUser, { meta }).update(id, updates); + +try { + const deleted = await users.asUser(adminUser, { meta }).delete('1234'); + // deleted === false means there was no such record +} catch (e) { + // ACL denial, cascade failure, or a hook rejection +} +``` + +When the caller has already loaded the record, pass it in so the scoped call does +not read it a second time and hooks see the same snapshot the caller worked from: + +```ts +await users.asUser(adminUser, { meta, oldRecord }).update(recordId, updates); +await users.asUser(adminUser, { meta, record }).delete(recordId); +``` + +An optional `adminUser` can be attached to a system operation when hooks need +user attribution without enabling user ACL checks: + +```ts +await users.asSystem({ adminUser, meta }).create(record); +``` + +Deprecated unscoped calls remain aliases for the trusted, hook-free scope for +backward compatibility: + +```ts +await admin.resource('adminuser').create(record); +await admin.resource('adminuser').asSystem({ hooks: false }).create(record); +``` + +The two calls have the same behavior. This applies to `get`, `list`, `count`, +`aggregate`, `create`, `update`, and `delete`. Unscoped calls will be removed in +the next major version, so use an explicit scope in new code. + +The legacy `admin.createResourceRecord`, `admin.updateResourceRecord`, and +`admin.deleteResourceRecord` methods are deprecated and will be removed in the +next major version. + ## Get one item from database @@ -48,7 +112,7 @@ Signature: Get item by ID: ```ts -const user = await admin.resource('adminuser').get( +const user = await admin.resource('adminuser').asSystem({ hooks: false }).get( [Filters.EQ('id', '1234')] ); ``` @@ -56,7 +120,7 @@ const user = await admin.resource('adminuser').get( Check School with name 'Hawkins Elementary' exits in DB ```ts -const schoolExists = !!(await admin.resource('schools').get( +const schoolExists = !!(await admin.resource('schools').asSystem({ hooks: false }).get( [Filters.EQ('name', 'Hawkins Elementary')] )); ``` @@ -65,7 +129,7 @@ const schoolExists = !!(await admin.resource('schools').get( Get user with name 'John' and role not 'SuperAdmin' ```ts -const user = await admin.resource('adminuser').get( +const user = await admin.resource('adminuser').asSystem({ hooks: false }).get( Filters.EQ('name', 'John'), Filters.NEQ('role', 'SuperAdmin') ); @@ -88,7 +152,7 @@ Signature: Get 15 latest users which role is not Admin: ```ts -const users = await admin.resource('adminuser').list( +const users = await admin.resource('adminuser').asSystem({ hooks: false }).list( [Filters.NEQ('role', 'Admin')], 15, 0, Sorts.DESC('createdAt') ); ``` @@ -96,19 +160,19 @@ const users = await admin.resource('adminuser').list( Get 10 oldest users (with highest age): ```ts -const users = await admin.resource('adminuser').list([], 10, 0, Sorts.ASC('age')); +const users = await admin.resource('adminuser').asSystem({ hooks: false }).list([], 10, 0, Sorts.ASC('age')); ``` Get next page of oldest users: ```ts -const users = await admin.resource('adminuser').list([], 10, 10, Sorts.ASC('age')); +const users = await admin.resource('adminuser').asSystem({ hooks: false }).list([], 10, 10, Sorts.ASC('age')); ``` Get 10 schools, sort by rating first, then oldest by founded year: ```ts -const schools = await admin.resource('schools').list( +const schools = await admin.resource('schools').asSystem({ hooks: false }).list( [], 10, 0, [Sorts.DESC('rating'), Sorts.ASC('foundedYear')] ); ``` @@ -116,7 +180,7 @@ const schools = await admin.resource('schools').list( Get all users that have gmail address AND the ones created not in 2024 ```ts -const users = await admin.resource('adminuser').list( +const users = await admin.resource('adminuser').asSystem({ hooks: false }).list( Filters.AND( Filters.LIKE('email', '@gmail.com'), Filters.OR( @@ -134,7 +198,7 @@ Technically it happened that AdminForth allows you to do this also ```js const minUgcAge = 18; -const usersWithNoUgcAccess = await admin.resource('adminuser').list( +const usersWithNoUgcAccess = await admin.resource('adminuser').asSystem({ hooks: false }).list( [ Filters.NEQ('role', 'Admin'), { @@ -172,7 +236,7 @@ Returns value representing created item with all fields, including fields which Create a new school: ```ts -await admin.resource('schools').create({ +await admin.resource('schools').asSystem().create({ name: 'Hawkins Elementary', rating: 5, foundedYear: 1950, @@ -194,7 +258,7 @@ Returns number of items in database which match the filters. Count number of schools with rating above 4: ```ts -const schoolsCount = await admin.resource('schools').count(Filters.GT('rating', 4)); +const schoolsCount = await admin.resource('schools').asSystem({ hooks: false }).count(Filters.GT('rating', 4)); ``` Create data for daily report with number of users signed up daily for last 7 days: @@ -211,7 +275,7 @@ const dailyReports = await Promise.all( const dateEnd = new Date(dateStart); dateEnd.setDate(dateEnd.getDate() + 1); - return admin.resource('adminuser').count( + return admin.resource('adminuser').asSystem({ hooks: false }).count( [Filters.GTE('createdAt', dateStart.toISOString()), Filters.LT('createdAt', dateEnd.toISOString())] ); }) @@ -234,7 +298,7 @@ Signature: Update school rating to 4.8 ```ts -await admin.resource('schools').update('1234', { rating: 4.8 }); +await admin.resource('schools').asSystem().update('1234', { rating: 4.8 }); ``` ## Delete item from database @@ -250,7 +314,7 @@ Signature: Delete school with ID '1234' ```ts -await admin.resource('schools').delete('1234'); +await admin.resource('schools').asSystem().delete('1234'); ``` @@ -266,10 +330,10 @@ Golden rule: create one index per query you are going to use often or where you For example if you have two queries: ```ts -const users = await admin.resource('adminuser').list( +const users = await admin.resource('adminuser').asSystem({ hooks: false }).list( [Filters.NEQ('role', 'Admin')], 15, 0, Sorts.DESC('createdAt') ); -const users = await admin.resource('adminuser').list( +const users = await admin.resource('adminuser').asSystem({ hooks: false }).list( [Filters.EQ('name', 'John'), Filters.NEQ('role', 'SuperAdmin')] ); ``` @@ -388,7 +452,7 @@ With explicit grouping aliases: ### Get daily apartment stats (count, avg, sum, median) for listed apartments ```ts -const rows = await admin.resource('apartments').aggregate( +const rows = await admin.resource('apartments').asSystem({ hooks: false }).aggregate( Filters.EQ('listed', true), { count: Aggregates.count(), @@ -413,7 +477,7 @@ median('price') → median price ### Get apartment stats grouped by country ```ts -const rows = await admin.resource('apartments').aggregate( +const rows = await admin.resource('apartments').asSystem({ hooks: false }).aggregate( [], { count: Aggregates.count(), @@ -433,7 +497,7 @@ What is happening here: ### Get apartment stats grouped by country and month ```ts -const rows = await admin.resource('apartments').aggregate( +const rows = await admin.resource('apartments').asSystem({ hooks: false }).aggregate( [], { count: Aggregates.count(), diff --git a/adminforth/documentation/docs/tutorial/03-Customization/12-security.md b/adminforth/documentation/docs/tutorial/03-Customization/12-security.md index 014921526..5baed23b4 100644 --- a/adminforth/documentation/docs/tutorial/03-Customization/12-security.md +++ b/adminforth/documentation/docs/tutorial/03-Customization/12-security.md @@ -118,8 +118,9 @@ This is opt-in. It is especially important for the column configured as `auth.us | Path | When `normalize` runs | | --- | --- | -| AdminForth CRUD (`createResourceRecord`, `updateResourceRecord`) | Before validation and `beforeSave` hooks | -| Data API (`admin.resource(...).create()` and `.update()`) | Before the record reaches the connector | +| Scoped Data API with hooks (`admin.resource(...).asUser(...)` or `.asSystem()`) | Before validation and `beforeSave` hooks | +| Hook-free Data API (`admin.resource(...).asSystem({ hooks: false })` or an unscoped compatibility call) | Before validation and the connector operation | +| Deprecated AdminForth CRUD (`createResourceRecord`, `updateResourceRecord`) | Before validation and `beforeSave` hooks | | Core password login | On the submitted value of `auth.usernameField`, before the user lookup | | Reads and filters | Never — this includes `get`, `list`, `count`, search, and `Filters.EQ` | diff --git a/adminforth/index.ts b/adminforth/index.ts index 4e0f0b48a..eab43eb65 100644 --- a/adminforth/index.ts +++ b/adminforth/index.ts @@ -36,7 +36,8 @@ import { import AdminForthPlugin from './basePlugin.js'; import ConfigValidator from './modules/configValidator.js'; -import AdminForthRestAPI, { interpretResource, rejectApiRawFilters } from './modules/restApi.js'; +import AdminForthRestAPI, { rejectApiRawFilters } from './modules/restApi.js'; +import { interpretResource } from './modules/resourceAccess.js'; import OperationalResource from './modules/operationalResource.js'; import SocketBroker from './modules/socketBroker.js'; import { afLogger } from './modules/logger.js'; @@ -663,7 +664,17 @@ class AdminForth implements IAdminForth { this.operationalResources = {}; this.config.resources.forEach((resource) => { - this.operationalResources[resource.resourceId] = new OperationalResource(this.connectors[resource.dataSource], resource); + this.operationalResources[resource.resourceId] = new OperationalResource( + this.connectors[resource.dataSource], + resource, + this, + { + create: (params) => this.executeCreateResourceRecord(params), + update: (params) => this.executeUpdateResourceRecord(params), + delete: (params) => this.executeDeleteResourceRecord(params), + validate: (targetResource, record, mode) => this.validateRecordValues(targetResource, record, mode), + }, + ); }); const adminforthSecret = process.env.ADMINFORTH_SECRET; @@ -777,11 +788,19 @@ class AdminForth implements IAdminForth { /** * Create record and execute hooks + * @deprecated Will be removed in the next major version. Use the scoped resource API. * @param params - Parameters for record creation. See CreateResourceRecordParams. * @returns Result of record creation. See CreateResourceRecordResult. */ async createResourceRecord( params: CreateResourceRecordParams, + ): Promise { + this.warnDeprecatedResourceMutation('createResourceRecord', params.resource.resourceId, 'create'); + return this.executeCreateResourceRecord(params); + } + + private async executeCreateResourceRecord( + params: CreateResourceRecordParams, ): Promise { const { resource, record, adminUser, extra, response } = params; @@ -871,11 +890,25 @@ class AdminForth implements IAdminForth { * record is partial record with only changed fields * * Update record by id and execute hooks - * @param params - Parameters for record update. See UpdateResourceRecordParams. - * @returns Result of record update. See UpdateResourceRecordResult. + * @deprecated Will be removed in the next major version. Use the scoped resource API. + * @param params - Parameters for record update. See UpdateResourceRecordParams. + * @returns Result of record update. See UpdateResourceRecordResult. */ async updateResourceRecord( params: UpdateResourceRecordParams, + ): Promise { + this.warnDeprecatedResourceMutation('updateResourceRecord', params.resource.resourceId, 'update'); + const dataToUse = params.updates || params.record; + for (const column of params.resource.columns.filter((candidate) => candidate.editReadonly)) { + if (column.name in dataToUse) { + delete dataToUse[column.name]; + } + } + return this.executeUpdateResourceRecord(params); + } + + private async executeUpdateResourceRecord( + params: UpdateResourceRecordParams, ): Promise { const { resource, recordId, record, oldRecord, adminUser, response, extra, updates } = params; const dataToUse = updates || record; @@ -889,12 +922,6 @@ class AdminForth implements IAdminForth { afLogger.warn(`updateResourceRecord function received 'record' param which is deprecated and will be removed in future version, please use 'updates' instead.`); } - // remove editReadonly columns from record - for (const column of resource.columns.filter((col) => col.editReadonly)) { - if (column.name in dataToUse) - delete dataToUse[column.name]; - } - // execute hook if needed for (const hook of listify(resource.hooks?.edit?.beforeSave)) { const resp = await hook({ @@ -959,11 +986,19 @@ class AdminForth implements IAdminForth { /** * Delete record by id and execute hooks + * @deprecated Will be removed in the next major version. Use the scoped resource API. * @param params - Parameters for record deletion. See DeleteResourceRecordParams. * @returns Result of record deletion. See DeleteResourceRecordResult. */ async deleteResourceRecord( params: DeleteResourceRecordParams, + ): Promise { + this.warnDeprecatedResourceMutation('deleteResourceRecord', params.resource.resourceId, 'delete'); + return this.executeDeleteResourceRecord(params); + } + + private async executeDeleteResourceRecord( + params: DeleteResourceRecordParams, ): Promise { const { resource, recordId, adminUser, record, response, extra } = params; // execute hook if needed @@ -1006,6 +1041,26 @@ class AdminForth implements IAdminForth { return { error: null }; } + private warnedDeprecatedResourceMutations = new Set(); + + private warnDeprecatedResourceMutation( + method: 'createResourceRecord' | 'updateResourceRecord' | 'deleteResourceRecord', + resourceId: string, + operation: 'create' | 'update' | 'delete', + ): void { + // these run on every CRUD action of every plugin, so warn once per resource and method + const warnKey = `${resourceId}.${method}`; + if (this.warnedDeprecatedResourceMutations.has(warnKey)) { + return; + } + this.warnedDeprecatedResourceMutations.add(warnKey); + afLogger.warn( + `${method} is deprecated and will be removed in the next major version. ` + + `Use adminforth.resource('${resourceId}').asUser(adminUser, { meta }).${operation}(...) ` + + `or adminforth.resource('${resourceId}').asSystem({ hooks: false }).${operation}(...) instead.`, + ); + } + async runAction({ resourceId, actionId, diff --git a/adminforth/modules/columnAccess.ts b/adminforth/modules/columnAccess.ts new file mode 100644 index 000000000..804ca7295 --- /dev/null +++ b/adminforth/modules/columnAccess.ts @@ -0,0 +1,281 @@ +import type { + AdminForthResource, + AllowedActionValue, + BackendOnlyInput, + IAdminForth, +} from '../types/Back.js'; +import { + ActionCheckSource, + type AdminUser, +} from '../types/Common.js'; + +export interface ColumnAccessContext { + adminUser: AdminUser; + resource: AdminForthResource; + meta: any; + source: ActionCheckSource; + adminforth: IAdminForth; +} + +export async function resolveBoolOrFn( + value: BackendOnlyInput | AllowedActionValue | undefined, + context: ColumnAccessContext, +): Promise { + if (typeof value === 'function') { + return !!(await value(context)); + } + return !!value; +} + +export async function isBackendOnly( + column: AdminForthResource['columns'][number], + context: ColumnAccessContext, +): Promise { + return resolveBoolOrFn(column.backendOnly, context); +} + +export async function isShown( + column: AdminForthResource['columns'][number], + page: 'list' | 'show' | 'edit' | 'create' | 'filter', + context: ColumnAccessContext, +): Promise { + const showIn = column.showIn as Record | undefined; + if (showIn?.[page] !== undefined) { + return resolveBoolOrFn(showIn[page], context); + } + if (showIn?.all !== undefined) { + return resolveBoolOrFn(showIn.all, context); + } + return true; +} + +export interface AssertRecordWritableParams { + resource: AdminForthResource; + record: Record; + mode: 'create' | 'edit'; + adminUser: AdminUser; + meta: any; + adminforth: IAdminForth; +} + +export async function assertRecordWritable({ + resource, + record, + mode, + adminUser, + meta, + adminforth, +}: AssertRecordWritableParams): Promise { + const context: ColumnAccessContext = { + adminUser, + resource, + meta, + source: mode === 'create' ? ActionCheckSource.CreateRequest : ActionCheckSource.EditRequest, + adminforth, + }; + + for (const column of resource.columns) { + const fieldName = column.name; + if (!(fieldName in record)) { + continue; + } + + const shown = await isShown(column, mode, context); + const backendOnly = await isBackendOnly(column, context); + + if (backendOnly) { + throw new Error( + `Field "${fieldName}" cannot be modified as it is restricted from ${mode === 'create' ? 'creation' : 'editing'} (backendOnly is true).`, + ); + } + + if (mode === 'create') { + if ( + !shown + && !column.fillOnCreate + && !column.allowModifyWhenNotShowInCreate + ) { + throw new Error( + `Field "${fieldName}" cannot be modified as it is restricted from creation (showIn.create is false). If you need to set this hidden field during creation, either configure column.fillOnCreate or set column.allowModifyWhenNotShowInCreate = true.`, + ); + } + continue; + } + + if (column.editReadonly) { + throw new Error( + `Field "${fieldName}" cannot be modified as it is restricted from editing (editReadonly is true).`, + ); + } + + if (!shown && !column.allowModifyWhenNotShowInEdit) { + throw new Error( + `Field "${fieldName}" cannot be modified as it is restricted from editing (showIn.edit is false). If you need to allow updating this hidden field during editing, set column.allowModifyWhenNotShowInEdit = true.`, + ); + } + } +} + +export interface StripReadForbiddenColumnsParams { + resource: AdminForthResource; + record: Record; + adminUser: AdminUser; + meta: any; + source: ActionCheckSource; + adminforth: IAdminForth; +} + +export async function stripReadForbiddenColumns({ + resource, + record, + adminUser, + meta, + source, + adminforth, +}: StripReadForbiddenColumnsParams): Promise> { + const context: ColumnAccessContext = { + adminUser, + resource, + meta, + source, + adminforth, + }; + + for (const key of Object.keys(record)) { + const column = resource.columns.find((candidate) => candidate.name === key); + if (!column || await isBackendOnly(column, context)) { + delete record[key]; + } + } + + return record; +} + +/** + * Collects every column name referenced anywhere in a (possibly nested) filter tree, + * so the caller can check those columns against the visibility rules. + */ +export function collectFilterFields(filters: any, fields: Set = new Set()): Set { + if (!filters || typeof filters !== 'object') { + return fields; + } + + if (Array.isArray(filters)) { + filters.forEach((filter) => collectFilterFields(filter, fields)); + return fields; + } + + if (typeof filters.field === 'string') { + fields.add(filters.field); + } + if (typeof filters.rightField === 'string') { + fields.add(filters.rightField); + } + if (Array.isArray(filters.subFilters)) { + filters.subFilters.forEach((filter) => collectFilterFields(filter, fields)); + } + + return fields; +} + +export interface AssertFilterColumnsReadableParams { + resource: AdminForthResource; + filters: any; + adminUser: AdminUser; + meta: any; + source: ActionCheckSource; + adminforth: IAdminForth; +} + +/** + * Filter values are never echoed back, but combined with any readable output they turn into an + * oracle which reads a hidden value out one comparison at a time, so backendOnly columns must not + * be filterable either. + */ +export async function assertFilterColumnsReadable({ + resource, + filters, + adminUser, + meta, + source, + adminforth, +}: AssertFilterColumnsReadableParams): Promise { + const context: ColumnAccessContext = { adminUser, resource, meta, source, adminforth }; + + for (const fieldName of collectFilterFields(filters)) { + const column = resource.columns.find((candidate) => candidate.name === fieldName); + if (column && await isBackendOnly(column, context)) { + throw new Error(`Filter: column "${fieldName}" cannot be used (backendOnly is true).`); + } + } +} + +export interface AssertColumnsAggregatableParams { + resource: AdminForthResource; + aggregations?: { [alias: string]: { field?: string } }; + groupBy?: { field?: string } | Array<{ field?: string }>; + filters?: any; + adminUser: AdminUser; + meta: any; + adminforth: IAdminForth; +} + +/** + * A column may only take part in an aggregation if the user could have read the very same value + * from the show view, otherwise min/max/groupBy become a way to read hidden columns. + */ +export async function assertColumnsAggregatable({ + resource, + aggregations, + groupBy, + filters, + adminUser, + meta, + adminforth, +}: AssertColumnsAggregatableParams): Promise { + const context: ColumnAccessContext = { + adminUser, + resource, + meta, + source: ActionCheckSource.ShowRequest, + adminforth, + }; + + const assertExposable = async (fieldName: string, label: string): Promise => { + const column = resource.columns.find((candidate) => candidate.name === fieldName); + if (!column) { + throw new Error(`${label}: unknown column "${fieldName}"`); + } + if (await isBackendOnly(column, context)) { + throw new Error(`${label}: column "${fieldName}" cannot be aggregated (backendOnly is true).`); + } + if (!await isShown(column, 'show', context)) { + throw new Error(`${label}: column "${fieldName}" cannot be aggregated (showIn.show is false).`); + } + }; + + for (const [alias, rule] of Object.entries(aggregations || {})) { + // plain count does not reference any column + if (!rule?.field) { + continue; + } + await assertExposable(rule.field, `Aggregation "${alias}"`); + } + + const groupByRules = Array.isArray(groupBy) ? groupBy : (groupBy ? [groupBy] : []); + for (const groupByRule of groupByRules) { + if (!groupByRule?.field) { + continue; + } + await assertExposable(groupByRule.field, 'GroupBy'); + } + + await assertFilterColumnsReadable({ + resource, + filters, + adminUser, + meta, + source: ActionCheckSource.ShowRequest, + adminforth, + }); +} diff --git a/adminforth/modules/operationalResource.ts b/adminforth/modules/operationalResource.ts index be8006ab6..60f8d4e23 100644 --- a/adminforth/modules/operationalResource.ts +++ b/adminforth/modules/operationalResource.ts @@ -1,7 +1,52 @@ -import { IAdminForthSingleFilter, IAdminForthAndOrFilter, IAdminForthSort, IOperationalResource, IAdminForthDataSourceConnectorBase, AdminForthResource, IAggregationRule, IGroupByRule } from '../types/Back.js'; +import type { + AdminForthResource, + CreateResourceRecordParams, + CreateResourceRecordResult, + DeleteResourceRecordParams, + DeleteResourceRecordResult, + IAdminForth, + IAdminForthAndOrFilter, + IAdminForthDataSourceConnectorBase, + IAdminForthSingleFilter, + IAdminForthSort, + IAggregationRule, + IGroupByRule, + IOperationalResource, + IScopedOperationalResource, + OperationalResourceSystemOptions, + OperationalResourceUserOptions, + UpdateResourceRecordParams, + UpdateResourceRecordResult, +} from '../types/Back.js'; +import { ActionCheckSource, AllowedActionsEnum, type AdminUser } from '../types/Common.js'; import { compositePkValues } from './recordId.js'; -import { AdminForthFilterOperators } from '../types/Common.js'; import { normalizeRecordValues } from './columnValueNormalizer.js'; +import { assertColumnsAggregatable, assertRecordWritable, stripReadForbiddenColumns } from './columnAccess.js'; +import { interpretResource } from './resourceAccess.js'; +import { filtersTools } from './filtersTools.js'; +import { cascadeChildrenDelete, hookResponseError, listify } from './utils.js'; +import { afLogger } from './logger.js'; + +type ResourceScope = + | { + type: 'user'; + adminUser: AdminUser; + options: OperationalResourceUserOptions; + } + | { + type: 'system'; + adminUser: AdminUser | null; + options: OperationalResourceSystemOptions; + }; + +const warnedUnscopedOperations = new Set(); + +export interface OperationalResourceExecutors { + create(params: CreateResourceRecordParams): Promise; + update(params: UpdateResourceRecordParams): Promise; + delete(params: DeleteResourceRecordParams): Promise; + validate(resource: AdminForthResource, record: any, mode: 'create' | 'edit'): string | null; +} function sortsIfSort(sort: IAdminForthSort | IAdminForthSort[]): IAdminForthSort[] { return (Array.isArray(sort) ? sort : [sort]) as IAdminForthSort[]; @@ -11,21 +56,171 @@ export default class OperationalResource implements IOperationalResource { dataConnector: IAdminForthDataSourceConnectorBase; resourceConfig: AdminForthResource; - constructor(dataConnector: IAdminForthDataSourceConnectorBase, resourceConfig: AdminForthResource) { + constructor( + dataConnector: IAdminForthDataSourceConnectorBase, + resourceConfig: AdminForthResource, + private readonly adminforth: IAdminForth, + private readonly executors: OperationalResourceExecutors, + private readonly scope?: ResourceScope, + ) { this.dataConnector = dataConnector; this.resourceConfig = resourceConfig; } + asUser(adminUser: AdminUser, options: OperationalResourceUserOptions = {}): IScopedOperationalResource { + return new OperationalResource( + this.dataConnector, + this.resourceConfig, + this.adminforth, + this.executors, + { type: 'user', adminUser, options }, + ); + } + + asSystem(options: OperationalResourceSystemOptions = {}): IScopedOperationalResource { + return new OperationalResource( + this.dataConnector, + this.resourceConfig, + this.adminforth, + this.executors, + { type: 'system', adminUser: options.adminUser ?? null, options }, + ); + } + + private async actionError( + action: AllowedActionsEnum, + source: ActionCheckSource, + meta: any, + ): Promise { + if (this.scope?.type !== 'user') { + return null; + } + + const { allowedActions } = await interpretResource( + this.scope.adminUser, + this.resourceConfig, + meta, + source, + this.adminforth, + ); + const allowed = allowedActions[action] as boolean | string | undefined; + return allowed === true ? null : typeof allowed === 'string' ? allowed : 'Action is not allowed'; + } + + private get hooksEnabled(): boolean { + return this.scope?.type === 'user' || (this.scope?.type === 'system' && this.scope.options.hooks !== false); + } + + private warnUnscoped(operation: keyof IScopedOperationalResource): void { + const warnKey = `${this.resourceConfig.resourceId}.${operation}`; + if (warnedUnscopedOperations.has(warnKey)) { + return; + } + warnedUnscopedOperations.add(warnKey); + afLogger.warn( + `adminforth.resource('${this.resourceConfig.resourceId}').${operation}(...) is deprecated and will be removed in the next major version. ` + + `Use .asUser(adminUser, { meta }).${operation}(...) or .asSystem({ hooks: false }).${operation}(...) instead.`, + ); + } + + private readHookExtra(query: any) { + return this.scope.options.extra ?? { + body: query, + query: {}, + headers: {}, + cookies: [], + requestUrl: '', + response: this.scope.options.response, + }; + } + + private async runBeforeReadHooks(page: 'show' | 'list', query: any): Promise { + if (!this.hooksEnabled) { + return; + } + + for (const hook of listify(this.resourceConfig.hooks?.[page]?.beforeDatasourceRequest)) { + const response = await hook({ + resource: this.resourceConfig, + query, + adminUser: this.scope.adminUser, + filtersTools: filtersTools.get(query), + extra: this.readHookExtra(query), + adminforth: this.adminforth, + }); + const error = hookResponseError(response); + if (error) { + throw new Error(error.error); + } + } + } + + private async runAfterReadHooks(page: 'show' | 'list', query: any, records: any[]): Promise { + if (!this.hooksEnabled) { + return; + } + + for (const hook of listify(this.resourceConfig.hooks?.[page]?.afterDatasourceResponse)) { + const response = await hook({ + resource: this.resourceConfig, + query, + response: records, + adminUser: this.scope.adminUser, + extra: this.readHookExtra(query), + adminforth: this.adminforth, + }); + const error = hookResponseError(response); + if (error) { + throw new Error(error.error); + } + } + } + async get(filter: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array): Promise { - return ( + if (!this.scope) { + this.warnUnscoped('get'); + return this.asSystem({ hooks: false }).get(filter); + } + + const meta = this.scope?.options.meta ?? {}; + const accessError = await this.actionError( + AllowedActionsEnum.show, + ActionCheckSource.ShowRequest, + meta, + ); + if (accessError) { + throw new Error(accessError); + } + + const query = { + filters: filter, + limit: 1, + offset: 0, + sort: [], + }; + await this.runBeforeReadHooks('show', query); + const records = ( await this.dataConnector.getData({ resource: this.resourceConfig, - filters: this.dataConnector.validateAndNormalizeInputFilters(filter), - limit: 1, - offset: 0, - sort: [], + filters: this.dataConnector.validateAndNormalizeInputFilters(query.filters), + limit: query.limit, + offset: query.offset, + sort: query.sort, }) - ).data[0] || null; + ).data; + const record = records[0] || null; + if (record && this.scope?.type === 'user') { + await stripReadForbiddenColumns({ + resource: this.resourceConfig, + record, + adminUser: this.scope.adminUser, + meta, + source: ActionCheckSource.ShowRequest, + adminforth: this.adminforth, + }); + } + await this.runAfterReadHooks('show', query, records); + return record; } async list( @@ -35,6 +230,21 @@ export default class OperationalResource implements IOperationalResource { sort: IAdminForthSort | IAdminForthSort[] = [], columns?: string[] ): Promise { + if (!this.scope) { + this.warnUnscoped('list'); + return this.asSystem({ hooks: false }).list(filter, limit, offset, sort, columns); + } + + const meta = this.scope?.options.meta ?? {}; + const accessError = await this.actionError( + AllowedActionsEnum.list, + ActionCheckSource.ListRequest, + meta, + ); + if (accessError) { + throw new Error(accessError); + } + // check if type of limit and offset is number if (limit !== null && typeof limit !== 'number') { throw new Error('Limit must be a number'); @@ -52,15 +262,35 @@ export default class OperationalResource implements IOperationalResource { appliedOffset = 0; } - const { data } = await this.dataConnector.getData({ - resource: this.resourceConfig, - filters: this.dataConnector.validateAndNormalizeInputFilters(filter), + const query = { + filters: filter, limit: appliedLimit, offset: appliedOffset, sort: sortsIfSort(sort), + }; + await this.runBeforeReadHooks('list', query); + const { data } = await this.dataConnector.getData({ + resource: this.resourceConfig, + filters: this.dataConnector.validateAndNormalizeInputFilters(query.filters), + limit: query.limit, + offset: query.offset, + sort: query.sort, getTotals: false, columns: columns ? this.resourceConfig.dataSourceColumns.filter((column) => columns.includes(column.name)) : undefined, }); + if (this.scope?.type === 'user') { + for (const record of data) { + await stripReadForbiddenColumns({ + resource: this.resourceConfig, + record, + adminUser: this.scope.adminUser, + meta, + source: ActionCheckSource.ListRequest, + adminforth: this.adminforth, + }); + } + } + await this.runAfterReadHooks('list', query, data); return data; } @@ -70,6 +300,38 @@ export default class OperationalResource implements IOperationalResource { aggregations: { [alias: string]: IAggregationRule }, groupBy?: IGroupByRule | IGroupByRule[] ): Promise> { + if (!this.scope) { + this.warnUnscoped('aggregate'); + return this.asSystem({ hooks: false }).aggregate(filter, aggregations, groupBy); + } + + const meta = this.scope.options.meta ?? {}; + + // aggregation reads a whole set of records at once, so it needs list access + const listError = await this.actionError(AllowedActionsEnum.list, ActionCheckSource.ListRequest, meta); + if (listError) { + throw new Error(listError); + } + + // ...and min/max/groupBy return raw per-field values, which is what the show view does, + // so a resource with no reachable show view must not be aggregatable either + const showError = await this.actionError(AllowedActionsEnum.show, ActionCheckSource.ShowRequest, meta); + if (showError) { + throw new Error(showError); + } + + if (this.scope.type === 'user') { + await assertColumnsAggregatable({ + resource: this.resourceConfig, + aggregations, + groupBy, + filters: filter, + adminUser: this.scope.adminUser, + meta, + adminforth: this.adminforth, + }); + } + return this.dataConnector.aggregate({ resource: this.resourceConfig, filters: this.dataConnector.validateAndNormalizeInputFilters(filter), @@ -79,44 +341,212 @@ export default class OperationalResource implements IOperationalResource { } async count(filter?: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array | undefined): Promise { + if (!this.scope) { + this.warnUnscoped('count'); + return this.asSystem({ hooks: false }).count(filter); + } + + const accessError = await this.actionError( + AllowedActionsEnum.list, + ActionCheckSource.ListRequest, + this.scope?.options.meta ?? {}, + ); + if (accessError) { + throw new Error(accessError); + } return await this.dataConnector.getCount({ resource: this.resourceConfig, filters: this.dataConnector.validateAndNormalizeInputFilters(filter), }); } - async create(recordValues: any): Promise<{ ok: boolean; createdRecord: any; error?: string; }> { + async create(recordValues: any): Promise { + if (!this.scope) { + this.warnUnscoped('create'); + return this.asSystem({ hooks: false }).create(recordValues); + } + + const meta = this.scope.options.meta ?? {}; + const accessError = await this.actionError( + AllowedActionsEnum.create, + ActionCheckSource.CreateRequest, + meta, + ); + if (accessError) { + return { ok: false, createdRecord: undefined, error: accessError }; + } + + if (this.scope.type === 'user') { + try { + await assertRecordWritable({ + resource: this.resourceConfig, + record: recordValues, + mode: 'create', + adminUser: this.scope.adminUser, + meta, + adminforth: this.adminforth, + }); + } catch (error) { + return { ok: false, createdRecord: undefined, error: (error as Error).message }; + } + } + + if (this.hooksEnabled) { + const result = await this.executors.create({ + resource: this.resourceConfig, + record: recordValues, + adminUser: this.scope.adminUser, + extra: this.scope.options.extra, + response: this.scope.options.response, + }); + return { ...result, ok: !result.error, createdRecord: result.createdRecord }; + } + const normalizedRecord = { ...recordValues }; normalizeRecordValues(this.resourceConfig, normalizedRecord); + if (!this.hooksEnabled) { + const validationError = this.executors.validate(this.resourceConfig, normalizedRecord, 'create'); + if (validationError) { + return { ok: false, createdRecord: undefined, error: validationError }; + } + } const { ok, createdRecord, error } = await this.dataConnector.createRecord({ resource: this.resourceConfig, record: normalizedRecord, - adminUser: null + adminUser: this.scope.adminUser, }); return { ok, createdRecord, error }; } async update(primaryKey: any, record: any): Promise { + if (!this.scope) { + this.warnUnscoped('update'); + return this.asSystem({ hooks: false }).update(primaryKey, record); + } + if (Object.keys(record).length === 0) { return { ok: true }; } - const normalizedRecord = { ...record }; - normalizeRecordValues(this.resourceConfig, normalizedRecord); + if (!this.hooksEnabled) { + const normalizedRecord = { ...record }; + normalizeRecordValues(this.resourceConfig, normalizedRecord); + const validationError = this.executors.validate(this.resourceConfig, normalizedRecord, 'edit'); + if (validationError) { + return { ok: false, error: validationError }; + } + return this.dataConnector.updateRecord({ + resource: this.resourceConfig, + recordId: primaryKey, + newValues: normalizedRecord, + }); + } + + const oldRecord = this.scope.options.oldRecord + ?? await this.dataConnector.getRecordByPrimaryKey(this.resourceConfig, primaryKey); + if (!oldRecord) { + const primaryKeyColumn = this.resourceConfig.columns.find((column) => column.primaryKey); + return { ok: false, error: `Record with ${primaryKeyColumn.name} ${primaryKey} not found` }; + } + + const meta = { + ...(this.scope.options.meta ?? {}), + newRecord: record, + oldRecord, + pk: primaryKey, + }; + const accessError = await this.actionError( + AllowedActionsEnum.edit, + ActionCheckSource.EditRequest, + meta, + ); + if (accessError) { + return { ok: false, error: accessError }; + } - return await this.dataConnector.updateRecord({ + if (this.scope.type === 'user') { + try { + await assertRecordWritable({ + resource: this.resourceConfig, + record, + mode: 'edit', + adminUser: this.scope.adminUser, + meta, + adminforth: this.adminforth, + }); + } catch (error) { + return { ok: false, error: (error as Error).message }; + } + } + + const result = await this.executors.update({ resource: this.resourceConfig, recordId: primaryKey, - newValues: normalizedRecord + updates: record, + oldRecord, + adminUser: this.scope.adminUser, + extra: this.scope.options.extra, + response: this.scope.options.response, }); + return { ...result, ok: !result.error }; } async delete(primaryKey: any): Promise { - return await this.dataConnector.deleteRecord({ + if (!this.scope) { + this.warnUnscoped('delete'); + return this.asSystem({ hooks: false }).delete(primaryKey); + } + + if (!this.hooksEnabled) { + return this.dataConnector.deleteRecord({ + resource: this.resourceConfig, + recordId: primaryKey, + pkValues: compositePkValues(this.dataConnector, this.resourceConfig, primaryKey), + }); + } + + const record = this.scope.options.record + ?? await this.dataConnector.getRecordByPrimaryKey(this.resourceConfig, primaryKey); + if (!record) { + return false; + } + + const meta = { + ...(this.scope.options.meta ?? {}), + record, + pk: primaryKey, + }; + const accessError = await this.actionError( + AllowedActionsEnum.delete, + ActionCheckSource.DeleteRequest, + meta, + ); + if (accessError) { + throw new Error(accessError); + } + + const { error: cascadeError } = await cascadeChildrenDelete( + this.resourceConfig, + primaryKey, + { adminUser: this.scope.adminUser, response: this.scope.options.response }, + this.adminforth, + ); + if (cascadeError) { + throw new Error(cascadeError); + } + + const result = await this.executors.delete({ resource: this.resourceConfig, recordId: primaryKey, - pkValues: compositePkValues(this.dataConnector, this.resourceConfig, primaryKey), + record, + adminUser: this.scope.adminUser, + extra: this.scope.options.extra, + response: this.scope.options.response, }); + if (result.error) { + throw new Error(result.error); + } + return true; } } diff --git a/adminforth/modules/resourceAccess.ts b/adminforth/modules/resourceAccess.ts new file mode 100644 index 000000000..97b109575 --- /dev/null +++ b/adminforth/modules/resourceAccess.ts @@ -0,0 +1,47 @@ +import type { AdminForthResource, AllowedActionValue, IAdminForth } from '../types/Back.js'; +import { + ActionCheckSource, + AllowedActionsEnum, + type AdminUser, + type AllowedActionsResolved, +} from '../types/Common.js'; +import { afLogger } from './logger.js'; + +export async function interpretResource( + adminUser: AdminUser, + resource: AdminForthResource, + meta: any, + source: ActionCheckSource, + adminforth: IAdminForth, +): Promise<{ allowedActions: AllowedActionsResolved }> { + afLogger.trace(`🪲Interpreting resource, ${resource.resourceId}, ${source}, 'adminUser', ${adminUser}`); + const allowedActions = {} as AllowedActionsResolved; + const neededActions = { + [ActionCheckSource.ShowRequest]: ['show'], + [ActionCheckSource.EditRequest]: ['edit'], + [ActionCheckSource.EditLoadRequest]: ['show'], + [ActionCheckSource.DeleteRequest]: ['delete'], + [ActionCheckSource.ListRequest]: ['list'], + [ActionCheckSource.CreateRequest]: ['create'], + [ActionCheckSource.DisplayButtons]: ['show', 'edit', 'delete', 'create', 'filter'], + [ActionCheckSource.BulkActionRequest]: ['show', 'edit', 'delete', 'create', 'filter'], + [ActionCheckSource.CustomActionRequest]: ['show', 'edit', 'delete', 'create', 'filter'], + }[source]; + + await Promise.all( + Object.entries(resource.options.allowedActions).map( + async ([key, value]: [string, AllowedActionValue]) => { + if (!neededActions.includes(key as AllowedActionsEnum)) { + allowedActions[key] = false; + return; + } + + allowedActions[key] = typeof value === 'function' + ? await value({ adminUser, resource, meta, source, adminforth }) + : value; + }, + ), + ); + + return { allowedActions }; +} diff --git a/adminforth/modules/restApi.ts b/adminforth/modules/restApi.ts index fe56b5af4..c5b2e65bc 100644 --- a/adminforth/modules/restApi.ts +++ b/adminforth/modules/restApi.ts @@ -14,15 +14,11 @@ import { IAdminForthSort, HttpExtra, IAdminForthAndOrFilter, - IAggregationRule, - BackendOnlyInput, Filters, } from "../types/Back.js"; import type { AnySchemaObject } from 'ajv'; -import {cascadeChildrenDelete} from './utils.js' import { encodeRecordId, isCompositePrimaryKey, primaryKeyColumnNames } from './recordId.js'; - import { afLogger } from "./logger.js"; import { ADMINFORTH_VERSION, listify, md5hash, getLoginPromptHTML, hookResponseError, parseLooseJson, RateLimiter } from './utils.js'; @@ -30,58 +26,17 @@ import { ADMINFORTH_VERSION, listify, md5hash, getLoginPromptHTML, hookResponseE import AdminForthAuth from "../auth.js"; import { ActionCheckSource, AdminForthActionFront, AdminForthConfigMenuItem, AdminForthDataTypes, AdminForthFilterOperators, AdminForthResourceColumnInputCommon, AdminForthResourceFrontend, AdminForthResourcePages, AdminForthSortDirections, - AdminUser, AllowedActionsEnum, AllowedActionsResolved, + AdminUser, AllowedActionsEnum, AnnouncementBadgeResponse, GetConfigResponse, ShowInResolved} from "../types/Common.js"; import { filtersTools } from "../modules/filtersTools.js"; import { normalizeColumnValue } from './columnValueNormalizer.js'; - - -async function resolveBoolOrFn( - val: BackendOnlyInput | undefined, - ctx: { - adminUser: AdminUser; - resource: AdminForthResource; - meta: any; - source: ActionCheckSource; - adminforth: IAdminForth; - } -): Promise { - if (typeof val === 'function') { - return !!(await (val)(ctx)); - } - return !!val; -} - -async function isBackendOnly( - col: AdminForthResource['columns'][number], - ctx: { - adminUser: AdminUser; - resource: AdminForthResource; - meta: any; - source: ActionCheckSource; - adminforth: IAdminForth; - } -): Promise { - return await resolveBoolOrFn(col.backendOnly, ctx); -} - -async function isShown( - col: AdminForthResource['columns'][number], - page: 'list' | 'show' | 'edit' | 'create' | 'filter', - ctx: Parameters[1] -): Promise { - const s = (col.showIn as any) || {}; - if (s[page] !== undefined) return await resolveBoolOrFn(s[page], ctx); - if (s.all !== undefined) return await resolveBoolOrFn(s.all, ctx); - return true; -} - -async function isFilledOnCreate( col: AdminForthResource['columns'][number] ): Promise { - const fillOnCreate = !!col.fillOnCreate; - return fillOnCreate; -} +import { + isShown, + stripReadForbiddenColumns, +} from './columnAccess.js'; +import { interpretResource } from './resourceAccess.js'; function stripResourceColumnFrontendMeta(column: Record) { const { default: _default, _baseTypeDebug, ...sanitizedColumn } = column; @@ -270,33 +225,6 @@ export function rejectApiRawFilters(filters: any): { error: string } | undefined } } -/** - * Collects every column name referenced anywhere in a (possibly nested) filter tree, - * so the caller can check those columns against the visibility rules. - */ -function collectFilterFields(filters: any, fields: Set = new Set()): Set { - if (!filters || typeof filters !== 'object') { - return fields; - } - - if (Array.isArray(filters)) { - filters.forEach((filter) => collectFilterFields(filter, fields)); - return fields; - } - - if (typeof filters.field === 'string') { - fields.add(filters.field); - } - if (typeof filters.rightField === 'string') { - fields.add(filters.rightField); - } - if (Array.isArray(filters.subFilters)) { - filters.subFilters.forEach((filter) => collectFilterFields(filter, fields)); - } - - return fields; -} - function createErrorOrSuccessSchema(successSchema: AnySchemaObject): AnySchemaObject { return { anyOf: [ @@ -646,55 +574,6 @@ const validateColumnsResponseSchema: AnySchemaObject = createErrorOrSuccessSchem additionalProperties: true, }); -export async function interpretResource( - adminUser: AdminUser, - resource: AdminForthResource, - meta: any, - source: ActionCheckSource, - adminforth: IAdminForth -): Promise<{allowedActions: AllowedActionsResolved}> { - afLogger.trace(`🪲Interpreting resource, ${resource.resourceId}, ${source}, 'adminUser', ${adminUser}`); - const allowedActions = {} as AllowedActionsResolved; - - // we need to compute only allowed actions for this source: - // 'show' needed for ActionCheckSource.showRequest and ActionCheckSource.editLoadRequest and ActionCheckSource.displayButtons - // 'edit' needed for ActionCheckSource.editRequest and ActionCheckSource.displayButtons - // 'delete' needed for ActionCheckSource.deleteRequest and ActionCheckSource.displayButtons and ActionCheckSource.bulkActionRequest - // 'list' needed for ActionCheckSource.listRequest - // 'create' needed for ActionCheckSource.createRequest and ActionCheckSource.displayButtons - // for bulk actions we need to check all actions because bulk action can use any of them e.g sync allowed with edit - const neededActions = { - [ActionCheckSource.ShowRequest]: ['show'], - [ActionCheckSource.EditRequest]: ['edit'], - [ActionCheckSource.EditLoadRequest]: ['show'], - [ActionCheckSource.DeleteRequest]: ['delete'], - [ActionCheckSource.ListRequest]: ['list'], - [ActionCheckSource.CreateRequest]: ['create'], - [ActionCheckSource.DisplayButtons]: ['show', 'edit', 'delete', 'create', 'filter'], - [ActionCheckSource.BulkActionRequest]: ['show', 'edit', 'delete', 'create', 'filter'], - [ActionCheckSource.CustomActionRequest]: ['show', 'edit', 'delete', 'create', 'filter'], - }[source]; - - await Promise.all( - Object.entries(resource.options.allowedActions).map( - async ([key, value]: [string, AllowedActionValue]) => { - if (!neededActions.includes(key as AllowedActionsEnum)) { - allowedActions[key] = false; - return; - } - - // if callable then call - if (typeof value === 'function') { - allowedActions[key] = await value({ adminUser, resource, meta, source, adminforth }); - } else { - allowedActions[key] = value; - } - }) - ); - - return { allowedActions }; -} - export default class AdminForthRestAPI implements IAdminForthRestAPI { adminforth: IAdminForth; @@ -1021,7 +900,10 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { } const usersResource = this.adminforth.config.resources.find((res) => res.resourceId === this.adminforth.config.auth.usersResourceId); - const defaultUserExists = await this.adminforth.resource(usersResource.resourceId).get(Filters.EQ(usernameField, 'adminforth')) ? true : false; + const defaultUserExists = await this.adminforth + .resource(usersResource.resourceId) + .asSystem({ hooks: false }) + .get(Filters.EQ(usernameField, 'adminforth')) ? true : false; const loggedInPart = { showBrandNameInSidebar: this.adminforth.config.customization.showBrandNameInSidebar, @@ -1090,13 +972,14 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { source: ActionCheckSource.ShowRequest, adminforth: this.adminforth, }; - for (const key of Object.keys(adminUser.dbUser)) { - const col = userResource.columns.find((c) => c.name === key); - const bo = col ? await isBackendOnly(col, ctx) : true; - if (!col || bo) { - delete adminUser.dbUser[key]; - } - } + await stripReadForbiddenColumns({ + resource: userResource, + record: adminUser.dbUser, + adminUser, + meta: ctx.meta, + source: ctx.source, + adminforth: this.adminforth, + }); return { loggedIn: true, @@ -1685,15 +1568,17 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { }; for (const item of data.data) { - for (const key of Object.keys(item)) { - if (key === '_primaryKeyValue') { - continue; - } - const col = resource.columns.find((c) => c.name === key); - const bo = col ? await isBackendOnly(col, ctx) : true; - if (!col || bo) { - delete item[key]; - } + const encodedId = item._primaryKeyValue; + await stripReadForbiddenColumns({ + resource, + record: item, + adminUser, + meta, + source: ctx.source, + adminforth: this.adminforth, + }); + if (encodedId !== undefined) { + item._primaryKeyValue = encodedId; } if (!selectedColumnNameSet || shouldAddListHelpers) { item._label = resource.recordLabel(item); @@ -1845,82 +1730,6 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { } const meta = { requestBody: body, pk: undefined }; - const { allowedActions } = await interpretResource( - adminUser, - resource, - meta, - ActionCheckSource.ListRequest, - this.adminforth - ); - - // aggregation reads a whole set of records at once, so it needs list access - const { allowed, error } = checkAccess(AllowedActionsEnum.list, allowedActions); - if (!allowed) { - return { error }; - } - - // ...and min/max/groupBy return raw per-field values, which is what the show view does, - // so a resource with no reachable show view must not be aggregatable either - const { allowed: showAllowed, error: showError } = checkAccess(AllowedActionsEnum.show, allowedActions); - if (!showAllowed) { - return { error: showError }; - } - - const columnCtx = { - adminUser, - resource, - meta, - source: ActionCheckSource.ShowRequest, - adminforth: this.adminforth, - }; - - // a column may only take part in an aggregation if the user could have read the - // very same value from the show view - const columnExposureError = async (fieldName: string, context: string): Promise => { - const column = resource.columns.find((col) => col.name === fieldName); - if (!column) { - return `${context}: unknown column "${fieldName}"`; - } - if (await isBackendOnly(column, columnCtx)) { - return `${context}: column "${fieldName}" cannot be aggregated (backendOnly is true).`; - } - if (!await isShown(column, 'show', columnCtx)) { - return `${context}: column "${fieldName}" cannot be aggregated (showIn.show is false).`; - } - return null; - }; - - for (const [alias, rule] of Object.entries((aggregations || {}) as { [alias: string]: IAggregationRule })) { - // plain count does not reference any column - if (!rule?.field) { - continue; - } - const fieldError = await columnExposureError(rule.field, `Aggregation "${alias}"`); - if (fieldError) { - return { error: fieldError }; - } - } - - const groupByRules = Array.isArray(groupBy) ? groupBy : (groupBy ? [groupBy] : []); - for (const groupByRule of groupByRules) { - if (!groupByRule?.field) { - continue; - } - const fieldError = await columnExposureError(groupByRule.field, 'GroupBy'); - if (fieldError) { - return { error: fieldError }; - } - } - - // filters are not returned to the caller, but combined with an aggregation they turn - // into an oracle which reads a value out one comparison at a time, so backendOnly - // columns are off limits here as well - for (const fieldName of collectFilterFields(filters)) { - const column = resource.columns.find((col) => col.name === fieldName); - if (column && await isBackendOnly(column, columnCtx)) { - return { error: `Filter: column "${fieldName}" cannot be used (backendOnly is true).` }; - } - } // normalize filters same way as get_resource_data const normalizedFilters = { operator: AdminForthFilterOperators.AND, subFilters: [] }; @@ -1949,12 +1758,10 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { ? groupBy.map(applyUserTimeZone) : applyUserTimeZone(groupBy); - const data = await this.adminforth.connectors[resource.dataSource].aggregate({ - resource, - filters: normalizedFilters as IAdminForthAndOrFilter, - aggregations, - groupBy: aggregateGroupBy, - }); + const data = await this.adminforth + .resource(resource.resourceId) + .asUser(adminUser, { meta }) + .aggregate(normalizedFilters as IAdminForthAndOrFilter, aggregations, aggregateGroupBy); return { data }; } catch (e) { return { error: e.message }; @@ -2205,13 +2012,16 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { if (!resource) { return { error: `Resource '${body['resourceId']}' not found` }; } - const { allowedActions } = await interpretResource( + // access is checked again inside the scoped create below, but it has to be answered + // before the handler reveals anything about existing records or required columns + const { allowedActions: createAllowedActions } = await interpretResource( adminUser, resource, { requestBody: body }, ActionCheckSource.CreateRequest, this.adminforth ); - - const { allowed, error } = checkAccess(AllowedActionsEnum.create, allowedActions); - if (!allowed) { - return { error }; + const { allowed: createAllowed, error: createNotAllowedError } = checkAccess( + AllowedActionsEnum.create, createAllowedActions + ); + if (!createAllowed) { + return { error: createNotAllowedError }; } const { record, requiredColumnsToSkip } = body; @@ -2233,7 +2043,7 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { if (isCompositePrimaryKey(resource)) { const createPkColumnNames = primaryKeyColumnNames(resource); if (createPkColumnNames.every((name) => record[name] !== undefined)) { - const existingRecord = await this.adminforth.resource(resource.resourceId).get( + const existingRecord = await this.adminforth.resource(resource.resourceId).asSystem({ hooks: false }).get( createPkColumnNames.map((name) => Filters.EQ(name, record[name])) ); if (existingRecord) { @@ -2246,7 +2056,9 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { } else { const primaryKeyColumn = resource.columns.find((col) => col.primaryKey); if (record[primaryKeyColumn.name] !== undefined) { - const existingRecord = await this.adminforth.resource(resource.resourceId).get([Filters.EQ(primaryKeyColumn.name, record[primaryKeyColumn.name])]); + const existingRecord = await this.adminforth.resource(resource.resourceId) + .asSystem({ hooks: false }) + .get([Filters.EQ(primaryKeyColumn.name, record[primaryKeyColumn.name])]); if (existingRecord) { return { error: `Record with ${primaryKeyColumn.name} '${record[primaryKeyColumn.name]}' already exists`, ok: false }; } @@ -2273,28 +2085,6 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { } } - for (const column of resource.columns) { - const fieldName = column.name; - if (fieldName in record) { - const shown = await isShown(column, 'create', ctxCreate); // - const bo = await isBackendOnly(column, ctxCreate); - const filledOnCreate = await isFilledOnCreate(column); - if (bo) { - return { - error: `Field "${fieldName}" cannot be modified as it is restricted from creation (backendOnly is true).`, - ok: false, - }; - } - - if (!shown && !filledOnCreate && !column.allowModifyWhenNotShowInCreate) { - return { - error: `Field "${fieldName}" cannot be modified as it is restricted from creation (showIn.create is false). If you need to set this hidden field during creation, either configure column.fillOnCreate or set column.allowModifyWhenNotShowInCreate = true.`, - ok: false, - }; - } - } - } - // for polymorphic foreign resources, we need to find out the value for polymorphicOn column for (const column of resource.columns) { if (column.foreignResource?.polymorphicOn && record[column.name] === null) { @@ -2343,10 +2133,14 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { return { error: jsonError, ok: false }; } - const createRecordResponse = await this.adminforth.createResourceRecord({ - resource, record, adminUser, response, - extra: { body, query, headers, cookies, requestUrl, response } - }); + const createRecordResponse = await this.adminforth + .resource(resource.resourceId) + .asUser(adminUser, { + meta: ctxCreate.meta, + response, + extra: { body, query, headers, cookies, requestUrl, response }, + }) + .create(record); if (createRecordResponse.error) { return { error: createRecordResponse.error, @@ -2392,17 +2186,20 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { } const record = body['record']; - const { allowedActions } = await interpretResource( - adminUser, - resource, - { requestBody: body, newRecord: record, oldRecord, pk: recordId }, + // access is checked again inside the scoped update below, but it has to be answered + // before the handler reveals whether another record with the same key exists + const { allowedActions: editAllowedActions } = await interpretResource( + adminUser, + resource, + { requestBody: body, newRecord: record, oldRecord, pk: recordId }, ActionCheckSource.EditRequest, this.adminforth ); - - const { allowed, error: allowedError } = checkAccess(AllowedActionsEnum.edit, allowedActions); - if (!allowed) { - return { error: allowedError }; + const { allowed: editAllowed, error: editNotAllowedError } = checkAccess( + AllowedActionsEnum.edit, editAllowedActions + ); + if (!editAllowed) { + return { error: editNotAllowedError }; } if (isCompositePrimaryKey(resource)) { @@ -2415,7 +2212,7 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { acc[name] = record[name] !== undefined ? record[name] : oldRecord[name]; return acc; }, {}); - const existingRecord = await this.adminforth.resource(resource.resourceId).get( + const existingRecord = await this.adminforth.resource(resource.resourceId).asSystem({ hooks: false }).get( pkColumnNames.map((name) => Filters.EQ(name, newPkValues[name])) ); if (existingRecord) { @@ -2428,48 +2225,15 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { } else { const primaryKeyColumn = resource.columns.find((col) => col.primaryKey); if (record[primaryKeyColumn.name] !== undefined) { - const existingRecord = await this.adminforth.resource(resource.resourceId).get([Filters.EQ(primaryKeyColumn.name, record[primaryKeyColumn.name])]); + const existingRecord = await this.adminforth.resource(resource.resourceId) + .asSystem({ hooks: false }) + .get([Filters.EQ(primaryKeyColumn.name, record[primaryKeyColumn.name])]); if (existingRecord) { return { error: `Record with ${primaryKeyColumn.name} '${record[primaryKeyColumn.name]}' already exists`, ok: false }; } } } - const ctxEdit = { - adminUser, - resource, - meta: { requestBody: body, newRecord: record, oldRecord, pk: recordId }, - source: ActionCheckSource.EditRequest, - adminforth: this.adminforth, - }; - - for (const column of resource.columns) { - const fieldName = column.name; - if (fieldName in record) { - const shown = await isShown(column, 'edit', ctxEdit); - const bo = await isBackendOnly(column, ctxEdit); - if (bo) { - return { - error: `Field "${fieldName}" cannot be modified as it is restricted from editing (backendOnly is true).`, - ok: false, - }; - } - - if (column.editReadonly) { - return { - error: `Field "${fieldName}" cannot be modified as it is restricted from editing (editReadonly is true).`, - ok: false, - }; - } - - if (!shown && !column.allowModifyWhenNotShowInEdit) { - return { - error: `Field "${fieldName}" cannot be modified as it is restricted from editing (showIn.edit is false). If you need to allow updating this hidden field during editing, set column.allowModifyWhenNotShowInEdit = true.`, - ok: false, - }; - } - } - } // for polymorphic foreign resources, we need to find out the value for polymorphicOn column for (const column of resource.columns) { if (column.foreignResource?.polymorphicOn && record[column.name] === null) { @@ -2525,10 +2289,15 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { if (jsonError) { return { error: jsonError, ok: false }; } - const { error } = await this.adminforth.updateResourceRecord({ - resource, updates: record, adminUser, oldRecord, recordId, response, - extra: { body, query, headers, cookies, requestUrl, response } - }); + const { error } = await this.adminforth + .resource(resource.resourceId) + .asUser(adminUser, { + meta: { requestBody: body, newRecord: record, oldRecord, pk: recordId }, + oldRecord, + response, + extra: { body, query, headers, cookies, requestUrl, response }, + }) + .update(recordId, record); if (error) { return { error }; } @@ -2559,30 +2328,18 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { return { error: `Record with ${body['primaryKey']} not found` }; } - const { allowedActions } = await interpretResource( - adminUser, - resource, - { requestBody: body, record: record }, - ActionCheckSource.DeleteRequest, - this.adminforth - ); - - const { allowed, error } = checkAccess(AllowedActionsEnum.delete, allowedActions); - if (!allowed) { - return { error }; - } - - const { error: cascadeError } = await cascadeChildrenDelete(resource, body.primaryKey, {adminUser, response}, this.adminforth); - if (cascadeError) { - return { error: cascadeError }; - } - - const { error: deleteError } = await this.adminforth.deleteResourceRecord({ - resource, record, adminUser, recordId: body['primaryKey'], response, - extra: { body, query, headers, cookies, requestUrl, response } - }); - if (deleteError) { - return { error: deleteError }; + try { + await this.adminforth + .resource(resource.resourceId) + .asUser(adminUser, { + meta: { requestBody: body, record }, + record, + response, + extra: { body, query, headers, cookies, requestUrl, response }, + }) + .delete(body.primaryKey); + } catch (error) { + return { error: (error as Error).message }; } return { ok: true, diff --git a/adminforth/modules/utils.ts b/adminforth/modules/utils.ts index 6ce7b8d5a..4c91ef59c 100644 --- a/adminforth/modules/utils.ts +++ b/adminforth/modules/utils.ts @@ -3,7 +3,7 @@ import { fileURLToPath } from 'url'; import fs from 'fs'; import Fuse from 'fuse.js'; import crypto from 'crypto'; -import { AdminForthConfig, AdminForthResource, AdminForthResourceColumnInputCommon,Filters, IAdminForth, Predicate } from '../index.js'; +import { Filters, type AdminForthResource, type IAdminForth } from '../types/Back.js'; import { RateLimiterMemory, RateLimiterAbstract } from "rate-limiter-flexible"; import { encodeRecordId, isCompositePrimaryKey } from './recordId.js'; import { PERIOD_UNITS, type PeriodString, type PeriodUnit } from '../types/Back.js'; @@ -546,7 +546,10 @@ export async function cascadeChildrenDelete(resource: AdminForthResource, primar const strategy = foreignColumn.foreignResource.onDelete; - const childRecords = await adminforth.resource(childRes.resourceId).list(Filters.EQ(foreignColumn.name, primaryKey)); + const childRecords = await adminforth + .resource(childRes.resourceId) + .asSystem({ hooks: false }) + .list(Filters.EQ(foreignColumn.name, primaryKey)); const childPk = childRes.columns.find(c => c.primaryKey)?.name; const childRecordId = (childRecord: any) => isCompositePrimaryKey(childRes) @@ -555,21 +558,25 @@ export async function cascadeChildrenDelete(resource: AdminForthResource, primar if (strategy === 'cascade') { for (const childRecord of childRecords) { - const childResult = await cascadeChildrenDelete(childRes, childRecordId(childRecord), context, adminforth); - if (childResult?.error) { - return childResult; - } - const deleteChild = await adminforth.deleteResourceRecord({resource: childRes, record: childRecord, adminUser, recordId: childRecordId(childRecord), response}); - if (deleteChild.error) return { error: deleteChild.error }; - if (childResult?.error) { - return childResult; + try { + await adminforth.resource(childRes.resourceId) + .asSystem({ adminUser, response, record: childRecord }) + .delete(childRecordId(childRecord)); + } catch (e) { + return { error: (e as Error).message }; } } } if (strategy === 'setNull') { for (const childRecord of childRecords) { - await adminforth.resource(childRes.resourceId).update(childRecordId(childRecord), {[foreignColumn.name]: null}); + const result = await adminforth.resource(childRes.resourceId).asSystem({ hooks: false }).update( + childRecordId(childRecord), + { [foreignColumn.name]: null }, + ); + if (result.error) { + return { error: result.error }; + } } } } @@ -671,4 +678,4 @@ export function checkIfLinkInAllowedHosts(url: string, allowedHosts: string[]) { if (!allowed) { throw new Error(`Attachment host "${hostname}" is not in attachImagesAllowedHosts`); } -} \ No newline at end of file +} diff --git a/adminforth/types/Back.ts b/adminforth/types/Back.ts index da2147e00..432313377 100644 --- a/adminforth/types/Back.ts +++ b/adminforth/types/Back.ts @@ -612,14 +612,27 @@ export interface IAdminForth { tr(msg: string, category: string, lang: string, params: any, pluralizationNumber?: number): Promise; + /** + * @deprecated Will be removed in the next major version. Use + * `resource(resourceId).asUser(adminUser, { meta }).create(record)` or + * `resource(resourceId).asSystem({ hooks: false }).create(record)`. + */ createResourceRecord( params: CreateResourceRecordParams, ): Promise; + /** + * @deprecated Will be removed in the next major version. Use the scoped + * resource API through `asUser()` or `asSystem()`. + */ updateResourceRecord( params: UpdateResourceRecordParams, ): Promise; + /** + * @deprecated Will be removed in the next major version. Use the scoped + * resource API through `asUser()` or `asSystem()`. + */ deleteResourceRecord( params: DeleteResourceRecordParams, ): Promise; @@ -2209,7 +2222,16 @@ export class Sorts { } } -export interface IOperationalResource { +/** + * Resource API scoped to a trust level by {@link IOperationalResource.asUser} or + * {@link IOperationalResource.asSystem}. + * + * Error contract: a denied or failed operation is always visible. `get`, `list`, `count`, + * `aggregate` and `delete` throw, since their return value carries no room for an error; + * `create` and `update` resolve to `{ ok: false, error }`. `delete` resolves to `false` when the + * record simply did not exist. + */ +export interface IScopedOperationalResource { get: (filter: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array) => Promise; list: (filter: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array, limit?: number, offset?: number, sort?: IAdminForthSort | IAdminForthSort[], columns?: string[]) => Promise; @@ -2222,7 +2244,7 @@ export interface IOperationalResource { groupBy?: IGroupByRule | IGroupByRule[] ) => Promise>; - create: (record: any) => Promise<{ ok: boolean; createdRecord: any; error?: string; }>; + create: (record: any) => Promise; update: (primaryKey: any, record: any) => Promise; @@ -2231,6 +2253,70 @@ export interface IOperationalResource { dataConnector: IAdminForthDataSourceConnectorBase; } +export interface IOperationalResource { + /** + * Returns a resource API scoped to an authenticated admin user. Operations enforce + * resource ACL and column access and run lifecycle hooks; mutations also validate records. + */ + asUser: (adminUser: AdminUser, options?: OperationalResourceUserOptions) => IScopedOperationalResource; + + /** + * Returns a trusted resource API which skips ACL and column access. Hooks run by + * default and can be disabled explicitly for connector-level system operations. + */ + asSystem: (options?: OperationalResourceSystemOptions) => IScopedOperationalResource; + + /** @deprecated Use `asUser(...).get(...)` or `asSystem({ hooks: false }).get(...)`. */ + get: IScopedOperationalResource['get']; + + /** @deprecated Use `asUser(...).list(...)` or `asSystem({ hooks: false }).list(...)`. */ + list: IScopedOperationalResource['list']; + + /** @deprecated Use `asUser(...).count(...)` or `asSystem({ hooks: false }).count(...)`. */ + count: IScopedOperationalResource['count']; + + /** @deprecated Use `asUser(...).aggregate(...)` or `asSystem({ hooks: false }).aggregate(...)`. */ + aggregate: IScopedOperationalResource['aggregate']; + + /** @deprecated Use `asUser(...).create(...)` or `asSystem({ hooks: false }).create(...)`. */ + create: IScopedOperationalResource['create']; + + /** @deprecated Use `asUser(...).update(...)` or `asSystem({ hooks: false }).update(...)`. */ + update: IScopedOperationalResource['update']; + + /** @deprecated Use `asUser(...).delete(...)` or `asSystem({ hooks: false }).delete(...)`. */ + delete: IScopedOperationalResource['delete']; + + dataConnector: IAdminForthDataSourceConnectorBase; +} + +export interface OperationalResourceContextOptions { + meta?: any; + extra?: HttpExtra; + response?: IAdminForthHttpResponse; + + /** + * Record as it is stored before the mutation. Supply it when the caller has already loaded the + * record, so `update()` does not read it a second time and hooks see the same snapshot the + * caller worked from. + */ + oldRecord?: any; + + /** + * Record to delete, when the caller has already loaded it. Same purpose as `oldRecord`, for + * `delete()`. + */ + record?: any; +} + +export type OperationalResourceUserOptions = OperationalResourceContextOptions; + +export interface OperationalResourceSystemOptions extends OperationalResourceContextOptions { + hooks?: boolean; + /** User attribution passed to hooks and fillOnCreate without enabling ACL checks. */ + adminUser?: AdminUser; +} + /** diff --git a/tests/jest_tests/operational_resource_scope.test.ts b/tests/jest_tests/operational_resource_scope.test.ts new file mode 100644 index 000000000..03cde6504 --- /dev/null +++ b/tests/jest_tests/operational_resource_scope.test.ts @@ -0,0 +1,281 @@ +import OperationalResource from '../../adminforth/modules/operationalResource.js'; +import { ActionCheckSource } from '../../adminforth/types/Common.js'; + +function setup(resourceId = 'users') { + const calls = { + acl: 0, + createExecutor: 0, + updateExecutor: 0, + connectorCreate: 0, + connectorUpdate: 0, + connectorDelete: 0, + connectorGetData: 0, + connectorGetByPk: 0, + connectorCount: 0, + connectorAggregate: 0, + validate: 0, + beforeList: 0, + afterList: 0, + }; + const resource = { + resourceId, + dataSource: 'main', + columns: [ + { name: 'id', primaryKey: true }, + { name: 'name', showIn: { create: true, edit: true } }, + { name: 'readonly', showIn: { create: true, edit: true }, editReadonly: true }, + { name: 'private', backendOnly: true }, + { + name: 'secret', + showIn: { create: true, edit: true }, + backendOnly: ({ source }) => source === ActionCheckSource.CreateRequest, + }, + ], + dataSourceColumns: [], + options: { + allowedActions: { + create: ({ meta }) => { + calls.acl++; + return Promise.resolve(meta.allowed === true); + }, + edit: ({ meta }) => { + calls.acl++; + return Promise.resolve(meta.allowed === true); + }, + show: true, + list: ({ meta }) => { + calls.acl++; + return Promise.resolve(meta.allowed === true); + }, + delete: ({ meta }) => { + calls.acl++; + return Promise.resolve(meta.allowed === true); + }, + }, + }, + hooks: { + list: { + beforeDatasourceRequest: [async () => { + calls.beforeList++; + return { ok: true }; + }], + afterDatasourceResponse: [async () => { + calls.afterList++; + return { ok: true }; + }], + }, + }, + } as any; + resource.dataSourceColumns = resource.columns; + + const connector = { + createRecord: async ({ record }) => { + calls.connectorCreate++; + return { ok: true, createdRecord: { id: 1, ...record } }; + }, + updateRecord: async () => { + calls.connectorUpdate++; + return { ok: true }; + }, + deleteRecord: async () => { + calls.connectorDelete++; + return true; + }, + getData: async () => { + calls.connectorGetData++; + return { data: [{ id: 1, name: 'John', private: 'hidden' }], total: 1 }; + }, + getCount: async () => { + calls.connectorCount++; + return 1; + }, + aggregate: async () => { + calls.connectorAggregate++; + return [{ total: 1 }]; + }, + validateAndNormalizeInputFilters: (filter) => filter, + getRecordByPrimaryKey: async () => { + calls.connectorGetByPk++; + return { id: 1, name: 'Old name', readonly: 'old' }; + }, + } as any; + const adminforth = { config: { resources: [resource] } } as any; + const executors = { + create: async ({ record }) => { + calls.createExecutor++; + return { createdRecord: { id: 1, ...record } }; + }, + update: async () => { + calls.updateExecutor++; + return { error: null }; + }, + delete: async () => ({ error: null }), + validate: () => { + calls.validate++; + return null; + }, + } as any; + + return { + calls, + resource: new OperationalResource(connector, resource, adminforth, executors), + }; +} + +describe('OperationalResource access scopes', () => { + it('enforces ACL, column access, validation, and hooks for asUser()', async () => { + const { calls, resource } = setup(); + const denied = await resource.asUser({} as any, { meta: { allowed: false } }).create({ name: 'John' }); + expect(denied).toMatchObject({ ok: false, error: 'Action is not allowed' }); + + const forbidden = await resource.asUser({} as any, { meta: { allowed: true } }).create({ secret: 'value' }); + expect(forbidden).toMatchObject({ ok: false }); + expect(forbidden.error).toContain('backendOnly is true'); + + const created = await resource.asUser({} as any, { meta: { allowed: true } }).create({ name: 'John' }); + expect(created).toMatchObject({ ok: true, createdRecord: { id: 1, name: 'John' } }); + expect(calls).toMatchObject({ acl: 3, createExecutor: 1, connectorCreate: 0 }); + }); + + it('runs hooks by default for asSystem()', async () => { + const { calls, resource } = setup(); + const created = await resource.asSystem({ meta: { allowed: false } }).create({ secret: 'value' }); + + expect(created).toMatchObject({ ok: true, createdRecord: { id: 1, secret: 'value' } }); + expect(calls).toMatchObject({ acl: 0, createExecutor: 1, connectorCreate: 0 }); + }); + + it('uses validation and the connector when system hooks are disabled', async () => { + const { calls, resource } = setup(); + const created = await resource.asSystem({ hooks: false }).create({ secret: 'value' }); + + expect(created).toMatchObject({ ok: true, createdRecord: { id: 1, secret: 'value' } }); + expect(calls).toMatchObject({ acl: 0, createExecutor: 0, connectorCreate: 1, validate: 1 }); + }); + + it('keeps the unscoped API equivalent to asSystem({ hooks: false })', async () => { + const unscoped = setup(); + const scoped = setup(); + const filter = { field: 'id', operator: 'eq', value: 1 } as any; + const aggregations = { total: { fn: 'count', field: 'id' } } as any; + + const unscopedResults = [ + await unscoped.resource.get(filter), + await unscoped.resource.list(filter), + await unscoped.resource.count(filter), + await unscoped.resource.aggregate(filter, aggregations), + await unscoped.resource.create({ name: 'John' }), + await unscoped.resource.update(1, { name: 'Jane' }), + await unscoped.resource.delete(1), + ]; + const hooksFreeSystem = scoped.resource.asSystem({ hooks: false }); + const scopedResults = [ + await hooksFreeSystem.get(filter), + await hooksFreeSystem.list(filter), + await hooksFreeSystem.count(filter), + await hooksFreeSystem.aggregate(filter, aggregations), + await hooksFreeSystem.create({ name: 'John' }), + await hooksFreeSystem.update(1, { name: 'Jane' }), + await hooksFreeSystem.delete(1), + ]; + + expect(unscopedResults).toEqual(scopedResults); + expect(unscoped.calls).toEqual(scoped.calls); + }); + + it('allows system hooks to update editReadonly fields while asUser rejects them', async () => { + const { calls, resource } = setup(); + + const forbidden = await resource + .asUser({} as any, { meta: { allowed: true } }) + .update(1, { readonly: 'new' }); + expect(forbidden.error).toContain('editReadonly is true'); + + const updated = await resource.asSystem().update(1, { readonly: 'new' }); + expect(updated).toMatchObject({ ok: true, error: null }); + expect(calls).toMatchObject({ acl: 1, updateExecutor: 1, connectorUpdate: 0 }); + }); + + it('applies read ACL, column access, and hooks for asUser()', async () => { + const { calls, resource } = setup(); + + await expect(resource.asUser({} as any, { meta: { allowed: false } }).list([])) + .rejects.toThrow('Action is not allowed'); + + const records = await resource.asUser({} as any, { meta: { allowed: true } }).list([]); + + expect(records).toEqual([{ id: 1, name: 'John' }]); + expect(calls).toMatchObject({ acl: 2, beforeList: 1, afterList: 1 }); + }); + + it('keeps system reads unrestricted while honoring the hooks option', async () => { + const withHooks = setup(); + const withoutHooks = setup(); + + const systemRecords = await withHooks.resource.asSystem().list([]); + const hooksFreeRecords = await withoutHooks.resource.asSystem({ hooks: false }).list([]); + + expect(systemRecords).toEqual([{ id: 1, name: 'John', private: 'hidden' }]); + expect(hooksFreeRecords).toEqual(systemRecords); + expect(withHooks.calls).toMatchObject({ acl: 0, beforeList: 1, afterList: 1 }); + expect(withoutHooks.calls).toMatchObject({ acl: 0, beforeList: 0, afterList: 0 }); + }); + + it('rejects denied deletes and leaves the connector untouched', async () => { + const { calls, resource } = setup(); + + await expect(resource.asUser({} as any, { meta: { allowed: false } }).delete(1)) + .rejects.toThrow('Action is not allowed'); + expect(calls).toMatchObject({ acl: 1, connectorDelete: 0 }); + }); + + it('reuses the record supplied by the caller instead of reading it again', async () => { + const { calls, resource } = setup(); + + const updated = await resource + .asUser({} as any, { meta: { allowed: true }, oldRecord: { id: 1, name: 'Old name' } }) + .update(1, { name: 'Jane' }); + + expect(updated).toMatchObject({ ok: true }); + expect(calls).toMatchObject({ connectorGetByPk: 0, updateExecutor: 1 }); + }); + + it('requires list and show access for asUser() aggregations', async () => { + const { calls, resource } = setup(); + + await expect( + resource.asUser({} as any, { meta: { allowed: false } }).aggregate([], { total: { fn: 'count' } } as any), + ).rejects.toThrow('Action is not allowed'); + expect(calls).toMatchObject({ connectorAggregate: 0 }); + }); + + it('refuses to aggregate, group by, or filter on columns the user cannot read', async () => { + const { calls, resource } = setup(); + const scoped = resource.asUser({} as any, { meta: { allowed: true } }); + + await expect(scoped.aggregate([], { max: { fn: 'max', field: 'private' } } as any)) + .rejects.toThrow('cannot be aggregated (backendOnly is true)'); + await expect(scoped.aggregate([], { total: { fn: 'count' } } as any, { field: 'private' } as any)) + .rejects.toThrow('cannot be aggregated (backendOnly is true)'); + await expect(scoped.aggregate( + { field: 'private', operator: 'eq', value: 'hidden' } as any, + { total: { fn: 'count' } } as any, + )).rejects.toThrow('Filter: column "private" cannot be used'); + + expect(calls).toMatchObject({ connectorAggregate: 0 }); + + const allowed = await scoped.aggregate([], { max: { fn: 'max', field: 'name' } } as any); + expect(allowed).toEqual([{ total: 1 }]); + expect(calls).toMatchObject({ connectorAggregate: 1 }); + }); + + it('still delegates unscoped calls per resource, warning about each one separately', async () => { + const first = setup('warn-probe-a'); + const second = setup('warn-probe-b'); + + expect(await first.resource.list([])).toEqual([{ id: 1, name: 'John', private: 'hidden' }]); + expect(await second.resource.count([])).toBe(1); + expect(first.calls).toMatchObject({ acl: 0, connectorGetData: 1 }); + expect(second.calls).toMatchObject({ acl: 0, connectorCount: 1 }); + }); +}); From 8ca7569a549ac38856541377e3ae2f4d62c647e6 Mon Sep 17 00:00:00 2001 From: Maksym Pipkun Date: Mon, 14 Sep 2026 15:20:37 +0300 Subject: [PATCH 02/11] fix: row-scope aggregate and count through read hooks --- adminforth/modules/columnAccess.ts | 234 ++++------ adminforth/modules/operationalResource.ts | 406 ++++++++---------- adminforth/modules/restApi.ts | 18 +- .../operational_resource_scope.test.ts | 37 +- 4 files changed, 308 insertions(+), 387 deletions(-) diff --git a/adminforth/modules/columnAccess.ts b/adminforth/modules/columnAccess.ts index 804ca7295..bf62566e3 100644 --- a/adminforth/modules/columnAccess.ts +++ b/adminforth/modules/columnAccess.ts @@ -9,6 +9,10 @@ import { type AdminUser, } from '../types/Common.js'; +/** + * Everything a column rule needs to resolve itself. Column rules may be plain booleans or + * functions of the current user and request, so they can only be answered in a context. + */ export interface ColumnAccessContext { adminUser: AdminUser; resource: AdminForthResource; @@ -19,131 +23,95 @@ export interface ColumnAccessContext { export async function resolveBoolOrFn( value: BackendOnlyInput | AllowedActionValue | undefined, - context: ColumnAccessContext, + ctx: ColumnAccessContext, ): Promise { if (typeof value === 'function') { - return !!(await value(context)); + return !!(await value(ctx)); } return !!value; } export async function isBackendOnly( column: AdminForthResource['columns'][number], - context: ColumnAccessContext, + ctx: ColumnAccessContext, ): Promise { - return resolveBoolOrFn(column.backendOnly, context); + return resolveBoolOrFn(column.backendOnly, ctx); } export async function isShown( column: AdminForthResource['columns'][number], page: 'list' | 'show' | 'edit' | 'create' | 'filter', - context: ColumnAccessContext, + ctx: ColumnAccessContext, ): Promise { const showIn = column.showIn as Record | undefined; if (showIn?.[page] !== undefined) { - return resolveBoolOrFn(showIn[page], context); + return resolveBoolOrFn(showIn[page], ctx); } if (showIn?.all !== undefined) { - return resolveBoolOrFn(showIn.all, context); + return resolveBoolOrFn(showIn.all, ctx); } return true; } -export interface AssertRecordWritableParams { - resource: AdminForthResource; - record: Record; - mode: 'create' | 'edit'; - adminUser: AdminUser; - meta: any; - adminforth: IAdminForth; -} - -export async function assertRecordWritable({ - resource, - record, - mode, - adminUser, - meta, - adminforth, -}: AssertRecordWritableParams): Promise { - const context: ColumnAccessContext = { - adminUser, - resource, - meta, - source: mode === 'create' ? ActionCheckSource.CreateRequest : ActionCheckSource.EditRequest, - adminforth, - }; - - for (const column of resource.columns) { +/** + * Checks every field the caller wants to write against the column rules which restrict writing: + * backendOnly, editReadonly, showIn plus its allowModifyWhenNotShowIn* / fillOnCreate escapes. + * + * @returns the reason the record cannot be written, or null when it can. + */ +export async function recordWriteError( + ctx: ColumnAccessContext, + record: Record, + mode: 'create' | 'edit', +): Promise { + for (const column of ctx.resource.columns) { const fieldName = column.name; if (!(fieldName in record)) { continue; } - const shown = await isShown(column, mode, context); - const backendOnly = await isBackendOnly(column, context); - - if (backendOnly) { - throw new Error( - `Field "${fieldName}" cannot be modified as it is restricted from ${mode === 'create' ? 'creation' : 'editing'} (backendOnly is true).`, - ); + if (await isBackendOnly(column, ctx)) { + return `Field "${fieldName}" cannot be modified as it is restricted from ` + + `${mode === 'create' ? 'creation' : 'editing'} (backendOnly is true).`; } + const shown = await isShown(column, mode, ctx); + if (mode === 'create') { - if ( - !shown - && !column.fillOnCreate - && !column.allowModifyWhenNotShowInCreate - ) { - throw new Error( - `Field "${fieldName}" cannot be modified as it is restricted from creation (showIn.create is false). If you need to set this hidden field during creation, either configure column.fillOnCreate or set column.allowModifyWhenNotShowInCreate = true.`, - ); + if (!shown && !column.fillOnCreate && !column.allowModifyWhenNotShowInCreate) { + return `Field "${fieldName}" cannot be modified as it is restricted from creation ` + + `(showIn.create is false). If you need to set this hidden field during creation, either ` + + `configure column.fillOnCreate or set column.allowModifyWhenNotShowInCreate = true.`; } continue; } if (column.editReadonly) { - throw new Error( - `Field "${fieldName}" cannot be modified as it is restricted from editing (editReadonly is true).`, - ); + return `Field "${fieldName}" cannot be modified as it is restricted from editing ` + + `(editReadonly is true).`; } if (!shown && !column.allowModifyWhenNotShowInEdit) { - throw new Error( - `Field "${fieldName}" cannot be modified as it is restricted from editing (showIn.edit is false). If you need to allow updating this hidden field during editing, set column.allowModifyWhenNotShowInEdit = true.`, - ); + return `Field "${fieldName}" cannot be modified as it is restricted from editing ` + + `(showIn.edit is false). If you need to allow updating this hidden field during editing, ` + + `set column.allowModifyWhenNotShowInEdit = true.`; } } -} -export interface StripReadForbiddenColumnsParams { - resource: AdminForthResource; - record: Record; - adminUser: AdminUser; - meta: any; - source: ActionCheckSource; - adminforth: IAdminForth; + return null; } -export async function stripReadForbiddenColumns({ - resource, - record, - adminUser, - meta, - source, - adminforth, -}: StripReadForbiddenColumnsParams): Promise> { - const context: ColumnAccessContext = { - adminUser, - resource, - meta, - source, - adminforth, - }; - +/** + * Drops in place every key the user is not allowed to read: backendOnly columns, and keys which + * are not described in the resource at all. + */ +export async function stripReadForbiddenColumns( + ctx: ColumnAccessContext, + record: Record, +): Promise> { for (const key of Object.keys(record)) { - const column = resource.columns.find((candidate) => candidate.name === key); - if (!column || await isBackendOnly(column, context)) { + const column = ctx.resource.columns.find((candidate) => candidate.name === key); + if (!column || await isBackendOnly(column, ctx)) { delete record[key]; } } @@ -178,104 +146,76 @@ export function collectFilterFields(filters: any, fields: Set = new Set( return fields; } -export interface AssertFilterColumnsReadableParams { - resource: AdminForthResource; - filters: any; - adminUser: AdminUser; - meta: any; - source: ActionCheckSource; - adminforth: IAdminForth; -} - /** * Filter values are never echoed back, but combined with any readable output they turn into an * oracle which reads a hidden value out one comparison at a time, so backendOnly columns must not * be filterable either. + * + * @returns the reason the filter cannot be used, or null when it can. */ -export async function assertFilterColumnsReadable({ - resource, - filters, - adminUser, - meta, - source, - adminforth, -}: AssertFilterColumnsReadableParams): Promise { - const context: ColumnAccessContext = { adminUser, resource, meta, source, adminforth }; - +export async function filterColumnsReadableError( + ctx: ColumnAccessContext, + filters: any, +): Promise { for (const fieldName of collectFilterFields(filters)) { - const column = resource.columns.find((candidate) => candidate.name === fieldName); - if (column && await isBackendOnly(column, context)) { - throw new Error(`Filter: column "${fieldName}" cannot be used (backendOnly is true).`); + const column = ctx.resource.columns.find((candidate) => candidate.name === fieldName); + if (column && await isBackendOnly(column, ctx)) { + return `Filter: column "${fieldName}" cannot be used (backendOnly is true).`; } } -} -export interface AssertColumnsAggregatableParams { - resource: AdminForthResource; - aggregations?: { [alias: string]: { field?: string } }; - groupBy?: { field?: string } | Array<{ field?: string }>; - filters?: any; - adminUser: AdminUser; - meta: any; - adminforth: IAdminForth; + return null; } /** * A column may only take part in an aggregation if the user could have read the very same value * from the show view, otherwise min/max/groupBy become a way to read hidden columns. + * + * @returns the reason the aggregation cannot run, or null when it can. */ -export async function assertColumnsAggregatable({ - resource, - aggregations, - groupBy, - filters, - adminUser, - meta, - adminforth, -}: AssertColumnsAggregatableParams): Promise { - const context: ColumnAccessContext = { - adminUser, - resource, - meta, - source: ActionCheckSource.ShowRequest, - adminforth, - }; - - const assertExposable = async (fieldName: string, label: string): Promise => { - const column = resource.columns.find((candidate) => candidate.name === fieldName); +export async function columnsAggregatableError( + ctx: ColumnAccessContext, + query: { + aggregations?: { [alias: string]: { field?: string } }; + groupBy?: { field?: string } | Array<{ field?: string }>; + filters?: any; + }, +): Promise { + const exposureError = async (fieldName: string, label: string): Promise => { + const column = ctx.resource.columns.find((candidate) => candidate.name === fieldName); if (!column) { - throw new Error(`${label}: unknown column "${fieldName}"`); + return `${label}: unknown column "${fieldName}"`; } - if (await isBackendOnly(column, context)) { - throw new Error(`${label}: column "${fieldName}" cannot be aggregated (backendOnly is true).`); + if (await isBackendOnly(column, ctx)) { + return `${label}: column "${fieldName}" cannot be aggregated (backendOnly is true).`; } - if (!await isShown(column, 'show', context)) { - throw new Error(`${label}: column "${fieldName}" cannot be aggregated (showIn.show is false).`); + if (!await isShown(column, 'show', ctx)) { + return `${label}: column "${fieldName}" cannot be aggregated (showIn.show is false).`; } + return null; }; - for (const [alias, rule] of Object.entries(aggregations || {})) { + for (const [alias, rule] of Object.entries(query.aggregations || {})) { // plain count does not reference any column if (!rule?.field) { continue; } - await assertExposable(rule.field, `Aggregation "${alias}"`); + const error = await exposureError(rule.field, `Aggregation "${alias}"`); + if (error) { + return error; + } } - const groupByRules = Array.isArray(groupBy) ? groupBy : (groupBy ? [groupBy] : []); + const groupByRules = Array.isArray(query.groupBy) ? query.groupBy : (query.groupBy ? [query.groupBy] : []); for (const groupByRule of groupByRules) { if (!groupByRule?.field) { continue; } - await assertExposable(groupByRule.field, 'GroupBy'); + const error = await exposureError(groupByRule.field, 'GroupBy'); + if (error) { + return error; + } } - await assertFilterColumnsReadable({ - resource, - filters, - adminUser, - meta, - source: ActionCheckSource.ShowRequest, - adminforth, - }); + return filterColumnsReadableError(ctx, query.filters); } diff --git a/adminforth/modules/operationalResource.ts b/adminforth/modules/operationalResource.ts index 60f8d4e23..7770d61a2 100644 --- a/adminforth/modules/operationalResource.ts +++ b/adminforth/modules/operationalResource.ts @@ -21,7 +21,12 @@ import type { import { ActionCheckSource, AllowedActionsEnum, type AdminUser } from '../types/Common.js'; import { compositePkValues } from './recordId.js'; import { normalizeRecordValues } from './columnValueNormalizer.js'; -import { assertColumnsAggregatable, assertRecordWritable, stripReadForbiddenColumns } from './columnAccess.js'; +import { + columnsAggregatableError, + recordWriteError, + stripReadForbiddenColumns, + type ColumnAccessContext, +} from './columnAccess.js'; import { interpretResource } from './resourceAccess.js'; import { filtersTools } from './filtersTools.js'; import { cascadeChildrenDelete, hookResponseError, listify } from './utils.js'; @@ -39,6 +44,22 @@ type ResourceScope = options: OperationalResourceSystemOptions; }; +/** + * Which resource permission guards which operation, and which check source the permission + * callbacks are told about. Kept as one table so the whole mapping can be audited at a glance + * instead of being read out of seven method bodies. + */ +const OPERATION_ACCESS = { + get: [AllowedActionsEnum.show, ActionCheckSource.ShowRequest], + list: [AllowedActionsEnum.list, ActionCheckSource.ListRequest], + count: [AllowedActionsEnum.list, ActionCheckSource.ListRequest], + create: [AllowedActionsEnum.create, ActionCheckSource.CreateRequest], + update: [AllowedActionsEnum.edit, ActionCheckSource.EditRequest], + delete: [AllowedActionsEnum.delete, ActionCheckSource.DeleteRequest], +} as const; + +type GuardedOperation = keyof typeof OPERATION_ACCESS; + const warnedUnscopedOperations = new Set(); export interface OperationalResourceExecutors { @@ -52,50 +73,48 @@ function sortsIfSort(sort: IAdminForthSort | IAdminForthSort[]): IAdminForthSort return (Array.isArray(sort) ? sort : [sort]) as IAdminForthSort[]; } -export default class OperationalResource implements IOperationalResource { - dataConnector: IAdminForthDataSourceConnectorBase; - resourceConfig: AdminForthResource; - +/** + * Resource API bound to a trust level. `scope` is always present here, so every method can ask + * for permissions and column access without re-deciding whether it is allowed to. + */ +class ScopedOperationalResource implements IScopedOperationalResource { constructor( - dataConnector: IAdminForthDataSourceConnectorBase, - resourceConfig: AdminForthResource, + public dataConnector: IAdminForthDataSourceConnectorBase, + public resourceConfig: AdminForthResource, private readonly adminforth: IAdminForth, private readonly executors: OperationalResourceExecutors, - private readonly scope?: ResourceScope, - ) { - this.dataConnector = dataConnector; - this.resourceConfig = resourceConfig; + private readonly scope: ResourceScope, + ) {} + + private get meta(): any { + return this.scope.options.meta ?? {}; } - asUser(adminUser: AdminUser, options: OperationalResourceUserOptions = {}): IScopedOperationalResource { - return new OperationalResource( - this.dataConnector, - this.resourceConfig, - this.adminforth, - this.executors, - { type: 'user', adminUser, options }, - ); + private get hooksEnabled(): boolean { + return this.scope.type === 'user' || this.scope.options.hooks !== false; } - asSystem(options: OperationalResourceSystemOptions = {}): IScopedOperationalResource { - return new OperationalResource( - this.dataConnector, - this.resourceConfig, - this.adminforth, - this.executors, - { type: 'system', adminUser: options.adminUser ?? null, options }, - ); + /** Column rules only restrict what a real user may touch; system scopes are trusted. */ + private columnCtx(source: ActionCheckSource, meta: any = this.meta): ColumnAccessContext | null { + if (this.scope.type !== 'user') { + return null; + } + return { + adminUser: this.scope.adminUser, + resource: this.resourceConfig, + meta, + source, + adminforth: this.adminforth, + }; } - private async actionError( - action: AllowedActionsEnum, - source: ActionCheckSource, - meta: any, - ): Promise { - if (this.scope?.type !== 'user') { + /** @returns the reason the operation is not allowed, or null when it is. */ + private async accessError(operation: GuardedOperation, meta: any = this.meta): Promise { + if (this.scope.type !== 'user') { return null; } + const [action, source] = OPERATION_ACCESS[operation]; const { allowedActions } = await interpretResource( this.scope.adminUser, this.resourceConfig, @@ -107,22 +126,6 @@ export default class OperationalResource implements IOperationalResource { return allowed === true ? null : typeof allowed === 'string' ? allowed : 'Action is not allowed'; } - private get hooksEnabled(): boolean { - return this.scope?.type === 'user' || (this.scope?.type === 'system' && this.scope.options.hooks !== false); - } - - private warnUnscoped(operation: keyof IScopedOperationalResource): void { - const warnKey = `${this.resourceConfig.resourceId}.${operation}`; - if (warnedUnscopedOperations.has(warnKey)) { - return; - } - warnedUnscopedOperations.add(warnKey); - afLogger.warn( - `adminforth.resource('${this.resourceConfig.resourceId}').${operation}(...) is deprecated and will be removed in the next major version. ` - + `Use .asUser(adminUser, { meta }).${operation}(...) or .asSystem({ hooks: false }).${operation}(...) instead.`, - ); - } - private readHookExtra(query: any) { return this.scope.options.extra ?? { body: query, @@ -140,11 +143,15 @@ export default class OperationalResource implements IOperationalResource { } for (const hook of listify(this.resourceConfig.hooks?.[page]?.beforeDatasourceRequest)) { + const tools = filtersTools.get(query); + // hooks reach these either as their own argument or off the query, and the documented + // spelling is query.filtersTools — so both have to be present, same as the REST path + query.filtersTools = tools; const response = await hook({ resource: this.resourceConfig, query, adminUser: this.scope.adminUser, - filtersTools: filtersTools.get(query), + filtersTools: tools, extra: this.readHookExtra(query), adminforth: this.adminforth, }); @@ -177,17 +184,7 @@ export default class OperationalResource implements IOperationalResource { } async get(filter: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array): Promise { - if (!this.scope) { - this.warnUnscoped('get'); - return this.asSystem({ hooks: false }).get(filter); - } - - const meta = this.scope?.options.meta ?? {}; - const accessError = await this.actionError( - AllowedActionsEnum.show, - ActionCheckSource.ShowRequest, - meta, - ); + const accessError = await this.accessError('get'); if (accessError) { throw new Error(accessError); } @@ -208,39 +205,24 @@ export default class OperationalResource implements IOperationalResource { sort: query.sort, }) ).data; + const record = records[0] || null; - if (record && this.scope?.type === 'user') { - await stripReadForbiddenColumns({ - resource: this.resourceConfig, - record, - adminUser: this.scope.adminUser, - meta, - source: ActionCheckSource.ShowRequest, - adminforth: this.adminforth, - }); + const ctx = this.columnCtx(ActionCheckSource.ShowRequest); + if (record && ctx) { + await stripReadForbiddenColumns(ctx, record); } await this.runAfterReadHooks('show', query, records); return record; } async list( - filter: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array, - limit: number | null = null, + filter: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array, + limit: number | null = null, offset: number | null = null, sort: IAdminForthSort | IAdminForthSort[] = [], columns?: string[] ): Promise { - if (!this.scope) { - this.warnUnscoped('list'); - return this.asSystem({ hooks: false }).list(filter, limit, offset, sort, columns); - } - - const meta = this.scope?.options.meta ?? {}; - const accessError = await this.actionError( - AllowedActionsEnum.list, - ActionCheckSource.ListRequest, - meta, - ); + const accessError = await this.accessError('list'); if (accessError) { throw new Error(accessError); } @@ -253,19 +235,10 @@ export default class OperationalResource implements IOperationalResource { throw new Error('Offset must be a number'); } - let appliedLimit = limit; - if (limit === null) { - appliedLimit = 1000000000; - } - let appliedOffset = offset; - if (offset === null) { - appliedOffset = 0; - } - const query = { filters: filter, - limit: appliedLimit, - offset: appliedOffset, + limit: limit === null ? 1000000000 : limit, + offset: offset === null ? 0 : offset, sort: sortsIfSort(sort), }; await this.runBeforeReadHooks('list', query); @@ -278,117 +251,80 @@ export default class OperationalResource implements IOperationalResource { getTotals: false, columns: columns ? this.resourceConfig.dataSourceColumns.filter((column) => columns.includes(column.name)) : undefined, }); - if (this.scope?.type === 'user') { + + const ctx = this.columnCtx(ActionCheckSource.ListRequest); + if (ctx) { for (const record of data) { - await stripReadForbiddenColumns({ - resource: this.resourceConfig, - record, - adminUser: this.scope.adminUser, - meta, - source: ActionCheckSource.ListRequest, - adminforth: this.adminforth, - }); + await stripReadForbiddenColumns(ctx, record); } } await this.runAfterReadHooks('list', query, data); return data; } - async aggregate( filter: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array, aggregations: { [alias: string]: IAggregationRule }, groupBy?: IGroupByRule | IGroupByRule[] ): Promise> { - if (!this.scope) { - this.warnUnscoped('aggregate'); - return this.asSystem({ hooks: false }).aggregate(filter, aggregations, groupBy); - } - - const meta = this.scope.options.meta ?? {}; - - // aggregation reads a whole set of records at once, so it needs list access - const listError = await this.actionError(AllowedActionsEnum.list, ActionCheckSource.ListRequest, meta); - if (listError) { - throw new Error(listError); + // an aggregation reads a whole set of records at once, so it needs list access, and its + // min/max/groupBy return raw per-field values, which is what the show view does + const accessError = (await this.accessError('list')) ?? (await this.accessError('get')); + if (accessError) { + throw new Error(accessError); } - // ...and min/max/groupBy return raw per-field values, which is what the show view does, - // so a resource with no reachable show view must not be aggregatable either - const showError = await this.actionError(AllowedActionsEnum.show, ActionCheckSource.ShowRequest, meta); - if (showError) { - throw new Error(showError); + const ctx = this.columnCtx(ActionCheckSource.ShowRequest); + if (ctx) { + const columnError = await columnsAggregatableError(ctx, { aggregations, groupBy, filters: filter }); + if (columnError) { + throw new Error(columnError); + } } - if (this.scope.type === 'user') { - await assertColumnsAggregatable({ - resource: this.resourceConfig, - aggregations, - groupBy, - filters: filter, - adminUser: this.scope.adminUser, - meta, - adminforth: this.adminforth, - }); - } + // Row-scoping hooks are how multi-tenancy is expressed, and an aggregation reads the same + // rows a list does, so it has to be narrowed by them too — otherwise groupBy/min/max/sum + // report across every tenant. They run after the column check above, so that check still + // sees the caller's own filters and cannot be tripped by a filter a trusted hook added. + // Only the request side runs: the response here is aggregated rows, not records an + // afterDatasourceResponse hook could meaningfully process. + const query = { filters: filter, aggregations, groupBy, limit: null, offset: 0, sort: [] }; + await this.runBeforeReadHooks('list', query); return this.dataConnector.aggregate({ resource: this.resourceConfig, - filters: this.dataConnector.validateAndNormalizeInputFilters(filter), + filters: this.dataConnector.validateAndNormalizeInputFilters(query.filters), aggregations, groupBy, }); } async count(filter?: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array | undefined): Promise { - if (!this.scope) { - this.warnUnscoped('count'); - return this.asSystem({ hooks: false }).count(filter); - } - - const accessError = await this.actionError( - AllowedActionsEnum.list, - ActionCheckSource.ListRequest, - this.scope?.options.meta ?? {}, - ); + const accessError = await this.accessError('count'); if (accessError) { throw new Error(accessError); } + + // a count is a list the caller only learns the size of, so it is row-scoped the same way + const query = { filters: filter, limit: null, offset: 0, sort: [] }; + await this.runBeforeReadHooks('list', query); + return await this.dataConnector.getCount({ resource: this.resourceConfig, - filters: this.dataConnector.validateAndNormalizeInputFilters(filter), + filters: this.dataConnector.validateAndNormalizeInputFilters(query.filters), }); } async create(recordValues: any): Promise { - if (!this.scope) { - this.warnUnscoped('create'); - return this.asSystem({ hooks: false }).create(recordValues); - } - - const meta = this.scope.options.meta ?? {}; - const accessError = await this.actionError( - AllowedActionsEnum.create, - ActionCheckSource.CreateRequest, - meta, - ); + const accessError = await this.accessError('create'); if (accessError) { return { ok: false, createdRecord: undefined, error: accessError }; } - if (this.scope.type === 'user') { - try { - await assertRecordWritable({ - resource: this.resourceConfig, - record: recordValues, - mode: 'create', - adminUser: this.scope.adminUser, - meta, - adminforth: this.adminforth, - }); - } catch (error) { - return { ok: false, createdRecord: undefined, error: (error as Error).message }; - } + const ctx = this.columnCtx(ActionCheckSource.CreateRequest); + const columnError = ctx && await recordWriteError(ctx, recordValues, 'create'); + if (columnError) { + return { ok: false, createdRecord: undefined, error: columnError }; } if (this.hooksEnabled) { @@ -404,14 +340,13 @@ export default class OperationalResource implements IOperationalResource { const normalizedRecord = { ...recordValues }; normalizeRecordValues(this.resourceConfig, normalizedRecord); - if (!this.hooksEnabled) { - const validationError = this.executors.validate(this.resourceConfig, normalizedRecord, 'create'); - if (validationError) { - return { ok: false, createdRecord: undefined, error: validationError }; - } + const validationError = this.executors.validate(this.resourceConfig, normalizedRecord, 'create'); + if (validationError) { + return { ok: false, createdRecord: undefined, error: validationError }; } - const { ok, createdRecord, error } = await this.dataConnector.createRecord({ - resource: this.resourceConfig, + + const { ok, createdRecord, error } = await this.dataConnector.createRecord({ + resource: this.resourceConfig, record: normalizedRecord, adminUser: this.scope.adminUser, }); @@ -419,11 +354,6 @@ export default class OperationalResource implements IOperationalResource { } async update(primaryKey: any, record: any): Promise { - if (!this.scope) { - this.warnUnscoped('update'); - return this.asSystem({ hooks: false }).update(primaryKey, record); - } - if (Object.keys(record).length === 0) { return { ok: true }; } @@ -449,34 +379,17 @@ export default class OperationalResource implements IOperationalResource { return { ok: false, error: `Record with ${primaryKeyColumn.name} ${primaryKey} not found` }; } - const meta = { - ...(this.scope.options.meta ?? {}), - newRecord: record, - oldRecord, - pk: primaryKey, - }; - const accessError = await this.actionError( - AllowedActionsEnum.edit, - ActionCheckSource.EditRequest, - meta, - ); + const meta = { ...this.meta, newRecord: record, oldRecord, pk: primaryKey }; + + const accessError = await this.accessError('update', meta); if (accessError) { return { ok: false, error: accessError }; } - if (this.scope.type === 'user') { - try { - await assertRecordWritable({ - resource: this.resourceConfig, - record, - mode: 'edit', - adminUser: this.scope.adminUser, - meta, - adminforth: this.adminforth, - }); - } catch (error) { - return { ok: false, error: (error as Error).message }; - } + const ctx = this.columnCtx(ActionCheckSource.EditRequest, meta); + const columnError = ctx && await recordWriteError(ctx, record, 'edit'); + if (columnError) { + return { ok: false, error: columnError }; } const result = await this.executors.update({ @@ -492,11 +405,6 @@ export default class OperationalResource implements IOperationalResource { } async delete(primaryKey: any): Promise { - if (!this.scope) { - this.warnUnscoped('delete'); - return this.asSystem({ hooks: false }).delete(primaryKey); - } - if (!this.hooksEnabled) { return this.dataConnector.deleteRecord({ resource: this.resourceConfig, @@ -511,16 +419,7 @@ export default class OperationalResource implements IOperationalResource { return false; } - const meta = { - ...(this.scope.options.meta ?? {}), - record, - pk: primaryKey, - }; - const accessError = await this.actionError( - AllowedActionsEnum.delete, - ActionCheckSource.DeleteRequest, - meta, - ); + const accessError = await this.accessError('delete', { ...this.meta, record, pk: primaryKey }); if (accessError) { throw new Error(accessError); } @@ -535,7 +434,7 @@ export default class OperationalResource implements IOperationalResource { throw new Error(cascadeError); } - const result = await this.executors.delete({ + const { error } = await this.executors.delete({ resource: this.resourceConfig, recordId: primaryKey, record, @@ -543,10 +442,75 @@ export default class OperationalResource implements IOperationalResource { extra: this.scope.options.extra, response: this.scope.options.response, }); - if (result.error) { - throw new Error(result.error); + if (error) { + throw new Error(error); } return true; } +} + +/** + * Entry point returned by `adminforth.resource(id)`. It carries no trust level of its own — + * pick one with `asUser()` or `asSystem()`. The bare operations are deprecated aliases of + * `asSystem({ hooks: false })`, kept for backward compatibility. + */ +export default class OperationalResource implements IOperationalResource { + constructor( + public dataConnector: IAdminForthDataSourceConnectorBase, + public resourceConfig: AdminForthResource, + private readonly adminforth: IAdminForth, + private readonly executors: OperationalResourceExecutors, + ) {} + + private scoped(scope: ResourceScope): IScopedOperationalResource { + return new ScopedOperationalResource( + this.dataConnector, + this.resourceConfig, + this.adminforth, + this.executors, + scope, + ); + } + + asUser(adminUser: AdminUser, options: OperationalResourceUserOptions = {}): IScopedOperationalResource { + return this.scoped({ type: 'user', adminUser, options }); + } + + asSystem(options: OperationalResourceSystemOptions = {}): IScopedOperationalResource { + return this.scoped({ type: 'system', adminUser: options.adminUser ?? null, options }); + } + + /** Warns once per resource and operation, then falls back to the trusted, hook-free scope. */ + private legacy(operation: keyof IScopedOperationalResource): IScopedOperationalResource { + const warnKey = `${this.resourceConfig.resourceId}.${operation}`; + if (!warnedUnscopedOperations.has(warnKey)) { + warnedUnscopedOperations.add(warnKey); + afLogger.warn( + `adminforth.resource('${this.resourceConfig.resourceId}').${operation}(...) is deprecated and will be removed in the next major version. ` + + `Use .asUser(adminUser, { meta }).${operation}(...) or .asSystem({ hooks: false }).${operation}(...) instead.`, + ); + } + return this.asSystem({ hooks: false }); + } + + /** @deprecated Use `asUser(...).get(...)` or `asSystem({ hooks: false }).get(...)`. */ + get(...args: Parameters) { return this.legacy('get').get(...args); } + + /** @deprecated Use `asUser(...).list(...)` or `asSystem({ hooks: false }).list(...)`. */ + list(...args: Parameters) { return this.legacy('list').list(...args); } + + /** @deprecated Use `asUser(...).count(...)` or `asSystem({ hooks: false }).count(...)`. */ + count(...args: Parameters) { return this.legacy('count').count(...args); } + + /** @deprecated Use `asUser(...).aggregate(...)` or `asSystem({ hooks: false }).aggregate(...)`. */ + aggregate(...args: Parameters) { return this.legacy('aggregate').aggregate(...args); } + + /** @deprecated Use `asUser(...).create(...)` or `asSystem({ hooks: false }).create(...)`. */ + create(...args: Parameters) { return this.legacy('create').create(...args); } + + /** @deprecated Use `asUser(...).update(...)` or `asSystem({ hooks: false }).update(...)`. */ + update(...args: Parameters) { return this.legacy('update').update(...args); } + /** @deprecated Use `asUser(...).delete(...)` or `asSystem({ hooks: false }).delete(...)`. */ + delete(...args: Parameters) { return this.legacy('delete').delete(...args); } } diff --git a/adminforth/modules/restApi.ts b/adminforth/modules/restApi.ts index c5b2e65bc..de96b4d32 100644 --- a/adminforth/modules/restApi.ts +++ b/adminforth/modules/restApi.ts @@ -972,14 +972,7 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { source: ActionCheckSource.ShowRequest, adminforth: this.adminforth, }; - await stripReadForbiddenColumns({ - resource: userResource, - record: adminUser.dbUser, - adminUser, - meta: ctx.meta, - source: ctx.source, - adminforth: this.adminforth, - }); + await stripReadForbiddenColumns(ctx, adminUser.dbUser); return { loggedIn: true, @@ -1569,14 +1562,7 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { for (const item of data.data) { const encodedId = item._primaryKeyValue; - await stripReadForbiddenColumns({ - resource, - record: item, - adminUser, - meta, - source: ctx.source, - adminforth: this.adminforth, - }); + await stripReadForbiddenColumns(ctx, item); if (encodedId !== undefined) { item._primaryKeyValue = encodedId; } diff --git a/tests/jest_tests/operational_resource_scope.test.ts b/tests/jest_tests/operational_resource_scope.test.ts index 03cde6504..b72c67e41 100644 --- a/tests/jest_tests/operational_resource_scope.test.ts +++ b/tests/jest_tests/operational_resource_scope.test.ts @@ -55,8 +55,9 @@ function setup(resourceId = 'users') { }, hooks: { list: { - beforeDatasourceRequest: [async () => { + beforeDatasourceRequest: [async ({ query }) => { calls.beforeList++; + query.filtersTools.replaceOrAddTopFilter({ field: 'tenant', operator: 'eq', value: 't1' }); return { ok: true }; }], afterDatasourceResponse: [async () => { @@ -68,6 +69,7 @@ function setup(resourceId = 'users') { } as any; resource.dataSourceColumns = resource.columns; + const seenFilters: Record = {}; const connector = { createRecord: async ({ record }) => { calls.connectorCreate++; @@ -85,12 +87,14 @@ function setup(resourceId = 'users') { calls.connectorGetData++; return { data: [{ id: 1, name: 'John', private: 'hidden' }], total: 1 }; }, - getCount: async () => { + getCount: async ({ filters }) => { calls.connectorCount++; + seenFilters.count = filters; return 1; }, - aggregate: async () => { + aggregate: async ({ filters }) => { calls.connectorAggregate++; + seenFilters.aggregate = filters; return [{ total: 1 }]; }, validateAndNormalizeInputFilters: (filter) => filter, @@ -118,6 +122,7 @@ function setup(resourceId = 'users') { return { calls, + seenFilters, resource: new OperationalResource(connector, resource, adminforth, executors), }; } @@ -278,4 +283,30 @@ describe('OperationalResource access scopes', () => { expect(first.calls).toMatchObject({ acl: 0, connectorGetData: 1 }); expect(second.calls).toMatchObject({ acl: 0, connectorCount: 1 }); }); + + it('row-scopes aggregate and count through the same read hooks as list', async () => { + const { calls, seenFilters, resource } = setup(); + const scoped = resource.asUser({} as any, { meta: { allowed: true } }); + const tenantFilter = { field: 'tenant', operator: 'eq', value: 't1' }; + + await scoped.aggregate([], { total: { fn: 'count' } } as any, { field: 'name' } as any); + await scoped.count([]); + + // without this an aggregation reports across every tenant's rows + expect(seenFilters.aggregate).toContainEqual(tenantFilter); + expect(seenFilters.count).toContainEqual(tenantFilter); + expect(calls).toMatchObject({ beforeList: 2, connectorAggregate: 1, connectorCount: 1 }); + }); + + it('does not row-scope reads for a trusted system scope', async () => { + const { calls, seenFilters, resource } = setup(); + const hooksFree = resource.asSystem({ hooks: false }); + + await hooksFree.aggregate([], { total: { fn: 'count' } } as any); + await hooksFree.count([]); + + expect(seenFilters.aggregate).toEqual([]); + expect(seenFilters.count).toEqual([]); + expect(calls).toMatchObject({ beforeList: 0 }); + }); }); From 416960a7ebbe2106461de208663513768ebf9a72 Mon Sep 17 00:00:00 2001 From: Maksym Pipkun Date: Thu, 17 Sep 2026 11:59:25 +0300 Subject: [PATCH 03/11] refactor: split resource user access from data API --- .../tutorial/03-Customization/11-dataApi.md | 98 ++-- .../tutorial/03-Customization/12-security.md | 4 +- adminforth/index.ts | 115 ++--- adminforth/modules/configValidator.ts | 60 +-- adminforth/modules/operationalResource.ts | 456 ++---------------- adminforth/modules/recordValidator.ts | 63 +++ adminforth/modules/restApi.ts | 7 +- adminforth/modules/userScopedResource.ts | 336 +++++++++++++ adminforth/modules/utils.ts | 38 +- adminforth/types/Back.ts | 49 +- .../operational_resource_scope.test.ts | 103 +--- 11 files changed, 626 insertions(+), 703 deletions(-) create mode 100644 adminforth/modules/recordValidator.ts create mode 100644 adminforth/modules/userScopedResource.ts diff --git a/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md b/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md index 3bd099d09..c62368abf 100644 --- a/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md +++ b/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md @@ -29,27 +29,43 @@ const admin = new AdminForth({ }); // get the resource object -await admin.resource('adminuser').asSystem({ hooks: false }).get(Filters.EQ('id', '1234')); +await admin.resource('adminuser').get(Filters.EQ('id', '1234')); ``` Here we will show you how to use the Data API with simple examples. -## Access scope +## Access levels -Choose an explicit access scope for new code which works with resource data: +Resource data can be reached at two levels, and the difference is who asked for +the operation. ```ts const users = admin.resource('adminuser'); +// a user asked for this await users.asUser(adminUser, { meta }).create(record); -await users.asSystem({ meta }).create(record); -await users.asSystem({ hooks: false }).create(record); + +// plain data access the user did not ask for +await users.create(record); ``` -`asUser()` enforces the resource ACL and column access rules, validates the -record, and runs lifecycle hooks. `asSystem()` skips ACL and column access but -still validates the record and runs hooks. Pass `{ hooks: false }` for a trusted -connector-level operation which still performs normalization and validation. +`asUser()` is the level for anything that came from a request. It enforces the +resource ACL, applies the column access rules (`backendOnly`, `editReadonly`, +`showIn` and its `allowModifyWhenNotShowIn*` / `fillOnCreate` escapes), strips +columns the user may not read out of results, and runs the resource lifecycle +hooks — including the row-scoping `beforeDatasourceRequest` hooks that express +multi-tenancy. Use it in plugin endpoints: you do not have to remember the +individual checks, and you cannot forget one. + +The bare methods are plain data access for internal bookkeeping: no permission +checks, no column access rules, no hooks. Writes are still normalized and +validated. + +`admin.createResourceRecord`, `admin.updateResourceRecord` and +`admin.deleteResourceRecord` are the older entry points which this API replaces. +They still work and still run hooks, but they are deprecated: move calls to +`asUser()` when a user asked for the operation, and to the bare methods when +nothing did. A denied or failed operation is always visible. `get`, `list`, `count`, `aggregate` and `delete` throw, since their return value carries no room for an @@ -67,36 +83,14 @@ try { } ``` -When the caller has already loaded the record, pass it in so the scoped call does -not read it a second time and hooks see the same snapshot the caller worked from: +When the caller has already loaded the record, pass it in so the call does not +read it a second time and hooks see the same snapshot the caller worked from: ```ts await users.asUser(adminUser, { meta, oldRecord }).update(recordId, updates); await users.asUser(adminUser, { meta, record }).delete(recordId); ``` -An optional `adminUser` can be attached to a system operation when hooks need -user attribution without enabling user ACL checks: - -```ts -await users.asSystem({ adminUser, meta }).create(record); -``` - -Deprecated unscoped calls remain aliases for the trusted, hook-free scope for -backward compatibility: - -```ts -await admin.resource('adminuser').create(record); -await admin.resource('adminuser').asSystem({ hooks: false }).create(record); -``` - -The two calls have the same behavior. This applies to `get`, `list`, `count`, -`aggregate`, `create`, `update`, and `delete`. Unscoped calls will be removed in -the next major version, so use an explicit scope in new code. - -The legacy `admin.createResourceRecord`, `admin.updateResourceRecord`, and -`admin.deleteResourceRecord` methods are deprecated and will be removed in the -next major version. ## Get one item from database @@ -112,7 +106,7 @@ Signature: Get item by ID: ```ts -const user = await admin.resource('adminuser').asSystem({ hooks: false }).get( +const user = await admin.resource('adminuser').get( [Filters.EQ('id', '1234')] ); ``` @@ -120,7 +114,7 @@ const user = await admin.resource('adminuser').asSystem({ hooks: false }).get( Check School with name 'Hawkins Elementary' exits in DB ```ts -const schoolExists = !!(await admin.resource('schools').asSystem({ hooks: false }).get( +const schoolExists = !!(await admin.resource('schools').get( [Filters.EQ('name', 'Hawkins Elementary')] )); ``` @@ -129,7 +123,7 @@ const schoolExists = !!(await admin.resource('schools').asSystem({ hooks: false Get user with name 'John' and role not 'SuperAdmin' ```ts -const user = await admin.resource('adminuser').asSystem({ hooks: false }).get( +const user = await admin.resource('adminuser').get( Filters.EQ('name', 'John'), Filters.NEQ('role', 'SuperAdmin') ); @@ -152,7 +146,7 @@ Signature: Get 15 latest users which role is not Admin: ```ts -const users = await admin.resource('adminuser').asSystem({ hooks: false }).list( +const users = await admin.resource('adminuser').list( [Filters.NEQ('role', 'Admin')], 15, 0, Sorts.DESC('createdAt') ); ``` @@ -160,19 +154,19 @@ const users = await admin.resource('adminuser').asSystem({ hooks: false }).list( Get 10 oldest users (with highest age): ```ts -const users = await admin.resource('adminuser').asSystem({ hooks: false }).list([], 10, 0, Sorts.ASC('age')); +const users = await admin.resource('adminuser').list([], 10, 0, Sorts.ASC('age')); ``` Get next page of oldest users: ```ts -const users = await admin.resource('adminuser').asSystem({ hooks: false }).list([], 10, 10, Sorts.ASC('age')); +const users = await admin.resource('adminuser').list([], 10, 10, Sorts.ASC('age')); ``` Get 10 schools, sort by rating first, then oldest by founded year: ```ts -const schools = await admin.resource('schools').asSystem({ hooks: false }).list( +const schools = await admin.resource('schools').list( [], 10, 0, [Sorts.DESC('rating'), Sorts.ASC('foundedYear')] ); ``` @@ -180,7 +174,7 @@ const schools = await admin.resource('schools').asSystem({ hooks: false }).list( Get all users that have gmail address AND the ones created not in 2024 ```ts -const users = await admin.resource('adminuser').asSystem({ hooks: false }).list( +const users = await admin.resource('adminuser').list( Filters.AND( Filters.LIKE('email', '@gmail.com'), Filters.OR( @@ -198,7 +192,7 @@ Technically it happened that AdminForth allows you to do this also ```js const minUgcAge = 18; -const usersWithNoUgcAccess = await admin.resource('adminuser').asSystem({ hooks: false }).list( +const usersWithNoUgcAccess = await admin.resource('adminuser').list( [ Filters.NEQ('role', 'Admin'), { @@ -236,7 +230,7 @@ Returns value representing created item with all fields, including fields which Create a new school: ```ts -await admin.resource('schools').asSystem().create({ +await admin.resource('schools').create({ name: 'Hawkins Elementary', rating: 5, foundedYear: 1950, @@ -258,7 +252,7 @@ Returns number of items in database which match the filters. Count number of schools with rating above 4: ```ts -const schoolsCount = await admin.resource('schools').asSystem({ hooks: false }).count(Filters.GT('rating', 4)); +const schoolsCount = await admin.resource('schools').count(Filters.GT('rating', 4)); ``` Create data for daily report with number of users signed up daily for last 7 days: @@ -275,7 +269,7 @@ const dailyReports = await Promise.all( const dateEnd = new Date(dateStart); dateEnd.setDate(dateEnd.getDate() + 1); - return admin.resource('adminuser').asSystem({ hooks: false }).count( + return admin.resource('adminuser').count( [Filters.GTE('createdAt', dateStart.toISOString()), Filters.LT('createdAt', dateEnd.toISOString())] ); }) @@ -298,7 +292,7 @@ Signature: Update school rating to 4.8 ```ts -await admin.resource('schools').asSystem().update('1234', { rating: 4.8 }); +await admin.resource('schools').update('1234', { rating: 4.8 }); ``` ## Delete item from database @@ -314,7 +308,7 @@ Signature: Delete school with ID '1234' ```ts -await admin.resource('schools').asSystem().delete('1234'); +await admin.resource('schools').delete('1234'); ``` @@ -330,10 +324,10 @@ Golden rule: create one index per query you are going to use often or where you For example if you have two queries: ```ts -const users = await admin.resource('adminuser').asSystem({ hooks: false }).list( +const users = await admin.resource('adminuser').list( [Filters.NEQ('role', 'Admin')], 15, 0, Sorts.DESC('createdAt') ); -const users = await admin.resource('adminuser').asSystem({ hooks: false }).list( +const users = await admin.resource('adminuser').list( [Filters.EQ('name', 'John'), Filters.NEQ('role', 'SuperAdmin')] ); ``` @@ -452,7 +446,7 @@ With explicit grouping aliases: ### Get daily apartment stats (count, avg, sum, median) for listed apartments ```ts -const rows = await admin.resource('apartments').asSystem({ hooks: false }).aggregate( +const rows = await admin.resource('apartments').aggregate( Filters.EQ('listed', true), { count: Aggregates.count(), @@ -477,7 +471,7 @@ median('price') → median price ### Get apartment stats grouped by country ```ts -const rows = await admin.resource('apartments').asSystem({ hooks: false }).aggregate( +const rows = await admin.resource('apartments').aggregate( [], { count: Aggregates.count(), @@ -497,7 +491,7 @@ What is happening here: ### Get apartment stats grouped by country and month ```ts -const rows = await admin.resource('apartments').asSystem({ hooks: false }).aggregate( +const rows = await admin.resource('apartments').aggregate( [], { count: Aggregates.count(), diff --git a/adminforth/documentation/docs/tutorial/03-Customization/12-security.md b/adminforth/documentation/docs/tutorial/03-Customization/12-security.md index 5baed23b4..1e0216dc4 100644 --- a/adminforth/documentation/docs/tutorial/03-Customization/12-security.md +++ b/adminforth/documentation/docs/tutorial/03-Customization/12-security.md @@ -118,8 +118,8 @@ This is opt-in. It is especially important for the column configured as `auth.us | Path | When `normalize` runs | | --- | --- | -| Scoped Data API with hooks (`admin.resource(...).asUser(...)` or `.asSystem()`) | Before validation and `beforeSave` hooks | -| Hook-free Data API (`admin.resource(...).asSystem({ hooks: false })` or an unscoped compatibility call) | Before validation and the connector operation | +| User-scoped Data API (`admin.resource(...).asUser(...)`) and `admin.createResourceRecord` | Before validation and `beforeSave` hooks | +| Bare Data API (`admin.resource(...).create(...)` and siblings) | Before validation and the connector operation | | Deprecated AdminForth CRUD (`createResourceRecord`, `updateResourceRecord`) | Before validation and `beforeSave` hooks | | Core password login | On the submitted value of `auth.usernameField`, before the user lookup | | Reads and filters | Never — this includes `get`, `list`, `count`, search, and `Filters.EQ` | diff --git a/adminforth/index.ts b/adminforth/index.ts index eab43eb65..4038d0e01 100644 --- a/adminforth/index.ts +++ b/adminforth/index.ts @@ -39,6 +39,8 @@ import ConfigValidator from './modules/configValidator.js'; import AdminForthRestAPI, { rejectApiRawFilters } from './modules/restApi.js'; import { interpretResource } from './modules/resourceAccess.js'; import OperationalResource from './modules/operationalResource.js'; +import UserScopedResource from './modules/userScopedResource.js'; +import { validateRecordValues } from './modules/recordValidator.js'; import SocketBroker from './modules/socketBroker.js'; import { afLogger } from './modules/logger.js'; import { normalizeRecordValues } from './modules/columnValueNormalizer.js'; @@ -426,59 +428,8 @@ class AdminForth implements IAdminForth { }); } - validateRecordValues(resource: AdminForthResource, record: any, mode: 'create' | 'edit'): any { - // check if record with validation is valid - for (const column of resource.columns.filter((col) => col.name in record && col.validation)) { - const required = typeof column.required === 'object' - ? column.required[mode] - : true; - - if (!required && !record[column.name]) continue; - - let error = null; - if (column.isArray?.enabled) { - error = record[column.name].reduce((err, item) => { - return err || AdminForth.Utils.applyRegexValidation(item, column.validation); - }, null); - } else { - error = AdminForth.Utils.applyRegexValidation(record[column.name], column.validation); - } - if (error) { - return error; - } - } - - // check if record with minValue or maxValue is within limits - for (const column of resource.columns.filter((col) => col.name in record - && ['integer', 'decimal', 'float'].includes(col.isArray?.enabled ? col.isArray.itemType : col.type) - && (col.minValue !== undefined || col.maxValue !== undefined))) { - if (column.isArray?.enabled) { - const error = record[column.name].reduce((err, item) => { - if (err) return err; - - if (column.minValue !== undefined && item < column.minValue) { - return `Value in "${column.name}" must be greater than ${column.minValue}`; - } - if (column.maxValue !== undefined && item > column.maxValue) { - return `Value in "${column.name}" must be less than ${column.maxValue}`; - } - - return null; - }, null); - if (error) { - return error; - } - } else { - if (column.minValue !== undefined && record[column.name] && record[column.name] < column.minValue) { - return `Value in "${column.name}" must be greater than ${column.minValue}`; - } - if (column.maxValue !== undefined && record[column.name] && record[column.name] > column.maxValue) { - return `Value in "${column.name}" must be less than ${column.maxValue}`; - } - } - } - - return null; + validateRecordValues(resource: AdminForthResource, record: any, mode: 'create' | 'edit'): string | null { + return validateRecordValues(resource, record, mode); } async tryToImportConnector(connectorName: string, doesUserHavePnpmLock: boolean) { @@ -667,13 +618,17 @@ class AdminForth implements IAdminForth { this.operationalResources[resource.resourceId] = new OperationalResource( this.connectors[resource.dataSource], resource, - this, - { - create: (params) => this.executeCreateResourceRecord(params), - update: (params) => this.executeUpdateResourceRecord(params), - delete: (params) => this.executeDeleteResourceRecord(params), - validate: (targetResource, record, mode) => this.validateRecordValues(targetResource, record, mode), - }, + (data, adminUser, options) => new UserScopedResource( + data, + this, + { + create: (params) => this.executeCreateResourceRecord(params), + update: (params) => this.executeUpdateResourceRecord(params), + delete: (params) => this.executeDeleteResourceRecord(params), + }, + adminUser, + options, + ), ); }); @@ -787,8 +742,10 @@ class AdminForth implements IAdminForth { } /** - * Create record and execute hooks - * @deprecated Will be removed in the next major version. Use the scoped resource API. + * Create record and execute hooks. + * @deprecated Being replaced by the Data API. Use + * `adminforth.resource(id).asUser(adminUser, { meta }).create(record)` for anything a user + * requested, or `adminforth.resource(id).create(record)` for plain data access. * @param params - Parameters for record creation. See CreateResourceRecordParams. * @returns Result of record creation. See CreateResourceRecordResult. */ @@ -806,7 +763,7 @@ class AdminForth implements IAdminForth { normalizeRecordValues(resource, record); - const err = this.validateRecordValues(resource, record, 'create'); + const err = validateRecordValues(resource, record, 'create'); if (err) { return { error: err }; } @@ -889,8 +846,10 @@ class AdminForth implements IAdminForth { /** * record is partial record with only changed fields * - * Update record by id and execute hooks - * @deprecated Will be removed in the next major version. Use the scoped resource API. + * Update record by id and execute hooks. + * @deprecated Being replaced by the Data API. Use + * `adminforth.resource(id).asUser(adminUser, { meta }).update(pk, updates)` for anything a + * user requested, or `adminforth.resource(id).update(pk, updates)` for plain data access. * @param params - Parameters for record update. See UpdateResourceRecordParams. * @returns Result of record update. See UpdateResourceRecordResult. */ @@ -898,12 +857,6 @@ class AdminForth implements IAdminForth { params: UpdateResourceRecordParams, ): Promise { this.warnDeprecatedResourceMutation('updateResourceRecord', params.resource.resourceId, 'update'); - const dataToUse = params.updates || params.record; - for (const column of params.resource.columns.filter((candidate) => candidate.editReadonly)) { - if (column.name in dataToUse) { - delete dataToUse[column.name]; - } - } return this.executeUpdateResourceRecord(params); } @@ -912,8 +865,17 @@ class AdminForth implements IAdminForth { ): Promise { const { resource, recordId, record, oldRecord, adminUser, response, extra, updates } = params; const dataToUse = updates || record; + + // a system update silently drops editReadonly columns, as it always has; a user update never + // reaches this point with one, it is rejected by the column access check inside asUser() + for (const column of resource.columns.filter((candidate) => candidate.editReadonly)) { + if (column.name in dataToUse) { + delete dataToUse[column.name]; + } + } + normalizeRecordValues(resource, dataToUse); - const err = this.validateRecordValues(resource, dataToUse, 'edit'); + const err = validateRecordValues(resource, dataToUse, 'edit'); if (err) { return { error: err }; } @@ -985,8 +947,10 @@ class AdminForth implements IAdminForth { } /** - * Delete record by id and execute hooks - * @deprecated Will be removed in the next major version. Use the scoped resource API. + * Delete record by id and execute hooks. + * @deprecated Being replaced by the Data API. Use + * `adminforth.resource(id).asUser(adminUser, { meta }).delete(pk)` for anything a user + * requested, or `adminforth.resource(id).delete(pk)` for plain data access. * @param params - Parameters for record deletion. See DeleteResourceRecordParams. * @returns Result of record deletion. See DeleteResourceRecordResult. */ @@ -1057,7 +1021,8 @@ class AdminForth implements IAdminForth { afLogger.warn( `${method} is deprecated and will be removed in the next major version. ` + `Use adminforth.resource('${resourceId}').asUser(adminUser, { meta }).${operation}(...) ` - + `or adminforth.resource('${resourceId}').asSystem({ hooks: false }).${operation}(...) instead.`, + + `for anything a user requested, or adminforth.resource('${resourceId}').${operation}(...) ` + + `for plain data access.`, ); } diff --git a/adminforth/modules/configValidator.ts b/adminforth/modules/configValidator.ts index 88859e435..5652a9064 100644 --- a/adminforth/modules/configValidator.ts +++ b/adminforth/modules/configValidator.ts @@ -34,7 +34,6 @@ import { import AdminForth from "adminforth"; import { AdminForthConfigMenuItem } from "adminforth"; import { afLogger } from "./logger.js"; -import {cascadeChildrenDelete} from './utils.js' const DEBOUNCE_TIME_MS = 300; const DEFAULT_AUTH_RATE_LIMIT: RateLimitString[] = ['500/5m', '5000/1h', '10000/1d']; @@ -262,59 +261,22 @@ export default class ConfigValidator implements IConfigValidator { dangerous: true, allowed: async ({ resource, adminUser, allowedActions }) => { return allowedActions.delete }, action: async ({ selectedIds, adminUser, response }) => { - const connector = this.adminforth.connectors[res.dataSource]; - - // for now if at least one error, stop and return error + // one path for deletion: asUser() checks the permission per record, cascades to children + // and runs the delete hooks, so this action does not carry its own copy of any of that let error = null; await Promise.all( selectedIds.map(async (recordId) => { - const record = await connector.getRecordByPrimaryKey(res as AdminForthResource, recordId); - - await Promise.all( - (res.hooks.delete.beforeSave).map( - async (hook) => { - const resp = await hook({ - recordId: recordId, - resource: res as AdminForthResource, - record, - adminUser, - response, - adminforth: this.adminforth - }); - if (!error && resp.error) { - error = resp.error; - } - } - ) - ) - - if (error) { - return; + try { + await this.adminforth + .resource(res.resourceId) + .asUser(adminUser, { response }) + .delete(recordId); + } catch (e) { + if (!error) { + error = (e as Error).message; + } } - - await cascadeChildrenDelete(res as AdminForthResource, recordId, { adminUser, response}, this.adminforth); - await connector.deleteRecord({ - resource: res as AdminForthResource, - recordId, - pkValues: compositePkValues(connector, res as AdminForthResource, recordId), - }); - - await Promise.all( - (res.hooks.delete.afterSave).map( - async (hook) => { - await hook({ - resource: res as AdminForthResource, - record, - adminUser, - recordId: recordId, - response, - adminforth: this.adminforth, - }); - } - ) - ) - }) ); diff --git a/adminforth/modules/operationalResource.ts b/adminforth/modules/operationalResource.ts index 7770d61a2..d2de7fbef 100644 --- a/adminforth/modules/operationalResource.ts +++ b/adminforth/modules/operationalResource.ts @@ -1,10 +1,6 @@ import type { AdminForthResource, - CreateResourceRecordParams, CreateResourceRecordResult, - DeleteResourceRecordParams, - DeleteResourceRecordResult, - IAdminForth, IAdminForthAndOrFilter, IAdminForthDataSourceConnectorBase, IAdminForthSingleFilter, @@ -13,206 +9,62 @@ import type { IGroupByRule, IOperationalResource, IScopedOperationalResource, - OperationalResourceSystemOptions, OperationalResourceUserOptions, - UpdateResourceRecordParams, - UpdateResourceRecordResult, } from '../types/Back.js'; -import { ActionCheckSource, AllowedActionsEnum, type AdminUser } from '../types/Common.js'; +import type { AdminUser } from '../types/Common.js'; import { compositePkValues } from './recordId.js'; import { normalizeRecordValues } from './columnValueNormalizer.js'; -import { - columnsAggregatableError, - recordWriteError, - stripReadForbiddenColumns, - type ColumnAccessContext, -} from './columnAccess.js'; -import { interpretResource } from './resourceAccess.js'; -import { filtersTools } from './filtersTools.js'; -import { cascadeChildrenDelete, hookResponseError, listify } from './utils.js'; -import { afLogger } from './logger.js'; - -type ResourceScope = - | { - type: 'user'; - adminUser: AdminUser; - options: OperationalResourceUserOptions; - } - | { - type: 'system'; - adminUser: AdminUser | null; - options: OperationalResourceSystemOptions; - }; +import { validateRecordValues } from './recordValidator.js'; /** - * Which resource permission guards which operation, and which check source the permission - * callbacks are told about. Kept as one table so the whole mapping can be audited at a glance - * instead of being read out of seven method bodies. + * Builds the user-scoped layer on top of a data-access resource. Injected by AdminForth so this + * module stays at the bottom of the stack: it knows the connector and the resource columns, and + * nothing about permissions, actions or lifecycle hooks. */ -const OPERATION_ACCESS = { - get: [AllowedActionsEnum.show, ActionCheckSource.ShowRequest], - list: [AllowedActionsEnum.list, ActionCheckSource.ListRequest], - count: [AllowedActionsEnum.list, ActionCheckSource.ListRequest], - create: [AllowedActionsEnum.create, ActionCheckSource.CreateRequest], - update: [AllowedActionsEnum.edit, ActionCheckSource.EditRequest], - delete: [AllowedActionsEnum.delete, ActionCheckSource.DeleteRequest], -} as const; - -type GuardedOperation = keyof typeof OPERATION_ACCESS; - -const warnedUnscopedOperations = new Set(); - -export interface OperationalResourceExecutors { - create(params: CreateResourceRecordParams): Promise; - update(params: UpdateResourceRecordParams): Promise; - delete(params: DeleteResourceRecordParams): Promise; - validate(resource: AdminForthResource, record: any, mode: 'create' | 'edit'): string | null; -} +export type UserScopeFactory = ( + data: OperationalResource, + adminUser: AdminUser, + options: OperationalResourceUserOptions, +) => IScopedOperationalResource; function sortsIfSort(sort: IAdminForthSort | IAdminForthSort[]): IAdminForthSort[] { return (Array.isArray(sort) ? sort : [sort]) as IAdminForthSort[]; } /** - * Resource API bound to a trust level. `scope` is always present here, so every method can ask - * for permissions and column access without re-deciding whether it is allowed to. + * Plain data access for one resource: talks to the connector, normalizes values and applies the + * column-level value rules. It has no notion of who is asking — no permissions, no column access + * rules, no lifecycle hooks. + * + * For anything a user asked for, take {@link asUser}, which adds those on top. */ -class ScopedOperationalResource implements IScopedOperationalResource { - constructor( - public dataConnector: IAdminForthDataSourceConnectorBase, - public resourceConfig: AdminForthResource, - private readonly adminforth: IAdminForth, - private readonly executors: OperationalResourceExecutors, - private readonly scope: ResourceScope, - ) {} - - private get meta(): any { - return this.scope.options.meta ?? {}; - } - - private get hooksEnabled(): boolean { - return this.scope.type === 'user' || this.scope.options.hooks !== false; - } - - /** Column rules only restrict what a real user may touch; system scopes are trusted. */ - private columnCtx(source: ActionCheckSource, meta: any = this.meta): ColumnAccessContext | null { - if (this.scope.type !== 'user') { - return null; - } - return { - adminUser: this.scope.adminUser, - resource: this.resourceConfig, - meta, - source, - adminforth: this.adminforth, - }; - } - - /** @returns the reason the operation is not allowed, or null when it is. */ - private async accessError(operation: GuardedOperation, meta: any = this.meta): Promise { - if (this.scope.type !== 'user') { - return null; - } - - const [action, source] = OPERATION_ACCESS[operation]; - const { allowedActions } = await interpretResource( - this.scope.adminUser, - this.resourceConfig, - meta, - source, - this.adminforth, - ); - const allowed = allowedActions[action] as boolean | string | undefined; - return allowed === true ? null : typeof allowed === 'string' ? allowed : 'Action is not allowed'; - } - - private readHookExtra(query: any) { - return this.scope.options.extra ?? { - body: query, - query: {}, - headers: {}, - cookies: [], - requestUrl: '', - response: this.scope.options.response, - }; - } - - private async runBeforeReadHooks(page: 'show' | 'list', query: any): Promise { - if (!this.hooksEnabled) { - return; - } +export default class OperationalResource implements IOperationalResource { + dataConnector: IAdminForthDataSourceConnectorBase; + resourceConfig: AdminForthResource; - for (const hook of listify(this.resourceConfig.hooks?.[page]?.beforeDatasourceRequest)) { - const tools = filtersTools.get(query); - // hooks reach these either as their own argument or off the query, and the documented - // spelling is query.filtersTools — so both have to be present, same as the REST path - query.filtersTools = tools; - const response = await hook({ - resource: this.resourceConfig, - query, - adminUser: this.scope.adminUser, - filtersTools: tools, - extra: this.readHookExtra(query), - adminforth: this.adminforth, - }); - const error = hookResponseError(response); - if (error) { - throw new Error(error.error); - } - } + constructor( + dataConnector: IAdminForthDataSourceConnectorBase, + resourceConfig: AdminForthResource, + private readonly scopeForUser: UserScopeFactory, + ) { + this.dataConnector = dataConnector; + this.resourceConfig = resourceConfig; } - private async runAfterReadHooks(page: 'show' | 'list', query: any, records: any[]): Promise { - if (!this.hooksEnabled) { - return; - } - - for (const hook of listify(this.resourceConfig.hooks?.[page]?.afterDatasourceResponse)) { - const response = await hook({ - resource: this.resourceConfig, - query, - response: records, - adminUser: this.scope.adminUser, - extra: this.readHookExtra(query), - adminforth: this.adminforth, - }); - const error = hookResponseError(response); - if (error) { - throw new Error(error.error); - } - } + asUser(adminUser: AdminUser, options: OperationalResourceUserOptions = {}): IScopedOperationalResource { + return this.scopeForUser(this, adminUser, options); } async get(filter: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array): Promise { - const accessError = await this.accessError('get'); - if (accessError) { - throw new Error(accessError); - } - - const query = { - filters: filter, - limit: 1, - offset: 0, - sort: [], - }; - await this.runBeforeReadHooks('show', query); - const records = ( + return ( await this.dataConnector.getData({ resource: this.resourceConfig, - filters: this.dataConnector.validateAndNormalizeInputFilters(query.filters), - limit: query.limit, - offset: query.offset, - sort: query.sort, + filters: this.dataConnector.validateAndNormalizeInputFilters(filter), + limit: 1, + offset: 0, + sort: [], }) - ).data; - - const record = records[0] || null; - const ctx = this.columnCtx(ActionCheckSource.ShowRequest); - if (record && ctx) { - await stripReadForbiddenColumns(ctx, record); - } - await this.runAfterReadHooks('show', query, records); - return record; + ).data[0] || null; } async list( @@ -222,12 +74,6 @@ class ScopedOperationalResource implements IScopedOperationalResource { sort: IAdminForthSort | IAdminForthSort[] = [], columns?: string[] ): Promise { - const accessError = await this.accessError('list'); - if (accessError) { - throw new Error(accessError); - } - - // check if type of limit and offset is number if (limit !== null && typeof limit !== 'number') { throw new Error('Limit must be a number'); } @@ -235,30 +81,15 @@ class ScopedOperationalResource implements IScopedOperationalResource { throw new Error('Offset must be a number'); } - const query = { - filters: filter, + const { data } = await this.dataConnector.getData({ + resource: this.resourceConfig, + filters: this.dataConnector.validateAndNormalizeInputFilters(filter), limit: limit === null ? 1000000000 : limit, offset: offset === null ? 0 : offset, sort: sortsIfSort(sort), - }; - await this.runBeforeReadHooks('list', query); - const { data } = await this.dataConnector.getData({ - resource: this.resourceConfig, - filters: this.dataConnector.validateAndNormalizeInputFilters(query.filters), - limit: query.limit, - offset: query.offset, - sort: query.sort, getTotals: false, columns: columns ? this.resourceConfig.dataSourceColumns.filter((column) => columns.includes(column.name)) : undefined, }); - - const ctx = this.columnCtx(ActionCheckSource.ListRequest); - if (ctx) { - for (const record of data) { - await stripReadForbiddenColumns(ctx, record); - } - } - await this.runAfterReadHooks('list', query, data); return data; } @@ -267,80 +98,25 @@ class ScopedOperationalResource implements IScopedOperationalResource { aggregations: { [alias: string]: IAggregationRule }, groupBy?: IGroupByRule | IGroupByRule[] ): Promise> { - // an aggregation reads a whole set of records at once, so it needs list access, and its - // min/max/groupBy return raw per-field values, which is what the show view does - const accessError = (await this.accessError('list')) ?? (await this.accessError('get')); - if (accessError) { - throw new Error(accessError); - } - - const ctx = this.columnCtx(ActionCheckSource.ShowRequest); - if (ctx) { - const columnError = await columnsAggregatableError(ctx, { aggregations, groupBy, filters: filter }); - if (columnError) { - throw new Error(columnError); - } - } - - // Row-scoping hooks are how multi-tenancy is expressed, and an aggregation reads the same - // rows a list does, so it has to be narrowed by them too — otherwise groupBy/min/max/sum - // report across every tenant. They run after the column check above, so that check still - // sees the caller's own filters and cannot be tripped by a filter a trusted hook added. - // Only the request side runs: the response here is aggregated rows, not records an - // afterDatasourceResponse hook could meaningfully process. - const query = { filters: filter, aggregations, groupBy, limit: null, offset: 0, sort: [] }; - await this.runBeforeReadHooks('list', query); - return this.dataConnector.aggregate({ resource: this.resourceConfig, - filters: this.dataConnector.validateAndNormalizeInputFilters(query.filters), + filters: this.dataConnector.validateAndNormalizeInputFilters(filter), aggregations, groupBy, }); } async count(filter?: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array | undefined): Promise { - const accessError = await this.accessError('count'); - if (accessError) { - throw new Error(accessError); - } - - // a count is a list the caller only learns the size of, so it is row-scoped the same way - const query = { filters: filter, limit: null, offset: 0, sort: [] }; - await this.runBeforeReadHooks('list', query); - return await this.dataConnector.getCount({ resource: this.resourceConfig, - filters: this.dataConnector.validateAndNormalizeInputFilters(query.filters), + filters: this.dataConnector.validateAndNormalizeInputFilters(filter), }); } async create(recordValues: any): Promise { - const accessError = await this.accessError('create'); - if (accessError) { - return { ok: false, createdRecord: undefined, error: accessError }; - } - - const ctx = this.columnCtx(ActionCheckSource.CreateRequest); - const columnError = ctx && await recordWriteError(ctx, recordValues, 'create'); - if (columnError) { - return { ok: false, createdRecord: undefined, error: columnError }; - } - - if (this.hooksEnabled) { - const result = await this.executors.create({ - resource: this.resourceConfig, - record: recordValues, - adminUser: this.scope.adminUser, - extra: this.scope.options.extra, - response: this.scope.options.response, - }); - return { ...result, ok: !result.error, createdRecord: result.createdRecord }; - } - const normalizedRecord = { ...recordValues }; normalizeRecordValues(this.resourceConfig, normalizedRecord); - const validationError = this.executors.validate(this.resourceConfig, normalizedRecord, 'create'); + const validationError = validateRecordValues(this.resourceConfig, normalizedRecord, 'create'); if (validationError) { return { ok: false, createdRecord: undefined, error: validationError }; } @@ -348,7 +124,7 @@ class ScopedOperationalResource implements IScopedOperationalResource { const { ok, createdRecord, error } = await this.dataConnector.createRecord({ resource: this.resourceConfig, record: normalizedRecord, - adminUser: this.scope.adminUser, + adminUser: null, }); return { ok, createdRecord, error }; } @@ -358,159 +134,25 @@ class ScopedOperationalResource implements IScopedOperationalResource { return { ok: true }; } - if (!this.hooksEnabled) { - const normalizedRecord = { ...record }; - normalizeRecordValues(this.resourceConfig, normalizedRecord); - const validationError = this.executors.validate(this.resourceConfig, normalizedRecord, 'edit'); - if (validationError) { - return { ok: false, error: validationError }; - } - return this.dataConnector.updateRecord({ - resource: this.resourceConfig, - recordId: primaryKey, - newValues: normalizedRecord, - }); - } - - const oldRecord = this.scope.options.oldRecord - ?? await this.dataConnector.getRecordByPrimaryKey(this.resourceConfig, primaryKey); - if (!oldRecord) { - const primaryKeyColumn = this.resourceConfig.columns.find((column) => column.primaryKey); - return { ok: false, error: `Record with ${primaryKeyColumn.name} ${primaryKey} not found` }; - } - - const meta = { ...this.meta, newRecord: record, oldRecord, pk: primaryKey }; - - const accessError = await this.accessError('update', meta); - if (accessError) { - return { ok: false, error: accessError }; - } - - const ctx = this.columnCtx(ActionCheckSource.EditRequest, meta); - const columnError = ctx && await recordWriteError(ctx, record, 'edit'); - if (columnError) { - return { ok: false, error: columnError }; + const normalizedRecord = { ...record }; + normalizeRecordValues(this.resourceConfig, normalizedRecord); + const validationError = validateRecordValues(this.resourceConfig, normalizedRecord, 'edit'); + if (validationError) { + return { ok: false, error: validationError }; } - const result = await this.executors.update({ + return await this.dataConnector.updateRecord({ resource: this.resourceConfig, recordId: primaryKey, - updates: record, - oldRecord, - adminUser: this.scope.adminUser, - extra: this.scope.options.extra, - response: this.scope.options.response, + newValues: normalizedRecord, }); - return { ...result, ok: !result.error }; } async delete(primaryKey: any): Promise { - if (!this.hooksEnabled) { - return this.dataConnector.deleteRecord({ - resource: this.resourceConfig, - recordId: primaryKey, - pkValues: compositePkValues(this.dataConnector, this.resourceConfig, primaryKey), - }); - } - - const record = this.scope.options.record - ?? await this.dataConnector.getRecordByPrimaryKey(this.resourceConfig, primaryKey); - if (!record) { - return false; - } - - const accessError = await this.accessError('delete', { ...this.meta, record, pk: primaryKey }); - if (accessError) { - throw new Error(accessError); - } - - const { error: cascadeError } = await cascadeChildrenDelete( - this.resourceConfig, - primaryKey, - { adminUser: this.scope.adminUser, response: this.scope.options.response }, - this.adminforth, - ); - if (cascadeError) { - throw new Error(cascadeError); - } - - const { error } = await this.executors.delete({ + return await this.dataConnector.deleteRecord({ resource: this.resourceConfig, recordId: primaryKey, - record, - adminUser: this.scope.adminUser, - extra: this.scope.options.extra, - response: this.scope.options.response, + pkValues: compositePkValues(this.dataConnector, this.resourceConfig, primaryKey), }); - if (error) { - throw new Error(error); - } - return true; - } -} - -/** - * Entry point returned by `adminforth.resource(id)`. It carries no trust level of its own — - * pick one with `asUser()` or `asSystem()`. The bare operations are deprecated aliases of - * `asSystem({ hooks: false })`, kept for backward compatibility. - */ -export default class OperationalResource implements IOperationalResource { - constructor( - public dataConnector: IAdminForthDataSourceConnectorBase, - public resourceConfig: AdminForthResource, - private readonly adminforth: IAdminForth, - private readonly executors: OperationalResourceExecutors, - ) {} - - private scoped(scope: ResourceScope): IScopedOperationalResource { - return new ScopedOperationalResource( - this.dataConnector, - this.resourceConfig, - this.adminforth, - this.executors, - scope, - ); - } - - asUser(adminUser: AdminUser, options: OperationalResourceUserOptions = {}): IScopedOperationalResource { - return this.scoped({ type: 'user', adminUser, options }); } - - asSystem(options: OperationalResourceSystemOptions = {}): IScopedOperationalResource { - return this.scoped({ type: 'system', adminUser: options.adminUser ?? null, options }); - } - - /** Warns once per resource and operation, then falls back to the trusted, hook-free scope. */ - private legacy(operation: keyof IScopedOperationalResource): IScopedOperationalResource { - const warnKey = `${this.resourceConfig.resourceId}.${operation}`; - if (!warnedUnscopedOperations.has(warnKey)) { - warnedUnscopedOperations.add(warnKey); - afLogger.warn( - `adminforth.resource('${this.resourceConfig.resourceId}').${operation}(...) is deprecated and will be removed in the next major version. ` - + `Use .asUser(adminUser, { meta }).${operation}(...) or .asSystem({ hooks: false }).${operation}(...) instead.`, - ); - } - return this.asSystem({ hooks: false }); - } - - /** @deprecated Use `asUser(...).get(...)` or `asSystem({ hooks: false }).get(...)`. */ - get(...args: Parameters) { return this.legacy('get').get(...args); } - - /** @deprecated Use `asUser(...).list(...)` or `asSystem({ hooks: false }).list(...)`. */ - list(...args: Parameters) { return this.legacy('list').list(...args); } - - /** @deprecated Use `asUser(...).count(...)` or `asSystem({ hooks: false }).count(...)`. */ - count(...args: Parameters) { return this.legacy('count').count(...args); } - - /** @deprecated Use `asUser(...).aggregate(...)` or `asSystem({ hooks: false }).aggregate(...)`. */ - aggregate(...args: Parameters) { return this.legacy('aggregate').aggregate(...args); } - - /** @deprecated Use `asUser(...).create(...)` or `asSystem({ hooks: false }).create(...)`. */ - create(...args: Parameters) { return this.legacy('create').create(...args); } - - /** @deprecated Use `asUser(...).update(...)` or `asSystem({ hooks: false }).update(...)`. */ - update(...args: Parameters) { return this.legacy('update').update(...args); } - - /** @deprecated Use `asUser(...).delete(...)` or `asSystem({ hooks: false }).delete(...)`. */ - delete(...args: Parameters) { return this.legacy('delete').delete(...args); } } diff --git a/adminforth/modules/recordValidator.ts b/adminforth/modules/recordValidator.ts new file mode 100644 index 000000000..acf0e28b1 --- /dev/null +++ b/adminforth/modules/recordValidator.ts @@ -0,0 +1,63 @@ +import type { AdminForthResource } from '../types/Back.js'; +import { applyRegexValidation } from './utils.js'; + +/** + * Column-level value rules: `validation` patterns and `minValue`/`maxValue` bounds. + * Depends on nothing but the resource columns, so any layer can apply it. + * + * @returns the first validation error, or null when the record passes. + */ +export function validateRecordValues(resource: AdminForthResource, record: any, mode: 'create' | 'edit'): string | null { + // check if record with validation is valid + for (const column of resource.columns.filter((col) => col.name in record && col.validation)) { + const required = typeof column.required === 'object' + ? column.required[mode] + : true; + + if (!required && !record[column.name]) continue; + + let error = null; + if (column.isArray?.enabled) { + error = record[column.name].reduce((err, item) => { + return err || applyRegexValidation(item, column.validation); + }, null); + } else { + error = applyRegexValidation(record[column.name], column.validation); + } + if (error) { + return error; + } + } + + // check if record with minValue or maxValue is within limits + for (const column of resource.columns.filter((col) => col.name in record + && ['integer', 'decimal', 'float'].includes(col.isArray?.enabled ? col.isArray.itemType : col.type) + && (col.minValue !== undefined || col.maxValue !== undefined))) { + if (column.isArray?.enabled) { + const error = record[column.name].reduce((err, item) => { + if (err) return err; + + if (column.minValue !== undefined && item < column.minValue) { + return `Value in "${column.name}" must be greater than ${column.minValue}`; + } + if (column.maxValue !== undefined && item > column.maxValue) { + return `Value in "${column.name}" must be less than ${column.maxValue}`; + } + + return null; + }, null); + if (error) { + return error; + } + } else { + if (column.minValue !== undefined && record[column.name] && record[column.name] < column.minValue) { + return `Value in "${column.name}" must be greater than ${column.minValue}`; + } + if (column.maxValue !== undefined && record[column.name] && record[column.name] > column.maxValue) { + return `Value in "${column.name}" must be less than ${column.maxValue}`; + } + } + } + + return null; +} diff --git a/adminforth/modules/restApi.ts b/adminforth/modules/restApi.ts index de96b4d32..d71f08092 100644 --- a/adminforth/modules/restApi.ts +++ b/adminforth/modules/restApi.ts @@ -902,7 +902,6 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { const usersResource = this.adminforth.config.resources.find((res) => res.resourceId === this.adminforth.config.auth.usersResourceId); const defaultUserExists = await this.adminforth .resource(usersResource.resourceId) - .asSystem({ hooks: false }) .get(Filters.EQ(usernameField, 'adminforth')) ? true : false; const loggedInPart = { @@ -2029,7 +2028,7 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { if (isCompositePrimaryKey(resource)) { const createPkColumnNames = primaryKeyColumnNames(resource); if (createPkColumnNames.every((name) => record[name] !== undefined)) { - const existingRecord = await this.adminforth.resource(resource.resourceId).asSystem({ hooks: false }).get( + const existingRecord = await this.adminforth.resource(resource.resourceId).get( createPkColumnNames.map((name) => Filters.EQ(name, record[name])) ); if (existingRecord) { @@ -2043,7 +2042,6 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { const primaryKeyColumn = resource.columns.find((col) => col.primaryKey); if (record[primaryKeyColumn.name] !== undefined) { const existingRecord = await this.adminforth.resource(resource.resourceId) - .asSystem({ hooks: false }) .get([Filters.EQ(primaryKeyColumn.name, record[primaryKeyColumn.name])]); if (existingRecord) { return { error: `Record with ${primaryKeyColumn.name} '${record[primaryKeyColumn.name]}' already exists`, ok: false }; @@ -2198,7 +2196,7 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { acc[name] = record[name] !== undefined ? record[name] : oldRecord[name]; return acc; }, {}); - const existingRecord = await this.adminforth.resource(resource.resourceId).asSystem({ hooks: false }).get( + const existingRecord = await this.adminforth.resource(resource.resourceId).get( pkColumnNames.map((name) => Filters.EQ(name, newPkValues[name])) ); if (existingRecord) { @@ -2212,7 +2210,6 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { const primaryKeyColumn = resource.columns.find((col) => col.primaryKey); if (record[primaryKeyColumn.name] !== undefined) { const existingRecord = await this.adminforth.resource(resource.resourceId) - .asSystem({ hooks: false }) .get([Filters.EQ(primaryKeyColumn.name, record[primaryKeyColumn.name])]); if (existingRecord) { return { error: `Record with ${primaryKeyColumn.name} '${record[primaryKeyColumn.name]}' already exists`, ok: false }; diff --git a/adminforth/modules/userScopedResource.ts b/adminforth/modules/userScopedResource.ts new file mode 100644 index 000000000..220448197 --- /dev/null +++ b/adminforth/modules/userScopedResource.ts @@ -0,0 +1,336 @@ +import type { + AdminForthResource, + CreateResourceRecordParams, + CreateResourceRecordResult, + DeleteResourceRecordParams, + DeleteResourceRecordResult, + IAdminForth, + IAdminForthAndOrFilter, + IAdminForthDataSourceConnectorBase, + IAdminForthSingleFilter, + IAdminForthSort, + IAggregationRule, + IGroupByRule, + IScopedOperationalResource, + OperationalResourceUserOptions, + UpdateResourceRecordParams, + UpdateResourceRecordResult, +} from '../types/Back.js'; +import { ActionCheckSource, AllowedActionsEnum, type AdminUser } from '../types/Common.js'; +import { + columnsAggregatableError, + recordWriteError, + stripReadForbiddenColumns, + type ColumnAccessContext, +} from './columnAccess.js'; +import { interpretResource } from './resourceAccess.js'; +import { filtersTools } from './filtersTools.js'; +import { cascadeChildrenDelete, hookResponseError, listify } from './utils.js'; +import type OperationalResource from './operationalResource.js'; + +/** + * Which resource permission guards which operation, and which check source the permission + * callbacks are told about. Kept as one table so the whole mapping can be audited at a glance + * instead of being read out of seven method bodies. + */ +const OPERATION_ACCESS = { + get: [AllowedActionsEnum.show, ActionCheckSource.ShowRequest], + list: [AllowedActionsEnum.list, ActionCheckSource.ListRequest], + count: [AllowedActionsEnum.list, ActionCheckSource.ListRequest], + create: [AllowedActionsEnum.create, ActionCheckSource.CreateRequest], + update: [AllowedActionsEnum.edit, ActionCheckSource.EditRequest], + delete: [AllowedActionsEnum.delete, ActionCheckSource.DeleteRequest], +} as const; + +type GuardedOperation = keyof typeof OPERATION_ACCESS; + +/** Hook-running write paths, supplied by AdminForth. */ +export interface ResourceHookExecutors { + create(params: CreateResourceRecordParams): Promise; + update(params: UpdateResourceRecordParams): Promise; + delete(params: DeleteResourceRecordParams): Promise; +} + + +/** + * Resource API bound to an authenticated admin user. Every operation enforces the resource ACL + * and the column access rules, and runs the resource lifecycle hooks. + */ +export default class UserScopedResource implements IScopedOperationalResource { + readonly dataConnector: IAdminForthDataSourceConnectorBase; + readonly resourceConfig: AdminForthResource; + + constructor( + private readonly data: OperationalResource, + private readonly adminforth: IAdminForth, + private readonly executors: ResourceHookExecutors, + private readonly adminUser: AdminUser, + private readonly options: OperationalResourceUserOptions, + ) { + this.dataConnector = data.dataConnector; + this.resourceConfig = data.resourceConfig; + } + + private get meta(): any { + return this.options.meta ?? {}; + } + + private columnCtx(source: ActionCheckSource, meta: any = this.meta): ColumnAccessContext { + return { + adminUser: this.adminUser, + resource: this.resourceConfig, + meta, + source, + adminforth: this.adminforth, + }; + } + + /** @returns the reason the operation is not allowed, or null when it is. */ + private async accessError(operation: GuardedOperation, meta: any = this.meta): Promise { + const [action, source] = OPERATION_ACCESS[operation]; + const { allowedActions } = await interpretResource( + this.adminUser, + this.resourceConfig, + meta, + source, + this.adminforth, + ); + const allowed = allowedActions[action] as boolean | string | undefined; + return allowed === true ? null : typeof allowed === 'string' ? allowed : 'Action is not allowed'; + } + + /** + * Runs one phase of the resource read hooks. `beforeDatasourceRequest` may narrow `query` + * (this is how row-level multi-tenancy is expressed); `afterDatasourceResponse` may rewrite + * the records it is handed. + */ + private async runReadHooks( + page: 'show' | 'list', + phase: 'beforeDatasourceRequest' | 'afterDatasourceResponse', + query: any, + records?: any[], + ): Promise { + const hooks = listify(this.resourceConfig.hooks?.[page]?.[phase]); + if (!hooks.length) { + return; + } + + for (const hook of hooks) { + const payload: any = { + resource: this.resourceConfig, + query, + adminUser: this.adminUser, + extra: this.options.extra ?? { + body: query, + query: {}, + headers: {}, + cookies: [], + requestUrl: '', + response: this.options.response, + }, + adminforth: this.adminforth, + }; + + if (phase === 'beforeDatasourceRequest') { + // hooks reach these either as their own argument or off the query, and the documented + // spelling is query.filtersTools — so both have to be present, same as the REST path + payload.filtersTools = filtersTools.get(query); + query.filtersTools = payload.filtersTools; + } else { + payload.response = records; + } + + const error = hookResponseError(await hook(payload)); + if (error) { + throw new Error(error.error); + } + } + } + + async get(filter: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array): Promise { + const accessError = await this.accessError('get'); + if (accessError) { + throw new Error(accessError); + } + + const query = { filters: filter, limit: 1, offset: 0, sort: [] }; + await this.runReadHooks('show', 'beforeDatasourceRequest', query); + const record = await this.data.get(query.filters); + const records = record ? [record] : []; + + if (record) { + await stripReadForbiddenColumns(this.columnCtx(ActionCheckSource.ShowRequest), record); + } + await this.runReadHooks('show', 'afterDatasourceResponse', query, records); + return record; + } + + async list( + filter: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array, + limit: number | null = null, + offset: number | null = null, + sort: IAdminForthSort | IAdminForthSort[] = [], + columns?: string[] + ): Promise { + const accessError = await this.accessError('list'); + if (accessError) { + throw new Error(accessError); + } + + const query = { filters: filter, limit, offset, sort }; + await this.runReadHooks('list', 'beforeDatasourceRequest', query); + const data = await this.data.list(query.filters, query.limit, query.offset, query.sort, columns); + + const ctx = this.columnCtx(ActionCheckSource.ListRequest); + for (const record of data) { + await stripReadForbiddenColumns(ctx, record); + } + await this.runReadHooks('list', 'afterDatasourceResponse', query, data); + return data; + } + + async aggregate( + filter: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array, + aggregations: { [alias: string]: IAggregationRule }, + groupBy?: IGroupByRule | IGroupByRule[] + ): Promise> { + // an aggregation reads a whole set of records at once, so it needs list access, and its + // min/max/groupBy return raw per-field values, which is what the show view does + const accessError = (await this.accessError('list')) ?? (await this.accessError('get')); + if (accessError) { + throw new Error(accessError); + } + + const columnError = await columnsAggregatableError( + this.columnCtx(ActionCheckSource.ShowRequest), + { aggregations, groupBy, filters: filter }, + ); + if (columnError) { + throw new Error(columnError); + } + + // An aggregation reads the same rows a list does, so the row-scoping hooks have to narrow + // it too, or groupBy/min/max/sum report across every tenant. They run after the column check + // above, so that check still sees the caller's own filters. Only the request phase runs: the + // response here is aggregated rows, not records an after hook could process. + const query = { filters: filter, aggregations, groupBy, limit: null, offset: 0, sort: [] }; + await this.runReadHooks('list', 'beforeDatasourceRequest', query); + + return this.data.aggregate(query.filters, aggregations, groupBy); + } + + async count(filter?: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array | undefined): Promise { + const accessError = await this.accessError('count'); + if (accessError) { + throw new Error(accessError); + } + + // a count is a list the caller only learns the size of, so it is row-scoped the same way + const query = { filters: filter, limit: null, offset: 0, sort: [] }; + await this.runReadHooks('list', 'beforeDatasourceRequest', query); + + return this.data.count(query.filters); + } + + async create(recordValues: any): Promise { + const accessError = await this.accessError('create'); + if (accessError) { + return { ok: false, createdRecord: undefined, error: accessError }; + } + + const columnError = await recordWriteError( + this.columnCtx(ActionCheckSource.CreateRequest), + recordValues, + 'create', + ); + if (columnError) { + return { ok: false, createdRecord: undefined, error: columnError }; + } + + const result = await this.executors.create({ + resource: this.resourceConfig, + record: recordValues, + adminUser: this.adminUser, + extra: this.options.extra, + response: this.options.response, + }); + return { ...result, ok: !result.error, createdRecord: result.createdRecord }; + } + + async update(primaryKey: any, record: any): Promise { + if (Object.keys(record).length === 0) { + return { ok: true }; + } + + const oldRecord = this.options.oldRecord + ?? await this.dataConnector.getRecordByPrimaryKey(this.resourceConfig, primaryKey); + if (!oldRecord) { + const primaryKeyColumn = this.resourceConfig.columns.find((column) => column.primaryKey); + return { ok: false, error: `Record with ${primaryKeyColumn.name} ${primaryKey} not found` }; + } + + const meta = { ...this.meta, newRecord: record, oldRecord, pk: primaryKey }; + + const accessError = await this.accessError('update', meta); + if (accessError) { + return { ok: false, error: accessError }; + } + + const columnError = await recordWriteError( + this.columnCtx(ActionCheckSource.EditRequest, meta), + record, + 'edit', + ); + if (columnError) { + return { ok: false, error: columnError }; + } + + const result = await this.executors.update({ + resource: this.resourceConfig, + recordId: primaryKey, + updates: record, + oldRecord, + adminUser: this.adminUser, + extra: this.options.extra, + response: this.options.response, + }); + return { ...result, ok: !result.error }; + } + + async delete(primaryKey: any): Promise { + const record = this.options.record + ?? await this.dataConnector.getRecordByPrimaryKey(this.resourceConfig, primaryKey); + if (!record) { + return false; + } + + const accessError = await this.accessError('delete', { ...this.meta, record, pk: primaryKey }); + if (accessError) { + throw new Error(accessError); + } + + const { error: cascadeError } = await cascadeChildrenDelete( + this.resourceConfig, + primaryKey, + { adminUser: this.adminUser, response: this.options.response }, + this.adminforth, + (params) => this.executors.delete(params), + ); + if (cascadeError) { + throw new Error(cascadeError); + } + + const { error } = await this.executors.delete({ + resource: this.resourceConfig, + recordId: primaryKey, + record, + adminUser: this.adminUser, + extra: this.options.extra, + response: this.options.response, + }); + if (error) { + throw new Error(error); + } + return true; + } +} diff --git a/adminforth/modules/utils.ts b/adminforth/modules/utils.ts index 4c91ef59c..8a84b130b 100644 --- a/adminforth/modules/utils.ts +++ b/adminforth/modules/utils.ts @@ -534,7 +534,22 @@ export function slugifyString(str: string): string { .replace(/[^a-z0-9-_]/g, '-'); } -export async function cascadeChildrenDelete(resource: AdminForthResource, primaryKey: string, context: {adminUser: any, response: any}, adminforth: IAdminForth): Promise<{ error: string | null }> { +/** + * Applies the configured onDelete strategy to every child record pointing at `primaryKey`. + * + * Deleting a child has to run that resource's delete hooks — upload releases its S3 objects, + * many2many drops junction rows, audit-log records the deletion from there. `deleteWithHooks` + * supplies that, so this does not depend on any particular public entry point. + */ +export async function cascadeChildrenDelete( + resource: AdminForthResource, + primaryKey: string, + context: {adminUser: any, response: any}, + adminforth: IAdminForth, + deleteWithHooks: (params: { + resource: AdminForthResource, recordId: any, record: any, adminUser: any, response: any, + }) => Promise<{ error?: string }>, +): Promise<{ error: string | null }> { const { adminUser, response } = context; const childResources = adminforth.config.resources.filter(r =>r.columns.some(c => c.foreignResource?.resourceId === resource.resourceId)); @@ -548,7 +563,6 @@ export async function cascadeChildrenDelete(resource: AdminForthResource, primar const childRecords = await adminforth .resource(childRes.resourceId) - .asSystem({ hooks: false }) .list(Filters.EQ(foreignColumn.name, primaryKey)); const childPk = childRes.columns.find(c => c.primaryKey)?.name; @@ -558,19 +572,25 @@ export async function cascadeChildrenDelete(resource: AdminForthResource, primar if (strategy === 'cascade') { for (const childRecord of childRecords) { - try { - await adminforth.resource(childRes.resourceId) - .asSystem({ adminUser, response, record: childRecord }) - .delete(childRecordId(childRecord)); - } catch (e) { - return { error: (e as Error).message }; + // Grandchildren first, then the child itself. + const childResult = await cascadeChildrenDelete( + childRes, childRecordId(childRecord), context, adminforth, deleteWithHooks, + ); + if (childResult?.error) { + return childResult; + } + const deleteChild = await deleteWithHooks({ + resource: childRes, record: childRecord, adminUser, recordId: childRecordId(childRecord), response, + }); + if (deleteChild.error) { + return { error: deleteChild.error }; } } } if (strategy === 'setNull') { for (const childRecord of childRecords) { - const result = await adminforth.resource(childRes.resourceId).asSystem({ hooks: false }).update( + const result = await adminforth.resource(childRes.resourceId).update( childRecordId(childRecord), { [foreignColumn.name]: null }, ); diff --git a/adminforth/types/Back.ts b/adminforth/types/Back.ts index 432313377..b97c0287e 100644 --- a/adminforth/types/Back.ts +++ b/adminforth/types/Back.ts @@ -613,25 +613,24 @@ export interface IAdminForth { tr(msg: string, category: string, lang: string, params: any, pluralizationNumber?: number): Promise; /** - * @deprecated Will be removed in the next major version. Use - * `resource(resourceId).asUser(adminUser, { meta }).create(record)` or - * `resource(resourceId).asSystem({ hooks: false }).create(record)`. + * Creates a record and runs the resource create hooks, without checking permissions. + * For an operation a user requested, use `resource(resourceId).asUser(adminUser, { meta })`. */ createResourceRecord( params: CreateResourceRecordParams, ): Promise; /** - * @deprecated Will be removed in the next major version. Use the scoped - * resource API through `asUser()` or `asSystem()`. + * Updates a record and runs the resource edit hooks, without checking permissions. + * For an operation a user requested, use `resource(resourceId).asUser(adminUser, { meta })`. */ updateResourceRecord( params: UpdateResourceRecordParams, ): Promise; /** - * @deprecated Will be removed in the next major version. Use the scoped - * resource API through `asUser()` or `asSystem()`. + * Deletes a record and runs the resource delete hooks, without checking permissions. + * For an operation a user requested, use `resource(resourceId).asUser(adminUser, { meta })`. */ deleteResourceRecord( params: DeleteResourceRecordParams, @@ -2223,8 +2222,9 @@ export class Sorts { } /** - * Resource API scoped to a trust level by {@link IOperationalResource.asUser} or - * {@link IOperationalResource.asSystem}. + * Resource API bound to an authenticated admin user by {@link IOperationalResource.asUser}. + * Every operation enforces the resource ACL and the column access rules, and runs the resource + * lifecycle hooks. * * Error contract: a denied or failed operation is always visible. `get`, `list`, `count`, * `aggregate` and `delete` throw, since their return value carries no room for an error; @@ -2255,36 +2255,36 @@ export interface IScopedOperationalResource { export interface IOperationalResource { /** - * Returns a resource API scoped to an authenticated admin user. Operations enforce - * resource ACL and column access and run lifecycle hooks; mutations also validate records. + * Returns a resource API scoped to an authenticated admin user. Operations enforce the + * resource ACL and the column access rules, run lifecycle hooks, and validate records. + * Use it for everything a user asked for. */ asUser: (adminUser: AdminUser, options?: OperationalResourceUserOptions) => IScopedOperationalResource; /** - * Returns a trusted resource API which skips ACL and column access. Hooks run by - * default and can be disabled explicitly for connector-level system operations. + * Plain data access: no permission checks, no column access rules, no lifecycle hooks. + * Writes are still normalized and validated. Use it for internal bookkeeping the user did not + * ask for; to run hooks without permission checks, use {@link IAdminForth.createResourceRecord} + * and its siblings. */ - asSystem: (options?: OperationalResourceSystemOptions) => IScopedOperationalResource; - - /** @deprecated Use `asUser(...).get(...)` or `asSystem({ hooks: false }).get(...)`. */ get: IScopedOperationalResource['get']; - /** @deprecated Use `asUser(...).list(...)` or `asSystem({ hooks: false }).list(...)`. */ + /** Plain data access — see {@link IOperationalResource.get}. */ list: IScopedOperationalResource['list']; - /** @deprecated Use `asUser(...).count(...)` or `asSystem({ hooks: false }).count(...)`. */ + /** Plain data access — see {@link IOperationalResource.get}. */ count: IScopedOperationalResource['count']; - /** @deprecated Use `asUser(...).aggregate(...)` or `asSystem({ hooks: false }).aggregate(...)`. */ + /** Plain data access — see {@link IOperationalResource.get}. */ aggregate: IScopedOperationalResource['aggregate']; - /** @deprecated Use `asUser(...).create(...)` or `asSystem({ hooks: false }).create(...)`. */ + /** Plain data access — see {@link IOperationalResource.get}. */ create: IScopedOperationalResource['create']; - /** @deprecated Use `asUser(...).update(...)` or `asSystem({ hooks: false }).update(...)`. */ + /** Plain data access — see {@link IOperationalResource.get}. */ update: IScopedOperationalResource['update']; - /** @deprecated Use `asUser(...).delete(...)` or `asSystem({ hooks: false }).delete(...)`. */ + /** Plain data access — see {@link IOperationalResource.get}. */ delete: IScopedOperationalResource['delete']; dataConnector: IAdminForthDataSourceConnectorBase; @@ -2311,11 +2311,6 @@ export interface OperationalResourceContextOptions { export type OperationalResourceUserOptions = OperationalResourceContextOptions; -export interface OperationalResourceSystemOptions extends OperationalResourceContextOptions { - hooks?: boolean; - /** User attribution passed to hooks and fillOnCreate without enabling ACL checks. */ - adminUser?: AdminUser; -} diff --git a/tests/jest_tests/operational_resource_scope.test.ts b/tests/jest_tests/operational_resource_scope.test.ts index b72c67e41..89efa6bdc 100644 --- a/tests/jest_tests/operational_resource_scope.test.ts +++ b/tests/jest_tests/operational_resource_scope.test.ts @@ -1,4 +1,5 @@ import OperationalResource from '../../adminforth/modules/operationalResource.js'; +import UserScopedResource from '../../adminforth/modules/userScopedResource.js'; import { ActionCheckSource } from '../../adminforth/types/Common.js'; function setup(resourceId = 'users') { @@ -13,7 +14,6 @@ function setup(resourceId = 'users') { connectorGetByPk: 0, connectorCount: 0, connectorAggregate: 0, - validate: 0, beforeList: 0, afterList: 0, }; @@ -114,20 +114,20 @@ function setup(resourceId = 'users') { return { error: null }; }, delete: async () => ({ error: null }), - validate: () => { - calls.validate++; - return null; - }, } as any; return { calls, seenFilters, - resource: new OperationalResource(connector, resource, adminforth, executors), + resource: new OperationalResource( + connector, + resource, + (data, adminUser, options) => new UserScopedResource(data, adminforth, executors, adminUser, options), + ), }; } -describe('OperationalResource access scopes', () => { +describe('OperationalResource access tiers', () => { it('enforces ACL, column access, validation, and hooks for asUser()', async () => { const { calls, resource } = setup(); const denied = await resource.asUser({} as any, { meta: { allowed: false } }).create({ name: 'John' }); @@ -142,63 +142,23 @@ describe('OperationalResource access scopes', () => { expect(calls).toMatchObject({ acl: 3, createExecutor: 1, connectorCreate: 0 }); }); - it('runs hooks by default for asSystem()', async () => { + it('runs no permission checks, no column checks and no hooks on the bare API', async () => { const { calls, resource } = setup(); - const created = await resource.asSystem({ meta: { allowed: false } }).create({ secret: 'value' }); + const created = await resource.create({ secret: 'value' }); expect(created).toMatchObject({ ok: true, createdRecord: { id: 1, secret: 'value' } }); - expect(calls).toMatchObject({ acl: 0, createExecutor: 1, connectorCreate: 0 }); - }); - - it('uses validation and the connector when system hooks are disabled', async () => { - const { calls, resource } = setup(); - const created = await resource.asSystem({ hooks: false }).create({ secret: 'value' }); - - expect(created).toMatchObject({ ok: true, createdRecord: { id: 1, secret: 'value' } }); - expect(calls).toMatchObject({ acl: 0, createExecutor: 0, connectorCreate: 1, validate: 1 }); - }); - - it('keeps the unscoped API equivalent to asSystem({ hooks: false })', async () => { - const unscoped = setup(); - const scoped = setup(); - const filter = { field: 'id', operator: 'eq', value: 1 } as any; - const aggregations = { total: { fn: 'count', field: 'id' } } as any; - - const unscopedResults = [ - await unscoped.resource.get(filter), - await unscoped.resource.list(filter), - await unscoped.resource.count(filter), - await unscoped.resource.aggregate(filter, aggregations), - await unscoped.resource.create({ name: 'John' }), - await unscoped.resource.update(1, { name: 'Jane' }), - await unscoped.resource.delete(1), - ]; - const hooksFreeSystem = scoped.resource.asSystem({ hooks: false }); - const scopedResults = [ - await hooksFreeSystem.get(filter), - await hooksFreeSystem.list(filter), - await hooksFreeSystem.count(filter), - await hooksFreeSystem.aggregate(filter, aggregations), - await hooksFreeSystem.create({ name: 'John' }), - await hooksFreeSystem.update(1, { name: 'Jane' }), - await hooksFreeSystem.delete(1), - ]; - - expect(unscopedResults).toEqual(scopedResults); - expect(unscoped.calls).toEqual(scoped.calls); + expect(calls).toMatchObject({ acl: 0, createExecutor: 0, connectorCreate: 1 }); }); - it('allows system hooks to update editReadonly fields while asUser rejects them', async () => { + it('rejects editReadonly for asUser()', async () => { const { calls, resource } = setup(); const forbidden = await resource .asUser({} as any, { meta: { allowed: true } }) .update(1, { readonly: 'new' }); - expect(forbidden.error).toContain('editReadonly is true'); - const updated = await resource.asSystem().update(1, { readonly: 'new' }); - expect(updated).toMatchObject({ ok: true, error: null }); - expect(calls).toMatchObject({ acl: 1, updateExecutor: 1, connectorUpdate: 0 }); + expect(forbidden.error).toContain('editReadonly is true'); + expect(calls).toMatchObject({ acl: 1, updateExecutor: 0, connectorUpdate: 0 }); }); it('applies read ACL, column access, and hooks for asUser()', async () => { @@ -213,17 +173,17 @@ describe('OperationalResource access scopes', () => { expect(calls).toMatchObject({ acl: 2, beforeList: 1, afterList: 1 }); }); - it('keeps system reads unrestricted while honoring the hooks option', async () => { - const withHooks = setup(); - const withoutHooks = setup(); + it('returns backendOnly columns on the bare API and strips them for asUser()', async () => { + const bare = setup(); + const scoped = setup(); - const systemRecords = await withHooks.resource.asSystem().list([]); - const hooksFreeRecords = await withoutHooks.resource.asSystem({ hooks: false }).list([]); + const bareRecords = await bare.resource.list([]); + const userRecords = await scoped.resource.asUser({} as any, { meta: { allowed: true } }).list([]); - expect(systemRecords).toEqual([{ id: 1, name: 'John', private: 'hidden' }]); - expect(hooksFreeRecords).toEqual(systemRecords); - expect(withHooks.calls).toMatchObject({ acl: 0, beforeList: 1, afterList: 1 }); - expect(withoutHooks.calls).toMatchObject({ acl: 0, beforeList: 0, afterList: 0 }); + expect(bareRecords).toEqual([{ id: 1, name: 'John', private: 'hidden' }]); + expect(userRecords).toEqual([{ id: 1, name: 'John' }]); + expect(bare.calls).toMatchObject({ acl: 0, beforeList: 0, afterList: 0 }); + expect(scoped.calls).toMatchObject({ acl: 1, beforeList: 1, afterList: 1 }); }); it('rejects denied deletes and leaves the connector untouched', async () => { @@ -274,16 +234,6 @@ describe('OperationalResource access scopes', () => { expect(calls).toMatchObject({ connectorAggregate: 1 }); }); - it('still delegates unscoped calls per resource, warning about each one separately', async () => { - const first = setup('warn-probe-a'); - const second = setup('warn-probe-b'); - - expect(await first.resource.list([])).toEqual([{ id: 1, name: 'John', private: 'hidden' }]); - expect(await second.resource.count([])).toBe(1); - expect(first.calls).toMatchObject({ acl: 0, connectorGetData: 1 }); - expect(second.calls).toMatchObject({ acl: 0, connectorCount: 1 }); - }); - it('row-scopes aggregate and count through the same read hooks as list', async () => { const { calls, seenFilters, resource } = setup(); const scoped = resource.asUser({} as any, { meta: { allowed: true } }); @@ -298,15 +248,14 @@ describe('OperationalResource access scopes', () => { expect(calls).toMatchObject({ beforeList: 2, connectorAggregate: 1, connectorCount: 1 }); }); - it('does not row-scope reads for a trusted system scope', async () => { + it('does not row-scope reads on the bare API', async () => { const { calls, seenFilters, resource } = setup(); - const hooksFree = resource.asSystem({ hooks: false }); - - await hooksFree.aggregate([], { total: { fn: 'count' } } as any); - await hooksFree.count([]); + await resource.aggregate([], { total: { fn: 'count' } } as any); + await resource.count([]); expect(seenFilters.aggregate).toEqual([]); expect(seenFilters.count).toEqual([]); expect(calls).toMatchObject({ beforeList: 0 }); }); + }); From a5fc573877451a7436672ea8b8bbad76af7d8155 Mon Sep 17 00:00:00 2001 From: Maksym Pipkun Date: Thu, 17 Sep 2026 12:15:02 +0300 Subject: [PATCH 04/11] fix: block hidden columns in scoped read filters --- adminforth/modules/userScopedResource.ts | 25 +++++++++++++++++++ .../operational_resource_scope.test.ts | 12 +++++++++ 2 files changed, 37 insertions(+) diff --git a/adminforth/modules/userScopedResource.ts b/adminforth/modules/userScopedResource.ts index 220448197..d7698fbab 100644 --- a/adminforth/modules/userScopedResource.ts +++ b/adminforth/modules/userScopedResource.ts @@ -19,6 +19,7 @@ import type { import { ActionCheckSource, AllowedActionsEnum, type AdminUser } from '../types/Common.js'; import { columnsAggregatableError, + filterColumnsReadableError, recordWriteError, stripReadForbiddenColumns, type ColumnAccessContext, @@ -153,6 +154,14 @@ export default class UserScopedResource implements IScopedOperationalResource { throw new Error(accessError); } + const filterError = await filterColumnsReadableError( + this.columnCtx(ActionCheckSource.ShowRequest), + filter, + ); + if (filterError) { + throw new Error(filterError); + } + const query = { filters: filter, limit: 1, offset: 0, sort: [] }; await this.runReadHooks('show', 'beforeDatasourceRequest', query); const record = await this.data.get(query.filters); @@ -177,6 +186,14 @@ export default class UserScopedResource implements IScopedOperationalResource { throw new Error(accessError); } + const filterError = await filterColumnsReadableError( + this.columnCtx(ActionCheckSource.ListRequest), + filter, + ); + if (filterError) { + throw new Error(filterError); + } + const query = { filters: filter, limit, offset, sort }; await this.runReadHooks('list', 'beforeDatasourceRequest', query); const data = await this.data.list(query.filters, query.limit, query.offset, query.sort, columns); @@ -225,6 +242,14 @@ export default class UserScopedResource implements IScopedOperationalResource { throw new Error(accessError); } + const filterError = await filterColumnsReadableError( + this.columnCtx(ActionCheckSource.ListRequest), + filter, + ); + if (filterError) { + throw new Error(filterError); + } + // a count is a list the caller only learns the size of, so it is row-scoped the same way const query = { filters: filter, limit: null, offset: 0, sort: [] }; await this.runReadHooks('list', 'beforeDatasourceRequest', query); diff --git a/tests/jest_tests/operational_resource_scope.test.ts b/tests/jest_tests/operational_resource_scope.test.ts index 89efa6bdc..8f5a6f3e5 100644 --- a/tests/jest_tests/operational_resource_scope.test.ts +++ b/tests/jest_tests/operational_resource_scope.test.ts @@ -234,6 +234,18 @@ describe('OperationalResource access tiers', () => { expect(calls).toMatchObject({ connectorAggregate: 1 }); }); + it('refuses to filter backendOnly columns through user-scoped reads', async () => { + const { calls, resource } = setup(); + const scoped = resource.asUser({} as any, { meta: { allowed: true } }); + const privateFilter = { field: 'private', operator: 'eq', value: 'hidden' } as any; + + await expect(scoped.get(privateFilter)).rejects.toThrow('Filter: column "private" cannot be used'); + await expect(scoped.list(privateFilter)).rejects.toThrow('Filter: column "private" cannot be used'); + await expect(scoped.count(privateFilter)).rejects.toThrow('Filter: column "private" cannot be used'); + + expect(calls).toMatchObject({ connectorGetData: 0, connectorCount: 0, beforeList: 0 }); + }); + it('row-scopes aggregate and count through the same read hooks as list', async () => { const { calls, seenFilters, resource } = setup(); const scoped = resource.asUser({} as any, { meta: { allowed: true } }); From 8bbca8a03fb2dac594e87e9099cf86a009957b02 Mon Sep 17 00:00:00 2001 From: Maksym Pipkun Date: Thu, 17 Sep 2026 12:17:19 +0300 Subject: [PATCH 05/11] fix: scope user resource mutations --- adminforth/modules/userScopedResource.ts | 23 +++++++++++++--- .../operational_resource_scope.test.ts | 27 +++++++++++++++++-- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/adminforth/modules/userScopedResource.ts b/adminforth/modules/userScopedResource.ts index d7698fbab..8a1d3b635 100644 --- a/adminforth/modules/userScopedResource.ts +++ b/adminforth/modules/userScopedResource.ts @@ -16,6 +16,7 @@ import type { UpdateResourceRecordParams, UpdateResourceRecordResult, } from '../types/Back.js'; +import { Filters } from '../types/Back.js'; import { ActionCheckSource, AllowedActionsEnum, type AdminUser } from '../types/Common.js'; import { columnsAggregatableError, @@ -148,6 +149,22 @@ export default class UserScopedResource implements IScopedOperationalResource { } } + /** + * Finds a record through the list scope before a mutation. A primary-key connector lookup + * would bypass tenant filters installed by `beforeDatasourceRequest` hooks. + */ + private async findScopedRecord(primaryKey: any): Promise { + const primaryKeyColumn = this.resourceConfig.columns.find((column) => column.primaryKey); + const query = { + filters: [Filters.EQ(primaryKeyColumn.name, primaryKey)], + limit: 1, + offset: 0, + sort: [], + }; + await this.runReadHooks('list', 'beforeDatasourceRequest', query); + return this.data.get(query.filters); + } + async get(filter: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array): Promise { const accessError = await this.accessError('get'); if (accessError) { @@ -287,8 +304,7 @@ export default class UserScopedResource implements IScopedOperationalResource { return { ok: true }; } - const oldRecord = this.options.oldRecord - ?? await this.dataConnector.getRecordByPrimaryKey(this.resourceConfig, primaryKey); + const oldRecord = await this.findScopedRecord(primaryKey); if (!oldRecord) { const primaryKeyColumn = this.resourceConfig.columns.find((column) => column.primaryKey); return { ok: false, error: `Record with ${primaryKeyColumn.name} ${primaryKey} not found` }; @@ -323,8 +339,7 @@ export default class UserScopedResource implements IScopedOperationalResource { } async delete(primaryKey: any): Promise { - const record = this.options.record - ?? await this.dataConnector.getRecordByPrimaryKey(this.resourceConfig, primaryKey); + const record = await this.findScopedRecord(primaryKey); if (!record) { return false; } diff --git a/tests/jest_tests/operational_resource_scope.test.ts b/tests/jest_tests/operational_resource_scope.test.ts index 8f5a6f3e5..a0be2875d 100644 --- a/tests/jest_tests/operational_resource_scope.test.ts +++ b/tests/jest_tests/operational_resource_scope.test.ts @@ -83,8 +83,12 @@ function setup(resourceId = 'users') { calls.connectorDelete++; return true; }, - getData: async () => { + getData: async ({ filters }) => { calls.connectorGetData++; + seenFilters.getData = filters; + if (Array.isArray(filters) && filters.some((filter) => filter.field === 'tenant' && filter.value === 'not-owned')) { + return { data: [], total: 0 }; + } return { data: [{ id: 1, name: 'John', private: 'hidden' }], total: 1 }; }, getCount: async ({ filters }) => { @@ -202,7 +206,26 @@ describe('OperationalResource access tiers', () => { .update(1, { name: 'Jane' }); expect(updated).toMatchObject({ ok: true }); - expect(calls).toMatchObject({ connectorGetByPk: 0, updateExecutor: 1 }); + expect(calls).toMatchObject({ connectorGetByPk: 0, updateExecutor: 1, beforeList: 1 }); + }); + + it('row-scopes updates and deletes before loading the target record', async () => { + const { calls, seenFilters, resource } = setup(); + resource.resourceConfig.hooks.list.beforeDatasourceRequest = [async ({ query }) => { + calls.beforeList++; + query.filtersTools.replaceOrAddTopFilter({ field: 'tenant', operator: 'eq', value: 'not-owned' }); + return { ok: true }; + }]; + const scoped = resource.asUser({} as any, { meta: { allowed: true } }); + + await expect(scoped.update(1, { name: 'Jane' })).resolves.toMatchObject({ + ok: false, + error: expect.stringContaining('not found'), + }); + await expect(scoped.delete(1)).resolves.toBe(false); + + expect(seenFilters.getData).toContainEqual({ field: 'tenant', operator: 'eq', value: 'not-owned' }); + expect(calls).toMatchObject({ beforeList: 2, updateExecutor: 0, connectorDelete: 0 }); }); it('requires list and show access for asUser() aggregations', async () => { From 0a4ba8a24c7423edb9ed50c380a45794a76f8c79 Mon Sep 17 00:00:00 2001 From: Maksym Pipkun Date: Thu, 17 Sep 2026 12:18:32 +0300 Subject: [PATCH 06/11] fix: preserve scoped mutation and bulk delete contracts --- adminforth/modules/configValidator.ts | 32 +++++++++++++++---- adminforth/modules/userScopedResource.ts | 4 --- .../operational_resource_scope.test.ts | 14 ++++++++ 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/adminforth/modules/configValidator.ts b/adminforth/modules/configValidator.ts index 5652a9064..4aa707b6c 100644 --- a/adminforth/modules/configValidator.ts +++ b/adminforth/modules/configValidator.ts @@ -20,7 +20,7 @@ import { compositePkValues, isCompositePrimaryKey } from './recordId.js'; import fs from 'fs'; import path from 'path'; -import { guessLabelFromName, md5hash, RateLimiter, suggestIfTypo, slugifyString } from './utils.js'; +import { cascadeChildrenDelete, guessLabelFromName, md5hash, RateLimiter, suggestIfTypo, slugifyString } from './utils.js'; import { AdminForthSortDirections, type AdminForthComponentDeclarationFull, @@ -261,17 +261,35 @@ export default class ConfigValidator implements IConfigValidator { dangerous: true, allowed: async ({ resource, adminUser, allowedActions }) => { return allowedActions.delete }, action: async ({ selectedIds, adminUser, response }) => { - // one path for deletion: asUser() checks the permission per record, cascades to children - // and runs the delete hooks, so this action does not carry its own copy of any of that + // The bulk action's `allowed` callback is its ACL boundary. Keep that action-level + // contract instead of introducing a second, per-record `asUser()` permission check. let error = null; + const connector = this.adminforth.connectors[res.dataSource]; await Promise.all( selectedIds.map(async (recordId) => { try { - await this.adminforth - .resource(res.resourceId) - .asUser(adminUser, { response }) - .delete(recordId); + const record = await connector.getRecordByPrimaryKey(res as AdminForthResource, recordId); + const cascadeResult = await cascadeChildrenDelete( + res as AdminForthResource, + recordId, + { adminUser, response }, + this.adminforth, + (params) => this.adminforth.deleteResourceRecord(params), + ); + if (cascadeResult.error) { + throw new Error(cascadeResult.error); + } + const result = await this.adminforth.deleteResourceRecord({ + resource: res as AdminForthResource, + recordId, + record, + adminUser, + response, + }); + if (result.error) { + throw new Error(result.error); + } } catch (e) { if (!error) { error = (e as Error).message; diff --git a/adminforth/modules/userScopedResource.ts b/adminforth/modules/userScopedResource.ts index 8a1d3b635..436c726e0 100644 --- a/adminforth/modules/userScopedResource.ts +++ b/adminforth/modules/userScopedResource.ts @@ -300,10 +300,6 @@ export default class UserScopedResource implements IScopedOperationalResource { } async update(primaryKey: any, record: any): Promise { - if (Object.keys(record).length === 0) { - return { ok: true }; - } - const oldRecord = await this.findScopedRecord(primaryKey); if (!oldRecord) { const primaryKeyColumn = this.resourceConfig.columns.find((column) => column.primaryKey); diff --git a/tests/jest_tests/operational_resource_scope.test.ts b/tests/jest_tests/operational_resource_scope.test.ts index a0be2875d..3be6aaf84 100644 --- a/tests/jest_tests/operational_resource_scope.test.ts +++ b/tests/jest_tests/operational_resource_scope.test.ts @@ -228,6 +228,20 @@ describe('OperationalResource access tiers', () => { expect(calls).toMatchObject({ beforeList: 2, updateExecutor: 0, connectorDelete: 0 }); }); + it('applies the full user-scoped update path to an empty update', async () => { + const denied = setup(); + const deniedResult = await denied.resource.asUser({} as any, { meta: { allowed: false } }).update(1, {}); + + expect(deniedResult).toMatchObject({ ok: false, error: 'Action is not allowed' }); + expect(denied.calls).toMatchObject({ beforeList: 1, updateExecutor: 0 }); + + const allowed = setup(); + const allowedResult = await allowed.resource.asUser({} as any, { meta: { allowed: true } }).update(1, {}); + + expect(allowedResult).toMatchObject({ ok: true }); + expect(allowed.calls).toMatchObject({ beforeList: 1, updateExecutor: 1 }); + }); + it('requires list and show access for asUser() aggregations', async () => { const { calls, resource } = setup(); From badc630f183f8587d972bbd24a7a2e6094253fb4 Mon Sep 17 00:00:00 2001 From: Maksym Pipkun Date: Thu, 17 Sep 2026 12:19:39 +0300 Subject: [PATCH 07/11] test: use aggregation operation field --- .../jest_tests/operational_resource_scope.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/jest_tests/operational_resource_scope.test.ts b/tests/jest_tests/operational_resource_scope.test.ts index 3be6aaf84..f88826642 100644 --- a/tests/jest_tests/operational_resource_scope.test.ts +++ b/tests/jest_tests/operational_resource_scope.test.ts @@ -246,7 +246,7 @@ describe('OperationalResource access tiers', () => { const { calls, resource } = setup(); await expect( - resource.asUser({} as any, { meta: { allowed: false } }).aggregate([], { total: { fn: 'count' } } as any), + resource.asUser({} as any, { meta: { allowed: false } }).aggregate([], { total: { operation: 'count' } } as any), ).rejects.toThrow('Action is not allowed'); expect(calls).toMatchObject({ connectorAggregate: 0 }); }); @@ -255,18 +255,18 @@ describe('OperationalResource access tiers', () => { const { calls, resource } = setup(); const scoped = resource.asUser({} as any, { meta: { allowed: true } }); - await expect(scoped.aggregate([], { max: { fn: 'max', field: 'private' } } as any)) + await expect(scoped.aggregate([], { max: { operation: 'max', field: 'private' } } as any)) .rejects.toThrow('cannot be aggregated (backendOnly is true)'); - await expect(scoped.aggregate([], { total: { fn: 'count' } } as any, { field: 'private' } as any)) + await expect(scoped.aggregate([], { total: { operation: 'count' } } as any, { field: 'private' } as any)) .rejects.toThrow('cannot be aggregated (backendOnly is true)'); await expect(scoped.aggregate( { field: 'private', operator: 'eq', value: 'hidden' } as any, - { total: { fn: 'count' } } as any, + { total: { operation: 'count' } } as any, )).rejects.toThrow('Filter: column "private" cannot be used'); expect(calls).toMatchObject({ connectorAggregate: 0 }); - const allowed = await scoped.aggregate([], { max: { fn: 'max', field: 'name' } } as any); + const allowed = await scoped.aggregate([], { max: { operation: 'max', field: 'name' } } as any); expect(allowed).toEqual([{ total: 1 }]); expect(calls).toMatchObject({ connectorAggregate: 1 }); }); @@ -288,7 +288,7 @@ describe('OperationalResource access tiers', () => { const scoped = resource.asUser({} as any, { meta: { allowed: true } }); const tenantFilter = { field: 'tenant', operator: 'eq', value: 't1' }; - await scoped.aggregate([], { total: { fn: 'count' } } as any, { field: 'name' } as any); + await scoped.aggregate([], { total: { operation: 'count' } } as any, { field: 'name' } as any); await scoped.count([]); // without this an aggregation reports across every tenant's rows @@ -299,7 +299,7 @@ describe('OperationalResource access tiers', () => { it('does not row-scope reads on the bare API', async () => { const { calls, seenFilters, resource } = setup(); - await resource.aggregate([], { total: { fn: 'count' } } as any); + await resource.aggregate([], { total: { operation: 'count' } } as any); await resource.count([]); expect(seenFilters.aggregate).toEqual([]); From 5445ada98d8f51d9b6e228a91406582b14d6f883 Mon Sep 17 00:00:00 2001 From: Maksym Pipkun Date: Fri, 18 Sep 2026 16:05:31 +0300 Subject: [PATCH 08/11] fix: preserve security and compatibility in scoped resource operations --- .../tutorial/03-Customization/11-dataApi.md | 5 +- adminforth/index.ts | 25 ++- adminforth/modules/configValidator.ts | 14 +- adminforth/modules/polymorphicReferences.ts | 49 +++++ adminforth/modules/resourceAccess.ts | 55 +++++ adminforth/modules/restApi.ts | 158 ++++----------- adminforth/modules/userScopedResource.ts | 55 ++--- adminforth/types/Back.ts | 12 +- .../operational_resource_scope.test.ts | 188 ++++++++++++++++-- .../resource_bulk_delete_order.test.ts | 100 ++++++++++ tests/jest_tests/rest_resource_delete.test.ts | 27 +++ 11 files changed, 502 insertions(+), 186 deletions(-) create mode 100644 adminforth/modules/polymorphicReferences.ts create mode 100644 tests/jest_tests/resource_bulk_delete_order.test.ts create mode 100644 tests/jest_tests/rest_resource_delete.test.ts diff --git a/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md b/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md index c62368abf..639bbfadc 100644 --- a/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md +++ b/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md @@ -83,8 +83,9 @@ try { } ``` -When the caller has already loaded the record, pass it in so the call does not -read it a second time and hooks see the same snapshot the caller worked from: +When the caller has already loaded the record, pass its snapshot to the save +hooks. ACL and row scope use the current record from a scoped lookup before +mutating it: ```ts await users.asUser(adminUser, { meta, oldRecord }).update(recordId, updates); diff --git a/adminforth/index.ts b/adminforth/index.ts index 4038d0e01..efd297f09 100644 --- a/adminforth/index.ts +++ b/adminforth/index.ts @@ -4,7 +4,7 @@ import CodeInjector from './modules/codeInjector.js'; import ExpressServer from './servers/express.js'; import OpenApiRegistry from './servers/openapi.js'; // import FastifyServer from './servers/fastify.js'; -import { ADMINFORTH_VERSION, listify, suggestIfTypo, RateLimiter, RAMLock, getClientIp, isProbablyUUIDColumn, convertPeriodToSeconds, hookResponseError, md5hash, applyRegexValidation, formatHugePluginError } from './modules/utils.js'; +import { ADMINFORTH_VERSION, cascadeChildrenDelete, listify, suggestIfTypo, RateLimiter, RAMLock, getClientIp, isProbablyUUIDColumn, convertPeriodToSeconds, hookResponseError, md5hash, applyRegexValidation, formatHugePluginError } from './modules/utils.js'; import { type AdminForthConfig, type IAdminForth, @@ -624,7 +624,7 @@ class AdminForth implements IAdminForth { { create: (params) => this.executeCreateResourceRecord(params), update: (params) => this.executeUpdateResourceRecord(params), - delete: (params) => this.executeDeleteResourceRecord(params), + delete: (params, cascadeChildren) => this.executeDeleteResourceRecord(params, cascadeChildren), }, adminUser, options, @@ -956,13 +956,17 @@ class AdminForth implements IAdminForth { */ async deleteResourceRecord( params: DeleteResourceRecordParams, + cascadeChildren = false, ): Promise { - this.warnDeprecatedResourceMutation('deleteResourceRecord', params.resource.resourceId, 'delete'); - return this.executeDeleteResourceRecord(params); + if (!cascadeChildren) { + this.warnDeprecatedResourceMutation('deleteResourceRecord', params.resource.resourceId, 'delete'); + } + return this.executeDeleteResourceRecord(params, cascadeChildren); } private async executeDeleteResourceRecord( params: DeleteResourceRecordParams, + cascadeChildren = false, ): Promise { const { resource, recordId, adminUser, record, response, extra } = params; // execute hook if needed @@ -982,6 +986,19 @@ class AdminForth implements IAdminForth { } } + if (cascadeChildren) { + const cascadeResult = await cascadeChildrenDelete( + resource, + recordId, + { adminUser, response }, + this, + (childParams) => this.executeDeleteResourceRecord(childParams), + ); + if (cascadeResult.error) { + return cascadeResult; + } + } + const connector = this.connectors[resource.dataSource]; await connector.deleteRecord({ resource, recordId, pkValues: compositePkValues(connector, resource, recordId) }); diff --git a/adminforth/modules/configValidator.ts b/adminforth/modules/configValidator.ts index 4aa707b6c..7efebcf90 100644 --- a/adminforth/modules/configValidator.ts +++ b/adminforth/modules/configValidator.ts @@ -20,7 +20,7 @@ import { compositePkValues, isCompositePrimaryKey } from './recordId.js'; import fs from 'fs'; import path from 'path'; -import { cascadeChildrenDelete, guessLabelFromName, md5hash, RateLimiter, suggestIfTypo, slugifyString } from './utils.js'; +import { guessLabelFromName, md5hash, RateLimiter, suggestIfTypo, slugifyString } from './utils.js'; import { AdminForthSortDirections, type AdminForthComponentDeclarationFull, @@ -270,23 +270,13 @@ export default class ConfigValidator implements IConfigValidator { selectedIds.map(async (recordId) => { try { const record = await connector.getRecordByPrimaryKey(res as AdminForthResource, recordId); - const cascadeResult = await cascadeChildrenDelete( - res as AdminForthResource, - recordId, - { adminUser, response }, - this.adminforth, - (params) => this.adminforth.deleteResourceRecord(params), - ); - if (cascadeResult.error) { - throw new Error(cascadeResult.error); - } const result = await this.adminforth.deleteResourceRecord({ resource: res as AdminForthResource, recordId, record, adminUser, response, - }); + }, true); if (result.error) { throw new Error(result.error); } diff --git a/adminforth/modules/polymorphicReferences.ts b/adminforth/modules/polymorphicReferences.ts new file mode 100644 index 000000000..b30d9231a --- /dev/null +++ b/adminforth/modules/polymorphicReferences.ts @@ -0,0 +1,49 @@ +import type { AdminForthResource, IAdminForth } from '../types/Back.js'; +import { Filters } from '../types/Back.js'; + +/** Fill discriminator columns only after user-provided columns have passed access checks. */ +export async function resolvePolymorphicReferences( + resource: AdminForthResource, + record: Record, + adminforth: IAdminForth, + oldRecord?: Record, +): Promise { + for (const column of resource.columns) { + const foreignResource = column.foreignResource; + if (!foreignResource?.polymorphicOn || !(column.name in record)) { + continue; + } + + let discriminator: string; + if (record[column.name] === null) { + record[foreignResource.polymorphicOn] = foreignResource.polymorphicResources.find((target) => target.resourceId === null).whenValue; + continue; + } + if (record[column.name]) { + for (const target of foreignResource.polymorphicResources) { + if (target.resourceId === null) { + continue; + } + const targetResource = adminforth.config.resources.find((candidate) => candidate.resourceId === target.resourceId); + const targetPrimaryKey = targetResource.columns.find((candidate) => candidate.primaryKey).name; + const { data } = await adminforth.connectors[targetResource.dataSource].getData({ + resource: targetResource, + limit: 1, + offset: 0, + filters: Filters.AND(Filters.EQ(targetPrimaryKey, record[column.name])), + sort: [], + }); + if (data.length) { + discriminator = target.whenValue; + break; + } + } + } else { + continue; + } + + if (!oldRecord || oldRecord[foreignResource.polymorphicOn] !== discriminator) { + record[foreignResource.polymorphicOn] = discriminator; + } + } +} diff --git a/adminforth/modules/resourceAccess.ts b/adminforth/modules/resourceAccess.ts index 97b109575..e56218506 100644 --- a/adminforth/modules/resourceAccess.ts +++ b/adminforth/modules/resourceAccess.ts @@ -45,3 +45,58 @@ export async function interpretResource( return { allowedActions }; } + +export const RESOURCE_ACCESS_GRANT = Symbol('resourceAccessGrant'); + +type ResourceAccessGrant = object; + +const grantedOperations = new WeakMap(); + +/** The REST preflight checks access before returning field or existence errors. */ +export async function authorizeResourceOperation( + adminUser: AdminUser, + resource: AdminForthResource, + meta: any, + source: ActionCheckSource, + action: AllowedActionsEnum, + adminforth: IAdminForth, + record: any, + primaryKey?: any, +): Promise<{ error: string | null; grant?: ResourceAccessGrant }> { + const { allowedActions } = await interpretResource(adminUser, resource, meta, source, adminforth); + const allowed = allowedActions[action] as boolean | string | undefined; + if (allowed !== true) { + return { error: typeof allowed === 'string' ? allowed : 'Action is not allowed' }; + } + + const grant = {}; + grantedOperations.set(grant, { adminUser, resource, action, record, primaryKey }); + return { error: null, grant }; +} + +/** A grant can only skip the matching scoped ACL check once; row scope still runs normally. */ +export function consumeResourceAccessGrant( + grant: ResourceAccessGrant | undefined, + adminUser: AdminUser, + resource: AdminForthResource, + action: AllowedActionsEnum, + record: any, + primaryKey?: any, +): boolean { + if (!grant) { + return false; + } + const granted = grantedOperations.get(grant); + grantedOperations.delete(grant); + return granted?.adminUser === adminUser + && granted.resource === resource + && granted.action === action + && granted.record === record + && granted.primaryKey === primaryKey; +} diff --git a/adminforth/modules/restApi.ts b/adminforth/modules/restApi.ts index d71f08092..02e28fd8d 100644 --- a/adminforth/modules/restApi.ts +++ b/adminforth/modules/restApi.ts @@ -36,7 +36,7 @@ import { isShown, stripReadForbiddenColumns, } from './columnAccess.js'; -import { interpretResource } from './resourceAccess.js'; +import { authorizeResourceOperation, interpretResource, RESOURCE_ACCESS_GRANT } from './resourceAccess.js'; function stripResourceColumnFrontendMeta(column: Record) { const { default: _default, _baseTypeDebug, ...sanitizedColumn } = column; @@ -1997,16 +1997,14 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { if (!resource) { return { error: `Resource '${body['resourceId']}' not found` }; } - // access is checked again inside the scoped create below, but it has to be answered - // before the handler reveals anything about existing records or required columns - const { allowedActions: createAllowedActions } = await interpretResource( - adminUser, resource, { requestBody: body }, ActionCheckSource.CreateRequest, this.adminforth + // Check before revealing existing records or required columns. The scoped write + // consumes this one-use grant, so the ACL callback is not invoked twice. + const createAccess = await authorizeResourceOperation( + adminUser, resource, { requestBody: body }, ActionCheckSource.CreateRequest, + AllowedActionsEnum.create, this.adminforth, body.record, ); - const { allowed: createAllowed, error: createNotAllowedError } = checkAccess( - AllowedActionsEnum.create, createAllowedActions - ); - if (!createAllowed) { - return { error: createNotAllowedError }; + if (createAccess.error) { + return { error: createAccess.error }; } const { record, requiredColumnsToSkip } = body; @@ -2069,61 +2067,21 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { } } - // for polymorphic foreign resources, we need to find out the value for polymorphicOn column - for (const column of resource.columns) { - if (column.foreignResource?.polymorphicOn && record[column.name] === null) { - const systemResource = column.foreignResource.polymorphicResources.find(pr => pr.resourceId === null); - record[column.foreignResource.polymorphicOn] = systemResource.whenValue; - } else if (column.foreignResource?.polymorphicOn && record[column.name]) { - const targetResources = {}; - const targetConnectors = {}; - const targetResourcePkFields = {}; - column.foreignResource.polymorphicResources.forEach((pr) => { - if (pr.resourceId === null) { - return; - } - const targetResource = this.adminforth.config.resources.find((res) => res.resourceId == pr.resourceId); - if (!targetResource) { - return; - } - targetResources[pr.whenValue] = targetResource; - targetConnectors[pr.whenValue] = this.adminforth.connectors[targetResources[pr.whenValue].dataSource]; - targetResourcePkFields[pr.whenValue] = targetResources[pr.whenValue].columns.find((col) => col.primaryKey).name; - }); - const targetData = (await Promise.all(Object.keys(targetResources).map((polymorphicOnValue) => - targetConnectors[polymorphicOnValue].getData({ - resource: targetResources[polymorphicOnValue], - limit: 1, - offset: 0, - filters: { operator: AdminForthFilterOperators.AND, subFilters: [ - { - field: targetResourcePkFields[polymorphicOnValue], - operator: AdminForthFilterOperators.EQ, - value: record[column.name], - } - ]}, - sort: [], - }) - ))).reduce((acc: any, td: any, tdi) => ({ - ...acc, - [Object.keys(targetResources)[tdi]]: td.data, - }), {}); - record[column.foreignResource.polymorphicOn] = Object.keys(targetData).find((tdk) => targetData[tdk].length); - } - } const jsonError = this.normalizeJsonColumns(resource, record); if (jsonError) { return { error: jsonError, ok: false }; } + const scopedCreateOptions = { + meta: ctxCreate.meta, + response, + extra: { body, query, headers, cookies, requestUrl, response }, + [RESOURCE_ACCESS_GRANT]: createAccess.grant, + }; const createRecordResponse = await this.adminforth .resource(resource.resourceId) - .asUser(adminUser, { - meta: ctxCreate.meta, - response, - extra: { body, query, headers, cookies, requestUrl, response }, - }) + .asUser(adminUser, scopedCreateOptions) .create(record); if (createRecordResponse.error) { return { @@ -2170,20 +2128,19 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { } const record = body['record']; - // access is checked again inside the scoped update below, but it has to be answered - // before the handler reveals whether another record with the same key exists - const { allowedActions: editAllowedActions } = await interpretResource( + // Check before revealing whether another record has the requested key. + const editAccess = await authorizeResourceOperation( adminUser, resource, { requestBody: body, newRecord: record, oldRecord, pk: recordId }, ActionCheckSource.EditRequest, - this.adminforth - ); - const { allowed: editAllowed, error: editNotAllowedError } = checkAccess( - AllowedActionsEnum.edit, editAllowedActions + AllowedActionsEnum.edit, + this.adminforth, + record, + recordId, ); - if (!editAllowed) { - return { error: editNotAllowedError }; + if (editAccess.error) { + return { error: editAccess.error }; } if (isCompositePrimaryKey(resource)) { @@ -2217,69 +2174,21 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { } } - // for polymorphic foreign resources, we need to find out the value for polymorphicOn column - for (const column of resource.columns) { - if (column.foreignResource?.polymorphicOn && record[column.name] === null) { - const systemResource = column.foreignResource.polymorphicResources.find(pr => pr.resourceId === null); - record[column.foreignResource.polymorphicOn] = systemResource.whenValue; - } else if (column.foreignResource?.polymorphicOn && record[column.name]) { - let newPolymorphicOnValue = null; - if (record[column.name]) { - const targetResources = {}; - const targetConnectors = {}; - const targetResourcePkFields = {}; - column.foreignResource.polymorphicResources.forEach((pr) => { - if (pr.resourceId === null) { - return; - } - const targetResource = this.adminforth.config.resources.find((res) => res.resourceId == pr.resourceId); - if (!targetResource) { - return; - } - targetResources[pr.whenValue] = targetResource; - targetConnectors[pr.whenValue] = this.adminforth.connectors[targetResources[pr.whenValue].dataSource]; - targetResourcePkFields[pr.whenValue] = targetResources[pr.whenValue].columns.find((col) => col.primaryKey).name; - }); - const targetData = (await Promise.all(Object.keys(targetResources).map((polymorphicOnValue) => - targetConnectors[polymorphicOnValue].getData({ - resource: targetResources[polymorphicOnValue], - limit: 1, - offset: 0, - filters: { operator: AdminForthFilterOperators.AND, subFilters: [ - { - field: targetResourcePkFields[polymorphicOnValue], - operator: AdminForthFilterOperators.EQ, - value: record[column.name], - } - ]}, - sort: [], - }) - ))).reduce((acc: any, td: any, tdi) => ({ - ...acc, - [Object.keys(targetResources)[tdi]]: td.data, - }), {}); - newPolymorphicOnValue = Object.keys(targetData).find((tdk) => targetData[tdk].length); - } - - if (oldRecord[column.foreignResource.polymorphicOn] !== newPolymorphicOnValue) { - record[column.foreignResource.polymorphicOn] = newPolymorphicOnValue; - } - } - } - const jsonError = this.normalizeJsonColumns(resource, record); if (jsonError) { return { error: jsonError, ok: false }; } + const scopedEditOptions = { + meta: { requestBody: body, newRecord: record, oldRecord, pk: recordId }, + oldRecord, + response, + extra: { body, query, headers, cookies, requestUrl, response }, + [RESOURCE_ACCESS_GRANT]: editAccess.grant, + }; const { error } = await this.adminforth .resource(resource.resourceId) - .asUser(adminUser, { - meta: { requestBody: body, newRecord: record, oldRecord, pk: recordId }, - oldRecord, - response, - extra: { body, query, headers, cookies, requestUrl, response }, - }) + .asUser(adminUser, scopedEditOptions) .update(recordId, record); if (error) { return { error }; @@ -2312,7 +2221,7 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { } try { - await this.adminforth + const deleted = await this.adminforth .resource(resource.resourceId) .asUser(adminUser, { meta: { requestBody: body, record }, @@ -2321,6 +2230,9 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { extra: { body, query, headers, cookies, requestUrl, response }, }) .delete(body.primaryKey); + if (!deleted) { + return { error: `Record with ${body.primaryKey} not found` }; + } } catch (error) { return { error: (error as Error).message }; } diff --git a/adminforth/modules/userScopedResource.ts b/adminforth/modules/userScopedResource.ts index 436c726e0..5796c6fbb 100644 --- a/adminforth/modules/userScopedResource.ts +++ b/adminforth/modules/userScopedResource.ts @@ -25,9 +25,10 @@ import { stripReadForbiddenColumns, type ColumnAccessContext, } from './columnAccess.js'; -import { interpretResource } from './resourceAccess.js'; +import { consumeResourceAccessGrant, interpretResource, RESOURCE_ACCESS_GRANT } from './resourceAccess.js'; import { filtersTools } from './filtersTools.js'; -import { cascadeChildrenDelete, hookResponseError, listify } from './utils.js'; +import { hookResponseError, listify } from './utils.js'; +import { resolvePolymorphicReferences } from './polymorphicReferences.js'; import type OperationalResource from './operationalResource.js'; /** @@ -50,7 +51,7 @@ type GuardedOperation = keyof typeof OPERATION_ACCESS; export interface ResourceHookExecutors { create(params: CreateResourceRecordParams): Promise; update(params: UpdateResourceRecordParams): Promise; - delete(params: DeleteResourceRecordParams): Promise; + delete(params: DeleteResourceRecordParams, cascadeChildren?: boolean): Promise; } @@ -67,7 +68,7 @@ export default class UserScopedResource implements IScopedOperationalResource { private readonly adminforth: IAdminForth, private readonly executors: ResourceHookExecutors, private readonly adminUser: AdminUser, - private readonly options: OperationalResourceUserOptions, + private readonly options: OperationalResourceUserOptions & { [RESOURCE_ACCESS_GRANT]?: object }, ) { this.dataConnector = data.dataConnector; this.resourceConfig = data.resourceConfig; @@ -162,7 +163,8 @@ export default class UserScopedResource implements IScopedOperationalResource { sort: [], }; await this.runReadHooks('list', 'beforeDatasourceRequest', query); - return this.data.get(query.filters); + const scopedFilters = this.dataConnector.validateAndNormalizeInputFilters(query.filters); + return this.data.get(Filters.AND(Filters.EQ(primaryKeyColumn.name, primaryKey), scopedFilters)); } async get(filter: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array): Promise { @@ -275,7 +277,10 @@ export default class UserScopedResource implements IScopedOperationalResource { } async create(recordValues: any): Promise { - const accessError = await this.accessError('create'); + const accessError = consumeResourceAccessGrant( + this.options[RESOURCE_ACCESS_GRANT], this.adminUser, this.resourceConfig, + AllowedActionsEnum.create, recordValues, + ) ? null : await this.accessError('create'); if (accessError) { return { ok: false, createdRecord: undefined, error: accessError }; } @@ -289,6 +294,8 @@ export default class UserScopedResource implements IScopedOperationalResource { return { ok: false, createdRecord: undefined, error: columnError }; } + await resolvePolymorphicReferences(this.resourceConfig, recordValues, this.adminforth); + const result = await this.executors.create({ resource: this.resourceConfig, record: recordValues, @@ -300,15 +307,17 @@ export default class UserScopedResource implements IScopedOperationalResource { } async update(primaryKey: any, record: any): Promise { - const oldRecord = await this.findScopedRecord(primaryKey); - if (!oldRecord) { + const scopedRecord = await this.findScopedRecord(primaryKey); + if (!scopedRecord) { const primaryKeyColumn = this.resourceConfig.columns.find((column) => column.primaryKey); return { ok: false, error: `Record with ${primaryKeyColumn.name} ${primaryKey} not found` }; } - - const meta = { ...this.meta, newRecord: record, oldRecord, pk: primaryKey }; - - const accessError = await this.accessError('update', meta); + const oldRecord = this.options.oldRecord ?? scopedRecord; + const meta = { ...this.meta, newRecord: record, oldRecord: scopedRecord, pk: primaryKey }; + const accessError = consumeResourceAccessGrant( + this.options[RESOURCE_ACCESS_GRANT], this.adminUser, this.resourceConfig, + AllowedActionsEnum.edit, record, primaryKey, + ) ? null : await this.accessError('update', meta); if (accessError) { return { ok: false, error: accessError }; } @@ -322,6 +331,8 @@ export default class UserScopedResource implements IScopedOperationalResource { return { ok: false, error: columnError }; } + await resolvePolymorphicReferences(this.resourceConfig, record, this.adminforth, scopedRecord); + const result = await this.executors.update({ resource: this.resourceConfig, recordId: primaryKey, @@ -335,27 +346,17 @@ export default class UserScopedResource implements IScopedOperationalResource { } async delete(primaryKey: any): Promise { - const record = await this.findScopedRecord(primaryKey); - if (!record) { + const scopedRecord = await this.findScopedRecord(primaryKey); + if (!scopedRecord) { return false; } + const record = this.options.record ?? scopedRecord; - const accessError = await this.accessError('delete', { ...this.meta, record, pk: primaryKey }); + const accessError = await this.accessError('delete', { ...this.meta, record: scopedRecord, pk: primaryKey }); if (accessError) { throw new Error(accessError); } - const { error: cascadeError } = await cascadeChildrenDelete( - this.resourceConfig, - primaryKey, - { adminUser: this.adminUser, response: this.options.response }, - this.adminforth, - (params) => this.executors.delete(params), - ); - if (cascadeError) { - throw new Error(cascadeError); - } - const { error } = await this.executors.delete({ resource: this.resourceConfig, recordId: primaryKey, @@ -363,7 +364,7 @@ export default class UserScopedResource implements IScopedOperationalResource { adminUser: this.adminUser, extra: this.options.extra, response: this.options.response, - }); + }, true); if (error) { throw new Error(error); } diff --git a/adminforth/types/Back.ts b/adminforth/types/Back.ts index b97c0287e..49a9ed2f3 100644 --- a/adminforth/types/Back.ts +++ b/adminforth/types/Back.ts @@ -631,9 +631,12 @@ export interface IAdminForth { /** * Deletes a record and runs the resource delete hooks, without checking permissions. * For an operation a user requested, use `resource(resourceId).asUser(adminUser, { meta })`. + * Pass `true` as the second argument to apply configured child deletion after the parent + * `beforeSave` hooks have allowed the deletion. */ deleteResourceRecord( params: DeleteResourceRecordParams, + cascadeChildren?: boolean, ): Promise; auth: IAdminForthAuth; @@ -2296,15 +2299,14 @@ export interface OperationalResourceContextOptions { response?: IAdminForthHttpResponse; /** - * Record as it is stored before the mutation. Supply it when the caller has already loaded the - * record, so `update()` does not read it a second time and hooks see the same snapshot the - * caller worked from. + * Snapshot passed to edit save hooks when the caller has already loaded the record. + * `update()` still performs a scoped lookup to confirm access to the current row. */ oldRecord?: any; /** - * Record to delete, when the caller has already loaded it. Same purpose as `oldRecord`, for - * `delete()`. + * Snapshot passed to delete save hooks when the caller has already loaded the record. + * `delete()` still performs a scoped lookup to confirm access to the current row. */ record?: any; } diff --git a/tests/jest_tests/operational_resource_scope.test.ts b/tests/jest_tests/operational_resource_scope.test.ts index f88826642..51b3a8bc2 100644 --- a/tests/jest_tests/operational_resource_scope.test.ts +++ b/tests/jest_tests/operational_resource_scope.test.ts @@ -1,6 +1,15 @@ import OperationalResource from '../../adminforth/modules/operationalResource.js'; import UserScopedResource from '../../adminforth/modules/userScopedResource.js'; -import { ActionCheckSource } from '../../adminforth/types/Common.js'; +import AdminForthRestAPI from '../../adminforth/modules/restApi.js'; +import { authorizeResourceOperation, RESOURCE_ACCESS_GRANT } from '../../adminforth/modules/resourceAccess.js'; +import { ActionCheckSource, AllowedActionsEnum } from '../../adminforth/types/Common.js'; + +function singleFilters(filters: any): any[] { + if (Array.isArray(filters)) { + return filters.flatMap(singleFilters); + } + return filters.subFilters ? singleFilters(filters.subFilters) : [filters]; +} function setup(resourceId = 'users') { const calls = { @@ -70,6 +79,7 @@ function setup(resourceId = 'users') { resource.dataSourceColumns = resource.columns; const seenFilters: Record = {}; + const seenWrites: Record = {}; const connector = { createRecord: async ({ record }) => { calls.connectorCreate++; @@ -86,7 +96,9 @@ function setup(resourceId = 'users') { getData: async ({ filters }) => { calls.connectorGetData++; seenFilters.getData = filters; - if (Array.isArray(filters) && filters.some((filter) => filter.field === 'tenant' && filter.value === 'not-owned')) { + const requestedFilters = singleFilters(filters); + if (requestedFilters.some((filter) => filter.field === 'tenant' && filter.value === 'not-owned') + || requestedFilters.some((filter) => filter.field === 'id' && filter.value !== 1)) { return { data: [], total: 0 }; } return { data: [{ id: 1, name: 'John', private: 'hidden' }], total: 1 }; @@ -106,6 +118,7 @@ function setup(resourceId = 'users') { calls.connectorGetByPk++; return { id: 1, name: 'Old name', readonly: 'old' }; }, + getPrimaryKey: () => 'id', } as any; const adminforth = { config: { resources: [resource] } } as any; const executors = { @@ -113,21 +126,30 @@ function setup(resourceId = 'users') { calls.createExecutor++; return { createdRecord: { id: 1, ...record } }; }, - update: async () => { + update: async ({ oldRecord, updates }) => { calls.updateExecutor++; + seenWrites.update = { oldRecord, updates }; + return { error: null }; + }, + delete: async ({ record }, cascadeChildren) => { + seenWrites.delete = { record, cascadeChildren }; return { error: null }; }, - delete: async () => ({ error: null }), } as any; + const operationalResource = new OperationalResource( + connector, + resource, + (data, adminUser, options) => new UserScopedResource(data, adminforth, executors, adminUser, options), + ); + adminforth.resource = () => operationalResource; + return { calls, seenFilters, - resource: new OperationalResource( - connector, - resource, - (data, adminUser, options) => new UserScopedResource(data, adminforth, executors, adminUser, options), - ), + seenWrites, + adminforth, + resource: operationalResource, }; } @@ -198,14 +220,36 @@ describe('OperationalResource access tiers', () => { expect(calls).toMatchObject({ acl: 1, connectorDelete: 0 }); }); - it('reuses the record supplied by the caller instead of reading it again', async () => { - const { calls, resource } = setup(); + it('passes the caller delete snapshot to hooks after the scoped lookup', async () => { + const { seenWrites, resource } = setup(); + const record = { id: 1, name: 'Earlier snapshot' }; + let aclRecord: any; + resource.resourceConfig.options.allowedActions.delete = ({ meta }) => { + aclRecord = meta.record; + return true; + }; + + await expect(resource.asUser({} as any, { meta: { allowed: true }, record }).delete(1)) + .resolves.toBe(true); + expect(aclRecord.name).toBe('John'); + expect(seenWrites.delete).toEqual({ record, cascadeChildren: true }); + }); + + it('passes the caller snapshot to hooks after checking current row scope', async () => { + const { calls, seenWrites, resource } = setup(); + let aclRecord: any; + resource.resourceConfig.options.allowedActions.edit = ({ meta }) => { + aclRecord = meta.oldRecord; + return true; + }; const updated = await resource - .asUser({} as any, { meta: { allowed: true }, oldRecord: { id: 1, name: 'Old name' } }) + .asUser({} as any, { meta: { allowed: true }, oldRecord: { id: 1, name: 'Earlier snapshot' } }) .update(1, { name: 'Jane' }); expect(updated).toMatchObject({ ok: true }); + expect(aclRecord.name).toBe('John'); + expect(seenWrites.update.oldRecord).toEqual({ id: 1, name: 'Earlier snapshot' }); expect(calls).toMatchObject({ connectorGetByPk: 0, updateExecutor: 1, beforeList: 1 }); }); @@ -224,10 +268,128 @@ describe('OperationalResource access tiers', () => { }); await expect(scoped.delete(1)).resolves.toBe(false); - expect(seenFilters.getData).toContainEqual({ field: 'tenant', operator: 'eq', value: 'not-owned' }); + expect(singleFilters(seenFilters.getData)).toContainEqual({ field: 'tenant', operator: 'eq', value: 'not-owned' }); expect(calls).toMatchObject({ beforeList: 2, updateExecutor: 0, connectorDelete: 0 }); }); + it('keeps the requested primary key when a scope hook replaces its filters', async () => { + const { calls, seenFilters, resource } = setup(); + resource.resourceConfig.hooks.list.beforeDatasourceRequest = [async ({ query }) => { + query.filters = [{ field: 'tenant', operator: 'eq', value: 't1' }]; + return { ok: true }; + }]; + const scoped = resource.asUser({} as any, { meta: { allowed: true } }); + + await expect(scoped.update(2, { name: 'Jane' })).resolves.toMatchObject({ + ok: false, + error: expect.stringContaining('not found'), + }); + await expect(scoped.delete(2)).resolves.toBe(false); + + expect(singleFilters(seenFilters.getData)).toContainEqual({ field: 'id', operator: 'eq', value: 2 }); + expect(calls).toMatchObject({ updateExecutor: 0 }); + }); + + it('derives a hidden polymorphic discriminator after validating user fields', async () => { + const { calls, seenWrites, adminforth, resource } = setup(); + resource.resourceConfig.columns.push( + { name: 'resource_id', showIn: { create: false, edit: false } }, + { + name: 'record_id', + foreignResource: { + polymorphicOn: 'resource_id', + polymorphicResources: [ + { resourceId: 'targets', whenValue: 'target' }, + { resourceId: null, whenValue: 'system' }, + ], + }, + }, + ); + adminforth.config.resources.push({ + resourceId: 'targets', + dataSource: 'targets', + columns: [{ name: 'id', primaryKey: true }], + }); + adminforth.connectors = { + targets: { getData: async () => ({ data: [{ id: 'target-1' }] }) }, + }; + const scoped = resource.asUser({} as any, { meta: { allowed: true } }); + + const created = await scoped.create({ record_id: 'target-1' }); + const updated = await scoped.update(1, { record_id: 'target-1' }); + const forbidden = await scoped.create({ record_id: 'target-1', resource_id: 'target' }); + + expect(created.createdRecord.resource_id).toBe('target'); + expect(updated).toMatchObject({ ok: true }); + expect(seenWrites.update.updates.resource_id).toBe('target'); + expect(forbidden).toMatchObject({ ok: false, error: expect.stringContaining('showIn.create is false') }); + expect(calls).toMatchObject({ createExecutor: 1, updateExecutor: 1 }); + + let createChecks = 0; + resource.resourceConfig.options.allowedActions.create = () => ++createChecks === 1; + adminforth.config.auth = { rateLimit: [] }; + adminforth.activatedPlugins = []; + adminforth.connectors.main = resource.dataConnector; + const endpoints: Record = {}; + new AdminForthRestAPI(adminforth).registerEndpoints({ + endpoint: (endpoint: any) => { endpoints[endpoint.path] = endpoint; }, + } as any); + const requestRecord = { record_id: 'target-1' } as any; + const restResult = await endpoints['/create_record'].handler({ + body: { resourceId: 'users', record: requestRecord, requiredColumnsToSkip: [] }, + adminUser: {}, + query: {}, + headers: {}, + cookies: [], + requestUrl: '', + response: {}, + }); + + expect(restResult).toMatchObject({ ok: true, newRecordId: 1 }); + expect(requestRecord.resource_id).toBe('target'); + expect(createChecks).toBe(1); + + let editChecks = 0; + resource.resourceConfig.options.allowedActions.edit = () => ++editChecks === 1; + const editRecord = { record_id: 'target-1' } as any; + const editResult = await endpoints['/update_record'].handler({ + body: { resourceId: 'users', recordId: 1, record: editRecord }, + adminUser: {}, + query: {}, + headers: {}, + cookies: [], + requestUrl: '', + response: {}, + }); + + expect(editResult).toMatchObject({ ok: true }); + expect(editRecord.resource_id).toBe('target'); + expect(editChecks).toBe(1); + + resource.resourceConfig.options.allowedActions.edit = true; + await scoped.update(1, { record_id: null }); + expect(seenWrites.update.updates.resource_id).toBe('system'); + }); + + it('consumes a REST ACL grant only once for the same user, resource, and record', async () => { + const { adminforth, calls, resource } = setup(); + const adminUser = {} as any; + const record = { name: 'Jane' }; + const meta = { allowed: true }; + const access = await authorizeResourceOperation( + adminUser, resource.resourceConfig, meta, ActionCheckSource.CreateRequest, + AllowedActionsEnum.create, adminforth, record, + ); + expect(access.error).toBeNull(); + + resource.resourceConfig.options.allowedActions.create = false; + const scopedOptions = { meta, [RESOURCE_ACCESS_GRANT]: access.grant }; + const scoped = resource.asUser(adminUser, scopedOptions); + await expect(scoped.create(record)).resolves.toMatchObject({ ok: true }); + await expect(scoped.create(record)).resolves.toMatchObject({ ok: false, error: 'Action is not allowed' }); + expect(calls.createExecutor).toBe(1); + }); + it('applies the full user-scoped update path to an empty update', async () => { const denied = setup(); const deniedResult = await denied.resource.asUser({} as any, { meta: { allowed: false } }).update(1, {}); diff --git a/tests/jest_tests/resource_bulk_delete_order.test.ts b/tests/jest_tests/resource_bulk_delete_order.test.ts new file mode 100644 index 000000000..c39677a52 --- /dev/null +++ b/tests/jest_tests/resource_bulk_delete_order.test.ts @@ -0,0 +1,100 @@ +import AdminForth from '../../adminforth/index.js'; +import ConfigValidator from '../../adminforth/modules/configValidator.js'; + +function setup(parentBeforeSave: () => Promise<{ ok: boolean; error?: string }>) { + const events: string[] = []; + const parent = { + resourceId: 'parents', + dataSource: 'main', + columns: [{ name: 'id', primaryKey: true }], + hooks: { + delete: { + beforeSave: [async () => { + events.push('parent-before'); + return parentBeforeSave(); + }], + afterSave: [async () => { + events.push('parent-after'); + return { ok: true }; + }], + }, + }, + } as any; + const child = { + resourceId: 'children', + dataSource: 'main', + columns: [ + { name: 'id', primaryKey: true }, + { name: 'parent_id', foreignResource: { resourceId: 'parents', onDelete: 'cascade' } }, + ], + hooks: { + delete: { + beforeSave: [async () => { + events.push('child-before'); + return { ok: true }; + }], + afterSave: [async () => { + events.push('child-after'); + return { ok: true }; + }], + }, + }, + } as any; + const admin = Object.create(AdminForth.prototype) as any; + admin.config = { resources: [parent, child] }; + admin.statuses = { dbDiscover: 'done' }; + admin.warnedDeprecatedResourceMutations = new Set(); + admin.connectors = { + main: { + getRecordByPrimaryKey: async () => ({ id: 'p1' }), + deleteRecord: async ({ resource }) => { + events.push(resource.resourceId === 'parents' ? 'parent-delete' : 'child-delete'); + return true; + }, + }, + }; + admin.operationalResources = { + children: { + list: async () => { + events.push('child-list'); + return [{ id: 'c1', parent_id: 'p1' }]; + }, + }, + }; + const actions = new ConfigValidator(admin, {} as any) + .validateAndNormalizeBulkActions({ options: { bulkActions: [] } } as any, parent, []); + const deleteChecked = actions[actions.length - 1]; + + return { + events, + deleteChecked: () => deleteChecked.action({ + selectedIds: ['p1'], + adminUser: {} as any, + response: {} as any, + } as any), + }; +} + +describe('default bulk delete', () => { + it('does not cascade when the parent beforeSave hook vetoes deletion', async () => { + const { events, deleteChecked } = setup(async () => ({ ok: false, error: 'blocked' })); + + await expect(deleteChecked()).resolves.toMatchObject({ ok: false, error: 'blocked' }); + expect(events).toEqual(['parent-before']); + }); + + it('runs the parent veto hook before cascading and deletes each record once', async () => { + const { events, deleteChecked } = setup(async () => ({ ok: true })); + + await expect(deleteChecked()).resolves.toMatchObject({ ok: true }); + expect(events).toEqual([ + 'parent-before', + 'child-list', + 'child-before', + 'child-delete', + 'child-after', + 'parent-delete', + 'parent-after', + ]); + }); +}); diff --git a/tests/jest_tests/rest_resource_delete.test.ts b/tests/jest_tests/rest_resource_delete.test.ts new file mode 100644 index 000000000..69f3b6ba1 --- /dev/null +++ b/tests/jest_tests/rest_resource_delete.test.ts @@ -0,0 +1,27 @@ +import AdminForthRestAPI from '../../adminforth/modules/restApi.js'; + +it('does not report a successful REST deletion when the row is outside user scope', async () => { + const endpoints: Record = {}; + const resource = { resourceId: 'items', dataSource: 'main' }; + const adminforth = { + config: { auth: { rateLimit: [] }, resources: [resource] }, + activatedPlugins: [], + connectors: { main: { getRecordByPrimaryKey: async () => ({ id: 'item-1' }) } }, + resource: () => ({ asUser: () => ({ delete: async () => false }) }), + } as any; + new AdminForthRestAPI(adminforth).registerEndpoints({ + endpoint: (endpoint: any) => { endpoints[endpoint.path] = endpoint; }, + } as any); + + const result = await endpoints['/delete_record'].handler({ + body: { resourceId: 'items', primaryKey: 'item-1' }, + adminUser: {}, + query: {}, + headers: {}, + cookies: [], + requestUrl: '', + response: {}, + }); + + expect(result).toEqual({ error: 'Record with item-1 not found' }); +}); From 4425d18aaf5d4ab83346b21bad0733c40b4a8af5 Mon Sep 17 00:00:00 2001 From: Maksym Pipkun Date: Fri, 18 Sep 2026 16:31:36 +0300 Subject: [PATCH 09/11] fix: preserve resource API behavior during access checks refactor --- .../tutorial/03-Customization/11-dataApi.md | 4 +- .../tutorial/03-Customization/12-security.md | 2 +- adminforth/index.ts | 45 +++++++--- adminforth/modules/configValidator.ts | 19 ++-- adminforth/modules/operationalResource.ts | 15 +--- adminforth/modules/polymorphicReferences.ts | 8 +- adminforth/modules/userScopedResource.ts | 21 +++-- adminforth/types/Back.ts | 4 +- .../operational_resource_scope.test.ts | 87 ++++++++++++++++++- .../resource_bulk_delete_order.test.ts | 59 ++++++++++++- 10 files changed, 212 insertions(+), 52 deletions(-) diff --git a/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md b/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md index 639bbfadc..626fe2ba7 100644 --- a/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md +++ b/adminforth/documentation/docs/tutorial/03-Customization/11-dataApi.md @@ -58,8 +58,8 @@ multi-tenancy. Use it in plugin endpoints: you do not have to remember the individual checks, and you cannot forget one. The bare methods are plain data access for internal bookkeeping: no permission -checks, no column access rules, no hooks. Writes are still normalized and -validated. +checks, no column access rules, no hooks. Writes are still normalized; the +connector remains responsible for its own constraints. `admin.createResourceRecord`, `admin.updateResourceRecord` and `admin.deleteResourceRecord` are the older entry points which this API replaces. diff --git a/adminforth/documentation/docs/tutorial/03-Customization/12-security.md b/adminforth/documentation/docs/tutorial/03-Customization/12-security.md index 1e0216dc4..50b104b00 100644 --- a/adminforth/documentation/docs/tutorial/03-Customization/12-security.md +++ b/adminforth/documentation/docs/tutorial/03-Customization/12-security.md @@ -119,7 +119,7 @@ This is opt-in. It is especially important for the column configured as `auth.us | Path | When `normalize` runs | | --- | --- | | User-scoped Data API (`admin.resource(...).asUser(...)`) and `admin.createResourceRecord` | Before validation and `beforeSave` hooks | -| Bare Data API (`admin.resource(...).create(...)` and siblings) | Before validation and the connector operation | +| Bare Data API (`admin.resource(...).create(...)` and siblings) | Before the connector operation | | Deprecated AdminForth CRUD (`createResourceRecord`, `updateResourceRecord`) | Before validation and `beforeSave` hooks | | Core password login | On the submitted value of `auth.usernameField`, before the user lookup | | Reads and filters | Never — this includes `get`, `list`, `count`, search, and `Filters.EQ` | diff --git a/adminforth/index.ts b/adminforth/index.ts index efd297f09..2378ad54d 100644 --- a/adminforth/index.ts +++ b/adminforth/index.ts @@ -624,7 +624,7 @@ class AdminForth implements IAdminForth { { create: (params) => this.executeCreateResourceRecord(params), update: (params) => this.executeUpdateResourceRecord(params), - delete: (params, cascadeChildren) => this.executeDeleteResourceRecord(params, cascadeChildren), + delete: (params, cascadeChildren, bulkHooks) => this.executeDeleteResourceRecord(params, cascadeChildren, bulkHooks), }, adminUser, options, @@ -967,10 +967,11 @@ class AdminForth implements IAdminForth { private async executeDeleteResourceRecord( params: DeleteResourceRecordParams, cascadeChildren = false, + bulkHooks = false, ): Promise { const { resource, recordId, adminUser, record, response, extra } = params; - // execute hook if needed - for (const hook of listify(resource.hooks?.delete?.beforeSave)) { + const beforeHooks = listify(resource.hooks?.delete?.beforeSave); + const runBeforeHook = async (hook: typeof beforeHooks[number]) => { const resp = await hook({ resource, record, @@ -980,9 +981,21 @@ class AdminForth implements IAdminForth { response, extra, }); - const hookRespError = hookResponseError(resp); - if (hookRespError) { - return hookRespError; + return bulkHooks ? resp.error : hookResponseError(resp)?.error; + }; + if (bulkHooks) { + // The old default bulk action ran every beforeSave hook, even if one vetoed deletion. + const errors = await Promise.all(beforeHooks.map(runBeforeHook)); + const error = errors.find(Boolean); + if (error) { + return { error }; + } + } else { + for (const hook of beforeHooks) { + const error = await runBeforeHook(hook); + if (error) { + return { error }; + } } } @@ -1002,9 +1015,9 @@ class AdminForth implements IAdminForth { const connector = this.connectors[resource.dataSource]; await connector.deleteRecord({ resource, recordId, pkValues: compositePkValues(connector, resource, recordId) }); - // execute hook if needed - for (const hook of listify(resource.hooks?.delete?.afterSave)) { - const resp = await hook({ + const afterHooks = listify(resource.hooks?.delete?.afterSave); + const runAfterHook = async (hook: typeof afterHooks[number]) => { + const resp = await hook({ resource, record, adminUser, @@ -1013,9 +1026,17 @@ class AdminForth implements IAdminForth { response, extra, }); - const hookRespError = hookResponseError(resp); - if (hookRespError) { - return hookRespError; + return bulkHooks ? null : hookResponseError(resp)?.error; + }; + if (bulkHooks) { + // Returned afterSave errors never changed the legacy bulk action result. + await Promise.all(afterHooks.map(runAfterHook)); + } else { + for (const hook of afterHooks) { + const error = await runAfterHook(hook); + if (error) { + return { error }; + } } } diff --git a/adminforth/modules/configValidator.ts b/adminforth/modules/configValidator.ts index 7efebcf90..e9c7b9165 100644 --- a/adminforth/modules/configValidator.ts +++ b/adminforth/modules/configValidator.ts @@ -261,24 +261,17 @@ export default class ConfigValidator implements IConfigValidator { dangerous: true, allowed: async ({ resource, adminUser, allowedActions }) => { return allowedActions.delete }, action: async ({ selectedIds, adminUser, response }) => { - // The bulk action's `allowed` callback is its ACL boundary. Keep that action-level - // contract instead of introducing a second, per-record `asUser()` permission check. let error = null; - const connector = this.adminforth.connectors[res.dataSource]; await Promise.all( selectedIds.map(async (recordId) => { try { - const record = await connector.getRecordByPrimaryKey(res as AdminForthResource, recordId); - const result = await this.adminforth.deleteResourceRecord({ - resource: res as AdminForthResource, - recordId, - record, - adminUser, - response, - }, true); - if (result.error) { - throw new Error(result.error); + const deleted = await this.adminforth + .resource(res.resourceId) + .asUser(adminUser, { response, bulkDeleteHooks: true }) + .delete(recordId); + if (!deleted) { + throw new Error(`Record with ${recordId} not found`); } } catch (e) { if (!error) { diff --git a/adminforth/modules/operationalResource.ts b/adminforth/modules/operationalResource.ts index d2de7fbef..d18635131 100644 --- a/adminforth/modules/operationalResource.ts +++ b/adminforth/modules/operationalResource.ts @@ -14,7 +14,6 @@ import type { import type { AdminUser } from '../types/Common.js'; import { compositePkValues } from './recordId.js'; import { normalizeRecordValues } from './columnValueNormalizer.js'; -import { validateRecordValues } from './recordValidator.js'; /** * Builds the user-scoped layer on top of a data-access resource. Injected by AdminForth so this @@ -32,8 +31,8 @@ function sortsIfSort(sort: IAdminForthSort | IAdminForthSort[]): IAdminForthSort } /** - * Plain data access for one resource: talks to the connector, normalizes values and applies the - * column-level value rules. It has no notion of who is asking — no permissions, no column access + * Plain data access for one resource: talks to the connector and normalizes values. + * It has no notion of who is asking — no permissions, no column access * rules, no lifecycle hooks. * * For anything a user asked for, take {@link asUser}, which adds those on top. @@ -116,11 +115,6 @@ export default class OperationalResource implements IOperationalResource { async create(recordValues: any): Promise { const normalizedRecord = { ...recordValues }; normalizeRecordValues(this.resourceConfig, normalizedRecord); - const validationError = validateRecordValues(this.resourceConfig, normalizedRecord, 'create'); - if (validationError) { - return { ok: false, createdRecord: undefined, error: validationError }; - } - const { ok, createdRecord, error } = await this.dataConnector.createRecord({ resource: this.resourceConfig, record: normalizedRecord, @@ -136,11 +130,6 @@ export default class OperationalResource implements IOperationalResource { const normalizedRecord = { ...record }; normalizeRecordValues(this.resourceConfig, normalizedRecord); - const validationError = validateRecordValues(this.resourceConfig, normalizedRecord, 'edit'); - if (validationError) { - return { ok: false, error: validationError }; - } - return await this.dataConnector.updateRecord({ resource: this.resourceConfig, recordId: primaryKey, diff --git a/adminforth/modules/polymorphicReferences.ts b/adminforth/modules/polymorphicReferences.ts index b30d9231a..9cebb27ad 100644 --- a/adminforth/modules/polymorphicReferences.ts +++ b/adminforth/modules/polymorphicReferences.ts @@ -14,7 +14,7 @@ export async function resolvePolymorphicReferences( continue; } - let discriminator: string; + let discriminator: string | null; if (record[column.name] === null) { record[foreignResource.polymorphicOn] = foreignResource.polymorphicResources.find((target) => target.resourceId === null).whenValue; continue; @@ -42,6 +42,12 @@ export async function resolvePolymorphicReferences( continue; } + // Keep an existing SQL NULL when a changed reference matches no configured target. + // Writing undefined here can make a connector store a different value. + if (discriminator === undefined && oldRecord?.[foreignResource.polymorphicOn] === null) { + discriminator = null; + } + if (!oldRecord || oldRecord[foreignResource.polymorphicOn] !== discriminator) { record[foreignResource.polymorphicOn] = discriminator; } diff --git a/adminforth/modules/userScopedResource.ts b/adminforth/modules/userScopedResource.ts index 5796c6fbb..5fe4c0b95 100644 --- a/adminforth/modules/userScopedResource.ts +++ b/adminforth/modules/userScopedResource.ts @@ -51,7 +51,7 @@ type GuardedOperation = keyof typeof OPERATION_ACCESS; export interface ResourceHookExecutors { create(params: CreateResourceRecordParams): Promise; update(params: UpdateResourceRecordParams): Promise; - delete(params: DeleteResourceRecordParams, cascadeChildren?: boolean): Promise; + delete(params: DeleteResourceRecordParams, cascadeChildren?: boolean, bulkHooks?: boolean): Promise; } @@ -155,16 +155,27 @@ export default class UserScopedResource implements IScopedOperationalResource { * would bypass tenant filters installed by `beforeDatasourceRequest` hooks. */ private async findScopedRecord(primaryKey: any): Promise { - const primaryKeyColumn = this.resourceConfig.columns.find((column) => column.primaryKey); + const primaryKeyColumns = this.resourceConfig.columns.filter((column) => column.primaryKey); + // Connectors own composite recordId interpretation. A scalar key needs no extra lookup. + let identityFilters: ReturnType[]; + if (primaryKeyColumns.length === 1) { + identityFilters = [Filters.EQ(primaryKeyColumns[0].name, primaryKey)]; + } else { + const candidate = await this.dataConnector.getRecordByPrimaryKey(this.resourceConfig, primaryKey); + if (!candidate) { + return null; + } + identityFilters = primaryKeyColumns.map((column) => Filters.EQ(column.name, candidate[column.name])); + } const query = { - filters: [Filters.EQ(primaryKeyColumn.name, primaryKey)], + filters: identityFilters.map((filter) => ({ ...filter })), limit: 1, offset: 0, sort: [], }; await this.runReadHooks('list', 'beforeDatasourceRequest', query); const scopedFilters = this.dataConnector.validateAndNormalizeInputFilters(query.filters); - return this.data.get(Filters.AND(Filters.EQ(primaryKeyColumn.name, primaryKey), scopedFilters)); + return this.data.get(Filters.AND(...identityFilters, scopedFilters)); } async get(filter: IAdminForthSingleFilter | IAdminForthAndOrFilter | Array): Promise { @@ -364,7 +375,7 @@ export default class UserScopedResource implements IScopedOperationalResource { adminUser: this.adminUser, extra: this.options.extra, response: this.options.response, - }, true); + }, true, this.options.bulkDeleteHooks); if (error) { throw new Error(error); } diff --git a/adminforth/types/Back.ts b/adminforth/types/Back.ts index 49a9ed2f3..e658717c6 100644 --- a/adminforth/types/Back.ts +++ b/adminforth/types/Back.ts @@ -2266,7 +2266,7 @@ export interface IOperationalResource { /** * Plain data access: no permission checks, no column access rules, no lifecycle hooks. - * Writes are still normalized and validated. Use it for internal bookkeeping the user did not + * Writes are still normalized. Use it for internal bookkeeping the user did not * ask for; to run hooks without permission checks, use {@link IAdminForth.createResourceRecord} * and its siblings. */ @@ -2297,6 +2297,8 @@ export interface OperationalResourceContextOptions { meta?: any; extra?: HttpExtra; response?: IAdminForthHttpResponse; + /** Preserve the legacy default bulk action's hook result contract. */ + bulkDeleteHooks?: boolean; /** * Snapshot passed to edit save hooks when the caller has already loaded the record. diff --git a/tests/jest_tests/operational_resource_scope.test.ts b/tests/jest_tests/operational_resource_scope.test.ts index 51b3a8bc2..314de673d 100644 --- a/tests/jest_tests/operational_resource_scope.test.ts +++ b/tests/jest_tests/operational_resource_scope.test.ts @@ -131,8 +131,8 @@ function setup(resourceId = 'users') { seenWrites.update = { oldRecord, updates }; return { error: null }; }, - delete: async ({ record }, cascadeChildren) => { - seenWrites.delete = { record, cascadeChildren }; + delete: async ({ record }, cascadeChildren, bulkHooks) => { + seenWrites.delete = { record, cascadeChildren, bulkHooks }; return { error: null }; }, } as any; @@ -176,6 +176,15 @@ describe('OperationalResource access tiers', () => { expect(calls).toMatchObject({ acl: 0, createExecutor: 0, connectorCreate: 1 }); }); + it('keeps bare writes free of resource validation', async () => { + const { calls, resource } = setup(); + resource.resourceConfig.columns.push({ name: 'score', type: 'integer', minValue: 10 } as any); + + await expect(resource.create({ score: 1 })).resolves.toMatchObject({ ok: true }); + await expect(resource.update(1, { score: 1 })).resolves.toMatchObject({ ok: true }); + expect(calls).toMatchObject({ connectorCreate: 1, connectorUpdate: 1 }); + }); + it('rejects editReadonly for asUser()', async () => { const { calls, resource } = setup(); @@ -290,6 +299,54 @@ describe('OperationalResource access tiers', () => { expect(calls).toMatchObject({ updateExecutor: 0 }); }); + it('uses connector identity for composite keys and scopes every key column', async () => { + const { resource, seenWrites } = setup(); + resource.resourceConfig.columns.push({ name: 'partition', primaryKey: true } as any); + resource.resourceConfig.hooks.list.beforeDatasourceRequest = [async ({ query }) => { + query.filters.splice(0); + return { ok: true }; + }]; + const connector = resource.dataConnector as any; + const recordId = { id: 1, partition: 'second' }; + let lookedUpId: any; + let scopedFilters: any; + connector.getRecordByPrimaryKey = async (_resource, id) => { + lookedUpId = id; + return { ...recordId, name: 'Old name' }; + }; + connector.getData = async ({ filters }) => { + scopedFilters = filters; + const fields = singleFilters(filters); + const matches = fields.some(({ field, value }) => field === 'id' && value === 1) + && fields.some(({ field, value }) => field === 'partition' && value === 'second'); + return { data: matches ? [{ ...recordId, name: 'Old name' }] : [] }; + }; + + const result = await resource.asUser({} as any, { meta: { allowed: true } }) + .update(recordId, { name: 'New name' }); + + expect(result).toMatchObject({ ok: true }); + expect(lookedUpId).toBe(recordId); + expect(seenWrites.update.oldRecord).toMatchObject(recordId); + expect(singleFilters(scopedFilters)) + .toEqual(expect.arrayContaining([ + expect.objectContaining({ field: 'id', value: 1 }), + expect.objectContaining({ field: 'partition', value: 'second' }), + ])); + + const deleted = await resource.asUser({} as any, { meta: { allowed: true } }).delete(recordId); + expect(deleted).toBe(true); + expect(seenWrites.delete.record).toMatchObject(recordId); + }); + + it('passes the legacy bulk hook mode through scoped delete', async () => { + const { resource, seenWrites } = setup(); + + await resource.asUser({} as any, { meta: { allowed: true }, bulkDeleteHooks: true }).delete(1); + + expect(seenWrites.delete).toMatchObject({ cascadeChildren: true, bulkHooks: true }); + }); + it('derives a hidden polymorphic discriminator after validating user fields', async () => { const { calls, seenWrites, adminforth, resource } = setup(); resource.resourceConfig.columns.push( @@ -371,6 +428,32 @@ describe('OperationalResource access tiers', () => { expect(seenWrites.update.updates.resource_id).toBe('system'); }); + it('keeps a null polymorphic discriminator when no target matches', async () => { + const { adminforth, resource, seenWrites } = setup(); + resource.resourceConfig.columns.push( + { name: 'resource_id', showIn: { edit: false } }, + { + name: 'record_id', + foreignResource: { + polymorphicOn: 'resource_id', + polymorphicResources: [{ resourceId: 'targets', whenValue: 'target' }], + }, + }, + ); + adminforth.config.resources.push({ + resourceId: 'targets', dataSource: 'targets', columns: [{ name: 'id', primaryKey: true }], + }); + adminforth.connectors = { targets: { getData: async () => ({ data: [] }) } }; + (resource.dataConnector as any).getData = async () => ({ + data: [{ id: 1, record_id: null, resource_id: null }], + }); + + await resource.asUser({} as any, { meta: { allowed: true } }) + .update(1, { record_id: 'missing' }); + + expect(seenWrites.update.updates).toEqual({ record_id: 'missing' }); + }); + it('consumes a REST ACL grant only once for the same user, resource, and record', async () => { const { adminforth, calls, resource } = setup(); const adminUser = {} as any; diff --git a/tests/jest_tests/resource_bulk_delete_order.test.ts b/tests/jest_tests/resource_bulk_delete_order.test.ts index c39677a52..b580f4443 100644 --- a/tests/jest_tests/resource_bulk_delete_order.test.ts +++ b/tests/jest_tests/resource_bulk_delete_order.test.ts @@ -3,6 +3,7 @@ import ConfigValidator from '../../adminforth/modules/configValidator.js'; function setup(parentBeforeSave: () => Promise<{ ok: boolean; error?: string }>) { const events: string[] = []; + let scopedDeleteCalls = 0; const parent = { resourceId: 'parents', dataSource: 'main', @@ -61,12 +62,37 @@ function setup(parentBeforeSave: () => Promise<{ ok: boolean; error?: string }>) }, }, }; + admin.resource = (resourceId: string) => { + if (resourceId === 'children') { + return admin.operationalResources.children; + } + return { + asUser: (adminUser: any, { response, bulkDeleteHooks }: any) => ({ + delete: async (recordId: string) => { + scopedDeleteCalls += 1; + const result = await admin.executeDeleteResourceRecord({ + resource: parent, + recordId, + record: await admin.connectors.main.getRecordByPrimaryKey(parent, recordId), + adminUser, + response, + }, true, bulkDeleteHooks); + if (result.error) { + throw new Error(result.error); + } + return true; + }, + }), + }; + }; const actions = new ConfigValidator(admin, {} as any) .validateAndNormalizeBulkActions({ options: { bulkActions: [] } } as any, parent, []); const deleteChecked = actions[actions.length - 1]; return { events, + parent, + scopedDeleteCalls: () => scopedDeleteCalls, deleteChecked: () => deleteChecked.action({ selectedIds: ['p1'], adminUser: {} as any, @@ -77,16 +103,18 @@ function setup(parentBeforeSave: () => Promise<{ ok: boolean; error?: string }>) describe('default bulk delete', () => { it('does not cascade when the parent beforeSave hook vetoes deletion', async () => { - const { events, deleteChecked } = setup(async () => ({ ok: false, error: 'blocked' })); + const { events, scopedDeleteCalls, deleteChecked } = setup(async () => ({ ok: false, error: 'blocked' })); await expect(deleteChecked()).resolves.toMatchObject({ ok: false, error: 'blocked' }); + expect(scopedDeleteCalls()).toBe(1); expect(events).toEqual(['parent-before']); }); it('runs the parent veto hook before cascading and deletes each record once', async () => { - const { events, deleteChecked } = setup(async () => ({ ok: true })); + const { events, scopedDeleteCalls, deleteChecked } = setup(async () => ({ ok: true })); await expect(deleteChecked()).resolves.toMatchObject({ ok: true }); + expect(scopedDeleteCalls()).toBe(1); expect(events).toEqual([ 'parent-before', 'child-list', @@ -97,4 +125,31 @@ describe('default bulk delete', () => { 'parent-after', ]); }); + + it('runs every beforeSave hook when one vetoes deletion', async () => { + const { events, parent, deleteChecked } = setup(async () => ({ ok: false, error: 'blocked' })); + parent.hooks.delete.beforeSave.push(async () => { + events.push('parent-before-second'); + return { ok: true }; + }); + + await expect(deleteChecked()).resolves.toMatchObject({ ok: false, error: 'blocked' }); + expect(events).toEqual(['parent-before', 'parent-before-second']); + }); + + it('runs every afterSave hook and ignores returned errors', async () => { + const { events, parent, deleteChecked } = setup(async () => ({ ok: true })); + parent.hooks.delete.afterSave[0] = async () => { + events.push('parent-after'); + return { ok: false, error: 'after failed' }; + }; + parent.hooks.delete.afterSave.push(async () => { + events.push('parent-after-second'); + return { ok: true }; + }); + + await expect(deleteChecked()).resolves.toMatchObject({ ok: true }); + expect(events).toContain('parent-after-second'); + expect(events).toContain('parent-delete'); + }); }); From cdb659c9ab496a37565c18f29ef313a54b2304d6 Mon Sep 17 00:00:00 2001 From: Maksym Pipkun Date: Fri, 18 Sep 2026 16:51:37 +0300 Subject: [PATCH 10/11] fix: close scoped access gaps and preserve polymorphic behavior --- adminforth/modules/columnAccess.ts | 16 ++++++ adminforth/modules/polymorphicReferences.ts | 11 ++--- adminforth/modules/restApi.ts | 15 ++++++ adminforth/modules/userScopedResource.ts | 17 +++++-- .../operational_resource_scope.test.ts | 49 +++++++++++++++++++ 5 files changed, 97 insertions(+), 11 deletions(-) diff --git a/adminforth/modules/columnAccess.ts b/adminforth/modules/columnAccess.ts index bf62566e3..de30f1ccf 100644 --- a/adminforth/modules/columnAccess.ts +++ b/adminforth/modules/columnAccess.ts @@ -3,6 +3,7 @@ import type { AllowedActionValue, BackendOnlyInput, IAdminForth, + IAdminForthSort, } from '../types/Back.js'; import { ActionCheckSource, @@ -167,6 +168,21 @@ export async function filterColumnsReadableError( return null; } +/** Sorting can reveal a hidden field through the order of otherwise readable rows. */ +export async function sortColumnsReadableError( + ctx: ColumnAccessContext, + sort: IAdminForthSort | IAdminForthSort[], +): Promise { + const rules = Array.isArray(sort) ? sort : [sort]; + for (const rule of rules) { + const column = ctx.resource.columns.find((candidate) => candidate.name === rule.field); + if (column && await isBackendOnly(column, ctx)) { + return `Sort: column "${rule.field}" cannot be used (backendOnly is true).`; + } + } + return null; +} + /** * A column may only take part in an aggregation if the user could have read the very same value * from the show view, otherwise min/max/groupBy become a way to read hidden columns. diff --git a/adminforth/modules/polymorphicReferences.ts b/adminforth/modules/polymorphicReferences.ts index 9cebb27ad..b428fa8e7 100644 --- a/adminforth/modules/polymorphicReferences.ts +++ b/adminforth/modules/polymorphicReferences.ts @@ -14,7 +14,7 @@ export async function resolvePolymorphicReferences( continue; } - let discriminator: string | null; + let discriminator: string | null | undefined = oldRecord ? null : undefined; if (record[column.name] === null) { record[foreignResource.polymorphicOn] = foreignResource.polymorphicResources.find((target) => target.resourceId === null).whenValue; continue; @@ -25,6 +25,9 @@ export async function resolvePolymorphicReferences( continue; } const targetResource = adminforth.config.resources.find((candidate) => candidate.resourceId === target.resourceId); + if (!targetResource) { + continue; + } const targetPrimaryKey = targetResource.columns.find((candidate) => candidate.primaryKey).name; const { data } = await adminforth.connectors[targetResource.dataSource].getData({ resource: targetResource, @@ -42,12 +45,6 @@ export async function resolvePolymorphicReferences( continue; } - // Keep an existing SQL NULL when a changed reference matches no configured target. - // Writing undefined here can make a connector store a different value. - if (discriminator === undefined && oldRecord?.[foreignResource.polymorphicOn] === null) { - discriminator = null; - } - if (!oldRecord || oldRecord[foreignResource.polymorphicOn] !== discriminator) { record[foreignResource.polymorphicOn] = discriminator; } diff --git a/adminforth/modules/restApi.ts b/adminforth/modules/restApi.ts index 02e28fd8d..25f2185cc 100644 --- a/adminforth/modules/restApi.ts +++ b/adminforth/modules/restApi.ts @@ -34,6 +34,7 @@ import { filtersTools } from "../modules/filtersTools.js"; import { normalizeColumnValue } from './columnValueNormalizer.js'; import { isShown, + sortColumnsReadableError, stripReadForbiddenColumns, } from './columnAccess.js'; import { authorizeResourceOperation, interpretResource, RESOURCE_ACCESS_GRANT } from './resourceAccess.js'; @@ -1385,6 +1386,20 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { const col = resource.columns.find((col) => col.name === sortItem.field); return col && !col.virtual; }); + const sortError = await sortColumnsReadableError({ + adminUser, + resource, + meta, + source: { + show: ActionCheckSource.ShowRequest, + list: ActionCheckSource.ListRequest, + edit: ActionCheckSource.EditLoadRequest, + }[source], + adminforth: this.adminforth, + }, sortFiltered); + if (sortError) { + return { error: sortError }; + } // after beforeDatasourceRequest hook, filter can be anything // so, we need to turn it into AndOr filter diff --git a/adminforth/modules/userScopedResource.ts b/adminforth/modules/userScopedResource.ts index 5fe4c0b95..445df94ae 100644 --- a/adminforth/modules/userScopedResource.ts +++ b/adminforth/modules/userScopedResource.ts @@ -22,6 +22,7 @@ import { columnsAggregatableError, filterColumnsReadableError, recordWriteError, + sortColumnsReadableError, stripReadForbiddenColumns, type ColumnAccessContext, } from './columnAccess.js'; @@ -155,17 +156,17 @@ export default class UserScopedResource implements IScopedOperationalResource { * would bypass tenant filters installed by `beforeDatasourceRequest` hooks. */ private async findScopedRecord(primaryKey: any): Promise { - const primaryKeyColumns = this.resourceConfig.columns.filter((column) => column.primaryKey); + const keyColumns = this.resourceConfig.columns.filter((column) => column.primaryKey); // Connectors own composite recordId interpretation. A scalar key needs no extra lookup. let identityFilters: ReturnType[]; - if (primaryKeyColumns.length === 1) { - identityFilters = [Filters.EQ(primaryKeyColumns[0].name, primaryKey)]; + if (keyColumns.length === 1) { + identityFilters = [Filters.EQ(keyColumns[0].name, primaryKey)]; } else { const candidate = await this.dataConnector.getRecordByPrimaryKey(this.resourceConfig, primaryKey); if (!candidate) { return null; } - identityFilters = primaryKeyColumns.map((column) => Filters.EQ(column.name, candidate[column.name])); + identityFilters = keyColumns.map((column) => Filters.EQ(column.name, candidate[column.name])); } const query = { filters: identityFilters.map((filter) => ({ ...filter })), @@ -224,6 +225,14 @@ export default class UserScopedResource implements IScopedOperationalResource { throw new Error(filterError); } + const sortError = await sortColumnsReadableError( + this.columnCtx(ActionCheckSource.ListRequest), + sort, + ); + if (sortError) { + throw new Error(sortError); + } + const query = { filters: filter, limit, offset, sort }; await this.runReadHooks('list', 'beforeDatasourceRequest', query); const data = await this.data.list(query.filters, query.limit, query.offset, query.sort, columns); diff --git a/tests/jest_tests/operational_resource_scope.test.ts b/tests/jest_tests/operational_resource_scope.test.ts index 314de673d..04c72579c 100644 --- a/tests/jest_tests/operational_resource_scope.test.ts +++ b/tests/jest_tests/operational_resource_scope.test.ts @@ -452,6 +452,21 @@ describe('OperationalResource access tiers', () => { .update(1, { record_id: 'missing' }); expect(seenWrites.update.updates).toEqual({ record_id: 'missing' }); + + resource.resourceConfig.columns.find((column) => column.name === 'record_id') + .foreignResource.polymorphicResources.unshift({ resourceId: 'removed-target', whenValue: 'removed' }); + (resource.dataConnector as any).getData = async () => ({ + data: [{ id: 1, record_id: 'old', resource_id: 'target' }], + }); + + await resource.asUser({} as any, { meta: { allowed: true } }) + .update(1, { record_id: 'missing' }); + + expect(seenWrites.update.updates).toEqual({ record_id: 'missing', resource_id: null }); + + const created = await resource.asUser({} as any, { meta: { allowed: true } }) + .create({ record_id: 'missing' }); + expect(created.createdRecord).toHaveProperty('resource_id', undefined); }); it('consumes a REST ACL grant only once for the same user, resource, and record', async () => { @@ -528,6 +543,40 @@ describe('OperationalResource access tiers', () => { expect(calls).toMatchObject({ connectorGetData: 0, connectorCount: 0, beforeList: 0 }); }); + it('refuses to sort by backendOnly columns through user-scoped lists', async () => { + const { calls, resource } = setup(); + + await expect(resource.asUser({} as any, { meta: { allowed: true } }) + .list([], null, null, { field: 'private', direction: 'asc' } as any)) + .rejects.toThrow('backendOnly is true'); + expect(calls.connectorGetData).toBe(0); + }); + + it('refuses to sort by backendOnly columns through the REST list endpoint', async () => { + const { adminforth, calls, resource } = setup(); + resource.resourceConfig.options.allowedActions.list = true; + adminforth.config.auth = { rateLimit: [] }; + adminforth.activatedPlugins = []; + adminforth.statuses = { dbDiscover: 'done' }; + adminforth.connectors = { main: resource.dataConnector }; + const endpoints: Record = {}; + new AdminForthRestAPI(adminforth).registerEndpoints({ + endpoint: (endpoint: any) => { endpoints[endpoint.path] = endpoint; }, + } as any); + + const result = await endpoints['/get_resource_data'].handler({ + body: { + resourceId: 'users', source: 'list', filters: [], limit: 10, offset: 0, + sort: [{ field: 'private', direction: 'asc' }], + }, + adminUser: {}, headers: {}, query: {}, cookies: [], requestUrl: '', + abortSignal: new AbortController().signal, + }); + + expect(result.error).toContain('backendOnly is true'); + expect(calls.connectorGetData).toBe(0); + }); + it('row-scopes aggregate and count through the same read hooks as list', async () => { const { calls, seenFilters, resource } = setup(); const scoped = resource.asUser({} as any, { meta: { allowed: true } }); From 9077acb02a37ba7c78cafae2eb8df62d3849873b Mon Sep 17 00:00:00 2001 From: Maksym Pipkun Date: Mon, 21 Sep 2026 13:51:54 +0300 Subject: [PATCH 11/11] fix: preserve scoped mutation security and compatibility --- adminforth/index.ts | 18 ++- adminforth/modules/configValidator.ts | 9 +- adminforth/modules/restApi.ts | 50 +++++-- adminforth/modules/userScopedResource.ts | 58 +++++--- adminforth/modules/utils.ts | 10 +- adminforth/types/Back.ts | 5 +- .../operational_resource_scope.test.ts | 90 +++++++++++- .../resource_bulk_delete_order.test.ts | 133 +++++++++++++----- tests/jest_tests/rest_resource_delete.test.ts | 54 ++++++- 9 files changed, 334 insertions(+), 93 deletions(-) diff --git a/adminforth/index.ts b/adminforth/index.ts index 2378ad54d..67b39682f 100644 --- a/adminforth/index.ts +++ b/adminforth/index.ts @@ -865,15 +865,6 @@ class AdminForth implements IAdminForth { ): Promise { const { resource, recordId, record, oldRecord, adminUser, response, extra, updates } = params; const dataToUse = updates || record; - - // a system update silently drops editReadonly columns, as it always has; a user update never - // reaches this point with one, it is rejected by the column access check inside asUser() - for (const column of resource.columns.filter((candidate) => candidate.editReadonly)) { - if (column.name in dataToUse) { - delete dataToUse[column.name]; - } - } - normalizeRecordValues(resource, dataToUse); const err = validateRecordValues(resource, dataToUse, 'edit'); if (err) { @@ -884,6 +875,13 @@ class AdminForth implements IAdminForth { afLogger.warn(`updateResourceRecord function received 'record' param which is deprecated and will be removed in future version, please use 'updates' instead.`); } + // remove editReadonly columns from record + for (const column of resource.columns.filter((candidate) => candidate.editReadonly)) { + if (column.name in dataToUse) { + delete dataToUse[column.name]; + } + } + // execute hook if needed for (const hook of listify(resource.hooks?.edit?.beforeSave)) { const resp = await hook({ @@ -1056,7 +1054,7 @@ class AdminForth implements IAdminForth { return; } this.warnedDeprecatedResourceMutations.add(warnKey); - afLogger.warn( + afLogger.trace( `${method} is deprecated and will be removed in the next major version. ` + `Use adminforth.resource('${resourceId}').asUser(adminUser, { meta }).${operation}(...) ` + `for anything a user requested, or adminforth.resource('${resourceId}').${operation}(...) ` diff --git a/adminforth/modules/configValidator.ts b/adminforth/modules/configValidator.ts index e9c7b9165..bba1c8220 100644 --- a/adminforth/modules/configValidator.ts +++ b/adminforth/modules/configValidator.ts @@ -260,7 +260,7 @@ export default class ConfigValidator implements IConfigValidator { }, dangerous: true, allowed: async ({ resource, adminUser, allowedActions }) => { return allowedActions.delete }, - action: async ({ selectedIds, adminUser, response }) => { + action: async ({ selectedIds, adminUser, response, extra }) => { let error = null; await Promise.all( @@ -268,7 +268,12 @@ export default class ConfigValidator implements IConfigValidator { try { const deleted = await this.adminforth .resource(res.resourceId) - .asUser(adminUser, { response, bulkDeleteHooks: true }) + .asUser(adminUser, { + meta: { requestBody: extra?.body }, + response, + extra, + bulkDeleteHooks: true, + }) .delete(recordId); if (!deleted) { throw new Error(`Record with ${recordId} not found`); diff --git a/adminforth/modules/restApi.ts b/adminforth/modules/restApi.ts index 25f2185cc..bac056c8d 100644 --- a/adminforth/modules/restApi.ts +++ b/adminforth/modules/restApi.ts @@ -2137,10 +2137,6 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { const recordId = body['recordId']; const connector = this.adminforth.connectors[resource.dataSource]; const oldRecord = await connector.getRecordByPrimaryKey(resource, recordId) - if (!oldRecord) { - const primaryKeyColumn = resource.columns.find((col) => col.primaryKey); - return { error: `Record with ${isCompositePrimaryKey(resource) ? primaryKeyColumnNames(resource).join(', ') : primaryKeyColumn.name} ${recordId} not found` }; - } const record = body['record']; // Check before revealing whether another record has the requested key. @@ -2157,6 +2153,10 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { if (editAccess.error) { return { error: editAccess.error }; } + if (!oldRecord) { + const primaryKeyColumn = resource.columns.find((col) => col.primaryKey); + return { error: `Record with ${isCompositePrimaryKey(resource) ? primaryKeyColumnNames(resource).join(', ') : primaryKeyColumn.name} ${recordId} not found` }; + } if (isCompositePrimaryKey(resource)) { const pkColumnNames = primaryKeyColumnNames(resource); @@ -2231,19 +2231,34 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { return { error: `Resource '${body['resourceId']}' not found` }; } const record = await this.adminforth.connectors[resource.dataSource].getRecordByPrimaryKey(resource, body['primaryKey']); - if (!record){ - return { error: `Record with ${body['primaryKey']} not found` }; + const deleteAccess = await authorizeResourceOperation( + adminUser, + resource, + { requestBody: body, record }, + ActionCheckSource.DeleteRequest, + AllowedActionsEnum.delete, + this.adminforth, + record, + body.primaryKey, + ); + if (deleteAccess.error) { + return { error: deleteAccess.error }; + } + if (!record) { + return { error: `Record with ${body['primaryKey']} not found` }; } try { + const scopedDeleteOptions = { + meta: { requestBody: body, record }, + record, + response, + extra: { body, query, headers, cookies, requestUrl, response }, + [RESOURCE_ACCESS_GRANT]: deleteAccess.grant, + }; const deleted = await this.adminforth .resource(resource.resourceId) - .asUser(adminUser, { - meta: { requestBody: body, record }, - record, - response, - extra: { body, query, headers, cookies, requestUrl, response }, - }) + .asUser(adminUser, scopedDeleteOptions) .delete(body.primaryKey); if (!deleted) { return { error: `Record with ${body.primaryKey} not found` }; @@ -2260,7 +2275,7 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { server.endpoint({ method: 'POST', path: '/start_bulk_action', - handler: async ({ body, adminUser, tr, response }) => { + handler: async ({ body, adminUser, tr, response, query, headers, cookies, requestUrl }) => { const { resourceId, actionId, recordIds } = body; const resource = this.adminforth.config.resources.find((res) => res.resourceId == resourceId); if (!resource) { @@ -2285,7 +2300,14 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI { return { error: await tr(`Action "{actionId}" not allowed`, 'errors', { actionId: action.label }) }; } } - const bulkActionResponse = await action.action({selectedIds: recordIds, adminUser, resource, response, tr}); + const bulkActionResponse = await action.action({ + selectedIds: recordIds, + adminUser, + resource, + response, + tr, + extra: { body, query, headers, cookies, requestUrl, response }, + }); return { actionId, diff --git a/adminforth/modules/userScopedResource.ts b/adminforth/modules/userScopedResource.ts index 445df94ae..c266d4710 100644 --- a/adminforth/modules/userScopedResource.ts +++ b/adminforth/modules/userScopedResource.ts @@ -113,6 +113,7 @@ export default class UserScopedResource implements IScopedOperationalResource { phase: 'beforeDatasourceRequest' | 'afterDatasourceResponse', query: any, records?: any[], + extra = this.options.extra, ): Promise { const hooks = listify(this.resourceConfig.hooks?.[page]?.[phase]); if (!hooks.length) { @@ -124,7 +125,7 @@ export default class UserScopedResource implements IScopedOperationalResource { resource: this.resourceConfig, query, adminUser: this.adminUser, - extra: this.options.extra ?? { + extra: extra ?? { body: query, query: {}, headers: {}, @@ -155,26 +156,32 @@ export default class UserScopedResource implements IScopedOperationalResource { * Finds a record through the list scope before a mutation. A primary-key connector lookup * would bypass tenant filters installed by `beforeDatasourceRequest` hooks. */ - private async findScopedRecord(primaryKey: any): Promise { + private async findScopedRecord(primaryKey: any, candidate?: any): Promise { const keyColumns = this.resourceConfig.columns.filter((column) => column.primaryKey); // Connectors own composite recordId interpretation. A scalar key needs no extra lookup. let identityFilters: ReturnType[]; if (keyColumns.length === 1) { identityFilters = [Filters.EQ(keyColumns[0].name, primaryKey)]; } else { - const candidate = await this.dataConnector.getRecordByPrimaryKey(this.resourceConfig, primaryKey); - if (!candidate) { + const compositeRecord = candidate + ?? await this.dataConnector.getRecordByPrimaryKey(this.resourceConfig, primaryKey); + if (!compositeRecord) { return null; } - identityFilters = keyColumns.map((column) => Filters.EQ(column.name, candidate[column.name])); + identityFilters = keyColumns.map((column) => Filters.EQ(column.name, compositeRecord[column.name])); } const query = { + resourceId: this.resourceConfig.resourceId, + source: 'list', filters: identityFilters.map((filter) => ({ ...filter })), limit: 1, offset: 0, sort: [], }; - await this.runReadHooks('list', 'beforeDatasourceRequest', query); + const listExtra = this.options.extra + ? { ...this.options.extra, body: query } + : undefined; + await this.runReadHooks('list', 'beforeDatasourceRequest', query, undefined, listExtra); const scopedFilters = this.dataConnector.validateAndNormalizeInputFilters(query.filters); return this.data.get(Filters.AND(...identityFilters, scopedFilters)); } @@ -327,13 +334,8 @@ export default class UserScopedResource implements IScopedOperationalResource { } async update(primaryKey: any, record: any): Promise { - const scopedRecord = await this.findScopedRecord(primaryKey); - if (!scopedRecord) { - const primaryKeyColumn = this.resourceConfig.columns.find((column) => column.primaryKey); - return { ok: false, error: `Record with ${primaryKeyColumn.name} ${primaryKey} not found` }; - } - const oldRecord = this.options.oldRecord ?? scopedRecord; - const meta = { ...this.meta, newRecord: record, oldRecord: scopedRecord, pk: primaryKey }; + const currentRecord = await this.dataConnector.getRecordByPrimaryKey(this.resourceConfig, primaryKey); + const meta = { ...this.meta, newRecord: record, oldRecord: currentRecord, pk: primaryKey }; const accessError = consumeResourceAccessGrant( this.options[RESOURCE_ACCESS_GRANT], this.adminUser, this.resourceConfig, AllowedActionsEnum.edit, record, primaryKey, @@ -342,6 +344,17 @@ export default class UserScopedResource implements IScopedOperationalResource { return { ok: false, error: accessError }; } + if (!currentRecord) { + const primaryKeyColumn = this.resourceConfig.columns.find((column) => column.primaryKey); + return { ok: false, error: `Record with ${primaryKeyColumn.name} ${primaryKey} not found` }; + } + const scopedRecord = await this.findScopedRecord(primaryKey, currentRecord); + if (!scopedRecord) { + const primaryKeyColumn = this.resourceConfig.columns.find((column) => column.primaryKey); + return { ok: false, error: `Record with ${primaryKeyColumn.name} ${primaryKey} not found` }; + } + const oldRecord = this.options.oldRecord ?? scopedRecord; + const columnError = await recordWriteError( this.columnCtx(ActionCheckSource.EditRequest, meta), record, @@ -366,17 +379,24 @@ export default class UserScopedResource implements IScopedOperationalResource { } async delete(primaryKey: any): Promise { - const scopedRecord = await this.findScopedRecord(primaryKey); + const currentRecord = await this.dataConnector.getRecordByPrimaryKey(this.resourceConfig, primaryKey); + const accessError = consumeResourceAccessGrant( + this.options[RESOURCE_ACCESS_GRANT], this.adminUser, this.resourceConfig, + AllowedActionsEnum.delete, this.options.record ?? currentRecord, primaryKey, + ) ? null : await this.accessError('delete', { ...this.meta, record: currentRecord, pk: primaryKey }); + if (accessError) { + throw new Error(accessError); + } + + if (!currentRecord) { + return false; + } + const scopedRecord = await this.findScopedRecord(primaryKey, currentRecord); if (!scopedRecord) { return false; } const record = this.options.record ?? scopedRecord; - const accessError = await this.accessError('delete', { ...this.meta, record: scopedRecord, pk: primaryKey }); - if (accessError) { - throw new Error(accessError); - } - const { error } = await this.executors.delete({ resource: this.resourceConfig, recordId: primaryKey, diff --git a/adminforth/modules/utils.ts b/adminforth/modules/utils.ts index 8a84b130b..749c31826 100644 --- a/adminforth/modules/utils.ts +++ b/adminforth/modules/utils.ts @@ -546,11 +546,15 @@ export async function cascadeChildrenDelete( primaryKey: string, context: {adminUser: any, response: any}, adminforth: IAdminForth, - deleteWithHooks: (params: { + deleteWithHooks?: (params: { resource: AdminForthResource, recordId: any, record: any, adminUser: any, response: any, }) => Promise<{ error?: string }>, ): Promise<{ error: string | null }> { const { adminUser, response } = context; + // Preserve the public four-argument contract. Core callers inject the non-deprecated executor; + // legacy external callers retain the hook-aware behavior they had before that executor existed. + const deleteChildWithHooks = deleteWithHooks + ?? ((params) => adminforth.deleteResourceRecord(params)); const childResources = adminforth.config.resources.filter(r =>r.columns.some(c => c.foreignResource?.resourceId === resource.resourceId)); @@ -574,12 +578,12 @@ export async function cascadeChildrenDelete( for (const childRecord of childRecords) { // Grandchildren first, then the child itself. const childResult = await cascadeChildrenDelete( - childRes, childRecordId(childRecord), context, adminforth, deleteWithHooks, + childRes, childRecordId(childRecord), context, adminforth, deleteChildWithHooks, ); if (childResult?.error) { return childResult; } - const deleteChild = await deleteWithHooks({ + const deleteChild = await deleteChildWithHooks({ resource: childRes, record: childRecord, adminUser, recordId: childRecordId(childRecord), response, }); if (deleteChild.error) { diff --git a/adminforth/types/Back.ts b/adminforth/types/Back.ts index e658717c6..fcde20b75 100644 --- a/adminforth/types/Back.ts +++ b/adminforth/types/Back.ts @@ -2533,8 +2533,9 @@ export interface AdminForthBulkAction extends AdminForthBulkActionCommon { * Callback which will be called on backend when user clicks on action button. * It should return Promise which will be resolved when action is done. */ - action: ({ resource, selectedIds, adminUser, response, tr }: { - resource: AdminForthResource, selectedIds: Array, adminUser: AdminUser, response: IAdminForthHttpResponse, tr: ITranslateFunction + action: ({ resource, selectedIds, adminUser, response, tr, extra }: { + resource: AdminForthResource, selectedIds: Array, adminUser: AdminUser, response: IAdminForthHttpResponse, tr: ITranslateFunction, + extra?: HttpExtra, }) => Promise<{ ok: boolean, error?: string, successMessage?: string }>, /** diff --git a/tests/jest_tests/operational_resource_scope.test.ts b/tests/jest_tests/operational_resource_scope.test.ts index 04c72579c..1b07a1be2 100644 --- a/tests/jest_tests/operational_resource_scope.test.ts +++ b/tests/jest_tests/operational_resource_scope.test.ts @@ -1,6 +1,7 @@ import OperationalResource from '../../adminforth/modules/operationalResource.js'; import UserScopedResource from '../../adminforth/modules/userScopedResource.js'; import AdminForthRestAPI from '../../adminforth/modules/restApi.js'; +import AdminForth from '../../adminforth/index.js'; import { authorizeResourceOperation, RESOURCE_ACCESS_GRANT } from '../../adminforth/modules/resourceAccess.js'; import { ActionCheckSource, AllowedActionsEnum } from '../../adminforth/types/Common.js'; @@ -114,8 +115,11 @@ function setup(resourceId = 'users') { return [{ total: 1 }]; }, validateAndNormalizeInputFilters: (filter) => filter, - getRecordByPrimaryKey: async () => { + getRecordByPrimaryKey: async (_resource, recordId) => { calls.connectorGetByPk++; + if (recordId === 'missing') { + return null; + } return { id: 1, name: 'Old name', readonly: 'old' }; }, getPrimaryKey: () => 'id', @@ -185,6 +189,42 @@ describe('OperationalResource access tiers', () => { expect(calls).toMatchObject({ connectorCreate: 1, connectorUpdate: 1 }); }); + it('preserves validation before editReadonly stripping in the deprecated update executor', async () => { + let connectorUpdates = 0; + const admin = Object.create(AdminForth.prototype) as any; + admin.warnedDeprecatedResourceMutations = new Set(); + admin.connectors = { + main: { + updateRecord: async () => { + connectorUpdates++; + return { ok: true }; + }, + }, + }; + const resource = { + resourceId: 'users', + dataSource: 'main', + columns: [{ + name: 'readonly', + editReadonly: true, + validation: [{ regExp: '^valid$', message: 'readonly validation failed' }], + }], + hooks: {}, + } as any; + const updates = { readonly: 'invalid' }; + + await expect(admin.updateResourceRecord({ + resource, + recordId: 1, + updates, + oldRecord: { readonly: 'old' }, + adminUser: {}, + })).resolves.toEqual({ error: 'readonly validation failed' }); + + expect(updates).toEqual({ readonly: 'invalid' }); + expect(connectorUpdates).toBe(0); + }); + it('rejects editReadonly for asUser()', async () => { const { calls, resource } = setup(); @@ -229,6 +269,16 @@ describe('OperationalResource access tiers', () => { expect(calls).toMatchObject({ acl: 1, connectorDelete: 0 }); }); + it('does not reveal whether a denied update or delete target exists', async () => { + const { calls, resource } = setup(); + const scoped = resource.asUser({} as any, { meta: { allowed: false } }); + + await expect(scoped.update('missing', { name: 'Jane' })) + .resolves.toMatchObject({ ok: false, error: 'Action is not allowed' }); + await expect(scoped.delete('missing')).rejects.toThrow('Action is not allowed'); + expect(calls).toMatchObject({ acl: 2, beforeList: 0, updateExecutor: 0, connectorDelete: 0 }); + }); + it('passes the caller delete snapshot to hooks after the scoped lookup', async () => { const { seenWrites, resource } = setup(); const record = { id: 1, name: 'Earlier snapshot' }; @@ -240,7 +290,7 @@ describe('OperationalResource access tiers', () => { await expect(resource.asUser({} as any, { meta: { allowed: true }, record }).delete(1)) .resolves.toBe(true); - expect(aclRecord.name).toBe('John'); + expect(aclRecord.name).toBe('Old name'); expect(seenWrites.delete).toEqual({ record, cascadeChildren: true }); }); @@ -257,12 +307,12 @@ describe('OperationalResource access tiers', () => { .update(1, { name: 'Jane' }); expect(updated).toMatchObject({ ok: true }); - expect(aclRecord.name).toBe('John'); + expect(aclRecord.name).toBe('Old name'); expect(seenWrites.update.oldRecord).toEqual({ id: 1, name: 'Earlier snapshot' }); - expect(calls).toMatchObject({ connectorGetByPk: 0, updateExecutor: 1, beforeList: 1 }); + expect(calls).toMatchObject({ connectorGetByPk: 1, updateExecutor: 1, beforeList: 1 }); }); - it('row-scopes updates and deletes before loading the target record', async () => { + it('row-scopes updates and deletes before mutating the target record', async () => { const { calls, seenFilters, resource } = setup(); resource.resourceConfig.hooks.list.beforeDatasourceRequest = [async ({ query }) => { calls.beforeList++; @@ -281,6 +331,34 @@ describe('OperationalResource access tiers', () => { expect(calls).toMatchObject({ beforeList: 2, updateExecutor: 0, connectorDelete: 0 }); }); + it('gives mutation row-scope hooks list-shaped input with the original request context', async () => { + const { resource } = setup(); + let hookPayload: any; + resource.resourceConfig.hooks.list.beforeDatasourceRequest = [async (payload) => { + hookPayload = payload; + return { ok: true }; + }]; + const requestBody = { resourceId: 'users', recordId: 1, record: { name: 'Jane' } }; + + await resource.asUser({} as any, { + meta: { allowed: true }, + extra: { + body: requestBody, + query: { locale: 'en' }, + headers: { 'x-tenant': 't1' }, + cookies: [], + requestUrl: '/update_record', + response: {} as any, + }, + }).update(1, { name: 'Jane' }); + + expect(hookPayload.extra.headers).toEqual({ 'x-tenant': 't1' }); + expect(hookPayload.extra.query).toEqual({ locale: 'en' }); + expect(hookPayload.extra.body).toBe(hookPayload.query); + expect(hookPayload.extra.body).toMatchObject({ limit: 1, offset: 0, sort: [] }); + expect(hookPayload.extra.body).not.toBe(requestBody); + }); + it('keeps the requested primary key when a scope hook replaces its filters', async () => { const { calls, seenFilters, resource } = setup(); resource.resourceConfig.hooks.list.beforeDatasourceRequest = [async ({ query }) => { @@ -493,7 +571,7 @@ describe('OperationalResource access tiers', () => { const deniedResult = await denied.resource.asUser({} as any, { meta: { allowed: false } }).update(1, {}); expect(deniedResult).toMatchObject({ ok: false, error: 'Action is not allowed' }); - expect(denied.calls).toMatchObject({ beforeList: 1, updateExecutor: 0 }); + expect(denied.calls).toMatchObject({ beforeList: 0, updateExecutor: 0 }); const allowed = setup(); const allowedResult = await allowed.resource.asUser({} as any, { meta: { allowed: true } }).update(1, {}); diff --git a/tests/jest_tests/resource_bulk_delete_order.test.ts b/tests/jest_tests/resource_bulk_delete_order.test.ts index b580f4443..7c1cd6be3 100644 --- a/tests/jest_tests/resource_bulk_delete_order.test.ts +++ b/tests/jest_tests/resource_bulk_delete_order.test.ts @@ -1,5 +1,8 @@ import AdminForth from '../../adminforth/index.js'; import ConfigValidator from '../../adminforth/modules/configValidator.js'; +import OperationalResource from '../../adminforth/modules/operationalResource.js'; +import UserScopedResource from '../../adminforth/modules/userScopedResource.js'; +import { cascadeChildrenDelete } from '../../adminforth/modules/utils.js'; function setup(parentBeforeSave: () => Promise<{ ok: boolean; error?: string }>) { const events: string[] = []; @@ -8,6 +11,7 @@ function setup(parentBeforeSave: () => Promise<{ ok: boolean; error?: string }>) resourceId: 'parents', dataSource: 'main', columns: [{ name: 'id', primaryKey: true }], + options: { allowedActions: { delete: true } }, hooks: { delete: { beforeSave: [async () => { @@ -21,6 +25,7 @@ function setup(parentBeforeSave: () => Promise<{ ok: boolean; error?: string }>) }, }, } as any; + parent.dataSourceColumns = parent.columns; const child = { resourceId: 'children', dataSource: 'main', @@ -28,6 +33,7 @@ function setup(parentBeforeSave: () => Promise<{ ok: boolean; error?: string }>) { name: 'id', primaryKey: true }, { name: 'parent_id', foreignResource: { resourceId: 'parents', onDelete: 'cascade' } }, ], + options: { allowedActions: { delete: true } }, hooks: { delete: { beforeSave: [async () => { @@ -41,49 +47,45 @@ function setup(parentBeforeSave: () => Promise<{ ok: boolean; error?: string }>) }, }, } as any; + child.dataSourceColumns = child.columns; const admin = Object.create(AdminForth.prototype) as any; admin.config = { resources: [parent, child] }; admin.statuses = { dbDiscover: 'done' }; admin.warnedDeprecatedResourceMutations = new Set(); - admin.connectors = { - main: { - getRecordByPrimaryKey: async () => ({ id: 'p1' }), - deleteRecord: async ({ resource }) => { - events.push(resource.resourceId === 'parents' ? 'parent-delete' : 'child-delete'); - return true; - }, + const connector = { + validateAndNormalizeInputFilters: (filters: any) => filters, + getRecordByPrimaryKey: async (resource: any) => resource.resourceId === 'parents' + ? { id: 'p1' } + : { id: 'c1', parent_id: 'p1' }, + getData: async ({ resource }: any) => { + if (resource.resourceId === 'children') { + events.push('child-list'); + return { data: [{ id: 'c1', parent_id: 'p1' }], total: 1 }; + } + return { data: [{ id: 'p1' }], total: 1 }; + }, + deleteRecord: async ({ resource }: any) => { + events.push(resource.resourceId === 'parents' ? 'parent-delete' : 'child-delete'); + return true; }, }; - admin.operationalResources = { - children: { - list: async () => { - events.push('child-list'); - return [{ id: 'c1', parent_id: 'p1' }]; - }, + admin.connectors = { main: connector }; + const executors = { + create: (params: any) => admin.executeCreateResourceRecord(params), + update: (params: any) => admin.executeUpdateResourceRecord(params), + delete: (params: any, cascadeChildren?: boolean, bulkHooks?: boolean) => { + scopedDeleteCalls += 1; + return admin.executeDeleteResourceRecord(params, cascadeChildren, bulkHooks); }, }; - admin.resource = (resourceId: string) => { - if (resourceId === 'children') { - return admin.operationalResources.children; - } - return { - asUser: (adminUser: any, { response, bulkDeleteHooks }: any) => ({ - delete: async (recordId: string) => { - scopedDeleteCalls += 1; - const result = await admin.executeDeleteResourceRecord({ - resource: parent, - recordId, - record: await admin.connectors.main.getRecordByPrimaryKey(parent, recordId), - adminUser, - response, - }, true, bulkDeleteHooks); - if (result.error) { - throw new Error(result.error); - } - return true; - }, - }), - }; + const operationalResource = (resource: any) => new OperationalResource( + connector as any, + resource, + (data, adminUser, options) => new UserScopedResource(data, admin, executors, adminUser, options), + ); + admin.operationalResources = { + parents: operationalResource(parent), + children: operationalResource(child), }; const actions = new ConfigValidator(admin, {} as any) .validateAndNormalizeBulkActions({ options: { bulkActions: [] } } as any, parent, []); @@ -93,10 +95,11 @@ function setup(parentBeforeSave: () => Promise<{ ok: boolean; error?: string }>) events, parent, scopedDeleteCalls: () => scopedDeleteCalls, - deleteChecked: () => deleteChecked.action({ + deleteChecked: (extra?: any) => deleteChecked.action({ selectedIds: ['p1'], adminUser: {} as any, response: {} as any, + extra, } as any), }; } @@ -152,4 +155,62 @@ describe('default bulk delete', () => { expect(events).toContain('parent-after-second'); expect(events).toContain('parent-delete'); }); + + it('passes bulk request context to the row-scope hook using a list-shaped body', async () => { + const { parent, deleteChecked } = setup(async () => ({ ok: true })); + let hookPayload: any; + parent.hooks.list = { + beforeDatasourceRequest: [async (payload: any) => { + hookPayload = payload; + return { ok: true }; + }], + }; + const requestBody = { resourceId: 'parents', actionId: 'delete', recordIds: ['p1'] }; + + await expect(deleteChecked({ + body: requestBody, + query: {}, + headers: { 'x-tenant': 't1' }, + cookies: [], + requestUrl: '/start_bulk_action', + response: {}, + })).resolves.toMatchObject({ ok: true }); + + expect(hookPayload.extra.headers).toEqual({ 'x-tenant': 't1' }); + expect(hookPayload.extra.body).toBe(hookPayload.query); + expect(hookPayload.extra.body).not.toBe(requestBody); + }); +}); + +it('keeps the legacy four-argument cascade helper hook-aware', async () => { + const parent = { + resourceId: 'parents', + columns: [{ name: 'id', primaryKey: true }], + } as any; + const child = { + resourceId: 'children', + columns: [ + { name: 'id', primaryKey: true }, + { name: 'parent_id', foreignResource: { resourceId: 'parents', onDelete: 'cascade' } }, + ], + } as any; + const deleted: any[] = []; + const adminforth = { + config: { resources: [parent, child] }, + resource: () => ({ list: async () => [{ id: 'c1', parent_id: 'p1' }] }), + deleteResourceRecord: async (params: any) => { + deleted.push(params); + return { error: null }; + }, + } as any; + + await expect(cascadeChildrenDelete( + parent, + 'p1', + { adminUser: { id: 'admin' }, response: {} }, + adminforth, + )).resolves.toEqual({ error: null }); + + expect(deleted).toHaveLength(1); + expect(deleted[0]).toMatchObject({ resource: child, recordId: 'c1' }); }); diff --git a/tests/jest_tests/rest_resource_delete.test.ts b/tests/jest_tests/rest_resource_delete.test.ts index 69f3b6ba1..2fc957fa1 100644 --- a/tests/jest_tests/rest_resource_delete.test.ts +++ b/tests/jest_tests/rest_resource_delete.test.ts @@ -2,7 +2,11 @@ import AdminForthRestAPI from '../../adminforth/modules/restApi.js'; it('does not report a successful REST deletion when the row is outside user scope', async () => { const endpoints: Record = {}; - const resource = { resourceId: 'items', dataSource: 'main' }; + const resource = { + resourceId: 'items', + dataSource: 'main', + options: { allowedActions: { delete: true } }, + }; const adminforth = { config: { auth: { rateLimit: [] }, resources: [resource] }, activatedPlugins: [], @@ -25,3 +29,51 @@ it('does not report a successful REST deletion when the row is outside user scop expect(result).toEqual({ error: 'Record with item-1 not found' }); }); + +it('checks delete permission before reporting that a REST record is missing', async () => { + const endpoints: Record = {}; + const resource = { + resourceId: 'items', + dataSource: 'main', + options: { allowedActions: { delete: false } }, + }; + const adminforth = { + config: { auth: { rateLimit: [] }, resources: [resource] }, + activatedPlugins: [], + connectors: { main: { getRecordByPrimaryKey: async () => null } }, + } as any; + new AdminForthRestAPI(adminforth).registerEndpoints({ + endpoint: (endpoint: any) => { endpoints[endpoint.path] = endpoint; }, + } as any); + + const result = await endpoints['/delete_record'].handler({ + body: { resourceId: 'items', primaryKey: 'missing' }, + adminUser: {}, query: {}, headers: {}, cookies: [], requestUrl: '', response: {}, + }); + + expect(result).toEqual({ error: 'Action is not allowed' }); +}); + +it('checks edit permission before reporting that a REST record is missing', async () => { + const endpoints: Record = {}; + const resource = { + resourceId: 'items', + dataSource: 'main', + options: { allowedActions: { edit: false } }, + }; + const adminforth = { + config: { auth: { rateLimit: [] }, resources: [resource] }, + activatedPlugins: [], + connectors: { main: { getRecordByPrimaryKey: async () => null } }, + } as any; + new AdminForthRestAPI(adminforth).registerEndpoints({ + endpoint: (endpoint: any) => { endpoints[endpoint.path] = endpoint; }, + } as any); + + const result = await endpoints['/update_record'].handler({ + body: { resourceId: 'items', recordId: 'missing', record: { name: 'Jane' } }, + adminUser: {}, query: {}, headers: {}, cookies: [], requestUrl: '', response: {}, + }); + + expect(result).toEqual({ error: 'Action is not allowed' }); +});