From fa019db114adeaf2ef0c6507dd6d0ed77479594a Mon Sep 17 00:00:00 2001 From: Christa Hartsock Date: Tue, 30 Jun 2020 22:06:00 -0700 Subject: [PATCH 1/3] Raise error so 404 is triggered when URL is wrong Record.find_by_x doesn't raise an ActiveRecord::RecordNotFound error, which triggers the 404, so our app was returning a 500 error when incorrect district URLs were accessed. (e.g. /d/san-francisco-sdf). This should protect against that (but only in hosted environments) --- app/controllers/police_districts_controller.rb | 2 +- spec/controllers/police_districts_controller_spec.rb | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 spec/controllers/police_districts_controller_spec.rb diff --git a/app/controllers/police_districts_controller.rb b/app/controllers/police_districts_controller.rb index 75a9c90..d3dd6ec 100644 --- a/app/controllers/police_districts_controller.rb +++ b/app/controllers/police_districts_controller.rb @@ -7,7 +7,7 @@ def index end def show - @district = PoliceDistrict.find_by_slug(params[:slug]) + @district = PoliceDistrict.find_by_slug!(params[:slug]) @meeting = @district.next_meeting.present? ? @district.next_meeting : @district.most_recent_meeting end end diff --git a/spec/controllers/police_districts_controller_spec.rb b/spec/controllers/police_districts_controller_spec.rb new file mode 100644 index 0000000..4c805b6 --- /dev/null +++ b/spec/controllers/police_districts_controller_spec.rb @@ -0,0 +1,11 @@ +require 'rails_helper' + +RSpec.describe PoliceDistrictsController, type: :controller do + describe '#show' do + it 'returns 404 if district not present' do + expect do + get :show, params: { slug: 'asldkfjaslkfdj' } + end.to raise_exception(ActiveRecord::RecordNotFound) + end + end +end From 451e641b593d270af7e3bc3f5166215ee903b4b8 Mon Sep 17 00:00:00 2001 From: Christa Hartsock Date: Tue, 30 Jun 2020 22:12:49 -0700 Subject: [PATCH 2/3] Raise 404 if no district for meeting --- app/controllers/admin/meetings_controller.rb | 2 +- .../admin/meetings_controller_spec.rb | 23 +++++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/app/controllers/admin/meetings_controller.rb b/app/controllers/admin/meetings_controller.rb index 850020a..4a5d2b4 100644 --- a/app/controllers/admin/meetings_controller.rb +++ b/app/controllers/admin/meetings_controller.rb @@ -55,7 +55,7 @@ def destroy attr_reader :district def set_police_district - @district = PoliceDistrict.find_by_slug(params[:police_district_id]) + @district = PoliceDistrict.find_by_slug!(params[:police_district_id]) end def meeting_params diff --git a/spec/controllers/admin/meetings_controller_spec.rb b/spec/controllers/admin/meetings_controller_spec.rb index 3818763..0db9d2a 100644 --- a/spec/controllers/admin/meetings_controller_spec.rb +++ b/spec/controllers/admin/meetings_controller_spec.rb @@ -3,6 +3,7 @@ RSpec.describe Admin::MeetingsController, type: :controller do let(:user) { FactoryBot.create(:user) } let!(:district) { FactoryBot.create(:police_district, slug: 'oakland', timezone: 'Pacific Time (US & Canada)') } + let!(:meeting) { FactoryBot.create(:meeting, police_district: district, event_datetime: DateTime.new(2025,1,20,11,0,0,0)) } context 'when user is signed in' do before do @@ -63,6 +64,12 @@ } end + it 'returns 404 if district not present' do + expect do + post :update, params: { police_district_id: 'asldkfjaslkfdj', id: meeting.id } + end.to raise_exception(ActiveRecord::RecordNotFound) + end + it 'converts datetime from district timezone to UTC before storing' do travel_to Date.parse('2020-06-03') do expect do @@ -73,10 +80,14 @@ end describe '#edit' do - let!(:meeting) { FactoryBot.create(:meeting, police_district: district, event_datetime: DateTime.new(2025,1,20,11,0,0,0)) } - render_views + it 'returns 404 if district not present' do + expect do + post :edit, params: { police_district_id: 'asldkfjaslkfdj', id: meeting.id } + end.to raise_exception(ActiveRecord::RecordNotFound) + end + it 'converts time to district timezone, but strips timezone' do travel_to Date.parse('2020-06-03') do get :edit, params: { id: meeting.id, police_district_id: district.slug } @@ -87,9 +98,13 @@ end describe '#destroy' do - let!(:meeting) { FactoryBot.create(:meeting, police_district: district) } - context 'js request' do + it 'returns 404 if district not present' do + expect do + post :update, format: :js, params: { police_district_id: 'asldkfjaslkfdj', id: meeting.id } + end.to raise_exception(ActiveRecord::RecordNotFound) + end + it 'deletes the meeting' do expect do delete :destroy, format: :js, params: { police_district_id: district.slug, id: meeting.id } From a1e25fe065f2c8d2cc409231883a201186d74631 Mon Sep 17 00:00:00 2001 From: Christa Hartsock Date: Tue, 30 Jun 2020 22:17:49 -0700 Subject: [PATCH 3/3] Require district in district admin --- .../admin/police_districts_controller.rb | 11 +++-- .../admin/police_districts_controller_spec.rb | 46 +++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 spec/controllers/admin/police_districts_controller_spec.rb diff --git a/app/controllers/admin/police_districts_controller.rb b/app/controllers/admin/police_districts_controller.rb index 079e17f..0d92416 100644 --- a/app/controllers/admin/police_districts_controller.rb +++ b/app/controllers/admin/police_districts_controller.rb @@ -1,6 +1,8 @@ class Admin::PoliceDistrictsController < Admin::ApplicationController include GoogleCalendarable + before_action :set_district, except: [:new, :create, :index] + def new @district = PoliceDistrict.new @@ -19,17 +21,14 @@ def create end def show - @district = PoliceDistrict.find_by_slug(params[:id]) + render :show end def edit - @district = PoliceDistrict.find_by_slug(params[:id]) - render :edit end def update - @district = PoliceDistrict.find_by_slug(params[:id]) if @district.update(district_params) redirect_to admin_police_districts_path else @@ -43,6 +42,10 @@ def index private + def set_district + @district = PoliceDistrict.find_by_slug!(params[:id]) + end + def district_params params.require(:police_district).permit( :name, diff --git a/spec/controllers/admin/police_districts_controller_spec.rb b/spec/controllers/admin/police_districts_controller_spec.rb new file mode 100644 index 0000000..73f1b08 --- /dev/null +++ b/spec/controllers/admin/police_districts_controller_spec.rb @@ -0,0 +1,46 @@ +require 'rails_helper' + +RSpec.describe Admin::PoliceDistrictsController, type: :controller do + let(:user) { FactoryBot.create(:user) } + let!(:district) { FactoryBot.create(:police_district, slug: 'oakland', timezone: 'Pacific Time (US & Canada)') } + + context 'when user is signed in' do + before do + sign_in user + end + + describe '#update' do + it 'returns 404 if district not present' do + expect do + post :update, params: { id: 'asldkfjaslkfdj' } + end.to raise_exception(ActiveRecord::RecordNotFound) + end + end + + describe '#edit' do + render_views + + it 'returns 404 if district not present' do + expect do + post :edit, params: { id: 'asldkfjaslkfdj' } + end.to raise_exception(ActiveRecord::RecordNotFound) + end + end + + describe '#destroy' do + it 'returns 404 if district not present' do + expect do + post :update, params: { id: 'asldkfjaslkfdj' } + end.to raise_exception(ActiveRecord::RecordNotFound) + end + end + + describe '#show' do + it 'returns 404 if district not present' do + expect do + get :show, params: { id: 'asldkfjaslkfdj' } + end.to raise_exception(ActiveRecord::RecordNotFound) + end + end + end +end