Skip to content

Commit

Permalink
BREAKING CHANGE: Remove support for checkAuthMiddleware
Browse files Browse the repository at this point in the history
checkAuthMiddleware option was deprecated in 0.26.0, because this option was tightly bound to Express. Since Cube.js supports HTTP **and** WebSockets as transports, we want our authentication API to not rely on transport-specific details. We now recommend using checkAuth.
  • Loading branch information
ovr committed Sep 11, 2024
1 parent 8103896 commit f5d7eed
Show file tree
Hide file tree
Showing 8 changed files with 4 additions and 168 deletions.
4 changes: 2 additions & 2 deletions DEPRECATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ features:
| Removed | [`contextToDataSourceId`](#contexttodatasourceid) | v0.25.0 | v0.25.0 |
| Removed | [Absolute import for `@cubejs-backend/server-core`](#absolute-import-for-@cubejs-backendserver-core) | v0.25.4 | v0.32.0 |
| Removed | [Absolute import for `@cubejs-backend/schema-compiler`](#absolute-import-for-@cubejs-backendschema-compiler) | v0.25.21 | v0.32.0 |
| Deprecated | [`checkAuthMiddleware`](#checkauthmiddleware) | v0.26.0 | |
| Deprecated | [`checkAuthMiddleware`](#checkauthmiddleware) | v0.26.0 | v0.36.0 |
| Removed | [Node.js 10](#nodejs-10) | v0.26.0 | v0.29.0 |
| Removed | [Node.js 15](#nodejs-15) | v0.26.0 | v0.32.0 |
| Deprecated | [`USER_CONTEXT`](#user_context) | v0.26.0 | |
Expand Down Expand Up @@ -179,7 +179,7 @@ const { BaseQuery } = require("@cubejs-backend/schema-compiler");

### `checkAuthMiddleware`

**Deprecated in Release: v0.26.0**
**Removed in Release: v0.36.0**

The `checkAuthMiddleware` option was tightly bound to Express,
[which has been deprecated](#embedding-cubejs-within-express). Since Cube.js
Expand Down
42 changes: 1 addition & 41 deletions packages/cubejs-api-gateway/src/gateway.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ import {
ApiGatewayOptions,
} from './types/gateway';
import {
CheckAuthMiddlewareFn,
RequestLoggerMiddlewareFn,
ContextRejectionMiddlewareFn,
ContextAcceptorFn,
Expand Down Expand Up @@ -146,8 +145,6 @@ class ApiGateway {
protected readonly contextToApiScopesDefFn: ContextToApiScopesFn =
async () => ['graphql', 'meta', 'data'];

protected readonly checkAuthMiddleware: CheckAuthMiddlewareFn;

protected readonly requestLoggerMiddleware: RequestLoggerMiddlewareFn;

protected readonly securityContextExtractor: SecurityContextExtractorFn;
Expand Down Expand Up @@ -188,9 +185,6 @@ class ApiGateway {
this.checkAuthFn = this.createCheckAuthFn(options);
this.checkAuthSystemFn = this.createCheckAuthSystemFn();
this.contextToApiScopesFn = this.createContextToApiScopesFn(options);
this.checkAuthMiddleware = options.checkAuthMiddleware
? this.wrapCheckAuthMiddleware(options.checkAuthMiddleware)
: this.checkAuth;
this.securityContextExtractor = this.createSecurityContextExtractor(options.jwt);
this.requestLoggerMiddleware = options.requestLoggerMiddleware || this.requestLogger;
this.contextRejectionMiddleware = options.contextRejectionMiddleware || (async (req, res, next) => next());
Expand All @@ -214,7 +208,7 @@ class ApiGateway {

public initApp(app: ExpressApplication) {
const userMiddlewares: RequestHandler[] = [
this.checkAuthMiddleware,
this.checkAuth,
this.requestContextMiddleware,
this.contextRejectionMiddleware,
this.logNetworkUsage,
Expand Down Expand Up @@ -2089,40 +2083,6 @@ class ApiGateway {
}
}

protected wrapCheckAuthMiddleware(fn: CheckAuthMiddlewareFn): CheckAuthMiddlewareFn {
this.logger('CheckAuthMiddleware Middleware Deprecation', {
warning: (
'Option checkAuthMiddleware is now deprecated in favor of checkAuth, please migrate: ' +
'https://github.com/cube-js/cube.js/blob/master/DEPRECATION.md#checkauthmiddleware'
)
});

let showWarningAboutNotObject = false;

return (req, res, next) => {
fn(req, res, (e) => {
// We renamed authInfo to securityContext, but users can continue to use both ways
if (req.securityContext && !req.authInfo) {
req.authInfo = req.securityContext;
} else if (req.authInfo) {
req.securityContext = req.authInfo;
}

if ((typeof req.securityContext !== 'object' || req.securityContext === null) && !showWarningAboutNotObject) {
this.logger('Security Context Should Be Object', {
warning: (
`Value of securityContext (previously authInfo) expected to be object, actual: ${getRealType(req.securityContext)}`
)
});

showWarningAboutNotObject = true;
}

next(e);
});
};
}

protected wrapCheckAuth(fn: CheckAuthFn): PreparedCheckAuthFn {
// We dont need to span all logs with deprecation message
let warningShowed = false;
Expand Down
11 changes: 0 additions & 11 deletions packages/cubejs-api-gateway/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,6 @@ export {
ContextToApiScopesFn,
};

/**
* Auth middleware.
* @deprecated
*/
export type CheckAuthMiddlewareFn =
(
req: Request,
res: ExpressResponse,
next: ExpressNextFunction,
) => void;

/**
* Context rejection middleware.
*/
Expand Down
5 changes: 0 additions & 5 deletions packages/cubejs-api-gateway/src/types/gateway.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
CheckAuthFn,
} from './auth';
import {
CheckAuthMiddlewareFn,
RequestLoggerMiddlewareFn,
ContextRejectionMiddlewareFn,
ContextAcceptorFn,
Expand Down Expand Up @@ -66,10 +65,6 @@ interface ApiGatewayOptions {
contextRejectionMiddleware?: ContextRejectionMiddlewareFn;
wsContextAcceptor?: ContextAcceptorFn;
checkAuth?: CheckAuthFn;
/**
* @deprecated Use checkAuth property instead.
*/
checkAuthMiddleware?: CheckAuthMiddlewareFn;
contextToApiScopes?: ContextToApiScopesFn;
event?: (name: string, props?: object) => void;
}
Expand Down
106 changes: 1 addition & 105 deletions packages/cubejs-api-gateway/test/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function createApiGateway(handler: RequestHandler, logger: () => any, options: P

public initApp(app: ExpressApplication) {
const userMiddlewares: RequestHandler[] = [
this.checkAuthMiddleware,
this.checkAuth,
this.requestContextMiddleware,
];

Expand Down Expand Up @@ -388,110 +388,6 @@ describe('test authorization', () => {
expect(handlerMock.mock.calls[0][0].context.authInfo).toEqual(EXPECTED_SECURITY_CONTEXT);
});

test('custom checkAuthMiddleware with deprecated authInfo', async () => {
const loggerMock = jest.fn(() => {
//
});

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

const handlerMock = jest.fn((req, res) => {
expectSecurityContext(req.context.securityContext);
expectSecurityContext(req.context.authInfo);

res.status(200).end();
});

const { app } = createApiGateway(handlerMock, loggerMock, {
checkAuthMiddleware: (req: Request, res, next) => {
try {
if (req.headers.authorization) {
req.authInfo = jwt.verify(req.headers.authorization, 'secret');
}

next();
} catch (e) {
next(e);
}
}
});

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

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

expect(loggerMock.mock.calls.length).toEqual(1);
expect(loggerMock.mock.calls[0]).toEqual([
'CheckAuthMiddleware Middleware Deprecation',
{
warning: 'Option checkAuthMiddleware is now deprecated in favor of checkAuth, please migrate: https://github.com/cube-js/cube.js/blob/master/DEPRECATION.md#checkauthmiddleware',
}
]);
expect(handlerMock.mock.calls.length).toEqual(1);

expectSecurityContext(handlerMock.mock.calls[0][0].context.securityContext);
// authInfo was deprecated, but should exists as computability
expectSecurityContext(handlerMock.mock.calls[0][0].context.authInfo);
});

test('custom checkAuthMiddleware with securityInfo (not object)', async () => {
const loggerMock = jest.fn();

const EXPECTED_SECURITY_CONTEXT = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1aWQiOjUsImlhdCI6MTYxMTg1NzcwNSwiZXhwIjoyNDc1ODU3NzA1fQ.tTieqdIcxDLG8fHv8YWwfvg_rPVe1XpZKUvrCdzVn3g';

const handlerMock = jest.fn((req, res) => {
expect(req.context.securityContext).toEqual(EXPECTED_SECURITY_CONTEXT);
expect(req.context.authInfo).toEqual(EXPECTED_SECURITY_CONTEXT);

res.status(200).end();
});

const { app } = createApiGateway(handlerMock, loggerMock, {
checkAuthMiddleware: (req: Request, res, next) => {
if (req.headers.authorization) {
// It must be object, but some users are using string for securityContext
req.authInfo = req.headers.authorization;
}

if (next) {
next();
}
}
});

await request(app)
.get('/test-auth-fake')
// console.log(generateAuthToken({ uid: 5, }));
.set('Authorization', 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1aWQiOjUsImlhdCI6MTYxMTg1NzcwNSwiZXhwIjoyNDc1ODU3NzA1fQ.tTieqdIcxDLG8fHv8YWwfvg_rPVe1XpZKUvrCdzVn3g')
.expect(200);

expect(loggerMock.mock.calls.length).toEqual(2);
expect(loggerMock.mock.calls[0]).toEqual([
'CheckAuthMiddleware Middleware Deprecation',
{
warning: 'Option checkAuthMiddleware is now deprecated in favor of checkAuth, please migrate: https://github.com/cube-js/cube.js/blob/master/DEPRECATION.md#checkauthmiddleware',
}
]);
expect(loggerMock.mock.calls[1]).toEqual([
'Security Context Should Be Object',
{
warning: 'Value of securityContext (previously authInfo) expected to be object, actual: string',
}
]);

expect(handlerMock.mock.calls.length).toEqual(1);
expect(handlerMock.mock.calls[0][0].context.securityContext).toEqual(EXPECTED_SECURITY_CONTEXT);
// authInfo was deprecated, but should exists as computability
expect(handlerMock.mock.calls[0][0].context.authInfo).toEqual(EXPECTED_SECURITY_CONTEXT);
});

test('coerceForSqlQuery multiple', async () => {
const loggerMock = jest.fn(() => {
//
Expand Down
1 change: 0 additions & 1 deletion packages/cubejs-server-core/src/core/optionsValidate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ const schemaOptions = Joi.object().keys({
contextToApiScopes: Joi.func(),
repositoryFactory: Joi.func(),
checkAuth: Joi.func(),
checkAuthMiddleware: Joi.func(),
jwt: jwtOptions,
queryTransformer: Joi.func(),
queryRewrite: Joi.func(),
Expand Down
1 change: 0 additions & 1 deletion packages/cubejs-server-core/src/core/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,6 @@ export class CubejsServerCore {
standalone: this.standalone,
dataSourceStorage: this.orchestratorStorage,
basePath: this.options.basePath,
checkAuthMiddleware: this.options.checkAuthMiddleware,
contextRejectionMiddleware: this.contextRejectionMiddleware.bind(this),
wsContextAcceptor: this.contextAcceptor.shouldAcceptWs.bind(this.contextAcceptor),
checkAuth: this.options.checkAuth,
Expand Down
2 changes: 0 additions & 2 deletions packages/cubejs-server-core/src/core/types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Required, SchemaFileRepository } from '@cubejs-backend/shared';
import {
CheckAuthFn,
CheckAuthMiddlewareFn,
ExtendContextFn,
JWTOptions,
UserBackgroundContext,
Expand Down Expand Up @@ -180,7 +179,6 @@ export interface CreateOptions {
contextToOrchestratorId?: ContextToOrchestratorIdFn;
contextToApiScopes?: ContextToApiScopesFn;
repositoryFactory?: (context: RequestContext) => SchemaFileRepository;
checkAuthMiddleware?: CheckAuthMiddlewareFn;
checkAuth?: CheckAuthFn;
checkSqlAuth?: CheckSQLAuthFn;
canSwitchSqlUser?: CanSwitchSQLUserFn;
Expand Down

0 comments on commit f5d7eed

Please sign in to comment.