diff --git a/packages/cubejs-api-gateway/src/gateway.ts b/packages/cubejs-api-gateway/src/gateway.ts index f56d804e27001..6737ca6fef35c 100644 --- a/packages/cubejs-api-gateway/src/gateway.ts +++ b/packages/cubejs-api-gateway/src/gateway.ts @@ -1185,7 +1185,8 @@ class ApiGateway { let normalizedQueries: NormalizedQuery[] = await Promise.all( queries.map( async (currentQuery) => { - const hasExpressionsInQuery = this.hasExpressionsInQuery(currentQuery); + const hasExpressionsInQuery = + this.hasExpressionsInQuery(currentQuery); if (hasExpressionsInQuery) { if (!memberExpressions) { @@ -1195,7 +1196,15 @@ class ApiGateway { currentQuery = this.parseMemberExpressionsInQuery(currentQuery); } - const normalizedQuery = normalizeQuery(currentQuery, persistent); + let normalizedQuery = normalizeQuery(currentQuery, persistent); + + if (hasExpressionsInQuery) { + // We need to parse/eval all member expressions early as applyRowLevelSecurity + // needs to access the full SQL query in order to evaluate rules + normalizedQuery = + this.evalMemberExpressionsInQuery(normalizedQuery); + } + // First apply cube/view level security policies const queryWithRlsFilters = await compilerApi.applyRowLevelSecurity( normalizedQuery, @@ -1204,17 +1213,18 @@ class ApiGateway { // Then apply user-supplied queryRewrite let rewrittenQuery = await this.queryRewrite( queryWithRlsFilters, - context, + context ); - if (hasExpressionsInQuery) { + // applyRowLevelSecurity may add new filters which may contain raw member expressions + // if that's the case, we should run an extra pass of parsing here to make sure + // nothing breaks down the road + if (this.hasExpressionsInQuery(rewrittenQuery)) { + rewrittenQuery = this.parseMemberExpressionsInQuery(rewrittenQuery); rewrittenQuery = this.evalMemberExpressionsInQuery(rewrittenQuery); } - return normalizeQuery( - rewrittenQuery, - persistent, - ); + return normalizeQuery(rewrittenQuery, persistent); } ) ); diff --git a/packages/cubejs-testing/birdbox-fixtures/rbac/cube.js b/packages/cubejs-testing/birdbox-fixtures/rbac/cube.js index 0fc726c5c7ec0..e7c82f455e36f 100644 --- a/packages/cubejs-testing/birdbox-fixtures/rbac/cube.js +++ b/packages/cubejs-testing/birdbox-fixtures/rbac/cube.js @@ -43,6 +43,27 @@ module.exports = { }, }; } + if (user === 'default') { + if (password && password !== 'default_password') { + throw new Error(`Password doesn't match for ${user}`); + } + return { + password, + superuser: false, + securityContext: { + auth: { + username: 'default', + userAttributes: { + region: 'CA', + city: 'San Francisco', + canHaveAdmin: false, + minDefaultId: 20000, + }, + roles: [], + }, + }, + }; + } throw new Error(`User "${user}" doesn't exist`); } }; diff --git a/packages/cubejs-testing/birdbox-fixtures/rbac/model/cubes/users.yaml b/packages/cubejs-testing/birdbox-fixtures/rbac/model/cubes/users.yaml index d2b113e8a39ea..7904a2199dee2 100644 --- a/packages/cubejs-testing/birdbox-fixtures/rbac/model/cubes/users.yaml +++ b/packages/cubejs-testing/birdbox-fixtures/rbac/model/cubes/users.yaml @@ -19,6 +19,11 @@ cubes: access_policy: - role: "*" + row_level: + filters: + - member: "{CUBE}.city" + operator: equals + values: ["{ security_context.auth.userAttributes.city }"] - role: admin conditions: # This thing will fail if there's no auth info in the context diff --git a/packages/cubejs-testing/test/__snapshots__/smoke-rbac.test.ts.snap b/packages/cubejs-testing/test/__snapshots__/smoke-rbac.test.ts.snap index 256ad6116bd81..6543d2eeb65cb 100644 --- a/packages/cubejs-testing/test/__snapshots__/smoke-rbac.test.ts.snap +++ b/packages/cubejs-testing/test/__snapshots__/smoke-rbac.test.ts.snap @@ -587,4 +587,12 @@ Array [ ] `; +exports[`Cube RBAC Engine RBAC via SQL API default policy SELECT with member expressions: users_member_expression 1`] = ` +Array [ + Object { + "count": "149", + }, +] +`; + exports[`Cube RBAC Engine RBAC via SQL API manager SELECT * from line_items: line_items_manager 1`] = `Array []`; diff --git a/packages/cubejs-testing/test/smoke-rbac.test.ts b/packages/cubejs-testing/test/smoke-rbac.test.ts index 763844635510e..95e7dc5cbf723 100644 --- a/packages/cubejs-testing/test/smoke-rbac.test.ts +++ b/packages/cubejs-testing/test/smoke-rbac.test.ts @@ -165,6 +165,24 @@ describe('Cube RBAC Engine', () => { }); }); + describe('RBAC via SQL API default policy', () => { + let connection: PgClient; + + beforeAll(async () => { + connection = await createPostgresClient('default', 'default_password'); + }); + + afterAll(async () => { + await connection.end(); + }, JEST_AFTER_ALL_DEFAULT_TIMEOUT); + + test('SELECT with member expressions', async () => { + const res = await connection.query('SELECT COUNT(city) as count from "users" HAVING (COUNT(1) > 0)'); + // Pushed SQL queries should not fail + expect(res.rows).toMatchSnapshot('users_member_expression'); + }); + }); + describe('RBAC via REST API', () => { let client: CubeApi; let defaultClient: CubeApi;