FIX: require: false for rotp gem (#8540)

The ROTP gem is only used in a very small amount of places in the app, we don't need to globally require it.

Also set the Addressable gem to not have a specific version range, as it has not been a problem yet.

Some slight refactoring of UserSecondFactor here too to use SecondFactorManager to avoid code repetition

Gemfile

@@ -134,7 +134,7 @@ gem 'highline', '~> 1.7.0', require: false
 gem 'rack-protection' # security
 gem 'cbor', require: false
 gem 'cose', require: false
-gem 'addressable', '~> 2.7.0'
+gem 'addressable'
 
 # Gems used only for assets and not required in production environments by default.
 # Allow everywhere for now cause we are allowing asset debugging in production
@@ -229,7 +229,7 @@ gem 'logster'
 gem 'sassc', '2.0.1', require: false
 gem "sassc-rails"
 
-gem 'rotp'
+gem 'rotp', require: false
 gem 'rqrcode'
 
 gem 'rubyzip', require: false

Gemfile.lock

@@ -437,7 +437,7 @@ DEPENDENCIES
   activemodel (= 6.0.1)
   activerecord (= 6.0.1)
   activesupport (= 6.0.1)
-  addressable (~> 2.7.0)
+  addressable
   annotate
   aws-sdk-s3
   aws-sdk-sns

app/controllers/users_controller.rb

@@ -1218,6 +1218,7 @@ class UsersController < ApplicationController
   end
 
   def create_second_factor_totp
+    require 'rotp' if !defined? ROTP
     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(

app/models/concerns/second_factor_manager.rb

@@ -6,6 +6,7 @@ module SecondFactorManager
   extend ActiveSupport::Concern
 
   def create_totp(opts = {})
+    require_rotp
     UserSecondFactor.create!({
                                user_id: self.id,
                                method: UserSecondFactor.methods[:totp],
@@ -14,6 +15,7 @@ module SecondFactorManager
   end
 
   def get_totp_object(data)
+    require_rotp
     ROTP::TOTP.new(data, issuer: SiteSetting.title)
   end
 
@@ -32,7 +34,7 @@ module SecondFactorManager
         last_used = totp.last_used.to_i
       end
 
-      authenticated = !token.blank? && totp.get_totp_object.verify(
+      authenticated = !token.blank? && totp.totp_object.verify(
         token,
         drift_ahead: TOTP_ALLOWED_DRIFT_SECONDS,
         drift_behind: TOTP_ALLOWED_DRIFT_SECONDS,
@@ -132,4 +134,8 @@ module SecondFactorManager
   def hash_backup_code(code, salt)
     Pbkdf2.hash_password(code, salt, Rails.configuration.pbkdf2_iterations, Rails.configuration.pbkdf2_algorithm)
   end
+
+  def require_rotp
+    require 'rotp' if !defined? ROTP
+  end
 end

app/models/user_second_factor.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 class UserSecondFactor < ActiveRecord::Base
+  include SecondFactorManager
   belongs_to :user
 
   scope :backup_codes, -> do
@@ -22,12 +23,12 @@ class UserSecondFactor < ActiveRecord::Base
     )
   end
 
-  def get_totp_object
+  def totp_object
-    ROTP::TOTP.new(self.data, issuer: SiteSetting.title)
+    get_totp_object(self.data)
   end
 
   def totp_provisioning_uri
-    get_totp_object.provisioning_uri(user.email)
+    totp_object.provisioning_uri(user.email)
   end
 
 end

spec/components/concern/second_factor_manager_spec.rb

@@ -18,8 +18,8 @@ RSpec.describe SecondFactorManager do
         totp = another_user.create_totp(enabled: true)
       end.to change { UserSecondFactor.count }.by(1)
 
-      expect(totp.get_totp_object.issuer).to eq(SiteSetting.title)
+      expect(totp.totp_object.issuer).to eq(SiteSetting.title)
-      expect(totp.get_totp_object.secret).to eq(another_user.reload.user_second_factors.totps.first.data)
+      expect(totp.totp_object.secret).to eq(another_user.reload.user_second_factors.totps.first.data)
     end
   end
 
@@ -46,7 +46,7 @@ RSpec.describe SecondFactorManager do
       freeze_time do
         expect(user.user_second_factors.totps.first.last_used).to eq(nil)
 
-        token = user.user_second_factors.totps.first.get_totp_object.now
+        token = user.user_second_factors.totps.first.totp_object.now
 
         expect(user.authenticate_totp(token)).to eq(true)
         expect(user.user_second_factors.totps.first.last_used).to eq_time(DateTime.now)

spec/requests/session_controller_spec.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'rails_helper'
+require 'rotp'
 
 RSpec.describe SessionController do
   let(:email_token) { Fabricate(:email_token) }

spec/requests/users_controller_spec.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'rails_helper'
+require 'rotp'
 
 describe UsersController do
   let(:user) { Fabricate(:user) }

spec/requests/users_email_controller_spec.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'rails_helper'
+require 'rotp'
 
 describe UsersEmailController do