Skip to content

feat: Warn when multitenancy omits contextToOrchestratorId #11411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ovr wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: Warn when multitenancy omits contextToOrchestratorId #11411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions docs-mintlify/recipes/configuration/multiple-sources-same-schema.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,39 @@ the local database (bootstrapped in the `docker-compose.yml` file) which has the
same data model.

To enable multitenancy, use the
[`contextToAppId`](/reference/configuration/config#context_to_app_id) function to
provide distinct identifiers for each tenant. Also, implement the
[`contextToAppId`](/reference/configuration/config#context_to_app_id) and
[`contextToOrchestratorId`](/reference/configuration/config#context_to_orchestrator_id)
functions to provide distinct identifiers for each tenant. Also, implement the
[`driverFactory`](/reference/configuration/config#driver_factory) function where
you can select a data source based on the tenant name.
[JSON Web Token](/docs/data-modeling/access-control) includes information about the tenant name in
the `tenant` property of the `securityContext`.

<Warning>

Whenever `driverFactory` picks a connection based on the `securityContext`, you
must also define `contextToOrchestratorId` and derive it from the same tenant
identifier. `driverFactory` is called
[once per data source for every orchestrator id](/reference/configuration/config#driver_factory),
and the orchestrator id defaults to a single global value for all tenants — so
without it, every tenant reuses the database connection resolved for whichever
tenant queried first, and one tenant receives another tenant's data.

</Warning>

```javascript
module.exports = {
// Provides distinct identifiers for each tenant which are used as caching keys
// for the data model compilation results
contextToAppId: ({ securityContext }) =>
`CUBE_APP_${securityContext.tenant}`,

// Caching key for database connections, execution queues and pre-aggregation
// table caches. Must be tenant-specific because driverFactory below selects a
// connection based on the security context
contextToOrchestratorId: ({ securityContext }) =>
`CUBE_APP_${securityContext.tenant}`,

// Selects the database connection configuration based on the tenant name
driverFactory: ({ securityContext }) => {
if (!securityContext.tenant) {
Expand Down
15 changes: 15 additions & 0 deletions docs-mintlify/reference/configuration/config.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ It's a [multitenancy][ref-multitenancy] option.
caching key for various in-memory structures like data model compilation
results, etc.

It does not key database connections, execution queues, or pre-aggregation table
caches — those use [`context_to_orchestrator_id`][self-orchestrator-id], which
you must also define if [`driver_factory`][self-driver-factory] selects a
connection based on the security context.

Comment on lines 55 to +62

@claude claude Bot Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium — this correctly contradicts another docs page, which should be fixed in the same PR.

The new sentence ("It does not key database connections…") matches the implementation: drivers are memoized inside the closure created per orchestratorId in getOrchestratorApi (packages/cubejs-server-core/src/core/server.ts:572-620), never per app id.

But docs-mintlify/embedding/multitenancy.mdx:282-284 still says the opposite:

The App ID (the result of contextToAppId) is used as a caching key for various in-memory structures like data model compilation results, connection pool.

That's exactly the misconception this PR is trying to kill, on the primary multitenancy page. Worth dropping "connection pool" from that sentence here so the two pages agree.

Fix this →

Called on each request.

<CodeGroup>
Expand Down Expand Up @@ -321,6 +326,15 @@ execution queues, pre-aggregation table caches. By default, the same instance is
used for **all** tenants; override this property in situations where each tenant
requires their own Query Orchestrator.

Overriding it is **required** whenever [`driver_factory`][self-driver-factory]
selects a connection based on the security context, and the id must be derived
from the same tenant identifier. Setting
[`context_to_app_id`][self-opts-ctx-to-appid] alone is not enough: the app id keys
the data model compilation cache, while the driver is resolved once per data
source per *orchestrator* id. With the default single global orchestrator id,
every tenant reuses the connection resolved for whichever tenant queried first,
so one tenant receives another tenant's data.
Comment on lines +329 to +336

@claude claude Bot Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low — "required" is stated a bit more absolutely than the mechanism warrants.

Two edges where the blanket "required … whenever driver_factory selects a connection based on the security context" isn't quite the whole story:

  • A driver_factory that branches on securityContext but returns configurations that are equivalent for the tenants involved isn't broken; conversely a factory that ignores securityContext entirely is safe even in a heavily multitenant setup. The precise rule is "the connection returned must not vary within one orchestrator id".
  • This paragraph now sits immediately above the existing pre_aggregations_schema <Warning>, which is good, but the causality reads backwards to a reader arriving from the recipe: they're told they must override context_to_orchestrator_id, then told overriding it needs another override. Consider one sentence linking them explicitly ("because you're now creating one orchestrator per tenant, also override pre_aggregations_schema — see below") so nobody applies half the change.


<Warning>

Please remember to override
Expand Down Expand Up @@ -1511,6 +1525,7 @@ module.exports = {
[ref-rest-scopes]: /reference/core-data-apis/rest-api#api-scopes
[ref-config-options]: /admin/connect-to-data#configuration-options
[self-orchestrator-id]: #context_to_orchestrator_id
[self-driver-factory]: #driver_factory
[ref-multiple-data-sources]: /admin/connect-to-data/multiple-data-sources
[ref-websockets]: /recipes/core-data-api/real-time-data-fetch
[ref-matching-preaggs]: /docs/pre-aggregations/matching-pre-aggregations
Expand Down
8 changes: 8 additions & 0 deletions examples/recipes/multiple-data-sources/cube.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,17 @@ const PostgresDriver = require('@cubejs-backend/postgres-driver');

module.exports = {
// Provides distinct identifiers for each tenant which are used as caching keys
// for the data model compilation results
contextToAppId: ({ securityContext }) =>
`CUBEJS_APP_${securityContext.tenant}`,

// Caching key for database connections, execution queues and pre-aggregation
// table caches. It must be tenant-specific whenever driverFactory selects a
// connection based on the security context, otherwise every tenant shares the
// driver resolved for whichever tenant queried first.
contextToOrchestratorId: ({ securityContext }) =>
`CUBEJS_APP_${securityContext.tenant}`,

// Selects the database connection configuration based on the tenant name
driverFactory: ({ securityContext }) => {

Expand Down
14 changes: 14 additions & 0 deletions packages/cubejs-server-core/src/core/OptsHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,20 @@ export class OptsHandler {
});
}

if (
opts.contextToAppId &&
!opts.contextToOrchestratorId &&
this.isCustomDriverFactory()
) {
this.core.logger('Multitenancy Without ContextToOrchestratorId', {
warning: (
'You are using multitenancy with a custom driverFactory but without ' +
'configuring contextToOrchestratorId: ' +
'https://cube.dev/docs/reference/configuration/config#context_to_orchestrator_id'
),
});
}
Comment on lines +458 to +470

@claude claude Bot Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High — this will fire (and give harmful advice) for a large class of safe configurations.

The condition is contextToAppId && !contextToOrchestratorId && isCustomDriverFactory(). Since dbType was removed in v1.7.0, a custom driverFactory is now the normal way to configure a single database — including for tenants that share one physical DB and isolate via queryRewrite / securityContext filters. Those setups are correct today, and:

  • they will now see a scary warning on every boot, and
  • if they follow it, they fragment one orchestrator into N (connection pool + queue + pre-agg cache per tenant) and inherit the preAggregationsSchema clash problem documented in config.mdx — i.e. the warning pushes them toward a worse configuration.

The actual unsafe condition is "driverFactory result depends on securityContext", which isCustomDriverFactory() doesn't approximate. Two options:

  1. Narrow the check with a cheap heuristic on the user-supplied factory (available as this.createOptions.driverFactory), e.g. only warn when its source mentions securityContext: 
    const userFactory = this.createOptions.driverFactory;
const factoryUsesSecurityContext =
  typeof userFactory === 'function' &&
  /securityContext/.test(userFactory.toString());
    Not airtight (a factory could read the context indirectly), but for a warning it removes the bulk of false positives.
  2. If you'd rather keep it broad, at least make the message conditional in tone — "if your driverFactory selects a connection based on securityContext, you must also configure contextToOrchestratorId; if it returns the same configuration for all tenants you can ignore this" — and mention that overriding contextToOrchestratorId also requires overriding preAggregationsSchema. As written the message reads as an unconditional misconfiguration.

Fix this →


if (options.devServer && !options.apiSecret) {
options.apiSecret = crypto.randomBytes(16).toString('hex');
displayCLIWarning(
Expand Down
58 changes: 58 additions & 0 deletions packages/cubejs-server-core/test/unit/OptsHandler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1202,4 +1202,62 @@ describe('OptsHandler class', () => {
expect(Array.isArray(permissions)).toBeTruthy();
expect(permissions).toEqual(['graphql', 'meta', 'data', 'jobs']);
});

describe('contextToOrchestratorId warning', () => {
const WARNING_MSG = 'Multitenancy Without ContextToOrchestratorId';

const tenantId = ({ securityContext }: any) => (
`CUBE_APP_${securityContext.tenant}`
);

const buildCore = (opts: CreateOptions) => {
const logger = jest.fn();
const core = new CubejsServerCoreExposed({
...conf,
logger,
scheduledRefreshTimer: false,
...opts,
});

expect(core.options).toBeDefined();

return logger.mock.calls.filter((call) => call[0] === WARNING_MSG);
};

test('must warn on tenant-specific driverFactory without contextToOrchestratorId', () => {
const warnings = buildCore({
contextToAppId: tenantId,
driverFactory: ({ securityContext }: any) => ({
type: <DatabaseType>'postgres',
database: `tenant_${securityContext.tenant}`,
}),
});

expect(warnings).toHaveLength(1);
expect(warnings[0][1].warning).toContain('contextToOrchestratorId');
});

test('must not warn when contextToOrchestratorId is configured', () => {
const warnings = buildCore({
contextToAppId: tenantId,
contextToOrchestratorId: tenantId,
driverFactory: ({ securityContext }: any) => ({
type: <DatabaseType>'postgres',
database: `tenant_${securityContext.tenant}`,
}),
});

expect(warnings).toHaveLength(0);
});

test('must not warn without a custom driverFactory', () => {
process.env.CUBEJS_DB_TYPE = 'postgres';

const warnings = buildCore({
contextToAppId: tenantId,
});

expect(warnings).toHaveLength(0);
});
});
Comment on lines +1253 to +1262

@claude claude Bot Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low — test gaps and env leakage.

  • process.env.CUBEJS_DB_TYPE = 'postgres' is set without restoring it. It's consistent with the rest of this file, so not blocking, but this is the last describe in the file and the assignment is what makes the "no custom driverFactory" path reachable — an afterEach(() => { delete process.env.CUBEJS_DB_TYPE; }) inside this block would keep it self-contained.
  • Missing the case that matters most for false positives: a static driverFactory (one that ignores securityContext) plus contextToAppId. Today that warns; per my comment on OptsHandler.ts it arguably shouldn't. Whichever behaviour you settle on, pinning it with a test would document the intent.
  • Also uncovered: driverFactory present but no contextToAppId (single-tenant) — currently silent, which is intended but untested.

});
Loading