fix(email-verification): prevent double email validation (#12250)

Fix #12177 
Fix #12171

---------

Co-authored-by: Charles Bochet <charles@twenty.com>
This commit is contained in:
Antoine Moreaux
2025-05-23 18:24:26 +02:00
committed by GitHub
parent 8de85eea61
commit 5428348d7f
14 changed files with 206 additions and 43 deletions

View File

@ -9,10 +9,10 @@ import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/service
import { UserWorkspaceService } from 'src/engine/core-modules/user-workspace/user-workspace.service';
import { UserService } from 'src/engine/core-modules/user/services/user.service';
import { User } from 'src/engine/core-modules/user/user.entity';
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { PermissionsService } from 'src/engine/metadata-modules/permissions/permissions.service';
import { SSOService } from 'src/engine/core-modules/sso/services/sso.service';
import { SignInUpService } from 'src/engine/core-modules/auth/services/sign-in-up.service';
import { AppToken } from 'src/engine/core-modules/app-token/app-token.entity';
import { AuthResolver } from './auth.resolver';
@ -34,7 +34,7 @@ describe('AuthResolver', () => {
providers: [
AuthResolver,
{
provide: getRepositoryToken(Workspace, 'core'),
provide: getRepositoryToken(AppToken, 'core'),
useValue: {},
},
{

View File

@ -53,6 +53,7 @@ import { UserAuthGuard } from 'src/engine/guards/user-auth.guard';
import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard';
import { SettingPermissionType } from 'src/engine/metadata-modules/permissions/constants/setting-permission-type.constants';
import { PermissionsGraphqlApiExceptionFilter } from 'src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter';
import { AppToken } from 'src/engine/core-modules/app-token/app-token.entity';
import { GetAuthTokensFromLoginTokenInput } from './dto/get-auth-tokens-from-login-token.input';
import { GetLoginTokenFromCredentialsInput } from './dto/get-login-token-from-credentials.input';
@ -75,6 +76,8 @@ export class AuthResolver {
constructor(
@InjectRepository(User, 'core')
private readonly userRepository: Repository<User>,
@InjectRepository(AppToken, 'core')
private readonly appTokenRepository: Repository<AppToken>,
private authService: AuthService,
private renewTokenService: RenewTokenService,
private userService: UserService,
@ -168,21 +171,24 @@ export class AuthResolver {
getLoginTokenFromEmailVerificationTokenInput: GetLoginTokenFromEmailVerificationTokenInput,
@Args('origin') origin: string,
) {
const user =
const appToken =
await this.emailVerificationTokenService.validateEmailVerificationTokenOrThrow(
getLoginTokenFromEmailVerificationTokenInput.emailVerificationToken,
getLoginTokenFromEmailVerificationTokenInput,
);
const workspace =
(await this.domainManagerService.getWorkspaceByOriginOrDefaultWorkspace(
origin,
)) ??
(await this.userWorkspaceService.findFirstWorkspaceByUserId(user.id));
(await this.userWorkspaceService.findFirstWorkspaceByUserId(
appToken.user.id,
));
await this.userService.markEmailAsVerified(user.id);
await this.userService.markEmailAsVerified(appToken.user.id);
await this.appTokenRepository.remove(appToken);
const loginToken = await this.loginTokenService.generateLoginToken(
user.email,
appToken.user.email,
workspace.id,
);

View File

@ -9,6 +9,11 @@ export class GetLoginTokenFromEmailVerificationTokenInput {
@IsString()
emailVerificationToken: string;
@Field(() => String)
@IsNotEmpty()
@IsString()
email: string;
@Field(() => String, { nullable: true })
@IsString()
@IsOptional()

View File

@ -14,12 +14,14 @@ import {
EmailVerificationExceptionCode,
} from 'src/engine/core-modules/email-verification/email-verification.exception';
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
import { User } from 'src/engine/core-modules/user/user.entity';
import { EmailVerificationTokenService } from './email-verification-token.service';
describe('EmailVerificationTokenService', () => {
let service: EmailVerificationTokenService;
let appTokenRepository: Repository<AppToken>;
let userRepository: Repository<User>;
let twentyConfigService: TwentyConfigService;
beforeEach(async () => {
@ -30,6 +32,12 @@ describe('EmailVerificationTokenService', () => {
provide: getRepositoryToken(AppToken, 'core'),
useClass: Repository,
},
{
provide: getRepositoryToken(User, 'core'),
useValue: {
findOne: jest.fn(),
},
},
{
provide: TwentyConfigService,
useValue: {
@ -45,6 +53,9 @@ describe('EmailVerificationTokenService', () => {
appTokenRepository = module.get<Repository<AppToken>>(
getRepositoryToken(AppToken, 'core'),
);
userRepository = module.get<Repository<User>>(
getRepositoryToken(User, 'core'),
);
twentyConfigService = module.get<TwentyConfigService>(TwentyConfigService);
});
@ -92,14 +103,14 @@ describe('EmailVerificationTokenService', () => {
jest
.spyOn(appTokenRepository, 'findOne')
.mockResolvedValue(mockAppToken as AppToken);
jest
.spyOn(appTokenRepository, 'remove')
.mockResolvedValue(mockAppToken as AppToken);
jest.spyOn(userRepository, 'findOne').mockResolvedValue(null);
const result =
await service.validateEmailVerificationTokenOrThrow(plainToken);
const result = await service.validateEmailVerificationTokenOrThrow({
emailVerificationToken: plainToken,
email: 'test@example.com',
});
expect(result).toEqual(mockUser);
expect(result).toEqual(mockAppToken);
expect(appTokenRepository.findOne).toHaveBeenCalledWith({
where: {
value: hashedToken,
@ -107,14 +118,17 @@ describe('EmailVerificationTokenService', () => {
},
relations: ['user'],
});
expect(appTokenRepository.remove).toHaveBeenCalledWith(mockAppToken);
});
it('should throw exception for invalid token', async () => {
jest.spyOn(appTokenRepository, 'findOne').mockResolvedValue(null);
jest.spyOn(userRepository, 'findOne').mockResolvedValue(null);
await expect(
service.validateEmailVerificationTokenOrThrow('invalid-token'),
service.validateEmailVerificationTokenOrThrow({
emailVerificationToken: 'invalid-token',
email: 'test@twenty.com',
}),
).rejects.toThrow(
new EmailVerificationException(
'Invalid email verification token',
@ -123,6 +137,63 @@ describe('EmailVerificationTokenService', () => {
);
});
it('should throw exception for already validated token', async () => {
const mockUser = {
id: 'user-id',
email: 'test@example.com',
isEmailVerified: true,
};
jest.spyOn(appTokenRepository, 'findOne').mockResolvedValue(null);
jest.spyOn(userRepository, 'findOne').mockResolvedValue(mockUser as User);
await expect(
service.validateEmailVerificationTokenOrThrow({
emailVerificationToken: 'invalid-token',
email: 'test@example.com',
}),
).rejects.toThrow(
new EmailVerificationException(
'Email already verified',
EmailVerificationExceptionCode.EMAIL_ALREADY_VERIFIED,
),
);
});
it('should throw exception when email does not match appToken email', async () => {
const mockUser = {
id: 'user-id',
email: 'test@example.com',
isEmailVerified: false,
};
const mockAppToken = {
type: AppTokenType.EmailVerificationToken,
expiresAt: new Date(Date.now() + 86400000), // 24h from now
context: { email: 'other-email@example.com' },
user: {
email: 'other-email@example.com',
},
};
jest
.spyOn(appTokenRepository, 'findOne')
.mockResolvedValue(mockAppToken as AppToken);
jest.spyOn(userRepository, 'findOne').mockResolvedValue(null);
await expect(
service.validateEmailVerificationTokenOrThrow({
emailVerificationToken: 'valid-token',
email: mockUser.email,
}),
).rejects.toThrow(
new EmailVerificationException(
'Email does not match token',
EmailVerificationExceptionCode.INVALID_EMAIL,
),
);
});
it('should throw exception for wrong token type', async () => {
const mockAppToken = {
type: AppTokenType.PasswordResetToken,
@ -132,9 +203,13 @@ describe('EmailVerificationTokenService', () => {
jest
.spyOn(appTokenRepository, 'findOne')
.mockResolvedValue(mockAppToken as AppToken);
jest.spyOn(userRepository, 'findOne').mockResolvedValue(null);
await expect(
service.validateEmailVerificationTokenOrThrow('wrong-type-token'),
service.validateEmailVerificationTokenOrThrow({
emailVerificationToken: 'wrong-type-token',
email: 'test@example.com',
}),
).rejects.toThrow(
new EmailVerificationException(
'Invalid email verification token type',
@ -152,9 +227,13 @@ describe('EmailVerificationTokenService', () => {
jest
.spyOn(appTokenRepository, 'findOne')
.mockResolvedValue(mockAppToken as AppToken);
jest.spyOn(userRepository, 'findOne').mockResolvedValue(null);
await expect(
service.validateEmailVerificationTokenOrThrow('expired-token'),
service.validateEmailVerificationTokenOrThrow({
emailVerificationToken: 'expired-token',
email: 'test@example.com',
}),
).rejects.toThrow(
new EmailVerificationException(
'Email verification token expired',
@ -173,9 +252,13 @@ describe('EmailVerificationTokenService', () => {
jest
.spyOn(appTokenRepository, 'findOne')
.mockResolvedValue(mockAppToken as AppToken);
jest.spyOn(userRepository, 'findOne').mockResolvedValue(null);
await expect(
service.validateEmailVerificationTokenOrThrow('valid-token'),
service.validateEmailVerificationTokenOrThrow({
emailVerificationToken: 'valid-token',
email: 'test@example.com',
}),
).rejects.toThrow(
new EmailVerificationException(
'Email missing in email verification token context',

View File

@ -3,6 +3,7 @@ import { InjectRepository } from '@nestjs/typeorm';
import crypto from 'crypto';
import { isDefined } from 'twenty-shared/utils';
import { addMilliseconds } from 'date-fns';
import ms from 'ms';
import { Repository } from 'typeorm';
@ -17,12 +18,15 @@ import {
EmailVerificationExceptionCode,
} from 'src/engine/core-modules/email-verification/email-verification.exception';
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
import { User } from 'src/engine/core-modules/user/user.entity';
@Injectable()
export class EmailVerificationTokenService {
constructor(
@InjectRepository(AppToken, 'core')
private readonly appTokenRepository: Repository<AppToken>,
@InjectRepository(User, 'core')
private readonly userRepository: Repository<User>,
private readonly twentyConfigService: TwentyConfigService,
) {}
@ -54,7 +58,27 @@ export class EmailVerificationTokenService {
};
}
async validateEmailVerificationTokenOrThrow(emailVerificationToken: string) {
async validateEmailVerificationTokenOrThrow({
emailVerificationToken,
email,
}: {
emailVerificationToken: string;
email: string;
}) {
const user = await this.userRepository.findOne({
where: {
email,
isEmailVerified: true,
},
});
if (isDefined(user)) {
throw new EmailVerificationException(
'Email already verified',
EmailVerificationExceptionCode.EMAIL_ALREADY_VERIFIED,
);
}
const hashedToken = crypto
.createHash('sha256')
.update(emailVerificationToken)
@ -96,8 +120,13 @@ export class EmailVerificationTokenService {
);
}
await this.appTokenRepository.remove(appToken);
if (appToken.context?.email !== email) {
throw new EmailVerificationException(
'Email does not match token',
EmailVerificationExceptionCode.INVALID_EMAIL,
);
}
return appToken.user;
return appToken;
}
}

View File

@ -17,11 +17,16 @@ export class EmailVerificationExceptionFilter implements ExceptionFilter {
case EmailVerificationExceptionCode.INVALID_APP_TOKEN_TYPE:
case EmailVerificationExceptionCode.TOKEN_EXPIRED:
case EmailVerificationExceptionCode.RATE_LIMIT_EXCEEDED:
throw new ForbiddenError(exception.message);
throw new ForbiddenError(exception.message, {
subCode: exception.code,
});
case EmailVerificationExceptionCode.EMAIL_MISSING:
case EmailVerificationExceptionCode.EMAIL_ALREADY_VERIFIED:
case EmailVerificationExceptionCode.INVALID_EMAIL:
case EmailVerificationExceptionCode.EMAIL_VERIFICATION_NOT_REQUIRED:
throw new UserInputError(exception.message);
throw new UserInputError(exception.message, {
subCode: exception.code,
});
default: {
const _exhaustiveCheck: never = exception.code;

View File

@ -14,5 +14,6 @@ export enum EmailVerificationExceptionCode {
TOKEN_EXPIRED = 'TOKEN_EXPIRED',
EMAIL_MISSING = 'EMAIL_MISSING',
EMAIL_ALREADY_VERIFIED = 'EMAIL_ALREADY_VERIFIED',
INVALID_EMAIL = 'INVALID_EMAIL',
RATE_LIMIT_EXCEEDED = 'RATE_LIMIT_EXCEEDED',
}

View File

@ -10,10 +10,11 @@ import { EmailModule } from 'src/engine/core-modules/email/email.module';
import { TwentyConfigModule } from 'src/engine/core-modules/twenty-config/twenty-config.module';
import { UserWorkspaceModule } from 'src/engine/core-modules/user-workspace/user-workspace.module';
import { UserModule } from 'src/engine/core-modules/user/user.module';
import { User } from 'src/engine/core-modules/user/user.entity';
@Module({
imports: [
TypeOrmModule.forFeature([AppToken], 'core'),
TypeOrmModule.forFeature([AppToken, User], 'core'),
EmailModule,
TwentyConfigModule,
DomainManagerModule,

View File

@ -150,8 +150,9 @@ export class PersistedQueryNotSupportedError extends BaseGraphQLError {
}
export class UserInputError extends BaseGraphQLError {
constructor(message: string) {
super(message, ErrorCode.BAD_USER_INPUT);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
constructor(message: string, extensions?: Record<string, any>) {
super(message, ErrorCode.BAD_USER_INPUT, extensions);
Object.defineProperty(this, 'name', { value: 'UserInputError' });
}