Skip to content

Commit f751af6

Browse files
committed
fix(api-gateway): cache GraphQL schemas per security context using visibilityMaskHash
1 parent 86cd74d commit f751af6

5 files changed

Lines changed: 225 additions & 20 deletions

File tree

packages/cubejs-api-gateway/src/gateway.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -290,14 +290,25 @@ class ApiGateway {
290290
);
291291

292292
const compilerApi = await this.getCompilerApi(req.context);
293-
let schema = compilerApi.getGraphQLSchema();
293+
294+
// In devMode/playground, all fields are shown regardless of RBAC
295+
const showAll = getEnv('devMode') || req.context.signedWithPlaygroundAuthSecret;
296+
297+
// Get metaConfig with visibilityMaskHash for RBAC-based schema caching
298+
const metaConfigResult = await compilerApi.metaConfig(req.context, {
299+
requestId: req.context.requestId,
300+
includeVisibilityMaskHash: true,
301+
});
302+
const { visibilityMaskHash, cubes: metaConfigCubes } = metaConfigResult;
303+
304+
// Cache key: 'all' for devMode (RBAC doesn't matter), or visibilityMaskHash for RBAC-filtered schemas
305+
const cacheKey = showAll ? 'all' : (visibilityMaskHash || 'default');
306+
307+
let schema = compilerApi.getGraphQLSchema(cacheKey);
294308
if (!schema) {
295-
let metaConfig = await compilerApi.metaConfig(req.context, {
296-
requestId: req.context.requestId,
297-
});
298-
metaConfig = this.filterVisibleItemsInMeta(req.context, metaConfig);
299-
schema = makeSchema(metaConfig);
300-
compilerApi.setGraphQLSchema(schema);
309+
const filteredMeta = this.filterVisibleItemsInMeta(req.context, metaConfigCubes);
310+
schema = makeSchema(filteredMeta);
311+
compilerApi.setGraphQLSchema(schema, cacheKey);
301312
}
302313
return graphqlHTTP({
303314
schema,

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

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ export class CompilerApi {
125125

126126
protected compiledScriptCacheInterval?: NodeJS.Timeout;
127127

128-
protected graphqlSchema?: GraphQLSchema;
128+
protected graphqlSchemas: Map<string, GraphQLSchema> = new Map();
129129

130130
protected compilers?: Promise<Compiler>;
131131

@@ -193,15 +193,15 @@ export class CompilerApi {
193193
// using safeguard for potential dangling references.
194194
this.compilers = disposedProxy('compilers', 'disposed CompilerApi instance');
195195
this.queryFactory = disposedProxy('queryFactory', 'disposed CompilerApi instance');
196-
this.graphqlSchema = undefined;
196+
this.graphqlSchemas.clear();
197197
}
198198

199-
public setGraphQLSchema(schema: GraphQLSchema): void {
200-
this.graphqlSchema = schema;
199+
public setGraphQLSchema(schema: GraphQLSchema, cacheKey: string = 'default'): void {
200+
this.graphqlSchemas.set(cacheKey, schema);
201201
}
202202

203-
public getGraphQLSchema(): GraphQLSchema {
204-
return this.graphqlSchema;
203+
public getGraphQLSchema(cacheKey: string = 'default'): GraphQLSchema | undefined {
204+
return this.graphqlSchemas.get(cacheKey);
205205
}
206206

207207
public createNativeInstance(): NativeInstance {
@@ -959,25 +959,29 @@ export class CompilerApi {
959959

960960
public async metaConfig(
961961
requestContext: Context,
962-
options: { includeCompilerId?: boolean; requestId?: string } = {}
962+
options: { includeCompilerId?: boolean; includeVisibilityMaskHash?: boolean; requestId?: string } = {}
963963
): Promise<any> {
964-
const { includeCompilerId, ...restOptions } = options;
964+
const { includeCompilerId, includeVisibilityMaskHash, ...restOptions } = options;
965965
const compilers = await this.getCompilers(restOptions);
966966
const { cubes } = compilers.metaTransformer;
967967
const { visibilityMaskHash, cubes: patchedCubes } = await this.patchVisibilityByAccessPolicy(
968968
compilers,
969969
requestContext,
970970
cubes
971971
);
972-
if (includeCompilerId) {
973-
return {
974-
cubes: patchedCubes,
972+
if (includeCompilerId || includeVisibilityMaskHash) {
973+
const result: any = { cubes: patchedCubes };
974+
if (includeCompilerId) {
975975
// This compilerId is primarily used by the cubejs-backend-native or caching purposes.
976976
// By default, it doesn't account for member visibility changes introduced above by DAP.
977977
// Here we're modifying the original compilerId in a way that it's distinct for
978978
// distinct schema versions while still being a valid UUID.
979-
compilerId: visibilityMaskHash ? this.mixInVisibilityMaskHash(compilers.compilerId, visibilityMaskHash) : compilers.compilerId,
980-
};
979+
result.compilerId = visibilityMaskHash ? this.mixInVisibilityMaskHash(compilers.compilerId, visibilityMaskHash) : compilers.compilerId;
980+
}
981+
if (includeVisibilityMaskHash) {
982+
result.visibilityMaskHash = visibilityMaskHash;
983+
}
984+
return result;
981985
}
982986
return patchedCubes;
983987
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Cube.js configuration for testing GraphQL schema caching with different security contexts
2+
module.exports = {
3+
// Map security context to RBAC roles
4+
contextToRoles: ({ securityContext }) => securityContext?.auth?.roles || ['*'],
5+
6+
// SAME app ID for all tenants - this forces them to share a CompilerApi instance
7+
// which is where the GraphQL schema caching bug manifests
8+
contextToAppId: () => 'CUBEJS_APP_shared',
9+
};
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// Orders cube with RBAC-based field visibility using access policies
2+
// This tests the GraphQL schema caching bug where different tenants should see different fields
3+
4+
cube('Orders', {
5+
sql: `SELECT 1 as id, 100 as amount, 'secret123' as internal_code, 'premium' as tier`,
6+
7+
measures: {
8+
count: {
9+
type: 'count',
10+
},
11+
totalAmount: {
12+
sql: 'amount',
13+
type: 'sum',
14+
},
15+
},
16+
17+
dimensions: {
18+
id: {
19+
sql: 'id',
20+
type: 'number',
21+
primaryKey: true,
22+
},
23+
amount: {
24+
sql: 'amount',
25+
type: 'number',
26+
},
27+
// This field should only be visible to tenant-a
28+
internalCode: {
29+
sql: 'internal_code',
30+
type: 'string',
31+
},
32+
// This field should only be visible to tenant-b
33+
tier: {
34+
sql: 'tier',
35+
type: 'string',
36+
},
37+
},
38+
39+
// RBAC access policies - these apply visibility masks at runtime
40+
accessPolicy: [
41+
{
42+
// Default policy for all roles - hide special fields
43+
role: '*',
44+
memberLevel: {
45+
excludes: ['internalCode', 'tier'],
46+
},
47+
},
48+
{
49+
// tenant-a can see internalCode (include all, exclude only tier)
50+
role: 'tenant-a',
51+
memberLevel: {
52+
includes: '*',
53+
excludes: ['tier'],
54+
},
55+
},
56+
{
57+
// tenant-b can see tier (include all, exclude only internalCode)
58+
role: 'tenant-b',
59+
memberLevel: {
60+
includes: '*',
61+
excludes: ['internalCode'],
62+
},
63+
},
64+
],
65+
});
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/**
2+
* Integration test for GraphQL schema caching per security context.
3+
*
4+
* This test verifies that when different users (with different RBAC roles)
5+
* make GraphQL requests, they each get a schema appropriate to their
6+
* access level - not a cached schema from another user.
7+
*
8+
* The fix uses `visibilityMaskHash` as the cache key for GraphQL schemas.
9+
*/
10+
11+
// eslint-disable-next-line import/no-extraneous-dependencies
12+
import { afterAll, beforeAll, jest, expect } from '@jest/globals';
13+
import { sign } from 'jsonwebtoken';
14+
import fetch from 'node-fetch';
15+
16+
import { BirdBox, getBirdbox } from '../src';
17+
import {
18+
DEFAULT_CONFIG,
19+
JEST_AFTER_ALL_DEFAULT_TIMEOUT,
20+
JEST_BEFORE_ALL_DEFAULT_TIMEOUT,
21+
} from './smoke-tests';
22+
23+
describe('GraphQL Schema Caching per Security Context', () => {
24+
jest.setTimeout(60 * 5 * 1000);
25+
let birdbox: BirdBox;
26+
27+
beforeAll(async () => {
28+
birdbox = await getBirdbox(
29+
'duckdb',
30+
{
31+
...DEFAULT_CONFIG,
32+
CUBEJS_DEV_MODE: 'false',
33+
NODE_ENV: 'production',
34+
CUBEJS_DB_TYPE: 'duckdb',
35+
},
36+
{
37+
schemaDir: 'graphql-rbac/model',
38+
cubejsConfig: 'graphql-rbac/cube.js',
39+
}
40+
);
41+
}, JEST_BEFORE_ALL_DEFAULT_TIMEOUT);
42+
43+
afterAll(async () => {
44+
await birdbox.stop();
45+
}, JEST_AFTER_ALL_DEFAULT_TIMEOUT);
46+
47+
async function graphqlIntrospection(role: string): Promise<any> {
48+
const token = sign({
49+
auth: { roles: [role] },
50+
}, DEFAULT_CONFIG.CUBEJS_API_SECRET, { expiresIn: '1h' });
51+
52+
// apiUrl includes /cubejs-api/v1, but GraphQL is at /cubejs-api/graphql
53+
const baseUrl = birdbox.configuration.apiUrl.replace('/cubejs-api/v1', '');
54+
const res = await fetch(`${baseUrl}/cubejs-api/graphql`, {
55+
method: 'POST',
56+
headers: {
57+
'Content-Type': 'application/json',
58+
'Authorization': `Bearer ${token}`,
59+
},
60+
body: JSON.stringify({
61+
query: '{ __type(name: "OrdersMembers") { fields { name } } }',
62+
}),
63+
});
64+
return res.json();
65+
}
66+
67+
test('tenant-a sees internalCode but not tier', async () => {
68+
const result = await graphqlIntrospection('tenant-a');
69+
const fields = result.data?.__type?.fields?.map((f: any) => f.name) || [];
70+
71+
expect(fields).toContain('internalCode');
72+
expect(fields).not.toContain('tier');
73+
});
74+
75+
test('tenant-b sees tier but not internalCode', async () => {
76+
const result = await graphqlIntrospection('tenant-b');
77+
const fields = result.data?.__type?.fields?.map((f: any) => f.name) || [];
78+
79+
expect(fields).toContain('tier');
80+
expect(fields).not.toContain('internalCode');
81+
});
82+
83+
test('alternating requests maintain correct schemas', async () => {
84+
// Request A -> B -> A -> B to verify caching works correctly
85+
const resultA1 = await graphqlIntrospection('tenant-a');
86+
const resultB1 = await graphqlIntrospection('tenant-b');
87+
const resultA2 = await graphqlIntrospection('tenant-a');
88+
const resultB2 = await graphqlIntrospection('tenant-b');
89+
90+
const fieldsA1 = resultA1.data?.__type?.fields?.map((f: any) => f.name) || [];
91+
const fieldsB1 = resultB1.data?.__type?.fields?.map((f: any) => f.name) || [];
92+
const fieldsA2 = resultA2.data?.__type?.fields?.map((f: any) => f.name) || [];
93+
const fieldsB2 = resultB2.data?.__type?.fields?.map((f: any) => f.name) || [];
94+
95+
// A should always see internalCode, never tier
96+
expect(fieldsA1).toContain('internalCode');
97+
expect(fieldsA1).not.toContain('tier');
98+
expect(fieldsA2).toEqual(fieldsA1);
99+
100+
// B should always see tier, never internalCode
101+
expect(fieldsB1).toContain('tier');
102+
expect(fieldsB1).not.toContain('internalCode');
103+
expect(fieldsB2).toEqual(fieldsB1);
104+
});
105+
106+
test('default role sees neither internalCode nor tier', async () => {
107+
const result = await graphqlIntrospection('default');
108+
const fields = result.data?.__type?.fields?.map((f: any) => f.name) || [];
109+
110+
expect(fields).not.toContain('internalCode');
111+
expect(fields).not.toContain('tier');
112+
// Should still see basic fields
113+
expect(fields).toContain('count');
114+
expect(fields).toContain('amount');
115+
});
116+
});

0 commit comments

Comments
 (0)