Skip to content

Commit 1bffe2e

Browse files
committed
code review fixes
1 parent e589e7e commit 1bffe2e

File tree

4 files changed

+40
-25
lines changed

4 files changed

+40
-25
lines changed

packages/cubejs-schema-compiler/src/compiler/CompilerCache.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { QueryCache } from '../adapter/QueryCache';
44
export class CompilerCache extends QueryCache {
55
protected readonly queryCache: LRUCache<string, QueryCache>;
66

7+
protected readonly rbacCache: LRUCache<string, any>;
8+
79
public constructor({ maxQueryCacheSize, maxQueryCacheAge }) {
810
super();
911

@@ -12,6 +14,15 @@ export class CompilerCache extends QueryCache {
1214
maxAge: (maxQueryCacheAge * 1000) || 1000 * 60 * 10,
1315
updateAgeOnGet: true
1416
});
17+
18+
this.rbacCache = new LRUCache({
19+
max: 10000,
20+
maxAge: 1000 * 60 * 5, // 5 minutes
21+
});
22+
}
23+
24+
public getRbacCacheInstance(): LRUCache<string, any> {
25+
return this.rbacCache;
1526
}
1627

1728
public getQueryCache(key: unknown): QueryCache {

packages/cubejs-schema-compiler/src/compiler/CubeSymbols.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const CONTEXT_SYMBOLS = {
1313
// SECURITY_CONTEXT has been deprecated, however security_context (lowecase)
1414
// is allowed in RBAC policies for query-time attribute matching
1515
security_context: 'securityContext',
16+
securityContext: 'securityContext',
1617
FILTER_PARAMS: 'filterParams',
1718
FILTER_GROUP: 'filterGroup',
1819
SQL_UTILS: 'sqlUtils'

packages/cubejs-schema-compiler/test/unit/yaml-schema.test.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -374,11 +374,16 @@ describe('Yaml Schema Testing', () => {
374374
- member: status
375375
operator: equals
376376
values: ["completed"]
377-
- member: "{CUBE}.created_at"
378-
operator: notInDateRange
379-
values:
380-
- 2022-01-01
381-
- "{ security_context.currentDate }"
377+
- or:
378+
- member: "{CUBE}.created_at"
379+
operator: notInDateRange
380+
values:
381+
- 2022-01-01
382+
- "{ security_context.currentDate }"
383+
- member: "created_at"
384+
operator: equals
385+
values:
386+
- "{ securityContext.currentDate }"
382387
memberLevel:
383388
includes:
384389
- status

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

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,6 @@ export class CompilerApi {
3030
this.sqlCache = options.sqlCache;
3131
this.standalone = options.standalone;
3232
this.nativeInstance = this.createNativeInstance();
33-
this.rbacEvaluationCache = new LRUCache({
34-
max: 10000,
35-
maxAge: 1000 * 60 * 5, // 5 minutes
36-
});
3733
}
3834

3935
setGraphQLSchema(schema) {
@@ -227,20 +223,21 @@ export class CompilerApi {
227223
return context.__hash;
228224
}
229225

230-
async getApplicablePolicies(cube, context, cubeEvaluator) {
226+
async getApplicablePolicies(cube, context, compilers) {
227+
const cache = compilers.compilerCache.getRbacCacheInstance();
231228
const cacheKey = `${cube.name}_${this.hashRequestContext(context)}`;
232-
if (!this.rbacEvaluationCache.has(cacheKey)) {
229+
if (!cache.has(cacheKey)) {
233230
const userRoles = await this.getRolesFromContext(context);
234231
const policies = cube.accessPolicy.filter(policy => {
235232
const evaluatedConditions = (policy.conditions || []).map(
236-
condition => cubeEvaluator.evaluateContextFunction(cube, condition.if, context)
233+
condition => compilers.cubeEvaluator.evaluateContextFunction(cube, condition.if, context)
237234
);
238235
const res = this.userHasRole(userRoles, policy.role) && this.roleMeetsConditions(evaluatedConditions);
239236
return res;
240237
});
241-
this.rbacEvaluationCache.set(cacheKey, policies);
238+
cache.set(cacheKey, policies);
242239
}
243-
return this.rbacEvaluationCache.get(cacheKey);
240+
return cache.get(cacheKey);
244241
}
245242

246243
evaluateNestedFilter(filter, cube, context, cubeEvaluator) {
@@ -277,7 +274,8 @@ export class CompilerApi {
277274
* - combining cube and view filters with AND
278275
*/
279276
async applyRowLevelSecurity(query, context) {
280-
const { cubeEvaluator } = await this.getCompilers({ requestId: query.requestId });
277+
const compilers = await this.getCompilers({ requestId: query.requestId });
278+
const { cubeEvaluator } = compilers;
281279

282280
if (!cubeEvaluator.isRbacEnabled()) {
283281
return query;
@@ -297,7 +295,7 @@ export class CompilerApi {
297295

298296
if (cubeEvaluator.isRbacEnabledForCube(cube)) {
299297
let hasRoleWithAccess = false;
300-
const userPolicies = await this.getApplicablePolicies(cube, context, cubeEvaluator);
298+
const userPolicies = await this.getApplicablePolicies(cube, context, compilers);
301299

302300
for (const policy of userPolicies) {
303301
hasRoleWithAccess = true;
@@ -323,7 +321,7 @@ export class CompilerApi {
323321
query.segments.push({
324322
expression: () => '1 = 0',
325323
cubeName: cube.name,
326-
name: 'RLS Access Denied',
324+
name: 'rlsAccessDenied',
327325
});
328326
// If we hit this condition there's no need to evaluate the rest of the policy
329327
break;
@@ -336,7 +334,6 @@ export class CompilerApi {
336334
viewFiltersPerCubePerRole,
337335
hasAllowAllForCube
338336
);
339-
console.lg('RLS Filter', JSON.stringify(rlsFilter, null, 2));
340337
query.filters = query.filters || [];
341338
query.filters.push(rlsFilter);
342339
return query;
@@ -430,8 +427,9 @@ export class CompilerApi {
430427
* It evaluates all applicable memeberLevel accessPolicies givean a context
431428
* and retains members that are allowed by any policy (most permissive set).
432429
*/
433-
async filterVisibilityByAccessPolicy(cubeEvaluator, context, cubes) {
430+
async filterVisibilityByAccessPolicy(compilers, context, cubes) {
434431
const isMemberVisibleInContext = {};
432+
const { cubeEvaluator } = compilers;
435433

436434
if (!cubeEvaluator.isRbacEnabled()) {
437435
return cubes;
@@ -440,7 +438,7 @@ export class CompilerApi {
440438
for (const cube of cubes) {
441439
const evaluatedCube = cubeEvaluator.cubeFromPath(cube.config.name);
442440
if (cubeEvaluator.isRbacEnabledForCube(evaluatedCube)) {
443-
const applicablePolicies = await this.getApplicablePolicies(evaluatedCube, context, cubeEvaluator);
441+
const applicablePolicies = await this.getApplicablePolicies(evaluatedCube, context, compilers);
444442

445443
const computeMemberVisibility = (item) => {
446444
let isIncluded = false;
@@ -498,7 +496,7 @@ export class CompilerApi {
498496
const compilers = await this.getCompilers(restOptions);
499497
const { cubes } = compilers.metaTransformer;
500498
const filteredCubes = await this.filterVisibilityByAccessPolicy(
501-
compilers.cubeEvaluator,
499+
compilers,
502500
requestContext,
503501
cubes
504502
);
@@ -512,15 +510,15 @@ export class CompilerApi {
512510
}
513511

514512
async metaConfigExtended(requestContext, options) {
515-
const { metaTransformer, cubeEvaluator } = await this.getCompilers(options);
513+
const compilers = await this.getCompilers(options);
516514
const filteredCubes = await this.filterVisibilityByAccessPolicy(
517-
cubeEvaluator,
515+
compilers,
518516
requestContext,
519-
metaTransformer?.cubes
517+
compilers.metaTransformer?.cubes
520518
);
521519
return {
522520
metaConfig: filteredCubes,
523-
cubeDefinitions: metaTransformer?.cubeEvaluator?.cubeDefinitions,
521+
cubeDefinitions: compilers.metaTransformer?.cubeEvaluator?.cubeDefinitions,
524522
};
525523
}
526524

0 commit comments

Comments
 (0)