Skip to content

Commit a1936d2

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 a1936d2

File tree

7 files changed

+71
-504
lines changed

7 files changed

+71
-504
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/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@
7373
"smoke:postgres": "jest --verbose -i dist/test/smoke-postgres.test.js",
7474
"smoke:redshift": "jest --verbose -i dist/test/smoke-redshift.test.js",
7575
"smoke:redshift:snapshot": "jest --verbose --updateSnapshot -i dist/test/smoke-redshift.test.js",
76-
"smoke:rbac": "TZ=UTC jest --verbose --forceExit -i dist/test/smoke-rbac.test.js",
76+
"smoke:rbac": "TZ=UTC jest --verbose -i dist/test/smoke-rbac.test.js",
7777
"smoke:cubesql": "TZ=UTC jest --verbose --forceExit -i dist/test/smoke-cubesql.test.js",
7878
"smoke:cubesql:snapshot": "TZ=UTC jest --verbose --forceExit --updateSnapshot -i dist/test/smoke-cubesql.test.js",
7979
"smoke:prestodb": "jest --verbose -i dist/test/smoke-prestodb.test.js",

0 commit comments

Comments
 (0)