fix(api-gateway): fix DAP RLS issue with denied queries and python conf

Author: bsod90

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

@@ -1207,16 +1207,16 @@ class ApiGateway {
           }
 
           // First apply cube/view level security policies
-          const queryWithRlsFilters = await compilerApi.applyRowLevelSecurity(
+          const { query: queryWithRlsFilters, denied } = await compilerApi.applyRowLevelSecurity(
             normalizedQuery,
             evaluatedQuery,
             context
           );
           // Then apply user-supplied queryRewrite
-          let rewrittenQuery = await this.queryRewrite(
+          let rewrittenQuery = !denied ? await this.queryRewrite(
             queryWithRlsFilters,
             context
-          );
+          ) : queryWithRlsFilters;
 
           // 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

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

@@ -276,7 +276,7 @@ export class CompilerApi {
     const { cubeEvaluator } = compilers;
 
     if (!cubeEvaluator.isRbacEnabled()) {
-      return query;
+      return { query, denied: false };
     }
 
     const queryCubes = await this.getCubesFromQuery(evaluatedQuery, context);
@@ -322,7 +322,7 @@ export class CompilerApi {
             name: 'rlsAccessDenied',
           });
           // If we hit this condition there's no need to evaluate the rest of the policy
-          break;
+          return { query, denied: true };
         }
       }
     }
@@ -334,7 +334,7 @@ export class CompilerApi {
     );
     query.filters = query.filters || [];
     query.filters.push(rlsFilter);
-    return query;
+    return { query, denied: false };
   }
 
   buildFinalRlsFilter(cubeFiltersPerCubePerRole, viewFiltersPerCubePerRole, hasAllowAllForCube) {

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

@@ -0,0 +1,28 @@
+cubes:
+  - name: products
+    sql_table: products
+
+    measures:
+      - name: count
+        type: count
+
+    dimensions:
+      - name: id
+        sql: ID
+        type: number
+        primary_key: true
+
+      - name: product_category
+        sql: PRODUCT_CATEGORY
+        type: string
+
+      - name: name
+        sql: NAME
+        type: string
+
+      - name: created_at
+        sql: CREATED_AT
+        type: time
+
+    access_policy:
+      - role: some_role

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

@@ -0,0 +1,28 @@
+cubes:
+  - name: products
+    sql_table: products
+
+    measures:
+      - name: count
+        type: count
+
+    dimensions:
+      - name: id
+        sql: ID
+        type: number
+        primary_key: true
+
+      - name: product_category
+        sql: PRODUCT_CATEGORY
+        type: string
+
+      - name: name
+        sql: NAME
+        type: string
+
+      - name: created_at
+        sql: CREATED_AT
+        type: time
+
+    access_policy:
+      - role: some_role

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

@@ -8,6 +8,22 @@ Array [
 ]
 `;
 
+exports[`Cube RBAC Engine [Python config][dev mode] products with no matching policy: products_no_policy_python 1`] = `
+Array [
+  Object {
+    "products.count": "0",
+  },
+]
+`;
+
+exports[`Cube RBAC Engine [dev mode] products with no matching policy: products_no_policy 1`] = `
+Array [
+  Object {
+    "products.count": "0",
+  },
+]
+`;
+
 exports[`Cube RBAC Engine RBAC via REST API line_items hidden price_dim: line_items_view_no_policy_rest 1`] = `
 Array [
   Object {

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

@@ -16,6 +16,16 @@ import {
 const PG_PORT = 5656;
 let connectionId = 0;
 
+const DEFAULT_API_TOKEN = sign({
+  auth: {
+    username: 'nobody',
+    userAttributes: {},
+    roles: [],
+  },
+}, DEFAULT_CONFIG.CUBEJS_API_SECRET, {
+  expiresIn: '2 days'
+});
+
 async function createPostgresClient(user: string, password: string) {
   connectionId++;
   const currentConnId = connectionId;
@@ -96,7 +106,6 @@ describe('Cube RBAC Engine', () => {
       expect(res.rows).toMatchSnapshot('line_items');
     });
 
-    // ???
     test('SELECT * from line_items_view_no_policy', async () => {
       const res = await connection.query('SELECT * FROM line_items_view_no_policy limit 10');
       // This should query the line_items cube through the view that should
@@ -202,16 +211,6 @@ describe('Cube RBAC Engine', () => {
       expiresIn: '2 days'
     });
 
-    const DEFAULT_API_TOKEN = sign({
-      auth: {
-        username: 'nobody',
-        userAttributes: {},
-        roles: [],
-      },
-    }, DEFAULT_CONFIG.CUBEJS_API_SECRET, {
-      expiresIn: '2 days'
-    });
-
     beforeAll(async () => {
       client = cubejs(async () => ADMIN_API_TOKEN, {
         apiUrl: birdbox.configuration.apiUrl,
@@ -289,16 +288,6 @@ describe('Cube RBAC Engine [dev mode]', () => {
   let birdbox: BirdBox;
   let client: CubeApi;
 
-  const DEFAULT_API_TOKEN = sign({
-    auth: {
-      username: 'nobody',
-      userAttributes: {},
-      roles: [],
-    },
-  }, DEFAULT_CONFIG.CUBEJS_API_SECRET, {
-    expiresIn: '2 days'
-  });
-
   const pgPort = 5656;
 
   beforeAll(async () => {
@@ -344,6 +333,15 @@ describe('Cube RBAC Engine [dev mode]', () => {
       expect(dim.public).toBe(false);
     }
   });
+
+  test('products with no matching policy', async () => {
+    const result = await client.load({
+      measures: ['products.count'],
+    });
+
+    // Querying a cube with no matching access policy should return no data
+    expect(result.rawData()).toMatchSnapshot('products_no_policy');
+  });
 });
 
 describe('Cube RBAC Engine [Python config]', () => {
@@ -402,3 +400,53 @@ describe('Cube RBAC Engine [Python config]', () => {
     });
   });
 });
+
+describe('Cube RBAC Engine [Python config][dev mode]', () => {
+  jest.setTimeout(60 * 5 * 1000);
+  let db: StartedTestContainer;
+  let birdbox: BirdBox;
+  let client: CubeApi;
+
+  beforeAll(async () => {
+    db = await PostgresDBRunner.startContainer({});
+    await PostgresDBRunner.loadEcom(db);
+    birdbox = await getBirdbox(
+      'postgres',
+      {
+        ...DEFAULT_CONFIG,
+        CUBEJS_DEV_MODE: 'true',
+        NODE_ENV: 'dev',
+        //
+        CUBEJS_DB_TYPE: 'postgres',
+        CUBEJS_DB_HOST: db.getHost(),
+        CUBEJS_DB_PORT: `${db.getMappedPort(5432)}`,
+        CUBEJS_DB_NAME: 'test',
+        CUBEJS_DB_USER: 'test',
+        CUBEJS_DB_PASS: 'test',
+        //
+        CUBEJS_PG_SQL_PORT: `${PG_PORT}`,
+      },
+      {
+        schemaDir: 'rbac-python/model',
+        cubejsConfig: 'rbac-python/cube.py',
+      }
+    );
+    client = cubejs(async () => DEFAULT_API_TOKEN, {
+      apiUrl: birdbox.configuration.apiUrl,
+    });
+  }, JEST_BEFORE_ALL_DEFAULT_TIMEOUT);
+
+  afterAll(async () => {
+    await birdbox.stop();
+    await db.stop();
+  }, JEST_AFTER_ALL_DEFAULT_TIMEOUT);
+
+  test('products with no matching policy', async () => {
+    const result = await client.load({
+      measures: ['products.count'],
+    });
+
+    // Querying a cube with no matching access policy should return no data
+    expect(result.rawData()).toMatchSnapshot('products_no_policy_python');
+  });
+});