mirror of
https://github.com/hedgedoc/hedgedoc.git
synced 2024-11-29 17:54:33 -05:00
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>
This commit is contained in:
parent
8b27f6f393
commit
46b5cdfb47
2 changed files with 25 additions and 1 deletions
|
@ -13,6 +13,7 @@ import { UsersModule } from '../users/users.module';
|
||||||
import { Identity } from '../users/identity.entity';
|
import { Identity } from '../users/identity.entity';
|
||||||
import { LoggerModule } from '../logger/logger.module';
|
import { LoggerModule } from '../logger/logger.module';
|
||||||
import { AuthToken } from './auth-token.entity';
|
import { AuthToken } from './auth-token.entity';
|
||||||
|
import { TokenNotValidError } from '../errors/errors';
|
||||||
|
|
||||||
describe('AuthService', () => {
|
describe('AuthService', () => {
|
||||||
let service: AuthService;
|
let service: AuthService;
|
||||||
|
@ -105,6 +106,16 @@ describe('AuthService', () => {
|
||||||
.checkPassword(testPassword, hash)
|
.checkPassword(testPassword, hash)
|
||||||
.then((result) => expect(result).toBeTruthy());
|
.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', () => {
|
describe('getTokensByUsername', () => {
|
||||||
|
@ -148,6 +159,11 @@ describe('AuthService', () => {
|
||||||
authTokens: [authToken],
|
authTokens: [authToken],
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
it('fails on too long token', () => {
|
||||||
|
expect(
|
||||||
|
service.validateToken(`${authToken.keyId}.${'a'.repeat(73)}`),
|
||||||
|
).rejects.toBeInstanceOf(TokenNotValidError);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('removeToken', () => {
|
describe('removeToken', () => {
|
||||||
|
|
|
@ -37,6 +37,14 @@ export class AuthService {
|
||||||
async validateToken(token: string): Promise<User> {
|
async validateToken(token: string): Promise<User> {
|
||||||
try {
|
try {
|
||||||
const [keyId, secret] = token.split('.');
|
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);
|
const accessToken = await this.getAuthTokenAndValidate(keyId, secret);
|
||||||
await this.setLastUsedToken(keyId);
|
await this.setLastUsedToken(keyId);
|
||||||
return this.usersService.getUserByUsername(accessToken.user.userName);
|
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`,
|
`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 keyId = this.BufferToBase64Url(await this.randomString(8));
|
||||||
const accessToken = await this.hashPassword(secret);
|
const accessToken = await this.hashPassword(secret);
|
||||||
let token;
|
let token;
|
||||||
|
|
Loading…
Reference in a new issue