From 45bea6f9910949b02217315df1516e0fce97e98f Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Fri, 15 May 2026 10:47:46 +0200 Subject: [PATCH] feat(secret-encryption): drop APP_SECRET from approved-access-domain validation and session cookies (#20580) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Continues retiring `APP_SECRET` as a hot signing secret (after the TOTP migration in #20577). This PR moves the last two cryptographic uses of `APP_SECRET` off it: 1. **Approved-access-domain validation tokens** — was a one-shot `sha256(JSON.stringify({id, domain, key: APP_SECRET}))` HMAC with no built-in expiry. Now a JWT signed by the workspace `signingKey` with a 7-day expiry and claims bound to `approvedAccessDomainId`, `workspaceId`, and `domain`. 2. **Express-session cookie signing** — was `sha256(APP_SECRET || 'SESSION_STORE_SECRET')`. Now `HKDF(ENCRYPTION_KEY, info='twenty:hmac:v1:session-cookie')` with `FALLBACK_ENCRYPTION_KEY` supported for rotation. ### Approved-access-domain — strict cutover - `ApprovedAccessDomainService.mintValidationToken` issues a JWT via `JwtWrapperService.signAsyncOrThrow` (workspace `signingKey`, asymmetric ES256 with kid-based rotation built in). - `validateApprovedAccessDomain` verifies the JWT, asserts `type === APPROVED_ACCESS_DOMAIN`, cross-checks `claim.approvedAccessDomainId` against the URL's `approvedAccessDomainId`, then re-checks `domain` and `workspaceId` against the stored row. Any failure maps to `APPROVED_ACCESS_DOMAIN_VALIDATION_TOKEN_INVALID`. - **No legacy fallback:** any pending invitation link minted with the old SHA hash will fail validation and must be re-sent. Volume is small and admins can re-issue from settings — this is the cleanest cutover. ### Session cookies — bridged cutover - `resolveSessionCookieSecretsOrThrow` returns an array `[HKDF(ENCRYPTION_KEY), HKDF(FALLBACK_ENCRYPTION_KEY)?, sha256(APP_SECRET || 'SESSION_STORE_SECRET')?]`. - `express-session` signs new cookies with the first secret and verifies against any entry, so in-flight cookies signed under the legacy SHA keep verifying until `maxAge` (30 min) expires. - New `deriveInstanceHmacKey` HKDF utility uses a dedicated `twenty:hmac:v1:` info prefix — distinct from the AEAD subkey prefix `twenty:enc:v2:` — so HMAC and encryption subkeys can never collide for the same raw `ENCRYPTION_KEY`. - TODO comment marks the legacy slot for removal post-2.5. ### Notes on rotation behaviour - Rotating `ENCRYPTION_KEY` while keeping the old value in `FALLBACK_ENCRYPTION_KEY` keeps cookies signed under either key verifying. New cookies sign under the new key. After all in-flight cookies expire (≤30 min), the fallback slot can be dropped from env. - Rotating the workspace `signingKey` (already supported by `JwtKeyManagerService`) keeps already-issued approved-access-domain JWTs verifying via `kid` until their 7-day expiry. ## Test plan - [x] Unit tests for `ApprovedAccessDomainService` cover: happy path, JWT verify failure, wrong token type, JWT id ≠ input id, JWT-claimed domain ≠ row, missing row, already-validated row. - [x] Unit tests for `resolveSessionCookieSecretsOrThrow` cover: throws without keys, primary order (`ENCRYPTION_KEY` → APP_SECRET fallback), `FALLBACK_ENCRYPTION_KEY` placement, empty-string vars treated as unset, legacy slot omitted when `APP_SECRET` missing, HKDF domain separation across purposes. - [x] `nx lint:diff-with-main twenty-server` — clean. - [x] Full test surface across approved-access-domain, secret-encryption, session-storage — 78/78 pass. - [ ] CI green. - [ ] Manual smoke: boot with a dummy `ENCRYPTION_KEY`, confirm sign-in succeeds (session cookie works), create + validate an approved-access-domain end-to-end through the UI. --- .../approved-access-domain.module.ts | 2 + .../approved-access-domain.service.ts | 119 +++++++-- .../services/approved-access-domain.spec.ts | 233 ++++++++++++++---- .../auth/types/auth-context.type.ts | 11 +- .../constants/secret-encryption.constant.ts | 3 + ...esolve-session-cookie-secrets.util.spec.ts | 102 ++++++++ .../utils/derive-instance-hmac-key.util.ts | 25 ++ .../resolve-session-cookie-secrets.util.ts | 55 +++++ .../session-storage.module-factory.ts | 17 +- .../twenty-config/config-variables.ts | 2 +- 10 files changed, 482 insertions(+), 87 deletions(-) create mode 100644 packages/twenty-server/src/engine/core-modules/secret-encryption/utils/__tests__/resolve-session-cookie-secrets.util.spec.ts create mode 100644 packages/twenty-server/src/engine/core-modules/secret-encryption/utils/derive-instance-hmac-key.util.ts create mode 100644 packages/twenty-server/src/engine/core-modules/secret-encryption/utils/resolve-session-cookie-secrets.util.ts diff --git a/packages/twenty-server/src/engine/core-modules/approved-access-domain/approved-access-domain.module.ts b/packages/twenty-server/src/engine/core-modules/approved-access-domain/approved-access-domain.module.ts index 95b1a71973..de5f2e4c3d 100644 --- a/packages/twenty-server/src/engine/core-modules/approved-access-domain/approved-access-domain.module.ts +++ b/packages/twenty-server/src/engine/core-modules/approved-access-domain/approved-access-domain.module.ts @@ -7,12 +7,14 @@ import { ApprovedAccessDomainResolver } from 'src/engine/core-modules/approved-a import { ApprovedAccessDomainService } from 'src/engine/core-modules/approved-access-domain/services/approved-access-domain.service'; import { WorkspaceDomainsModule } from 'src/engine/core-modules/domain/workspace-domains/workspace-domains.module'; import { FileModule } from 'src/engine/core-modules/file/file.module'; +import { JwtModule } from 'src/engine/core-modules/jwt/jwt.module'; import { PermissionsModule } from 'src/engine/metadata-modules/permissions/permissions.module'; @Module({ imports: [ WorkspaceDomainsModule, FileModule, + JwtModule, NestjsQueryTypeOrmModule.forFeature([ApprovedAccessDomainEntity]), PermissionsModule, ], diff --git a/packages/twenty-server/src/engine/core-modules/approved-access-domain/services/approved-access-domain.service.ts b/packages/twenty-server/src/engine/core-modules/approved-access-domain/services/approved-access-domain.service.ts index 49a9b8fd08..21563844f2 100644 --- a/packages/twenty-server/src/engine/core-modules/approved-access-domain/services/approved-access-domain.service.ts +++ b/packages/twenty-server/src/engine/core-modules/approved-access-domain/services/approved-access-domain.service.ts @@ -1,8 +1,6 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, Logger } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; -import crypto from 'crypto'; - import { msg } from '@lingui/core/macro'; import { render } from '@react-email/render'; import { SendApprovedAccessDomainValidation } from 'twenty-emails'; @@ -16,16 +14,27 @@ import { ApprovedAccessDomainExceptionCode, } from 'src/engine/core-modules/approved-access-domain/approved-access-domain.exception'; import { approvedAccessDomainValidator } from 'src/engine/core-modules/approved-access-domain/approved-access-domain.validate'; +import { + type ApprovedAccessDomainJwtPayload, + JwtTokenTypeEnum, +} from 'src/engine/core-modules/auth/types/auth-context.type'; import { WorkspaceDomainsService } from 'src/engine/core-modules/domain/workspace-domains/services/workspace-domains.service'; import { EmailService } from 'src/engine/core-modules/email/email.service'; import { FileUrlService } from 'src/engine/core-modules/file/file-url/file-url.service'; +import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service'; +import { decodeJwtHeader } from 'src/engine/core-modules/jwt/utils/decode-jwt-header.util'; +import { isAsymmetricJwtHeader } from 'src/engine/core-modules/jwt/utils/is-asymmetric-jwt-header.util'; import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; import { WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity'; import { isWorkDomain } from 'src/utils/is-work-email'; +const APPROVED_ACCESS_DOMAIN_TOKEN_EXPIRES_IN = '7d'; + @Injectable() export class ApprovedAccessDomainService { + private readonly logger = new Logger(ApprovedAccessDomainService.name); + constructor( @InjectRepository(ApprovedAccessDomainEntity) private readonly approvedAccessDomainRepository: Repository, @@ -33,6 +42,7 @@ export class ApprovedAccessDomainService { private readonly twentyConfigService: TwentyConfigService, private readonly fileUrlService: FileUrlService, private readonly workspaceDomainsService: WorkspaceDomainsService, + private readonly jwtWrapperService: JwtWrapperService, ) {} async sendApprovedAccessDomainValidationEmail( @@ -66,7 +76,10 @@ export class ApprovedAccessDomainService { pathname: getSettingsPath(SettingsPath.WorkspaceMembersPage), searchParams: { wtdId: approvedAccessDomain.id, - validationToken: this.generateUniqueHash(approvedAccessDomain), + validationToken: await this.mintValidationToken({ + approvedAccessDomain, + workspaceId: workspace.id, + }), }, }); @@ -111,19 +124,62 @@ export class ApprovedAccessDomainService { }); } - private generateUniqueHash( - approvedAccessDomain: ApprovedAccessDomainEntity, - ): string { - return crypto - .createHash('sha256') - .update( - JSON.stringify({ - id: approvedAccessDomain.id, - domain: approvedAccessDomain.domain, - key: this.twentyConfigService.get('APP_SECRET'), - }), - ) - .digest('hex'); + private async mintValidationToken({ + approvedAccessDomain, + workspaceId, + }: { + approvedAccessDomain: ApprovedAccessDomainEntity; + workspaceId: string; + }): Promise { + return this.jwtWrapperService.signAsyncOrThrow( + { + sub: approvedAccessDomain.id, + type: JwtTokenTypeEnum.APPROVED_ACCESS_DOMAIN, + workspaceId, + approvedAccessDomainId: approvedAccessDomain.id, + domain: approvedAccessDomain.domain, + }, + { expiresIn: APPROVED_ACCESS_DOMAIN_TOKEN_EXPIRES_IN }, + ); + } + + private async verifyValidationTokenOrThrow( + validationToken: string, + ): Promise { + if (!isAsymmetricJwtHeader(decodeJwtHeader(validationToken))) { + throw new ApprovedAccessDomainException( + 'Invalid approved access domain validation token', + ApprovedAccessDomainExceptionCode.APPROVED_ACCESS_DOMAIN_VALIDATION_TOKEN_INVALID, + ); + } + + let payload: ApprovedAccessDomainJwtPayload; + + try { + payload = (await this.jwtWrapperService.verifyJwtToken( + validationToken, + )) as ApprovedAccessDomainJwtPayload; + } catch (error) { + this.logger.warn( + `Rejected approved-access-domain validation token: ${ + error instanceof Error ? error.message : 'unknown reason' + }`, + ); + + throw new ApprovedAccessDomainException( + 'Invalid approved access domain validation token', + ApprovedAccessDomainExceptionCode.APPROVED_ACCESS_DOMAIN_VALIDATION_TOKEN_INVALID, + ); + } + + if (payload.type !== JwtTokenTypeEnum.APPROVED_ACCESS_DOMAIN) { + throw new ApprovedAccessDomainException( + 'Invalid approved access domain validation token', + ApprovedAccessDomainExceptionCode.APPROVED_ACCESS_DOMAIN_VALIDATION_TOKEN_INVALID, + ); + } + + return payload; } async validateApprovedAccessDomain({ @@ -133,6 +189,15 @@ export class ApprovedAccessDomainService { validationToken: string; approvedAccessDomainId: string; }) { + const payload = await this.verifyValidationTokenOrThrow(validationToken); + + if (payload.approvedAccessDomainId !== approvedAccessDomainId) { + throw new ApprovedAccessDomainException( + 'Invalid approved access domain validation token', + ApprovedAccessDomainExceptionCode.APPROVED_ACCESS_DOMAIN_VALIDATION_TOKEN_INVALID, + ); + } + const approvedAccessDomain = await this.approvedAccessDomainRepository.findOneBy({ id: approvedAccessDomainId, @@ -140,6 +205,16 @@ export class ApprovedAccessDomainService { approvedAccessDomainValidator.assertIsDefinedOrThrow(approvedAccessDomain); + if ( + payload.domain !== approvedAccessDomain.domain || + payload.workspaceId !== approvedAccessDomain.workspaceId + ) { + throw new ApprovedAccessDomainException( + 'Invalid approved access domain validation token', + ApprovedAccessDomainExceptionCode.APPROVED_ACCESS_DOMAIN_VALIDATION_TOKEN_INVALID, + ); + } + if (approvedAccessDomain.isValidated) { throw new ApprovedAccessDomainException( 'Approved access domain has already been validated', @@ -150,16 +225,6 @@ export class ApprovedAccessDomainService { ); } - const isHashValid = - this.generateUniqueHash(approvedAccessDomain) === validationToken; - - if (!isHashValid) { - throw new ApprovedAccessDomainException( - 'Invalid approved access domain validation token', - ApprovedAccessDomainExceptionCode.APPROVED_ACCESS_DOMAIN_VALIDATION_TOKEN_INVALID, - ); - } - return await this.approvedAccessDomainRepository.save({ ...approvedAccessDomain, isValidated: true, diff --git a/packages/twenty-server/src/engine/core-modules/approved-access-domain/services/approved-access-domain.spec.ts b/packages/twenty-server/src/engine/core-modules/approved-access-domain/services/approved-access-domain.spec.ts index 556aa9a682..47a50896f9 100644 --- a/packages/twenty-server/src/engine/core-modules/approved-access-domain/services/approved-access-domain.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/approved-access-domain/services/approved-access-domain.spec.ts @@ -10,8 +10,10 @@ import { ApprovedAccessDomainException, ApprovedAccessDomainExceptionCode, } from 'src/engine/core-modules/approved-access-domain/approved-access-domain.exception'; +import { JwtTokenTypeEnum } from 'src/engine/core-modules/auth/types/auth-context.type'; import { WorkspaceDomainsService } from 'src/engine/core-modules/domain/workspace-domains/services/workspace-domains.service'; import { EmailService } from 'src/engine/core-modules/email/email.service'; +import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service'; import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { type WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; import { type WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity'; @@ -36,6 +38,7 @@ describe('ApprovedAccessDomainService', () => { let emailService: EmailService; let twentyConfigService: TwentyConfigService; let workspaceDomainsService: WorkspaceDomainsService; + let jwtWrapperService: jest.Mocked; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -76,6 +79,13 @@ describe('ApprovedAccessDomainService', () => { .mockReturnValue('https://signed-url.com/logo.png'), }, }, + { + provide: JwtWrapperService, + useValue: { + signAsyncOrThrow: jest.fn(), + verifyJwtToken: jest.fn(), + }, + }, ], }).compile(); @@ -90,6 +100,9 @@ describe('ApprovedAccessDomainService', () => { workspaceDomainsService = module.get( WorkspaceDomainsService, ); + jwtWrapperService = module.get( + JwtWrapperService, + ) as unknown as jest.Mocked; }); describe('createApprovedAccessDomain', () => { @@ -277,11 +290,13 @@ describe('ApprovedAccessDomainService', () => { locale: 'en', } as WorkspaceMemberWorkspaceEntity; const workspace = { + id: 'workspace-id', displayName: 'Test Workspace', logo: '/logo.png', } as WorkspaceEntity; const email = 'validator@custom-domain.com'; const approvedAccessDomain = { + id: 'approved-access-domain-id', isValidated: false, domain: 'custom-domain.com', } as ApprovedAccessDomainEntity; @@ -301,6 +316,8 @@ describe('ApprovedAccessDomainService', () => { if (key === 'SERVER_URL') return 'https://api.example.com'; }); + jwtWrapperService.signAsyncOrThrow.mockResolvedValue('signed.jwt.token'); + await service.sendApprovedAccessDomainValidationEmail( sender, email, @@ -308,10 +325,23 @@ describe('ApprovedAccessDomainService', () => { approvedAccessDomain, ); + expect(jwtWrapperService.signAsyncOrThrow).toHaveBeenCalledWith( + { + sub: approvedAccessDomain.id, + type: JwtTokenTypeEnum.APPROVED_ACCESS_DOMAIN, + workspaceId: workspace.id, + approvedAccessDomainId: approvedAccessDomain.id, + domain: approvedAccessDomain.domain, + }, + { expiresIn: '7d' }, + ); expect(workspaceDomainsService.buildWorkspaceURL).toHaveBeenCalledWith({ workspace: workspace, pathname: getSettingsPath(SettingsPath.WorkspaceMembersPage), - searchParams: { validationToken: expect.any(String) }, + searchParams: { + wtdId: approvedAccessDomain.id, + validationToken: 'signed.jwt.token', + }, }); expect(emailService.send).toHaveBeenCalledWith({ @@ -325,28 +355,45 @@ describe('ApprovedAccessDomainService', () => { }); describe('validateApprovedAccessDomain', () => { + const approvedAccessDomainId = 'domain-id'; + const workspaceId = 'workspace-id'; + const domain = 'example.com'; + const encodeSegment = (value: object) => + Buffer.from(JSON.stringify(value)).toString('base64url'); + const buildToken = (header: object) => + `${encodeSegment(header)}.${encodeSegment({})}.signature`; + const validationToken = buildToken({ alg: 'ES256', kid: 'test-kid' }); + const buildPayload = (overrides: Record = {}) => ({ + sub: approvedAccessDomainId, + type: JwtTokenTypeEnum.APPROVED_ACCESS_DOMAIN, + workspaceId, + approvedAccessDomainId, + domain, + ...overrides, + }); + it('should validate the approved access domain successfully with a correct token', async () => { - const approvedAccessDomainId = 'domain-id'; - const validationToken = 'valid-token'; const approvedAccessDomain = { id: approvedAccessDomainId, - domain: 'example.com', + workspaceId, + domain, isValidated: false, } as ApprovedAccessDomainEntity; + jwtWrapperService.verifyJwtToken.mockResolvedValue(buildPayload()); jest .spyOn(approvedAccessDomainRepository, 'findOneBy') .mockResolvedValue(approvedAccessDomain); - jest - .spyOn(service as any, 'generateUniqueHash') - .mockReturnValue(validationToken); const saveSpy = jest.spyOn(approvedAccessDomainRepository, 'save'); await service.validateApprovedAccessDomain({ validationToken, - approvedAccessDomainId: approvedAccessDomainId, + approvedAccessDomainId, }); + expect(jwtWrapperService.verifyJwtToken).toHaveBeenCalledWith( + validationToken, + ); expect(approvedAccessDomainRepository.findOneBy).toHaveBeenCalledWith({ id: approvedAccessDomainId, }); @@ -355,47 +402,98 @@ describe('ApprovedAccessDomainService', () => { ); }); - it('should throw an error if the approved access domain does not exist', async () => { - const approvedAccessDomainId = 'invalid-domain-id'; - const validationToken = 'valid-token'; - - jest - .spyOn(approvedAccessDomainRepository, 'findOneBy') - .mockResolvedValue(null); + it('should reject any token whose header is not asymmetric (no kid / wrong alg) before calling verify', async () => { + const legacyHs256Token = buildToken({ alg: 'HS256' }); await expect( service.validateApprovedAccessDomain({ - validationToken, - approvedAccessDomainId: approvedAccessDomainId, + validationToken: legacyHs256Token, + approvedAccessDomainId, }), ).rejects.toThrowError( new ApprovedAccessDomainException( - 'Approved access domain not found', - ApprovedAccessDomainExceptionCode.APPROVED_ACCESS_DOMAIN_NOT_FOUND, + 'Invalid approved access domain validation token', + ApprovedAccessDomainExceptionCode.APPROVED_ACCESS_DOMAIN_VALIDATION_TOKEN_INVALID, ), ); + expect(jwtWrapperService.verifyJwtToken).not.toHaveBeenCalled(); + expect(approvedAccessDomainRepository.findOneBy).not.toHaveBeenCalled(); }); - it('should throw an error if the validation token is invalid', async () => { - const approvedAccessDomainId = 'domain-id'; - const validationToken = 'invalid-token'; - const approvedAccessDomain = { - id: approvedAccessDomainId, - domain: 'example.com', - isValidated: false, - } as ApprovedAccessDomainEntity; - - jest - .spyOn(approvedAccessDomainRepository, 'findOneBy') - .mockResolvedValue(approvedAccessDomain); - jest - .spyOn(service as any, 'generateUniqueHash') - .mockReturnValue('valid-token'); + it('should reject when the JWT verification fails (bad signature or expired)', async () => { + jwtWrapperService.verifyJwtToken.mockRejectedValue( + new Error('jwt expired'), + ); await expect( service.validateApprovedAccessDomain({ validationToken, - approvedAccessDomainId: approvedAccessDomainId, + approvedAccessDomainId, + }), + ).rejects.toThrowError( + new ApprovedAccessDomainException( + 'Invalid approved access domain validation token', + ApprovedAccessDomainExceptionCode.APPROVED_ACCESS_DOMAIN_VALIDATION_TOKEN_INVALID, + ), + ); + expect(approvedAccessDomainRepository.findOneBy).not.toHaveBeenCalled(); + }); + + it('should reject a JWT minted with a different token type', async () => { + jwtWrapperService.verifyJwtToken.mockResolvedValue( + buildPayload({ type: JwtTokenTypeEnum.ACCESS }), + ); + + await expect( + service.validateApprovedAccessDomain({ + validationToken, + approvedAccessDomainId, + }), + ).rejects.toThrowError( + new ApprovedAccessDomainException( + 'Invalid approved access domain validation token', + ApprovedAccessDomainExceptionCode.APPROVED_ACCESS_DOMAIN_VALIDATION_TOKEN_INVALID, + ), + ); + expect(approvedAccessDomainRepository.findOneBy).not.toHaveBeenCalled(); + }); + + it('should reject when the JWT approvedAccessDomainId does not match the input id', async () => { + jwtWrapperService.verifyJwtToken.mockResolvedValue( + buildPayload({ approvedAccessDomainId: 'other-domain-id' }), + ); + + await expect( + service.validateApprovedAccessDomain({ + validationToken, + approvedAccessDomainId, + }), + ).rejects.toThrowError( + new ApprovedAccessDomainException( + 'Invalid approved access domain validation token', + ApprovedAccessDomainExceptionCode.APPROVED_ACCESS_DOMAIN_VALIDATION_TOKEN_INVALID, + ), + ); + expect(approvedAccessDomainRepository.findOneBy).not.toHaveBeenCalled(); + }); + + it('should reject when the JWT-claimed domain does not match the stored row', async () => { + jwtWrapperService.verifyJwtToken.mockResolvedValue( + buildPayload({ domain: 'attacker.com' }), + ); + jest + .spyOn(approvedAccessDomainRepository, 'findOneBy') + .mockResolvedValue({ + id: approvedAccessDomainId, + workspaceId, + domain, + isValidated: false, + } as ApprovedAccessDomainEntity); + + await expect( + service.validateApprovedAccessDomain({ + validationToken, + approvedAccessDomainId, }), ).rejects.toThrowError( new ApprovedAccessDomainException( @@ -405,23 +503,66 @@ describe('ApprovedAccessDomainService', () => { ); }); - it('should throw an error if the approved access domain is already validated', async () => { - const approvedAccessDomainId = 'domain-id'; - const validationToken = 'valid-token'; - const approvedAccessDomain = { - id: approvedAccessDomainId, - domain: 'example.com', - isValidated: true, - } as ApprovedAccessDomainEntity; - + it('should reject when the JWT-claimed workspaceId does not match the stored row', async () => { + jwtWrapperService.verifyJwtToken.mockResolvedValue( + buildPayload({ workspaceId: 'other-workspace-id' }), + ); jest .spyOn(approvedAccessDomainRepository, 'findOneBy') - .mockResolvedValue(approvedAccessDomain); + .mockResolvedValue({ + id: approvedAccessDomainId, + workspaceId, + domain, + isValidated: false, + } as ApprovedAccessDomainEntity); await expect( service.validateApprovedAccessDomain({ validationToken, - approvedAccessDomainId: approvedAccessDomainId, + approvedAccessDomainId, + }), + ).rejects.toThrowError( + new ApprovedAccessDomainException( + 'Invalid approved access domain validation token', + ApprovedAccessDomainExceptionCode.APPROVED_ACCESS_DOMAIN_VALIDATION_TOKEN_INVALID, + ), + ); + }); + + it('should throw an error if the approved access domain does not exist', async () => { + jwtWrapperService.verifyJwtToken.mockResolvedValue(buildPayload()); + jest + .spyOn(approvedAccessDomainRepository, 'findOneBy') + .mockResolvedValue(null); + + await expect( + service.validateApprovedAccessDomain({ + validationToken, + approvedAccessDomainId, + }), + ).rejects.toThrowError( + new ApprovedAccessDomainException( + 'Approved access domain not found', + ApprovedAccessDomainExceptionCode.APPROVED_ACCESS_DOMAIN_NOT_FOUND, + ), + ); + }); + + it('should throw an error if the approved access domain is already validated', async () => { + jwtWrapperService.verifyJwtToken.mockResolvedValue(buildPayload()); + jest + .spyOn(approvedAccessDomainRepository, 'findOneBy') + .mockResolvedValue({ + id: approvedAccessDomainId, + workspaceId, + domain, + isValidated: true, + } as ApprovedAccessDomainEntity); + + await expect( + service.validateApprovedAccessDomain({ + validationToken, + approvedAccessDomainId, }), ).rejects.toThrowError( new ApprovedAccessDomainException( diff --git a/packages/twenty-server/src/engine/core-modules/auth/types/auth-context.type.ts b/packages/twenty-server/src/engine/core-modules/auth/types/auth-context.type.ts index ad6f766f49..89e1f1a599 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/types/auth-context.type.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/types/auth-context.type.ts @@ -48,6 +48,7 @@ export enum JwtTokenTypeEnum { APPLICATION_ACCESS = 'APPLICATION_ACCESS', APPLICATION_REFRESH = 'APPLICATION_REFRESH', APP_OAUTH_STATE = 'APP_OAUTH_STATE', + APPROVED_ACCESS_DOMAIN = 'APPROVED_ACCESS_DOMAIN', } type CommonPropertiesJwtPayload = { @@ -154,6 +155,13 @@ export type AppOAuthStateJwtPayload = CommonPropertiesJwtPayload & { codeVerifier: string | null; }; +export type ApprovedAccessDomainJwtPayload = CommonPropertiesJwtPayload & { + type: JwtTokenTypeEnum.APPROVED_ACCESS_DOMAIN; + workspaceId: string; + approvedAccessDomainId: string; + domain: string; +}; + export type JwtPayload = | AccessTokenJwtPayload | ApiKeyTokenJwtPayload @@ -165,4 +173,5 @@ export type JwtPayload = | RefreshTokenJwtPayload | FileTokenJwtPayload | FileTokenJwtPayloadLegacy - | AppOAuthStateJwtPayload; + | AppOAuthStateJwtPayload + | ApprovedAccessDomainJwtPayload; diff --git a/packages/twenty-server/src/engine/core-modules/secret-encryption/constants/secret-encryption.constant.ts b/packages/twenty-server/src/engine/core-modules/secret-encryption/constants/secret-encryption.constant.ts index 8129236a41..aa02e5245f 100644 --- a/packages/twenty-server/src/engine/core-modules/secret-encryption/constants/secret-encryption.constant.ts +++ b/packages/twenty-server/src/engine/core-modules/secret-encryption/constants/secret-encryption.constant.ts @@ -8,3 +8,6 @@ export const SECRET_ENCRYPTION_GCM_TAG_LENGTH = 16; export const SECRET_ENCRYPTION_DERIVED_KEY_LENGTH = 32; export const SECRET_ENCRYPTION_HKDF_INFO_PREFIX = 'twenty:enc:v2:'; export const SECRET_ENCRYPTION_INSTANCE_CONTEXT = 'instance'; + +export const INSTANCE_HMAC_HKDF_INFO_PREFIX = 'twenty:hmac:v1:'; +export const INSTANCE_HMAC_DERIVED_KEY_LENGTH = 32; diff --git a/packages/twenty-server/src/engine/core-modules/secret-encryption/utils/__tests__/resolve-session-cookie-secrets.util.spec.ts b/packages/twenty-server/src/engine/core-modules/secret-encryption/utils/__tests__/resolve-session-cookie-secrets.util.spec.ts new file mode 100644 index 0000000000..baec7de9ec --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/secret-encryption/utils/__tests__/resolve-session-cookie-secrets.util.spec.ts @@ -0,0 +1,102 @@ +import { createHash } from 'crypto'; + +import { deriveInstanceHmacKey } from 'src/engine/core-modules/secret-encryption/utils/derive-instance-hmac-key.util'; +import { resolveSessionCookieSecretsOrThrow } from 'src/engine/core-modules/secret-encryption/utils/resolve-session-cookie-secrets.util'; +import { type TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; + +type EnvMap = Partial<{ + ENCRYPTION_KEY: string; + FALLBACK_ENCRYPTION_KEY: string; + APP_SECRET: string; +}>; + +const buildConfig = (env: EnvMap): Pick => ({ + get: jest.fn((key: keyof EnvMap) => env[key]) as never, +}); + +const hmacFor = (rawKey: string) => + deriveInstanceHmacKey({ rawKey, purpose: 'session-cookie' }).toString('hex'); + +const legacyAppSecretHash = (appSecret: string) => + createHash('sha256').update(`${appSecret}SESSION_STORE_SECRET`).digest('hex'); + +describe('resolveSessionCookieSecretsOrThrow', () => { + it('throws when neither ENCRYPTION_KEY nor APP_SECRET is configured', () => { + expect(() => + resolveSessionCookieSecretsOrThrow({ + twentyConfigService: buildConfig({}), + }), + ).toThrow(/ENCRYPTION_KEY/); + }); + + it('signs with ENCRYPTION_KEY first when set, with legacy APP_SECRET hash kept for verification', () => { + const secrets = resolveSessionCookieSecretsOrThrow({ + twentyConfigService: buildConfig({ + ENCRYPTION_KEY: 'new-key', + APP_SECRET: 'app', + }), + }); + + expect(secrets[0]).toBe(hmacFor('new-key')); + expect(secrets).toContain(legacyAppSecretHash('app')); + expect(secrets).toHaveLength(2); + }); + + it('places FALLBACK_ENCRYPTION_KEY between the primary and the legacy slot', () => { + const secrets = resolveSessionCookieSecretsOrThrow({ + twentyConfigService: buildConfig({ + ENCRYPTION_KEY: 'new-key', + FALLBACK_ENCRYPTION_KEY: 'previous-key', + APP_SECRET: 'app', + }), + }); + + expect(secrets).toEqual([ + hmacFor('new-key'), + hmacFor('previous-key'), + legacyAppSecretHash('app'), + ]); + }); + + it('omits the FALLBACK slot when FALLBACK_ENCRYPTION_KEY is empty', () => { + const secrets = resolveSessionCookieSecretsOrThrow({ + twentyConfigService: buildConfig({ + ENCRYPTION_KEY: 'new-key', + FALLBACK_ENCRYPTION_KEY: '', + APP_SECRET: 'app', + }), + }); + + expect(secrets).toEqual([hmacFor('new-key'), legacyAppSecretHash('app')]); + }); + + it('omits the legacy slot when APP_SECRET is unset', () => { + const secrets = resolveSessionCookieSecretsOrThrow({ + twentyConfigService: buildConfig({ ENCRYPTION_KEY: 'new-key' }), + }); + + expect(secrets).toEqual([hmacFor('new-key')]); + }); + + it('falls back to HKDF(APP_SECRET) as primary when ENCRYPTION_KEY is unset, while keeping the legacy SHA slot', () => { + const secrets = resolveSessionCookieSecretsOrThrow({ + twentyConfigService: buildConfig({ APP_SECRET: 'app' }), + }); + + expect(secrets).toEqual([hmacFor('app'), legacyAppSecretHash('app')]); + expect(secrets[0]).not.toBe(secrets[1]); + }); + + it('derives different keys for different purposes (domain separation)', () => { + const sessionCookieKey = deriveInstanceHmacKey({ + rawKey: 'same', + purpose: 'session-cookie', + }); + const otherPurposeKey = deriveInstanceHmacKey({ + rawKey: 'same', + purpose: 'something-else', + }); + + expect(sessionCookieKey.equals(otherPurposeKey)).toBe(false); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/secret-encryption/utils/derive-instance-hmac-key.util.ts b/packages/twenty-server/src/engine/core-modules/secret-encryption/utils/derive-instance-hmac-key.util.ts new file mode 100644 index 0000000000..d564213d0b --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/secret-encryption/utils/derive-instance-hmac-key.util.ts @@ -0,0 +1,25 @@ +import { hkdfSync } from 'crypto'; + +import { + INSTANCE_HMAC_DERIVED_KEY_LENGTH, + INSTANCE_HMAC_HKDF_INFO_PREFIX, +} from 'src/engine/core-modules/secret-encryption/constants/secret-encryption.constant'; + +const ZERO_SALT = Buffer.alloc(32); + +export const deriveInstanceHmacKey = ({ + rawKey, + purpose, +}: { + rawKey: string; + purpose: string; +}): Buffer => + Buffer.from( + hkdfSync( + 'sha256', + Buffer.from(rawKey), + ZERO_SALT, + Buffer.from(`${INSTANCE_HMAC_HKDF_INFO_PREFIX}${purpose}`), + INSTANCE_HMAC_DERIVED_KEY_LENGTH, + ), + ); diff --git a/packages/twenty-server/src/engine/core-modules/secret-encryption/utils/resolve-session-cookie-secrets.util.ts b/packages/twenty-server/src/engine/core-modules/secret-encryption/utils/resolve-session-cookie-secrets.util.ts new file mode 100644 index 0000000000..e6a14767ea --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/secret-encryption/utils/resolve-session-cookie-secrets.util.ts @@ -0,0 +1,55 @@ +import { createHash } from 'crypto'; + +import { isNonEmptyString } from '@sniptt/guards'; + +import { deriveInstanceHmacKey } from 'src/engine/core-modules/secret-encryption/utils/derive-instance-hmac-key.util'; +import { type TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; + +const SESSION_COOKIE_HMAC_PURPOSE = 'session-cookie'; + +const buildLegacySessionSecret = (appSecret: string) => + createHash('sha256').update(`${appSecret}SESSION_STORE_SECRET`).digest('hex'); + +export const resolveSessionCookieSecretsOrThrow = ({ + twentyConfigService, +}: { + twentyConfigService: Pick; +}): string[] => { + const encryptionKey = twentyConfigService.get('ENCRYPTION_KEY'); + const fallbackEncryptionKey = twentyConfigService.get( + 'FALLBACK_ENCRYPTION_KEY', + ); + const appSecret = twentyConfigService.get('APP_SECRET'); + + const rawPrimary = isNonEmptyString(encryptionKey) + ? encryptionKey + : appSecret; + + if (!isNonEmptyString(rawPrimary)) { + throw new Error( + 'Cannot derive session cookie secret: set ENCRYPTION_KEY (or APP_SECRET for legacy deployments).', + ); + } + + const secrets: string[] = [ + deriveInstanceHmacKey({ + rawKey: rawPrimary, + purpose: SESSION_COOKIE_HMAC_PURPOSE, + }).toString('hex'), + ]; + + if (isNonEmptyString(fallbackEncryptionKey)) { + secrets.push( + deriveInstanceHmacKey({ + rawKey: fallbackEncryptionKey, + purpose: SESSION_COOKIE_HMAC_PURPOSE, + }).toString('hex'), + ); + } + + if (isNonEmptyString(appSecret)) { + secrets.push(buildLegacySessionSecret(appSecret)); + } + + return secrets; +}; 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 457f20769c..69a9f7076a 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 @@ -1,5 +1,3 @@ -import { createHash } from 'crypto'; - import { Logger } from '@nestjs/common'; import RedisStore from 'connect-redis'; @@ -8,6 +6,7 @@ import { createClient } from 'redis'; import type session from 'express-session'; import { CacheStorageType } from 'src/engine/core-modules/cache-storage/types/cache-storage-type.enum'; +import { resolveSessionCookieSecretsOrThrow } from 'src/engine/core-modules/secret-encryption/utils/resolve-session-cookie-secrets.util'; import { type TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; const sessionStorageLogger = new Logger('SessionStorage'); @@ -21,18 +20,12 @@ export const getSessionStorageOptions = ( const SERVER_URL = twentyConfigService.get('SERVER_URL'); - const appSecret = twentyConfigService.get('APP_SECRET'); - - if (!appSecret) { - throw new Error('APP_SECRET is not set'); - } - - const sessionSecret = createHash('sha256') - .update(`${appSecret}SESSION_STORE_SECRET`) - .digest('hex'); + const sessionSecrets = resolveSessionCookieSecretsOrThrow({ + twentyConfigService, + }); const sessionStorage: session.SessionOptions = { - secret: sessionSecret, + secret: sessionSecrets, resave: false, saveUninitialized: false, proxy: true, 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 1023bad75a..fcad949659 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 @@ -1158,7 +1158,7 @@ export class ConfigVariables { group: ConfigVariablesGroup.SERVER_CONFIG, isSensitive: true, description: - 'Decrypt-only fallback key. During rotation, set this to the previous ENCRYPTION_KEY so rows encrypted with the old key remain readable.', + 'Verification-only fallback key. During rotation, set this to the previous ENCRYPTION_KEY so rows encrypted with the old key remain decryptable and session cookies signed under it still verify.', isEnvOnly: true, type: ConfigVariableType.STRING, })