From ab472fbd6212a7f82691f2d26dd4cfc4eda9df4d Mon Sep 17 00:00:00 2001 From: mrkeksz Date: Sun, 3 Dec 2023 21:58:19 +0700 Subject: [PATCH] verification codes --- src/auth/strategies/local.strategy.spec.ts | 41 +++++--- src/auth/strategies/local.strategy.ts | 10 +- .../strategies/refresh-token.strategy.spec.ts | 2 +- src/config/auth/auth.config.constants.ts | 6 ++ src/config/auth/auth.config.service.ts | 12 +++ src/config/auth/auth.config.ts | 13 ++- .../entities/login-attempt.entity.ts | 3 - src/login-attempts/login-attempts.module.ts | 1 - src/users/entities/user.entity.ts | 9 +- src/users/users.service.spec.ts | 16 +-- src/users/users.service.ts | 8 +- .../entities/verification-code.entity.ts | 25 ++++- .../verification-codes.module.ts | 13 +-- .../verification-codes.service.spec.ts | 97 +++---------------- .../verification-codes.service.ts | 56 +++++------ test/auth/gql-login.e2e-spec.ts | 4 +- 16 files changed, 150 insertions(+), 166 deletions(-) diff --git a/src/auth/strategies/local.strategy.spec.ts b/src/auth/strategies/local.strategy.spec.ts index 0f62c22..e07db20 100644 --- a/src/auth/strategies/local.strategy.spec.ts +++ b/src/auth/strategies/local.strategy.spec.ts @@ -60,7 +60,7 @@ describe(LocalStrategy.name, () => { usersService = module.get(UsersService) loginAttemptsService = module.get(LoginAttemptsService) - jest.spyOn(verificationCodesService, 'deleteByID').mockResolvedValueOnce(undefined) + jest.spyOn(verificationCodesService, 'setStatusByID').mockResolvedValueOnce(undefined) }) afterEach(() => { @@ -73,24 +73,29 @@ describe(LocalStrategy.name, () => { id: '1', email: 'test@test.com', refreshToken: null, - isActivated: false, + status: 'unconfirmed', createdAt: currentDate, updatedAt: currentDate, } - const verificationCode = { + const verificationCode: VerificationCode = { id: '1', user, code: 123456, sendingAttempts: 1, + status: 'active', + expirationDate: new Date(currentDate.getTime() + 10 * 60 * 1000), createdAt: currentDate, + updatedAt: currentDate, } const req = {socket: {remoteAddress: '123.123.123.123'}} as Request & { socket: {remoteAddress: string} } it('should return user if code matches', async () => { - jest.spyOn(verificationCodesService, 'findOne').mockResolvedValueOnce(verificationCode) - jest.spyOn(verificationCodesService, 'deleteByID').mockResolvedValueOnce(undefined) + jest + .spyOn(verificationCodesService, 'findOneByUserAndCode') + .mockResolvedValueOnce(verificationCode) + jest.spyOn(verificationCodesService, 'setStatusByID').mockResolvedValueOnce(undefined) jest.spyOn(usersService, 'activate').mockResolvedValueOnce(undefined) jest.spyOn(usersService, 'findOneByEmail').mockResolvedValueOnce(user) jest.spyOn(loginAttemptsService, 'findByIpWhereCreatedAtMoreThen').mockResolvedValueOnce([]) @@ -101,7 +106,7 @@ describe(LocalStrategy.name, () => { }) it('should throw UnauthorizedException if code does not match', async () => { - jest.spyOn(verificationCodesService, 'findOne').mockResolvedValueOnce(null) + jest.spyOn(verificationCodesService, 'findOneByUserAndCode').mockResolvedValueOnce(null) jest.spyOn(usersService, 'findOneByEmail').mockResolvedValueOnce(user) jest.spyOn(loginAttemptsService, 'findByIpWhereCreatedAtMoreThen').mockResolvedValueOnce([]) @@ -111,7 +116,7 @@ describe(LocalStrategy.name, () => { }) it('should throw UnauthorizedException if email does not match', async () => { - jest.spyOn(verificationCodesService, 'findOne').mockResolvedValueOnce(null) + jest.spyOn(verificationCodesService, 'findOneByUserAndCode').mockResolvedValueOnce(null) jest.spyOn(usersService, 'findOneByEmail').mockResolvedValueOnce(null) jest.spyOn(loginAttemptsService, 'findByIpWhereCreatedAtMoreThen').mockResolvedValueOnce([]) @@ -121,7 +126,7 @@ describe(LocalStrategy.name, () => { }) it('should throw UnauthorizedException if code is expired', async () => { - jest.spyOn(verificationCodesService, 'findOne').mockResolvedValueOnce({ + jest.spyOn(verificationCodesService, 'findOneByUserAndCode').mockResolvedValueOnce({ ...verificationCode, createdAt: new Date(currentDate.getTime() - 16 * 60 * 1000), }) @@ -134,7 +139,9 @@ describe(LocalStrategy.name, () => { }) it('should throw UnauthorizedException if too many login attempts', async () => { - jest.spyOn(verificationCodesService, 'findOne').mockResolvedValueOnce(verificationCode) + jest + .spyOn(verificationCodesService, 'findOneByUserAndCode') + .mockResolvedValueOnce(verificationCode) jest.spyOn(usersService, 'findOneByEmail').mockResolvedValueOnce(user) jest.spyOn(loginAttemptsService, 'findByIpWhereCreatedAtMoreThen').mockResolvedValueOnce([ { @@ -180,7 +187,9 @@ describe(LocalStrategy.name, () => { }) it('should activate user if user is not activated', async () => { - jest.spyOn(verificationCodesService, 'findOne').mockResolvedValueOnce(verificationCode) + jest + .spyOn(verificationCodesService, 'findOneByUserAndCode') + .mockResolvedValueOnce(verificationCode) jest.spyOn(usersService, 'findOneByEmail').mockResolvedValueOnce(user) jest.spyOn(loginAttemptsService, 'findByIpWhereCreatedAtMoreThen').mockResolvedValueOnce([]) jest.spyOn(usersService, 'activate').mockResolvedValueOnce(undefined) @@ -191,7 +200,9 @@ describe(LocalStrategy.name, () => { }) it('should create login attempt', async () => { - jest.spyOn(verificationCodesService, 'findOne').mockResolvedValueOnce(verificationCode) + jest + .spyOn(verificationCodesService, 'findOneByUserAndCode') + .mockResolvedValueOnce(verificationCode) jest.spyOn(usersService, 'findOneByEmail').mockResolvedValueOnce(user) jest.spyOn(loginAttemptsService, 'findByIpWhereCreatedAtMoreThen').mockResolvedValueOnce([]) jest.spyOn(loginAttemptsService, 'create').mockResolvedValueOnce({ @@ -211,14 +222,16 @@ describe(LocalStrategy.name, () => { }) }) - it('should delete verification code', async () => { - jest.spyOn(verificationCodesService, 'findOne').mockResolvedValueOnce(verificationCode) + it('should set "used" status for the verification code', async () => { + jest + .spyOn(verificationCodesService, 'findOneByUserAndCode') + .mockResolvedValueOnce(verificationCode) jest.spyOn(usersService, 'findOneByEmail').mockResolvedValueOnce(user) jest.spyOn(loginAttemptsService, 'findByIpWhereCreatedAtMoreThen').mockResolvedValueOnce([]) await localStrategy.validate(req, user.email, 123456) - expect(verificationCodesService.deleteByID).toBeCalledWith(verificationCode.id) + expect(verificationCodesService.setStatusByID).toBeCalledWith(verificationCode.id, 'used') }) }) }) diff --git a/src/auth/strategies/local.strategy.ts b/src/auth/strategies/local.strategy.ts index 397ef1c..76c3483 100644 --- a/src/auth/strategies/local.strategy.ts +++ b/src/auth/strategies/local.strategy.ts @@ -36,9 +36,9 @@ export class LocalStrategy extends PassportStrategy(Strategy, EMAIL_CODE_STRATEG const currentDate = new Date() // TODO: вынести логику loginAttempts в отдельный мидлвар (interceptor) - // TODO: вынести в конфиг. Пользователь может ввести код подтверждения не более 5 раз в 15 минут + // TODO: вынести в конфиг. Пользователь может ввести код подтверждения не более 3 раз в 10 минут const loginAttempts = await this.loginAttemptsService.findByIpWhereCreatedAtMoreThen( - new Date(currentDate.getTime() - 15 * 60 * 1000), + new Date(currentDate.getTime() - 10 * 60 * 1000), clientIp, ) if (loginAttempts.length >= 5) { @@ -50,7 +50,7 @@ export class LocalStrategy extends PassportStrategy(Strategy, EMAIL_CODE_STRATEG throw new UnauthorizedException('Too many login attempts') } - const verificationCode = await this.verificationCodesService.findOne({email}, code) + const verificationCode = await this.verificationCodesService.findOneByUserAndCode({email}, code) // TODO: move this to config. Verification code is valid for 15 minutes const validCreatedDate = new Date(currentDate.getTime() - 15 * 60 * 1000) if (!verificationCode || verificationCode.createdAt < validCreatedDate) { @@ -61,13 +61,13 @@ export class LocalStrategy extends PassportStrategy(Strategy, EMAIL_CODE_STRATEG }) throw new UnauthorizedException('Wrong email or verification code') } - await this.verificationCodesService.deleteByID(verificationCode.id) + await this.verificationCodesService.setStatusByID(verificationCode.id, 'used') await this.loginAttemptsService.create({ user: verificationCode.user, ipAddress: clientIp, isSuccessful: true, }) - if (!verificationCode.user.isActivated) { + if (verificationCode.user.status !== 'active') { await this.usersService.activate(verificationCode.user.id) } return {id: verificationCode.user.id, email: verificationCode.user.email} diff --git a/src/auth/strategies/refresh-token.strategy.spec.ts b/src/auth/strategies/refresh-token.strategy.spec.ts index aa6ba2e..28afe7b 100644 --- a/src/auth/strategies/refresh-token.strategy.spec.ts +++ b/src/auth/strategies/refresh-token.strategy.spec.ts @@ -83,7 +83,7 @@ describe(RefreshTokenStrategy.name, () => { id: '1', email: 'test@test.com', refreshToken: 'refreshToken', - isActivated: true, + status: 'active', createdAt: new Date(), updatedAt: new Date(), } diff --git a/src/config/auth/auth.config.constants.ts b/src/config/auth/auth.config.constants.ts index e927c50..3f604ca 100644 --- a/src/config/auth/auth.config.constants.ts +++ b/src/config/auth/auth.config.constants.ts @@ -35,3 +35,9 @@ export const MAX_SENDING_VERIFICATION_CODE_ATTEMPTS = { name: 'MAX_SENDING_VERIFICATION_CODE_ATTEMPTS', defaultValue: 3, } + +export type VerificationCodeLifetimeSecondsType = number +export const VERIFICATION_CODE_LIFETIME_SECONDS = { + name: 'VERIFICATION_CODE_LIFETIME_SECONDS', + defaultValue: 60 * 10, // 10 minutes +} diff --git a/src/config/auth/auth.config.service.ts b/src/config/auth/auth.config.service.ts index 615c0cb..6394366 100644 --- a/src/config/auth/auth.config.service.ts +++ b/src/config/auth/auth.config.service.ts @@ -11,6 +11,8 @@ import { JwtSecretTokenType, MAX_SENDING_VERIFICATION_CODE_ATTEMPTS, MaxSendingVerificationCodeAttemptsType, + VERIFICATION_CODE_LIFETIME_SECONDS, + VerificationCodeLifetimeSecondsType, } from './auth.config.constants' @Injectable() @@ -42,4 +44,14 @@ export class AuthConfigService { MAX_SENDING_VERIFICATION_CODE_ATTEMPTS.name, ) as MaxSendingVerificationCodeAttemptsType } + + public get verificationCodeLifetimeSeconds(): VerificationCodeLifetimeSecondsType { + return this.configService.get( + VERIFICATION_CODE_LIFETIME_SECONDS.name, + ) as VerificationCodeLifetimeSecondsType + } + + public get verificationCodeLifetimeMilliseconds(): number { + return this.verificationCodeLifetimeSeconds * 1000 + } } diff --git a/src/config/auth/auth.config.ts b/src/config/auth/auth.config.ts index 2066221..7070873 100644 --- a/src/config/auth/auth.config.ts +++ b/src/config/auth/auth.config.ts @@ -1,10 +1,13 @@ -import {HASH_ROUNDS, MAX_SENDING_VERIFICATION_CODE_ATTEMPTS} from './auth.config.constants' -import {NODE_ENV} from '../environment/environment.config.constants' - -const isTest = process.env[NODE_ENV.name] === NODE_ENV.options.TEST +import { + HASH_ROUNDS, + MAX_SENDING_VERIFICATION_CODE_ATTEMPTS, + VERIFICATION_CODE_LIFETIME_SECONDS, +} from './auth.config.constants' +import {EnvironmentConfigService} from '../environment/environment.config.service' export default (): Record => ({ - [HASH_ROUNDS.name]: isTest ? 1 : HASH_ROUNDS.defaultValue, + [HASH_ROUNDS.name]: EnvironmentConfigService.isTest ? 1 : HASH_ROUNDS.defaultValue, [MAX_SENDING_VERIFICATION_CODE_ATTEMPTS.name]: MAX_SENDING_VERIFICATION_CODE_ATTEMPTS.defaultValue, + [VERIFICATION_CODE_LIFETIME_SECONDS.name]: VERIFICATION_CODE_LIFETIME_SECONDS.defaultValue, }) diff --git a/src/login-attempts/entities/login-attempt.entity.ts b/src/login-attempts/entities/login-attempt.entity.ts index 3817e0b..d3efb16 100644 --- a/src/login-attempts/entities/login-attempt.entity.ts +++ b/src/login-attempts/entities/login-attempt.entity.ts @@ -18,6 +18,3 @@ export class LoginAttempt { @CreateDateColumn({type: 'timestamp with time zone'}) createdAt: Date } - -// TODO: автоматическое удаление записей из таблицы login_attempts с истекшим сроком действия -// (createdAt более 30-ти дней) diff --git a/src/login-attempts/login-attempts.module.ts b/src/login-attempts/login-attempts.module.ts index db8485c..03a38ec 100644 --- a/src/login-attempts/login-attempts.module.ts +++ b/src/login-attempts/login-attempts.module.ts @@ -2,7 +2,6 @@ import {Module} from '@nestjs/common' import {LoginAttemptsService} from './login-attempts.service' import {TypeOrmModule} from '@nestjs/typeorm' import {LoginAttempt} from './entities/login-attempt.entity' -// TODO: refactor whole module @Module({ imports: [TypeOrmModule.forFeature([LoginAttempt])], providers: [LoginAttemptsService], diff --git a/src/users/entities/user.entity.ts b/src/users/entities/user.entity.ts index 03f6966..130b67b 100644 --- a/src/users/entities/user.entity.ts +++ b/src/users/entities/user.entity.ts @@ -18,11 +18,12 @@ export class User { refreshToken: string | null @Column({ - type: 'boolean', - default: false, - comment: 'Is the user activated and is the email confirmed?', + comment: 'Status of the user', + type: 'enum', + enum: ['active', 'unconfirmed'], + default: 'unconfirmed', }) - isActivated: boolean + status: 'active' | 'unconfirmed' @CreateDateColumn({type: 'timestamp with time zone'}) createdAt: Date diff --git a/src/users/users.service.spec.ts b/src/users/users.service.spec.ts index f8b1a19..fcf36ba 100644 --- a/src/users/users.service.spec.ts +++ b/src/users/users.service.spec.ts @@ -34,12 +34,12 @@ describe(UsersService.name, () => { it('should return a created user', async () => { const currentDate = new Date() const userInput = {email: 't@t.com'} - const expected = { + const expected: User = { id: '1', refreshToken: null, createdAt: currentDate, updatedAt: currentDate, - isActivated: false, + status: 'active', ...userInput, } @@ -54,11 +54,11 @@ describe(UsersService.name, () => { describe(UsersService.prototype.findOneByID.name, () => { it('should return a user', async () => { const currentDate = new Date() - const expected = { + const expected: User = { id: '1', email: 't@t.com', refreshToken: null, - isActivated: true, + status: 'active', createdAt: currentDate, updatedAt: currentDate, } @@ -85,7 +85,7 @@ describe(UsersService.name, () => { id: '1', email: 't@t.com', refreshToken: null, - isActivated: true, + status: 'active', createdAt: currentDate, updatedAt: currentDate, } @@ -96,6 +96,10 @@ describe(UsersService.name, () => { expect(user).toStrictEqual({ id: expected.id, email: expected.email, + refreshToken: expected.refreshToken, + status: expected.status, + createdAt: expected.createdAt, + updatedAt: expected.updatedAt, }) }) @@ -133,7 +137,7 @@ describe(UsersService.name, () => { const userID = '1' await usersService.activate(userID) - expect(usersService.usersRepository.update).toHaveBeenCalledWith(userID, {isActivated: true}) + expect(usersService.usersRepository.update).toHaveBeenCalledWith(userID, {status: 'active'}) }) }) }) diff --git a/src/users/users.service.ts b/src/users/users.service.ts index a508c7b..a4d68d9 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -15,6 +15,12 @@ export class UsersService { return this.usersRepository.save(newUser) } + public async findOneOrCreateByEmail(email: User['email']): Promise { + const user = await this.findOneByEmail(email) + if (user) return user + return this.createOrUpdate({email}) + } + public findOneByID(id: User['id']): Promise { return this.usersRepository.findOneBy({id}) } @@ -24,7 +30,7 @@ export class UsersService { } public async activate(id: User['id']): Promise { - await this.usersRepository.update(id, {isActivated: true}) + await this.usersRepository.update(id, {status: 'active'}) } public async updateRefreshToken( diff --git a/src/verification-codes/entities/verification-code.entity.ts b/src/verification-codes/entities/verification-code.entity.ts index 6969562..10dd36f 100644 --- a/src/verification-codes/entities/verification-code.entity.ts +++ b/src/verification-codes/entities/verification-code.entity.ts @@ -1,4 +1,11 @@ -import {Column, CreateDateColumn, Entity, ManyToOne, PrimaryGeneratedColumn} from 'typeorm' +import { + Column, + CreateDateColumn, + Entity, + ManyToOne, + PrimaryGeneratedColumn, + UpdateDateColumn, +} from 'typeorm' import {User} from '../../users/entities/user.entity' @Entity() @@ -9,12 +16,26 @@ export class VerificationCode { @ManyToOne(() => User, {onDelete: 'CASCADE', nullable: false}) user: User - @Column({comment: 'The confirmation code received via email', type: 'int'}) + @Column({comment: 'The confirmation code sent via email', type: 'int'}) code: number @Column({default: 0, comment: 'Number of sending attempts', type: 'int'}) sendingAttempts: number + @Column({ + comment: 'Status of the verification code', + type: 'enum', + enum: ['active', 'expired', 'used'], + default: 'active', + }) + status: 'active' | 'expired' | 'used' + + @Column({type: 'timestamp with time zone'}) + expirationDate: Date + @CreateDateColumn({type: 'timestamp with time zone'}) createdAt: Date + + @UpdateDateColumn({type: 'timestamp with time zone'}) + updatedAt: Date } diff --git a/src/verification-codes/verification-codes.module.ts b/src/verification-codes/verification-codes.module.ts index 62897de..debe2d8 100644 --- a/src/verification-codes/verification-codes.module.ts +++ b/src/verification-codes/verification-codes.module.ts @@ -4,18 +4,11 @@ import {TypeOrmModule} from '@nestjs/typeorm' import {VerificationCode} from './entities/verification-code.entity' import {VerificationCodesResolver} from './verification-codes.resolver' import {UsersModule} from '../users/users.module' -import {AuthConfigService} from '../config/auth/auth.config.service' -import {ConfigService} from '@nestjs/config' +import {ConfigModule} from '../config/config.module' @Module({ - imports: [TypeOrmModule.forFeature([VerificationCode]), UsersModule], - providers: [ - VerificationCodesService, - VerificationCodesResolver, - // TODO: collect all config services in one module - ConfigService, - AuthConfigService, - ], + imports: [TypeOrmModule.forFeature([VerificationCode]), UsersModule, ConfigModule], + providers: [VerificationCodesService, VerificationCodesResolver], exports: [VerificationCodesService], }) export class VerificationCodesModule {} diff --git a/src/verification-codes/verification-codes.service.spec.ts b/src/verification-codes/verification-codes.service.spec.ts index c60a345..2c88660 100644 --- a/src/verification-codes/verification-codes.service.spec.ts +++ b/src/verification-codes/verification-codes.service.spec.ts @@ -25,8 +25,8 @@ describe(VerificationCodesService.name, () => { create: jest.fn(), save: jest.fn(), findOne: jest.fn(), - delete: jest.fn(), increment: jest.fn(), + update: jest.fn(), }, }, { @@ -41,82 +41,10 @@ describe(VerificationCodesService.name, () => { }) describe(VerificationCodesService.prototype.sendEmailVerificationCode.name, () => { - it('should create new user if user with given email not found', async () => { - jest.spyOn(usersService, 'findOneByEmail').mockResolvedValueOnce(null) - jest - .spyOn(usersService, 'createOrUpdate') - .mockResolvedValueOnce({id: 'test-id', email: 'test@test.test'} as any) - jest - .spyOn(verificationCodesService.verificationCodeRepository, 'save') - .mockResolvedValueOnce({} as any) - - await verificationCodesService.sendEmailVerificationCode('test@test.test') - - expect(usersService.findOneByEmail).toBeCalledWith('test@test.test') - expect(usersService.createOrUpdate).toBeCalledWith({email: 'test@test.test'}) - expect(verificationCodesService.verificationCodeRepository.save).toBeCalledWith({ - user: {id: 'test-id'}, - code: expect.any(Number), - sendingAttempts: 1, - }) - }) - - it('should not create new user if user with given email found', async () => { - jest - .spyOn(usersService, 'findOneByEmail') - .mockResolvedValueOnce({id: 'test-id', email: 'test@test.test'} as any) - jest.spyOn(usersService, 'createOrUpdate').mockResolvedValueOnce({} as any) - jest - .spyOn(verificationCodesService.verificationCodeRepository, 'save') - .mockResolvedValueOnce({} as any) - - await verificationCodesService.sendEmailVerificationCode('test@test.test') - - expect(usersService.findOneByEmail).toBeCalledWith('test@test.test') - expect(usersService.createOrUpdate).not.toBeCalled() - expect(verificationCodesService.verificationCodeRepository.save).toBeCalledWith({ - user: {id: 'test-id'}, - code: expect.any(Number), - sendingAttempts: 1, - }) - }) - - it('should increment sending attempts if verification code already exists', async () => { - jest - .spyOn(usersService, 'findOneByEmail') - .mockResolvedValueOnce({id: 'test-id', email: 'test@test.test'} as any) - jest.spyOn(usersService, 'createOrUpdate').mockResolvedValueOnce({} as any) - jest - .spyOn(verificationCodesService.verificationCodeRepository, 'save') - .mockResolvedValueOnce({} as any) - jest - .spyOn(verificationCodesService.verificationCodeRepository, 'findOne') - .mockResolvedValueOnce({id: 'test-id'} as any) - - await verificationCodesService.sendEmailVerificationCode('test@test.test') - - expect(usersService.findOneByEmail).toBeCalledWith('test@test.test') - expect(usersService.createOrUpdate).not.toBeCalled() - expect(verificationCodesService.verificationCodeRepository.save).not.toBeCalled() - expect(verificationCodesService.verificationCodeRepository.findOne).toBeCalledWith({ - where: {user: {email: 'test@test.test'}}, - relations: ['user'], - }) - expect(verificationCodesService.verificationCodeRepository.increment).toBeCalledWith( - {id: 'test-id'}, - 'sendingAttempts', - 1, - ) - }) - it('should throw ForbiddenError if verification code already exists and sending attempts >= 3', async () => { jest - .spyOn(usersService, 'findOneByEmail') + .spyOn(usersService, 'findOneOrCreateByEmail') .mockResolvedValueOnce({id: 'test-id', email: 'test@test.test'} as any) - jest.spyOn(usersService, 'createOrUpdate').mockResolvedValueOnce({} as any) - jest - .spyOn(verificationCodesService.verificationCodeRepository, 'save') - .mockResolvedValueOnce({} as any) jest .spyOn(verificationCodesService.verificationCodeRepository, 'findOne') .mockResolvedValueOnce({id: 'test-id', sendingAttempts: 3} as any) @@ -129,29 +57,30 @@ describe(VerificationCodesService.name, () => { }) }) - describe(VerificationCodesService.prototype.findOne.name, () => { - it('should return mail confirmation by user id and code', async () => { + describe(VerificationCodesService.prototype.findOneByUserAndCode.name, () => { + it('should return verification code by user id and code', async () => { jest .spyOn(verificationCodesService.verificationCodeRepository, 'findOne') .mockResolvedValueOnce({id: 'test-id'} as any) - const result = await verificationCodesService.findOne({id: 'test-id'}, 123456) + const result = await verificationCodesService.findOneByUserAndCode({id: 'test-id'}, 123456) expect(result).toEqual({id: 'test-id'}) }) }) - describe(VerificationCodesService.prototype.deleteByID.name, () => { - it('should delete mail confirmation by id', async () => { + describe(VerificationCodesService.prototype.setStatusByID.name, () => { + it('should change status', async () => { jest - .spyOn(verificationCodesService.verificationCodeRepository, 'delete') + .spyOn(verificationCodesService.verificationCodeRepository, 'update') .mockResolvedValueOnce({} as any) - await verificationCodesService.deleteByID('test-id') + await verificationCodesService.setStatusByID('test-id', 'active') - expect(verificationCodesService.verificationCodeRepository.delete).toBeCalledWith({ - id: 'test-id', - }) + expect(verificationCodesService.verificationCodeRepository.update).toBeCalledWith( + {id: 'test-id'}, + {status: 'active'}, + ) }) }) }) diff --git a/src/verification-codes/verification-codes.service.ts b/src/verification-codes/verification-codes.service.ts index 4e73d5e..68e74e0 100644 --- a/src/verification-codes/verification-codes.service.ts +++ b/src/verification-codes/verification-codes.service.ts @@ -1,7 +1,7 @@ import {Injectable, Logger} from '@nestjs/common' import {InjectRepository} from '@nestjs/typeorm' import {User} from '../users/entities/user.entity' -import {Repository} from 'typeorm' +import {MoreThan, Repository} from 'typeorm' import {VerificationCode} from './entities/verification-code.entity' import {UsersService} from '../users/users.service' import {ForbiddenError} from '@nestjs/apollo' @@ -18,32 +18,21 @@ export class VerificationCodesService { private readonly authConfigService: AuthConfigService, ) {} - // TODO: make automatic cleaning of verification_codes table - // from records with expired email verification code. - // Verification code expiration time - 15 minutes. public async sendEmailVerificationCode(email: User['email']): Promise { - const user = - (await this.usersService.findOneByEmail(email)) || - (await this.usersService.createOrUpdate({email})) - const maxSendingAttempts = this.authConfigService.maxSendingVerificationCodeAttempts - let verificationCode = await this.findOneByUserEmail(email) - if (verificationCode && verificationCode.sendingAttempts >= maxSendingAttempts) { + const verificationCode = await this.findOneOrCreateByUserEmail(email) + // TODO: check sending attempts with current ip address (max 3 attempts in 10 minutes from 1 ip address) + // I need to create a new module and entity for this + if (verificationCode.sendingAttempts >= maxSendingAttempts) { throw new ForbiddenError('Too many attempts') - } else if (!verificationCode) { - verificationCode = await this.create(user.id, { - code: this.generateRandomCode(), - sendingAttempts: 1, - }) - } else { - await this.incrementSendingAttempts(verificationCode.id) } + await this.incrementSendingAttempts(verificationCode.id) - this.logger.debug(`Sending the verification code "${verificationCode.code}" to "${user.email}"`) + this.logger.debug(`Sending the verification code "${verificationCode.code}" to "${email}"`) // TODO: send email with verification code } - public async findOne( + public async findOneByUserAndCode( user: {id: User['id']} | {email: User['email']}, code: VerificationCode['code'], ): Promise { @@ -53,13 +42,16 @@ export class VerificationCodesService { }) } - public async deleteByID(id: VerificationCode['id']): Promise { - await this.verificationCodeRepository.delete({id}) + public async setStatusByID( + id: VerificationCode['id'], + status: VerificationCode['status'], + ): Promise { + await this.verificationCodeRepository.update({id}, {status}) } private async findOneByUserEmail(email: User['email']): Promise { return this.verificationCodeRepository.findOne({ - where: {user: {email}}, + where: {user: {email}, status: 'active', expirationDate: MoreThan(new Date())}, relations: ['user'], }) } @@ -72,15 +64,23 @@ export class VerificationCodesService { return Math.floor(Math.random() * (999999 - 100000)) + 100000 } - private create( - userID: User['id'], - {code, sendingAttempts}: Pick, - ): Promise { + private createOne(userID: User['id'], code: VerificationCode['code']): Promise { + const expirationDate = new Date( + Date.now() + this.authConfigService.verificationCodeLifetimeMilliseconds, + ) + const user = {id: userID} const newVerificationCode = this.verificationCodeRepository.create({ - user: {id: userID}, + user, code, - sendingAttempts, + expirationDate, }) return this.verificationCodeRepository.save(newVerificationCode) } + + private async findOneOrCreateByUserEmail(email: User['email']): Promise { + const user = await this.usersService.findOneOrCreateByEmail(email) + const verificationCode = await this.findOneByUserEmail(email) + if (verificationCode) return verificationCode + return this.createOne(user.id, this.generateRandomCode()) + } } diff --git a/test/auth/gql-login.e2e-spec.ts b/test/auth/gql-login.e2e-spec.ts index 774599a..cc06251 100644 --- a/test/auth/gql-login.e2e-spec.ts +++ b/test/auth/gql-login.e2e-spec.ts @@ -31,8 +31,8 @@ describe(`login (GraphQL)`, () => { loginAttemptsService = app.get(LoginAttemptsService) users = await Promise.all([ - usersService.usersRepository.save({email: 'test@test.com', isActivated: true}), - usersService.usersRepository.save({email: 'test2@test2.com', isActivated: false}), + usersService.usersRepository.save({email: 'test@test.com', status: 'active'}), + usersService.usersRepository.save({email: 'test2@test2.com', status: 'unconfirmed'}), ]) verificationCodes = await Promise.all([ verificationCodesService.verificationCodeRepository.save({user: users[0], code: 123456}),