Upgrade to OmniAuth CAS 3.x

awilfox · awilfox · commit 092c462edaa4 · 2026-02-12T19:16:16.000-06:00
* Display button on `UnauthorizedError` instead of redirecting to login.

* Use `ForbiddenError` instead of redirecting to login when a user
  that is not an admin tries to access an admin page in the section
  for ref cards/stack passes.

* Update specs.
diff --git a/.idea/altmedia.iml b/.idea/altmedia.iml
diff --git a/Gemfile b/Gemfile
@@ -24,8 +24,9 @@ gem 'lograge', '>=0.11.2'
 gem 'netaddr', '~> 1.5', '>= 1.5.1'
 gem 'net-ssh'
 gem 'okcomputer', '~> 1.19'
-gem 'omniauth', '~> 1.9', '>= 1.9.2'
-gem 'omniauth-cas', '~> 2.0'
+gem 'omniauth', '~> 2.1'
+gem 'omniauth-cas', '~> 3.0'
+gem 'omniauth-rails_csrf_protection', '~> 1.0'
 gem 'pg', '~> 1.2'
 gem 'prawn', '~> 2.4'
 gem 'puma', '~> 4.3', '>= 4.3.12'
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -276,13 +276,18 @@ GEM
       ostruct (>= 0.2)
     okcomputer (1.19.1)
       benchmark
-    omniauth (1.9.2)
+    omniauth (2.1.4)
       hashie (>= 3.4.6)
-      rack (>= 1.6.2, < 3)
-    omniauth-cas (2.0.0)
-      addressable (~> 2.3)
-      nokogiri (~> 1.5)
-      omniauth (~> 1.2)
+      logger
+      rack (>= 2.2.3)
+      rack-protection
+    omniauth-cas (3.0.2)
+      addressable (~> 2.8)
+      nokogiri (~> 1.12)
+      omniauth (~> 2.1)
+    omniauth-rails_csrf_protection (1.0.2)
+      actionpack (>= 4.2)
+      omniauth (~> 2.0)
     ostruct (0.6.3)
     ougai (2.0.0)
       oj (~> 3.10)
@@ -310,6 +315,9 @@ GEM
     raabro (1.4.0)
     racc (1.8.1)
     rack (2.2.17)
+    rack-protection (3.2.0)
+      base64 (>= 0.1.0)
+      rack (~> 2.2, >= 2.2.4)
     rack-session (1.0.2)
       rack (< 3)
     rack-test (2.2.0)
@@ -533,8 +541,9 @@ DEPENDENCIES
   net-ssh
   netaddr (~> 1.5, >= 1.5.1)
   okcomputer (~> 1.19)
-  omniauth (~> 1.9, >= 1.9.2)
-  omniauth-cas (~> 2.0)
+  omniauth (~> 2.1)
+  omniauth-cas (~> 3.0)
+  omniauth-rails_csrf_protection (~> 1.0)
   pg (~> 1.2)
   prawn (~> 2.4)
   puma (~> 4.3, >= 4.3.12)
diff --git a/app/controllers/concerns/exception_handling.rb b/app/controllers/concerns/exception_handling.rb
@@ -64,7 +64,7 @@ def log_error(error)
       # this isn't really an error condition, it just means the user's
       # not logged in, so we don't need the full stack trace etc.
       logger.info(error.message)
-      redirect_to main_app.login_path(url: request.fullpath)
+      render :unauthorized, status: :unauthorized
     end
   end
   # rubocop:enable Metrics/BlockLength
diff --git a/app/controllers/reference_card_forms_controller.rb b/app/controllers/reference_card_forms_controller.rb
@@ -85,6 +85,6 @@ def validate_recaptcha!
 
   def require_admin!
     @user_is_admin = current_user.role?(Role.stackpass_admin)
-    redirect_to login_path(url: request.fullpath) unless @user_is_admin
+    raise Error::ForbiddenError unless @user_is_admin
   end
 end
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
@@ -10,12 +10,6 @@
 #
 # @see https://github.com/omniauth/omniauth
 class SessionsController < ApplicationController
-  # Redirect the user to Calnet for authentication
-  def new
-    redirect_args = { origin: params[:url] || home_path }.to_query
-    redirect_to "/auth/calnet?#{redirect_args}"
-  end
-
   # Generate a new user session using data returned from a valid Calnet login
   def callback
     logger.debug({ msg: 'Received omniauth callback', omniauth: auth_params })
diff --git a/app/controllers/stack_pass_forms_controller.rb b/app/controllers/stack_pass_forms_controller.rb
@@ -87,6 +87,6 @@ def validate_recaptcha!
 
   def require_admin!
     @user_is_admin = current_user.role?(Role.stackpass_admin)
-    redirect_to login_path(url: request.fullpath) unless @user_is_admin
+    raise Error::ForbiddenError unless @user_is_admin
   end
 end
diff --git a/app/views/application/unauthorized.html.erb b/app/views/application/unauthorized.html.erb
@@ -0,0 +1,7 @@
+<h1>Unauthorized</h1>
+
+<p>You need to log in to continue.</p>
+
+<%= form_tag('/auth/calnet', url: request.original_url, method: :post, data: { turbo: false }) do %>
+  <%= button_tag 'CalNet Login', class: :calnet_login, role: :link %>
+<% end %>
diff --git a/config/routes.rb b/config/routes.rb
@@ -74,7 +74,6 @@
 
   # Omniauth automatically handles requests to /auth/:provider. We need only
   # implement the callback.
-  get '/login', to: 'sessions#new', as: :login
   get '/logout', to: 'sessions#destroy', as: :logout
   get '/auth/:provider/callback', to: 'sessions#callback', as: :omniauth_callback
   get '/auth/failure', to: 'sessions#failure'
diff --git a/spec/calnet_helper.rb b/spec/calnet_helper.rb
@@ -120,8 +120,6 @@ def without_redirects
 
   def log_in_with_omniauth(auth_hash)
     OmniAuth.config.mock_auth[:calnet] = auth_hash
-    do_get login_path
-
     Rails.application.env_config['omniauth.auth'] = auth_hash
     do_get omniauth_callback_path(:calnet)
   end
diff --git a/spec/forms_helper.rb b/spec/forms_helper.rb
@@ -26,10 +26,9 @@ def show_path(id)
     send("#{form_name}_path", id)
   end
 
-  it 'redirects to login' do
-    expected_location = login_path(url: new_form_path)
+  it 'requires login' do
     get new_form_path
-    expect(response).to redirect_to(expected_location)
+    expect(response).to have_http_status(:unauthorized)
   end
 
   allowed_patron_types.each do |type|
diff --git a/spec/request/bibliographics_spec.rb b/spec/request/bibliographics_spec.rb
@@ -7,32 +7,28 @@
     end
 
     context 'new/create' do
-      it 'GET redirects to login' do
-        get(form_path = bibliographics_path)
-        login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: form_path)}"
-        expect(response).to redirect_to(login_with_callback_url)
+      it 'GET requires login' do
+        get bibliographics_path
+        expect(response).to have_http_status :unauthorized
       end
 
-      it 'POST redirects to login' do
+      it 'POST requires login' do
         post bibliographics_path, params: { upload_file: nil }
-        login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: bibliographics_path)}"
-        expect(response).to redirect_to(login_with_callback_url)
+        expect(response).to have_http_status :unauthorized
       end
     end
 
     # context 'index' do
-    #   it 'GET redirects to login' do
-    #     get(form_path = bibliographics_index_path)
-    #     login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: form_path)}"
-    #     expect(response).to redirect_to(login_with_callback_url)
+    #   it 'GET requires login' do
+    #     get bibliographics_index_path
+    #     expect(response).to have_http_status :unauthorized
     #   end
     # end
 
     context 'response' do
-      it 'GET redirects to login' do
-        get(form_path = bibliographics_response_path)
-        login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: form_path)}"
-        expect(response).to redirect_to(login_with_callback_url)
+      it 'GET requires login' do
+        get bibliographics_response_path
+        expect(response).to have_http_status :unauthorized
       end
     end
 
diff --git a/spec/request/departmental_card_form_request_spec.rb b/spec/request/departmental_card_form_request_spec.rb
@@ -7,10 +7,9 @@
     end
 
     context 'new' do
-      it 'GET redirects to login' do
-        get(form_path = new_departmental_card_form_path)
-        login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: form_path)}"
-        expect(response).to redirect_to(login_with_callback_url)
+      it 'GET requires login' do
+        get new_departmental_card_form_path
+        expect(response).to have_http_status :unauthorized
       end
     end
   end
diff --git a/spec/request/doemoff_patron_email_forms_spec.rb b/spec/request/doemoff_patron_email_forms_spec.rb
@@ -7,10 +7,9 @@
     end
 
     context 'index' do
-      it 'GET redirects to login' do
-        get(form_path = security_incident_report_forms_path)
-        login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: form_path)}"
-        expect(response).to redirect_to(login_with_callback_url)
+      it 'GET requires login' do
+        get security_incident_report_forms_path
+        expect(response).to have_http_status :unauthorized
       end
     end
   end
diff --git a/spec/request/mmsid_tind_request_spec.rb b/spec/request/mmsid_tind_request_spec.rb
@@ -9,17 +9,15 @@
     end
 
     context 'new/create' do
-      it 'GET redirects to login' do
-        get(form_path = mmsid_tind_path)
-        login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: form_path)}"
-        expect(response).to redirect_to(login_with_callback_url)
+      it 'GET requires login' do
+        get mmsid_tind_path
+        expect(response).to have_http_status :unauthorized
       end
 
-      it 'POST redirects to login' do
+      it 'POST requires login' do
         args = {}
         post mmsid_tind_path, params: args
-        login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: mmsid_tind_path)}"
-        expect(response).to redirect_to(login_with_callback_url)
+        expect(response).to have_http_status :unauthorized
       end
     end
   end
diff --git a/spec/request/reference_card_forms_request_spec.rb b/spec/request/reference_card_forms_request_spec.rb
@@ -17,11 +17,11 @@
       expect(response).to redirect_to(action: :new)
     end
 
-    it 'redirects to login if if user is not a stack pass admin' do
+    it 'requires login if user is not a stack pass admin' do
       form = ReferenceCardForm.create(id: 1, email: 'openreq@test.com', name: 'John Doe',
                                       pass_date: Date.current, pass_date_end: Date.current + 1)
-      get(form_path = reference_card_form_path(id: form.id))
-      expect(response).to redirect_to("#{login_path}?#{URI.encode_www_form(url: form_path)}")
+      get reference_card_form_path(id: form.id)
+      expect(response).to have_http_status :forbidden
     end
 
     it 'rejects a submission with a captcha verification error' do
diff --git a/spec/request/security_incident_report_forms_spec.rb b/spec/request/security_incident_report_forms_spec.rb
@@ -7,10 +7,9 @@
     end
 
     context 'index' do
-      it 'GET redirects to login' do
-        get(form_path = security_incident_report_forms_path)
-        login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: form_path)}"
-        expect(response).to redirect_to(login_with_callback_url)
+      it 'GET requires login' do
+        get security_incident_report_forms_path
+        expect(response).to have_http_status :unauthorized
       end
     end
   end
diff --git a/spec/request/stack_pass_form_request_spec.rb b/spec/request/stack_pass_form_request_spec.rb
@@ -23,12 +23,12 @@
       expect(response).to redirect_to(action: :new)
     end
 
-    it 'redirects to login if if user is not a stack pass admin' do
+    it 'requires login if user is not a stack pass admin' do
       form = StackPassForm.create(email: 'openreq@test.com', name: 'John Doe',
                                   phone: '925-555-1234', pass_date: Date.current, main_stack: true)
 
-      get(form_path = stack_pass_form_path(id: form.id))
-      expect(response).to redirect_to("#{login_path}?#{URI.encode_www_form(url: form_path)}")
+      get stack_pass_form_path(id: form.id)
+      expect(response).to have_http_status :forbidden
     end
 
     it 'rejects a submission with a captcha verification error' do
@@ -49,7 +49,6 @@
       get response.header['Location']
       expect(response.body).to match('RECaptcha Error')
     end
-
   end
 
   context 'specs with admin privledges' do
@@ -136,7 +135,6 @@
   end
 
   context 'specs with non-admin user logged in' do
-
     before do
       allow(Rails.application.config).to receive(:alma_api_key).and_return(alma_api_key)
 
diff --git a/spec/request/tind_download_request_spec.rb b/spec/request/tind_download_request_spec.rb
@@ -19,35 +19,30 @@
     end
 
     describe 'form' do
-      it 'redirects to login' do
-        get(form_path = tind_download_path)
-        login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: form_path)}"
-        expect(response).to redirect_to(login_with_callback_url)
+      it 'requires login' do
+        get tind_download_path
+        expect(response).to have_http_status :unauthorized
       end
     end
 
     describe 'find_collection' do
-      it 'redirects to login' do
+      it 'requires login' do
         get tind_download_find_collection_path
-        login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: tind_download_find_collection_path)}"
-        expect(response).to redirect_to(login_with_callback_url)
+        expect(response).to have_http_status :unauthorized
       end
     end
 
     describe 'download' do
-      it 'POST redirects to login' do
+      it 'POST requires login' do
         post tind_download_download_path, params: { collection_name:, export_format: 'csv' }
-        login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: tind_download_download_path)}"
-        expect(response).to redirect_to(login_with_callback_url)
+        expect(response).to have_http_status :unauthorized
       end
 
-      it 'GET redirects to login' do
+      it 'GET requires login' do
         params = { collection_name:, export_format: 'csv' }
         get tind_download_download_path, params: params
 
-        callback_url = "#{tind_download_download_path}?#{URI.encode_www_form(params)}"
-        login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: callback_url)}"
-        expect(response).to redirect_to(login_with_callback_url)
+        expect(response).to have_http_status :unauthorized
       end
     end
   end
diff --git a/spec/request/tind_marc_batch_request_spec.rb b/spec/request/tind_marc_batch_request_spec.rb
@@ -10,17 +10,15 @@
     end
 
     context 'new/create' do
-      it 'GET redirects to login' do
-        get(form_path = tind_marc_batch_path)
-        login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: form_path)}"
-        expect(response).to redirect_to(login_with_callback_url)
+      it 'GET requires login' do
+        get tind_marc_batch_path
+        expect(response).to have_http_status :unauthorized
       end
 
-      it 'POST redirects to login' do
+      it 'POST requires login' do
         args = {}
         post tind_marc_batch_path, params: args
-        login_with_callback_url = "#{login_path}?#{URI.encode_www_form(url: tind_marc_batch_path)}"
-        expect(response).to redirect_to(login_with_callback_url)
+        expect(response).to have_http_status :unauthorized
       end
     end
   end
diff --git a/spec/request/tind_validator_request_spec.rb b/spec/request/tind_validator_request_spec.rb
diff --git a/spec/requests/location_requests_spec.rb b/spec/requests/location_requests_spec.rb
diff --git a/spec/system/alma_item_set_system_spec.rb b/spec/system/alma_item_set_system_spec.rb
diff --git a/spec/system/bibliographics_spec.rb b/spec/system/bibliographics_spec.rb
diff --git a/spec/system/home_system_spec.rb b/spec/system/home_system_spec.rb
diff --git a/spec/system/location_requests_system_spec.rb b/spec/system/location_requests_system_spec.rb
diff --git a/spec/system/tind_download_system_spec.rb b/spec/system/tind_download_system_spec.rb
diff --git a/spec/system/tind_marc_batch_system_spec.rb b/spec/system/tind_marc_batch_system_spec.rb
