149 fixed remote-code-execution vulnerability, added tests for new code

Gemfile

@@ -65,6 +65,7 @@ group :development, :test do
 end
 
 group :development do
+  gem 'brakeman', '~> 6.0'
   gem 'capistrano', '~> 3.17.1', require: false
   gem 'capistrano-bundler', '~> 1.6', require: false
   gem 'capistrano-rails', '~> 1.4', require: false
@@ -73,10 +74,10 @@ group :development do
   gem 'capistrano-rvm', require: false
   # Access an interactive console on exception pages or by calling 'console' anywhere in the code.
   gem 'listen', '>= 3.0.5', '< 3.2'
-  gem 'web-console', '>= 3.3.0'
   # Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring
   gem 'spring'
   gem 'spring-watcher-listen', '~> 2.0.0'
+  gem 'web-console', '>= 3.3.0'
 end
 
 group :test do

Gemfile.lock

@@ -76,6 +76,7 @@ GEM
       autoprefixer-rails (>= 9.1.0)
       popper_js (>= 1.14.3, < 2)
       sassc-rails (>= 2.0.0)
+    brakeman (6.0.1)
     builder (3.2.4)
     byebug (11.1.3)
     capistrano (3.17.3)
@@ -372,6 +373,7 @@ DEPENDENCIES
   bcrypt_pbkdf
   bootsnap (>= 1.1.0)
   bootstrap (~> 4.4.1)
+  brakeman (~> 6.0)
   byebug
   capistrano (~> 3.17.1)
   capistrano-bundler (~> 1.6)

app/controllers/admin_controller.rb

@@ -2,6 +2,24 @@
 
 class AdminController < ApplicationController
   skip_before_action :check_date
+
+  ALLOWED_CONTROLLERS_TO_MODELS = {
+    'artworks' => Artwork,
+    'books' => Book,
+    'book_chapters' => BookChapter,
+    'digital_projects' => DigitalProject,
+    'editings' => Editing,
+    'films' => Film,
+    'journal_articles' => JournalArticle,
+    'musical_scores' => MusicalScore,
+    'photographies' => Photography,
+    'physical_media' => PhysicalMedium,
+    'plays' => Play,
+    'public_performances' => PublicPerformance,
+    'submitters' => Submitter,
+    'other_publications' => OtherPublication
+  }.freeze
+
   def login; end
 
   def validate
@@ -14,11 +32,16 @@ def validate
   end
 
   def csv
-    if session[:admin] && params[:id].nil?
-      instance_variable_set('@instance_variable', Object.const_get(params[:controller_name].classify).all)
-      respond_to do |format|
-        format.html { redirect_to publications_path }
-        format.csv { send_data @instance_variable.to_csv }
+    if session[:admin] && params[:id].nil? && allowed_model
+      begin
+        @instance_variable = allowed_model.all
+        respond_to do |format|
+          format.html { redirect_to publications_path }
+          format.csv { send_data @instance_variable.to_csv }
+        end
+      rescue StandardError => e
+        logger.error "CSV generation failed: #{e}"
+        redirect_to publications_path, notice: 'Something went wrong while generating the CSV.'
       end
     else
       redirect_to publications_path
@@ -27,10 +50,10 @@ def csv
 
   def citations
     if session[:admin]
-      all = (Artwork.all + Book.all + BookChapter.all + DigitalProject.all + Editing.all + Film.all + JournalArticle.all + MusicalScore.all + Photography.all + PhysicalMedium.all + Play.all + PublicPerformance.all + OtherPublication.all)
+      all = fetch_all_records
       @college_array = []
       (1..College.count).each do |i|
-        @college_array << [i, all.select { |p| p.college_ids.include? i }.group_by(&:uc_department)]
+        @college_array << [i, all.select { |p| p.respond_to?(:college_ids) && p.college_ids.include?(i) }.group_by(&:uc_department)]
       end
     else
       redirect_to publications_path
@@ -47,4 +70,16 @@ def toggle_links
   def check_credentials(username, password)
     username == ENV['ADMIN_USERNAME'] && password == ENV['ADMIN_PASSWORD']
   end
+
+  def allowed_model
+    ALLOWED_CONTROLLERS_TO_MODELS[params[:controller_name]]
+  end
+
+  def fetch_all_records
+    all_records = []
+    ALLOWED_CONTROLLERS_TO_MODELS.each_value do |model|
+      all_records += model.all
+    end
+    all_records
+  end
 end

spec/controllers/admin_controller/admin_controller_allowed_controllers_to_models_spec.rb

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe AdminController, type: :controller do
+  describe 'ALLOWED_CONTROLLERS_TO_MODELS' do
+    let(:excluded_model_names) { ['ActiveRecord::InternalMetadata', 'ActiveRecord::SchemaMigration', 'ApplicationRecord', 'College', '...eges', 'HABTM_Colleges', 'HABTM_Books', 'HABTM_OtherPublications'] }
+
+    def load_all_models
+      Dir[Rails.root.join('app/models/**/*.rb')].each { |file| require file }
+    end
+
+    def fetch_all_model_names
+      ActiveRecord::Base.descendants.map(&:name).sort
+    end
+
+    def fetch_allowed_model_names
+      AdminController::ALLOWED_CONTROLLERS_TO_MODELS.values.map(&:to_s).sort
+    end
+
+    before(:all) do
+      load_all_models
+    end
+
+    it 'contains all the relevant models in app/models' do
+      all_model_names = fetch_all_model_names
+      allowed_model_names = fetch_allowed_model_names
+      relevant_model_names = (all_model_names - excluded_model_names).sort
+      expect(allowed_model_names).to match_array(relevant_model_names)
+    end
+
+    it 'each model should have a to_csv class method' do
+      missing_methods = []
+
+      AdminController::ALLOWED_CONTROLLERS_TO_MODELS.each_value do |model|
+        missing_methods << "#{model.name} is missing a to_csv class method" unless model.respond_to?(:to_csv)
+      end
+
+      expect(missing_methods).to be_empty, missing_methods.join(', ')
+    end
+  end
+end

spec/controllers/admin_controller/admin_controller_citations_spec.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe AdminController, type: :controller do
+  describe 'GET #citations' do
+    context 'when admin is logged in' do
+      before do
+        allow(College).to receive(:count).and_return(2)
+        allow(controller).to receive(:session).and_return(admin: true)
+        allow(controller).to receive(:fetch_all_records).and_return(mocked_records)
+      end
+
+      let(:mocked_records) { [double('Artwork', college_ids: [1], uc_department: 'Art'), double('Book', college_ids: [2], uc_department: 'Literature')] }
+
+      it 'populates @college_array with grouped records' do
+        get :citations
+        expect(assigns(:college_array)).to eq([[1, { 'Art' => [mocked_records.first] }], [2, { 'Literature' => [mocked_records.last] }]])
+      end
+    end
+
+    context 'when admin is not logged in' do
+      before do
+        allow(controller).to receive(:session).and_return(admin: false)
+      end
+
+      it 'redirects to publications_path' do
+        get :citations
+        expect(response).to redirect_to(publications_path)
+      end
+    end
+  end
+end

spec/controllers/admin_controller/admin_controller_csv_spec.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe AdminController, type: :controller do
+  describe 'GET #csv' do
+    it 'returns a csv when admin' do
+      get(:csv, params: { :format => 'csv', 'controller_name' => 'other_publications' }, session: { 'admin' => true })
+      expect(response.status).to eq 200
+    end
+
+    it 'redirects when invalid format and admin' do
+      get(:csv, params: { :format => 'html', 'controller_name' => 'other_publications' }, session: { 'admin' => true })
+      expect(response).to redirect_to('/publications')
+    end
+
+    it 'redirects when not admin but valid' do
+      get(:csv, params: { :format => 'csv', 'controller_name' => 'other_publications' })
+      expect(response).to redirect_to('/publications')
+    end
+  end
+end

spec/controllers/admin_controller/admin_controller_login_spec.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe AdminController, type: :controller do
+  describe 'GET #login' do
+    it 'Visit manage path before expiration date' do
+      allow(Date).to receive(:current).and_return Date.parse(ENV.fetch('EXPIRATION_DATE'))
+      get(:login, params: { :format => 'html', 'controller_name' => 'admin_controller' }, session: { 'admin' => false })
+      expect(response.status).to eq 200
+    end
+
+    it 'Visit manage path after expiration date' do
+      allow(Date).to receive(:current).and_return Date.parse(ENV.fetch('PAST_DATE'))
+      get(:login, params: { :format => 'html', 'controller_name' => 'admin_controller' }, session: { 'admin' => false })
+      expect(response.status).to eq 200
+    end
+  end
+end

spec/controllers/admin_controller/admin_controller_private_methods_spec.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe AdminController, type: :controller do
+  describe '#fetch_all_records' do
+    before do
+      stub_const('AdminController::ALLOWED_CONTROLLERS_TO_MODELS', {
+                   'artworks' => double('ArtworkModel'),
+                   'books' => double('BookModel')
+                 })
+    end
+
+    it 'fetches all records from each allowed model' do
+      # Create doubles for the records
+      artwork_double = double('ArtworkRecord')
+      book_double = double('BookRecord')
+
+      # Allow the models to receive :all and return the doubles
+      allow(AdminController::ALLOWED_CONTROLLERS_TO_MODELS['artworks']).to receive(:all).and_return([artwork_double])
+      allow(AdminController::ALLOWED_CONTROLLERS_TO_MODELS['books']).to receive(:all).and_return([book_double])
+
+      all_records = subject.send(:fetch_all_records)
+
+      # Verify
+      expect(all_records).to match_array([artwork_double, book_double])
+    end
+
+    it 'returns an empty array if no records are present' do
+      # Allow the models to receive :all and return empty arrays
+      allow(AdminController::ALLOWED_CONTROLLERS_TO_MODELS['artworks']).to receive(:all).and_return([])
+      allow(AdminController::ALLOWED_CONTROLLERS_TO_MODELS['books']).to receive(:all).and_return([])
+
+      all_records = subject.send(:fetch_all_records)
+
+      # Verify
+      expect(all_records).to be_empty
+    end
+  end
+end

spec/controllers/admin_controller/admin_controller_toggle_links_spec.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe AdminController, type: :controller do
+  describe 'POST #toggle_links' do
+    context 'when session[:links] is true' do
+      before do
+        session[:links] = true
+      end
+
+      it 'toggles session[:links] to false' do
+        post :toggle_links
+        expect(session[:links]).to be false
+      end
+
+      it 'redirects to citations_path' do
+        post :toggle_links
+        expect(response).to redirect_to(citations_path)
+      end
+    end
+
+    context 'when session[:links] is false' do
+      before do
+        session[:links] = false
+      end
+
+      it 'toggles session[:links] to true' do
+        post :toggle_links
+        expect(session[:links]).to be true
+      end
+
+      it 'redirects to citations_path' do
+        post :toggle_links
+        expect(response).to redirect_to(citations_path)
+      end
+    end
+  end
+end

spec/controllers/admin_controller/admin_controller_validate_spec.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe AdminController, type: :controller do
+  let(:valid_attributes) do
+    { 'username' => ENV.fetch('ADMIN_USERNAME', nil), 'password' => ENV.fetch('ADMIN_PASSWORD', nil) }
+  end
+
+  let(:invalid_attributes) do
+    { 'username' => 'bad', 'password' => 'invalid' }
+  end
+
+  describe 'POST #validate' do
+    it 'returns a success response' do
+      post :validate, params: valid_attributes
+      expect(response).to redirect_to('/publications')
+    end
+
+    it 'returns a error response' do
+      post :validate, params: invalid_attributes
+      expect(response.status).to eq 302
+    end
+  end
+end