diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..1f82dc8 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,42 @@ +name: 'CI/CD Pipeline' + +on: + push: + branches: + - master + pull_request: + workflow_dispatch: + +defaults: + run: + shell: bash + +jobs: + + test: + name: Test + runs-on: ubuntu-latest + + env: + RAILS_ENV: test + GEMFILE_RUBY_VERSION: 3.0.3 + + # Rails verifies the time zone in DB is the same as the time zone of the Rails app + TZ: "Europe/London" + + + steps: + - name: Checkout + uses: actions/checkout@v2 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: 3.0.3 + # runs 'bundle install' and caches installed gems automatically + bundler-cache: true + + - name: Run tests + run: | + bundle exec rake spec + diff --git a/.rubocop.yml b/.rubocop.yml index eb429ff..88460ce 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -4,6 +4,24 @@ AllCops: - '**/Rakefile' UseCache: true +Layout/LineLength: + Description: Limit lines to 80 characters. + StyleGuide: https://github.com/bbatsov/ruby-style-guide#80-character-limits + Enabled: true + Max: 100 + AllowURI: true + URISchemes: + - http + - https +Layout/DotPosition: + Description: Checks the position of the dot in multi-line method calls. + StyleGuide: https://github.com/bbatsov/ruby-style-guide#consistent-multi-line-chains + Enabled: true + EnforcedStyle: trailing + SupportedStyles: + - leading + - trailing + Lint/AssignmentInCondition: Description: Don't use assignment in conditions. StyleGuide: https://github.com/bbatsov/ruby-style-guide#safe-assignment-in-condition @@ -12,11 +30,11 @@ Lint/AssignmentInCondition: Lint/EachWithObjectArgument: Description: Check for immutable argument given to each_with_object. Enabled: true -Lint/HandleExceptions: +Lint/SuppressedException: Description: Don't suppress exception. StyleGuide: https://github.com/bbatsov/ruby-style-guide#dont-hide-exceptions Enabled: true -Lint/LiteralInCondition: +Lint/LiteralAsCondition: Description: Checks of literals used in conditions. Enabled: true Lint/LiteralInInterpolation: @@ -46,15 +64,6 @@ Metrics/CyclomaticComplexity: cases needed to validate a method. Enabled: true Max: 6 -Metrics/LineLength: - Description: Limit lines to 80 characters. - StyleGuide: https://github.com/bbatsov/ruby-style-guide#80-character-limits - Enabled: true - Max: 100 - AllowURI: true - URISchemes: - - http - - https Metrics/MethodLength: Description: Avoid methods longer than 10 lines of code. StyleGuide: https://github.com/bbatsov/ruby-style-guide#short-methods @@ -82,21 +91,27 @@ Metrics/PerceivedComplexity: Enabled: true Max: 7 -Rails/ScopeArgs: - Description: Checks the arguments of ActiveRecord scopes. +Naming/AccessorMethodName: + Description: Check the naming of accessor methods for get_/set_. + Enabled: false +Naming/FileName: + Description: Use snake_case for source file names. + StyleGuide: https://github.com/bbatsov/ruby-style-guide#snake-case-files Enabled: true -Rails/TimeZone: - # The value `strict` means that `Time` should be used with `zone`. - # The value `flexible` allows usage of `in_time_zone` instead of `zone`. + Exclude: [] +Naming/PredicateName: + Description: Check the names of predicate methods. + StyleGuide: https://github.com/bbatsov/ruby-style-guide#bool-methods-qmark Enabled: true - EnforcedStyle: flexible - SupportedStyles: - - strict - - flexible + NamePrefix: + - is_ + - has_ + - have_ + ForbiddenPrefixes: + - is_ + Exclude: + - spec/**/* -Style/AccessorMethodName: - Description: Check the naming of accessor methods for get_/set_. - Enabled: false Style/AndOr: Description: Use &&/|| instead of and/or. StyleGuide: https://github.com/bbatsov/ruby-style-guide#no-and-or-or @@ -127,14 +142,6 @@ Style/CollectionMethods: Style/Documentation: Description: Document classes and non-namespace modules. Enabled: false -Style/DotPosition: - Description: Checks the position of the dot in multi-line method calls. - StyleGuide: https://github.com/bbatsov/ruby-style-guide#consistent-multi-line-chains - Enabled: true - EnforcedStyle: trailing - SupportedStyles: - - leading - - trailing Style/DoubleNegation: Description: Checks for uses of double negation (!!). StyleGuide: https://github.com/bbatsov/ruby-style-guide#no-bang-bang @@ -146,11 +153,6 @@ Style/EmptyLiteral: Description: Prefer literals to Array.new/Hash.new/String.new. StyleGuide: https://github.com/bbatsov/ruby-style-guide#literal-array-hash Enabled: true -Style/FileName: - Description: Use snake_case for source file names. - StyleGuide: https://github.com/bbatsov/ruby-style-guide#snake-case-files - Enabled: true - Exclude: [] Style/GuardClause: Description: Check for conditionals that can be replaced with guard clauses StyleGuide: https://github.com/bbatsov/ruby-style-guide#no-nested-conditionals @@ -160,7 +162,6 @@ Style/IfUnlessModifier: Description: Favor modifier if/unless usage when you have a single-line body. StyleGuide: https://github.com/bbatsov/ruby-style-guide#if-as-a-modifier Enabled: false - MaxLineLength: 80 Style/InlineComment: Description: Avoid inline comments. Enabled: false @@ -193,18 +194,6 @@ Style/PerlBackrefs: Description: Avoid Perl-style regex back references. StyleGuide: https://github.com/bbatsov/ruby-style-guide#no-perl-regexp-last-matchers Enabled: false -Style/PredicateName: - Description: Check the names of predicate methods. - StyleGuide: https://github.com/bbatsov/ruby-style-guide#bool-methods-qmark - Enabled: true - NamePrefix: - - is_ - - has_ - - have_ - NamePrefixBlacklist: - - is_ - Exclude: - - spec/**/* Style/RaiseArgs: Description: Checks the arguments passed to raise/fail. StyleGuide: https://github.com/bbatsov/ruby-style-guide#exception-class-messages @@ -272,7 +261,16 @@ Style/TrailingCommaInArguments: - comma - consistent_comma - no_comma -Style/TrailingCommaInLiteral: +Style/TrailingCommaInArrayLiteral: + Description: 'Checks for trailing comma in array and hash literals.' + StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-trailing-array-commas' + Enabled: true + EnforcedStyleForMultiline: no_comma + SupportedStyles: + - comma + - consistent_comma + - no_comma +Style/TrailingCommaInHashLiteral: Description: 'Checks for trailing comma in array and hash literals.' StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-trailing-array-commas' Enabled: true diff --git a/Gemfile b/Gemfile index 810ce29..dd64f57 100644 --- a/Gemfile +++ b/Gemfile @@ -1,30 +1,34 @@ +# frozen_string_literal: true + source 'https://rubygems.org' # Specify your gem's dependencies in devise_ip_filter.gemspec gemspec -rails_version = ENV["RAILS_VERSION"] || "default" +rails_version = ENV['RAILS_VERSION'] || 'default' rails = case rails_version - when "master" - {github: "rails/rails"} - when "default" - "~> 5.2" + when 'master' + { github: 'rails/rails' } + when 'default' + '~> 7.0.1' else "~> #{rails_version}" end -gem "rails", rails +gem 'rails', rails if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.2.0') - gem "test-unit", "~> 3.0" + gem 'test-unit', '~> 3.0' end group :test, :development do + gem 'pry' + gem 'sprockets-rails' gem 'sqlite3' end group :test do - gem 'rack_session_access' gem 'ammeter' + gem 'rack_session_access' end diff --git a/README.md b/README.md index f4f91d1..ea372ba 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,6 @@ # Two factor authentication for Devise -[![Gitter](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/Houdini/two_factor_authentication?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) - -[![Build Status](https://travis-ci.org/Houdini/two_factor_authentication.svg?branch=master)](https://travis-ci.org/Houdini/two_factor_authentication) +[![Build Status](https://github.com/baarkerlounger/two_factor_authentication/actions/workflows/tests.yml/badge.svg?branch=master&event=push)](https://github.com/baarkerlounger/two_factor_authentication/actions/workflows/tests.yml) [![Code Climate](https://codeclimate.com/github/Houdini/two_factor_authentication.svg)](https://codeclimate.com/github/Houdini/two_factor_authentication) ## Features diff --git a/Rakefile b/Rakefile index 7723dfb..1012285 100644 --- a/Rakefile +++ b/Rakefile @@ -1,14 +1,16 @@ -require "bundler/gem_tasks" +# frozen_string_literal: true -APP_RAKEFILE = File.expand_path("../spec/rails_app/Rakefile", __FILE__) +require 'bundler/gem_tasks' + +APP_RAKEFILE = File.expand_path('spec/rails_app/Rakefile', __dir__) load 'rails/tasks/engine.rake' require 'rspec/core/rake_task' -desc "Run all specs in spec directory (excluding plugin specs)" -RSpec::Core::RakeTask.new(:spec => 'app:db:test:prepare') +desc 'Run all specs in spec directory (excluding plugin specs)' +RSpec::Core::RakeTask.new(spec: 'app:db:test:prepare') -task :default => :spec +task default: :spec # To test against a specific version of Rails # export RAILS_VERSION=3.2.0; bundle update; rake diff --git a/app/controllers/devise/two_factor_authentication_controller.rb b/app/controllers/devise/two_factor_authentication_controller.rb index 0e7aed8..9430d6a 100644 --- a/app/controllers/devise/two_factor_authentication_controller.rb +++ b/app/controllers/devise/two_factor_authentication_controller.rb @@ -8,9 +8,9 @@ class Devise::TwoFactorAuthenticationController < DeviseController end def update - render :show and return if params[:code].nil? + render :show, status: :unprocessable_entity and return if params_code.empty? - if resource.authenticate_otp(params[:code]) + if resource.authenticate_otp(params_code) after_two_factor_success_for(resource) else after_two_factor_fail_for(resource) @@ -63,9 +63,9 @@ class Devise::TwoFactorAuthenticationController < DeviseController if resource.max_login_attempts? sign_out(resource) - render :max_login_attempts_reached + render :max_login_attempts_reached, status: :unprocessable_entity else - render :show + render :show, status: :unprocessable_entity end end @@ -78,7 +78,11 @@ class Devise::TwoFactorAuthenticationController < DeviseController @limit = resource.max_login_attempts if resource.max_login_attempts? sign_out(resource) - render :max_login_attempts_reached and return + render :max_login_attempts_reached, status: :unprocessable_entity and return end end + + def params_code + params[:code] || params.dig(resource_name, :code) + end end diff --git a/lib/two_factor_authentication/models/two_factor_authenticatable.rb b/lib/two_factor_authentication/models/two_factor_authenticatable.rb index 6d73a0f..d23cae8 100644 --- a/lib/two_factor_authentication/models/two_factor_authenticatable.rb +++ b/lib/two_factor_authentication/models/two_factor_authenticatable.rb @@ -101,7 +101,7 @@ module Devise 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 @@ module Devise 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 543059a..f198038 100644 --- a/lib/two_factor_authentication/routes.rb +++ b/lib/two_factor_authentication/routes.rb @@ -4,8 +4,16 @@ module ActionDispatch::Routing 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 - collection { get "resend_code" } + collection { get resend_code_path(mapping), as: "resend_code" } end end + + def resend_code_path(mapping) + Devise.mappings[resource_name(mapping)].path_names[:two_factor_authentication_resend_code] || "resend_code" + end + + def resource_name(mapping) + mapping.class_name.underscore.to_sym + end end end diff --git a/spec/features/two_factor_authenticatable_spec.rb b/spec/features/two_factor_authenticatable_spec.rb index d433d63..bdca322 100644 --- a/spec/features/two_factor_authenticatable_spec.rb +++ b/spec/features/two_factor_authenticatable_spec.rb @@ -12,7 +12,7 @@ feature "User of two factor authentication" do end it 'does not send an SMS before the user has signed in' do - expect(SMSProvider.messages).to be_empty + expect(SmsProvider.messages).to be_empty end it 'sends code via SMS after sign in' do @@ -21,8 +21,8 @@ feature "User of two factor authentication" do expect(page).to have_content 'Enter the code that was sent to you' - expect(SMSProvider.messages.size).to eq(1) - message = SMSProvider.last_message + expect(SmsProvider.messages.size).to eq(1) + message = SmsProvider.last_message expect(message.to).to eq(user.phone_number) expect(message.body).to eq(user.reload.direct_otp) end @@ -33,12 +33,13 @@ feature "User of two factor authentication" do expect(page).to have_content('You are signed in as Marissa') - fill_in 'code', with: SMSProvider.last_message.body + fill_in 'code', with: SmsProvider.last_message.body click_button 'Submit' - within('.flash.notice') do - expect(page).to have_content('Two factor authentication successful.') - end + expect(page).to have_selector( + ".notice", + text: "Two factor authentication successful." + ) expect(current_path).to eq root_path end @@ -67,7 +68,7 @@ feature "User of two factor authentication" do expect(page).to_not have_content("Your Personal Dashboard") - fill_in "code", with: SMSProvider.last_message.body + fill_in "code", with: SmsProvider.last_message.body click_button "Submit" expect(page).to have_content("Your Personal Dashboard") @@ -85,9 +86,7 @@ feature "User of two factor authentication" do fill_in "code", with: "incorrect#{rand(100)}" click_button "Submit" - within(".flash.alert") do - expect(page).to have_content("Attempt failed") - end + expect(page).to have_selector(".alert", text: "Attempt failed") end expect(page).to have_content("Access completely denied") @@ -154,9 +153,9 @@ feature "User of two factor authentication" do end def sms_sign_in - SMSProvider.messages.clear() + SmsProvider.messages.clear() visit user_two_factor_authentication_path - fill_in 'code', with: SMSProvider.last_message.body + fill_in 'code', with: SmsProvider.last_message.body click_button 'Submit' end 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 6fb4f50..6e58fd1 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 @@ -138,7 +138,7 @@ describe Devise::Models::TwoFactorAuthenticatable do it "returns uri with user's email" do expect(instance.provisioning_uri). - to match(%r{otpauth://totp/houdini@example.com\?secret=\w{32}}) + to match(%r{otpauth://totp/houdini%40example.com\?secret=\w{32}}) end it 'returns uri with issuer option' do diff --git a/spec/rails_app/Rakefile b/spec/rails_app/Rakefile old mode 100644 new mode 100755 index 3645852..7624cc4 --- a/spec/rails_app/Rakefile +++ b/spec/rails_app/Rakefile @@ -1,7 +1,9 @@ #!/usr/bin/env rake +# frozen_string_literal: true + # Add your own tasks in files placed in lib/tasks ending in .rake, # for example lib/tasks/capistrano.rake, and they will automatically be available to Rake. -require File.expand_path('../config/application', __FILE__) +require File.expand_path('config/application', __dir__) Dummy::Application.load_tasks 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 0000000..21a7880 --- /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 8003624..1222279 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/app/models/user.rb b/spec/rails_app/app/models/user.rb index 2093f61..edb1e4f 100644 --- a/spec/rails_app/app/models/user.rb +++ b/spec/rails_app/app/models/user.rb @@ -5,7 +5,7 @@ class User < ActiveRecord::Base has_one_time_password def send_two_factor_authentication_code(code) - SMSProvider.send_message(to: phone_number, body: code) + SmsProvider.send_message(to: phone_number, body: code) end def phone_number diff --git a/spec/rails_app/lib/sms_provider.rb b/spec/rails_app/lib/sms_provider.rb index 363ac10..b70f6e5 100644 --- a/spec/rails_app/lib/sms_provider.rb +++ b/spec/rails_app/lib/sms_provider.rb @@ -1,6 +1,6 @@ require 'ostruct' -class SMSProvider +class SmsProvider Message = Class.new(OpenStruct) class_attribute :messages diff --git a/spec/rails_app/script/rails b/spec/rails_app/script/rails index f8da2cf..8c3e6bb 100755 --- a/spec/rails_app/script/rails +++ b/spec/rails_app/script/rails @@ -1,6 +1,9 @@ #!/usr/bin/env ruby -# This command will automatically be run when you run "rails" with Rails 3 gems installed from the root of your application. +# frozen_string_literal: true -APP_PATH = File.expand_path('../../config/application', __FILE__) -require File.expand_path('../../config/boot', __FILE__) +# This command will automatically be run when you run "rails" with Rails 3 +# gems installed from the root of your application. + +APP_PATH = File.expand_path('../config/application', __dir__) +require File.expand_path('../config/boot', __dir__) require 'rails/commands' diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6370433..80bacae 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -24,3 +24,4 @@ RSpec.configure do |config| end Dir["#{Dir.pwd}/spec/support/**/*.rb"].each {|f| require f} +Dir["#{Dir.pwd}/spec/rails_app/lib/*.rb"].each {|f| require f} diff --git a/spec/support/sms_provider.rb b/spec/support/sms_provider.rb index 95556e1..eb37280 100644 --- a/spec/support/sms_provider.rb +++ b/spec/support/sms_provider.rb @@ -1,5 +1,5 @@ RSpec.configure do |c| c.before(:each) do - SMSProvider.messages.clear + SmsProvider.messages.clear end end