diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000000000..7c4ba90c76502 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,27 @@ +## Related Ticket + + +## Description + + +## Type of Change +- [ ] Bug fix +- [ ] New feature +- [ ] Improvement / Enhancement +- [ ] Refactoring +- [ ] Performance improvement +- [ ] Documentation +- [ ] Infrastructure / CI + +## Testing + +- [ ] Tested locally +- [ ] New / updated tests included + +## Screenshots + + +## Checklist +- [ ] No lint or build errors +- [ ] All existing tests pass +- [ ] Documentation updated (if needed) diff --git a/packages/twenty-front/src/pages/auth/SignInUp.tsx b/packages/twenty-front/src/pages/auth/SignInUp.tsx index 8c261f298324b..4819abe72579a 100644 --- a/packages/twenty-front/src/pages/auth/SignInUp.tsx +++ b/packages/twenty-front/src/pages/auth/SignInUp.tsx @@ -95,9 +95,23 @@ export const SignInUp = () => { // served separately from the backend. In the unified-image setup // both share an origin and the absolute URL collapses to the same // path; in split-deploy mode it correctly hits the API. - window.location.replace( - `${REACT_APP_SERVER_BASE_URL}/auth/sso/proxy-login`, + // + // Thread returnToPath so the server can redirect back after setting + // the JWT cookie -- without this, navigating to /authorize (MCP + // OAuth consent) would land the user on the dashboard instead of + // returning to the consent screen after SSO. + const returnToPath = + window.location.pathname + window.location.search + window.location.hash; + const proxyUrl = new URL( + '/auth/sso/proxy-login', + REACT_APP_SERVER_BASE_URL || window.location.origin, ); + + if (returnToPath && returnToPath !== '/') { + proxyUrl.searchParams.set('returnToPath', returnToPath); + } + + window.location.replace(proxyUrl.toString()); } }, [isSsoEnabled]); diff --git a/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-proxy-login.controller.ts b/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-proxy-login.controller.ts index aea556dd01aaa..e2745ff19b59b 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-proxy-login.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/controllers/sso-proxy-login.controller.ts @@ -1,16 +1,18 @@ import { Controller, + ForbiddenException, Get, Headers, HttpStatus, Logger, NotFoundException, + Req, Res, UseFilters, UseGuards, } from '@nestjs/common'; -import { Response } from 'express'; +import { type Request, Response } from 'express'; import ms from 'ms'; import { AuthRestApiExceptionFilter } from 'src/engine/core-modules/auth/filters/auth-rest-api-exception.filter'; @@ -22,6 +24,10 @@ import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twent import { AuthProviderEnum } from 'src/engine/core-modules/workspace/types/workspace.type'; import { NoPermissionGuard } from 'src/engine/guards/no-permission.guard'; import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; +import { + CorporateIdError, + assertCorporateId, +} from 'src/engine/utils/proxy-identity.util'; const TOKEN_PAIR_COOKIE_NAME = 'tokenPair'; @@ -54,6 +60,7 @@ export class SsoProxyLoginController { @Get('proxy-login') @UseGuards(PublicEndpointGuard, NoPermissionGuard) async proxyLogin( + @Req() req: Request, @Headers('x-auth-request-email') headerEmail: string | string[] | undefined, @Headers('x-auth-request-user') headerUser: string | string[] | undefined, @Res() res: Response, @@ -62,6 +69,31 @@ export class SsoProxyLoginController { throw new NotFoundException(); } + // Layer 2 corporate ID enforcement — reject at login time if the + // access token's corporate_id does not match this deployment. + // Unlike middleware/guard paths, proxy-login is always behind + // Traefik ForwardAuth, so a missing header with enforcement on + // is suspicious and must be rejected. + const expectedCorporateId = this.twentyConfigService.get('SMB_CORPORATE_ID'); + + if (expectedCorporateId && !req.get('x-auth-request-access-token')) { + this.logger.warn('SSO proxy-login refused: missing access token header'); + + throw new ForbiddenException({ error: 'access_denied' }); + } + + try { + assertCorporateId(req, this.twentyConfigService); + } catch (error) { + if (error instanceof CorporateIdError) { + this.logger.warn(`SSO proxy-login refused: ${error.message}`); + + throw new ForbiddenException({ error: 'access_denied' }); + } + + throw error; + } + const email = this.resolveEmail(headerEmail, headerUser); if (!email) { @@ -101,7 +133,17 @@ export class SsoProxyLoginController { }, }); - return res.redirect(HttpStatus.FOUND, '/'); + // Honour returnToPath so callers like the MCP OAuth /authorize page + // can survive the SSO round-trip. Validate that the value is a + // same-origin path (starts with exactly one `/`) to prevent open + // redirect attacks via protocol-relative URLs (`//evil.com`). + const returnToPath = req.query.returnToPath as string | undefined; + const destination = + returnToPath?.startsWith('/') && !returnToPath.startsWith('//') + ? returnToPath + : '/'; + + return res.redirect(HttpStatus.FOUND, destination); } private resolveEmail( diff --git a/packages/twenty-server/src/engine/core-modules/cache-storage/cache-storage.module-factory.ts b/packages/twenty-server/src/engine/core-modules/cache-storage/cache-storage.module-factory.ts index 303cf9cd2182f..cd068cde28587 100644 --- a/packages/twenty-server/src/engine/core-modules/cache-storage/cache-storage.module-factory.ts +++ b/packages/twenty-server/src/engine/core-modules/cache-storage/cache-storage.module-factory.ts @@ -1,13 +1,14 @@ import { type CacheModuleOptions } from '@nestjs/cache-manager'; -import { redisStore } from 'cache-manager-redis-yet'; +import { redisInsStore } from 'cache-manager-redis-yet'; +import { createClient, type RedisClientType } from 'redis'; import { CacheStorageType } from 'src/engine/core-modules/cache-storage/types/cache-storage-type.enum'; import { type TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; -export const cacheStorageModuleFactory = ( +export const cacheStorageModuleFactory = async ( twentyConfigService: TwentyConfigService, -): CacheModuleOptions => { +): Promise => { const cacheStorageType = CacheStorageType.Redis; const cacheStorageTtl = twentyConfigService.get('CACHE_STORAGE_TTL'); const cacheModuleOptions: CacheModuleOptions = { @@ -28,10 +29,27 @@ export const cacheStorageModuleFactory = ( ); } + const redisClient = createClient({ + url: redisUrl, + pingInterval: 30_000, + socket: { + keepAlive: 30_000, + reconnectStrategy: (retries: number) => + Math.min(retries * 200, 5_000), + }, + }); + + redisClient.on('error', (err) => + console.error('Cache Redis client error:', err), + ); + + await redisClient.connect(); + return { ...cacheModuleOptions, - store: redisStore, - url: redisUrl, + store: redisInsStore(redisClient as RedisClientType, { + ttl: cacheStorageTtl * 1000, + }), }; } default: diff --git a/packages/twenty-server/src/engine/core-modules/session-storage/session-storage.module-factory.ts b/packages/twenty-server/src/engine/core-modules/session-storage/session-storage.module-factory.ts index c8aab18e95272..00308f987c38a 100644 --- a/packages/twenty-server/src/engine/core-modules/session-storage/session-storage.module-factory.ts +++ b/packages/twenty-server/src/engine/core-modules/session-storage/session-storage.module-factory.ts @@ -57,11 +57,23 @@ export const getSessionStorageOptions = ( const redisClient = createClient({ url: connectionString, + pingInterval: 30_000, + socket: { + keepAlive: 30_000, + reconnectStrategy: (retries: number) => + Math.min(retries * 200, 5_000), + }, }); - redisClient.connect().catch((err) => { - throw new Error(`Redis connection failed: ${err}`); - }); + redisClient.on('error', (err) => + console.error('Session Redis client error:', err), + ); + + redisClient + .connect() + .catch((err) => + console.error('Session Redis initial connect failed:', err), + ); return { ...sessionStorage, diff --git a/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts b/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts index 460e807862337..cb74b7ca84631 100644 --- a/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts +++ b/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts @@ -1721,6 +1721,15 @@ export class ConfigVariables { @IsOptional() SMB_NAME = ''; + @ConfigVariablesMetadata({ + group: ConfigVariablesGroup.ADVANCED_SETTINGS, + description: + 'Cognito corporate_id for Layer 2 tenant isolation. When set, every authenticated request must carry an X-Auth-Request-Access-Token JWT whose custom:is_corporate="true" and custom:corporate_id matches this value. Empty = skip check (backward compatible).', + type: ConfigVariableType.STRING, + }) + @IsOptional() + SMB_CORPORATE_ID = ''; + @ConfigVariablesMetadata({ group: ConfigVariablesGroup.ADVANCED_SETTINGS, description: diff --git a/packages/twenty-server/src/engine/guards/jwt-auth.guard.ts b/packages/twenty-server/src/engine/guards/jwt-auth.guard.ts index c0530c05a4b2b..c99ef696de6e5 100644 --- a/packages/twenty-server/src/engine/guards/jwt-auth.guard.ts +++ b/packages/twenty-server/src/engine/guards/jwt-auth.guard.ts @@ -12,6 +12,8 @@ import { AccessTokenService } from 'src/engine/core-modules/auth/token/services/ import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { bindDataToRequestObject } from 'src/engine/utils/bind-data-to-request-object.util'; import { + CorporateIdError, + assertCorporateId, clearTokenPairCookie, matchesProxyIdentity, } from 'src/engine/utils/proxy-identity.util'; @@ -73,6 +75,22 @@ export class JwtAuthGuard implements CanActivate { return false; } + // Layer 2 corporate ID enforcement — when SMB_CORPORATE_ID is set + // and AUTH_TYPE=SSO, verify the access token's custom:corporate_id. + if (this.twentyConfigService.get('AUTH_TYPE') === 'SSO') { + try { + assertCorporateId(request, this.twentyConfigService); + } catch (error) { + if (error instanceof CorporateIdError) { + this.logger.warn(`Auth refused: ${error.message}`); + + return false; + } + + throw error; + } + } + const metadataVersion = data.workspace ? await this.workspaceStorageCacheService.getMetadataVersion( data.workspace.id, diff --git a/packages/twenty-server/src/engine/middlewares/middleware.service.ts b/packages/twenty-server/src/engine/middlewares/middleware.service.ts index 25fdea7eafc64..530257e329665 100644 --- a/packages/twenty-server/src/engine/middlewares/middleware.service.ts +++ b/packages/twenty-server/src/engine/middlewares/middleware.service.ts @@ -24,6 +24,8 @@ import { handleExceptionAndConvertToGraphQLError, } from 'src/engine/utils/global-exception-handler.util'; import { + CorporateIdError, + assertCorporateId, clearTokenPairCookie, matchesProxyIdentity, } from 'src/engine/utils/proxy-identity.util'; @@ -110,6 +112,7 @@ export class MiddlewareService { const data = await this.accessTokenService.validateTokenByRequest(request); this.assertProxyIdentityMatchesUser(request, response, data.user?.email); + this.assertCorporateIdMatches(request); const metadataVersion = data.workspace ? await this.workspaceStorageCacheService.getMetadataVersion( @@ -140,6 +143,7 @@ export class MiddlewareService { const data = await this.accessTokenService.validateTokenByRequest(request); this.assertProxyIdentityMatchesUser(request, response, data.user?.email); + this.assertCorporateIdMatches(request); const metadataVersion = data.workspace ? await this.workspaceStorageCacheService.getMetadataVersion( @@ -187,6 +191,29 @@ export class MiddlewareService { ); } + // Layer 2 corporate ID enforcement. When SMB_CORPORATE_ID is set, + // every authenticated request must carry an access token with matching + // custom:corporate_id. Gated on AUTH_TYPE=SSO so non-SSO deployments + // are unaffected. + private assertCorporateIdMatches(request: Request): void { + if (this.twentyConfigService.get('AUTH_TYPE') !== 'SSO') { + return; + } + + try { + assertCorporateId(request, this.twentyConfigService); + } catch (error) { + if (error instanceof CorporateIdError) { + throw new AuthException( + error.message, + AuthExceptionCode.FORBIDDEN_EXCEPTION, + ); + } + + throw error; + } + } + private hasErrorStatus(error: unknown): error is { status: number } { return isDefined((error as { status: number })?.status); } diff --git a/packages/twenty-server/src/engine/utils/proxy-identity.util.ts b/packages/twenty-server/src/engine/utils/proxy-identity.util.ts index 0d2f7e8bb39a0..3afbeb2c23b11 100644 --- a/packages/twenty-server/src/engine/utils/proxy-identity.util.ts +++ b/packages/twenty-server/src/engine/utils/proxy-identity.util.ts @@ -106,6 +106,67 @@ export const normalizeProxyIdentity = ( return `${trimmed}@${domain}`; }; +// Layer 2 tenant isolation: verify the caller's mPass access token +// carries custom:corporate_id matching SMB_CORPORATE_ID. Skipped when +// the env var is empty OR when the request has no access token header +// (MCP, API-key, and internal traffic bypass oauth2-proxy). +export const assertCorporateId = ( + request: Request, + configService: TwentyConfigService, +): void => { + const expectedCorporateId = configService.get('SMB_CORPORATE_ID'); + + if (!expectedCorporateId) { + return; + } + + const accessToken = request.get('x-auth-request-access-token'); + + if (!accessToken) { + return; + } + + try { + const parts = accessToken.split('.'); + + if (parts.length < 2) { + throw new CorporateIdError('Access denied: malformed access token'); + } + + const payloadB64 = parts[1]; + const payload = JSON.parse( + Buffer.from(payloadB64, 'base64url').toString(), + ); + + if (payload['custom:is_corporate'] !== 'true') { + throw new CorporateIdError('Access denied: not a corporate account'); + } + + if (payload['custom:corporate_id'] !== expectedCorporateId) { + throw new CorporateIdError('Access denied: corporate ID mismatch'); + } + } catch (error) { + if (error instanceof CorporateIdError) { + throw error; + } + + throw new CorporateIdError('Access denied: invalid access token'); + } +}; + +/** + * Typed error for corporate ID enforcement so callers can distinguish + * it from other errors and map to a 403 response. + */ +export class CorporateIdError extends Error { + readonly statusCode = 403; + + constructor(message: string) { + super(message); + this.name = 'CorporateIdError'; + } +} + /** * Expire the tokenPair cookie. Defensive: in some test harnesses * `response.clearCookie` may not be wired up, so guard with a typeof