dcodeIO / bcrypt.js

Optimized bcrypt in plain JavaScript with zero dependencies.
Other
3.47k stars 264 forks source link

Bcryptjs compares badly with jwt and its hash #137

Open JYachelini opened 1 year ago

JYachelini commented 1 year ago

Hello! I'm comparing a Refresh token Hash with another Refresh token and it returns true when it should return false.

The refresh token hash is stored in the DB in the User collection. When I send a refresh token through a client it compares this refresh token with the refresh token hash and always return true. The first comparation compares is fine because they are the same token. But when this process finishes it is not the same, it always saves a new refresh token hash in the DB. So ONLY in the first comparision it has to return true.

Steps: 1) Log in and returns 2 tokens, access token and refresh token. imagen

2) Refresh the tokens with the refresh token I got from the login. It returns two new tokens. So the refresh token in the DB has change, it is not the same when I log in. imagen

3) Refresh the token again with the SAME tokenI obtained in step 1. imagen

This means the comparition is true.

Code:

refreshTokens = async (_id: ObjectId, refresh_token: string) => {
    const user = await this.usersRepository.findById(_id)
    if (!user || !user.hashRT) throw new ForbiddenException('Access Denied.')

    // This compare the refresh token from client with the refresh token hash from db
    const refresh_tokenMatches = await bcrypt.compare(
      refresh_token,
      user.hashRT,
    );
    if (!refresh_tokenMatches) throw new ForbiddenException('Access Denied.');

    // This create a new tokens with jwt.sign
    const tokens = await this.getTokens(user);
    // This save the new refresh token hash in the db
    await this.usersRepository.updateRefreshTokenHash(
      user._id,
      tokens.refresh_token,
    );
    return tokens;
  };

Please notify me if I'm wrong and this comparision is correct. And tell me if I have explained myself wrong, English is not my first language and it's a bit difficult to me haha.

Fumarie commented 1 year ago

Same error! Comparing always returns true

Fumarie commented 1 year ago

@JYachelini In my opinion it happens because the output hash is too short comparing with jwt token, therefore we get collisions

JYachelini commented 1 year ago

Hello @Fumarie, sorry I did not answer, but I found what argon2, a dependency to encrypt data, does not have this problem. And I realized what this problem occurs with NestJS, at least to me it happened only with NestJS.

JYachelini commented 1 year ago

I correct myself, it happens without Nest.

EvilCheetah commented 1 year ago

Experiencing the same issue. I have tried both sync and async methods to compare refresh tokens, but both of them return true.