Merge pull request #4 from identification-io/feature/rails5_upgrade

Rails 5.x support (Dropped Rails 4.2 support)
rbCAS · Nov 16, 2017 · f473af1 · f473af1
2 parents ce179ee + 3cc12e8
commit f473af1
Show file tree

Hide file tree

Showing 50 changed files with 407 additions and 288 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,28 +1,24 @@
-# See http://help.github.com/ignore-files/ for more about ignoring files.
-#
-# If you find yourself ignoring temporary files generated by your text editor
-# or operating system, you probably want to add a global ignore instead:
-#   git config --global core.excludesfile ~/.gitignore_global
-
-# Ignore bundler config
-/.bundle
-
-# Ignore the default SQLite database.
-/db/*.sqlite3
-
-# Ignore all logfiles and tempfiles.
-/log/*.log
-/tmp
+*.gem
+*.rbc
+.bundle
+.config
+.yardoc
 .rails_generators~
-
-/coverage
-
-/pkg
-
-# http://yehudakatz.com/2010/12/16/clarifying-the-roles-of-the-gemspec-and-gemfile/
-/Gemfile.lock
-
-# Dummy application crap
-/spec/dummy/log/*.log
-/spec/dummy/tmp
-/spec/dummy/db/*.sqlite3
+gemfiles/vendor
+Gemfile.lock
+InstalledFiles
+_yardoc
+coverage
+doc/
+lib/bundler/man
+pkg
+rdoc
+spec/reports
+test/tmp
+test/version_tmp
+tmp
+*.lock
+.idea/
+.ruby-version
+*.sqlite*
+*.log
diff --git a/.travis.yml b/.travis.yml
@@ -1,9 +1,18 @@
 language: ruby
 rvm:
-- 2.1.0
 - 2.2.2
 - 2.4.0
 - 2.4.1
+bundler_args: --without development
+gemfile:
+  - gemfiles/rails_4.2.gemfile
+  - gemfiles/rails_5.0.gemfile
+  - gemfiles/rails_5.1.gemfile
+# matrix:
+#   allow_failures:
+#     - rvm: 2.4.1
+#       # gemfile: gemfiles/rails_5.0.gemfile
+#       # gemfile: gemfiles/rails_5.1.gemfile
 notifications:
   hipchat:
     rooms:

diff --git a/Appraisals b/Appraisals
@@ -0,0 +1,11 @@
+appraise 'rails-5.0' do
+  gem 'activerecord', '~> 5.0.0'
+  gem 'rails-controller-testing'
+  gem 'rspec-rails', '>= 3.5'
+end
+
+appraise 'rails-5.1' do
+  gem 'activerecord', '~> 5.1.0'
+  gem 'rails-controller-testing'
+  gem 'rspec-rails', '>= 3.5'
+end
diff --git a/Gemfile b/Gemfile
@@ -1,2 +1,18 @@
 source 'https://rubygems.org'
+
+group :test do
+  gem 'appraisal', '>= 2.1'
+  gem 'capybara', '>= 2.1'
+  gem 'coveralls', '>= 0.7'
+  gem 'factory_bot', '>= 4.1'
+  gem 'rake', '>= 10.0'
+  gem 'rspec-its', '>= 1.0'
+  gem 'webmock', '>= 1.9'
+end
+
+# Specify your gem's dependencies in groupify.gemspec
 gemspec
+
+platforms :ruby do
+  gem 'sqlite3', '>= 1.3'
+end
diff --git a/README.md b/README.md
@@ -8,6 +8,17 @@ It currently supports [CAS 1.0 and CAS 2.0](http://apereo.github.io/cas) as well
 
 Please check our [documentation](http://casino.rbcas.com/) for setup and configuration instructions.
 
+## Test Suite
+
+Run the RSpec test suite by installing the `appraisal` gem and dependencies:
+
+    $ gem install appraisal
+    $ appraisal install
+
+And then running tests using `appraisal`:
+
+    $ appraisal rake
+
 ## License
 
 CASino is released under the [MIT License](http://www.opensource.org/licenses/MIT). See LICENSE.txt for further details.
diff --git a/app/controllers/casino/application_controller.rb b/app/controllers/casino/application_controller.rb
@@ -5,6 +5,10 @@ class CASino::ApplicationController < ::ApplicationController
 
   layout 'application'
 
+  rescue_from ActionController::UnknownFormat do
+    head :not_acceptable
+  end
+
   unless Rails.env.development?
     rescue_from ActionView::MissingTemplate, with: :missing_template
   end

diff --git a/app/controllers/casino/controller_concern/ticket_validator.rb b/app/controllers/casino/controller_concern/ticket_validator.rb
@@ -7,7 +7,7 @@ def validate_ticket(ticket)
     validation_result = validate_ticket_for_service(ticket, params[:service], renew: params[:renew])
     if validation_result.success?
       options = { ticket: ticket }
-      options[:proxy_granting_ticket] = acquire_proxy_granting_ticket(params[:pgtUrl], ticket) unless params[:pgtUrl].nil?
+      options[:proxy_granting_ticket] = acquire_proxy_granting_ticket(params[:pgtUrl], ticket) if params[:pgtUrl].present?
       build_ticket_validation_response(true, options)
     else
       build_ticket_validation_response(false,
@@ -21,7 +21,7 @@ def build_ticket_validation_response(success, options = {})
   end
 
   def ensure_service_ticket_parameters_present
-    if params[:ticket].nil? || params[:service].nil?
+    if params[:ticket].blank? || params[:service].blank?
       build_ticket_validation_response(false,
                                        error_code: 'INVALID_REQUEST',
                                        error_message: '"ticket" and "service" parameters are both required')

diff --git a/app/controllers/casino/proxy_tickets_controller.rb b/app/controllers/casino/proxy_tickets_controller.rb
@@ -31,7 +31,7 @@ def build_proxy_response(success, options = {})
   end
 
   def ensure_proxy_parameters_present
-    if params[:pgt].nil? || params[:targetService].nil?
+    if params[:pgt].blank? || params[:targetService].blank?
       build_proxy_response(false,
                            error_code: 'INVALID_REQUEST',
                            error_message: '"pgt" and "targetService" parameters are both required')

diff --git a/app/controllers/casino/sessions_controller.rb b/app/controllers/casino/sessions_controller.rb
@@ -16,8 +16,8 @@ def index
 
   def new
     tgt = current_ticket_granting_ticket
-    return handle_signed_in(tgt) unless params[:renew] || tgt.nil?
-    redirect_to(params[:service]) if params[:gateway] && params[:service].present?
+    return handle_signed_in(tgt) unless params[:renew].present? || tgt.nil?
+    redirect_to(params[:service]) if params[:gateway].present? && params[:service].present?
   end
 
   def create
@@ -41,7 +41,7 @@ def destroy_others
       .ticket_granting_tickets
       .where('id != ?', current_ticket_granting_ticket.id)
       .destroy_all if signed_in?
-    redirect_to params[:service] || sessions_path
+    redirect_to params[:service].present? ? params[:service] : sessions_path
   end
 
   def logout

diff --git a/app/helpers/casino/sessions_helper.rb b/app/helpers/casino/sessions_helper.rb
@@ -26,7 +26,7 @@ def current_user
   def current_authenticator_context
     CASino.config.authenticator_context_builder.call(params, request)
   end
-  
+
   def ensure_signed_in
     redirect_to login_path unless signed_in?
   end
@@ -87,12 +87,12 @@ def handle_signed_in(tgt, options = {})
   end
 
   def handle_signed_in_with_service(tgt, options)
-    if !service_allowed?(params[:service])
-      @service = params[:service]
-      render 'casino/sessions/service_not_allowed', status: 403
-    else
+    if service_allowed?(params[:service])
       url = acquire_service_ticket(tgt, params[:service], options).service_with_ticket_url
       redirect_to url, status: :see_other
+    else
+      @service = params[:service]
+      render 'casino/sessions/service_not_allowed', status: 403
     end
   end
 end
diff --git a/app/models/casino/application_record.rb b/app/models/casino/application_record.rb
@@ -0,0 +1,3 @@
+class CASino::ApplicationRecord < ActiveRecord::Base
+  self.abstract_class = true
+end
diff --git a/app/models/casino/auth_token_ticket.rb b/app/models/casino/auth_token_ticket.rb
@@ -1,15 +1,14 @@
-class CASino::AuthTokenTicket < ActiveRecord::Base
+class CASino::AuthTokenTicket < CASino::ApplicationRecord
   include CASino::ModelConcern::Ticket
   include CASino::ModelConcern::ConsumableTicket
 
   self.ticket_prefix = 'ATT'.freeze
 
   def self.cleanup
-    delete_all(['created_at < ?', CASino.config.auth_token_ticket[:lifetime].seconds.ago])
+    where(['created_at < ?', CASino.config.auth_token_ticket[:lifetime].seconds.ago]).delete_all
   end
 
   def expired?
-    (Time.now - (self.created_at || Time.now)) > CASino.config.auth_token_ticket[:lifetime].seconds
+    (Time.now - (created_at || Time.now)) > CASino.config.auth_token_ticket[:lifetime].seconds
   end
-
 end
diff --git a/app/models/casino/login_attempt.rb b/app/models/casino/login_attempt.rb
@@ -1,4 +1,4 @@
-class CASino::LoginAttempt < ActiveRecord::Base
+class CASino::LoginAttempt < CASino::ApplicationRecord
   include CASino::ModelConcern::BrowserInfo
 
   belongs_to :user

diff --git a/app/models/casino/login_ticket.rb b/app/models/casino/login_ticket.rb
@@ -1,14 +1,14 @@
-class CASino::LoginTicket < ActiveRecord::Base
+class CASino::LoginTicket < CASino::ApplicationRecord
   include CASino::ModelConcern::Ticket
   include CASino::ModelConcern::ConsumableTicket
 
   self.ticket_prefix = 'LT'.freeze
 
   def self.cleanup
-    delete_all(['created_at < ?', CASino.config.login_ticket[:lifetime].seconds.ago])
+    where(['created_at < ?', CASino.config.login_ticket[:lifetime].seconds.ago]).delete_all
   end
 
   def expired?
-    (Time.now - (self.created_at || Time.now)) > CASino.config.login_ticket[:lifetime].seconds
+    (Time.now - (created_at || Time.now)) > CASino.config.login_ticket[:lifetime].seconds
   end
 end
diff --git a/app/models/casino/proxy_granting_ticket.rb b/app/models/casino/proxy_granting_ticket.rb
@@ -1,5 +1,5 @@
 
-class CASino::ProxyGrantingTicket < ActiveRecord::Base
+class CASino::ProxyGrantingTicket < CASino::ApplicationRecord
   include CASino::ModelConcern::Ticket
 
   self.ticket_prefix = 'PGT'.freeze

diff --git a/app/models/casino/proxy_ticket.rb b/app/models/casino/proxy_ticket.rb
@@ -1,6 +1,6 @@
 require 'addressable/uri'
 
-class CASino::ProxyTicket < ActiveRecord::Base
+class CASino::ProxyTicket < CASino::ApplicationRecord
   include CASino::ModelConcern::Ticket
 
   self.ticket_prefix = 'PT'.freeze
@@ -10,11 +10,11 @@ class CASino::ProxyTicket < ActiveRecord::Base
   has_many :proxy_granting_tickets, as: :granter, dependent: :destroy
 
   def self.cleanup_unconsumed
-    self.destroy_all(['created_at < ? AND consumed = ?', CASino.config.proxy_ticket[:lifetime_unconsumed].seconds.ago, false])
+    where(['created_at < ? AND consumed = ?', CASino.config.proxy_ticket[:lifetime_unconsumed].seconds.ago, false]).destroy_all
   end
 
   def self.cleanup_consumed
-    self.destroy_all(['created_at < ? AND consumed = ?', CASino.config.proxy_ticket[:lifetime_consumed].seconds.ago, true])
+    where(['created_at < ? AND consumed = ?', CASino.config.proxy_ticket[:lifetime_consumed].seconds.ago, true]).destroy_all
   end
 
   def expired?

diff --git a/app/models/casino/service_rule.rb b/app/models/casino/service_rule.rb
@@ -1,5 +1,5 @@
 
-class CASino::ServiceRule < ActiveRecord::Base
+class CASino::ServiceRule < CASino::ApplicationRecord
   validates :name, presence: true
   validates :url, uniqueness: true, presence: true
 

diff --git a/app/models/casino/service_ticket.rb b/app/models/casino/service_ticket.rb
@@ -1,6 +1,6 @@
 require 'addressable/uri'
 
-class CASino::ServiceTicket < ActiveRecord::Base
+class CASino::ServiceTicket < CASino::ApplicationRecord
   include CASino::ModelConcern::Ticket
 
   self.ticket_prefix = 'ST'.freeze
@@ -10,15 +10,15 @@ class CASino::ServiceTicket < ActiveRecord::Base
   has_many :proxy_granting_tickets, as: :granter, dependent: :destroy
 
   def self.cleanup_unconsumed
-    self.delete_all(['created_at < ? AND consumed = ?', CASino.config.service_ticket[:lifetime_unconsumed].seconds.ago, false])
+    where(['created_at < ? AND consumed = ?', CASino.config.service_ticket[:lifetime_unconsumed].seconds.ago, false]).delete_all
   end
 
   def self.cleanup_consumed
-    self.destroy_all(['(ticket_granting_ticket_id IS NULL OR created_at < ?) AND consumed = ?', CASino.config.service_ticket[:lifetime_consumed].seconds.ago, true])
+    where(['(ticket_granting_ticket_id IS NULL OR created_at < ?) AND consumed = ?', CASino.config.service_ticket[:lifetime_consumed].seconds.ago, true]).destroy_all
   end
 
   def self.cleanup_consumed_hard
-    self.delete_all(['created_at < ? AND consumed = ?', (CASino.config.service_ticket[:lifetime_consumed] * 2).seconds.ago, true])
+    where(['created_at < ? AND consumed = ?', (CASino.config.service_ticket[:lifetime_consumed] * 2).seconds.ago, true]).delete_all
   end
 
   def service=(service)
@@ -27,21 +27,22 @@ def service=(service)
   end
 
   def service_with_ticket_url
-    service_uri = Addressable::URI.parse(self.service)
-    service_uri.query_values = (service_uri.query_values(Array) || []) << ['ticket', self.ticket]
+    service_uri = Addressable::URI.parse(service)
+    service_uri.query_values = (service_uri.query_values(Array) || []) << ['ticket', ticket]
     service_uri.to_s
   end
 
   def expired?
     lifetime = if consumed?
-      CASino.config.service_ticket[:lifetime_consumed]
-    else
-      CASino.config.service_ticket[:lifetime_unconsumed]
-    end
-    (Time.now - (self.created_at || Time.now)) > lifetime
+                 CASino.config.service_ticket[:lifetime_consumed]
+               else
+                 CASino.config.service_ticket[:lifetime_unconsumed]
+               end
+    (Time.now - (created_at || Time.now)) > lifetime
   end
 
   private
+
   def send_single_sign_out_notification
     notifier = SingleSignOutNotifier.new(self)
     notifier.notify

diff --git a/app/models/casino/ticket_granting_ticket.rb b/app/models/casino/ticket_granting_ticket.rb
@@ -1,6 +1,6 @@
 require 'user_agent'
 
-class CASino::TicketGrantingTicket < ActiveRecord::Base
+class CASino::TicketGrantingTicket < CASino::ApplicationRecord
   include CASino::ModelConcern::Ticket
   include CASino::ModelConcern::BrowserInfo
 

diff --git a/app/models/casino/two_factor_authenticator.rb b/app/models/casino/two_factor_authenticator.rb
@@ -1,18 +1,18 @@
 
-class CASino::TwoFactorAuthenticator < ActiveRecord::Base
+class CASino::TwoFactorAuthenticator < CASino::ApplicationRecord
   belongs_to :user
 
   scope :active, -> { where(active: true) }
 
   def self.cleanup
-    self.delete_all(['(created_at < ?) AND active = ?', self.lifetime.ago, false])
+    where(['(created_at < ?) AND active = ?', lifetime.ago, false]).delete_all
   end
 
   def self.lifetime
     CASino.config.two_factor_authenticator[:lifetime_inactive].seconds
   end
 
   def expired?
-    !self.active? && (Time.now - (self.created_at || Time.now)) > self.class.lifetime
+    !active? && (Time.now - (created_at || Time.now)) > self.class.lifetime
   end
 end
diff --git a/app/models/casino/user.rb b/app/models/casino/user.rb
@@ -1,5 +1,5 @@
 
-class CASino::User < ActiveRecord::Base
+class CASino::User < CASino::ApplicationRecord
   serialize :extra_attributes, Hash
 
   has_many :ticket_granting_tickets

diff --git a/app/views/casino/sessions/new.html.erb b/app/views/casino/sessions/new.html.erb
@@ -14,7 +14,7 @@
     <div class="form">
       <%= form_tag(login_path, method: :post, id: 'login-form') do %>
         <%= hidden_field_tag :lt, CASino::LoginTicket.create.ticket %>
-        <%= hidden_field_tag :service, params[:service] unless params[:service].nil? %>
+        <%= hidden_field_tag :service, params[:service] unless params[:service].blank? %>
         <%= label_tag :username, t('login.label_username') %>
         <%= text_field_tag :username, params[:username], autofocus:true %>
         <%= label_tag :password, t('login.label_password') %>
@@ -28,4 +28,3 @@
   </div>
   <%= render 'footer' %>
 </div>
-