2 bugs in provisioning_uri for HOTP tokens

heapsource / active_model_otp

Adds methods to set and authenticate against one time passwords (Two-Factor Authentication). Inspired in AM::SecurePassword

MIT License

773 stars 81 forks source link

2 bugs in provisioning_uri for HOTP tokens #74

Closed kalsan closed 3 years ago

kalsan commented 3 years ago

Hi!

First of all, thanks for this super convenient gem which works great. I'm using it for HOTP support and discovered 2 bugs in provisioning_url:

Gemfile:

    active_model_otp (2.0.1)
      activemodel
      rotp (~> 5.0.0)

User u:

...
 otp_counter: 8,
...

Now if I run: u.provisioning_uri(nil, issuer: 'My fancy issuer')

I get: "otpauth://hotp/test@miau.ch?secret=XE7ZAMIPLYEHUOFFFMT5M52R6LO7762X&counter=0"

counter should be 8
issuer is missing

Cheers, Kalsan

wenderjean commented 3 years ago

I've created a pull request to handle part of your claims @kalsan, at least the capacity of bypass the otp_counter to the URIgenerator in order to have it in the final result.

As we can see at this point of the ROTP source, the issuer property is not supported when we're dealing with HOTP based. I took a look into RFC-6238 and RFC-4226, as well as I, performed some research trying to understand why that guard clause was added into ROTP but I couldn't find an explanation, I supposed to find some rationale as why we don't need issuer for HOTP but...

kalsan commented 3 years ago

Thanks for addressing this! Weird that they don't mention it in the RFC. Most apps seem to support it, I belive Google Authenticator is one of them.

pedrofurtado commented 3 years ago

Hey guys!

I think we can follow the progress of this issue directly in PR https://github.com/heapsource/active_model_otp/pull/76 🤝