Fix workspace hydratation (#12452)
We must separate the concept of hydratation which happens at the request level (take the token and pass auth/user context), from the concept of authorization which happens at the query/endpoint/mutation level. Previously, hydratation exemption happened at the operation name level which is not correct because the operation name is meaningless and optional. Still this gave an impression of security by enforcing a blacklist. So in this PR we introduce linting rule that aim to achieve a similar behavior, now every api method has to have a guard. That way if and endpoint is not protected by AuthUserGuard or AuthWorspaceGuard, then it has to be stated explicitly next to its code. --------- Co-authored-by: Charles Bochet <charles@twenty.com>
This commit is contained in:
@ -1,21 +0,0 @@
|
||||
export const EXCLUDED_MIDDLEWARE_OPERATIONS = [
|
||||
'GetClientConfig',
|
||||
'GetWorkspaceFromInviteHash',
|
||||
'Track',
|
||||
'TrackAnalytics',
|
||||
'AuditTrack',
|
||||
'CheckUserExists',
|
||||
'GetLoginTokenFromCredentials',
|
||||
'GetAuthTokensFromLoginToken',
|
||||
'GetLoginTokenFromEmailVerificationToken',
|
||||
'ResendEmailVerificationToken',
|
||||
'SignUp',
|
||||
'RenewToken',
|
||||
'EmailPasswordResetLink',
|
||||
'ValidatePasswordResetToken',
|
||||
'UpdatePasswordViaResetToken',
|
||||
'IntrospectionQuery',
|
||||
'ExchangeAuthorizationCode',
|
||||
'GetAuthorizationUrlForSSO',
|
||||
'GetPublicWorkspaceDataByDomain',
|
||||
] as const;
|
||||
@ -11,12 +11,8 @@ export class GraphQLHydrateRequestFromTokenMiddleware
|
||||
constructor(private readonly middlewareService: MiddlewareService) {}
|
||||
|
||||
async use(req: Request, res: Response, next: NextFunction) {
|
||||
if (this.middlewareService.checkUnauthenticatedAccess(req)) {
|
||||
return next();
|
||||
}
|
||||
|
||||
try {
|
||||
await this.middlewareService.authenticateGraphqlRequest(req);
|
||||
await this.middlewareService.hydrateGraphqlRequest(req);
|
||||
} catch (error) {
|
||||
this.middlewareService.writeGraphqlResponseOnExceptionCaught(res, error);
|
||||
|
||||
|
||||
@ -4,6 +4,7 @@ import { Request, Response } from 'express';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { AuthException } from 'src/engine/core-modules/auth/auth.exception';
|
||||
import { AuthGraphqlApiExceptionFilter } from 'src/engine/core-modules/auth/filters/auth-graphql-api-exception.filter';
|
||||
import { AccessTokenService } from 'src/engine/core-modules/auth/token/services/access-token.service';
|
||||
import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type';
|
||||
import { getAuthExceptionRestStatus } from 'src/engine/core-modules/auth/utils/get-auth-exception-rest-status.util';
|
||||
@ -13,8 +14,6 @@ import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrap
|
||||
import { DataSourceService } from 'src/engine/metadata-modules/data-source/data-source.service';
|
||||
import { WorkspaceMetadataCacheService } from 'src/engine/metadata-modules/workspace-metadata-cache/services/workspace-metadata-cache.service';
|
||||
import { INTERNAL_SERVER_ERROR } from 'src/engine/middlewares/constants/default-error-message.constant';
|
||||
import { EXCLUDED_MIDDLEWARE_OPERATIONS } from 'src/engine/middlewares/constants/excluded-middleware-operations.constant';
|
||||
import { GraphqlTokenValidationProxy } from 'src/engine/middlewares/utils/graphql-token-validation-utils';
|
||||
import {
|
||||
handleException,
|
||||
handleExceptionAndConvertToGraphQLError,
|
||||
@ -33,25 +32,12 @@ export class MiddlewareService {
|
||||
private readonly jwtWrapperService: JwtWrapperService,
|
||||
) {}
|
||||
|
||||
private excludedOperations = EXCLUDED_MIDDLEWARE_OPERATIONS;
|
||||
|
||||
public isTokenPresent(request: Request): boolean {
|
||||
const token = this.jwtWrapperService.extractJwtFromRequest()(request);
|
||||
|
||||
return !!token;
|
||||
}
|
||||
|
||||
public checkUnauthenticatedAccess(request: Request): boolean {
|
||||
const { body } = request;
|
||||
|
||||
const isUserUnauthenticated = !this.isTokenPresent(request);
|
||||
const isExcludedOperation =
|
||||
!body?.operationName ||
|
||||
this.excludedOperations.includes(body.operationName);
|
||||
|
||||
return isUserUnauthenticated && isExcludedOperation;
|
||||
}
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
public writeRestResponseOnExceptionCaught(res: Response, error: any) {
|
||||
const statusCode = this.getStatus(error);
|
||||
@ -77,12 +63,24 @@ export class MiddlewareService {
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
public writeGraphqlResponseOnExceptionCaught(res: Response, error: any) {
|
||||
const errors = [
|
||||
handleExceptionAndConvertToGraphQLError(
|
||||
error as Error,
|
||||
this.exceptionHandlerService,
|
||||
),
|
||||
];
|
||||
let errors;
|
||||
|
||||
if (error instanceof AuthException) {
|
||||
try {
|
||||
const authFilter = new AuthGraphqlApiExceptionFilter();
|
||||
|
||||
authFilter.catch(error);
|
||||
} catch (transformedError) {
|
||||
errors = [transformedError];
|
||||
}
|
||||
} else {
|
||||
errors = [
|
||||
handleExceptionAndConvertToGraphQLError(
|
||||
error as Error,
|
||||
this.exceptionHandlerService,
|
||||
),
|
||||
];
|
||||
}
|
||||
|
||||
const statusCode = 200;
|
||||
|
||||
@ -99,7 +97,7 @@ export class MiddlewareService {
|
||||
res.end();
|
||||
}
|
||||
|
||||
public async authenticateRestRequest(request: Request) {
|
||||
public async hydrateRestRequest(request: Request) {
|
||||
const data = await this.accessTokenService.validateTokenByRequest(request);
|
||||
const metadataVersion =
|
||||
await this.workspaceStorageCacheService.getMetadataVersion(
|
||||
@ -125,12 +123,12 @@ export class MiddlewareService {
|
||||
this.bindDataToRequestObject(data, request, metadataVersion);
|
||||
}
|
||||
|
||||
public async authenticateGraphqlRequest(request: Request) {
|
||||
const graphqlTokenValidationProxy = new GraphqlTokenValidationProxy(
|
||||
this.accessTokenService,
|
||||
);
|
||||
public async hydrateGraphqlRequest(request: Request) {
|
||||
if (!this.isTokenPresent(request)) {
|
||||
return;
|
||||
}
|
||||
|
||||
const data = await graphqlTokenValidationProxy.validateToken(request);
|
||||
const data = await this.accessTokenService.validateTokenByRequest(request);
|
||||
const metadataVersion =
|
||||
await this.workspaceStorageCacheService.getMetadataVersion(
|
||||
data.workspace.id,
|
||||
|
||||
@ -10,7 +10,7 @@ export class RestCoreMiddleware implements NestMiddleware {
|
||||
|
||||
async use(req: Request, res: Response, next: NextFunction) {
|
||||
try {
|
||||
await this.middlewareService.authenticateRestRequest(req);
|
||||
await this.middlewareService.hydrateRestRequest(req);
|
||||
} catch (error) {
|
||||
this.middlewareService.writeRestResponseOnExceptionCaught(res, error);
|
||||
|
||||
|
||||
@ -1,22 +0,0 @@
|
||||
import { Request } from 'express';
|
||||
|
||||
import { AuthGraphqlApiExceptionFilter } from 'src/engine/core-modules/auth/filters/auth-graphql-api-exception.filter';
|
||||
import { AccessTokenService } from 'src/engine/core-modules/auth/token/services/access-token.service';
|
||||
|
||||
export class GraphqlTokenValidationProxy {
|
||||
private accessTokenService: AccessTokenService;
|
||||
|
||||
constructor(accessTokenService: AccessTokenService) {
|
||||
this.accessTokenService = accessTokenService;
|
||||
}
|
||||
|
||||
async validateToken(req: Request) {
|
||||
try {
|
||||
return await this.accessTokenService.validateTokenByRequest(req);
|
||||
} catch (error) {
|
||||
const authGraphqlApiExceptionFilter = new AuthGraphqlApiExceptionFilter();
|
||||
|
||||
throw authGraphqlApiExceptionFilter.catch(error);
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user