From b0e0573e6b1d4df869df324e7c09bd164a970047 Mon Sep 17 00:00:00 2001 From: Michael Oleske Date: Thu, 14 Jul 2022 23:29:36 +0000 Subject: [PATCH] Check permissions on target space that is receiving ownership - return 422 if the target space either doesn't exist or you don't have write permissions for target space - next step is to start actually transferring ownership or do request body validation Authored-by: Michael Oleske --- app/controllers/v3/routes_controller.rb | 10 ++-- spec/request/routes_spec.rb | 61 ++++++++++++++++++++++--- 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/app/controllers/v3/routes_controller.rb b/app/controllers/v3/routes_controller.rb index 21b328c3cb0..76e374512c8 100644 --- a/app/controllers/v3/routes_controller.rb +++ b/app/controllers/v3/routes_controller.rb @@ -171,10 +171,12 @@ def transfer_owner FeatureFlag.raise_unless_enabled!(:route_sharing) unauthorized! unless permission_queryer.can_manage_apps_in_active_space?(route.space.guid) - # target_space_guid = hashed_params['space'] - # target_space = Space.where(guid: target_space_guid) - # check_spaces_exist_and_are_writeable!(route, [target_space_guid], [target_space]) - # resource_not_found!(:space) unless target_space && permission_queryer.can_read_from_space?(target_space_guid, target_space.organization.guid) + target_space_guid = hashed_params['space'] + target_space = Space.first(guid: target_space_guid) + if target_space.nil? || !can_write_space?(target_space) + unprocessable!("Unable to transfer owner of route #{route.uri} to space '#{target_space_guid}'. " \ + 'Ensure the space exists and that you have access to it.') + end render status: :ok, json: { status: 'ok' } end diff --git a/spec/request/routes_spec.rb b/spec/request/routes_spec.rb index 4df218931d7..e3482fb54ec 100644 --- a/spec/request/routes_spec.rb +++ b/spec/request/routes_spec.rb @@ -3155,13 +3155,12 @@ end describe 'PATCH /v3/routes/:guid/transfer-owner' do - let(:domain) { VCAP::CloudController::PrivateDomain.make(owning_organization: space.organization) } - let(:route) { VCAP::CloudController::Route.make(space: space, domain: domain, host: '') } - let(:api_call) { lambda { |user_headers| patch "/v3/routes/#{route.guid}/transfer_owner", params.to_json, user_headers } } - let(:other_space) { VCAP::CloudController::Space.make(organization: org) } - let(:params) do + let(:route) { VCAP::CloudController::Route.make(space: space) } + let(:api_call) { lambda { |user_headers| patch "/v3/routes/#{route.guid}/transfer_owner", request_body.to_json, user_headers } } + let(:target_space) { VCAP::CloudController::Space.make(organization: org) } + let(:request_body) do { - space: other_space.guid.to_s + 'space' => target_space.guid } end let(:space_dev_headers) do @@ -3175,7 +3174,7 @@ before do org.add_user(user) - other_space.add_developer(user) + target_space.add_developer(user) end context 'when the user logged in' do @@ -3192,6 +3191,54 @@ it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS end + describe 'target space to transfer to' do + context 'does not exist' do + let(:target_space_guid) { 'fake-target' } + let(:request_body) do + { + 'space' => target_space_guid + } + end + + it 'responds with 422' do + api_call.call(space_dev_headers) + + expect(last_response.status).to eq(422) + expect(parsed_response['errors']).to include( + include( + { + 'detail' => "Unable to transfer owner of route #{route.uri} to space '#{target_space_guid}'. " \ + 'Ensure the space exists and that you have access to it.', + 'title' => 'CF-UnprocessableEntity' + }) + ) + end + end + + context 'user does not have access to the target space' do + let(:no_access_target_space) { VCAP::CloudController::Space.make(organization: org) } + let(:request_body) do + { + 'space' => no_access_target_space.guid + } + end + + it 'responds with 422 and does not share the route' do + api_call.call(space_dev_headers) + + expect(last_response.status).to eq(422) + expect(parsed_response['errors']).to include( + include( + { + 'detail' => "Unable to transfer owner of route #{route.uri} to space '#{no_access_target_space.guid}'. "\ + 'Ensure the space exists and that you have access to it.', + 'title' => 'CF-UnprocessableEntity' + }) + ) + end + end + end + describe 'when route_sharing flag is disabled' do before do feature_flag.enabled = false