Skip to content

Commit

Permalink
verification codes
Browse files Browse the repository at this point in the history
  • Loading branch information
mrkeksz committed Dec 3, 2023
1 parent baff9a6 commit ab472fb
Show file tree
Hide file tree
Showing 16 changed files with 150 additions and 166 deletions.
41 changes: 27 additions & 14 deletions src/auth/strategies/local.strategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand All @@ -73,24 +73,29 @@ describe(LocalStrategy.name, () => {
id: '1',
email: '[email protected]',
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([])
Expand All @@ -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([])

Expand All @@ -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([])

Expand All @@ -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),
})
Expand All @@ -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([
{
Expand Down Expand Up @@ -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)
Expand All @@ -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({
Expand All @@ -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')
})
})
})
10 changes: 5 additions & 5 deletions src/auth/strategies/local.strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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}
Expand Down
2 changes: 1 addition & 1 deletion src/auth/strategies/refresh-token.strategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe(RefreshTokenStrategy.name, () => {
id: '1',
email: '[email protected]',
refreshToken: 'refreshToken',
isActivated: true,
status: 'active',
createdAt: new Date(),
updatedAt: new Date(),
}
Expand Down
6 changes: 6 additions & 0 deletions src/config/auth/auth.config.constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
12 changes: 12 additions & 0 deletions src/config/auth/auth.config.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {
JwtSecretTokenType,
MAX_SENDING_VERIFICATION_CODE_ATTEMPTS,
MaxSendingVerificationCodeAttemptsType,
VERIFICATION_CODE_LIFETIME_SECONDS,
VerificationCodeLifetimeSecondsType,
} from './auth.config.constants'

@Injectable()
Expand Down Expand Up @@ -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
}
}
13 changes: 8 additions & 5 deletions src/config/auth/auth.config.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown> => ({
[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,
})
3 changes: 0 additions & 3 deletions src/login-attempts/entities/login-attempt.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,3 @@ export class LoginAttempt {
@CreateDateColumn({type: 'timestamp with time zone'})
createdAt: Date
}

// TODO: автоматическое удаление записей из таблицы login_attempts с истекшим сроком действия
// (createdAt более 30-ти дней)
1 change: 0 additions & 1 deletion src/login-attempts/login-attempts.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
9 changes: 5 additions & 4 deletions src/users/entities/user.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 10 additions & 6 deletions src/users/users.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ describe(UsersService.name, () => {
it('should return a created user', async () => {
const currentDate = new Date()
const userInput = {email: '[email protected]'}
const expected = {
const expected: User = {
id: '1',
refreshToken: null,
createdAt: currentDate,
updatedAt: currentDate,
isActivated: false,
status: 'active',
...userInput,
}

Expand All @@ -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: '[email protected]',
refreshToken: null,
isActivated: true,
status: 'active',
createdAt: currentDate,
updatedAt: currentDate,
}
Expand All @@ -85,7 +85,7 @@ describe(UsersService.name, () => {
id: '1',
email: '[email protected]',
refreshToken: null,
isActivated: true,
status: 'active',
createdAt: currentDate,
updatedAt: currentDate,
}
Expand All @@ -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,
})
})

Expand Down Expand Up @@ -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'})
})
})
})
8 changes: 7 additions & 1 deletion src/users/users.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ export class UsersService {
return this.usersRepository.save(newUser)
}

public async findOneOrCreateByEmail(email: User['email']): Promise<User> {
const user = await this.findOneByEmail(email)
if (user) return user
return this.createOrUpdate({email})
}

public findOneByID(id: User['id']): Promise<User | null> {
return this.usersRepository.findOneBy({id})
}
Expand All @@ -24,7 +30,7 @@ export class UsersService {
}

public async activate(id: User['id']): Promise<void> {
await this.usersRepository.update(id, {isActivated: true})
await this.usersRepository.update(id, {status: 'active'})
}

public async updateRefreshToken(
Expand Down
25 changes: 23 additions & 2 deletions src/verification-codes/entities/verification-code.entity.ts
Original file line number Diff line number Diff line change
@@ -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()
Expand All @@ -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
}
13 changes: 3 additions & 10 deletions src/verification-codes/verification-codes.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Loading

0 comments on commit ab472fb

Please sign in to comment.