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..9662f4c1e 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import { Injectable, UnauthorizedException } from '@nestjs/common'; +import { Injectable } from '@nestjs/common'; import { UsersService } from '../users/users.service'; import { User } from '../users/user.entity'; import { AuthToken } from './auth-token.entity'; @@ -35,20 +35,21 @@ export class AuthService { } async validateToken(token: string): Promise { - try { - const [keyId, secret] = token.split('.'); - const accessToken = await this.getAuthTokenAndValidate(keyId, secret); - await this.setLastUsedToken(keyId); - return this.usersService.getUserByUsername(accessToken.user.userName); - } catch (error) { - if ( - error instanceof NotInDBError || - error instanceof TokenNotValidError - ) { - throw new UnauthorizedException(error.message); - } - throw error; + const [keyId, secret] = token.split('.'); + if (!secret) { + throw new TokenNotValidError('Invalid AuthToken format'); } + 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); } async hashPassword(cleartext: string): Promise { @@ -92,7 +93,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; diff --git a/src/auth/token.strategy.ts b/src/auth/token.strategy.ts index 4f4f4e002..fe00c8298 100644 --- a/src/auth/token.strategy.ts +++ b/src/auth/token.strategy.ts @@ -6,9 +6,10 @@ import { Strategy } from 'passport-http-bearer'; import { PassportStrategy } from '@nestjs/passport'; -import { Injectable } from '@nestjs/common'; +import { Injectable, UnauthorizedException } from '@nestjs/common'; import { AuthService } from './auth.service'; import { User } from '../users/user.entity'; +import { NotInDBError, TokenNotValidError } from '../errors/errors'; @Injectable() export class TokenStrategy extends PassportStrategy(Strategy, 'token') { @@ -17,6 +18,16 @@ export class TokenStrategy extends PassportStrategy(Strategy, 'token') { } async validate(token: string): Promise { - return this.authService.validateToken(token); + try { + return await this.authService.validateToken(token); + } catch (error) { + if ( + error instanceof NotInDBError || + error instanceof TokenNotValidError + ) { + throw new UnauthorizedException(error.message); + } + throw error; + } } }