Skip to content

Commit b8f7873

Browse files
authored
Fix IDOR in Requests and Donations controllers (5509) (#5519)
* Scope requests and donations to current org (#5509) * Fix trailing whitespace in spec files - Donations & Requests
1 parent 3a36b1d commit b8f7873

File tree

4 files changed

+66
-7
lines changed

4 files changed

+66
-7
lines changed

app/controllers/donations_controller.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ class DonationsController < ApplicationController
33
before_action :authorize_admin, only: [:destroy]
44

55
def print
6-
@donation = Donation.find(params[:id])
6+
@donation = current_organization.donations.find(params[:id])
77
respond_to do |format|
88
format.any do
99
pdf = DonationPdf.new(current_organization, @donation)
@@ -53,7 +53,7 @@ def new
5353
end
5454

5555
def edit
56-
@donation = Donation.find(params[:id])
56+
@donation = current_organization.donations.find(params[:id])
5757
@donation.line_items.build
5858
@changes_disallowed = SnapshotEvent.intervening(@donation).present?
5959
@audit_performed_and_finalized = Audit.finalized_since?(@donation, @donation.storage_location_id) &&
@@ -63,12 +63,12 @@ def edit
6363
end
6464

6565
def show
66-
@donation = Donation.includes(line_items: :item).find(params[:id])
66+
@donation = current_organization.donations.includes(line_items: :item).find(params[:id])
6767
@line_items = @donation.line_items
6868
end
6969

7070
def update
71-
@donation = Donation.find(params[:id])
71+
@donation = current_organization.donations.find(params[:id])
7272
@original_source = @donation.source
7373
ItemizableUpdateService.call(itemizable: @donation,
7474
params: donation_params,

app/controllers/requests_controller.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ def index
2828
end
2929

3030
def show
31-
@request = Request.find(params[:id])
31+
@request = current_organization.requests.find(params[:id])
3232
@item_requests = @request.item_requests.includes(:item)
3333

3434
@inventory = View::Inventory.new(@request.organization_id)
@@ -42,7 +42,7 @@ def show
4242
# and will move the user to the new distribution page with a
4343
# pre-filled distribution containing all the requested items.
4444
def start
45-
request = Request.find(params[:id])
45+
request = current_organization.requests.find(params[:id])
4646
begin
4747
request.status_started!
4848
flash[:notice] = "Request started"

spec/requests/donations_requests_spec.rb

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,36 @@
277277
end
278278
end
279279

280+
describe "when accessing a donation from another organization" do
281+
let(:other_organization) { create(:organization) }
282+
let(:other_donation) { create(:donation, organization: other_organization, comment: "Original comment") }
283+
284+
it "returns not found for show" do
285+
get donation_path(id: other_donation.id)
286+
287+
expect(response).to have_http_status(:not_found)
288+
end
289+
290+
it "returns not found for edit" do
291+
get edit_donation_path(id: other_donation.id)
292+
293+
expect(response).to have_http_status(:not_found)
294+
end
295+
296+
it "returns not found for print" do
297+
get print_donation_path(id: other_donation.id)
298+
299+
expect(response).to have_http_status(:not_found)
300+
end
301+
302+
it "returns not found for update and does not change donation" do
303+
put donation_path(id: other_donation.id, donation: {comment: "Changed comment"})
304+
305+
expect(response).to have_http_status(:not_found)
306+
expect(other_donation.reload.comment).to eq("Original comment")
307+
end
308+
end
309+
280310
describe "GET #edit" do
281311
it 'should not allow edits if there is an intervening snapshot' do
282312
donation = FactoryBot.create(:donation,

spec/requests/requests_requests_spec.rb

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,24 @@
7777
end
7878
end
7979

80+
context 'When the request belongs to another organization' do
81+
let(:other_organization) { create(:organization) }
82+
let(:other_request) { create(:request, organization: other_organization) }
83+
84+
it 'responds with not found' do
85+
get request_path(other_request)
86+
87+
expect(response).to have_http_status(:not_found)
88+
end
89+
end
90+
8091
context 'When organization has a default storage location' do
81-
let(:request) { create(:request, organization: create(:organization, default_storage_location: 1)) }
92+
let(:storage_location) { create(:storage_location, organization: organization) }
93+
let(:request) do
94+
organization.update!(default_storage_location: storage_location.id)
95+
create(:request, organization: organization)
96+
end
97+
8298
it 'shows the column Default storage location inventory' do
8399
get request_path(request)
84100

@@ -168,6 +184,19 @@
168184
expect(response).to have_http_status(:not_found)
169185
end
170186
end
187+
188+
context 'When the request belongs to another organization' do
189+
let(:other_organization) { create(:organization) }
190+
let(:other_request) { create(:request, organization: other_organization) }
191+
192+
it 'responds with not found and does not change status' do
193+
expect do
194+
post start_request_path(other_request)
195+
end.not_to change { other_request.reload.status }
196+
197+
expect(response).to have_http_status(:not_found)
198+
end
199+
end
171200
end
172201
end
173202
end

0 commit comments

Comments
 (0)