diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..bd054ba9 --- /dev/null +++ b/.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 diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index c3fc35f6..00000000 --- a/.travis.yml +++ /dev/null @@ -1,28 +0,0 @@ -language: ruby - -env: - - "RAILS_VERSION=4.2" - - "RAILS_VERSION=5.2" - - "RAILS_VERSION=master" - -rvm: - - 2.3.8 - - 2.4.5 - - 2.5.3 - -matrix: - fast_finish: true - allow_failures: - - env: "RAILS_VERSION=master" - include: - - rvm: 2.2 - env: RAILS_VERSION=4.2 - -before_install: - - gem uninstall -v '>= 2' -i $(rvm gemdir)@global -ax bundler || true - - gem install bundler -v '< 2' - -before_script: - - bundle exec rake app:db:setup - -script: bundle exec rake spec diff --git a/Gemfile b/Gemfile index 810ce296..7ec1c3f6 100644 --- a/Gemfile +++ b/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 diff --git a/lib/two_factor_authentication/models/two_factor_authenticatable.rb b/lib/two_factor_authentication/models/two_factor_authenticatable.rb index 6d73a0fb..ea1955b7 100644 --- a/lib/two_factor_authentication/models/two_factor_authenticatable.rb +++ b/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 diff --git a/lib/two_factor_authentication/routes.rb b/lib/two_factor_authentication/routes.rb index 543059a2..5e5442ba 100644 --- a/lib/two_factor_authentication/routes.rb +++ b/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 diff --git a/spec/generators/active_record/two_factor_authentication_generator_spec.rb b/spec/generators/active_record/two_factor_authentication_generator_spec.rb index 5a8989d0..1c9957b9 100644 --- a/spec/generators/active_record/two_factor_authentication_generator_spec.rb +++ b/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/ } diff --git a/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb b/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb index 6fb4f505..d72c8e1a 100644 --- a/spec/lib/two_factor_authentication/models/two_factor_authenticatable_spec.rb +++ b/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 diff --git a/spec/lib/two_factor_authentication/routes_spec.rb b/spec/lib/two_factor_authentication/routes_spec.rb new file mode 100644 index 00000000..55571760 --- /dev/null +++ b/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 diff --git a/spec/rails_app/app/assets/config/manifest.js b/spec/rails_app/app/assets/config/manifest.js new file mode 100644 index 00000000..21a78805 --- /dev/null +++ b/spec/rails_app/app/assets/config/manifest.js @@ -0,0 +1,2 @@ +//= link_directory ../javascripts .js +//= link_directory ../stylesheets .css diff --git a/spec/rails_app/app/models/guest_user.rb b/spec/rails_app/app/models/guest_user.rb index 8003624c..1222279a 100644 --- a/spec/rails_app/app/models/guest_user.rb +++ b/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 diff --git a/spec/rails_app/config/application.rb b/spec/rails_app/config/application.rb index 2d31d588..729fd66e 100644 --- a/spec/rails_app/config/application.rb +++ b/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 - diff --git a/spec/rails_app/config/initializers/inflections.rb b/spec/rails_app/config/initializers/inflections.rb index 5d8d9be2..1d5e5332 100644 --- a/spec/rails_app/config/initializers/inflections.rb +++ b/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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 63704333..189d5844 100644 --- a/spec/spec_helper.rb +++ b/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' diff --git a/spec/support/controller_helper.rb b/spec/support/controller_helper.rb index 2ca3a31f..cd9170e4 100644 --- a/spec/support/controller_helper.rb +++ b/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 diff --git a/two_factor_authentication.gemspec b/two_factor_authentication.gemspec index 9580f117..98d5b99c 100644 --- a/two_factor_authentication.gemspec +++ b/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