Skip to content

feat(api-gateway)!: Move '/v1/sql' to new sql API Scope #9406

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

Merged
merged 4 commits into from
Apr 11, 2025
Merged

feat(api-gateway)!: Move '/v1/sql' to new sql API Scope #9406

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
863ae6d
feat(api-gateway)!: Move '/v1/sql' to new `sql` API Scope
KSDaemon Mar 28, 2025
3efb6f6
fix tests
KSDaemon Mar 28, 2025
dfd2044
Update docs
igorlukanin Mar 30, 2025
afd787c
Fix docs
igorlukanin Apr 10, 2025
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
1 change: 1 addition & 0 deletions docs/pages/product/apis-integrations/rest-api.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ accessible for everyone.
| `meta` | [`/v1/meta`][ref-ref-meta] | ✅ Yes |
| `data` | [`/v1/load`][ref-ref-load], [`/v1/sql`][ref-ref-sql] | ✅ Yes |
| `graphql` | `/graphql` | ✅ Yes |
| `sql` | [`/v1/sql`][ref-ref-sql] | ✅ Yes |
| `jobs` | [`/v1/pre-aggregations/jobs`][ref-ref-paj] | ❌ No |
| No scope | `/livez`, `/readyz` | ✅ Yes, always |

Expand Down
4 changes: 2 additions & 2 deletions docs/pages/reference/configuration/config.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1008,13 +1008,13 @@ from cube import config

@config('context_to_api_scopes')
def context_to_api_scopes(context: dict, default_scopes: list[str]) -> list[str]:
return ['meta', 'data', 'graphql']
return ['meta', 'data', 'graphql', 'sql']
```

```javascript
module.exports = {
contextToApiScopes: (securityContext, defaultScopes) => {
return ['meta', 'data', 'graphql'];
return ['meta', 'data', 'graphql', 'sql'];
},
};
```
Expand Down
2 changes: 1 addition & 1 deletion docs/pages/reference/configuration/environment-variables.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,7 @@ endpoints.

| Possible Values | Default in Development | Default in Production |
| ------------------------------------------------------------------------------ | ---------------------- | --------------------- |
| A comma-delimited string with any combination of [API scopes][ref-rest-scopes] | `meta,data,graphql` | `meta,data,graphql` |
| A comma-delimited string with any combination of [API scopes][ref-rest-scopes] | `meta,data,graphql,sql`| `meta,data,graphql,sql`|

See also the [`context_to_api_scopes` configuration
option](/reference/configuration/config#context_to_api_scopes).
Expand Down
8 changes: 4 additions & 4 deletions packages/cubejs-api-gateway/src/gateway.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@
public readonly contextToApiScopesFn: ContextToApiScopesFn;

public readonly contextToApiScopesDefFn: ContextToApiScopesFn =
async () => ['graphql', 'meta', 'data'];
async () => ['graphql', 'meta', 'data', 'sql'];

protected readonly requestLoggerMiddleware: RequestLoggerMiddlewareFn;

Expand Down Expand Up @@ -1311,7 +1311,7 @@
res,
}: {query: string, disablePostProcessing: boolean} & BaseRequest) {
try {
await this.assertApiScope('data', context.securityContext);
await this.assertApiScope('sql', context.securityContext);

Check warning on line 1314 in packages/cubejs-api-gateway/src/gateway.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L1314 was not covered by tests

const result = await this.sqlServer.sql4sql(query, disablePostProcessing, context.securityContext);
res({ sql: result });
Expand Down Expand Up @@ -1339,7 +1339,7 @@
const requestStarted = new Date();

try {
await this.assertApiScope('data', context.securityContext);
await this.assertApiScope('sql', context.securityContext);

const [queryType, normalizedQueries] =
await this.getNormalizedQueries(query, context, disableLimitEnforcing, memberExpressions);
Expand Down Expand Up @@ -2448,7 +2448,7 @@
);
} else {
scopes.forEach((p) => {
if (['graphql', 'meta', 'data', 'jobs'].indexOf(p) === -1) {
if (['graphql', 'meta', 'data', 'sql', 'jobs'].indexOf(p) === -1) {
throw new Error(
`A user-defined contextToApiScopes function returns a wrong scope: ${p}`
);
Expand Down
1 change: 1 addition & 0 deletions packages/cubejs-api-gateway/src/types/strings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ type ApiScopes =
'graphql' |
'meta' |
'data' |
'sql' |
'jobs';

export {
Expand Down
43 changes: 29 additions & 14 deletions packages/cubejs-api-gateway/test/permissions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ describe('Gateway Api Scopes', () => {
expect(res.body && res.body.error)
.toStrictEqual('API scope is missing: data');

res = await request(app)
.get('/cubejs-api/v1/sql')
.set('Authorization', AUTH_TOKEN)
.expect(403);
expect(res.body && res.body.error)
.toStrictEqual('API scope is missing: sql');

res = await request(app)
.post('/cubejs-api/v1/pre-aggregations/jobs')
.set('Authorization', AUTH_TOKEN)
Expand Down Expand Up @@ -175,39 +182,47 @@ describe('Gateway Api Scopes', () => {
expect(res3.body && res3.body.error)
.toStrictEqual('API scope is missing: data');

const res4 = await request(app)
.get('/cubejs-api/v1/sql')
const res6 = await request(app)
.get('/cubejs-api/v1/dry-run')
.set('Authorization', AUTH_TOKEN)
.expect(403);

expect(res4.body && res4.body.error)
expect(res6.body && res6.body.error)
.toStrictEqual('API scope is missing: data');

const res5 = await request(app)
.post('/cubejs-api/v1/sql')
const res7 = await request(app)
.post('/cubejs-api/v1/dry-run')
.set('Content-type', 'application/json')
.set('Authorization', AUTH_TOKEN)
.expect(403);

expect(res5.body && res5.body.error)
expect(res7.body && res7.body.error)
.toStrictEqual('API scope is missing: data');

const res6 = await request(app)
.get('/cubejs-api/v1/dry-run')
apiGateway.release();
});

test('Sql declined', async () => {
const { app, apiGateway } = createApiGateway({
contextToApiScopes: async () => ['graphql', 'meta', 'jobs', 'data'],
});

const res1 = await request(app)
.get('/cubejs-api/v1/sql')
.set('Authorization', AUTH_TOKEN)
.expect(403);

expect(res6.body && res6.body.error)
.toStrictEqual('API scope is missing: data');
expect(res1.body && res1.body.error)
.toStrictEqual('API scope is missing: sql');

const res7 = await request(app)
.post('/cubejs-api/v1/dry-run')
const res2 = await request(app)
.post('/cubejs-api/v1/sql')
.set('Content-type', 'application/json')
.set('Authorization', AUTH_TOKEN)
.expect(403);

expect(res7.body && res7.body.error)
.toStrictEqual('API scope is missing: data');
expect(res2.body && res2.body.error)
.toStrictEqual('API scope is missing: sql');

apiGateway.release();
});
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -1140,7 +1140,7 @@ describe('OptsHandler class', () => {
const permissions = await gateway.contextToApiScopesFn();
expect(permissions).toBeDefined();
expect(Array.isArray(permissions)).toBeTruthy();
expect(permissions).toEqual(['graphql', 'meta', 'data']);
expect(permissions).toEqual(['graphql', 'meta', 'data', 'sql']);
});

test('must set env api scopes if fn not specified', async () => {
Expand Down
Loading