Skip to content

Commit 5a41311

Browse files
committed
fix: fix data access policies condition logic
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
1 parent 591a383 commit 5a41311

File tree

6 files changed

+69
-2
lines changed

6 files changed

+69
-2
lines changed

.github/actions/smoke.sh

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,8 @@ echo "::endgroup::"
5555

5656
echo "::group::MongoBI"
5757
yarn lerna run --concurrency 1 --stream --no-prefix smoke:mongobi
58-
echo "::endgroup::"
58+
echo "::endgroup::"
59+
60+
echo "::group::RBAC"
61+
yarn lerna run --concurrency 1 --stream --no-prefix smoke:rbac
62+
echo "::endgroup::"

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ export class CompilerApi {
205205
if (typeof b !== 'boolean') {
206206
throw new Error(`Access policy condition must return boolean, got ${JSON.stringify(b)}`);
207207
}
208-
return a || b;
208+
return a && b;
209209
});
210210
}
211211
return true;

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,27 @@ module.exports = {
2222
},
2323
};
2424
}
25+
if (user === 'manager') {
26+
if (password && password !== 'manager_password') {
27+
throw new Error(`Password doesn't match for ${user}`);
28+
}
29+
return {
30+
password,
31+
superuser: false,
32+
securityContext: {
33+
auth: {
34+
username: 'manager',
35+
userAttributes: {
36+
region: 'CA',
37+
city: 'Fresno',
38+
canHaveAdmin: false,
39+
minDefaultId: 10000,
40+
},
41+
roles: ['manager'],
42+
},
43+
},
44+
};
45+
}
2546
throw new Error(`User "${user}" doesn't exist`);
2647
}
2748
};

packages/cubejs-testing/birdbox-fixtures/rbac/model/cubes/line_items.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,5 +72,23 @@ cube('line_items', {
7272
excludes: ['created_at'],
7373
},
7474
},
75+
{
76+
role: 'manager',
77+
conditions: [
78+
{
79+
if: security_context.auth?.userAttributes?.region === 'CA',
80+
},
81+
{
82+
// This condition should not match the one defined in the "manager" mock context
83+
if: security_context.auth?.userAttributes?.region === 'San Francisco',
84+
},
85+
],
86+
rowLevel: {
87+
allowAll: true,
88+
},
89+
memberLevel: {
90+
excludes: ['created_at'],
91+
},
92+
},
7593
],
7694
});

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,3 +1081,5 @@ Array [
10811081
},
10821082
]
10831083
`;
1084+
1085+
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: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ describe('Cube RBAC Engine', () => {
5757
...DEFAULT_CONFIG,
5858
//
5959
CUBESQL_LOG_LEVEL: 'trace',
60+
CUBEJS_DEV_MODE: 'false',
61+
NODE_ENV: 'production',
6062
//
6163
CUBEJS_DB_TYPE: 'postgres',
6264
CUBEJS_DB_HOST: db.getHost(),
@@ -148,6 +150,26 @@ describe('Cube RBAC Engine', () => {
148150
});
149151
});
150152

153+
describe('RBAC via SQL API manager', () => {
154+
let connection: PgClient;
155+
156+
beforeAll(async () => {
157+
connection = await createPostgresClient('manager', 'manager_password');
158+
});
159+
160+
afterAll(async () => {
161+
await connection.end();
162+
}, JEST_AFTER_ALL_DEFAULT_TIMEOUT);
163+
164+
test('SELECT * from line_items', async () => {
165+
const res = await connection.query('SELECT * FROM line_items limit 10');
166+
// This query should return rows allowed by the default policy
167+
// because the manager security context has a wrong city and should not match
168+
// two conditions defined on the manager policy
169+
expect(res.rows).toMatchSnapshot('line_items_manager');
170+
});
171+
});
172+
151173
describe('RBAC via REST API', () => {
152174
let client: CubeApi;
153175
let defaultClient: CubeApi;

0 commit comments

Comments
 (0)