Skip to content

Commit ff89d65

Browse files
authored
fix(server-core): fix member level access policy evaluation (#8992)
Previous logic was incorrectly excluding members if any of the applicable policies exclude it. The right approach is to allow a member if any of the applicable policies allows it
1 parent 811a7ce commit ff89d65

File tree

4 files changed

+46
-43
lines changed

4 files changed

+46
-43
lines changed

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import crypto from 'crypto';
22
import R from 'ramda';
3-
import { getEnv } from '@cubejs-backend/shared';
43
import { createQuery, compile, queryClass, PreAggregations, QueryFactory } from '@cubejs-backend/schema-compiler';
54
import { v4 as uuidv4 } from 'uuid';
65
import { NativeInstance } from '@cubejs-backend/native';
@@ -439,17 +438,18 @@ export class CompilerApi {
439438
const applicablePolicies = await this.getApplicablePolicies(evaluatedCube, context, compilers);
440439

441440
const computeMemberVisibility = (item) => {
442-
let isIncluded = false;
443-
let isExplicitlyExcluded = false;
444441
for (const policy of applicablePolicies) {
445442
if (policy.memberLevel) {
446-
isIncluded = policy.memberLevel.includesMembers.includes(item.name) || isIncluded;
447-
isExplicitlyExcluded = policy.memberLevel.excludesMembers.includes(item.name) || isExplicitlyExcluded;
443+
if (policy.memberLevel.includesMembers.includes(item.name) &&
444+
!policy.memberLevel.excludesMembers.includes(item.name)) {
445+
return true;
446+
}
448447
} else {
449-
isIncluded = true;
448+
// If there's no memberLevel policy, we assume that all members are visible
449+
return true;
450450
}
451451
}
452-
return isIncluded && !isExplicitlyExcluded;
452+
return false;
453453
};
454454

455455
for (const dimension of cube.config.dimensions) {

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,10 @@ cube('line_items', {
5555
// This is to test dynamic values based on security context
5656
values: [`${security_context.auth?.userAttributes?.minDefaultId || 20000}`],
5757
}]
58-
}
58+
},
59+
memberLevel: {
60+
excludes: ['count', 'price', 'price_dim'],
61+
},
5962
},
6063
{
6164
role: 'admin',
@@ -69,7 +72,7 @@ cube('line_items', {
6972
allowAll: true,
7073
},
7174
memberLevel: {
72-
excludes: ['created_at'],
75+
excludes: ['price_dim'],
7376
},
7477
},
7578
{

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

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,46 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3-
exports[`Cube RBAC Engine RBAC via REST API line_items hidden created_at: line_items_view_no_policy_rest 1`] = `
3+
exports[`Cube RBAC Engine RBAC via REST API line_items hidden price_dim: line_items_view_no_policy_rest 1`] = `
44
Array [
55
Object {
6-
"line_items_view_no_policy.count": "1",
7-
"line_items_view_no_policy.created_at": "2016-01-01T23:00:21.000",
6+
"line_items_view_no_policy.count": "3",
7+
"line_items_view_no_policy.price_dim": 33,
88
},
99
Object {
1010
"line_items_view_no_policy.count": "1",
11-
"line_items_view_no_policy.created_at": "2016-02-27T06:09:59.000",
11+
"line_items_view_no_policy.price_dim": 34,
1212
},
1313
Object {
1414
"line_items_view_no_policy.count": "1",
15-
"line_items_view_no_policy.created_at": "2016-02-27T11:38:04.000",
15+
"line_items_view_no_policy.price_dim": 35,
1616
},
1717
Object {
18-
"line_items_view_no_policy.count": "1",
19-
"line_items_view_no_policy.created_at": "2016-03-06T17:41:43.000",
18+
"line_items_view_no_policy.count": "7",
19+
"line_items_view_no_policy.price_dim": 36,
2020
},
2121
Object {
22-
"line_items_view_no_policy.count": "1",
23-
"line_items_view_no_policy.created_at": "2016-03-07T19:41:43.000",
22+
"line_items_view_no_policy.count": "3",
23+
"line_items_view_no_policy.price_dim": 37,
2424
},
2525
Object {
26-
"line_items_view_no_policy.count": "1",
27-
"line_items_view_no_policy.created_at": "2016-03-13T22:34:22.000",
26+
"line_items_view_no_policy.count": "3",
27+
"line_items_view_no_policy.price_dim": 38,
2828
},
2929
Object {
30-
"line_items_view_no_policy.count": "1",
31-
"line_items_view_no_policy.created_at": "2016-03-21T21:13:47.000",
30+
"line_items_view_no_policy.count": "2",
31+
"line_items_view_no_policy.price_dim": 39,
3232
},
3333
Object {
34-
"line_items_view_no_policy.count": "1",
35-
"line_items_view_no_policy.created_at": "2016-05-01T07:26:55.000",
34+
"line_items_view_no_policy.count": "3",
35+
"line_items_view_no_policy.price_dim": 40,
3636
},
3737
Object {
38-
"line_items_view_no_policy.count": "1",
39-
"line_items_view_no_policy.created_at": "2016-05-13T07:03:15.000",
38+
"line_items_view_no_policy.count": "5",
39+
"line_items_view_no_policy.price_dim": 41,
4040
},
4141
Object {
42-
"line_items_view_no_policy.count": "1",
43-
"line_items_view_no_policy.created_at": "2016-05-15T22:44:26.000",
42+
"line_items_view_no_policy.count": "2",
43+
"line_items_view_no_policy.price_dim": 42,
4444
},
4545
]
4646
`;
@@ -98,80 +98,80 @@ Array [
9898
"__cubeJoinField": null,
9999
"__user": null,
100100
"count": "1",
101+
"created_at": 2018-10-23T07:54:39.000Z,
101102
"price": 267,
102-
"price_dim": 267,
103103
"quantity": 2,
104104
},
105105
Object {
106106
"__cubeJoinField": null,
107107
"__user": null,
108108
"count": "1",
109+
"created_at": 2018-01-01T13:50:20.000Z,
109110
"price": 263,
110-
"price_dim": 263,
111111
"quantity": 7,
112112
},
113113
Object {
114114
"__cubeJoinField": null,
115115
"__user": null,
116116
"count": "1",
117+
"created_at": 2017-05-13T21:23:08.000Z,
117118
"price": 180,
118-
"price_dim": 180,
119119
"quantity": 8,
120120
},
121121
Object {
122122
"__cubeJoinField": null,
123123
"__user": null,
124124
"count": "1",
125+
"created_at": 2018-04-10T22:51:15.000Z,
125126
"price": 169,
126-
"price_dim": 169,
127127
"quantity": 6,
128128
},
129129
Object {
130130
"__cubeJoinField": null,
131131
"__user": null,
132132
"count": "1",
133+
"created_at": 2017-07-16T15:00:34.000Z,
133134
"price": 156,
134-
"price_dim": 156,
135135
"quantity": 1,
136136
},
137137
Object {
138138
"__cubeJoinField": null,
139139
"__user": null,
140140
"count": "1",
141+
"created_at": 2019-05-23T04:25:27.000Z,
141142
"price": 36,
142-
"price_dim": 36,
143143
"quantity": 5,
144144
},
145145
Object {
146146
"__cubeJoinField": null,
147147
"__user": null,
148148
"count": "1",
149+
"created_at": 2018-09-29T20:29:30.000Z,
149150
"price": 245,
150-
"price_dim": 245,
151151
"quantity": 4,
152152
},
153153
Object {
154154
"__cubeJoinField": null,
155155
"__user": null,
156156
"count": "1",
157+
"created_at": 2019-04-17T03:32:54.000Z,
157158
"price": 232,
158-
"price_dim": 232,
159159
"quantity": 8,
160160
},
161161
Object {
162162
"__cubeJoinField": null,
163163
"__user": null,
164164
"count": "1",
165+
"created_at": 2019-11-15T18:22:17.000Z,
165166
"price": 63,
166-
"price_dim": 63,
167167
"quantity": 8,
168168
},
169169
Object {
170170
"__cubeJoinField": null,
171171
"__user": null,
172172
"count": "1",
173+
"created_at": 2019-12-16T08:09:36.000Z,
173174
"price": 68,
174-
"price_dim": 68,
175175
"quantity": 6,
176176
},
177177
]

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -203,12 +203,12 @@ describe('Cube RBAC Engine', () => {
203203
});
204204
});
205205

206-
test('line_items hidden created_at', async () => {
206+
test('line_items hidden price_dim', async () => {
207207
let query: Query = {
208208
measures: ['line_items.count'],
209-
dimensions: ['line_items.created_at'],
209+
dimensions: ['line_items.price_dim'],
210210
order: {
211-
'line_items.created_at': 'asc',
211+
'line_items.price_dim': 'asc',
212212
},
213213
};
214214
let error = '';
@@ -220,9 +220,9 @@ describe('Cube RBAC Engine', () => {
220220
expect(error).toContain('You requested hidden member');
221221
query = {
222222
measures: ['line_items_view_no_policy.count'],
223-
dimensions: ['line_items_view_no_policy.created_at'],
223+
dimensions: ['line_items_view_no_policy.price_dim'],
224224
order: {
225-
'line_items_view_no_policy.created_at': 'asc',
225+
'line_items_view_no_policy.price_dim': 'asc',
226226
},
227227
limit: 10,
228228
};

0 commit comments

Comments
 (0)