Skip to content

Commit

Permalink
fix(api-gateway): fix DAP RLS issue with denied queries and python co…
Browse files Browse the repository at this point in the history
…nf (#9054)
  • Loading branch information
bsod90 authored Dec 17, 2024
1 parent cb507c1 commit e661d2a
Show file tree
Hide file tree
Showing 7 changed files with 148 additions and 28 deletions.
6 changes: 3 additions & 3 deletions packages/cubejs-api-gateway/src/gateway.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/cubejs-api-gateway/test/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export const compilerApi = jest.fn().mockImplementation(async () => ({
},

async applyRowLevelSecurity(query: any) {
return query;
return { query, denied: false };
},

async metaConfig() {
Expand Down
6 changes: 3 additions & 3 deletions packages/cubejs-server-core/src/core/CompilerApi.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 };
}
}
}
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
16 changes: 16 additions & 0 deletions packages/cubejs-testing/test/__snapshots__/smoke-rbac.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
90 changes: 69 additions & 21 deletions packages/cubejs-testing/test/smoke-rbac.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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]', () => {
Expand Down Expand Up @@ -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');
});
});

0 comments on commit e661d2a

Please sign in to comment.