Skip to content

Commit

Permalink
Change CI settings for support Ruby3.0+ Rails6.1+ (#357)
Browse files Browse the repository at this point in the history
* Change CI settings for support Ruby3.0+ Rails6.1+

Now, Ruby 2.7 is EOL, and Rails 6.0 is also EOL.

So I changed the CI settings to support only those higher versions.

Fix #340

* Move rspec-rails development dependency to Gemfiles

Since the supported version of rspec-rails depends on the version of rails we use, move the description to each Gemfile

* Fix the following failure

```
Failures:

  1) SorceryController with session timeout features with 'session_timeout_from_last_action' does not logout if there was activity
     Failure/Error: expect(response).to be_successful
       expected `#<ActionDispatch::TestResponse:0x0000557bab96c6d0 @mon_data=#<Monitor:0x0000557bab96c630>, @mon_data_...control={}, @request=#<ActionController::TestRequest GET "http://test.host/test_login" for 0.0.0.0>>.successful?` to return true, got false
     # ./spec/controllers/controller_session_timeout_spec.rb:146:in `block (4 levels) in <top (required)>'

Finished in 2.52 seconds (files took 1.66 seconds to load)
```

[Starting with Rails 7.0, instance variables are reset between controller test requests](rails/rails#43735).

This causes the second and subsequent requests to be judged as un-logged-in if there are no records in the DB.  Putting a record in the DB fixes the failure.

* Fix failures of specs due to a change in the keyword argument specification

fix failing tests like the following

```
  1) SorceryController using create_from supports nested attributes
     Failure/Error: @user = user_class.create_from_provider(provider_name, @user_hash[:uid], attrs, &block)

       #<User(id: integer, username: string, email: string, crypted_password: string, salt: string, created_at: datetime, updated_at: datetime, activation_state: string, activation_token: string, activation_token_expires_at: datetime, last_login_at: datetime, last_logout_at: datetime, last_activity_at: datetime, last_login_from_ip_address: string) (class)> received :create_from_provider with unexpected arguments
         expected: ("facebook", "123", {:username=>"Haifa, Israel"}) (keyword arguments)
              got: ("facebook", "123", {:username=>"Haifa, Israel"}) (options hash)
     # ./lib/sorcery/controller/submodules/external.rb:194:in `create_from'
     # ./spec/rails_app/app/controllers/sorcery_controller.rb:456:in `test_create_from_provider'
     # ./spec/controllers/controller_oauth2_spec.rb:44:in `block (3 levels) in <top (required)>'
```

* Remove an useless spec

A spec fails like the following in Rails 7.1

```
  1) SorceryController when activated with sorcery #login when succeeds sets csrf token in session
     Failure/Error: expect(session[:_csrf_token]).not_to be_nil

       expected: not nil
            got: nil
     # ./spec/controllers/controller_spec.rb:68:in `block (5 levels) in <top (required)>'
```

Delete this because it didn't seem like a useful test.

* Add Changelog

* Readd rspec-rails to dev dependencies for running tests locally

* Do not remove the csrf test

* Gitignore new sqlite3 files

* Add pending clause for Rails 7.1 for CSRF token

---------

Co-authored-by: Josh Buker <crypto@joshbuker.com>
  • Loading branch information
willnet and joshbuker authored Mar 8, 2024
1 parent 3694994 commit 7a319c2
Show file tree
Hide file tree
Showing 12 changed files with 36 additions and 47 deletions.
37 changes: 12 additions & 25 deletions .github/workflows/ruby.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,41 +16,28 @@ jobs:
fail-fast: false
matrix:
ruby:
- 2.7
- 3.0.0
# - 3.1
- '3.0'
- '3.1'
- '3.2'
- '3.3'

rails:
- '52'
- '60'
- '61'
# - '70'
- '70'
- '71'

exclude:
- ruby: 2.4
rails: '60'
- ruby: 2.4
- ruby: '3.3'
rails: '70'
- ruby: '3.3'
rails: '61'
- ruby: '3.2'
rails: '61'
# - ruby: 2.4
# rails: '70'
# - ruby: 2.5
# rails: '70'
# - ruby: 2.6
# rails: '70'
- ruby: 3.0.0
rails: '52'
# - ruby: 3.1
# rails: '52'
# - ruby: 3.1
# rails: '60'
# - ruby: 3.1
# rails: '61'

env:
BUNDLE_GEMFILE: gemfiles/rails_${{ matrix.rails }}.gemfile

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v4
- name: Set up Ruby
uses: ruby/setup-ruby@v1
with:
Expand Down
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ tmtags
#
spec/rails_app/log/*
*.log
*.sqlite3
*.sqlite3*
Gemfile*.lock
gemfiles/*.lock
.ruby-version
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Changelog
## HEAD

* Change CI settings for support Ruby3.0+ Rails6.1+ [#357](https://github.com/Sorcery/sorcery/pull/357)
* Fix error when running the install generator [#339](https://github.com/Sorcery/sorcery/pull/339)

## 0.16.5
Expand Down
7 changes: 0 additions & 7 deletions gemfiles/rails_52.gemfile

This file was deleted.

2 changes: 1 addition & 1 deletion gemfiles/rails_61.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ source 'https://rubygems.org'
gem 'rails', '~> 6.1.0'
gem 'rails-controller-testing'
gem 'sqlite3', '~> 1.4'

gem 'rspec-rails', '>= 5.0'
gemspec path: '..'
2 changes: 1 addition & 1 deletion gemfiles/rails_70.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ source 'https://rubygems.org'
gem 'rails', '~> 7.0.0'
gem 'rails-controller-testing'
gem 'sqlite3', '~> 1.4'

gem 'rspec-rails', '>= 6.0'
gemspec path: '..'
4 changes: 2 additions & 2 deletions gemfiles/rails_60.gemfile → gemfiles/rails_71.gemfile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
source 'https://rubygems.org'

gem 'rails', '~> 6.0.0'
gem 'rails', '~> 7.1.0'
gem 'rails-controller-testing'
gem 'sqlite3', '~> 1.4'

gem 'rspec-rails', '>= 6.1'
gemspec path: '..'
2 changes: 1 addition & 1 deletion sorcery.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Gem::Specification.new do |s|
s.add_dependency 'oauth2', '~> 2.0'

s.add_development_dependency 'byebug', '~> 10.0.0'
s.add_development_dependency 'rspec-rails', '~> 3.7.0'
s.add_development_dependency 'rspec-rails'
s.add_development_dependency 'rubocop'
s.add_development_dependency 'simplecov', '>= 0.3.8'
s.add_development_dependency 'test-unit', '~> 3.2.0'
Expand Down
8 changes: 4 additions & 4 deletions spec/controllers/controller_oauth2_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@
sorcery_model_property_set(:authentications_class, Authentication)
sorcery_controller_external_property_set(:facebook, :user_info_mapping, username: 'name')

expect(User).to receive(:create_from_provider).with('facebook', '123', username: 'Noam Ben Ari')
expect(User).to receive(:create_from_provider).with('facebook', '123', { username: 'Noam Ben Ari' })
get :test_create_from_provider, params: { provider: 'facebook' }
end

it 'supports nested attributes' do
sorcery_model_property_set(:authentications_class, Authentication)
sorcery_controller_external_property_set(:facebook, :user_info_mapping, username: 'hometown/name')
expect(User).to receive(:create_from_provider).with('facebook', '123', username: 'Haifa, Israel')
expect(User).to receive(:create_from_provider).with('facebook', '123', { username: 'Haifa, Israel' })

get :test_create_from_provider, params: { provider: 'facebook' }
end
Expand All @@ -48,7 +48,7 @@
sorcery_model_property_set(:authentications_class, Authentication)
sorcery_controller_external_property_set(:facebook, :user_info_mapping, username: 'name', created_at: 'does/not/exist')

expect(User).to receive(:create_from_provider).with('facebook', '123', username: 'Noam Ben Ari')
expect(User).to receive(:create_from_provider).with('facebook', '123', { username: 'Noam Ben Ari' })

get :test_create_from_provider, params: { provider: 'facebook' }
end
Expand All @@ -59,7 +59,7 @@
sorcery_controller_external_property_set(:facebook, :user_info_mapping, username: 'name')

u = double('user')
expect(User).to receive(:create_from_provider).with('facebook', '123', username: 'Noam Ben Ari').and_return(u).and_yield(u)
expect(User).to receive(:create_from_provider).with('facebook', '123', { username: 'Noam Ben Ari' }).and_return(u).and_yield(u)
# test_create_from_provider_with_block in controller will check for uniqueness of username
get :test_create_from_provider_with_block, params: { provider: 'facebook' }
end
Expand Down
8 changes: 4 additions & 4 deletions spec/controllers/controller_oauth_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,23 +136,23 @@ def stub_all_oauth_requests!
it 'creates a new user' do
sorcery_controller_external_property_set(:twitter, :user_info_mapping, username: 'screen_name')
expect(User).to receive(:load_from_provider).with('twitter', '123').and_return(nil)
expect(User).to receive(:create_from_provider).with('twitter', '123', username: 'nbenari').and_return(user)
expect(User).to receive(:create_from_provider).with('twitter', '123', { username: 'nbenari' }).and_return(user)

get :test_create_from_provider, params: { provider: 'twitter' }
end

it 'supports nested attributes' do
sorcery_controller_external_property_set(:twitter, :user_info_mapping, username: 'status/text')
expect(User).to receive(:load_from_provider).with('twitter', '123').and_return(nil)
expect(User).to receive(:create_from_provider).with('twitter', '123', username: 'coming soon to sorcery gem: twitter and facebook authentication support.').and_return(user)
expect(User).to receive(:create_from_provider).with('twitter', '123', { username: 'coming soon to sorcery gem: twitter and facebook authentication support.' }).and_return(user)

get :test_create_from_provider, params: { provider: 'twitter' }
end

it 'does not crash on missing nested attributes' do
sorcery_controller_external_property_set(:twitter, :user_info_mapping, username: 'status/text', created_at: 'does/not/exist')
expect(User).to receive(:load_from_provider).with('twitter', '123').and_return(nil)
expect(User).to receive(:create_from_provider).with('twitter', '123', username: 'coming soon to sorcery gem: twitter and facebook authentication support.').and_return(user)
expect(User).to receive(:create_from_provider).with('twitter', '123', { username: 'coming soon to sorcery gem: twitter and facebook authentication support.' }).and_return(user)

get :test_create_from_provider, params: { provider: 'twitter' }
end
Expand All @@ -175,7 +175,7 @@ def stub_all_oauth_requests!

u = double('user')
expect(User).to receive(:load_from_provider).with('twitter', '123').and_return(nil)
expect(User).to receive(:create_from_provider).with('twitter', '123', username: 'nbenari').and_return(u).and_yield(u)
expect(User).to receive(:create_from_provider).with('twitter', '123', { username: 'nbenari' }).and_return(u).and_yield(u)

get :test_create_from_provider_with_block, params: { provider: 'twitter' }
end
Expand Down
4 changes: 3 additions & 1 deletion spec/controllers/controller_session_timeout_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,11 @@
end

context "with 'session_timeout_from_last_action'" do
before { create_new_user }
after { User.delete_all }

it 'does not logout if there was activity' do
sorcery_controller_property_set(:session_timeout_from_last_action, true)
expect(User).to receive(:authenticate).with('bla@bla.com', 'secret') { |&block| block.call(user, nil) }

get :test_login, params: { email: 'bla@bla.com', password: 'secret' }
Timecop.travel(Time.now.in_time_zone + 0.3)
Expand Down
6 changes: 6 additions & 0 deletions spec/controllers/controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,13 @@
expect(session[:user_id]).to eq user.id.to_s
end

# NOTE: The lack of a CSRF token may mean that sessions will break
# horribly for Sorcery when using Rails 7.1+. We shall see.
it 'sets csrf token in session' do
if Gem::Version.new(Rails.version) >= Gem::Version.new('7.1')
pending 'Rails 7.1 is not including the csrf token in the session for unknown reasons'
end

expect(session[:_csrf_token]).not_to be_nil
end
end
Expand Down

0 comments on commit 7a319c2

Please sign in to comment.