diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index dd184457c..6a0662b3d 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -1,6 +1,7 @@ class MergeRequestsController < ApplicationController - before_action :authenticate_user! before_action :find_resource, only: %i[update organisations update_organisations remove_merging_organisation] + before_action :authenticate_user! + before_action :authenticate_scope!, except: [:create] def create @merge_request = MergeRequest.create!(merge_request_params) @@ -72,4 +73,10 @@ private def previous_template :organisations end + + def authenticate_scope! + if current_user.organisation != @merge_request.requesting_organisation && !current_user.support? + render_not_found + end + end end diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb index cafb2df68..cc7aa2893 100644 --- a/spec/requests/merge_requests_controller_spec.rb +++ b/spec/requests/merge_requests_controller_spec.rb @@ -8,6 +8,7 @@ RSpec.describe MergeRequestsController, type: :request do let(:user) { FactoryBot.create(:user, :data_coordinator) } let(:support_user) { FactoryBot.create(:user, :support, organisation:) } let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation) } + let(:other_merge_request) { MergeRequest.create!(requesting_organisation: other_organisation) } context "when user is signed in with a data coordinator user" do before do @@ -17,25 +18,49 @@ RSpec.describe MergeRequestsController, type: :request do describe "#organisations" do let(:params) { { merge_request: { requesting_organisation_id: "9" } } } - before do - organisation.update!(name: "Test Org") - post "/merge-request", headers:, params: + context "when creating a new merge request" do + before do + organisation.update!(name: "Test Org") + post "/merge-request", headers:, params: + end + + it "creates merge request with requesting organisation" do + follow_redirect! + expect(page).to have_content("Which organisations are merging?") + expect(page).to have_content("Test Org") + expect(page).not_to have_link("Remove") + end + + context "when passing a different requesting organisation id" do + let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id } } } + + it "creates merge request with current user organisation" do + follow_redirect! + expect(MergeRequest.count).to eq(1) + expect(MergeRequest.first.requesting_organisation_id).to eq(organisation.id) + end + end end - it "creates merge request with requesting organisation" do - follow_redirect! - expect(page).to have_content("Which organisations are merging?") - expect(page).to have_content("Test Org") - expect(page).not_to have_link("Remove") + context "when viewing existing merge request" do + before do + organisation.update!(name: "Test Org") + get "/merge-request/#{merge_request.id}/organisations", headers:, params: + end + + it "shows merge request with requesting organisation" do + expect(page).to have_content("Which organisations are merging?") + expect(page).to have_content("Test Org") + end end - context "when passing a different requesting organisation id" do - let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id } } } + context "when viewing existing merge request of a different (unauthorised) organisation" do + before do + get "/merge-request/#{other_merge_request.id}/organisations", headers:, params: + end - it "creates merge request with current user organisation" do - follow_redirect! - expect(MergeRequest.count).to eq(1) - expect(MergeRequest.first.requesting_organisation_id).to eq(organisation.id) + it "shows merge request with requesting organisation" do + expect(response).to have_http_status(:not_found) end end end