From ecf21774dd14a7cf630123725c252bacdfa5c088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Mon, 9 Jun 2025 14:14:32 +0200 Subject: [PATCH] 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 --- packages/twenty-server/.eslintrc.cjs | 2 + .../core-modules/audit/audit.resolver.ts | 10 +- .../engine/core-modules/auth/auth.resolver.ts | 21 ++- .../google-apis-auth.controller.ts | 5 +- .../controllers/google-auth.controller.ts | 5 +- .../microsoft-apis-auth.controller.ts | 5 +- .../controllers/microsoft-auth.controller.ts | 13 +- .../auth/controllers/sso-auth.controller.ts | 11 +- .../billing/billing.controller.ts | 4 + .../client-config/client-config.controller.ts | 4 +- .../client-config/client-config.resolver.ts | 3 + .../controllers/cloudflare.controller.ts | 4 +- .../email-verification.resolver.ts | 4 +- .../file/controllers/file.controller.ts | 2 + .../health/controllers/health.controller.ts | 12 +- .../open-api/open-api.controller.ts | 5 +- .../core-modules/search/search.resolver.ts | 6 +- .../workflow-trigger.controller.ts | 13 +- .../workspace/workspace.resolver.ts | 4 +- .../decorators/auth/auth-user.decorator.ts | 3 +- .../auth/auth-workspace.decorator.ts | 21 ++- .../{ => __tests__}/impersonate-guard.spec.ts | 0 .../__tests__/public-endpoint.guard.spec.ts | 37 ++++ .../engine/guards/public-endpoint.guard.ts | 16 ++ .../metadata-modules/role/role.resolver.ts | 4 + ...excluded-middleware-operations.constant.ts | 21 --- ...l-hydrate-request-from-token.middleware.ts | 6 +- .../engine/middlewares/middleware.service.ts | 52 +++--- .../middlewares/rest-core.middleware.ts | 2 +- .../utils/graphql-token-validation-utils.ts | 22 --- tools/eslint-rules/index.ts | 66 +++++--- ...raphql-resolvers-should-be-guarded.spec.ts | 159 ++++++++++++++++++ .../graphql-resolvers-should-be-guarded.ts | 70 ++++++++ ...rest-api-methods-should-be-guarded.spec.ts | 138 +++++++++++++++ .../rest-api-methods-should-be-guarded.ts | 70 ++++++++ tools/eslint-rules/utils/createRule.ts | 3 + tools/eslint-rules/utils/typedTokenHelpers.ts | 56 ++++++ 37 files changed, 741 insertions(+), 138 deletions(-) rename packages/twenty-server/src/engine/guards/{ => __tests__}/impersonate-guard.spec.ts (100%) create mode 100644 packages/twenty-server/src/engine/guards/__tests__/public-endpoint.guard.spec.ts create mode 100644 packages/twenty-server/src/engine/guards/public-endpoint.guard.ts delete mode 100644 packages/twenty-server/src/engine/middlewares/constants/excluded-middleware-operations.constant.ts delete mode 100644 packages/twenty-server/src/engine/middlewares/utils/graphql-token-validation-utils.ts create mode 100644 tools/eslint-rules/rules/graphql-resolvers-should-be-guarded.spec.ts create mode 100644 tools/eslint-rules/rules/graphql-resolvers-should-be-guarded.ts create mode 100644 tools/eslint-rules/rules/rest-api-methods-should-be-guarded.spec.ts create mode 100644 tools/eslint-rules/rules/rest-api-methods-should-be-guarded.ts create mode 100644 tools/eslint-rules/utils/createRule.ts create mode 100644 tools/eslint-rules/utils/typedTokenHelpers.ts diff --git a/packages/twenty-server/.eslintrc.cjs b/packages/twenty-server/.eslintrc.cjs index 6224cb477..dd968f156 100644 --- a/packages/twenty-server/.eslintrc.cjs +++ b/packages/twenty-server/.eslintrc.cjs @@ -97,6 +97,8 @@ module.exports = { 'prefer-arrow/prefer-arrow-functions': 'off', '@nx/workspace-max-consts-per-file': 'off', '@nx/workspace-inject-workspace-repository': 'warn', + '@nx/workspace-rest-api-methods-should-be-guarded': 'error', + '@nx/workspace-graphql-resolvers-should-be-guarded': 'error', }, }, { diff --git a/packages/twenty-server/src/engine/core-modules/audit/audit.resolver.ts b/packages/twenty-server/src/engine/core-modules/audit/audit.resolver.ts index cf40e45f2..25d83da0c 100644 --- a/packages/twenty-server/src/engine/core-modules/audit/audit.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/audit/audit.resolver.ts @@ -1,4 +1,4 @@ -import { UseFilters } from '@nestjs/common'; +import { UseFilters, UseGuards } from '@nestjs/common'; import { Args, Mutation, Resolver } from '@nestjs/graphql'; import { AuditExceptionFilter } from 'src/engine/core-modules/audit/audit-exception-filter'; @@ -11,6 +11,8 @@ import { User } from 'src/engine/core-modules/user/user.entity'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { AuthUser } from 'src/engine/decorators/auth/auth-user.decorator'; import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; +import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; import { CreateAnalyticsInputV2, @@ -29,13 +31,14 @@ export class AuditResolver { async createPageview( @Args() createAnalyticsInput: CreateAnalyticsInputV2, - @AuthWorkspace() workspace: Workspace | undefined, + @AuthWorkspace({ allowUndefined: true }) workspace: Workspace | undefined, @AuthUser({ allowUndefined: true }) user: User | undefined, ) { return this.trackAnalytics(createAnalyticsInput, workspace, user); } @Mutation(() => Analytics) + @UseGuards(WorkspaceAuthGuard) async createObjectEvent( @Args() createObjectEventInput: CreateObjectEventInput, @@ -63,10 +66,11 @@ export class AuditResolver { } @Mutation(() => Analytics) + @UseGuards(PublicEndpointGuard) async trackAnalytics( @Args() createAnalyticsInput: CreateAnalyticsInputV2, - @AuthWorkspace() workspace: Workspace | undefined, + @AuthWorkspace({ allowUndefined: true }) workspace: Workspace | undefined, @AuthUser({ allowUndefined: true }) user: User | undefined, ) { const analyticsContext = this.auditService.createContext({ diff --git a/packages/twenty-server/src/engine/core-modules/auth/auth.resolver.ts b/packages/twenty-server/src/engine/core-modules/auth/auth.resolver.ts index 621d4f105..6c6f91a10 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/auth.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/auth.resolver.ts @@ -49,6 +49,7 @@ import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { workspaceValidator } from 'src/engine/core-modules/workspace/workspace.validate'; import { AuthUser } from 'src/engine/decorators/auth/auth-user.decorator'; import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; import { SettingsPermissionsGuard } from 'src/engine/guards/settings-permissions.guard'; import { UserAuthGuard } from 'src/engine/guards/user-auth.guard'; import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; @@ -94,7 +95,7 @@ export class AuthResolver { private sSOService: SSOService, ) {} - @UseGuards(CaptchaGuard) + @UseGuards(CaptchaGuard, PublicEndpointGuard) @Query(() => UserExistsOutput) async checkUserExists( @Args() checkUserExistsInput: CheckUserExistsInput, @@ -105,6 +106,7 @@ export class AuthResolver { } @Mutation(() => GetAuthorizationUrlForSSOOutput) + @UseGuards(PublicEndpointGuard) async getAuthorizationUrlForSSO( @Args('input') params: GetAuthorizationUrlForSSOInput, ) { @@ -115,6 +117,7 @@ export class AuthResolver { } @Query(() => WorkspaceInviteHashValid) + @UseGuards(PublicEndpointGuard) async checkWorkspaceInviteHashIsValid( @Args() workspaceInviteHashValidInput: WorkspaceInviteHashValidInput, ): Promise { @@ -124,6 +127,7 @@ export class AuthResolver { } @Query(() => Workspace) + @UseGuards(PublicEndpointGuard) async findWorkspaceFromInviteHash( @Args() workspaceInviteHashValidInput: WorkspaceInviteHashValidInput, ): Promise { @@ -132,7 +136,7 @@ export class AuthResolver { ); } - @UseGuards(CaptchaGuard) + @UseGuards(CaptchaGuard, PublicEndpointGuard) @Mutation(() => LoginToken) async getLoginTokenFromCredentials( @Args() @@ -166,6 +170,7 @@ export class AuthResolver { } @Mutation(() => GetLoginTokenFromEmailVerificationTokenOutput) + @UseGuards(PublicEndpointGuard) async getLoginTokenFromEmailVerificationToken( @Args() getLoginTokenFromEmailVerificationTokenInput: GetLoginTokenFromEmailVerificationTokenInput, @@ -197,7 +202,7 @@ export class AuthResolver { return { loginToken, workspaceUrls }; } - @UseGuards(CaptchaGuard) + @UseGuards(CaptchaGuard, PublicEndpointGuard) @Mutation(() => SignUpOutput) async signUp(@Args() signUpInput: SignUpInput): Promise { const currentWorkspace = await this.authService.findWorkspaceForSignInUp({ @@ -269,6 +274,7 @@ export class AuthResolver { } @Mutation(() => SignUpOutput) + @UseGuards(UserAuthGuard) async signUpInNewWorkspace( @AuthUser() currentUser: User, ): Promise { @@ -300,7 +306,7 @@ export class AuthResolver { // } @Mutation(() => TransientToken) - @UseGuards(WorkspaceAuthGuard, UserAuthGuard) + @UseGuards(UserAuthGuard) async generateTransientToken( @AuthUser() user: User, @AuthWorkspace() workspace: Workspace, @@ -324,6 +330,7 @@ export class AuthResolver { } @Mutation(() => AuthTokens) + @UseGuards(PublicEndpointGuard) async getAuthTokensFromLoginToken( @Args() getAuthTokensFromLoginTokenInput: GetAuthTokensFromLoginTokenInput, @Args('origin') origin: string, @@ -351,7 +358,7 @@ export class AuthResolver { } @Mutation(() => AuthorizeApp) - @UseGuards(WorkspaceAuthGuard, UserAuthGuard) + @UseGuards(UserAuthGuard) async authorizeApp( @Args() authorizeAppInput: AuthorizeAppInput, @AuthUser() user: User, @@ -365,6 +372,7 @@ export class AuthResolver { } @Mutation(() => AuthTokens) + @UseGuards(PublicEndpointGuard) async renewToken(@Args() args: AppTokenInput): Promise { const tokens = await this.renewTokenService.generateTokensFromRefreshToken( args.appToken, @@ -390,6 +398,7 @@ export class AuthResolver { } @Mutation(() => EmailPasswordResetLink) + @UseGuards(PublicEndpointGuard) async emailPasswordResetLink( @Args() emailPasswordResetInput: EmailPasswordResetLinkInput, @Context() context: I18nContext, @@ -408,6 +417,7 @@ export class AuthResolver { } @Mutation(() => InvalidatePassword) + @UseGuards(PublicEndpointGuard) async updatePasswordViaResetToken( @Args() { passwordResetToken, newPassword }: UpdatePasswordViaResetTokenInput, @@ -428,6 +438,7 @@ export class AuthResolver { } @Query(() => ValidatePasswordResetToken) + @UseGuards(PublicEndpointGuard) async validatePasswordResetToken( @Args() args: ValidatePasswordResetTokenInput, ): Promise { diff --git a/packages/twenty-server/src/engine/core-modules/auth/controllers/google-apis-auth.controller.ts b/packages/twenty-server/src/engine/core-modules/auth/controllers/google-apis-auth.controller.ts index 6d4a4ad1a..9724dffaa 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/controllers/google-apis-auth.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/controllers/google-apis-auth.controller.ts @@ -26,6 +26,7 @@ import { GuardRedirectService } from 'src/engine/core-modules/guard-redirect/ser import { OnboardingService } from 'src/engine/core-modules/onboarding/onboarding.service'; import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; @Controller('auth/google-apis') @UseFilters(AuthRestApiExceptionFilter) @@ -42,14 +43,14 @@ export class GoogleAPIsAuthController { ) {} @Get() - @UseGuards(GoogleAPIsOauthRequestCodeGuard) + @UseGuards(GoogleAPIsOauthRequestCodeGuard, PublicEndpointGuard) async googleAuth() { // As this method is protected by Google Auth guard, it will trigger Google SSO flow return; } @Get('get-access-token') - @UseGuards(GoogleAPIsOauthExchangeCodeForTokenGuard) + @UseGuards(GoogleAPIsOauthExchangeCodeForTokenGuard, PublicEndpointGuard) async googleAuthGetAccessToken( @Req() req: GoogleAPIsRequest, @Res() res: Response, diff --git a/packages/twenty-server/src/engine/core-modules/auth/controllers/google-auth.controller.ts b/packages/twenty-server/src/engine/core-modules/auth/controllers/google-auth.controller.ts index 9104479f5..461004260 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/controllers/google-auth.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/controllers/google-auth.controller.ts @@ -21,6 +21,7 @@ import { LoginTokenService } from 'src/engine/core-modules/auth/token/services/l import { DomainManagerService } from 'src/engine/core-modules/domain-manager/services/domain-manager.service'; import { GuardRedirectService } from 'src/engine/core-modules/guard-redirect/services/guard-redirect.service'; import { User } from 'src/engine/core-modules/user/user.entity'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; @Controller('auth/google') @UseFilters(AuthRestApiExceptionFilter) @@ -35,14 +36,14 @@ export class GoogleAuthController { ) {} @Get() - @UseGuards(GoogleProviderEnabledGuard, GoogleOauthGuard) + @UseGuards(GoogleProviderEnabledGuard, GoogleOauthGuard, PublicEndpointGuard) async googleAuth() { // As this method is protected by Google Auth guard, it will trigger Google SSO flow return; } @Get('redirect') - @UseGuards(GoogleProviderEnabledGuard, GoogleOauthGuard) + @UseGuards(GoogleProviderEnabledGuard, GoogleOauthGuard, PublicEndpointGuard) @UseFilters(AuthOAuthExceptionFilter) async googleAuthRedirect(@Req() req: GoogleRequest, @Res() res: Response) { const { diff --git a/packages/twenty-server/src/engine/core-modules/auth/controllers/microsoft-apis-auth.controller.ts b/packages/twenty-server/src/engine/core-modules/auth/controllers/microsoft-apis-auth.controller.ts index 9614d7134..564ee8634 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/controllers/microsoft-apis-auth.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/controllers/microsoft-apis-auth.controller.ts @@ -26,6 +26,7 @@ import { GuardRedirectService } from 'src/engine/core-modules/guard-redirect/ser import { OnboardingService } from 'src/engine/core-modules/onboarding/onboarding.service'; import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; @Controller('auth/microsoft-apis') @UseFilters(AuthRestApiExceptionFilter) @@ -42,14 +43,14 @@ export class MicrosoftAPIsAuthController { ) {} @Get() - @UseGuards(MicrosoftAPIsOauthRequestCodeGuard) + @UseGuards(MicrosoftAPIsOauthRequestCodeGuard, PublicEndpointGuard) async MicrosoftAuth() { // As this method is protected by Microsoft Auth guard, it will trigger Microsoft SSO flow return; } @Get('get-access-token') - @UseGuards(MicrosoftAPIsOauthExchangeCodeForTokenGuard) + @UseGuards(MicrosoftAPIsOauthExchangeCodeForTokenGuard, PublicEndpointGuard) async MicrosoftAuthGetAccessToken( @Req() req: MicrosoftAPIsRequest, @Res() res: Response, diff --git a/packages/twenty-server/src/engine/core-modules/auth/controllers/microsoft-auth.controller.ts b/packages/twenty-server/src/engine/core-modules/auth/controllers/microsoft-auth.controller.ts index 07b8610cf..964861e24 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/controllers/microsoft-auth.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/controllers/microsoft-auth.controller.ts @@ -20,6 +20,7 @@ import { LoginTokenService } from 'src/engine/core-modules/auth/token/services/l import { DomainManagerService } from 'src/engine/core-modules/domain-manager/services/domain-manager.service'; import { GuardRedirectService } from 'src/engine/core-modules/guard-redirect/services/guard-redirect.service'; import { User } from 'src/engine/core-modules/user/user.entity'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; @Controller('auth/microsoft') @UseFilters(AuthRestApiExceptionFilter) @@ -34,14 +35,22 @@ export class MicrosoftAuthController { ) {} @Get() - @UseGuards(MicrosoftProviderEnabledGuard, MicrosoftOAuthGuard) + @UseGuards( + MicrosoftProviderEnabledGuard, + MicrosoftOAuthGuard, + PublicEndpointGuard, + ) async microsoftAuth() { // As this method is protected by Microsoft Auth guard, it will trigger Microsoft SSO flow return; } @Get('redirect') - @UseGuards(MicrosoftProviderEnabledGuard, MicrosoftOAuthGuard) + @UseGuards( + MicrosoftProviderEnabledGuard, + MicrosoftOAuthGuard, + PublicEndpointGuard, + ) async microsoftAuthRedirect( @Req() req: MicrosoftRequest, @Res() res: Response, diff --git a/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts b/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts index a7c43ebeb..d220c1ff7 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts @@ -38,6 +38,7 @@ import { import { User } from 'src/engine/core-modules/user/user.entity'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { workspaceValidator } from 'src/engine/core-modules/workspace/workspace.validate'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; @Controller('auth') export class SSOAuthController { @@ -55,7 +56,7 @@ export class SSOAuthController { ) {} @Get('saml/metadata/:identityProviderId') - @UseGuards(EnterpriseFeaturesEnabledGuard) + @UseGuards(EnterpriseFeaturesEnabledGuard, PublicEndpointGuard) @UseFilters(AuthRestApiExceptionFilter) // eslint-disable-next-line @typescript-eslint/no-explicit-any async generateMetadata(@Req() req: any): Promise { @@ -73,7 +74,7 @@ export class SSOAuthController { } @Get('oidc/login/:identityProviderId') - @UseGuards(EnterpriseFeaturesEnabledGuard, OIDCAuthGuard) + @UseGuards(EnterpriseFeaturesEnabledGuard, OIDCAuthGuard, PublicEndpointGuard) @UseFilters(AuthRestApiExceptionFilter) async oidcAuth() { // As this method is protected by OIDC Auth guard, it will trigger OIDC SSO flow @@ -81,7 +82,7 @@ export class SSOAuthController { } @Get('saml/login/:identityProviderId') - @UseGuards(EnterpriseFeaturesEnabledGuard, SAMLAuthGuard) + @UseGuards(EnterpriseFeaturesEnabledGuard, SAMLAuthGuard, PublicEndpointGuard) @UseFilters(AuthRestApiExceptionFilter) async samlAuth() { // As this method is protected by SAML Auth guard, it will trigger SAML SSO flow @@ -89,14 +90,14 @@ export class SSOAuthController { } @Get('oidc/callback') - @UseGuards(EnterpriseFeaturesEnabledGuard, OIDCAuthGuard) + @UseGuards(EnterpriseFeaturesEnabledGuard, OIDCAuthGuard, PublicEndpointGuard) @UseFilters(AuthOAuthExceptionFilter) async oidcAuthCallback(@Req() req: OIDCRequest, @Res() res: Response) { return await this.authCallback(req, res); } @Post('saml/callback/:identityProviderId') - @UseGuards(EnterpriseFeaturesEnabledGuard, SAMLAuthGuard) + @UseGuards(EnterpriseFeaturesEnabledGuard, SAMLAuthGuard, PublicEndpointGuard) @UseFilters(AuthOAuthExceptionFilter) async samlAuthCallback(@Req() req: SAMLRequest, @Res() res: Response) { try { diff --git a/packages/twenty-server/src/engine/core-modules/billing/billing.controller.ts b/packages/twenty-server/src/engine/core-modules/billing/billing.controller.ts index 2ad6a407b..383569a7f 100644 --- a/packages/twenty-server/src/engine/core-modules/billing/billing.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/billing/billing.controller.ts @@ -9,6 +9,7 @@ import { Req, Res, UseFilters, + UseGuards, } from '@nestjs/common'; import { Response } from 'express'; @@ -29,6 +30,8 @@ import { BillingWebhookInvoiceService } from 'src/engine/core-modules/billing/we import { BillingWebhookPriceService } from 'src/engine/core-modules/billing/webhooks/services/billing-webhook-price.service'; import { BillingWebhookProductService } from 'src/engine/core-modules/billing/webhooks/services/billing-webhook-product.service'; import { BillingWebhookSubscriptionService } from 'src/engine/core-modules/billing/webhooks/services/billing-webhook-subscription.service'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; + @Controller() @UseFilters(BillingRestApiExceptionFilter) export class BillingController { @@ -47,6 +50,7 @@ export class BillingController { ) {} @Post(['webhooks/stripe']) + @UseGuards(PublicEndpointGuard) async handleWebhooks( @Headers('stripe-signature') signature: string, @Req() req: RawBodyRequest, diff --git a/packages/twenty-server/src/engine/core-modules/client-config/client-config.controller.ts b/packages/twenty-server/src/engine/core-modules/client-config/client-config.controller.ts index fa619d512..3d29b2e94 100644 --- a/packages/twenty-server/src/engine/core-modules/client-config/client-config.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/client-config/client-config.controller.ts @@ -1,13 +1,15 @@ -import { Controller, Get } from '@nestjs/common'; +import { Controller, Get, UseGuards } from '@nestjs/common'; import { ClientConfig } from 'src/engine/core-modules/client-config/client-config.entity'; import { ClientConfigService } from 'src/engine/core-modules/client-config/services/client-config.service'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; @Controller('/client-config') export class ClientConfigController { constructor(private readonly clientConfigService: ClientConfigService) {} @Get() + @UseGuards(PublicEndpointGuard) async getClientConfig(): Promise { return this.clientConfigService.getClientConfig(); } diff --git a/packages/twenty-server/src/engine/core-modules/client-config/client-config.resolver.ts b/packages/twenty-server/src/engine/core-modules/client-config/client-config.resolver.ts index 37ab9f7de..c2fd86b08 100644 --- a/packages/twenty-server/src/engine/core-modules/client-config/client-config.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/client-config/client-config.resolver.ts @@ -1,6 +1,8 @@ +import { UseGuards } from '@nestjs/common'; import { Query, Resolver } from '@nestjs/graphql'; import { ClientConfigService } from 'src/engine/core-modules/client-config/services/client-config.service'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; import { ClientConfig } from './client-config.entity'; @@ -9,6 +11,7 @@ export class ClientConfigResolver { constructor(private clientConfigService: ClientConfigService) {} @Query(() => ClientConfig) + @UseGuards(PublicEndpointGuard) async clientConfig(): Promise { return this.clientConfigService.getClientConfig(); } diff --git a/packages/twenty-server/src/engine/core-modules/domain-manager/controllers/cloudflare.controller.ts b/packages/twenty-server/src/engine/core-modules/domain-manager/controllers/cloudflare.controller.ts index d0b97ece8..cd84c1394 100644 --- a/packages/twenty-server/src/engine/core-modules/domain-manager/controllers/cloudflare.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/domain-manager/controllers/cloudflare.controller.ts @@ -25,8 +25,8 @@ import { CustomDomainService } from 'src/engine/core-modules/domain-manager/serv import { DomainManagerService } from 'src/engine/core-modules/domain-manager/services/domain-manager.service'; import { ExceptionHandlerService } from 'src/engine/core-modules/exception-handler/exception-handler.service'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; import { handleException } from 'src/engine/utils/global-exception-handler.util'; - @Controller() @UseFilters(AuthRestApiExceptionFilter) export class CloudflareController { @@ -40,7 +40,7 @@ export class CloudflareController { ) {} @Post(['cloudflare/custom-hostname-webhooks', 'webhooks/cloudflare']) - @UseGuards(CloudflareSecretMatchGuard) + @UseGuards(CloudflareSecretMatchGuard, PublicEndpointGuard) async customHostnameWebhooks(@Req() req: Request, @Res() res: Response) { if (!req.body?.data?.data?.hostname) { handleException({ diff --git a/packages/twenty-server/src/engine/core-modules/email-verification/email-verification.resolver.ts b/packages/twenty-server/src/engine/core-modules/email-verification/email-verification.resolver.ts index 5f08b1168..e9b0ae0bc 100644 --- a/packages/twenty-server/src/engine/core-modules/email-verification/email-verification.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/email-verification/email-verification.resolver.ts @@ -1,4 +1,4 @@ -import { UseFilters } from '@nestjs/common'; +import { UseFilters, UseGuards } from '@nestjs/common'; import { Args, Context, Mutation, Resolver } from '@nestjs/graphql'; import { SOURCE_LOCALE } from 'twenty-shared/translations'; @@ -9,6 +9,7 @@ import { ResendEmailVerificationTokenOutput } from 'src/engine/core-modules/emai import { EmailVerificationExceptionFilter } from 'src/engine/core-modules/email-verification/email-verification-exception-filter.util'; import { EmailVerificationService } from 'src/engine/core-modules/email-verification/services/email-verification.service'; import { I18nContext } from 'src/engine/core-modules/i18n/types/i18n-context.type'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; @Resolver() @UseFilters(EmailVerificationExceptionFilter) @@ -19,6 +20,7 @@ export class EmailVerificationResolver { ) {} @Mutation(() => ResendEmailVerificationTokenOutput) + @UseGuards(PublicEndpointGuard) async resendEmailVerificationToken( @Args() resendEmailVerificationTokenInput: ResendEmailVerificationTokenInput, diff --git a/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts b/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts index e77a759dc..559b0663a 100644 --- a/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts @@ -23,6 +23,7 @@ import { FileApiExceptionFilter } from 'src/engine/core-modules/file/filters/fil import { FilePathGuard } from 'src/engine/core-modules/file/guards/file-path-guard'; import { FileService } from 'src/engine/core-modules/file/services/file.service'; import { extractFileInfoFromRequest } from 'src/engine/core-modules/file/utils/extract-file-info-from-request.utils'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; @Controller('files') @UseFilters(FileApiExceptionFilter) @@ -31,6 +32,7 @@ export class FileController { constructor(private readonly fileService: FileService) {} @Get('*/:filename') + @UseGuards(PublicEndpointGuard) async getFile( @Param() params: string[], @Res() res: Response, diff --git a/packages/twenty-server/src/engine/core-modules/health/controllers/health.controller.ts b/packages/twenty-server/src/engine/core-modules/health/controllers/health.controller.ts index dfd974710..572651ee0 100644 --- a/packages/twenty-server/src/engine/core-modules/health/controllers/health.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/health/controllers/health.controller.ts @@ -1,4 +1,10 @@ -import { BadRequestException, Controller, Get, Param } from '@nestjs/common'; +import { + BadRequestException, + Controller, + Get, + Param, + UseGuards, +} from '@nestjs/common'; import { HealthCheck, HealthCheckService } from '@nestjs/terminus'; import { HealthIndicatorId } from 'src/engine/core-modules/health/enums/health-indicator-id.enum'; @@ -7,6 +13,8 @@ import { ConnectedAccountHealth } from 'src/engine/core-modules/health/indicator import { DatabaseHealthIndicator } from 'src/engine/core-modules/health/indicators/database.health'; import { RedisHealthIndicator } from 'src/engine/core-modules/health/indicators/redis.health'; import { WorkerHealthIndicator } from 'src/engine/core-modules/health/indicators/worker.health'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; + @Controller('healthz') export class HealthController { constructor( @@ -19,12 +27,14 @@ export class HealthController { ) {} @Get() + @UseGuards(PublicEndpointGuard) @HealthCheck() check() { return this.health.check([]); } @Get(':indicatorId') + @UseGuards(PublicEndpointGuard) @HealthCheck() checkService(@Param('indicatorId') indicatorId: HealthIndicatorId) { const checks = { diff --git a/packages/twenty-server/src/engine/core-modules/open-api/open-api.controller.ts b/packages/twenty-server/src/engine/core-modules/open-api/open-api.controller.ts index e66c09dc2..bfeefd03c 100644 --- a/packages/twenty-server/src/engine/core-modules/open-api/open-api.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/open-api/open-api.controller.ts @@ -1,14 +1,16 @@ -import { Controller, Get, Req, Res } from '@nestjs/common'; +import { Controller, Get, Req, Res, UseGuards } from '@nestjs/common'; import { Request, Response } from 'express'; import { OpenApiService } from 'src/engine/core-modules/open-api/open-api.service'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; @Controller() export class OpenApiController { constructor(private readonly openApiService: OpenApiService) {} @Get(['open-api/core', 'rest/open-api/core']) + @UseGuards(PublicEndpointGuard) async generateOpenApiSchemaCore( @Req() request: Request, @Res() res: Response, @@ -19,6 +21,7 @@ export class OpenApiController { } @Get(['open-api/metadata', 'rest/open-api/metadata']) + @UseGuards(PublicEndpointGuard) async generateOpenApiSchemaMetaData( @Req() request: Request, @Res() res: Response, diff --git a/packages/twenty-server/src/engine/core-modules/search/search.resolver.ts b/packages/twenty-server/src/engine/core-modules/search/search.resolver.ts index d13d2ef66..0e49fbade 100644 --- a/packages/twenty-server/src/engine/core-modules/search/search.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/search/search.resolver.ts @@ -1,12 +1,13 @@ -import { UseFilters } from '@nestjs/common'; +import { UseFilters, UseGuards } from '@nestjs/common'; import { Args, Query, Resolver } from '@nestjs/graphql'; import { SearchArgs } from 'src/engine/core-modules/search/dtos/search-args'; +import { SearchResultConnectionDTO } from 'src/engine/core-modules/search/dtos/search-result-connection.dto'; import { SearchApiExceptionFilter } from 'src/engine/core-modules/search/filters/search-api-exception.filter'; import { SearchService } from 'src/engine/core-modules/search/services/search.service'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator'; -import { SearchResultConnectionDTO } from 'src/engine/core-modules/search/dtos/search-result-connection.dto'; +import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; @Resolver() @UseFilters(SearchApiExceptionFilter) @@ -14,6 +15,7 @@ export class SearchResolver { constructor(private readonly searchService: SearchService) {} @Query(() => SearchResultConnectionDTO) + @UseGuards(WorkspaceAuthGuard) async search( @AuthWorkspace() workspace: Workspace, @Args() diff --git a/packages/twenty-server/src/engine/core-modules/workflow/controllers/workflow-trigger.controller.ts b/packages/twenty-server/src/engine/core-modules/workflow/controllers/workflow-trigger.controller.ts index a38859e1c..01664cdbe 100644 --- a/packages/twenty-server/src/engine/core-modules/workflow/controllers/workflow-trigger.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/workflow/controllers/workflow-trigger.controller.ts @@ -1,9 +1,18 @@ -import { Controller, Get, Param, Post, Req, UseFilters } from '@nestjs/common'; +import { + Controller, + Get, + Param, + Post, + Req, + UseFilters, + UseGuards, +} from '@nestjs/common'; import { Request } from 'express'; import { isDefined } from 'twenty-shared/utils'; import { WorkflowTriggerRestApiExceptionFilter } from 'src/engine/core-modules/workflow/filters/workflow-trigger-rest-api-exception.filter'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; import { FieldActorSource } from 'src/engine/metadata-modules/field-metadata/composite-types/actor.composite-type'; import { TwentyORMManager } from 'src/engine/twenty-orm/twenty-orm.manager'; import { @@ -27,6 +36,7 @@ export class WorkflowTriggerController { ) {} @Post('workflows/:workspaceId/:workflowId') + @UseGuards(PublicEndpointGuard) async runWorkflowByPostRequest( @Param('workspaceId') workspaceId: string, @Param('workflowId') workflowId: string, @@ -40,6 +50,7 @@ export class WorkflowTriggerController { } @Get('workflows/:workspaceId/:workflowId') + @UseGuards(PublicEndpointGuard) async runWorkflowByGetRequest( @Param('workspaceId') workspaceId: string, @Param('workflowId') workflowId: string, diff --git a/packages/twenty-server/src/engine/core-modules/workspace/workspace.resolver.ts b/packages/twenty-server/src/engine/core-modules/workspace/workspace.resolver.ts index 719479a7b..6bca3f8ad 100644 --- a/packages/twenty-server/src/engine/core-modules/workspace/workspace.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/workspace/workspace.resolver.ts @@ -24,6 +24,7 @@ import { DomainManagerService } from 'src/engine/core-modules/domain-manager/ser import { FeatureFlagDTO } from 'src/engine/core-modules/feature-flag/dtos/feature-flag-dto'; import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/feature-flag-key.enum'; import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/services/feature-flag.service'; +import { SignedFileDTO } from 'src/engine/core-modules/file/file-upload/dtos/signed-file.dto'; import { FileUploadService } from 'src/engine/core-modules/file/file-upload/services/file-upload.service'; import { FileService } from 'src/engine/core-modules/file/services/file.service'; import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; @@ -43,6 +44,7 @@ import { AuthApiKey } from 'src/engine/decorators/auth/auth-api-key.decorator'; import { AuthUserWorkspaceId } from 'src/engine/decorators/auth/auth-user-workspace-id.decorator'; import { AuthUser } from 'src/engine/decorators/auth/auth-user.decorator'; import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; import { SettingsPermissionsGuard } from 'src/engine/guards/settings-permissions.guard'; import { UserAuthGuard } from 'src/engine/guards/user-auth.guard'; import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; @@ -52,7 +54,6 @@ import { RoleDTO } from 'src/engine/metadata-modules/role/dtos/role.dto'; import { RoleService } from 'src/engine/metadata-modules/role/role.service'; import { GraphqlValidationExceptionFilter } from 'src/filters/graphql-validation-exception.filter'; import { streamToBuffer } from 'src/utils/stream-to-buffer'; -import { SignedFileDTO } from 'src/engine/core-modules/file/file-upload/dtos/signed-file.dto'; import { Workspace } from './workspace.entity'; @@ -283,6 +284,7 @@ export class WorkspaceResolver { } @Query(() => PublicWorkspaceDataOutput) + @UseGuards(PublicEndpointGuard) async getPublicWorkspaceDataByDomain( @Args('origin') origin: string, ): Promise { diff --git a/packages/twenty-server/src/engine/decorators/auth/auth-user.decorator.ts b/packages/twenty-server/src/engine/decorators/auth/auth-user.decorator.ts index 35d3ccc08..8f78cf219 100644 --- a/packages/twenty-server/src/engine/decorators/auth/auth-user.decorator.ts +++ b/packages/twenty-server/src/engine/decorators/auth/auth-user.decorator.ts @@ -16,7 +16,8 @@ export const AuthUser = createParamDecorator( if (!options?.allowUndefined && !request.user) { throw new ForbiddenException( - "You're not authorized to do this. Note: This endpoint requires a user and won't work with just an API key.", + "You're not authorized to do this. " + + "Note: This endpoint requires a user and won't work with just an API key.", ); } diff --git a/packages/twenty-server/src/engine/decorators/auth/auth-workspace.decorator.ts b/packages/twenty-server/src/engine/decorators/auth/auth-workspace.decorator.ts index 3cbbf02d4..d107f8b8e 100644 --- a/packages/twenty-server/src/engine/decorators/auth/auth-workspace.decorator.ts +++ b/packages/twenty-server/src/engine/decorators/auth/auth-workspace.decorator.ts @@ -1,11 +1,28 @@ -import { ExecutionContext, createParamDecorator } from '@nestjs/common'; +import { + ExecutionContext, + InternalServerErrorException, + createParamDecorator, +} from '@nestjs/common'; import { getRequest } from 'src/utils/extract-request'; +interface DecoratorOptions { + allowUndefined?: boolean; +} + export const AuthWorkspace = createParamDecorator( - (data: unknown, ctx: ExecutionContext) => { + (options: DecoratorOptions | undefined, ctx: ExecutionContext) => { const request = getRequest(ctx); + if (!options?.allowUndefined && !request.workspace) { + // We're throwing an internal error and not a ForbiddenException + // because this should never happen, this is an extra security measure + // but Auth should be handled through the guards not the decorator + throw new InternalServerErrorException( + "You're not authorized to do this. This should not ever happen.", + ); + } + return request.workspace; }, ); diff --git a/packages/twenty-server/src/engine/guards/impersonate-guard.spec.ts b/packages/twenty-server/src/engine/guards/__tests__/impersonate-guard.spec.ts similarity index 100% rename from packages/twenty-server/src/engine/guards/impersonate-guard.spec.ts rename to packages/twenty-server/src/engine/guards/__tests__/impersonate-guard.spec.ts diff --git a/packages/twenty-server/src/engine/guards/__tests__/public-endpoint.guard.spec.ts b/packages/twenty-server/src/engine/guards/__tests__/public-endpoint.guard.spec.ts new file mode 100644 index 000000000..d7249c553 --- /dev/null +++ b/packages/twenty-server/src/engine/guards/__tests__/public-endpoint.guard.spec.ts @@ -0,0 +1,37 @@ +import { ExecutionContext } from '@nestjs/common'; +import { Test, TestingModule } from '@nestjs/testing'; + +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; + +describe('PublicEndpointGuard', () => { + let guard: PublicEndpointGuard; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + providers: [PublicEndpointGuard], + }).compile(); + + guard = module.get(PublicEndpointGuard); + }); + + it('should be defined', () => { + expect(guard).toBeDefined(); + }); + + it('should always return true for any execution context', () => { + const mockContext = {} as ExecutionContext; + const result = guard.canActivate(mockContext); + + expect(result).toBe(true); + }); + + it('should return true even with null context', () => { + const result = guard.canActivate(null as any); + + expect(result).toBe(true); + }); + + it('should be injectable', () => { + expect(guard).toBeInstanceOf(PublicEndpointGuard); + }); +}); diff --git a/packages/twenty-server/src/engine/guards/public-endpoint.guard.ts b/packages/twenty-server/src/engine/guards/public-endpoint.guard.ts new file mode 100644 index 000000000..5e540ea60 --- /dev/null +++ b/packages/twenty-server/src/engine/guards/public-endpoint.guard.ts @@ -0,0 +1,16 @@ +import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common'; + +/** + * Guard that explicitly marks an endpoint as public/unprotected. + * This guard always returns true and serves as documentation + * that the endpoint is intentionally accessible without authentication. + * + * Usage: @UseGuards(PublicEndpointGuard) + */ +@Injectable() +export class PublicEndpointGuard implements CanActivate { + canActivate(_context: ExecutionContext): boolean { + // Always allow access - this is an explicit marker for public endpoints + return true; + } +} diff --git a/packages/twenty-server/src/engine/metadata-modules/role/role.resolver.ts b/packages/twenty-server/src/engine/metadata-modules/role/role.resolver.ts index 223460cbc..013916d3f 100644 --- a/packages/twenty-server/src/engine/metadata-modules/role/role.resolver.ts +++ b/packages/twenty-server/src/engine/metadata-modules/role/role.resolver.ts @@ -17,6 +17,8 @@ import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { AuthWorkspaceMemberId } from 'src/engine/decorators/auth/auth-workspace-member-id.decorator'; import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator'; import { SettingsPermissionsGuard } from 'src/engine/guards/settings-permissions.guard'; +import { UserAuthGuard } from 'src/engine/guards/user-auth.guard'; +import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; import { ObjectPermissionDTO } from 'src/engine/metadata-modules/object-permission/dtos/object-permission.dto'; import { UpsertObjectPermissionsInput } from 'src/engine/metadata-modules/object-permission/dtos/upsert-object-permissions.input'; import { ObjectPermissionService } from 'src/engine/metadata-modules/object-permission/object-permission.service'; @@ -38,6 +40,7 @@ import { UserRoleService } from 'src/engine/metadata-modules/user-role/user-role import { WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity'; @Resolver(() => RoleDTO) +@UseGuards(WorkspaceAuthGuard) @UseGuards(SettingsPermissionsGuard(SettingPermissionType.ROLES)) @UseFilters(PermissionsGraphqlApiExceptionFilter) export class RoleResolver { @@ -57,6 +60,7 @@ export class RoleResolver { } @Mutation(() => WorkspaceMember) + @UseGuards(UserAuthGuard) async updateWorkspaceMemberRole( @AuthWorkspace() workspace: Workspace, @Args('workspaceMemberId') workspaceMemberId: string, diff --git a/packages/twenty-server/src/engine/middlewares/constants/excluded-middleware-operations.constant.ts b/packages/twenty-server/src/engine/middlewares/constants/excluded-middleware-operations.constant.ts deleted file mode 100644 index 3265e4e29..000000000 --- a/packages/twenty-server/src/engine/middlewares/constants/excluded-middleware-operations.constant.ts +++ /dev/null @@ -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; diff --git a/packages/twenty-server/src/engine/middlewares/graphql-hydrate-request-from-token.middleware.ts b/packages/twenty-server/src/engine/middlewares/graphql-hydrate-request-from-token.middleware.ts index 5a4892610..b966bfe8c 100644 --- a/packages/twenty-server/src/engine/middlewares/graphql-hydrate-request-from-token.middleware.ts +++ b/packages/twenty-server/src/engine/middlewares/graphql-hydrate-request-from-token.middleware.ts @@ -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); diff --git a/packages/twenty-server/src/engine/middlewares/middleware.service.ts b/packages/twenty-server/src/engine/middlewares/middleware.service.ts index 35961a645..3ba346356 100644 --- a/packages/twenty-server/src/engine/middlewares/middleware.service.ts +++ b/packages/twenty-server/src/engine/middlewares/middleware.service.ts @@ -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, diff --git a/packages/twenty-server/src/engine/middlewares/rest-core.middleware.ts b/packages/twenty-server/src/engine/middlewares/rest-core.middleware.ts index 420bb6ee5..2f7dc9772 100644 --- a/packages/twenty-server/src/engine/middlewares/rest-core.middleware.ts +++ b/packages/twenty-server/src/engine/middlewares/rest-core.middleware.ts @@ -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); diff --git a/packages/twenty-server/src/engine/middlewares/utils/graphql-token-validation-utils.ts b/packages/twenty-server/src/engine/middlewares/utils/graphql-token-validation-utils.ts deleted file mode 100644 index 3f2d41c2c..000000000 --- a/packages/twenty-server/src/engine/middlewares/utils/graphql-token-validation-utils.ts +++ /dev/null @@ -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); - } - } -} diff --git a/tools/eslint-rules/index.ts b/tools/eslint-rules/index.ts index 98419b674..b28a2350c 100644 --- a/tools/eslint-rules/index.ts +++ b/tools/eslint-rules/index.ts @@ -1,54 +1,62 @@ import { - RULE_NAME as injectWorkspaceRepositoryName, - rule as injectWorkspaceRepository, -} from './rules/inject-workspace-repository'; -import { - rule as componentPropsNaming, - RULE_NAME as componentPropsNamingName, + rule as componentPropsNaming, + RULE_NAME as componentPropsNamingName, } from './rules/component-props-naming'; import { - rule as effectComponents, - RULE_NAME as effectComponentsName, + rule as effectComponents, + RULE_NAME as effectComponentsName, } from './rules/effect-components'; import { - rule as explicitBooleanPredicatesInIf, - RULE_NAME as explicitBooleanPredicatesInIfName, + rule as explicitBooleanPredicatesInIf, + RULE_NAME as explicitBooleanPredicatesInIfName, } from './rules/explicit-boolean-predicates-in-if'; import { - rule as matchingStateVariable, - RULE_NAME as matchingStateVariableName, + rule as graphqlResolversShouldBeGuarded, + RULE_NAME as graphqlResolversShouldBeGuardedName, +} from './rules/graphql-resolvers-should-be-guarded'; +import { + rule as injectWorkspaceRepository, + RULE_NAME as injectWorkspaceRepositoryName, +} from './rules/inject-workspace-repository'; +import { + rule as matchingStateVariable, + RULE_NAME as matchingStateVariableName, } from './rules/matching-state-variable'; import { - rule as maxConstsPerFile, - RULE_NAME as maxConstsPerFileName, + rule as maxConstsPerFile, + RULE_NAME as maxConstsPerFileName, } from './rules/max-consts-per-file'; import { - rule as noHardcodedColors, - RULE_NAME as noHardcodedColorsName, + rule as noHardcodedColors, + RULE_NAME as noHardcodedColorsName, } from './rules/no-hardcoded-colors'; import { - rule as noNavigatePreferLink, - RULE_NAME as noNavigatePreferLinkName, + rule as noNavigatePreferLink, + RULE_NAME as noNavigatePreferLinkName, } from './rules/no-navigate-prefer-link'; import { - rule as noStateUseref, - RULE_NAME as noStateUserefName, + rule as noStateUseref, + RULE_NAME as noStateUserefName, } from './rules/no-state-useref'; import { - rule as sortCssPropertiesAlphabetically, - RULE_NAME as sortCssPropertiesAlphabeticallyName, + rule as restApiMethodsShouldBeGuarded, + RULE_NAME as restApiMethodsShouldBeGuardedName, +} from './rules/rest-api-methods-should-be-guarded'; +import { + rule as sortCssPropertiesAlphabetically, + RULE_NAME as sortCssPropertiesAlphabeticallyName, } from './rules/sort-css-properties-alphabetically'; import { - rule as styledComponentsPrefixedWithStyled, - RULE_NAME as styledComponentsPrefixedWithStyledName, + rule as styledComponentsPrefixedWithStyled, + RULE_NAME as styledComponentsPrefixedWithStyledName, } from './rules/styled-components-prefixed-with-styled'; import { - rule as useGetLoadableAndGetValueToGetAtoms, - RULE_NAME as useGetLoadableAndGetValueToGetAtomsName, + rule as useGetLoadableAndGetValueToGetAtoms, + RULE_NAME as useGetLoadableAndGetValueToGetAtomsName, } from './rules/use-getLoadable-and-getValue-to-get-atoms'; import { - rule as useRecoilCallbackHasDependencyArray, - RULE_NAME as useRecoilCallbackHasDependencyArrayName, + rule as useRecoilCallbackHasDependencyArray, + RULE_NAME as useRecoilCallbackHasDependencyArrayName, } from './rules/useRecoilCallback-has-dependency-array'; /** @@ -93,5 +101,7 @@ module.exports = { useRecoilCallbackHasDependencyArray, [noNavigatePreferLinkName]: noNavigatePreferLink, [injectWorkspaceRepositoryName]: injectWorkspaceRepository, + [restApiMethodsShouldBeGuardedName]: restApiMethodsShouldBeGuarded, + [graphqlResolversShouldBeGuardedName]: graphqlResolversShouldBeGuarded, }, }; diff --git a/tools/eslint-rules/rules/graphql-resolvers-should-be-guarded.spec.ts b/tools/eslint-rules/rules/graphql-resolvers-should-be-guarded.spec.ts new file mode 100644 index 000000000..a878df3aa --- /dev/null +++ b/tools/eslint-rules/rules/graphql-resolvers-should-be-guarded.spec.ts @@ -0,0 +1,159 @@ +import { TSESLint } from '@typescript-eslint/utils'; + +import { rule, RULE_NAME } from './graphql-resolvers-should-be-guarded'; + +const ruleTester = new TSESLint.RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), +}); + +ruleTester.run(RULE_NAME, rule, { + valid: [ + { + code: ` + class TestResolver { + @Query() + @UseGuards(UserAuthGuard) + testQuery() {} + } + `, + }, + { + code: ` + class TestResolver { + @Query() + @UseGuards(WorkspaceAuthGuard) + testQuery() {} + } + `, + }, + { + code: ` + class TestResolver { + @Query() + @UseGuards(PublicEndpointGuard) + testQuery() {} + } + `, + }, + { + code: ` + class TestResolver { + @Query() + @UseGuards(CaptchaGuard, PublicEndpointGuard) + testQuery() {} + } + `, + }, + { + code: ` + @UseGuards(UserAuthGuard) + class TestResolver { + @Query() + testQuery() {} + } + `, + }, + { + code: ` + @UseGuards(WorkspaceAuthGuard) + class TestResolver { + @Query() + testQuery() {} + } + `, + }, + { + code: ` + @UseGuards(PublicEndpointGuard) + class TestResolver { + @Query() + testQuery() {} + } + `, + }, + { + code: ` + class TestResolver { + regularMethod() {} + } + `, + }, + { + code: ` + class TestResolver { + @ResolveField() + testField() {} + } + `, + }, + ], + invalid: [ + { + code: ` + class TestResolver { + @Query() + testQuery() {} + } + `, + errors: [ + { + messageId: 'graphqlResolversShouldBeGuarded', + }, + ], + }, + { + code: ` + class TestResolver { + @Mutation() + testMutation() {} + } + `, + errors: [ + { + messageId: 'graphqlResolversShouldBeGuarded', + }, + ], + }, + { + code: ` + class TestResolver { + @Subscription() + testSubscription() {} + } + `, + errors: [ + { + messageId: 'graphqlResolversShouldBeGuarded', + }, + ], + }, + { + code: ` + class TestResolver { + @Query() + @UseGuards(CaptchaGuard) + testQuery() {} + } + `, + errors: [ + { + messageId: 'graphqlResolversShouldBeGuarded', + }, + ], + }, + { + code: ` + @UseGuards(CaptchaGuard) + class TestResolver { + @Query() + testQuery() {} + } + `, + errors: [ + { + messageId: 'graphqlResolversShouldBeGuarded', + }, + ], + }, + ], +}); \ No newline at end of file diff --git a/tools/eslint-rules/rules/graphql-resolvers-should-be-guarded.ts b/tools/eslint-rules/rules/graphql-resolvers-should-be-guarded.ts new file mode 100644 index 000000000..8da8998d4 --- /dev/null +++ b/tools/eslint-rules/rules/graphql-resolvers-should-be-guarded.ts @@ -0,0 +1,70 @@ +import { TSESTree } from '@typescript-eslint/utils'; + +import { createRule } from '../utils/createRule'; +import { typedTokenHelpers } from '../utils/typedTokenHelpers'; + +// NOTE: The rule will be available in ESLint configs as "@nx/workspace-graphql-resolvers-should-be-guarded" +export const RULE_NAME = 'graphql-resolvers-should-be-guarded'; + +export const graphqlResolversShouldBeGuarded = (node: TSESTree.MethodDefinition) => { + const hasGraphQLResolverDecorator = typedTokenHelpers.nodeHasDecoratorsNamed( + node, + ['Query', 'Mutation', 'Subscription'] + ); + + const hasAuthGuards = typedTokenHelpers.nodeHasAuthGuards(node); + + function findClassDeclaration( + node: TSESTree.Node + ): TSESTree.ClassDeclaration | null { + if (node.type === TSESTree.AST_NODE_TYPES.ClassDeclaration) { + return node; + } + if (node.parent) { + return findClassDeclaration(node.parent); + } + return null; + } + + const classNode = findClassDeclaration(node); + + const hasAuthGuardsOnResolver = classNode + ? typedTokenHelpers.nodeHasAuthGuards(classNode) + : false; + + return ( + hasGraphQLResolverDecorator && + !hasAuthGuards && + !hasAuthGuardsOnResolver + ); +}; + +export const rule = createRule<[], 'graphqlResolversShouldBeGuarded'>({ + name: RULE_NAME, + meta: { + docs: { + description: + 'GraphQL root resolvers (Query, Mutation, Subscription) should have authentication guards (UserAuthGuard or WorkspaceAuthGuard) or be explicitly marked as public (PublicEndpointGuard) to maintain our security model.', + }, + messages: { + graphqlResolversShouldBeGuarded: + 'All GraphQL root resolver methods (@Query, @Mutation, @Subscription) should have @UseGuards(UserAuthGuard), @UseGuards(WorkspaceAuthGuard), or @UseGuards(PublicEndpointGuard) decorators, or one decorating the root of the Resolver class.', + }, + schema: [], + hasSuggestions: false, + type: 'suggestion', + }, + defaultOptions: [], + create(context) { + return { + MethodDefinition(node: TSESTree.MethodDefinition): void { + if (graphqlResolversShouldBeGuarded(node)) { + context.report({ + node: node, + messageId: 'graphqlResolversShouldBeGuarded', + }); + } + }, + }; + }, +}); \ No newline at end of file diff --git a/tools/eslint-rules/rules/rest-api-methods-should-be-guarded.spec.ts b/tools/eslint-rules/rules/rest-api-methods-should-be-guarded.spec.ts new file mode 100644 index 000000000..0d8c8f8c7 --- /dev/null +++ b/tools/eslint-rules/rules/rest-api-methods-should-be-guarded.spec.ts @@ -0,0 +1,138 @@ +import { TSESLint } from '@typescript-eslint/utils'; + +import { rule, RULE_NAME } from './rest-api-methods-should-be-guarded'; + +const ruleTester = new TSESLint.RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), +}); + +ruleTester.run(RULE_NAME, rule, { + valid: [ + { + code: ` + class TestController { + @Get() + @UseGuards(UserAuthGuard) + testMethod() {} + } + `, + }, + { + code: ` + class TestController { + @Get() + @UseGuards(WorkspaceAuthGuard) + testMethod() {} + } + `, + }, + { + code: ` + class TestController { + @Get() + @UseGuards(PublicEndpoint) + testMethod() {} + } + `, + }, + { + code: ` + class TestController { + @Get() + @UseGuards(CaptchaGuard, PublicEndpoint) + testMethod() {} + } + `, + }, + { + code: ` + @UseGuards(UserAuthGuard) + class TestController { + @Get() + testMethod() {} + } + `, + }, + { + code: ` + @UseGuards(WorkspaceAuthGuard) + class TestController { + @Get() + testMethod() {} + } + `, + }, + { + code: ` + @UseGuards(PublicEndpoint) + class TestController { + @Get() + testMethod() {} + } + `, + }, + { + code: ` + class TestController { + regularMethod() {} + } + `, + }, + ], + invalid: [ + { + code: ` + class TestController { + @Get() + testMethod() {} + } + `, + errors: [ + { + messageId: 'restApiMethodsShouldBeGuarded', + }, + ], + }, + { + code: ` + class TestController { + @Post() + testMethod() {} + } + `, + errors: [ + { + messageId: 'restApiMethodsShouldBeGuarded', + }, + ], + }, + { + code: ` + class TestController { + @Get() + @UseGuards(CaptchaGuard) + testMethod() {} + } + `, + errors: [ + { + messageId: 'restApiMethodsShouldBeGuarded', + }, + ], + }, + { + code: ` + @UseGuards(CaptchaGuard) + class TestController { + @Get() + testMethod() {} + } + `, + errors: [ + { + messageId: 'restApiMethodsShouldBeGuarded', + }, + ], + }, + ], +}); \ No newline at end of file diff --git a/tools/eslint-rules/rules/rest-api-methods-should-be-guarded.ts b/tools/eslint-rules/rules/rest-api-methods-should-be-guarded.ts new file mode 100644 index 000000000..e020f382d --- /dev/null +++ b/tools/eslint-rules/rules/rest-api-methods-should-be-guarded.ts @@ -0,0 +1,70 @@ +import { TSESTree } from '@typescript-eslint/utils'; + +import { createRule } from '../utils/createRule'; +import { typedTokenHelpers } from '../utils/typedTokenHelpers'; + +// NOTE: The rule will be available in ESLint configs as "@nx/workspace-rest-api-methods-should-be-guarded" +export const RULE_NAME = 'rest-api-methods-should-be-guarded'; + +export const restApiMethodsShouldBeGuarded = (node: TSESTree.MethodDefinition) => { + const hasRestApiMethodDecorator = typedTokenHelpers.nodeHasDecoratorsNamed( + node, + ['Get', 'Post', 'Put', 'Delete', 'Patch', 'Options', 'Head', 'All'] + ); + + const hasAuthGuards = typedTokenHelpers.nodeHasAuthGuards(node); + + function findClassDeclaration( + node: TSESTree.Node + ): TSESTree.ClassDeclaration | null { + if (node.type === TSESTree.AST_NODE_TYPES.ClassDeclaration) { + return node; + } + if (node.parent) { + return findClassDeclaration(node.parent); + } + return null; + } + + const classNode = findClassDeclaration(node); + + const hasAuthGuardsOnController = classNode + ? typedTokenHelpers.nodeHasAuthGuards(classNode) + : false; + + return ( + hasRestApiMethodDecorator && + !hasAuthGuards && + !hasAuthGuardsOnController + ); +}; + +export const rule = createRule<[], 'restApiMethodsShouldBeGuarded'>({ + name: RULE_NAME, + meta: { + docs: { + description: + 'REST API endpoints should have authentication guards (UserAuthGuard or WorkspaceAuthGuard) or be explicitly marked as public (PublicEndpointGuard) to maintain our security model.', + }, + messages: { + restApiMethodsShouldBeGuarded: + 'All REST API controller endpoints should have @UseGuards(UserAuthGuard), @UseGuards(WorkspaceAuthGuard), or @UseGuards(PublicEndpointGuard) decorators, or one decorating the root of the Controller.', + }, + schema: [], + hasSuggestions: false, + type: 'suggestion', + }, + defaultOptions: [], + create(context) { + return { + MethodDefinition(node: TSESTree.MethodDefinition): void { + if (restApiMethodsShouldBeGuarded(node)) { + context.report({ + node: node, + messageId: 'restApiMethodsShouldBeGuarded', + }); + } + }, + }; + }, +}); \ No newline at end of file diff --git a/tools/eslint-rules/utils/createRule.ts b/tools/eslint-rules/utils/createRule.ts new file mode 100644 index 000000000..901bc483d --- /dev/null +++ b/tools/eslint-rules/utils/createRule.ts @@ -0,0 +1,3 @@ +import { ESLintUtils } from '@typescript-eslint/utils'; + +export const createRule = ESLintUtils.RuleCreator(() => __filename); \ No newline at end of file diff --git a/tools/eslint-rules/utils/typedTokenHelpers.ts b/tools/eslint-rules/utils/typedTokenHelpers.ts new file mode 100644 index 000000000..102ae161e --- /dev/null +++ b/tools/eslint-rules/utils/typedTokenHelpers.ts @@ -0,0 +1,56 @@ +import { TSESTree } from '@typescript-eslint/utils'; + +export const typedTokenHelpers = { + nodeHasDecoratorsNamed( + node: TSESTree.MethodDefinition | TSESTree.ClassDeclaration, + decoratorNames: string[] + ): boolean { + if (!node.decorators) { + return false; + } + + return node.decorators.some((decorator) => { + if (decorator.expression.type === TSESTree.AST_NODE_TYPES.Identifier) { + return decoratorNames.includes(decorator.expression.name); + } + + if (decorator.expression.type === TSESTree.AST_NODE_TYPES.CallExpression) { + const callee = decorator.expression.callee; + if (callee.type === TSESTree.AST_NODE_TYPES.Identifier) { + return decoratorNames.includes(callee.name); + } + } + + return false; + }); + }, + + nodeHasAuthGuards( + node: TSESTree.MethodDefinition | TSESTree.ClassDeclaration + ): boolean { + if (!node.decorators) { + return false; + } + + return node.decorators.some((decorator) => { + // Check for @UseGuards() call expression + if ( + decorator.expression.type === TSESTree.AST_NODE_TYPES.CallExpression && + decorator.expression.callee.type === TSESTree.AST_NODE_TYPES.Identifier && + decorator.expression.callee.name === 'UseGuards' + ) { + // Check the arguments for UserAuthGuard, WorkspaceAuthGuard, or PublicEndpoint + return decorator.expression.arguments.some((arg) => { + if (arg.type === TSESTree.AST_NODE_TYPES.Identifier) { + return arg.name === 'UserAuthGuard' || + arg.name === 'WorkspaceAuthGuard' || + arg.name === 'PublicEndpointGuard'; + } + return false; + }); + } + + return false; + }); + }, +}; \ No newline at end of file