Skip to content

Conversation

@szabolcsnagy
Copy link
Contributor

This PR will address issue #21.

I believe moving to BigInt broke the TOTP verification because the authenticator apps are still using 4bytes from the generated hashBytes while this library uses 8bytes now that it is switched to BigInt.

In this PR I essentially mask out the top 32 bits of the BigInt hotpVal for 6 digit codes allowing it to run with the same value that an authenticator app would.

@szabolcsnagy szabolcsnagy force-pushed the stay-compatible-with-auth-apps branch from 8c4d360 to bc5b120 Compare February 15, 2025 15:37
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks!

@kentcdodds kentcdodds merged commit 09f5f48 into epicweb-dev:main Feb 15, 2025
2 checks passed
@github-actions
Copy link

🎉 This PR is included in version 3.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@szabolcsnagy
Copy link
Contributor Author

Hold on. I found a bug in it. I guess I need to open a new PR?

@kentcdodds
Copy link
Member

Yes, we need a test for the bug too. Thanks!

@kentcdodds
Copy link
Member

This has been reverted because it broke base 2fa functionality. I welcome a contribution which adds this back but does not break this test: 52dabf0

Sorry for the trouble and thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants