search

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

Ensure `authentication_otp` returns a boolean value #85

Closed EmCousin closed 3 years ago

EmCousin commented 3 years ago

ref. https://github.com/heapsource/active_model_otp/issues/84 https://github.com/heapsource/active_model_otp/issues/57#issuecomment-628334701

robertomiranda commented 3 years ago

Thanks, @EmCousin for your contribution!!! 🙇

graaff commented 3 years ago

This change is not backward compatible and may break existing code. For example, we were doing this:

result = !@user.authenticate_otp(otp, auto_increment: true).nil?

With this change a failure code now evaluates to true and passes because false is not nil.

EmCousin commented 3 years ago

@graaff The current documentation README.md indicates this method should return either true or false, which used to be the case until changes were made in ROTP. Ref https://github.com/heapsource/active_model_otp/issues/57

This Pull Request ensures the behaviour of this gem remains consistent according to that documentation, since ROTP's changes were breaking for people relying on this gem based on it.

I get your point though, I wouldn't have anything against the maintainer's decision to follow ROTP's direction.