From 7f0d3d39007bab12c96530a262f2d515022b1c5d Mon Sep 17 00:00:00 2001 From: Dan Milne Date: Mon, 20 Apr 2026 18:58:39 +1000 Subject: [PATCH] Tighten TOTP enrollment comments to explain the threat, not the change Replace the changelog-flavored "view no longer round-trips" line with a one-liner naming the actual threat (session-holder substituting a secret they control). Drop the narration comment above session.delete + deliver_later -- the identifiers already say what the two lines do. Co-Authored-By: Claude Opus 4 --- app/controllers/totp_controller.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/controllers/totp_controller.rb b/app/controllers/totp_controller.rb index 39774b7..068d7f8 100644 --- a/app/controllers/totp_controller.rb +++ b/app/controllers/totp_controller.rb @@ -12,8 +12,8 @@ class TotpController < ApplicationController @totp_secret = ROTP::Base32.random @provisioning_uri = ROTP::TOTP.new(@totp_secret, issuer: "Clinch").provisioning_uri(@user.email_address) - # Hold the secret server-side until the user confirms it with a valid code. - # The view no longer round-trips it through a hidden form field. + # Hold the secret server-side until the user confirms it with a valid code, + # so an attacker with session access cannot substitute one they control. session[:pending_totp_secret] = @totp_secret # Generate QR code @@ -39,7 +39,6 @@ class TotpController < ApplicationController plain_codes = @user.send(:generate_backup_codes) # Use private method from User model @user.save! - # Consume the pending secret and notify the user that 2FA is now active session.delete(:pending_totp_secret) TotpMailer.enabled(@user).deliver_later