diff --git a/Gemfile.lock b/Gemfile.lock index 6f83d6cc2a4..539531d510f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -315,7 +315,8 @@ GEM request_store (1.4.1) rack (>= 1.4) rinku (2.0.6) - rotp (3.3.1) + rotp (5.1.0) + addressable (~> 2.5) rqrcode (0.10.1) chunky_png (~> 1.0) rspec (3.8.0) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 46843c32f72..3c2fe8382d4 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1219,7 +1219,7 @@ class UsersController < ApplicationController end def create_second_factor_totp - totp_data = ROTP::Base32.random_base32 + totp_data = ROTP::Base32.random secure_session["staged-totp-#{current_user.id}"] = totp_data qrcode_svg = RQRCode::QRCode.new(current_user.totp_provisioning_uri(totp_data)).as_svg( offset: 0, @@ -1295,7 +1295,11 @@ class UsersController < ApplicationController RateLimiter.new(nil, "second-factor-min-#{key}", 3, 1.minute).performed! end - authenticated = !auth_token.blank? && totp_object.verify_with_drift(auth_token, 30) + authenticated = !auth_token.blank? && totp_object.verify( + auth_token, + drift_ahead: SecondFactorManager::TOTP_ALLOWED_DRIFT_SECONDS, + drift_behind: SecondFactorManager::TOTP_ALLOWED_DRIFT_SECONDS + ) unless authenticated return render json: failed_json.merge( error: I18n.t("login.invalid_second_factor_code") diff --git a/app/models/concerns/second_factor_manager.rb b/app/models/concerns/second_factor_manager.rb index 7c20cc7ce2c..1f713ed5120 100644 --- a/app/models/concerns/second_factor_manager.rb +++ b/app/models/concerns/second_factor_manager.rb @@ -1,13 +1,15 @@ # frozen_string_literal: true module SecondFactorManager + TOTP_ALLOWED_DRIFT_SECONDS = 30 + extend ActiveSupport::Concern def create_totp(opts = {}) UserSecondFactor.create!({ user_id: self.id, method: UserSecondFactor.methods[:totp], - data: ROTP::Base32.random_base32 + data: ROTP::Base32.random }.merge(opts)) end @@ -30,7 +32,13 @@ module SecondFactorManager last_used = totp.last_used.to_i end - authenticated = !token.blank? && totp.get_totp_object.verify_with_drift_and_prior(token, 30, last_used) + authenticated = !token.blank? && totp.get_totp_object.verify( + token, + drift_ahead: TOTP_ALLOWED_DRIFT_SECONDS, + drift_behind: TOTP_ALLOWED_DRIFT_SECONDS, + after: last_used + ) + if authenticated totp.update!(last_used: DateTime.now) break