Merge pull request #180 from uclibs/149-remote-code-execution-vulnera…

…bility

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

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