Add token type validation and remove dead code in JWT verification (#17784)
## Summary - Removes unreachable dead code in `verifyJwtToken` — a legacy API key verification block where the condition (`!payload.type && type === API_KEY`) was logically impossible. Also removes the now-unused `isLegacyApiKey` parameter and `generateAppSecretLegacy` method. - Adds explicit token type validation after JWT decode in `verifyLoginToken`, `verifyRefreshToken`, and `verifyTransientToken`. Each function now rejects tokens whose `type` field doesn't match what's expected (defense-in-depth — the HMAC secret already binds the type, but this makes the contract explicit). ## Test plan - [x] Updated existing specs for login-token, refresh-token, renew-token services — all 16 tests passing - [x] Lint clean Made with [Cursor](https://cursor.com) --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
This commit is contained in:
+8
-4
@@ -137,13 +137,17 @@ describe('LoginTokenService', () => {
|
||||
jest
|
||||
.spyOn(jwtWrapperService, 'verifyJwtToken')
|
||||
.mockResolvedValue(undefined);
|
||||
jest
|
||||
.spyOn(jwtWrapperService, 'decode')
|
||||
.mockReturnValue({ sub: mockEmail });
|
||||
jest.spyOn(jwtWrapperService, 'decode').mockReturnValue({
|
||||
sub: mockEmail,
|
||||
type: JwtTokenTypeEnum.LOGIN,
|
||||
});
|
||||
|
||||
const result = await service.verifyLoginToken(mockToken);
|
||||
|
||||
expect(result).toEqual({ sub: mockEmail });
|
||||
expect(result).toEqual({
|
||||
sub: mockEmail,
|
||||
type: JwtTokenTypeEnum.LOGIN,
|
||||
});
|
||||
expect(jwtWrapperService.verifyJwtToken).toHaveBeenCalledWith(mockToken);
|
||||
expect(jwtWrapperService.decode).toHaveBeenCalledWith(mockToken, {
|
||||
json: true,
|
||||
|
||||
+17
-3
@@ -4,6 +4,10 @@ import { addMilliseconds } from 'date-fns';
|
||||
import ms from 'ms';
|
||||
|
||||
import { type AuthToken } from 'src/engine/core-modules/auth/dto/auth-token.dto';
|
||||
import {
|
||||
AuthException,
|
||||
AuthExceptionCode,
|
||||
} from 'src/engine/core-modules/auth/auth.exception';
|
||||
import {
|
||||
type LoginTokenJwtPayload,
|
||||
JwtTokenTypeEnum,
|
||||
@@ -54,8 +58,18 @@ export class LoginTokenService {
|
||||
async verifyLoginToken(loginToken: string): Promise<LoginTokenJwtPayload> {
|
||||
await this.jwtWrapperService.verifyJwtToken(loginToken);
|
||||
|
||||
return this.jwtWrapperService.decode(loginToken, {
|
||||
json: true,
|
||||
});
|
||||
const decoded = this.jwtWrapperService.decode<LoginTokenJwtPayload>(
|
||||
loginToken,
|
||||
{ json: true },
|
||||
);
|
||||
|
||||
if (decoded.type !== JwtTokenTypeEnum.LOGIN) {
|
||||
throw new AuthException(
|
||||
'Expected a login token',
|
||||
AuthExceptionCode.INVALID_JWT_TOKEN_TYPE,
|
||||
);
|
||||
}
|
||||
|
||||
return decoded;
|
||||
}
|
||||
}
|
||||
|
||||
+1
@@ -73,6 +73,7 @@ describe('RefreshTokenService', () => {
|
||||
const mockJwtPayload = {
|
||||
jti: 'token-id',
|
||||
sub: 'user-id',
|
||||
type: JwtTokenTypeEnum.REFRESH,
|
||||
};
|
||||
const mockAppToken = {
|
||||
id: 'token-id',
|
||||
|
||||
+7
@@ -42,6 +42,13 @@ export class RefreshTokenService {
|
||||
const jwtPayload =
|
||||
this.jwtWrapperService.decode<RefreshTokenJwtPayload>(refreshToken);
|
||||
|
||||
if (jwtPayload.type !== JwtTokenTypeEnum.REFRESH) {
|
||||
throw new AuthException(
|
||||
'Expected a refresh token',
|
||||
AuthExceptionCode.INVALID_JWT_TOKEN_TYPE,
|
||||
);
|
||||
}
|
||||
|
||||
if (!(jwtPayload.jti && jwtPayload.sub)) {
|
||||
throw new AuthException(
|
||||
'This refresh token is malformed',
|
||||
|
||||
+1
@@ -91,6 +91,7 @@ describe('TransientTokenService', () => {
|
||||
const mockToken = 'valid-token';
|
||||
const mockPayload = {
|
||||
sub: 'workspace-member-id',
|
||||
type: JwtTokenTypeEnum.LOGIN,
|
||||
userId: 'user-id',
|
||||
workspaceId: 'workspace-id',
|
||||
workspaceMemberId: 'workspace-member-id',
|
||||
|
||||
+12
-1
@@ -4,6 +4,10 @@ import { addMilliseconds } from 'date-fns';
|
||||
import ms from 'ms';
|
||||
|
||||
import { type AuthToken } from 'src/engine/core-modules/auth/dto/auth-token.dto';
|
||||
import {
|
||||
AuthException,
|
||||
AuthExceptionCode,
|
||||
} from 'src/engine/core-modules/auth/auth.exception';
|
||||
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 {
|
||||
@@ -55,9 +59,16 @@ export class TransientTokenService {
|
||||
): Promise<Omit<TransientTokenJwtPayload, 'type' | 'sub'>> {
|
||||
await this.jwtWrapperService.verifyJwtToken(transientToken);
|
||||
|
||||
const { type: _type, ...payload } =
|
||||
const { type, ...payload } =
|
||||
this.jwtWrapperService.decode<TransientTokenJwtPayload>(transientToken);
|
||||
|
||||
if (type !== JwtTokenTypeEnum.LOGIN) {
|
||||
throw new AuthException(
|
||||
'Expected a transient token',
|
||||
AuthExceptionCode.INVALID_JWT_TOKEN_TYPE,
|
||||
);
|
||||
}
|
||||
|
||||
return payload;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -47,11 +47,7 @@ export class JwtWrapperService {
|
||||
return this.jwtService.decode(payload, options);
|
||||
}
|
||||
|
||||
verifyJwtToken(
|
||||
token: string,
|
||||
options?: JwtVerifyOptions,
|
||||
isLegacyApiKey = false,
|
||||
) {
|
||||
verifyJwtToken(token: string, options?: JwtVerifyOptions) {
|
||||
const payload = this.decode<JwtPayload>(token, {
|
||||
json: true,
|
||||
});
|
||||
@@ -77,25 +73,11 @@ export class JwtWrapperService {
|
||||
}
|
||||
|
||||
try {
|
||||
// Supporting old API KEY tokens
|
||||
if (
|
||||
!payload.type &&
|
||||
!('workspaceId' in payload) &&
|
||||
type === JwtTokenTypeEnum.API_KEY
|
||||
) {
|
||||
return this.jwtService.verify(token, {
|
||||
...options,
|
||||
secret: this.generateAppSecretLegacy(),
|
||||
});
|
||||
}
|
||||
|
||||
// This is due to an unfortunate mistake in the secret generation of API_KEY
|
||||
// tokens. We used to sign with ACCESS Jwt Token Type instead of API_KEY.
|
||||
// Now we need to check both cases not to break the existing api keys
|
||||
// See this PR for context -> https://github.com/twentyhq/twenty/pull/16504
|
||||
// This code block can be deleted, but all api keys created before
|
||||
// 12/12/2025 will be broken
|
||||
if (type === JwtTokenTypeEnum.API_KEY && !isLegacyApiKey) {
|
||||
// API_KEY tokens created before 12/12/2025 were accidentally signed
|
||||
// with ACCESS type instead of API_KEY. Try the correct secret first,
|
||||
// fall back to the old one for backward compatibility.
|
||||
// See https://github.com/twentyhq/twenty/pull/16504
|
||||
if (type === JwtTokenTypeEnum.API_KEY) {
|
||||
try {
|
||||
return this.jwtService.verify(token, {
|
||||
...options,
|
||||
@@ -148,18 +130,6 @@ export class JwtWrapperService {
|
||||
.digest('hex');
|
||||
}
|
||||
|
||||
generateAppSecretLegacy(): string {
|
||||
const accessTokenSecret = this.twentyConfigService.get(
|
||||
'ACCESS_TOKEN_SECRET',
|
||||
);
|
||||
|
||||
if (!accessTokenSecret) {
|
||||
throw new Error('ACCESS_TOKEN_SECRET is not set');
|
||||
}
|
||||
|
||||
return accessTokenSecret;
|
||||
}
|
||||
|
||||
extractJwtFromRequest(): JwtFromRequestFunction {
|
||||
return (request: ExpressRequest) => {
|
||||
// First try to extract token from Authorization header
|
||||
|
||||
@@ -238,17 +238,6 @@ export class ConfigVariables {
|
||||
})
|
||||
CALENDAR_PROVIDER_MICROSOFT_ENABLED = false;
|
||||
|
||||
@ConfigVariablesMetadata({
|
||||
group: ConfigVariablesGroup.OTHER,
|
||||
isSensitive: true,
|
||||
description:
|
||||
'Legacy variable to be deprecated when all API Keys expire. Replaced by APP_KEY',
|
||||
type: ConfigVariableType.STRING,
|
||||
isEnvOnly: true,
|
||||
})
|
||||
@IsOptional()
|
||||
ACCESS_TOKEN_SECRET: string;
|
||||
|
||||
@ConfigVariablesMetadata({
|
||||
group: ConfigVariablesGroup.TOKENS_DURATION,
|
||||
description: 'Duration for which the access token is valid',
|
||||
|
||||
Reference in New Issue
Block a user