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
cube-js · Nov 21, 2024 · 47e7897 · 47e7897
1 parent e948856
commit 47e7897
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 504 deletions.
diff --git 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::"
+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
@@ -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
@@ -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
@@ -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
@@ -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",