Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bcryptjs compares badly with jwt and its hash #137

Open
JYachelini opened this issue Aug 24, 2022 · 5 comments
Open

Bcryptjs compares badly with jwt and its hash #137

JYachelini opened this issue Aug 24, 2022 · 5 comments

Comments

@JYachelini
Copy link

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
Copy link

Fumarie commented Sep 6, 2022

Same error! Comparing always returns true

@Fumarie
Copy link

Fumarie commented Sep 6, 2022

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

@JYachelini
Copy link
Author

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
Copy link
Author

I correct myself, it happens without Nest.

@EvilCheetah
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants