Skip to content

Commit

Permalink
fix(auth): amend custom validators
Browse files Browse the repository at this point in the history
Verify whether the values are valid before checking against the DB
  • Loading branch information
leosuncin committed Sep 19, 2022
1 parent b3c5d13 commit 495048c
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 28 deletions.
4 changes: 1 addition & 3 deletions .pactum/snapshots/login-errors.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,11 @@
"password must be longer than or equal to 8 characters",
"password should not be empty",
"password must be a string",
"password is incorrect",
"username should not be null or undefined",
"username must match /^[\\w.-]+$/i regular expression",
"username must be shorter than or equal to 30 characters",
"username should not be empty",
"username must be a string",
"username is incorrect"
"username must be a string"
],
"error": "Unprocessable Entity"
}
2 changes: 0 additions & 2 deletions src/auth/dto/__snapshots__/login-user.dto.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ exports[`Login user validations should require all properties 1`] = `
ValidationError {
"children": [],
"constraints": {
"credential": "password is incorrect",
"isDefined": "password should not be null or undefined",
"isNotEmpty": "password should not be empty",
"isString": "password must be a string",
Expand All @@ -19,7 +18,6 @@ exports[`Login user validations should require all properties 1`] = `
ValidationError {
"children": [],
"constraints": {
"credential": "username is incorrect",
"isDefined": "username should not be null or undefined",
"isNotEmpty": "username should not be empty",
"isString": "username must be a string",
Expand Down
3 changes: 3 additions & 0 deletions src/auth/validators/is-already-register.validator.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Injectable } from '@nestjs/common';
import {
isEmpty,
registerDecorator,
ValidationArguments,
ValidationOptions,
Expand All @@ -17,6 +18,8 @@ export class IsAlreadyRegisterConstraint
constructor(private readonly authenticationService: AuthenticationService) {}

async validate(value: string, { object, property }: ValidationArguments) {
if (isEmpty(value)) return true;

const userExist = await this.authenticationService.isRegistered({
[property]: value,
// @ts-expect-error object has an id property and it's defined
Expand Down
36 changes: 17 additions & 19 deletions src/auth/validators/validate-credential.validator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,29 +64,27 @@ describe('ValidateCredential', () => {
mockedAuthenticationService = module.get(AuthenticationService);
});

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

const errors = await validate(dto);

expect(errors).toHaveLength(0);
expect(
mockedAuthenticationService.verifyCredentials,
).toHaveBeenNthCalledWith(1, dto, 'username');
expect(
mockedAuthenticationService.verifyCredentials,
).toHaveBeenNthCalledWith(2, dto, 'password');
});
it.each([credentials, { id: user.id, password: user.password }])(
'should pass with the correct credentials',
async (data) => {
const dto = DTO.from(data);

const errors = await validate(dto);

expect(errors).toHaveLength(0);
expect(
mockedAuthenticationService.verifyCredentials,
).toHaveBeenNthCalledWith(1, dto, 'username');
expect(
mockedAuthenticationService.verifyCredentials,
).toHaveBeenNthCalledWith(2, dto, 'password');
},
);

it.each([
{},
{ username: undefined, password: undefined },
{ username: 42, password: false },
{ username: 'jane-doe', password: credentials.password },
{ username: credentials.username, password: 'MiContraseña' },
{ password: credentials.password },
{ username: credentials.username },
{ usuario: credentials.username, contrasena: credentials.password },
{ id: user.id, password: 'Anim ex fugiat sunt ut culpa.' },
])('should fail with invalid credentials: %o', async (data) => {
const dto = DTO.from(data);

Expand Down
23 changes: 19 additions & 4 deletions src/auth/validators/validate-credential.validator.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { Injectable } from '@nestjs/common';
import {
isString,
isUUID,
matches,
maxLength,
minLength,
registerDecorator,
ValidationArguments,
ValidationOptions,
Expand All @@ -17,7 +22,7 @@ export class ValidateCredentialConstraint
constructor(private readonly authenticationService: AuthenticationService) {}

validate(_: unknown, { object, property }: ValidationArguments) {
if (!this.hasCredentials(object)) return false;
if (!this.hasCredentials(object)) return true;

return this.authenticationService.verifyCredentials(object, property);
}
Expand All @@ -32,9 +37,19 @@ export class ValidateCredentialConstraint
id?: string;
[key: string]: unknown;
} {
return (
object.hasOwnProperty('username') || object.hasOwnProperty('password')
);
const { id, password, username } = object as Record<string, unknown>;
const isValidId = isString(id) && isUUID(id, '4');
const isValidPassword =
isString(password) && minLength(password, 8) && maxLength(password, 30);
const isValidUsername =
isString(username) &&
maxLength(username, 30) &&
matches(username, /^[\w.-]+$/i);

if (isValidId && isValidPassword) return true;
if (isValidPassword && isValidUsername) return true;

return false;
}
}

Expand Down

0 comments on commit 495048c

Please sign in to comment.