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

Add support for different HMAC algorithms. #63

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ShaneMcC
Copy link

RFC 6238 (https://tools.ietf.org/html/rfc6238) states:

TOTP implementations MAY use HMAC-SHA-256 or HMAC-SHA-512 functions,
based on SHA-256 or SHA-512 [SHA2] hash functions, instead of the
HMAC-SHA-1 function that has been specified for the HOTP computation
in [RFC4226].

This PR adds support for that.

I've also added additional tests to codeProvider() based on the code and output of the sample code in the RFC to validate the changes.

As a side effect of implementing the above, I've also fixed the case where if you change the _codeLength, verifyCode() always returned false for any value other than 6, and added some boilerplate at the top of GoogleAuthenticatorTest.php to make the tests run under phpunit 6+

@ShaneMcC
Copy link
Author

Looks like #55 also fixed the verifyCode() issue, didn't see it before I did this.

@MarkMaldaba
Copy link

Good idea, but I think this would be better if it was handled in the same way as code length, for consistency (and also for ease-of-use).

@MarkMaldaba
Copy link

Also, it might be sensible to validate the algorithm string, to avoid errors and potential URL-injection attacks.

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

Successfully merging this pull request may close these issues.

2 participants