Split methods getAuthTokenAndValidate and createTokenForUser

Signed-off-by: Yannick Bungers <git@innay.de>

src/api/private/tokens/tokens.controller.ts

@@ -53,7 +53,7 @@ export class TokensController {
     @Body() createDto: AuthTokenCreateDto,
     @RequestUser() user: User,
   ): Promise<AuthTokenWithSecretDto> {
-    return await this.authService.createTokenForUser(
+    return await this.authService.addToken(
       user,
       createDto.label,
       createDto.validUntil,

src/auth/auth-token.entity.ts

@@ -48,16 +48,16 @@ export class AuthToken {
     keyId: string,
     user: User,
     label: string,
-    accessToken: string,
+    tokenString: string,
     validUntil: Date,
   ): Omit<AuthToken, 'id' | 'createdAt'> {
-    const newToken = new AuthToken();
+    const token = new AuthToken();
-    newToken.keyId = keyId;
+    token.keyId = keyId;
-    newToken.user = Promise.resolve(user);
+    token.user = Promise.resolve(user);
-    newToken.label = label;
+    token.label = label;
-    newToken.accessTokenHash = accessToken;
+    token.accessTokenHash = tokenString;
-    newToken.validUntil = validUntil;
+    token.validUntil = validUntil;
-    newToken.lastUsedAt = null;
+    token.lastUsedAt = null;
-    return newToken;
+    return token;
   }
 }

src/auth/auth.service.spec.ts

@@ -17,7 +17,6 @@ import { LoggerModule } from '../logger/logger.module';
 import { Session } from '../users/session.entity';
 import { User } from '../users/user.entity';
 import { UsersModule } from '../users/users.module';
-import { hashPassword } from '../utils/password';
 import { AuthToken } from './auth-token.entity';
 import { AuthService } from './auth.service';
 
@@ -96,10 +95,7 @@ describe('AuthService', () => {
         user: Promise.resolve(user),
         accessTokenHash: accessTokenHash,
       });
-      const authTokenFromCall = await service.getAuthTokenAndValidate(
+      const authTokenFromCall = await service.getAuthToken(authToken.keyId);
-        authToken.keyId,
-        token,
-      );
       expect(authTokenFromCall).toEqual({
         ...authToken,
         user: Promise.resolve(user),
@@ -109,34 +105,41 @@ describe('AuthService', () => {
     describe('fails:', () => {
       it('AuthToken could not be found', async () => {
         jest.spyOn(authTokenRepo, 'findOne').mockResolvedValueOnce(null);
-        await expect(
+        await expect(service.getAuthToken(authToken.keyId)).rejects.toThrow(
-          service.getAuthTokenAndValidate(authToken.keyId, token),
+          NotInDBError,
-        ).rejects.toThrow(NotInDBError);
+        );
-      });
-      it('AuthToken has wrong hash', async () => {
-        jest.spyOn(authTokenRepo, 'findOne').mockResolvedValueOnce({
-          ...authToken,
-          user: Promise.resolve(user),
-          accessTokenHash: 'the wrong hash',
-        });
-        await expect(
-          service.getAuthTokenAndValidate(authToken.keyId, token),
-        ).rejects.toThrow(TokenNotValidError);
-      });
-      it('AuthToken has wrong validUntil Date', async () => {
-        const accessTokenHash = await hashPassword(token);
-        jest.spyOn(authTokenRepo, 'findOne').mockResolvedValueOnce({
-          ...authToken,
-          user: Promise.resolve(user),
-          accessTokenHash: accessTokenHash,
-          validUntil: new Date(1549312452000),
-        });
-        await expect(
-          service.getAuthTokenAndValidate(authToken.keyId, token),
-        ).rejects.toThrow(TokenNotValidError);
       });
     });
   });
+  describe('checkToken', () => {
+    it('works', () => {
+      const [accessToken, secret] = service.createToken(
+        user,
+        'TestToken',
+        undefined,
+      );
+
+      expect(() =>
+        service.checkToken(secret, accessToken as AuthToken),
+      ).not.toThrow();
+    });
+    it('AuthToken has wrong hash', () => {
+      const [accessToken] = service.createToken(user, 'TestToken', undefined);
+      expect(() =>
+        service.checkToken('secret', accessToken as AuthToken),
+      ).toThrow(TokenNotValidError);
+    });
+    it('AuthToken has wrong validUntil Date', () => {
+      const [accessToken, secret] = service.createToken(
+        user,
+        'Test',
+        1549312452000,
+      );
+      expect(() =>
+        service.checkToken(secret, accessToken as AuthToken),
+      ).toThrow(TokenNotValidError);
+    });
+  });
 
   describe('setLastUsedToken', () => {
     it('works', async () => {
@@ -233,7 +236,7 @@ describe('AuthService', () => {
     });
   });
 
-  describe('createTokenForUser', () => {
+  describe('addToken', () => {
     describe('works', () => {
       const identifier = 'testIdentifier';
       it('with validUntil 0', async () => {
@@ -246,7 +249,7 @@ describe('AuthService', () => {
               return authTokenSaved;
             },
           );
-        const token = await service.createTokenForUser(user, identifier, 0);
+        const token = await service.addToken(user, identifier, 0);
         expect(token.label).toEqual(identifier);
         expect(
           token.validUntil.getTime() -
@@ -266,11 +269,7 @@ describe('AuthService', () => {
             },
           );
         const validUntil = new Date().getTime() + 30000;
-        const token = await service.createTokenForUser(
+        const token = await service.addToken(user, identifier, validUntil);
-          user,
-          identifier,
-          validUntil,
-        );
         expect(token.label).toEqual(identifier);
         expect(token.validUntil.getTime()).toEqual(validUntil);
         expect(token.lastUsedAt).toBeNull();

src/auth/auth.service.ts

@@ -33,39 +33,29 @@ export class AuthService {
     this.logger.setContext(AuthService.name);
   }
 
-  async validateToken(token: string): Promise<User> {
+  async validateToken(tokenString: string): Promise<User> {
-    const [keyId, secret] = token.split('.');
+    const [keyId, secret] = tokenString.split('.');
     if (!secret) {
       throw new TokenNotValidError('Invalid AuthToken format');
     }
     if (secret.length != 86) {
       // We always expect 86 characters, as the secret is generated with 64 bytes
       // and then converted to a base64url string
-      throw new TokenNotValidError(`AuthToken '${token}' has incorrect length`);
+      throw new TokenNotValidError(
+        `AuthToken '${tokenString}' has incorrect length`,
+      );
     }
-    const accessToken = await this.getAuthTokenAndValidate(keyId, secret);
+    const token = await this.getAuthToken(keyId);
+    this.checkToken(secret, token);
     await this.setLastUsedToken(keyId);
-    return await this.usersService.getUserByUsername(
+    return await token.user;
-      (
-        await accessToken.user
-      ).username,
-    );
   }
 
-  async createTokenForUser(
+  createToken(
     user: User,
     identifier: string,
     validUntil: TimestampMillis | undefined,
-  ): Promise<AuthTokenWithSecretDto> {
+  ): [Omit<AuthToken, 'id' | 'createdAt'>, string] {
-    user.authTokens = this.getTokensByUser(user);
-
-    if ((await user.authTokens).length >= 200) {
-      // This is a very high ceiling unlikely to hinder legitimate usage,
-      // but should prevent possible attack vectors
-      throw new TooManyTokensError(
-        `User '${user.username}' has already 200 tokens and can't have anymore`,
-      );
-    }
     const secret = bufferToBase64Url(randomBytes(64));
     const keyId = bufferToBase64Url(randomBytes(8));
     // More about the choice of SHA-512 in the dev docs
@@ -94,39 +84,60 @@ export class AuthService {
         new Date(validUntil),
       );
     }
+    return [token, secret];
+  }
+
+  async addToken(
+    user: User,
+    identifier: string,
+    validUntil: TimestampMillis | undefined,
+  ): Promise<AuthTokenWithSecretDto> {
+    user.authTokens = this.getTokensByUser(user);
+
+    if ((await user.authTokens).length >= 200) {
+      // This is a very high ceiling unlikely to hinder legitimate usage,
+      // but should prevent possible attack vectors
+      throw new TooManyTokensError(
+        `User '${user.username}' has already 200 tokens and can't have anymore`,
+      );
+    }
+    const [token, secret] = this.createToken(user, identifier, validUntil);
     const createdToken = (await this.authTokenRepository.save(
       token,
     )) as AuthToken;
-    return this.toAuthTokenWithSecretDto(createdToken, `${keyId}.${secret}`);
+    return this.toAuthTokenWithSecretDto(
+      createdToken,
+      `${createdToken.keyId}.${secret}`,
+    );
   }
 
   async setLastUsedToken(keyId: string): Promise<void> {
-    const accessToken = await this.authTokenRepository.findOne({
+    const token = await this.authTokenRepository.findOne({
       where: { keyId: keyId },
     });
-    if (accessToken === null) {
+    if (token === null) {
       throw new NotInDBError(`AuthToken for key '${keyId}' not found`);
     }
-    accessToken.lastUsedAt = new Date();
+    token.lastUsedAt = new Date();
-    await this.authTokenRepository.save(accessToken);
+    await this.authTokenRepository.save(token);
   }
 
-  async getAuthTokenAndValidate(
+  async getAuthToken(keyId: string): Promise<AuthToken> {
-    keyId: string,
+    const token = await this.authTokenRepository.findOne({
-    token: string,
-  ): Promise<AuthToken> {
-    const accessToken = await this.authTokenRepository.findOne({
       where: { keyId: keyId },
       relations: ['user'],
     });
-    if (accessToken === null) {
+    if (token === null) {
-      throw new NotInDBError(`AuthToken '${token}' not found`);
+      throw new NotInDBError(`AuthToken '${keyId}' not found`);
     }
-    // Hash the user-provided token
+    return token;
+  }
+
+  checkToken(secret: string, token: AuthToken): void {
     const userHash = Buffer.from(
-      crypto.createHash('sha512').update(token).digest('hex'),
+      crypto.createHash('sha512').update(secret).digest('hex'),
     );
-    const dbHash = Buffer.from(accessToken.accessTokenHash);
+    const dbHash = Buffer.from(token.accessTokenHash);
     if (
       // Normally, both hashes have the same length, as they are both SHA512
       // This is only defense-in-depth, as timingSafeEqual throws if the buffers are not of the same length
@@ -134,18 +145,18 @@ export class AuthService {
       !crypto.timingSafeEqual(userHash, dbHash)
     ) {
       // hashes are not the same
-      throw new TokenNotValidError(`AuthToken '${token}' is not valid.`);
-    }
-    if (
-      accessToken.validUntil &&
-      accessToken.validUntil.getTime() < new Date().getTime()
-    ) {
-      // tokens validUntil Date lies in the past
       throw new TokenNotValidError(
-        `AuthToken '${token}' is not valid since ${accessToken.validUntil.toISOString()}.`,
+        `Secret does not match Token ${token.label}.`,
+      );
+    }
+    if (token.validUntil && token.validUntil.getTime() < new Date().getTime()) {
+      // tokens validUntil Date lies in the past
+      throw new TokenNotValidError(
+        `AuthToken '${
+          token.label
+        }' is not valid since ${token.validUntil.toISOString()}.`,
       );
     }
-    return accessToken;
   }
 
   async getTokensByUser(user: User): Promise<AuthToken[]> {

test/test-setup.ts

@@ -343,7 +343,7 @@ export class TestSetupBuilder {
       // create auth tokens
       this.testSetup.authTokens = await Promise.all(
         this.testSetup.users.map(async (user) => {
-          return await this.testSetup.authService.createTokenForUser(
+          return await this.testSetup.authService.addToken(
             user,
             'test',
             new Date().getTime() + 60 * 60 * 1000,