Skip to content

Commit

Permalink
Check permissions on target space that is receiving ownership
Browse files Browse the repository at this point in the history
- 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 <moleske@pivotal.io>
  • Loading branch information
moleske committed Jul 28, 2022
1 parent 2e481ce commit b0e0573
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 11 deletions.
10 changes: 6 additions & 4 deletions app/controllers/v3/routes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
61 changes: 54 additions & 7 deletions spec/request/routes_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit b0e0573

Please sign in to comment.