Skip to content

Commit dfc7fc0

Browse files
committed
Merge branch 'main' into backend_typescript_improvements
2 parents d7db8f9 + 4d3e905 commit dfc7fc0

7 files changed

Lines changed: 515 additions & 2 deletions

File tree

backend/src/entities/visualizations/panel/use-cases/execute-panel.use.case.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,11 @@ import { IGlobalDatabaseContext } from '../../../../common/application/global-da
66
import { BaseType } from '../../../../common/data-injection.tokens.js';
77
import { Messages } from '../../../../exceptions/text/messages.js';
88
import { isConnectionTypeAgent } from '../../../../helpers/is-connection-entity-agent.js';
9+
import { CedarAction } from '../../../cedar-authorization/cedar-action-map.js';
10+
import { CedarAuthorizationService } from '../../../cedar-authorization/cedar-authorization.service.js';
911
import { ExecuteSavedDbQueryDs } from '../data-structures/execute-saved-db-query.ds.js';
1012
import { ExecuteSavedDbQueryResultDto } from '../dto/execute-saved-db-query-result.dto.js';
13+
import { assertUserCanReadQueryTables } from '../utils/assert-query-tables-readable.util.js';
1114
import { validateQuerySafety } from '../utils/check-query-is-safe.util.js';
1215
import { IExecuteSavedDbQuery } from './panel-use-cases.interface.js';
1316

@@ -19,6 +22,7 @@ export class ExecuteSavedDbQueryUseCase
1922
constructor(
2023
@Inject(BaseType.GLOBAL_DB_CONTEXT)
2124
protected _dbContext: IGlobalDatabaseContext,
25+
private readonly cedarAuthService: CedarAuthorizationService,
2226
) {
2327
super();
2428
}
@@ -43,12 +47,27 @@ export class ExecuteSavedDbQueryUseCase
4347

4448
validateQuerySafety(foundQuery.query_text, foundConnection.type as ConnectionTypesEnum);
4549

50+
const dao = getDataAccessObject(foundConnection);
51+
52+
await assertUserCanReadQueryTables({
53+
query: foundQuery.query_text,
54+
connectionType: foundConnection.type as ConnectionTypesEnum,
55+
connectionId,
56+
validateTableRead: (referencedTableName) =>
57+
this.cedarAuthService.validate({
58+
userId,
59+
action: CedarAction.TableRead,
60+
connectionId,
61+
tableName: referencedTableName,
62+
}),
63+
listAllTableNames: async () => (await dao.getTablesFromDB()).map((table) => table.tableName),
64+
});
65+
4666
let userEmail: string | null = null;
4767
if (isConnectionTypeAgent(foundConnection.type)) {
4868
userEmail = await this._dbContext.userRepository.getUserEmailOrReturnNull(userId);
4969
}
5070

51-
const dao = getDataAccessObject(foundConnection);
5271
const startTime = Date.now();
5372

5473
const executionResult = await dao.executeRawQuery(foundQuery.query_text, tableName, userEmail);

backend/src/entities/visualizations/panel/use-cases/test-db-query.use.case.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,11 @@ import { IGlobalDatabaseContext } from '../../../../common/application/global-da
66
import { BaseType } from '../../../../common/data-injection.tokens.js';
77
import { Messages } from '../../../../exceptions/text/messages.js';
88
import { isConnectionTypeAgent } from '../../../../helpers/is-connection-entity-agent.js';
9+
import { CedarAction } from '../../../cedar-authorization/cedar-action-map.js';
10+
import { CedarAuthorizationService } from '../../../cedar-authorization/cedar-authorization.service.js';
911
import { TestDbQueryDs } from '../data-structures/test-db-query.ds.js';
1012
import { TestDbQueryResultDto } from '../dto/test-db-query-result.dto.js';
13+
import { assertUserCanReadQueryTables } from '../utils/assert-query-tables-readable.util.js';
1114
import { validateQuerySafety } from '../utils/check-query-is-safe.util.js';
1215
import { ITestDbQuery } from './panel-use-cases.interface.js';
1316

@@ -16,6 +19,7 @@ export class TestDbQueryUseCase extends AbstractUseCase<TestDbQueryDs, TestDbQue
1619
constructor(
1720
@Inject(BaseType.GLOBAL_DB_CONTEXT)
1821
protected _dbContext: IGlobalDatabaseContext,
22+
private readonly cedarAuthService: CedarAuthorizationService,
1923
) {
2024
super();
2125
}
@@ -34,12 +38,27 @@ export class TestDbQueryUseCase extends AbstractUseCase<TestDbQueryDs, TestDbQue
3438

3539
validateQuerySafety(query_text, foundConnection.type as ConnectionTypesEnum);
3640

41+
const dao = getDataAccessObject(foundConnection);
42+
43+
await assertUserCanReadQueryTables({
44+
query: query_text,
45+
connectionType: foundConnection.type as ConnectionTypesEnum,
46+
connectionId,
47+
validateTableRead: (referencedTableName) =>
48+
this.cedarAuthService.validate({
49+
userId,
50+
action: CedarAction.TableRead,
51+
connectionId,
52+
tableName: referencedTableName,
53+
}),
54+
listAllTableNames: async () => (await dao.getTablesFromDB()).map((table) => table.tableName),
55+
});
56+
3757
let userEmail: string | null = null;
3858
if (isConnectionTypeAgent(foundConnection.type)) {
3959
userEmail = await this._dbContext.userRepository.getUserEmailOrReturnNull(userId);
4060
}
4161

42-
const dao = getDataAccessObject(foundConnection);
4362
const startTime = Date.now();
4463

4564
const executionResult = await dao.executeRawQuery(query_text, tableName, userEmail);
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { ForbiddenException } from '@nestjs/common';
2+
import { ConnectionTypesEnum } from '@rocketadmin/shared-code/dist/src/shared/enums/connection-types-enum.js';
3+
import { Messages } from '../../../../exceptions/text/messages.js';
4+
import { slackPostMessage } from '../../../../helpers/slack/slack-post-message.js';
5+
import { collectQueryTables } from './collect-query-tables.util.js';
6+
7+
interface AssertQueryTablesReadableParams {
8+
query: string;
9+
connectionType: ConnectionTypesEnum;
10+
connectionId: string;
11+
/** Resolves to `true` when the user is allowed to read the given table (table:read). */
12+
validateTableRead: (tableName: string) => Promise<boolean>;
13+
/** Lists every table in the connection; used for the all-tables fallback check. */
14+
listAllTableNames: () => Promise<Array<string>>;
15+
}
16+
17+
/**
18+
* Guards a raw read-only query against table-level read permissions: the user must have read access
19+
* to every table the query touches.
20+
*
21+
* When the exact set of tables cannot be resolved (non-SQL connection, parse failure, or a statement
22+
* that resolves to no concrete table), we cannot trust the query to be harmless, so we fall back to
23+
* requiring read permission on EVERY table in the connection. This guarantees a user can never read
24+
* data from a table they lack permission on through this endpoint, while still letting users with
25+
* full read access run such queries.
26+
*/
27+
export async function assertUserCanReadQueryTables(params: AssertQueryTablesReadableParams): Promise<void> {
28+
const { query, connectionType, connectionId, validateTableRead, listAllTableNames } = params;
29+
30+
const collected = collectQueryTables(query, connectionType);
31+
32+
let tablesToCheck: Array<string>;
33+
if (collected.kind === 'tables') {
34+
tablesToCheck = collected.tables;
35+
} else {
36+
slackPostMessage(
37+
`Saved-query permission check could not resolve referenced tables for connection ${connectionId} ` +
38+
`(reason: ${collected.reason}); falling back to all-tables read check. Query: ${query}`,
39+
);
40+
tablesToCheck = await listAllTableNames();
41+
}
42+
43+
for (const tableName of tablesToCheck) {
44+
const allowed = await validateTableRead(tableName);
45+
if (!allowed) {
46+
throw new ForbiddenException(Messages.NO_READ_PERMISSION_FOR_TABLE(tableName));
47+
}
48+
}
49+
}
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import { ConnectionTypesEnum } from '@rocketadmin/shared-code/dist/src/shared/enums/connection-types-enum.js';
2+
import sqlParser from 'node-sql-parser';
3+
import { connectionTypeToParserDialect } from '../../../table-schema/utils/assert-dialect-supported.js';
4+
5+
const { Parser } = sqlParser;
6+
7+
// Connection types that node-sql-parser cannot analyze. Their queries are not SQL, so we never
8+
// attempt to parse them and instead let the caller fall back to the all-tables permission check.
9+
const NON_SQL_CONNECTION_TYPES: ReadonlySet<ConnectionTypesEnum> = new Set([
10+
ConnectionTypesEnum.mongodb,
11+
ConnectionTypesEnum.agent_mongodb,
12+
ConnectionTypesEnum.elasticsearch,
13+
ConnectionTypesEnum.redis,
14+
ConnectionTypesEnum.agent_redis,
15+
ConnectionTypesEnum.dynamodb,
16+
]);
17+
18+
export type CollectQueryTablesResult =
19+
| { kind: 'tables'; tables: Array<string> }
20+
| { kind: 'indeterminate'; reason: string };
21+
22+
/**
23+
* Extracts the real tables a read-only query references, so the caller can verify the user has
24+
* read permission on each of them.
25+
*
26+
* Returns `{ kind: 'tables' }` only when a confident, non-empty list of concrete tables could be
27+
* resolved. In every other case — non-SQL connections, parse failures, or statements that resolve
28+
* to no concrete table (e.g. `SELECT 1`, `SHOW`, `DESCRIBE`) — it returns `{ kind: 'indeterminate' }`
29+
* and the caller must fall back to a stricter check rather than assume the query is harmless.
30+
*/
31+
export function collectQueryTables(query: string, connectionType: ConnectionTypesEnum): CollectQueryTablesResult {
32+
if (NON_SQL_CONNECTION_TYPES.has(connectionType)) {
33+
return { kind: 'indeterminate', reason: `non-SQL connection type "${connectionType}"` };
34+
}
35+
36+
const dialect = connectionTypeToParserDialect(connectionType);
37+
const parser = new Parser();
38+
39+
let rawTableList: Array<string>;
40+
let cteNames: Set<string>;
41+
try {
42+
rawTableList = parser.tableList(query, { database: dialect });
43+
cteNames = collectCteNames(parser, query, dialect);
44+
} catch (error) {
45+
return { kind: 'indeterminate', reason: `parse error: ${(error as Error).message}` };
46+
}
47+
48+
const tables = new Set<string>();
49+
for (const entry of rawTableList) {
50+
// node-sql-parser returns entries formatted as "{type}::{db}::{table}". We ignore the schema
51+
// ("db") segment since permissions are keyed on bare table names.
52+
const tableName = entry.split('::').pop();
53+
if (!tableName || tableName === 'null') {
54+
continue;
55+
}
56+
// Common Table Expressions are aliases for inline subqueries, not real tables; the subqueries'
57+
// underlying tables are already present in the list, so the CTE names must be dropped.
58+
if (cteNames.has(tableName.toLowerCase())) {
59+
continue;
60+
}
61+
tables.add(tableName);
62+
}
63+
64+
if (tables.size === 0) {
65+
return { kind: 'indeterminate', reason: 'query resolved to no concrete table' };
66+
}
67+
68+
return { kind: 'tables', tables: Array.from(tables) };
69+
}
70+
71+
function collectCteNames(parser: InstanceType<typeof Parser>, query: string, dialect: string): Set<string> {
72+
const names = new Set<string>();
73+
const ast = parser.astify(query, { database: dialect });
74+
const statements = Array.isArray(ast) ? ast : [ast];
75+
for (const statement of statements) {
76+
const withClause = (statement as { with?: unknown })?.with;
77+
if (!Array.isArray(withClause)) {
78+
continue;
79+
}
80+
for (const cte of withClause) {
81+
const name = extractCteName((cte as { name?: unknown })?.name);
82+
if (name) {
83+
names.add(name.toLowerCase());
84+
}
85+
}
86+
}
87+
return names;
88+
}
89+
90+
function extractCteName(nameNode: unknown): string | null {
91+
if (typeof nameNode === 'string') {
92+
return nameNode;
93+
}
94+
if (nameNode && typeof nameNode === 'object') {
95+
const value = (nameNode as { value?: unknown }).value;
96+
if (typeof value === 'string') {
97+
return value;
98+
}
99+
}
100+
return null;
101+
}

backend/src/exceptions/text/messages.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ export const Messages = {
199199
`There are no such fields: ${fields.join(', ')} - in the table "${tableName}"`,
200200
NO_SUCH_FIELD_IN_TABLE: (fieldName: string, tableName: string) =>
201201
`There is no such field: "${fieldName}" in the table "${tableName}"`,
202+
NO_READ_PERMISSION_FOR_TABLE: (tableName: string) => `You do not have read permission for table "${tableName}".`,
202203
NOT_ALLOWED_IN_THIS_MODE: 'This operation is not allowed in this mode',
203204
ORDERING_FIELD_INCORRECT: `Value of sorting order is incorrect. You can choose from values ${enumToString(
204205
QueryOrderingEnum,
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { ConnectionTypesEnum } from '@rocketadmin/shared-code/dist/src/shared/enums/connection-types-enum.js';
2+
import test from 'ava';
3+
import { collectQueryTables } from '../../../src/entities/visualizations/panel/utils/collect-query-tables.util.js';
4+
5+
function tablesOf(query: string, type = ConnectionTypesEnum.postgres): Array<string> {
6+
const result = collectQueryTables(query, type);
7+
if (result.kind !== 'tables') {
8+
throw new Error(`expected resolved tables, got indeterminate: ${result.reason}`);
9+
}
10+
return [...result.tables].sort();
11+
}
12+
13+
test('resolves a single table from a simple SELECT', (t) => {
14+
t.deepEqual(tablesOf('SELECT id, name FROM users'), ['users']);
15+
});
16+
17+
test('resolves all tables referenced in a JOIN', (t) => {
18+
t.deepEqual(tablesOf('SELECT * FROM users u JOIN orders o ON u.id = o.user_id'), ['orders', 'users']);
19+
});
20+
21+
test('resolves tables hidden inside a subquery', (t) => {
22+
t.deepEqual(tablesOf('SELECT * FROM users WHERE id IN (SELECT user_id FROM secret_payouts)'), [
23+
'secret_payouts',
24+
'users',
25+
]);
26+
});
27+
28+
test('resolves tables across a UNION', (t) => {
29+
t.deepEqual(tablesOf('SELECT id FROM a UNION SELECT id FROM b'), ['a', 'b']);
30+
});
31+
32+
test('excludes CTE aliases but keeps their underlying tables', (t) => {
33+
t.deepEqual(tablesOf('WITH cte AS (SELECT * FROM orders) SELECT * FROM cte JOIN users ON true'), ['orders', 'users']);
34+
});
35+
36+
test('strips the schema qualifier and keeps the bare table name', (t) => {
37+
t.deepEqual(tablesOf('SELECT * FROM analytics.events'), ['events']);
38+
});
39+
40+
test('treats a query with no concrete table as indeterminate', (t) => {
41+
const result = collectQueryTables('SELECT 1', ConnectionTypesEnum.postgres);
42+
t.is(result.kind, 'indeterminate');
43+
});
44+
45+
test('treats DESCRIBE as indeterminate (target table not resolvable)', (t) => {
46+
const result = collectQueryTables('DESCRIBE users', ConnectionTypesEnum.mysql);
47+
t.is(result.kind, 'indeterminate');
48+
});
49+
50+
test('treats unparseable SQL as indeterminate', (t) => {
51+
const result = collectQueryTables('EXPLAIN SELECT * FROM users', ConnectionTypesEnum.postgres);
52+
t.is(result.kind, 'indeterminate');
53+
});
54+
55+
test('treats non-SQL connection types as indeterminate without parsing', (t) => {
56+
const result = collectQueryTables('db.users.find({})', ConnectionTypesEnum.mongodb);
57+
t.is(result.kind, 'indeterminate');
58+
if (result.kind === 'indeterminate') {
59+
t.true(result.reason.includes('non-SQL'));
60+
}
61+
});
62+
63+
test('resolves tables from a multi-statement query', (t) => {
64+
t.deepEqual(tablesOf('SELECT * FROM a; SELECT * FROM b'), ['a', 'b']);
65+
});

0 commit comments

Comments
 (0)