Skip to content

Commit e233725

Browse files
committed
fix: reject filters on backendOnly columns in get_resource_data and get_resource_foreign_data
1 parent d6741bf commit e233725

2 files changed

Lines changed: 198 additions & 23 deletions

File tree

‎adminforth/modules/restApi.ts‎

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,18 @@ function collectFilterFields(filters: any, fields: Set<string> = new Set()): Set
298298
return fields;
299299
}
300300

301+
async function rejectBackendOnlyFilters(
302+
filters: any,
303+
ctx: Parameters<typeof isBackendOnly>[1]
304+
): Promise<{ error: string } | undefined> {
305+
for (const fieldName of collectFilterFields(filters)) {
306+
const column = ctx.resource.columns.find((col) => col.name === fieldName);
307+
if (column && await isBackendOnly(column, ctx)) {
308+
return { error: `Filter: column "${fieldName}" cannot be used (backendOnly is true).` };
309+
}
310+
}
311+
}
312+
301313
function createErrorOrSuccessSchema(successSchema: AnySchemaObject): AnySchemaObject {
302314
return {
303315
anyOf: [
@@ -1461,15 +1473,23 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI {
14611473
}
14621474
}
14631475

1464-
const { allowedActions } = await interpretResource(
1476+
const columnCtx = {
14651477
adminUser,
14661478
resource,
14671479
meta,
1468-
{
1480+
source: {
14691481
'show': ActionCheckSource.ShowRequest,
14701482
'list': ActionCheckSource.ListRequest,
14711483
'edit': ActionCheckSource.EditLoadRequest,
14721484
}[source],
1485+
adminforth: this.adminforth,
1486+
};
1487+
1488+
const { allowedActions } = await interpretResource(
1489+
adminUser,
1490+
resource,
1491+
meta,
1492+
columnCtx.source,
14731493
this.adminforth
14741494
);
14751495

@@ -1482,6 +1502,10 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI {
14821502
if (!allowed) {
14831503
return { error };
14841504
}
1505+
const backendOnlyFilterError = await rejectBackendOnlyFilters(body.filters, columnCtx);
1506+
if (backendOnlyFilterError) {
1507+
return backendOnlyFilterError;
1508+
}
14851509
if (this.checkAbortSignal(abortSignal)) { return { error: 'Request aborted' }; }
14861510
const hookSource = {
14871511
'show': 'show',
@@ -1723,26 +1747,14 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI {
17231747

17241748
// remove all columns which are not defined in resources, or defined but backendOnly
17251749
{
1726-
const ctx = {
1727-
adminUser,
1728-
resource,
1729-
meta,
1730-
source: {
1731-
show: ActionCheckSource.ShowRequest,
1732-
list: ActionCheckSource.ListRequest,
1733-
edit: ActionCheckSource.EditLoadRequest,
1734-
}[source],
1735-
adminforth: this.adminforth,
1736-
};
1737-
17381750
for (const [i, item] of data.data.entries()) {
17391751
const rawItem = rawItems[i];
17401752
for (const key of Object.keys(item)) {
17411753
if (key === '_primaryKeyValue') {
17421754
continue;
17431755
}
17441756
const col = resource.columns.find((c) => c.name === key);
1745-
const bo = col ? await isBackendOnly(col, ctx) : true;
1757+
const bo = col ? await isBackendOnly(col, columnCtx) : true;
17461758
if (!col || bo) {
17471759
delete item[key];
17481760
delete rawItem[key];
@@ -1976,14 +1988,9 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI {
19761988
}
19771989
}
19781990

1979-
// filters are not returned to the caller, but combined with an aggregation they turn
1980-
// into an oracle which reads a value out one comparison at a time, so backendOnly
1981-
// columns are off limits here as well
1982-
for (const fieldName of collectFilterFields(filters)) {
1983-
const column = resource.columns.find((col) => col.name === fieldName);
1984-
if (column && await isBackendOnly(column, columnCtx)) {
1985-
return { error: `Filter: column "${fieldName}" cannot be used (backendOnly is true).` };
1986-
}
1991+
const backendOnlyFilterError = await rejectBackendOnlyFilters(filters, columnCtx);
1992+
if (backendOnlyFilterError) {
1993+
return backendOnlyFilterError;
19871994
}
19881995

19891996
// normalize filters same way as get_resource_data
@@ -2075,6 +2082,20 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI {
20752082
const targetResourceIds = columnConfig.foreignResource.resourceId ? [columnConfig.foreignResource.resourceId] : columnConfig.foreignResource.polymorphicResources.filter(pr => pr.resourceId !== null).map((pr) => pr.resourceId);
20762083
const targetResources = targetResourceIds.map((trId) => this.adminforth.config.resources.find((res) => res.resourceId == trId));
20772084

2085+
// request filters are applied to the target resources, so their columns are what has to be readable
2086+
for (const targetResource of targetResources) {
2087+
const backendOnlyFilterError = await rejectBackendOnlyFilters(body.filters, {
2088+
adminUser,
2089+
resource: targetResource,
2090+
meta,
2091+
source: ActionCheckSource.ListRequest,
2092+
adminforth: this.adminforth,
2093+
});
2094+
if (backendOnlyFilterError) {
2095+
return backendOnlyFilterError;
2096+
}
2097+
}
2098+
20782099
const responses = (await Promise.all(
20792100
targetResources.map(async (targetResource) => {
20802101
return new Promise(async (resolve) => {
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
import { mkdtempSync, rmSync } from 'node:fs';
2+
import { createRequire } from 'node:module';
3+
import os from 'node:os';
4+
import path from 'node:path';
5+
import { fileURLToPath, pathToFileURL } from 'node:url';
6+
7+
const currentDir = path.dirname(fileURLToPath(import.meta.url));
8+
const applicationDir = path.resolve(currentDir, '../application');
9+
10+
// resolves to the BUILT package (adminforth/dist) the test application runs on;
11+
// rebuild it (`npx tsc` in adminforth/) after editing adminforth sources or the assertions below test stale code
12+
const require = createRequire(import.meta.url);
13+
// jest's resolver does not know the `node:sqlite` builtin, so it is required directly
14+
const { DatabaseSync } = require('node:sqlite');
15+
const adminforthEntry = require.resolve('adminforth', { paths: [applicationDir] });
16+
const { default: AdminForth } = await import(pathToFileURL(adminforthEntry).href);
17+
18+
const fixtureDir = mkdtempSync(path.join(os.tmpdir(), 'af-backend-only-filters-'));
19+
const dbPath = path.join(fixtureDir, 'db.sqlite');
20+
const db = new DatabaseSync(dbPath);
21+
db.exec(`
22+
CREATE TABLE users (id INTEGER PRIMARY KEY, email TEXT NOT NULL, password_hash TEXT NOT NULL, last_ip TEXT NOT NULL);
23+
INSERT INTO users VALUES (1, 'a@example.test', 'hash-a', '10.0.0.1');
24+
INSERT INTO users VALUES (2, 'b@example.test', 'hash-b', '10.0.0.2');
25+
CREATE TABLE posts (id INTEGER PRIMARY KEY, user_id INTEGER NOT NULL);
26+
INSERT INTO posts VALUES (1, 1);
27+
`);
28+
db.close();
29+
30+
process.env.ADMINFORTH_SECRET ??= 'x'.repeat(64);
31+
32+
delete (globalThis as any).adminforth;
33+
const app = new AdminForth({
34+
baseUrl: '',
35+
auth: {
36+
usersResourceId: 'users',
37+
usernameField: 'email',
38+
passwordHashField: 'password_hash',
39+
},
40+
dataSources: [{ id: 'sqlite', url: `sqlite://${dbPath}` }],
41+
resources: [
42+
{
43+
resourceId: 'users',
44+
table: 'users',
45+
dataSource: 'sqlite',
46+
columns: [
47+
{ name: 'id', primaryKey: true },
48+
{ name: 'email' },
49+
{ name: 'password_hash', backendOnly: true },
50+
// hidden by role, like PII which only some admins may read
51+
{ name: 'last_ip', backendOnly: ({ adminUser }) => adminUser.dbUser.role !== 'superadmin' },
52+
],
53+
hooks: {
54+
list: {
55+
// server code may still filter by hidden columns
56+
beforeDatasourceRequest: async ({ query }) => {
57+
if (query.hookFilter) {
58+
query.filters.push({ field: 'password_hash', operator: 'eq', value: 'hash-b' });
59+
}
60+
return { ok: true };
61+
},
62+
},
63+
},
64+
},
65+
{
66+
resourceId: 'posts',
67+
table: 'posts',
68+
dataSource: 'sqlite',
69+
columns: [
70+
{ name: 'id', primaryKey: true },
71+
{ name: 'user_id', foreignResource: { resourceId: 'users' } },
72+
],
73+
},
74+
],
75+
menu: [{ label: 'Users', resourceId: 'users' }],
76+
});
77+
await app.discoverDatabases();
78+
79+
const endpoints: any[] = [];
80+
app.restApi.registerEndpoints({ endpoint: (config: any) => endpoints.push(config) });
81+
82+
afterAll(() => {
83+
rmSync(fixtureDir, { recursive: true, force: true });
84+
});
85+
86+
function call(endpointPath: string, body: Record<string, unknown>, role = 'admin') {
87+
const { handler } = endpoints.find((endpoint) => endpoint.path === endpointPath);
88+
return handler({
89+
body,
90+
adminUser: { pk: '1', username: 'a@example.test', dbUser: { role } },
91+
headers: {},
92+
query: {},
93+
cookies: {},
94+
requestUrl: endpointPath,
95+
abortSignal: new AbortController().signal,
96+
});
97+
}
98+
99+
const listUsers = (filters: unknown, extra: Record<string, unknown> = {}, role?: string) =>
100+
call('/get_resource_data', { resourceId: 'users', source: 'list', limit: 10, offset: 0, sort: [], filters, ...extra }, role);
101+
102+
const passwordHashFilter = { field: 'password_hash', operator: 'ilike', value: 'hash-a%' };
103+
const passwordHashError = { error: 'Filter: column "password_hash" cannot be used (backendOnly is true).' };
104+
105+
describe('filters on backendOnly columns', () => {
106+
it.each([
107+
['top level', [passwordHashFilter]],
108+
['single filter object', passwordHashFilter],
109+
['nested in or', { operator: 'and', subFilters: [{ operator: 'or', subFilters: [{ field: 'email', operator: 'eq', value: 'x' }, passwordHashFilter] }] }],
110+
['compared as rightField', [{ field: 'email', operator: 'eq', rightField: 'password_hash' }]],
111+
])('get_resource_data rejects them %s', async (_, filters) => {
112+
expect(await listUsers(filters)).toEqual(passwordHashError);
113+
});
114+
115+
it('get_resource_data rejects them for show source', async () => {
116+
const result = await call('/get_resource_data', {
117+
resourceId: 'users', source: 'show', limit: 1, offset: 0, filters: [{ field: 'id', operator: 'eq', value: 1 }, passwordHashFilter],
118+
});
119+
expect(result).toEqual(passwordHashError);
120+
});
121+
122+
it('get_resource_data keeps filters on readable columns', async () => {
123+
const result = await listUsers([{ field: 'email', operator: 'eq', value: 'b@example.test' }]);
124+
expect(result.data).toEqual([expect.objectContaining({ id: 2, email: 'b@example.test' })]);
125+
});
126+
127+
it('get_resource_data resolves role based backendOnly per user', async () => {
128+
const lastIpFilter = [{ field: 'last_ip', operator: 'eq', value: '10.0.0.2' }];
129+
expect(await listUsers(lastIpFilter)).toEqual({ error: 'Filter: column "last_ip" cannot be used (backendOnly is true).' });
130+
131+
const result = await listUsers(lastIpFilter, {}, 'superadmin');
132+
expect(result.data).toEqual([expect.objectContaining({ id: 2, last_ip: '10.0.0.2' })]);
133+
});
134+
135+
it('get_resource_data applies backendOnly filters added by hooks', async () => {
136+
const result = await listUsers([], { hookFilter: true });
137+
expect(result.data).toEqual([expect.objectContaining({ id: 2 })]);
138+
expect(result.data[0]).not.toHaveProperty('password_hash');
139+
});
140+
141+
it('get_resource_foreign_data rejects them on the target resource', async () => {
142+
const result = await call('/get_resource_foreign_data', {
143+
resourceId: 'posts', column: 'user_id', limit: 10, offset: 0, filters: [passwordHashFilter],
144+
});
145+
expect(result).toEqual(passwordHashError);
146+
});
147+
148+
it('aggregate rejects them', async () => {
149+
const result = await call('/aggregate', {
150+
resourceId: 'users', aggregations: { total: { operation: 'count' } }, filters: [passwordHashFilter],
151+
});
152+
expect(result).toEqual(passwordHashError);
153+
});
154+
});

0 commit comments

Comments
 (0)