refactor(security): use bcrypt the right way,

not hashing refresh token

src/modules/auth/auth.service.ts

@@ -61,10 +61,7 @@ export class AuthService {
 
     const user = await this.userService.findOne(sub);
 
-    const isMatchedRefreshToken = Hash.compare(
-      dto.refreshToken,
-      user.refreshToken,
-    );
+    const isMatchedRefreshToken = dto.refreshToken === user.refreshToken;
     if (!isMatchedRefreshToken) {
       throw new UnauthorizedException('Invalid refresh token');
     }

src/modules/auth/dto/login.dto.ts

@@ -1,5 +1,5 @@
 import { ApiProperty } from '@nestjs/swagger';
-import { IsEmail, IsNotEmpty, MinLength } from 'class-validator';
+import { IsEmail, IsNotEmpty, MaxLength, MinLength } from 'class-validator';
 
 export class LoginDto {
   @ApiProperty({
@@ -10,8 +10,11 @@ export class LoginDto {
 
   @ApiProperty({
     required: true,
+    minLength: 5,
+    maxLength: 72,
   })
   @IsNotEmpty()
   @MinLength(5)
+  @MaxLength(72)
   password: string;
 }

src/modules/auth/dto/signup.dto.ts

@@ -1,5 +1,5 @@
 import { ApiProperty } from '@nestjs/swagger';
-import { IsEmail, IsNotEmpty, MinLength } from 'class-validator';
+import { IsEmail, IsNotEmpty, MaxLength, MinLength } from 'class-validator';
 import { SameAs } from '@modules/common/validator/same-as.validator';
 
 export class SignupDto {
@@ -23,9 +23,12 @@ export class SignupDto {
 
   @ApiProperty({
     required: true,
+    minLength: 5,
+    maxLength: 72,
   })
   @IsNotEmpty()
   @MinLength(5)
+  @MaxLength(72)
   password: string;
 
   @ApiProperty({ required: true })

src/modules/common/exception-filter/all-exceptions.filter.ts

@@ -50,7 +50,7 @@ export class AllExceptionsFilter implements ExceptionFilter {
         error: 'Record Not Found Error',
       };
     } else {
-      this.logger.error(exception);
+      this.logger.error((exception as Error).stack);
       statusCode = HttpStatus.INTERNAL_SERVER_ERROR;
       responseBody = {
         statusCode,

src/modules/user/user.entity.ts

@@ -46,8 +46,6 @@ export class User {
 
   @Column({
     name: 'refresh_token',
-    length: 255,
-    transformer: new PasswordTransformer(),
     nullable: true,
   })
   @Exclude()

src/utils/hash.util.spec.ts

@@ -0,0 +1,22 @@
+import { Hash } from './hash.util';
+
+describe('Hash.make', () => {
+  test('panics if a password longer than 72 is given', () => {
+    const tooLongPassword =
+      "abcdefghijklmnopqrstuvwxyz1234567890!@#$%^&*(),.:;'-+_abcdefghijklmnopqrstuvwxyz1234567890abcdefghijklmnopqrstuvwxyz1234567890";
+
+    expect(() => Hash.make(tooLongPassword)).toThrow();
+  });
+
+  test('panics if an empty password is given', () => {
+    expect(() => Hash.make('')).toThrow();
+  });
+});
+
+describe('Hash.compare', () => {
+  test('panics if password or hash is an empty string', () => {
+    const anyHash = Hash.make('any string');
+    expect(() => Hash.compare('', anyHash)).toThrow();
+    expect(() => Hash.compare('any string', '')).toThrow();
+  });
+});

src/utils/hash.util.ts

@@ -2,11 +2,24 @@ import * as bcrypt from 'bcrypt';
 
 export class Hash {
   static make(plainText: string) {
+    // Per bcrypt implementation, only the first 72 **bytes** of a string are used. Any extra bytes are ignored when matching passwords.
+    if (plainText === '' || plainText.length > 72) {
+      throw new Error('Password can not be empty or longer than 72');
+    }
+
     const salt = bcrypt.genSaltSync();
+
     return bcrypt.hashSync(plainText, salt);
   }
 
   static compare(plainText: string, hash: string) {
+    if (plainText.length === 0) {
+      throw new Error('Password can not be empty');
+    }
+    if (hash.length === 0) {
+      throw new Error('Hash can not be empty');
+    }
+
     return bcrypt.compareSync(plainText, hash);
   }
 }