Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
5 changes: 5 additions & 0 deletions packages/cubejs-api-gateway/src/gateway.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2169,6 +2169,11 @@ class ApiGateway {
}

public async contextByReq(req: Request, securityContext, requestId: string): Promise<ExtendedRequestContext> {
req.securityContext = {
Copy link
Member

Choose a reason for hiding this comment

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

First, I am sorry for the late reply.

@morford-brex Good catch! However, I believe it's correct to assign securityContext for req.securityContext instead of merging.

I assume that securityContext is correct for checkAuth in HTTP (see pic 1), while it's incorrect for WebSocket server and checkSqlAuth (SQL API).

I am OK with changing it once as the last mile instead of fixing it in (sql-server.ts#contextByNativeReq and SubscriptionServer.ts#processMessage), but I suggest using req.securityContext = securityContext instead of merging.

There is no need to merge it, because:

  1. HTTP checkAuth will receive the same object. this.contextByReq(req, req.securityContext, getRequestIdFromRequest(req)), see pic 1
  2. WS request doesn't have securityContext, it stores it separately
  3. SQL API request doesn't have securityContext, it stores it separately
Image

Thanks

Copy link
Contributor Author

@morford-brex morford-brex Oct 29, 2025

Choose a reason for hiding this comment

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

no worries at all - thanks for the follow up!

i think that makes sense and just updated here to skip the merging and directly set it

...req.securityContext,
...securityContext,
};

const extensions = typeof this.extendContext === 'function' ? await this.extendContext(req) : {};

return {
Expand Down
107 changes: 107 additions & 0 deletions packages/cubejs-api-gateway/test/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -673,4 +673,111 @@ describe('test authorization', () => {
// no warnings, done on checkAuth/checkAuthMiddleware level
expect(loggerMock.mock.calls.length).toEqual(0);
});

test('extendContext receives securityContext from checkAuth', async () => {
const loggerMock = jest.fn(() => {
//
});

const extendContextMock = jest.fn((req) => ({
securityContext: {
...req.securityContext,
extendedField: 'added_by_extend_context',
}
}));

const expectSecurityContext = (securityContext) => {
expect(securityContext.uid).toEqual(5);
expect(securityContext.extendedField).toEqual('added_by_extend_context');
expect(securityContext.iat).toBeDefined();
expect(securityContext.exp).toBeDefined();
};

const handlerMock = jest.fn((req, res) => {
expectSecurityContext(req.context.securityContext);
res.status(200).end();
});

const { app } = createApiGateway(handlerMock, loggerMock, {
extendContext: extendContextMock,
});

const token = generateAuthToken({ uid: 5 });

await request(app)
.get('/test-auth-fake')
.set('Authorization', `Authorization: ${token}`)
.expect(200);

expect(handlerMock.mock.calls.length).toEqual(1);
expect(extendContextMock.mock.calls.length).toEqual(1);

// should receive securityContext from checkAuth
expect(extendContextMock.mock.calls[0][0].securityContext).toMatchObject({
uid: 5,
iat: expect.any(Number),
exp: expect.any(Number),
});
expectSecurityContext(handlerMock.mock.calls[0][0].context.securityContext);
});

test('extendContext with custom checkAuth returning securityContext', async () => {
const loggerMock = jest.fn(() => {
//
});

const checkAuthMock = jest.fn(async (req: Request, auth?: string) => {
if (auth) {
const decoded = jwt.verify(auth, 'secret') as any;
return {
security_context: {
...decoded,
tenantId: 'tenant_123',
customField: 'from_check_auth',
}
};
}
return {};
});

const extendContextMock = jest.fn((req) => {
// should receive securityContext from checkAuth
expect(req.securityContext).toBeDefined();
expect(req.securityContext.customField).toEqual('from_check_auth');

return {
securityContext: {
...req.securityContext,
extendedField: 'from_extend_context',
}
};
});

const handlerMock = jest.fn((req, res) => {
expect(req.context.securityContext.customField).toEqual('from_check_auth');
expect(req.context.securityContext.extendedField).toEqual('from_extend_context');
res.status(200).end();
});

const { app } = createApiGateway(handlerMock, loggerMock, {
checkAuth: checkAuthMock,
extendContext: extendContextMock,
});

const token = generateAuthToken({ uid: 5 });

await request(app)
.get('/test-auth-fake')
.set('Authorization', `Authorization: ${token}`)
.expect(200);

expect(checkAuthMock.mock.calls.length).toEqual(1);
expect(extendContextMock.mock.calls.length).toEqual(1);
expect(handlerMock.mock.calls.length).toEqual(1);
expect(extendContextMock.mock.calls[0][0].securityContext).toMatchObject({
uid: 5,
tenantId: 'tenant_123',
customField: 'from_check_auth',
});
});
});
Loading