Skip to content

Commit c43c080

Browse files
committed
fix(schema-compiler): fix DAP with query_rewrite and python config
1 parent 215e456 commit c43c080

File tree

7 files changed

+226
-37
lines changed

7 files changed

+226
-37
lines changed

packages/cubejs-api-gateway/src/gateway.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1196,18 +1196,20 @@ class ApiGateway {
11961196
currentQuery = this.parseMemberExpressionsInQuery(currentQuery);
11971197
}
11981198

1199-
let normalizedQuery = normalizeQuery(currentQuery, persistent);
1199+
const normalizedQuery = normalizeQuery(currentQuery, persistent);
1200+
let evaluatedQuery = normalizedQuery;
12001201

12011202
if (hasExpressionsInQuery) {
12021203
// We need to parse/eval all member expressions early as applyRowLevelSecurity
12031204
// needs to access the full SQL query in order to evaluate rules
1204-
normalizedQuery =
1205+
evaluatedQuery =
12051206
this.evalMemberExpressionsInQuery(normalizedQuery);
12061207
}
12071208

12081209
// First apply cube/view level security policies
12091210
const queryWithRlsFilters = await compilerApi.applyRowLevelSecurity(
12101211
normalizedQuery,
1212+
evaluatedQuery,
12111213
context
12121214
);
12131215
// Then apply user-supplied queryRewrite
@@ -1219,7 +1221,7 @@ class ApiGateway {
12191221
// applyRowLevelSecurity may add new filters which may contain raw member expressions
12201222
// if that's the case, we should run an extra pass of parsing here to make sure
12211223
// nothing breaks down the road
1222-
if (this.hasExpressionsInQuery(rewrittenQuery)) {
1224+
if (hasExpressionsInQuery || this.hasExpressionsInQuery(rewrittenQuery)) {
12231225
rewrittenQuery = this.parseMemberExpressionsInQuery(rewrittenQuery);
12241226
rewrittenQuery = this.evalMemberExpressionsInQuery(rewrittenQuery);
12251227
}

packages/cubejs-server-core/src/core/CompilerApi.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,15 +271,15 @@ export class CompilerApi {
271271
* - combining all filters for different roles with OR
272272
* - combining cube and view filters with AND
273273
*/
274-
async applyRowLevelSecurity(query, context) {
274+
async applyRowLevelSecurity(query, evaluatedQuery, context) {
275275
const compilers = await this.getCompilers({ requestId: context.requestId });
276276
const { cubeEvaluator } = compilers;
277277

278278
if (!cubeEvaluator.isRbacEnabled()) {
279279
return query;
280280
}
281281

282-
const queryCubes = await this.getCubesFromQuery(query, context);
282+
const queryCubes = await this.getCubesFromQuery(evaluatedQuery, context);
283283

284284
// We collect Cube and View filters separately because they have to be
285285
// applied in "two layers": first Cube filters, then View filters on top
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
# Cube configuration options: https://cube.dev/docs/config
2+
3+
from cube import config
4+
5+
6+
@config('context_to_roles')
7+
def context_to_roles(context):
8+
return context.get("securityContext", {}).get("auth", {}).get("roles", [])
9+
10+
11+
def extract_matching_dicts(data):
12+
matching_dicts = []
13+
keys = ['values', 'member', 'operator']
14+
15+
# Recursive function to traverse through the list or dictionary
16+
def traverse(element):
17+
if isinstance(element, dict):
18+
# Check if any of the specified keys are in the dictionary
19+
if any(key in element for key in keys):
20+
matching_dicts.append(element)
21+
# Traverse the dictionary values
22+
for value in element.values():
23+
traverse(value)
24+
elif isinstance(element, list):
25+
# Traverse the list items
26+
for item in element:
27+
traverse(item)
28+
29+
traverse(data)
30+
return matching_dicts
31+
32+
33+
@config('query_rewrite')
34+
def query_rewrite(query: dict, ctx: dict) -> dict:
35+
filters = extract_matching_dicts(query.get('filters'))
36+
37+
for value in range(len(query['timeDimensions'])):
38+
filters.append(query['timeDimensions'][value]['dateRange'])
39+
40+
if not filters or None in filters:
41+
raise Exception("Queries can't be run without a filter")
42+
return query
43+
44+
45+
@config('check_sql_auth')
46+
def check_sql_auth(query: dict, username: str, password: str) -> dict:
47+
if username == 'admin':
48+
return {
49+
'username': 'admin',
50+
'password': password,
51+
'securityContext': {
52+
'auth': {
53+
'username': 'admin',
54+
'userAttributes': {
55+
'canHaveAdmin': True,
56+
'city': 'New York'
57+
},
58+
'roles': ['admin']
59+
}
60+
}
61+
}
62+
raise Exception("Invalid username or password")
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
cubes:
2+
- name: users
3+
sql_table: users
4+
5+
measures:
6+
- name: count
7+
sql: id
8+
type: count
9+
10+
dimensions:
11+
- name: city
12+
sql: city
13+
type: string
14+
15+
- name: id
16+
sql: id
17+
type: number
18+
primary_key: true
19+
20+
access_policy:
21+
- role: "*"
22+
row_level:
23+
filters:
24+
- member: "{CUBE}.city"
25+
operator: equals
26+
values: ["{ security_context.auth.userAttributes.city }"]
27+
- role: admin
28+
conditions:
29+
# This thing will fail if there's no auth info in the context
30+
# Unfortunately, as of now, there's no way to write more complex expressions
31+
# that would allow us to check for the existence of the auth object
32+
- if: "{ security_context.auth.userAttributes.canHaveAdmin }"
33+
row_level:
34+
filters:
35+
- or:
36+
- and:
37+
- member: "{CUBE}.city"
38+
operator: notStartsWith
39+
values:
40+
- London
41+
- "{ security_context.auth.userAttributes.city }"
42+
# mixing string, dynamic values, integers and bools should not
43+
# cause any compilation issues
44+
- 4
45+
- true
46+
- member: "city"
47+
operator: notEquals
48+
values:
49+
- 'San Francisco'
50+
- member: "{CUBE}.city"
51+
operator: equals
52+
values:
53+
- "New York"
54+

packages/cubejs-testing/src/birdbox.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ export async function startBirdBoxFromContainer(
262262
if (pid !== null) {
263263
process.kill(pid, signal);
264264
} else {
265-
process.stdout.write(`[Birdbox] Cannot kill Cube instance running in TEST_CUBE_HOST mode without TEST_CUBE_PID defined\n`);
265+
process.stdout.write('[Birdbox] Cannot kill Cube instance running in TEST_CUBE_HOST mode without TEST_CUBE_PID defined\n');
266266
throw new Error('Attempted to use killCube while running with TEST_CUBE_HOST');
267267
}
268268
},
@@ -541,9 +541,15 @@ export async function startBirdBoxFromCli(
541541
}
542542

543543
if (options.cubejsConfig) {
544+
const configType = options.cubejsConfig.split('.').at(-1);
545+
for (const configFile of ['cube.js', 'cube.py']) {
546+
if (fs.existsSync(path.join(testDir, configFile))) {
547+
fs.removeSync(path.join(testDir, configFile));
548+
}
549+
}
544550
fs.copySync(
545551
path.join(process.cwd(), 'birdbox-fixtures', options.cubejsConfig),
546-
path.join(testDir, 'cube.js')
552+
path.join(testDir, `cube.${configType}`)
547553
);
548554
}
549555

packages/cubejs-testing/test/__snapshots__/smoke-rbac.test.ts.snap

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3+
exports[`Cube RBAC Engine [Python config] RBAC via SQL API [python config] SELECT * from users: users_python 1`] = `
4+
Array [
5+
Object {
6+
"count": "551",
7+
},
8+
]
9+
`;
10+
311
exports[`Cube RBAC Engine RBAC via REST API line_items hidden price_dim: line_items_view_no_policy_rest 1`] = `
412
Array [
513
Object {

packages/cubejs-testing/test/smoke-rbac.test.ts

Lines changed: 87 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,40 +13,40 @@ import {
1313
JEST_BEFORE_ALL_DEFAULT_TIMEOUT,
1414
} from './smoke-tests';
1515

16+
const PG_PORT = 5656;
17+
let connectionId = 0;
18+
19+
async function createPostgresClient(user: string, password: string) {
20+
connectionId++;
21+
const currentConnId = connectionId;
22+
23+
console.debug(`[pg] new connection ${currentConnId}`);
24+
25+
const conn = new PgClient({
26+
database: 'db',
27+
port: PG_PORT,
28+
host: '127.0.0.1',
29+
user,
30+
password,
31+
ssl: false,
32+
});
33+
conn.on('error', (err) => {
34+
console.log(err);
35+
});
36+
conn.on('end', () => {
37+
console.debug(`[pg] end ${currentConnId}`);
38+
});
39+
40+
await conn.connect();
41+
42+
return conn;
43+
}
44+
1645
describe('Cube RBAC Engine', () => {
1746
jest.setTimeout(60 * 5 * 1000);
1847
let db: StartedTestContainer;
1948
let birdbox: BirdBox;
2049

21-
const pgPort = 5656;
22-
let connectionId = 0;
23-
24-
async function createPostgresClient(user: string, password: string) {
25-
connectionId++;
26-
const currentConnId = connectionId;
27-
28-
console.debug(`[pg] new connection ${currentConnId}`);
29-
30-
const conn = new PgClient({
31-
database: 'db',
32-
port: pgPort,
33-
host: '127.0.0.1',
34-
user,
35-
password,
36-
ssl: false,
37-
});
38-
conn.on('error', (err) => {
39-
console.log(err);
40-
});
41-
conn.on('end', () => {
42-
console.debug(`[pg] end ${currentConnId}`);
43-
});
44-
45-
await conn.connect();
46-
47-
return conn;
48-
}
49-
5050
beforeAll(async () => {
5151
db = await PostgresDBRunner.startContainer({});
5252
await PostgresDBRunner.loadEcom(db);
@@ -64,7 +64,7 @@ describe('Cube RBAC Engine', () => {
6464
CUBEJS_DB_USER: 'test',
6565
CUBEJS_DB_PASS: 'test',
6666
//
67-
CUBEJS_PG_SQL_PORT: `${pgPort}`,
67+
CUBEJS_PG_SQL_PORT: `${PG_PORT}`,
6868
},
6969
{
7070
schemaDir: 'rbac/model',
@@ -345,3 +345,60 @@ describe('Cube RBAC Engine [dev mode]', () => {
345345
}
346346
});
347347
});
348+
349+
describe('Cube RBAC Engine [Python config]', () => {
350+
jest.setTimeout(60 * 5 * 1000);
351+
let db: StartedTestContainer;
352+
let birdbox: BirdBox;
353+
354+
beforeAll(async () => {
355+
db = await PostgresDBRunner.startContainer({});
356+
await PostgresDBRunner.loadEcom(db);
357+
birdbox = await getBirdbox(
358+
'postgres',
359+
{
360+
...DEFAULT_CONFIG,
361+
CUBEJS_DEV_MODE: 'false',
362+
NODE_ENV: 'production',
363+
//
364+
CUBEJS_DB_TYPE: 'postgres',
365+
CUBEJS_DB_HOST: db.getHost(),
366+
CUBEJS_DB_PORT: `${db.getMappedPort(5432)}`,
367+
CUBEJS_DB_NAME: 'test',
368+
CUBEJS_DB_USER: 'test',
369+
CUBEJS_DB_PASS: 'test',
370+
//
371+
CUBEJS_PG_SQL_PORT: `${PG_PORT}`,
372+
},
373+
{
374+
schemaDir: 'rbac-python/model',
375+
cubejsConfig: 'rbac-python/cube.py',
376+
}
377+
);
378+
}, JEST_BEFORE_ALL_DEFAULT_TIMEOUT);
379+
380+
afterAll(async () => {
381+
await birdbox.stop();
382+
await db.stop();
383+
}, JEST_AFTER_ALL_DEFAULT_TIMEOUT);
384+
385+
describe('RBAC via SQL API [python config]', () => {
386+
let connection: PgClient;
387+
388+
beforeAll(async () => {
389+
connection = await createPostgresClient('admin', 'admin_password');
390+
});
391+
392+
afterAll(async () => {
393+
await connection.end();
394+
}, JEST_AFTER_ALL_DEFAULT_TIMEOUT);
395+
396+
test('SELECT * from users', async () => {
397+
const res = await connection.query('SELECT COUNT(city) as count from "users" HAVING (COUNT(1) > 0)');
398+
// const res = await connection.query('SELECT * FROM users limit 10');
399+
// This query should return all rows because of the `allow_all` statement
400+
// It should also exclude the `created_at` dimension as per memberLevel policy
401+
expect(res.rows).toMatchSnapshot('users_python');
402+
});
403+
});
404+
});

0 commit comments

Comments
 (0)