search

daegalus / dart-otp

RFC6238 Time-Based One-Time Password / Google Authenticator Library
MIT License
100 stars 25 forks source link

v3.1.0 fixes subtle bug but introduces breaking changes #40

Closed maurovanetti closed 2 years ago

maurovanetti commented 2 years ago

I updated a commercial project to v3.1.0 and this broke the unit tests. It's not a problem because the tests were checking the subtle differences between isGoogle:true and isGoogle:false. Before v3.1.0, those differences were relevant only for short secrets, but there was no difference when the secrets had the correct length. Nevertheless, you are now potentially breaking any app that was relying on the isGoogle:false behaviour while using Base32. My suggestion is that this change in behaviour should be properly documented (particularly since this is happening in a minor version change). Thanks for your amazing work with this plugin.

daegalus commented 2 years ago

That is a fair request. In my mind, since I was fixing a bug, and while the generated code was different, it didn't impact the actual secret. and it was going from broken codes to fixed codes. I didn't think it needed a deeper explanation, or a Major version change, figured a minor one was more adequate for what was happening.

But I do agree its fair to give users a better explanation of what happened. I updated the top of the Readme to have a link to https://github.com/Daegalus/dart-otp/wiki/Explanation-of-Changes-from-3.0-to-3.1 which goes into more detail on the change, why it was made, and how it impacts users.

Let me know if this satisfies what you were requesting.

maurovanetti commented 2 years ago

This is truly a great explanation. Actually, I understood my problem even better after reading it. :-) Thank you for your effort and contribution to the community.