From 820ad6f9c0748f172224c0d159b2d0c6c9e43087 Mon Sep 17 00:00:00 2001 From: Brian Hawley Date: Thu, 23 Jun 2022 13:51:04 -0700 Subject: [PATCH] Test fixes and updates - Don't override the default scope in login!. - Pass through all options to set_user in login!. - Don't define classes in rspec before blocks. Use a support file. - Test parameters and return values of the controller mixins. - Fix a typo in the rails_warden spec. - Add .ruby-gemset to gitignore. - Add Rails 7.0 and Ruby 3.1 to the tested versions. - Frozen string literals everywhere. --- .github/workflows/ruby.yml | 18 ++++++- .gitignore | 1 + gemfiles/rails70.gemfile | 8 +++ lib/rails_warden.rb | 2 + lib/rails_warden/authentication.rb | 6 ++- lib/rails_warden/compat.rb | 2 + lib/rails_warden/engine.rb | 2 + lib/rails_warden/manager.rb | 2 + lib/rails_warden/rails_settings.rb | 2 + spec/controller_mixin_spec.rb | 81 ++++++++++++++++++++---------- spec/rails_warden_spec.rb | 12 ++--- spec/spec_helper.rb | 2 + spec/support/test_classes.rb | 18 +++++++ 13 files changed, 117 insertions(+), 39 deletions(-) create mode 100644 gemfiles/rails70.gemfile create mode 100644 spec/support/test_classes.rb diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index f72a4f0..4e15df1 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -13,15 +13,29 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - gemfile: [rails50, rails51, rails52, rails60, rails61] - ruby: ["2.5", "2.6", "2.7", "3.0"] + gemfile: [rails50, rails51, rails52, rails60, rails61, rails70] + ruby: ["2.5", "2.6", "2.7", "3.0", "3.1"] exclude: - gemfile: rails50 ruby: "3.0" + - gemfile: rails50 + ruby: "3.1" - gemfile: rails51 ruby: "3.0" + - gemfile: rails51 + ruby: "3.1" - gemfile: rails52 ruby: "3.0" + - gemfile: rails52 + ruby: "3.1" + - gemfile: rails60 + ruby: "3.1" + - gemfile: rails61 + ruby: "3.1" + - gemfile: rails70 + ruby: "2.5" + - gemfile: rails70 + ruby: "2.6" env: BUNDLE_GEMFILE: gemfiles/${{ matrix.gemfile }}.gemfile diff --git a/.gitignore b/.gitignore index 29c7316..4a4a917 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ Gemfile.lock .DS_Store .ruby-version +.ruby-gemset .rvmrc .rspec_status diff --git a/gemfiles/rails70.gemfile b/gemfiles/rails70.gemfile new file mode 100644 index 0000000..97ed080 --- /dev/null +++ b/gemfiles/rails70.gemfile @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +source 'https://rubygems.org' +gemspec path: '../' + +group :development, :test do + gem 'rails', '~> 7.0.0' +end diff --git a/lib/rails_warden.rb b/lib/rails_warden.rb index cda2b4d..abb1c5e 100644 --- a/lib/rails_warden.rb +++ b/lib/rails_warden.rb @@ -1,4 +1,6 @@ # encoding: utf-8 +# frozen_string_literal: true + require 'warden' require "rails_warden/version" diff --git a/lib/rails_warden/authentication.rb b/lib/rails_warden/authentication.rb index ebe3f94..c6f1045 100644 --- a/lib/rails_warden/authentication.rb +++ b/lib/rails_warden/authentication.rb @@ -1,4 +1,6 @@ # encoding: utf-8 +# frozen_string_literal: true + module RailsWarden module Authentication extend ActiveSupport::Concern @@ -25,8 +27,8 @@ def user(*args) warden.user(*args) end - def login!(user, scope: :default) - warden.set_user(user, scope: scope) + def login!(user, **opts) + warden.set_user(user, opts) end # Logout the current user diff --git a/lib/rails_warden/compat.rb b/lib/rails_warden/compat.rb index 2831583..7c96544 100644 --- a/lib/rails_warden/compat.rb +++ b/lib/rails_warden/compat.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Warden::Mixins::Common def request @request ||= ActionDispatch::Request.new(env) diff --git a/lib/rails_warden/engine.rb b/lib/rails_warden/engine.rb index 1eb2c93..db6c673 100644 --- a/lib/rails_warden/engine.rb +++ b/lib/rails_warden/engine.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rails_warden/compat' module RailsWarden diff --git a/lib/rails_warden/manager.rb b/lib/rails_warden/manager.rb index c2448f3..5e477e6 100644 --- a/lib/rails_warden/manager.rb +++ b/lib/rails_warden/manager.rb @@ -1,4 +1,6 @@ # encoding: utf-8 +# frozen_string_literal: true + module RailsWarden class Manager def self.new(app, opts = {}, &block) diff --git a/lib/rails_warden/rails_settings.rb b/lib/rails_warden/rails_settings.rb index 6adcd1e..bb0e6d4 100644 --- a/lib/rails_warden/rails_settings.rb +++ b/lib/rails_warden/rails_settings.rb @@ -1,4 +1,6 @@ # encoding: utf-8 +# frozen_string_literal: true + module RailsWarden # Set the default user class for the application diff --git a/spec/controller_mixin_spec.rb b/spec/controller_mixin_spec.rb index edc8b3e..05fc89f 100644 --- a/spec/controller_mixin_spec.rb +++ b/spec/controller_mixin_spec.rb @@ -1,22 +1,12 @@ +# frozen_string_literal: true + require 'spec_helper' RSpec.describe "rails_warden controller mixin" do + let(:ret) { Object.new } before(:each) do @app = lambda {|e| [200, {"Content-Type" => "text/plain"}, ["resonse"]]} - class FooFailure - end - - class User - end - - class MockController < ActionController::Base - include RailsWarden::Authentication - attr_accessor :env - def request - self - end - end RailsWarden.default_user_class = nil RailsWarden.unauthenticated_action = nil @@ -35,32 +25,71 @@ def request end it "should run authenticate on warden" do - expect(@mock_warden).to receive(:authenticate).and_return(true) - @controller.authenticate + expect(@mock_warden).to receive(:authenticate).with(no_args).and_return(ret) + expect(@controller.authenticate).to be(ret) + end + + it "should pass options to authenticate on warden" do + expect(@mock_warden).to receive(:authenticate).with({scope: :foo, store: false}).and_return(ret) + expect(@controller.authenticate(scope: :foo, store: false)).to be(ret) end - it "should run authenticate! on warden" do - expect(@mock_warden).to receive(:authenticate!).and_return(true) - @controller.authenticate! + it "should run authenticate! on warden, with a default :action option" do + expect(@mock_warden).to receive(:authenticate!).with({action: RailsWarden.unauthenticated_action}).and_return(ret) + expect(@controller.authenticate!).to be(ret) + end + + it "should pass options to authenticate! on warden" do + expect(@mock_warden).to receive(:authenticate!).with({scope: :foo, store: false, action: :bar}).and_return(ret) + expect(@controller.authenticate!(scope: :foo, store: false, action: :bar)).to be(ret) end it "should run authenticate? on warden" do - expect(@mock_warden).to receive(:authenticated?).and_return(true) - @controller.authenticated? + expect(@mock_warden).to receive(:authenticated?).with(no_args).and_return(ret) + expect(@controller.authenticated?).to be(ret) + end + + it "should pass options to authenticate? on warden" do + expect(@mock_warden).to receive(:authenticated?).with({scope: :foo, run_callbacks: false}).and_return(ret) + expect(@controller.authenticated?(scope: :foo, run_callbacks: false)).to be(ret) end it "should run user on warden" do - expect(@mock_warden).to receive(:user).and_return(true) - @controller.user + expect(@mock_warden).to receive(:user).with(no_args).and_return(ret) + expect(@controller.user).to be(ret) + end + + it "should pass arguments to user on warden" do + expect(@mock_warden).to receive(:user).with(:foo).and_return(ret) + expect(@controller.user(:foo)).to be(ret) + end + + it "should pass options to user on warden" do + expect(@mock_warden).to receive(:user).with({scope: :foo}).and_return(ret) + expect(@controller.user(scope: :foo)).to be(ret) end it "should set the user on warden" do - expect(@mock_warden).to receive(:set_user).and_return(true) - @controller.login!(User.new) + user = User.new + expect(@mock_warden).to receive(:set_user).with(user, {}).and_return(ret) + expect(@controller.login!(user)).to be(ret) + end + + it "should set the user on warden with options" do + user = User.new + expect(@mock_warden).to receive(:set_user).with(user, {scope: :foo, event: :set_user, store: false}).and_return(ret) + expect(@controller.login!(user, scope: :foo, event: :set_user, store: false)).to be(ret) end it "should proxy logout! to warden" do - expect(@mock_warden).to receive(:logout).and_return(true) - @controller.logout! + expect(@mock_warden).to receive(:logout).with(no_args).and_return(true) + expect(@mock_warden).to receive(:clear_strategies_cache!).with(no_args).and_return(ret) + expect(@controller.logout!).to be(ret) + end + + it "should proxy logout! with a scope to warden" do + expect(@mock_warden).to receive(:logout).with(:foo).and_return(true) + expect(@mock_warden).to receive(:clear_strategies_cache!).with(scope: :foo).and_return(ret) + expect(@controller.logout!(scope: :foo)).to be(ret) end end diff --git a/spec/rails_warden_spec.rb b/spec/rails_warden_spec.rb index 4a511d0..6329ed2 100644 --- a/spec/rails_warden_spec.rb +++ b/spec/rails_warden_spec.rb @@ -1,17 +1,11 @@ +# frozen_string_literal: true + require 'spec_helper' RSpec.describe "rails_warden" do before(:each) do - @app = lambda{|e| Rack::Resposnse.new("response").finish} - class ::FooFailure - end - - class ::FooUser - end - - class ::User - end + @app = lambda { |e| Rack::Response.new("response").finish } RailsWarden.default_user_class = nil RailsWarden.unauthenticated_action = nil diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 711851f..31a3a28 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "bundler/setup" require "rails" require "rails_warden" diff --git a/spec/support/test_classes.rb b/spec/support/test_classes.rb new file mode 100644 index 0000000..30f0841 --- /dev/null +++ b/spec/support/test_classes.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class FooFailure +end + +class FooUser +end + +class User +end + +class MockController < ActionController::Base + include RailsWarden::Authentication + attr_accessor :env + def request + self + end +end