Add counter value when triggering ROTP#provisioning_uri

heapsource / active_model_otp

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

MIT License

769 stars 81 forks source link

Add counter value when triggering ROTP#provisioning_uri #76

Closed wenderjean closed 3 years ago

wenderjean commented 3 years ago

Changes

Add self.otp_counter when calling ROTP#provisioning_uri in order to have it on the final URI

Concerns

Even though it seems somethings that never caused issues, reading through the RFC-4226 I noticed some requirements on keeping the counter synchronized between server/client, please, let me know your thoughts regarding that.

Note.: Taking a look at the source it doesn't seem we're caring about the houndci-bot for line length metrics. Did we enable houndci-bot recently?

pedrofurtado commented 3 years ago

Hi @wenderjean ! 👋

Thanks for your contribution (and delayed review of my part) 🤝 🍻

That's a great fix! In fact, when we generate the provisioning uri with otp counter previously used (and then with a value greater than zero), the authenticator app and server/backend must be in sync (i.e., with the same counter value).

Please, could you fix the CI warnings and file conflicts? After that, @robertomiranda , I think we are ready to merge it ✅ 🏁

wenderjean commented 3 years ago

I'll take a look on tests asap guys, I'm using a different PC right now for that reason I'm getting those issues.

pedrofurtado commented 3 years ago

Thanks a lot @wenderjean 🍻 Let us know when you got tests working well 🤝 We will be glad to review it

Thanks again for your effort on it 🥇

wenderjean commented 3 years ago

All good @pedrofurtado :)

pedrofurtado commented 3 years ago

Thank you for your effort on it, @wenderjean ! 🤝

Our team is planning (especially me and @robertomiranda) to release this and some other improvements/bugfixes in a new release soon 🎉