Skip to content

Commit 23695b2

Browse files
authored
fix(api-gateway): make sure DAP works sql pushdown (#9021)
fixes an issue when processing queries with member expressions
1 parent e55974e commit 23695b2

File tree

5 files changed

+70
-8
lines changed

5 files changed

+70
-8
lines changed

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

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,7 +1185,8 @@ class ApiGateway {
11851185
let normalizedQueries: NormalizedQuery[] = await Promise.all(
11861186
queries.map(
11871187
async (currentQuery) => {
1188-
const hasExpressionsInQuery = this.hasExpressionsInQuery(currentQuery);
1188+
const hasExpressionsInQuery =
1189+
this.hasExpressionsInQuery(currentQuery);
11891190

11901191
if (hasExpressionsInQuery) {
11911192
if (!memberExpressions) {
@@ -1195,7 +1196,15 @@ class ApiGateway {
11951196
currentQuery = this.parseMemberExpressionsInQuery(currentQuery);
11961197
}
11971198

1198-
const normalizedQuery = normalizeQuery(currentQuery, persistent);
1199+
let normalizedQuery = normalizeQuery(currentQuery, persistent);
1200+
1201+
if (hasExpressionsInQuery) {
1202+
// We need to parse/eval all member expressions early as applyRowLevelSecurity
1203+
// needs to access the full SQL query in order to evaluate rules
1204+
normalizedQuery =
1205+
this.evalMemberExpressionsInQuery(normalizedQuery);
1206+
}
1207+
11991208
// First apply cube/view level security policies
12001209
const queryWithRlsFilters = await compilerApi.applyRowLevelSecurity(
12011210
normalizedQuery,
@@ -1204,17 +1213,18 @@ class ApiGateway {
12041213
// Then apply user-supplied queryRewrite
12051214
let rewrittenQuery = await this.queryRewrite(
12061215
queryWithRlsFilters,
1207-
context,
1216+
context
12081217
);
12091218

1210-
if (hasExpressionsInQuery) {
1219+
// applyRowLevelSecurity may add new filters which may contain raw member expressions
1220+
// if that's the case, we should run an extra pass of parsing here to make sure
1221+
// nothing breaks down the road
1222+
if (this.hasExpressionsInQuery(rewrittenQuery)) {
1223+
rewrittenQuery = this.parseMemberExpressionsInQuery(rewrittenQuery);
12111224
rewrittenQuery = this.evalMemberExpressionsInQuery(rewrittenQuery);
12121225
}
12131226

1214-
return normalizeQuery(
1215-
rewrittenQuery,
1216-
persistent,
1217-
);
1227+
return normalizeQuery(rewrittenQuery, persistent);
12181228
}
12191229
)
12201230
);

packages/cubejs-testing/birdbox-fixtures/rbac/cube.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,27 @@ module.exports = {
4343
},
4444
};
4545
}
46+
if (user === 'default') {
47+
if (password && password !== 'default_password') {
48+
throw new Error(`Password doesn't match for ${user}`);
49+
}
50+
return {
51+
password,
52+
superuser: false,
53+
securityContext: {
54+
auth: {
55+
username: 'default',
56+
userAttributes: {
57+
region: 'CA',
58+
city: 'San Francisco',
59+
canHaveAdmin: false,
60+
minDefaultId: 20000,
61+
},
62+
roles: [],
63+
},
64+
},
65+
};
66+
}
4667
throw new Error(`User "${user}" doesn't exist`);
4768
}
4869
};

packages/cubejs-testing/birdbox-fixtures/rbac/model/cubes/users.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ cubes:
1919

2020
access_policy:
2121
- role: "*"
22+
row_level:
23+
filters:
24+
- member: "{CUBE}.city"
25+
operator: equals
26+
values: ["{ security_context.auth.userAttributes.city }"]
2227
- role: admin
2328
conditions:
2429
# This thing will fail if there's no auth info in the context

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,4 +587,12 @@ Array [
587587
]
588588
`;
589589

590+
exports[`Cube RBAC Engine RBAC via SQL API default policy SELECT with member expressions: users_member_expression 1`] = `
591+
Array [
592+
Object {
593+
"count": "149",
594+
},
595+
]
596+
`;
597+
590598
exports[`Cube RBAC Engine RBAC via SQL API manager SELECT * from line_items: line_items_manager 1`] = `Array []`;

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,24 @@ describe('Cube RBAC Engine', () => {
165165
});
166166
});
167167

168+
describe('RBAC via SQL API default policy', () => {
169+
let connection: PgClient;
170+
171+
beforeAll(async () => {
172+
connection = await createPostgresClient('default', 'default_password');
173+
});
174+
175+
afterAll(async () => {
176+
await connection.end();
177+
}, JEST_AFTER_ALL_DEFAULT_TIMEOUT);
178+
179+
test('SELECT with member expressions', async () => {
180+
const res = await connection.query('SELECT COUNT(city) as count from "users" HAVING (COUNT(1) > 0)');
181+
// Pushed SQL queries should not fail
182+
expect(res.rows).toMatchSnapshot('users_member_expression');
183+
});
184+
});
185+
168186
describe('RBAC via REST API', () => {
169187
let client: CubeApi;
170188
let defaultClient: CubeApi;

0 commit comments

Comments
 (0)