From 47e7897f7d16b9e24f350234c093d81f0b599dca Mon Sep 17 00:00:00 2001 From: Maxim Date: Thu, 21 Nov 2024 08:56:58 -0800 Subject: [PATCH] fix: fix data access policies condition logic (#8976) Data access policies conditions should be joined via AND operator, but the initial implementation used OR by mistake Also ensured that rbac smoke tests are ran as part of the CI --- .github/actions/smoke.sh | 6 +- .../src/core/CompilerApi.js | 2 +- .../birdbox-fixtures/rbac/cube.js | 21 + .../rbac/model/cubes/line_items.js | 18 + packages/cubejs-testing/package.json | 2 +- .../__snapshots__/smoke-rbac.test.ts.snap | 497 +----------------- .../cubejs-testing/test/smoke-rbac.test.ts | 29 +- 7 files changed, 71 insertions(+), 504 deletions(-) diff --git a/.github/actions/smoke.sh b/.github/actions/smoke.sh index 57eceb824b904..40256699f1429 100755 --- a/.github/actions/smoke.sh +++ b/.github/actions/smoke.sh @@ -55,4 +55,8 @@ echo "::endgroup::" echo "::group::MongoBI" yarn lerna run --concurrency 1 --stream --no-prefix smoke:mongobi -echo "::endgroup::" \ No newline at end of file +echo "::endgroup::" + +echo "::group::RBAC" +yarn lerna run --concurrency 1 --stream --no-prefix smoke:rbac +echo "::endgroup::" diff --git a/packages/cubejs-server-core/src/core/CompilerApi.js b/packages/cubejs-server-core/src/core/CompilerApi.js index 7418428ade32c..3a529e85300b4 100644 --- a/packages/cubejs-server-core/src/core/CompilerApi.js +++ b/packages/cubejs-server-core/src/core/CompilerApi.js @@ -205,7 +205,7 @@ export class CompilerApi { if (typeof b !== 'boolean') { throw new Error(`Access policy condition must return boolean, got ${JSON.stringify(b)}`); } - return a || b; + return a && b; }); } return true; diff --git a/packages/cubejs-testing/birdbox-fixtures/rbac/cube.js b/packages/cubejs-testing/birdbox-fixtures/rbac/cube.js index 45dd3af535f9c..0fc726c5c7ec0 100644 --- a/packages/cubejs-testing/birdbox-fixtures/rbac/cube.js +++ b/packages/cubejs-testing/birdbox-fixtures/rbac/cube.js @@ -22,6 +22,27 @@ module.exports = { }, }; } + if (user === 'manager') { + if (password && password !== 'manager_password') { + throw new Error(`Password doesn't match for ${user}`); + } + return { + password, + superuser: false, + securityContext: { + auth: { + username: 'manager', + userAttributes: { + region: 'CA', + city: 'Fresno', + canHaveAdmin: false, + minDefaultId: 10000, + }, + roles: ['manager'], + }, + }, + }; + } throw new Error(`User "${user}" doesn't exist`); } }; diff --git a/packages/cubejs-testing/birdbox-fixtures/rbac/model/cubes/line_items.js b/packages/cubejs-testing/birdbox-fixtures/rbac/model/cubes/line_items.js index 3d4ddd8ced1a5..c4941a5051b73 100644 --- a/packages/cubejs-testing/birdbox-fixtures/rbac/model/cubes/line_items.js +++ b/packages/cubejs-testing/birdbox-fixtures/rbac/model/cubes/line_items.js @@ -72,5 +72,23 @@ cube('line_items', { excludes: ['created_at'], }, }, + { + role: 'manager', + conditions: [ + { + if: security_context.auth?.userAttributes?.region === 'CA', + }, + { + // This condition should not match the one defined in the "manager" mock context + if: security_context.auth?.userAttributes?.region === 'San Francisco', + }, + ], + rowLevel: { + allowAll: true, + }, + memberLevel: { + excludes: ['created_at'], + }, + }, ], }); diff --git a/packages/cubejs-testing/package.json b/packages/cubejs-testing/package.json index 4b093c6f3951b..136ed2c5655c3 100644 --- a/packages/cubejs-testing/package.json +++ b/packages/cubejs-testing/package.json @@ -73,7 +73,7 @@ "smoke:postgres": "jest --verbose -i dist/test/smoke-postgres.test.js", "smoke:redshift": "jest --verbose -i dist/test/smoke-redshift.test.js", "smoke:redshift:snapshot": "jest --verbose --updateSnapshot -i dist/test/smoke-redshift.test.js", - "smoke:rbac": "TZ=UTC jest --verbose --forceExit -i dist/test/smoke-rbac.test.js", + "smoke:rbac": "TZ=UTC jest --verbose -i dist/test/smoke-rbac.test.js", "smoke:cubesql": "TZ=UTC jest --verbose --forceExit -i dist/test/smoke-cubesql.test.js", "smoke:cubesql:snapshot": "TZ=UTC jest --verbose --forceExit --updateSnapshot -i dist/test/smoke-cubesql.test.js", "smoke:prestodb": "jest --verbose -i dist/test/smoke-prestodb.test.js", 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 10275b929d670..b429076171933 100644 --- a/packages/cubejs-testing/test/__snapshots__/smoke-rbac.test.ts.snap +++ b/packages/cubejs-testing/test/__snapshots__/smoke-rbac.test.ts.snap @@ -1,500 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Cube RBAC Engine RBAC through SQL API SELECT * from line_items: line_items 1`] = ` -Array [ - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "price": 267, - "price_dim": 267, - "quantity": 2, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "price": 263, - "price_dim": 263, - "quantity": 7, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "price": 180, - "price_dim": 180, - "quantity": 8, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "price": 169, - "price_dim": 169, - "quantity": 6, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "price": 156, - "price_dim": 156, - "quantity": 1, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "price": 36, - "price_dim": 36, - "quantity": 5, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "price": 245, - "price_dim": 245, - "quantity": 4, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "price": 232, - "price_dim": 232, - "quantity": 8, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "price": 63, - "price_dim": 63, - "quantity": 8, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "price": 68, - "price_dim": 68, - "quantity": 6, - }, -] -`; - -exports[`Cube RBAC Engine RBAC through SQL API SELECT * from line_items_view_joined_orders: orders_view 1`] = ` -Array [ - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2018-10-23T07:54:39.000Z, - "id": 1, - "orders_count": "1", - "orders_created_at": 2018-10-23T07:54:39.000Z, - "orders_id": 1, - "price": 267, - "price_dim": 267, - "quantity": 2, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2019-12-16T08:09:36.000Z, - "id": 10, - "orders_count": "1", - "orders_created_at": 2019-12-16T08:09:36.000Z, - "orders_id": 10, - "price": 68, - "price_dim": 68, - "quantity": 6, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2019-04-22T15:52:17.000Z, - "id": 11, - "orders_count": "1", - "orders_created_at": 2019-04-22T15:52:17.000Z, - "orders_id": 11, - "price": 170, - "price_dim": 170, - "quantity": 1, - }, -] -`; - -exports[`Cube RBAC Engine RBAC through SQL API SELECT * from line_items_view_no_policy: line_items_view_no_policy 1`] = ` -Array [ - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2018-10-23T07:54:39.000Z, - "id": 1, - "price": 267, - "price_dim": 267, - "quantity": 2, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2018-01-01T13:50:20.000Z, - "id": 2, - "price": 263, - "price_dim": 263, - "quantity": 7, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2017-05-13T21:23:08.000Z, - "id": 3, - "price": 180, - "price_dim": 180, - "quantity": 8, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2018-04-10T22:51:15.000Z, - "id": 4, - "price": 169, - "price_dim": 169, - "quantity": 6, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2017-07-16T15:00:34.000Z, - "id": 5, - "price": 156, - "price_dim": 156, - "quantity": 1, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2019-05-23T04:25:27.000Z, - "id": 6, - "price": 36, - "price_dim": 36, - "quantity": 5, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2018-09-29T20:29:30.000Z, - "id": 7, - "price": 245, - "price_dim": 245, - "quantity": 4, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2019-04-17T03:32:54.000Z, - "id": 8, - "price": 232, - "price_dim": 232, - "quantity": 8, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2019-11-15T18:22:17.000Z, - "id": 9, - "price": 63, - "price_dim": 63, - "quantity": 8, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2019-12-16T08:09:36.000Z, - "id": 10, - "price": 68, - "price_dim": 68, - "quantity": 6, - }, -] -`; - -exports[`Cube RBAC Engine RBAC through SQL API SELECT * from line_items_view_price_gt_200: line_items_view_price_gt_200 1`] = ` -Array [ - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2018-10-23T07:54:39.000Z, - "id": 1, - "price": 267, - "price_dim": 267, - "quantity": 2, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2018-01-01T13:50:20.000Z, - "id": 2, - "price": 263, - "price_dim": 263, - "quantity": 7, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2018-09-29T20:29:30.000Z, - "id": 7, - "price": 245, - "price_dim": 245, - "quantity": 4, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2019-04-17T03:32:54.000Z, - "id": 8, - "price": 232, - "price_dim": 232, - "quantity": 8, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2017-12-04T06:39:17.000Z, - "id": 12, - "price": 266, - "price_dim": 266, - "quantity": 9, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2019-01-28T20:03:53.000Z, - "id": 13, - "price": 286, - "price_dim": 286, - "quantity": 6, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2017-08-04T21:41:23.000Z, - "id": 15, - "price": 237, - "price_dim": 237, - "quantity": 6, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2018-01-14T05:31:34.000Z, - "id": 25, - "price": 262, - "price_dim": 262, - "quantity": 1, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2016-01-01T23:00:21.000Z, - "id": 29, - "price": 241, - "price_dim": 241, - "quantity": 7, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2017-07-05T18:07:39.000Z, - "id": 34, - "price": 249, - "price_dim": 249, - "quantity": 8, - }, -] -`; - -exports[`Cube RBAC Engine RBAC through SQL API SELECT * from orders: orders_open 1`] = ` -Array [ - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2018-01-01T13:50:20.000Z, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2017-05-13T21:23:08.000Z, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2018-04-10T22:51:15.000Z, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2017-07-16T15:00:34.000Z, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2018-09-29T20:29:30.000Z, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2019-04-17T03:32:54.000Z, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2019-11-15T18:22:17.000Z, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2019-12-16T08:09:36.000Z, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2019-04-22T15:52:17.000Z, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2017-12-04T06:39:17.000Z, - }, -] -`; - -exports[`Cube RBAC Engine RBAC through SQL API SELECT * from orders_view: orders_view 1`] = ` -Array [ - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2019-12-16T08:09:36.000Z, - "id": 10, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2019-04-22T15:52:17.000Z, - "id": 11, - }, - Object { - "__cubeJoinField": null, - "__user": null, - "count": "1", - "created_at": 2018-10-23T07:54:39.000Z, - "id": 1, - }, -] -`; - -exports[`Cube RBAC Engine RBAC through SQL API SELECT * from users: users 1`] = ` -Array [ - Object { - "__cubeJoinField": null, - "__user": null, - "city": "Austin", - "count": "1", - }, - Object { - "__cubeJoinField": null, - "__user": null, - "city": "Austin", - "count": "1", - }, - Object { - "__cubeJoinField": null, - "__user": null, - "city": "Austin", - "count": "1", - }, - Object { - "__cubeJoinField": null, - "__user": null, - "city": "Austin", - "count": "1", - }, - Object { - "__cubeJoinField": null, - "__user": null, - "city": "Austin", - "count": "1", - }, - Object { - "__cubeJoinField": null, - "__user": null, - "city": "Austin", - "count": "1", - }, - Object { - "__cubeJoinField": null, - "__user": null, - "city": "Austin", - "count": "1", - }, - Object { - "__cubeJoinField": null, - "__user": null, - "city": "Austin", - "count": "1", - }, - Object { - "__cubeJoinField": null, - "__user": null, - "city": "Austin", - "count": "1", - }, - Object { - "__cubeJoinField": null, - "__user": null, - "city": "Austin", - "count": "1", - }, -] -`; - exports[`Cube RBAC Engine RBAC via REST API line_items hidden created_at: line_items_view_no_policy_rest 1`] = ` Array [ Object { @@ -1081,3 +586,5 @@ Array [ }, ] `; + +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 b2476395968e7..a6f544d47bb8e 100644 --- a/packages/cubejs-testing/test/smoke-rbac.test.ts +++ b/packages/cubejs-testing/test/smoke-rbac.test.ts @@ -15,9 +15,8 @@ import { describe('Cube RBAC Engine', () => { jest.setTimeout(60 * 5 * 1000); - - let birdbox: BirdBox; let db: StartedTestContainer; + let birdbox: BirdBox; const pgPort = 5656; let connectionId = 0; @@ -55,8 +54,8 @@ describe('Cube RBAC Engine', () => { 'postgres', { ...DEFAULT_CONFIG, - // - CUBESQL_LOG_LEVEL: 'trace', + CUBEJS_DEV_MODE: 'false', + NODE_ENV: 'production', // CUBEJS_DB_TYPE: 'postgres', CUBEJS_DB_HOST: db.getHost(), @@ -66,8 +65,6 @@ describe('Cube RBAC Engine', () => { CUBEJS_DB_PASS: 'test', // CUBEJS_PG_SQL_PORT: `${pgPort}`, - CUBESQL_SQL_PUSH_DOWN: 'true', - CUBESQL_STREAM_MODE: 'true', }, { schemaDir: 'rbac/model', @@ -148,6 +145,26 @@ describe('Cube RBAC Engine', () => { }); }); + describe('RBAC via SQL API manager', () => { + let connection: PgClient; + + beforeAll(async () => { + connection = await createPostgresClient('manager', 'manager_password'); + }); + + afterAll(async () => { + await connection.end(); + }, JEST_AFTER_ALL_DEFAULT_TIMEOUT); + + test('SELECT * from line_items', async () => { + const res = await connection.query('SELECT * FROM line_items limit 10'); + // This query should return rows allowed by the default policy + // because the manager security context has a wrong city and should not match + // two conditions defined on the manager policy + expect(res.rows).toMatchSnapshot('line_items_manager'); + }); + }); + describe('RBAC via REST API', () => { let client: CubeApi; let defaultClient: CubeApi;