diff --git a/app/controllers/patron_requests_controller.rb b/app/controllers/patron_requests_controller.rb index ded9198d0..2be5b5129 100644 --- a/app/controllers/patron_requests_controller.rb +++ b/app/controllers/patron_requests_controller.rb @@ -39,6 +39,9 @@ def current_request def assign_new_attributes @patron_request.assign_attributes(**new_params) + # This validation prevents an injection attack in GraphQL. Ideally, this would be a model validation, + # but there are other validations that require first running a GraphQL query. + raise ActionController::BadRequest unless /\A(a|L|in)?\d+\z/.match?(new_params[:instance_hrid]) end # SSO or library-id users don't need to re-login, but name/email users always need to provide their information diff --git a/config/environments/test.rb b/config/environments/test.rb index 3491faf35..e8fe8c61d 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -29,7 +29,7 @@ config.cache_store = :null_store # Render exception templates for rescuable exceptions and raise for other exceptions. - config.action_dispatch.show_exceptions = :none + config.action_dispatch.show_exceptions = :all # Disable request forgery protection in test environment. config.action_controller.allow_forgery_protection = false diff --git a/spec/requests/create_request_spec.rb b/spec/requests/create_request_spec.rb new file mode 100644 index 000000000..b18fd25be --- /dev/null +++ b/spec/requests/create_request_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Create a new request' do + let(:url) { '/requests/new' } + let(:user) { nil } + + before do + login_as(user, run_callbacks: false) + get '/patron_requests/new?instance_hrid=%22p3nr6p&origin_location_code=SPEC-SAL3-U-ARCHIVES' + end + + it 'prevents graphql injection' do + expect(response).to have_http_status :bad_request + end +end diff --git a/spec/requests/sidekiq_requests_spec.rb b/spec/requests/sidekiq_requests_spec.rb index f65859ffc..67a3d1e83 100644 --- a/spec/requests/sidekiq_requests_spec.rb +++ b/spec/requests/sidekiq_requests_spec.rb @@ -3,24 +3,24 @@ require 'rails_helper' RSpec.describe 'Sidekiq requests' do - let(:url) { '/sidekiq' } let(:user) { nil } before do login_as(user, run_callbacks: false) + get '/sidekiq' end context 'with superadmin privileges' do let(:user) { instance_double(CurrentUser, user_object: instance_double(User, super_admin?: true)) } it 'is successful' do - expect { get(url) }.to raise_error(RedisClient::CannotConnectError) + expect(response).to have_http_status :internal_server_error # due to sidekiq not running in test end end context 'without superadmin privileges' do - it 'raises an access denied error' do - expect { get(url) }.to raise_error(ActionController::RoutingError) + it 'shows the not found page' do + expect(response).to have_http_status :not_found end end end