update omniauth-cul to 0.3.0

Author: goldsmithb

Gemfile

@@ -29,8 +29,7 @@ gem 'grape-swagger', '~> 2.0.0'
 gem 'mustermann', '~> 2.0'
 gem 'om', '3.1.1'
 gem 'omniauth', '>= 2.1'
-gem 'omniauth-cul'
-gem 'omniauth-rails_csrf_protection', '~> 1.0'
+gem 'omniauth-cul', '~> 0.3.0'
 
 gem 'http'
 gem 'jbuilder', '~> 2.13.0'

Gemfile.lock

@@ -387,12 +387,9 @@ GEM
       hashie (>= 3.4.6)
       rack (>= 2.2.3)
       rack-protection
-    omniauth-cul (0.2.0)
+    omniauth-cul (0.3.0)
       devise (>= 4.9)
       omniauth (>= 2.0)
-    omniauth-rails_csrf_protection (1.0.2)
-      actionpack (>= 4.2)
-      omniauth (~> 2.0)
     orm_adapter (0.5.0)
     ostruct (0.6.3)
     parallel (1.27.0)
@@ -753,8 +750,7 @@ DEPENDENCIES
   okcomputer
   om (= 3.1.1)
   omniauth (>= 2.1)
-  omniauth-cul
-  omniauth-rails_csrf_protection (~> 1.0)
+  omniauth-cul (~> 0.3.0)
   premailer (~> 1.27.0)
   premailer-rails
   puma (~> 5.2)

app/controllers/users/omniauth_callbacks_controller.rb

@@ -3,39 +3,43 @@
 require 'omniauth/cul'
 
 class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController
-  # Adding the line below so that if the auth endpoint POSTs to our cas endpoint, it won't
-  # be rejected by authenticity token verification.
   # See https://github.com/omniauth/omniauth/wiki/FAQ#rails-session-is-clobbered-after-callback-on-developer-strategy
-  skip_before_action :verify_authenticity_token, only: :cas
+  # The CAS login redirect to the columbia_cas callback endpoint AND the developer form submission to the
+  # developer_uid callback do not send authenticity tokens, so we'll skip token verification for these actions.
+  skip_before_action :verify_authenticity_token, only: [:columbia_cas, :developer_uid]
 
-  def app_cas_callback_endpoint
-    "#{request.base_url}/users/auth/cas/callback"
-  end
+  # POST /users/auth/developer_uid/callback
+  def developer_uid
+    return unless Rails.env.development? # Only allow this action to run in the development environment
 
-  # In local development, use devise's controller action. In deployed env, use CAS server
-  def passthru
-    if Rails.env.development?
-      super
-    else
-      redirect_to Omniauth::Cul::Cas3.passthru_redirect_url(app_cas_callback_endpoint), allow_other_host: true
-    end
-  end
+    uid = params[:uid]
+    user = User.find_by(uid: uid)
 
-  def developer
-    current_user ||= User.find_or_create_by(
-      uid: request.env['omniauth.auth'][:uid], provider: :developer
-    )
+    unless user
+      flash[:alert] = "Login attempt failed.  User #{uid} does not have an account."
+      redirect_to root_path
+      return
+    end
 
-    sign_in_and_redirect current_user, event: :authentication
+    flash[:success] = 'You have succesfully logged in.'
+    sign_in_and_redirect user, event: :authentication # this will throw if user is not activated
   end
 
-  def cas
-    user_id, _affils = Omniauth::Cul::Cas3.validation_callback(request.params['ticket'], app_cas_callback_endpoint)
+  # POST /users/auth/columbia_cas/callback
+  def columbia_cas # rubocop:disable Metrics/AbcSize
+    callback_url = user_columbia_cas_omniauth_callback_url # The columbia_cas callback route in this application
+    uid, _affils = Omniauth::Cul::ColumbiaCas.validation_callback(request.params['ticket'], callback_url)
 
-    user = User.find_by(uid: user_id) || User.create!(
-      uid: user_id,
-      email: "#{user_id}@columbia.edu"
-    )
+    user = User.find_by(uid: uid) || User.create(uid: uid, email: "#{uid}@columbia.edu")
+    flash[:success] = 'You have succesfully logged in.'
     sign_in_and_redirect user, event: :authentication
+  rescue Omniauth::Cul::Exceptions::Error => e
+    # If an unexpected CAS ticket validation occurs, log the error message and ask the user to try
+    # logging in again.  Do not display the exception object's original message to the user because it may
+    # contain information that only a developer should see.
+    error_message = 'CAS login validation failed. Please try again.'
+    Rails.logger.debug(error_message + "  #{e.class.name}: #{e.message}")
+    flash[:alert] = error_message
+    redirect_to root_path
   end
 end

app/controllers/users/sessions_controller.rb

@@ -7,8 +7,8 @@ class Users::SessionsController < Devise::SessionsController
   # that sends a POST req to our omniauth endpoint (this is the secure way and
   # POST is not possible with redirect_to)
   # inspiration: https://stackoverflow.com/questions/985596/redirect-to-using-post-in-rails
-  def new
-    render 'users/sessions/new'
+  def new # rubocop:disable Lint/UselessMethodDefinition
+    super
   end
 
   # This is needed if not using database authenticable (see https://github.com/heartcombo/devise/wiki/OmniAuth:-Overview#using-omniauth-without-other-authentications)

app/models/user.rb

@@ -11,7 +11,7 @@ class User < ApplicationRecord
   before_save       :set_personal_info_via_ldap
 
   # Configure devise for our User model
-  devise :rememberable, :trackable, :omniauthable, omniauth_providers: [Rails.env.development? ? :developer : :cas]
+  devise :rememberable, :trackable, :omniauthable, omniauth_providers: Devise.omniauth_configs.keys
 
   ADMIN = 'admin'.freeze
   ROLES = [ADMIN].freeze

app/views/users/sessions/new.html.erb

@@ -4,7 +4,7 @@
     Our Sessions Controller is scoped to users (not user), hence the directory
     structure. %>
 
-<%= form_with url: (Rails.env.development? ? user_developer_omniauth_authorize_path : user_cas_omniauth_authorize_path),
+<%= form_with url: (Rails.env.development? ? user_developer_uid_omniauth_authorize_path : user_columbia_cas_omniauth_authorize_path),
               method: :post, data: { turbo: false }, id: 'signInForm' do |f| %>
     <p>Redirecting you to be authenticated...</p>
     <%= f.submit 'Sign In', style: 'display: none;', id: 'signInRedirect' %>

config/initializers/devise.rb

@@ -250,11 +250,8 @@
   # config.omniauth :github, 'APP_ID', 'APP_SECRET', scope: 'user,public_repo'
   # config.omniauth :developer, { fields: [:uid], uid_field: :uid } # TODO : These options...
 
-  if Rails.env.development?
-    config.omniauth :developer, fields: [:uid], uid_field: :uid
-  else
-    config.omniauth :cas, strategy_class: Omniauth::Cul::Strategies::Cas3Strategy
-  end
+  config.omniauth :columbia_cas, { label: 'Columbia SSO (CAS)' }
+  config.omniauth :developer_uid, { label: 'Developer UID' } if Rails.env.development?
 
   # ==> Warden configuration
   # If you want to use other strategies, that are not supported by Devise, or

config/initializers/omniauth.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+# From omniauth-cul v0.3.0 Readme
+# Mitigate CVE-2015-9284
+OmniAuth.config.request_validation_phase = OmniAuth::AuthenticityTokenProtection.new(key: :_csrf_token)

config/routes.rb

@@ -4,32 +4,13 @@
   # temporarily removing range slider
   # concern :range_searchable, BlacklightRangeLimit::Routes::RangeSearchable.new
 
-  # When we are doing local development, we use omniauth's built-in developer strategy with devise's built-in omniauth
-  # engine. By not skipping :omniauth_callbacks in development, devise creates the routes /users/auth/developer and
-  # /users/auth/developer/callback and configures them to use devise's omniauth_callbacks_controller actions.
-  # In deployed environments, we use CUL's cas server (with the omniauth-cul gem) to authenticate users, with our
-  #  custom defined routes and custom omniauth_callbacks_controller.
-  # If we had not skipped the creation of these routes by devise, we would be able to authenticate users with our CAS
-  # server, but Academic Commons would be vulnerable to CVE-2015-9284, because the devise routes allow clients to make
-  # a GET request to users/auth/cas (instead of requiring a POST request).
-  # We are trying to find a more elegant way to do this (and the documentation for omniauth implies that with v2, GET
-  # requests should not be allowed, even if we have not gotten it to work), but this is a secure workaround for now.
-  skip_omniauth_callbacks = Rails.env.development? ? [] : [:omniauth_callbacks]
-  devise_for :users,
-             controllers: { sessions: 'users/sessions', omniauth_callbacks: 'users/omniauth_callbacks' },
-             skip: skip_omniauth_callbacks
-
+  # Create the sign in and sign out routes (simply redirects to our auth endpoint), needed for redirecting on
+  # cancancan access denied error
   devise_scope :user do
-    # Create the sign in and sign out routes (simply redirects to our auth endpoint), needed for redirecting on
-    # cancancan access denied error
     delete 'sign_out', to: 'users/sessions#destroy', as: :destroy_user_session
     get 'sign_in', to: 'users/sessions#new', as: :new_user_session
-    # Create the routes for omniauth auth and callback routes (unless doing local development) (see comment above)
-    unless Rails.env.development?
-      post 'users/auth/cas', to: 'users/omniauth_callbacks#passthru', as: :user_cas_omniauth_authorize
-      get 'users/auth/cas/callback', to: 'users/omniauth_callbacks#cas', as: :user_cas_omniauth_callback
-    end
   end
+  devise_for :users, controllers: { omniauth_callbacks: 'users/omniauth_callbacks' }
 
   root to: "catalog#home"
 

spec/controllers/users/omniauth_callbacks_controller_spec.rb

@@ -5,26 +5,19 @@
 RSpec.describe Users::OmniauthCallbacksController, type: :controller do
   include_context 'mock ldap request' # provides uni, cul_ldap, and cul_ldap entry as well as mocks LDAP methods
 
-  # let(:uid) { 'abc123' }
   let(:auth_hash) do
     OmniAuth::AuthHash.new({ 'uid' => uni, 'extra' => {} })
   end
 
-  # let(:cul_ldap_entry) do
-  #   instance_double('Cul::LDAP::Entry', uni: 'abc123', email: 'abc123@columbia.edu', last_name: 'Doe', first_name: 'Jane')
-  # end
-
   before :each do
     request.env['devise.mapping'] = Devise.mappings[:user]
-    allow(Omniauth::Cul::Cas3).to receive(:validation_callback).and_return([uni, nil])
-    # allow_any_instance_of(Cul::LDAP).to receive(:initialize).and_return(cul_ldap)
-    # allow_any_instance_of(Cul::LDAP).to receive(:find_by_uni).with(uid).and_return(cul_ldap_entry)
+    allow(Omniauth::Cul::ColumbiaCas).to receive(:validation_callback).and_return([uni, nil])
   end
 
   # GET :cas
   describe '#cas' do
     before :each do
-      get :cas
+      get :columbia_cas
     end
 
     it 'creates new user' do