Disallow TOTP reuse #4911
Disallow TOTP reuse #4911
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4911 +/- ##
=======================================
Coverage 97.01% 97.02%
=======================================
Files 420 420
Lines 8724 8726 +2
=======================================
+ Hits 8464 8466 +2
Misses 260 260 ☔ View full report in Codecov by Sentry. |
|
I want to point out that this has the potential to break any workflows that use the TOTP to make multiple requests |
@segiddins do you consider this a blocker for now? |
|
It's not necessarily a blocker, I just want us all to be on the same page with what the consequences are. |
|
@simi @colby-swandale @indirect let's come to a conclusion on this, and if we decide to move forward I can write a tiny blog post announcing it? |
simi
force-pushed
the
segiddins/disallow-totp-reuse
branch
from
187854e
to
0562dcb
Compare
|
Happy to move this forward,but I'm afraid there are workflows possibly affected like Rails release process. I have left message at Rails discord to confirm this is ok for now. Once confirmed, happy to merge, deploy and read the following blog post 👀. |
|
If I got this correctly, yes, this would break the Rails release. As we release gems in sequence, most of the time the OTP code didn't change yet between the releases so we would errors. I personally am not using the OTP codes anymore, I'm using WebAuthn, but I know Aaron and John are still using the OTP workflow. Would this also change the behavior of WebAuthn? Even for the WebAuthn, I'd love a way to reuse the same token for a certain period of time line npm does, since opening many browser windows and clicking in the same button 14 times isn't fun. |
|
Can I convince y'all to move to trusted publishing as an alternative, since that doesn't have the same reuse concerns? |
|
I don't think you need to convince us. We would like to use it. I just not sure if we can right now. Rails release is complex, it has a sequence, it doesn't use bundler release tasks, and it also releases to npm. Changing the release process is always tricky since there is no "staging" environment. |
|
I'm started to test the trusted publisher with the thor gem. I was able to make it work. The annoying part is more a GitHub limitation than a limitation on Rubygems. It only allow us to require approval from 6 people. Rails core has 12, so it means that a few people will lose the ability to release :(. I'm not confident to make the same kind of test with Rails because we need to test it live, and I don't want to have to yank Rails versions. |
|
@rafaelfranca would testing/staging environment for rubygems.org help you building trust to new process? |
For sure! |
|
Actually, I thin I can just rename rails to something else in my fork and release on those new names. Them I can just yank all those new gems I'm publishing when I'm happy with the setup. I'll try that. |
|
@rafaelfranca take your time, no pressure. Feel free to ping me on Discord for support if needed, we can help you with smooth transition. |
|
Looks amazing @rafaelfranca. Feel free to ping us once you'll get brave enough to release another Rails this way. I'll check meanwhile other major gems. |
See https://rubydoc.info/github/mdp/rotp/#preventing-reuse-of-time-based-otps
See https://www.rfc-editor.org/rfc/rfc6238#section-5.2