LT-53534: support Rails 8 and Devise 5

.github/workflows/ci.yml

@@ -0,0 +1,60 @@
+name: CI
+
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+
+concurrency:
+  group: ci-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
+permissions:
+  contents: read
+
+jobs:
+  test:
+    name: Ruby ${{ matrix.ruby }} / Rails ${{ matrix.rails }} / Devise ${{ matrix.devise }}
+    runs-on: ubuntu-latest
+    timeout-minutes: 20
+
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - ruby: "3.4.10"
+            rails: "8.0.5.1"
+            devise: "4.9.4"
+          - ruby: "3.4.10"
+            rails: "8.0.5.1"
+            devise: "5.0.4"
+          - ruby: "3.4.10"
+            rails: "8.1.3.1"
+            devise: "4.9.4"
+          - ruby: "3.4.10"
+            rails: "8.1.3.1"
+            devise: "5.0.4"
+
+    env:
+      RAILS_VERSION: ${{ matrix.rails }}
+      DEVISE_VERSION: ${{ matrix.devise }}
+      ROTP_VERSION: "6.2.0"
+
+    steps:
+      - name: Check out repository
+        uses: actions/checkout@v7
+        with:
+          persist-credentials: false
+
+      - name: Set up Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: ${{ matrix.ruby }}
+          bundler-cache: true
+
+      - name: Set up test database
+        run: bundle exec rake app:db:setup
+
+      - name: Run specs
+        run: bundle exec rake spec

Gemfile

@@ -6,22 +6,51 @@ gemspec
 rails_version = ENV["RAILS_VERSION"] || "default"
 
 rails = case rails_version
-        when "master"
-          {github: "rails/rails"}
+        when "main"
+          {github: "rails/rails", branch: "main"}
         when "default"
           "~> 5.2"
         else
-          "~> #{rails_version}"
+          requirement = rails_version.split('.').length == 2 ? "#{rails_version}.0" : rails_version
+          "~> #{requirement}"
         end
 
 gem "rails", rails
 
-if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.2.0')
+devise_version = ENV["DEVISE_VERSION"]
+gem "devise", devise_version if devise_version
+
+rotp_version = ENV["ROTP_VERSION"]
+gem "rotp", rotp_version if rotp_version
+
+ruby_version = Gem::Version.new(RUBY_VERSION)
+
+if ruby_version >= Gem::Version.new('2.2.0')
   gem "test-unit", "~> 3.0"
 end
 
+if ruby_version < Gem::Version.new('2.3.0')
+  gem 'nokogiri', '~> 1.8.5'
+elsif ruby_version < Gem::Version.new('2.5.0')
+  gem 'nokogiri', '~> 1.10.10'
+elsif ruby_version < Gem::Version.new('2.6.0')
+  gem 'nokogiri', '~> 1.12.5'
+end
+
+gem 'loofah', '< 2.21' if ruby_version < Gem::Version.new('2.5.0')
+gem 'psych', '< 5' if rails_version == '7.1' && ruby_version < Gem::Version.new('3.0.0')
+
 group :test, :development do
-  gem 'sqlite3'
+  gem 'ostruct' if ruby_version >= Gem::Version.new('4.0.0')
+  case rails_version
+  when '4.2'
+    gem 'sqlite3', '~> 1.3.6'
+  when 'default', '5.2', '6.0', '6.1', '7.0'
+    gem 'sqlite3', '~> 1.4'
+  else
+    gem 'sqlite3'
+  end
+  gem 'sprockets-rails'
 end
 
 group :test do

lib/two_factor_authentication/models/two_factor_authenticatable.rb

@@ -44,7 +44,7 @@ def authenticate_totp(code, options = {})
             drift_ahead: drift, drift_behind: drift, after: totp_timestamp
           )
           return false unless new_timestamp
-          self.totp_timestamp = new_timestamp
+          self.totp_timestamp = Time.at(new_timestamp).utc
           true
         end
 
@@ -101,7 +101,7 @@ def generate_totp_secret
         def create_direct_otp(options = {})
           # Create a new random OTP and store it in the database
           digits = options[:length] || self.class.direct_otp_length || 6
-          update_attributes(
+          update(
             direct_otp: random_base10(digits),
             direct_otp_sent_at: Time.now.utc
           )
@@ -122,7 +122,7 @@ def direct_otp_expired?
         end
 
         def clear_direct_otp
-          update_attributes(direct_otp: nil, direct_otp_sent_at: nil)
+          update(direct_otp: nil, direct_otp_sent_at: nil)
         end
       end
 

lib/two_factor_authentication/routes.rb

@@ -3,7 +3,7 @@ class Mapper
     protected
 
       def devise_two_factor_authentication(mapping, controllers)
-        resource :two_factor_authentication, :only => [:show, :update, :resend_code], :path => mapping.path_names[:two_factor_authentication], :controller => controllers[:two_factor_authentication] do
+        resource :two_factor_authentication, :only => [:show, :update], :path => mapping.path_names[:two_factor_authentication], :controller => controllers[:two_factor_authentication] do
           collection { get "resend_code" }
         end
       end

spec/generators/active_record/two_factor_authentication_generator_spec.rb

@@ -1,9 +1,10 @@
 require 'spec_helper'
+require 'tmpdir'
 
 require 'generators/active_record/two_factor_authentication_generator'
 
 describe ActiveRecord::Generators::TwoFactorAuthenticationGenerator, type: :generator do
-  destination File.expand_path('../../../../../tmp', __FILE__)
+  destination File.join(Dir.tmpdir, 'two_factor_authentication_generator')
 
   before do
     prepare_destination
@@ -23,7 +24,7 @@
     describe 'the migration' do
       subject { migration_file('db/migrate/two_factor_authentication_add_to_users.rb') }
 
-      it { is_expected.to exist }
+      it { is_expected.to satisfy { |path| File.exist?(path) } }
       it { is_expected.to be_a_migration }
       it { is_expected.to contain /def change/ }
       it { is_expected.to contain /add_column :users, :second_factor_attempts_count, :integer, default: 0/ }

spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb

@@ -105,6 +105,29 @@ def do_invoke(code, user)
 
     it_behaves_like 'authenticate_totp', GuestUser.new
     it_behaves_like 'authenticate_totp', EncryptedUser.new
+
+    it 'persists the verification timestamp and rejects the same code after reload' do
+      instance = create_user('not_encrypted')
+      instance.update!(otp_secret_key: '2z6hxkdwi3uvrnpn')
+      code = TotpHelper.new(instance.otp_secret_key, instance.class.otp_length).totp_code
+
+      expect(instance.authenticate_totp(code)).to eq(true)
+      instance.save!
+
+      instance.reload
+      expect(instance.totp_timestamp).to be_a(Time)
+      expect(instance.authenticate_totp(code)).to eq(false)
+    end
+
+    it 'decrypts a secret encrypted before the Devise 5 upgrade' do
+      allow(Devise).to receive(:otp_secret_encryption_key).and_return('a' * 32)
+      instance = EncryptedUser.new
+      instance.encrypted_otp_secret_key = "qqtceBScHArOXNFRTZfNyDih+kzYDujh7emlkGi4V6A=\n"
+      instance.encrypted_otp_secret_key_iv = "ezgScHq7FcShFtQ2WYPP2g==\n"
+      instance.encrypted_otp_secret_key_salt = "_NemuOAzhuv7qvoPP3RVyBA==\n"
+
+      expect(instance.otp_secret_key).to eq('JBSWY3DPEHPK3PXP')
+    end
   end
 
   describe '#send_two_factor_authentication_code' do
@@ -137,8 +160,13 @@ def instance.send_two_factor_authentication_code(code)
         end
 
         it "returns uri with user's email" do
-          expect(instance.provisioning_uri).
-            to match(%r{otpauth://totp/houdini@example.com\?secret=\w{32}})
+          uri = URI.parse(instance.provisioning_uri)
+          params = URI.decode_www_form(uri.query).to_h
+
+          expect(uri.scheme).to eq('otpauth')
+          expect(uri.host).to eq('totp')
+          expect(URI.decode_www_form_component(uri.path)).to eq('/houdini@example.com')
+          expect(params['secret']).to match(/\w{32}/)
         end
 
         it 'returns uri with issuer option' do
@@ -147,15 +175,14 @@ def instance.send_two_factor_authentication_code(code)
         end
 
         it 'returns uri with issuer option' do
-          require 'cgi'
           uri = URI.parse(instance.provisioning_uri('houdini', issuer: 'Magic'))
-          params = CGI.parse(uri.query)
+          params = URI.decode_www_form(uri.query).to_h
 
           expect(uri.scheme).to eq('otpauth')
           expect(uri.host).to eq('totp')
-          expect(uri.path).to eq('/Magic:houdini')
-          expect(params['issuer'].shift).to eq('Magic')
-          expect(params['secret'].shift).to match(/\w{32}/)
+          expect(URI.decode_www_form_component(uri.path)).to eq('/Magic:houdini')
+          expect(params['issuer']).to eq('Magic')
+          expect(params['secret']).to match(/\w{32}/)
         end
       end
     end

spec/lib/two_factor_authentication/routes_spec.rb

@@ -0,0 +1,10 @@
+require 'spec_helper'
+
+describe 'two-factor authentication routes', type: :routing do
+  it 'routes resend_code through the collection endpoint' do
+    expect(get: '/users/two_factor_authentication/resend_code').to route_to(
+      controller: 'devise/two_factor_authentication',
+      action: 'resend_code'
+    )
+  end
+end

spec/rails_app/app/assets/config/manifest.js

@@ -0,0 +1,2 @@
+//= link_directory ../javascripts .js
+//= link_directory ../stylesheets .css

spec/rails_app/app/models/guest_user.rb

@@ -7,7 +7,7 @@ class GuestUser
   attr_accessor :direct_otp, :direct_otp_sent_at, :otp_secret_key, :email,
     :second_factor_attempts_count, :totp_timestamp
 
-  def update_attributes(attrs)
+  def update(attrs)
     attrs.each do |key, value|
       send(key.to_s + '=', value)
     end

spec/rails_app/config/application.rb

@@ -1,3 +1,4 @@
+require 'logger'
 require File.expand_path('../boot', __FILE__)
 
 require "active_record/railtie"
@@ -60,4 +61,3 @@ class Application < Rails::Application
     config.secret_key_base = 'secretvalue'
   end
 end
-

spec/rails_app/config/initializers/inflections.rb

@@ -13,3 +13,7 @@
 # ActiveSupport::Inflector.inflections do |inflect|
 #   inflect.acronym 'RESTful'
 # end
+
+ActiveSupport::Inflector.inflections do |inflect|
+  inflect.acronym 'SMS'
+end

spec/spec_helper.rb

@@ -1,5 +1,6 @@
 ENV["RAILS_ENV"] ||= "test"
 require File.expand_path("../rails_app/config/environment.rb",  __FILE__)
+require File.expand_path("../rails_app/lib/sms_provider.rb", __FILE__)
 
 require 'rspec/rails'
 require 'timecop'

spec/support/controller_helper.rb

@@ -11,6 +11,7 @@ def sign_in(user = create_user('not_encrypted'))
   config.include ControllerHelper, type: :controller
 
   config.before(:example, type: :controller) do
-    @request.env['devise.mapping'] = Devise.mappings[:user]
+    Rails.application.reload_routes! unless Devise.mappings.key?(:user)
+    @request.env['devise.mapping'] = Devise.mappings.fetch(:user)
   end
 end

two_factor_authentication.gemspec

@@ -34,7 +34,7 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'bundler'
   s.add_development_dependency 'rake'
   s.add_development_dependency 'rspec-rails', '>= 3.0.1'
-  s.add_development_dependency 'capybara', '~> 2.5'
+  s.add_development_dependency 'capybara', '>= 2.5', '< 4'
   s.add_development_dependency 'pry'
   s.add_development_dependency 'timecop'
 end