From 46b5cdfb4764b05a48774e69b2cc3af3a7c8f566 Mon Sep 17 00:00:00 2001
From: Philip Molares <philip.molares@udo.edu>
Date: Thu, 28 Jan 2021 12:18:20 +0100
Subject: [PATCH] auth: Fix secret length

The former length of 64 bytes (512-bit) is transformed into base64url (a 6-bit code) ~86 characters long. This is too long for bcrypt as it ignores any characters beyond the 72th.
This fix therefore reduces the amount of generated bytes to 54 (as 72*6/8 = 54) characters. This ensures that removing one character from the token the hash won't be the same anymore.

Signed-off-by: Philip Molares <philip.molares@udo.edu>
---
 src/auth/auth.service.spec.ts | 16 ++++++++++++++++
 src/auth/auth.service.ts      | 10 +++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/auth/auth.service.spec.ts b/src/auth/auth.service.spec.ts
index 271973dd5..b5a7e2c1d 100644
--- a/src/auth/auth.service.spec.ts
+++ b/src/auth/auth.service.spec.ts
@@ -13,6 +13,7 @@ import { UsersModule } from '../users/users.module';
 import { Identity } from '../users/identity.entity';
 import { LoggerModule } from '../logger/logger.module';
 import { AuthToken } from './auth-token.entity';
+import { TokenNotValidError } from '../errors/errors';
 
 describe('AuthService', () => {
   let service: AuthService;
@@ -105,6 +106,16 @@ describe('AuthService', () => {
         .checkPassword(testPassword, hash)
         .then((result) => expect(result).toBeTruthy());
     });
+    it('fails, if secret is too short', async () => {
+      const secret = service.BufferToBase64Url(await service.randomString(54));
+      const hash = await service.hashPassword(secret);
+      service
+        .checkPassword(secret, hash)
+        .then((result) => expect(result).toBeTruthy());
+      service
+        .checkPassword(secret.substr(0, secret.length - 1), hash)
+        .then((result) => expect(result).toBeFalsy());
+    });
   });
 
   describe('getTokensByUsername', () => {
@@ -148,6 +159,11 @@ describe('AuthService', () => {
         authTokens: [authToken],
       });
     });
+    it('fails on too long token', () => {
+      expect(
+        service.validateToken(`${authToken.keyId}.${'a'.repeat(73)}`),
+      ).rejects.toBeInstanceOf(TokenNotValidError);
+    });
   });
 
   describe('removeToken', () => {
diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts
index e6b7c3a94..386ee60eb 100644
--- a/src/auth/auth.service.ts
+++ b/src/auth/auth.service.ts
@@ -37,6 +37,14 @@ export class AuthService {
   async validateToken(token: string): Promise<User> {
     try {
       const [keyId, secret] = token.split('.');
+      if (secret.length > 72) {
+        // Only the first 72 characters of the tokens are considered by bcrypt
+        // This should prevent strange corner cases
+        // At the very least it won't hurt us
+        throw new TokenNotValidError(
+          `AuthToken '${secret}' is too long the be a proper token`,
+        );
+      }
       const accessToken = await this.getAuthTokenAndValidate(keyId, secret);
       await this.setLastUsedToken(keyId);
       return this.usersService.getUserByUsername(accessToken.user.userName);
@@ -92,7 +100,7 @@ export class AuthService {
         `User '${user.userName}' has already 200 tokens and can't have anymore`,
       );
     }
-    const secret = this.BufferToBase64Url(await this.randomString(64));
+    const secret = this.BufferToBase64Url(await this.randomString(54));
     const keyId = this.BufferToBase64Url(await this.randomString(8));
     const accessToken = await this.hashPassword(secret);
     let token;