Skip to content

Commit

Permalink
refactor(auth): verify the credentials
Browse files Browse the repository at this point in the history
Extract validation logic from custom validation to a service
Update tests
  • Loading branch information
leosuncin committed Sep 19, 2022
1 parent 072ac1f commit b3c5d13
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 76 deletions.
14 changes: 4 additions & 10 deletions src/auth/dto/login-user.dto.spec.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
import { faker } from '@faker-js/faker';
import { Test } from '@nestjs/testing';
import { getRepositoryToken } from '@nestjs/typeorm';
import { plainToInstance } from 'class-transformer';
import { useContainer, validate } from 'class-validator';
import fc from 'fast-check';
import { createMock } from 'ts-auto-mock';
import type { Repository } from 'typeorm';

import { LoginUser } from '~auth/dto/login-user.dto';
import { User } from '~auth/entities/user.entity';
import { loginUserFactory } from '~auth/factories/login-user.factory';
import { login } from '~auth/fixtures/credentials';
import { AuthenticationService } from '~auth/services/authentication.service';
import { ValidateCredentialConstraint } from '~auth/validators/validate-credential.validator';

describe('Login user validations', () => {
Expand All @@ -19,13 +17,9 @@ describe('Login user validations', () => {
providers: [ValidateCredentialConstraint],
})
.useMocker((token) => {
if (token === getRepositoryToken(User)) {
return createMock<Repository<User>>({
findOne: jest.fn().mockResolvedValue(
createMock<User>({
checkPassword: jest.fn().mockResolvedValue(true),
}),
),
if (token === AuthenticationService) {
return createMock<AuthenticationService>({
verifyCredentials: jest.fn().mockResolvedValue(true),
});
}

Expand Down
16 changes: 1 addition & 15 deletions src/auth/dto/update-user.dto.spec.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
import { faker } from '@faker-js/faker';
import { HttpStatus } from '@nestjs/common';
import { Test } from '@nestjs/testing';
import { getRepositoryToken } from '@nestjs/typeorm';
import { plainToInstance } from 'class-transformer';
import { useContainer, validate } from 'class-validator';
import fc from 'fast-check';
import nock, { cleanAll, enableNetConnect } from 'nock';
import { createMock } from 'ts-auto-mock';
import type { Repository } from 'typeorm';

import { UpdateUser } from '~auth/dto/update-user.dto';
import { User } from '~auth/entities/user.entity';
import { updateUserFactory } from '~auth/factories/update-user.factory';
import { PASSWORD_HASHES } from '~auth/fixtures/password-hashes';
import { john as user } from '~auth/fixtures/users';
Expand All @@ -19,20 +16,15 @@ import { IsAlreadyRegisterConstraint } from '~auth/validators/is-already-registe
import { ValidateCredentialConstraint } from '~auth/validators/validate-credential.validator';

describe('Update user validations', () => {
let mockUserRepository: jest.Mocked<Repository<User>>;

beforeAll(async () => {
const module = await Test.createTestingModule({
providers: [IsAlreadyRegisterConstraint, ValidateCredentialConstraint],
})
.useMocker((token) => {
if (token === getRepositoryToken(User)) {
return createMock<Repository<User>>();
}

if (token === AuthenticationService) {
return createMock<AuthenticationService>({
isRegistered: jest.fn().mockResolvedValue(false),
verifyCredentials: jest.fn().mockResolvedValue(true),
});
}

Expand All @@ -47,8 +39,6 @@ describe('Update user validations', () => {
.replyDate()
.get(/range\/\w{5}/)
.reply(HttpStatus.OK, PASSWORD_HASHES);

mockUserRepository = module.get(getRepositoryToken(User));
});

afterAll(() => {
Expand All @@ -57,10 +47,6 @@ describe('Update user validations', () => {
});

it('should pass with valid data', async () => {
mockUserRepository.findOne.mockResolvedValue(
createMock<User>({ checkPassword: jest.fn().mockResolvedValue(true) }),
);

await fc.assert(
fc.asyncProperty(
fc
Expand Down
27 changes: 27 additions & 0 deletions src/auth/services/authentication.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,31 @@ describe('AuthenticationService', () => {
expect(mockedUserRepository.countBy).toHaveBeenCalledWith(where);
},
);

it.each([
[credentials, 'username', true],
[credentials, 'password', true],
[user, 'username', true],
[user, 'password', true],
[{ id: user.id, password: 'password' }, 'password', false],
[{ username: '' }, 'username', false],
])(
'should verify the credentials %j',
async (credentials, property, expected) => {
mockedUserRepository.findOneBy.mockResolvedValueOnce(
// eslint-disable-next-line unicorn/no-null
Object.keys(credentials).length > 1 ? user : null,
);

await expect(
// @ts-expect-error mocked value
service.verifyCredentials(credentials, property),
).resolves.toBe(expected);
expect(mockedUserRepository.findOneBy).toHaveBeenCalledWith(
'id' in credentials
? { id: credentials.id }
: { username: credentials.username },
);
},
);
});
21 changes: 21 additions & 0 deletions src/auth/services/authentication.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,25 @@ export class AuthenticationService {

return count >= 1;
}

async verifyCredentials(
credentials: Required<LoginUser | UpdateUser>,
property: string,
): Promise<boolean> {
const where: FindOptionsWhere<User> = {};

if ('id' in credentials) {
where['id'] = credentials.id;
} else {
where['username'] = credentials.username;
}

const user = await this.userRepository.findOneBy(where);

if (!user) return false;

if (property !== 'password') return true;

return user.checkPassword(credentials.password);
}
}
56 changes: 28 additions & 28 deletions src/auth/validators/validate-credential.validator.spec.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import { Test } from '@nestjs/testing';
import { getRepositoryToken } from '@nestjs/typeorm';
import { useContainer, validate } from 'class-validator';
import { createMock } from 'ts-auto-mock';
import type { Repository } from 'typeorm';

import { User } from '~auth/entities/user.entity';
import { login as credentials } from '~auth/fixtures/credentials';
import { john as user } from '~auth/fixtures/users';
import { AuthenticationService } from '~auth/services/authentication.service';
import {
ValidateCredential,
ValidateCredentialConstraint,
Expand Down Expand Up @@ -38,38 +36,46 @@ class Update {
}

describe('ValidateCredential', () => {
let mockedUserRepository: jest.Mocked<Repository<User>>;
let mockedAuthenticationService: jest.Mocked<AuthenticationService>;

beforeEach(async () => {
const module = await Test.createTestingModule({
providers: [ValidateCredentialConstraint],
})
.useMocker((token) => {
if (token === getRepositoryToken(User)) {
return createMock<Repository<User>>();
if (token === AuthenticationService) {
return createMock<AuthenticationService>({
verifyCredentials: jest
.fn()
.mockImplementation(({ password, username, id }, property) => {
if (id ? id !== user.id : username !== credentials.username)
return false;
if (property !== 'password') return true;
return password === credentials.password;
}),
});
}

return;
})
.compile();

useContainer(module, { fallbackOnErrors: true });
mockedUserRepository = module.get(getRepositoryToken(User));
mockedAuthenticationService = module.get(AuthenticationService);
});

it('should pass with the correct credentials', async () => {
const dto = new DTO(user.username, user.password);
mockedUserRepository.findOne.mockResolvedValue(user);

const errors = await validate(dto);

expect(errors).toHaveLength(0);
expect(mockedUserRepository.findOne).toHaveBeenCalledWith({
where: {
username: dto.username,
},
cache: true,
});
expect(
mockedAuthenticationService.verifyCredentials,
).toHaveBeenNthCalledWith(1, dto, 'username');
expect(
mockedAuthenticationService.verifyCredentials,
).toHaveBeenNthCalledWith(2, dto, 'password');
});

it.each([
Expand All @@ -91,31 +97,25 @@ describe('ValidateCredential', () => {

it('should pass with the correct current password', async () => {
const dto = new Update(user.password, user.id);
mockedUserRepository.findOne.mockResolvedValue(user);

const errors = await validate(dto);

expect(errors).toHaveLength(0);
expect(mockedUserRepository.findOne).toHaveBeenCalledWith({
where: {
id: dto.id,
},
cache: true,
});
expect(mockedAuthenticationService.verifyCredentials).toHaveBeenCalledWith(
dto,
'password',
);
});

it('should fail with the incorrect current password', async () => {
const dto = new Update('ji32k7au4a83', user.id);
mockedUserRepository.findOne.mockResolvedValue(user);

const errors = await validate(dto);

expect(errors).toHaveLength(1);
expect(mockedUserRepository.findOne).toHaveBeenCalledWith({
where: {
id: dto.id,
},
cache: true,
});
expect(mockedAuthenticationService.verifyCredentials).toHaveBeenCalledWith(
dto,
'password',
);
});
});
27 changes: 4 additions & 23 deletions src/auth/validators/validate-credential.validator.ts
Original file line number Diff line number Diff line change
@@ -1,44 +1,25 @@
import { Injectable } from '@nestjs/common';
import { InjectRepository } from '@nestjs/typeorm';
import {
registerDecorator,
ValidationArguments,
ValidationOptions,
ValidatorConstraint,
ValidatorConstraintInterface,
} from 'class-validator';
import type { Repository } from 'typeorm';

import { User } from '~auth/entities/user.entity';
import { AuthenticationService } from '~auth/services/authentication.service';

@Injectable()
@ValidatorConstraint({ name: 'credential', async: true })
export class ValidateCredentialConstraint
implements ValidatorConstraintInterface
{
constructor(
@InjectRepository(User)
private readonly userRepository: Repository<User>,
) {}
constructor(private readonly authenticationService: AuthenticationService) {}

async validate(
value: string,
{ object, property }: ValidationArguments,
): Promise<boolean> {
validate(_: unknown, { object, property }: ValidationArguments) {
if (!this.hasCredentials(object)) return false;

const user = await this.userRepository.findOne({
where: {
...(object.id ? { id: object.id } : { username: object.username }),
},
cache: true,
});

if (!user) return false;

if (property !== 'password') return true;

return user.checkPassword(value);
return this.authenticationService.verifyCredentials(object, property);
}

defaultMessage(): string {
Expand Down

0 comments on commit b3c5d13

Please sign in to comment.