From 84d1489da16475290a60229a8733425c85530287 Mon Sep 17 00:00:00 2001 From: Maxim Leonovich Date: Wed, 18 Dec 2024 10:49:44 -0800 Subject: [PATCH] fix(schema-compiler): make sure view members are resolvable in DAP --- .../src/compiler/CubeSymbols.js | 18 ++- .../src/compiler/DataSchemaCompiler.js | 2 + .../src/compiler/PrepareCompiler.ts | 10 +- .../src/compiler/YamlCompiler.ts | 5 +- .../transpilers/CubePropContextTranspiler.ts | 5 +- .../rbac-python/model/views/users_view.yaml | 13 ++ .../rbac/model/views/users.js | 16 ++ .../__snapshots__/smoke-rbac.test.ts.snap | 150 ++++++++++++++++++ .../cubejs-testing/test/smoke-rbac.test.ts | 12 ++ 9 files changed, 225 insertions(+), 6 deletions(-) create mode 100644 packages/cubejs-testing/birdbox-fixtures/rbac-python/model/views/users_view.yaml create mode 100644 packages/cubejs-testing/birdbox-fixtures/rbac/model/views/users.js diff --git a/packages/cubejs-schema-compiler/src/compiler/CubeSymbols.js b/packages/cubejs-schema-compiler/src/compiler/CubeSymbols.js index 754f340fd609c..1a82a0ee9a77d 100644 --- a/packages/cubejs-schema-compiler/src/compiler/CubeSymbols.js +++ b/packages/cubejs-schema-compiler/src/compiler/CubeSymbols.js @@ -22,13 +22,20 @@ const CONTEXT_SYMBOLS = { const CURRENT_CUBE_CONSTANTS = ['CUBE', 'TABLE']; export class CubeSymbols { - constructor(evaluateViews) { + constructor(evaluateViews, skipCompilationIfNoAccessPolicies) { this.symbols = {}; this.builtCubes = {}; this.cubeDefinitions = {}; this.funcArgumentsValues = {}; this.cubeList = []; this.evaluateViews = evaluateViews || false; + // This flag controls whether the additional compilation pass for views will happen or not. + // Background: + // When implemmenting Data Access Policies we realized, that in order to properly resolve + // Cube references in views, we need to have an additional pass of .compile with the evaluateViews + // flag set to true. This pass comes with a significant performance penalty and we'd like to avoid + // it if no access policies are defined for views in the data model. + this.skipCompilationIfNoAccessPolicies = skipCompilationIfNoAccessPolicies || false; } compile(cubes, errorReporter) { @@ -37,6 +44,11 @@ export class CubeSymbols { R.fromPairs )(cubes); this.cubeList = cubes.map(c => (c.name ? this.getCubeDefinition(c.name) : this.createCube(c))); + + if (this.skipCompilationIfNoAccessPolicies && !this.viewsHaveAccessPolicies(cubes)) { + return; + } + // TODO support actual dependency sorting to allow using views inside views const sortedByDependency = R.pipe( R.sortBy(c => !!c.isView), @@ -52,6 +64,10 @@ export class CubeSymbols { } } + viewsHaveAccessPolicies(cubes) { + return cubes.some(c => c.isView && c.accessPolicy); + } + getCubeDefinition(cubeName) { if (!this.builtCubes[cubeName]) { const cubeDefinition = this.cubeDefinitions[cubeName]; diff --git a/packages/cubejs-schema-compiler/src/compiler/DataSchemaCompiler.js b/packages/cubejs-schema-compiler/src/compiler/DataSchemaCompiler.js index 0d16153fad2d7..0c4407431e19b 100644 --- a/packages/cubejs-schema-compiler/src/compiler/DataSchemaCompiler.js +++ b/packages/cubejs-schema-compiler/src/compiler/DataSchemaCompiler.js @@ -23,6 +23,7 @@ export class DataSchemaCompiler { this.cubeCompilers = options.cubeCompilers || []; this.contextCompilers = options.contextCompilers || []; this.transpilers = options.transpilers || []; + this.viewCompilers = options.viewCompilers || []; this.preTranspileCubeCompilers = options.preTranspileCubeCompilers || []; this.cubeNameCompilers = options.cubeNameCompilers || []; this.extensions = options.extensions || {}; @@ -94,6 +95,7 @@ export class DataSchemaCompiler { return compilePhase({ cubeCompilers: this.cubeNameCompilers }) .then(() => compilePhase({ cubeCompilers: this.preTranspileCubeCompilers })) + .then(() => compilePhase({ cubeCompilers: this.viewCompilers })) .then(() => compilePhase({ cubeCompilers: this.cubeCompilers, contextCompilers: this.contextCompilers, diff --git a/packages/cubejs-schema-compiler/src/compiler/PrepareCompiler.ts b/packages/cubejs-schema-compiler/src/compiler/PrepareCompiler.ts index 446769688f07d..6817c028a2bf9 100644 --- a/packages/cubejs-schema-compiler/src/compiler/PrepareCompiler.ts +++ b/packages/cubejs-schema-compiler/src/compiler/PrepareCompiler.ts @@ -37,6 +37,9 @@ export const prepareCompiler = (repo: SchemaFileRepository, options: PrepareComp const nativeInstance = options.nativeInstance || new NativeInstance(); const cubeDictionary = new CubeDictionary(); const cubeSymbols = new CubeSymbols(); + // check the comment in CubeSymbols.js for the second agument + // skipCompilationIfNoAccessPolicies + const viewCompiler = new CubeSymbols(true, true); const cubeValidator = new CubeValidator(cubeSymbols); const cubeEvaluator = new CubeEvaluator(cubeValidator); const contextEvaluator = new ContextEvaluator(cubeEvaluator); @@ -44,12 +47,12 @@ export const prepareCompiler = (repo: SchemaFileRepository, options: PrepareComp const metaTransformer = new CubeToMetaTransformer(cubeValidator, cubeEvaluator, contextEvaluator, joinGraph); const { maxQueryCacheSize, maxQueryCacheAge } = options; const compilerCache = new CompilerCache({ maxQueryCacheSize, maxQueryCacheAge }); - const yamlCompiler = new YamlCompiler(cubeSymbols, cubeDictionary, nativeInstance); + const yamlCompiler = new YamlCompiler(cubeSymbols, cubeDictionary, nativeInstance, viewCompiler); const transpilers: TranspilerInterface[] = [ new ValidationTranspiler(), new ImportExportTranspiler(), - new CubePropContextTranspiler(cubeSymbols, cubeDictionary), + new CubePropContextTranspiler(cubeSymbols, cubeDictionary, viewCompiler), ]; if (!options.allowJsDuplicatePropsInSchema) { @@ -60,6 +63,7 @@ export const prepareCompiler = (repo: SchemaFileRepository, options: PrepareComp cubeNameCompilers: [cubeDictionary], preTranspileCubeCompilers: [cubeSymbols, cubeValidator], transpilers, + viewCompilers: [viewCompiler], cubeCompilers: [cubeEvaluator, joinGraph, metaTransformer], contextCompilers: [contextEvaluator], cubeFactory: cubeSymbols.createCube.bind(cubeSymbols), @@ -72,7 +76,7 @@ export const prepareCompiler = (repo: SchemaFileRepository, options: PrepareComp compileContext: options.compileContext, standalone: options.standalone, nativeInstance, - yamlCompiler + yamlCompiler, }, options)); return { diff --git a/packages/cubejs-schema-compiler/src/compiler/YamlCompiler.ts b/packages/cubejs-schema-compiler/src/compiler/YamlCompiler.ts index f5e7320243d03..2a4ce4059a7d0 100644 --- a/packages/cubejs-schema-compiler/src/compiler/YamlCompiler.ts +++ b/packages/cubejs-schema-compiler/src/compiler/YamlCompiler.ts @@ -33,6 +33,7 @@ export class YamlCompiler { private readonly cubeSymbols: CubeSymbols, private readonly cubeDictionary: CubeDictionary, private readonly nativeInstance: NativeInstance, + private readonly viewCompiler: CubeSymbols, ) { } @@ -288,7 +289,9 @@ export class YamlCompiler { }, ); - resolveSymbol = resolveSymbol || (n => this.cubeSymbols.resolveSymbol(cubeName, n) || this.cubeSymbols.isCurrentCube(n)); + resolveSymbol = resolveSymbol || (n => this.viewCompiler.resolveSymbol(cubeName, n) || + this.cubeSymbols.resolveSymbol(cubeName, n) || + this.cubeSymbols.isCurrentCube(n)); const traverseObj = { Program: (babelPath) => { diff --git a/packages/cubejs-schema-compiler/src/compiler/transpilers/CubePropContextTranspiler.ts b/packages/cubejs-schema-compiler/src/compiler/transpilers/CubePropContextTranspiler.ts index a7dcb2e85b26a..232e00af66b06 100644 --- a/packages/cubejs-schema-compiler/src/compiler/transpilers/CubePropContextTranspiler.ts +++ b/packages/cubejs-schema-compiler/src/compiler/transpilers/CubePropContextTranspiler.ts @@ -41,6 +41,7 @@ export class CubePropContextTranspiler implements TranspilerInterface { public constructor( protected readonly cubeSymbols: CubeSymbols, protected readonly cubeDictionary: CubeDictionary, + protected readonly viewCompiler: CubeSymbols, ) { } @@ -88,7 +89,9 @@ export class CubePropContextTranspiler implements TranspilerInterface { } protected sqlAndReferencesFieldVisitor(cubeName): TraverseObject { - const resolveSymbol = n => this.cubeSymbols.resolveSymbol(cubeName, n) || this.cubeSymbols.isCurrentCube(n); + const resolveSymbol = n => this.viewCompiler.resolveSymbol(cubeName, n) || + this.cubeSymbols.resolveSymbol(cubeName, n) || + this.cubeSymbols.isCurrentCube(n); return { ObjectProperty: (path) => { diff --git a/packages/cubejs-testing/birdbox-fixtures/rbac-python/model/views/users_view.yaml b/packages/cubejs-testing/birdbox-fixtures/rbac-python/model/views/users_view.yaml new file mode 100644 index 0000000000000..0163cc4e83681 --- /dev/null +++ b/packages/cubejs-testing/birdbox-fixtures/rbac-python/model/views/users_view.yaml @@ -0,0 +1,13 @@ +views: + - name: users_view + cubes: + - join_path: users + includes: "*" + + access_policy: + - role: '*' + row_level: + filters: + - member: id + operator: gt + values: [10] diff --git a/packages/cubejs-testing/birdbox-fixtures/rbac/model/views/users.js b/packages/cubejs-testing/birdbox-fixtures/rbac/model/views/users.js new file mode 100644 index 0000000000000..1c9488769f092 --- /dev/null +++ b/packages/cubejs-testing/birdbox-fixtures/rbac/model/views/users.js @@ -0,0 +1,16 @@ +view('users_view', { + cubes: [{ + join_path: users, + includes: '*', + }], + accessPolicy: [{ + role: '*', + rowLevel: { + filters: [{ + member: 'id', + operator: 'gt', + values: [10], + }], + }, + }] +}); 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 c6fd88b4ced72..b4a7ce5537530 100644 --- a/packages/cubejs-testing/test/__snapshots__/smoke-rbac.test.ts.snap +++ b/packages/cubejs-testing/test/__snapshots__/smoke-rbac.test.ts.snap @@ -8,6 +8,81 @@ Array [ ] `; +exports[`Cube RBAC Engine [Python config] RBAC via SQL API [python config] SELECT * from users_view: users_view_python 1`] = ` +Array [ + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 400, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 401, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 402, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 403, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 404, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 405, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 406, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 407, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 408, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 409, + }, +] +`; + exports[`Cube RBAC Engine [Python config][dev mode] products with no matching policy: products_no_policy_python 1`] = ` Array [ Object { @@ -611,6 +686,81 @@ Array [ ] `; +exports[`Cube RBAC Engine RBAC via SQL API SELECT * from users_view: users_view_js 1`] = ` +Array [ + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 400, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 401, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 402, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 403, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 404, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 405, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 406, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 407, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 408, + }, + Object { + "__cubeJoinField": null, + "__user": null, + "city": "Austin", + "count": "1", + "id": 409, + }, +] +`; + exports[`Cube RBAC Engine RBAC via SQL API default policy SELECT with member expressions: users_member_expression 1`] = ` Array [ Object { diff --git a/packages/cubejs-testing/test/smoke-rbac.test.ts b/packages/cubejs-testing/test/smoke-rbac.test.ts index 56823762e3eb5..f318711932a8c 100644 --- a/packages/cubejs-testing/test/smoke-rbac.test.ts +++ b/packages/cubejs-testing/test/smoke-rbac.test.ts @@ -152,6 +152,12 @@ describe('Cube RBAC Engine', () => { // Querying a cube with nested filters and mixed values should not cause any issues expect(res.rows).toMatchSnapshot('users'); }); + + test('SELECT * from users_view', async () => { + const res = await connection.query('SELECT * FROM users_view limit 10'); + // Make sure view policies are evaluated correctly in yaml schemas + expect(res.rows).toMatchSnapshot('users_view_js'); + }); }); describe('RBAC via SQL API manager', () => { @@ -398,6 +404,12 @@ describe('Cube RBAC Engine [Python config]', () => { // It should also exclude the `created_at` dimension as per memberLevel policy expect(res.rows).toMatchSnapshot('users_python'); }); + + test('SELECT * from users_view', async () => { + const res = await connection.query('SELECT * FROM users_view limit 10'); + // Make sure view policies are evaluated correctly in yaml schemas + expect(res.rows).toMatchSnapshot('users_view_python'); + }); }); });