Implements PATCH /v3/routes/:guid/transfer_owner

Note: This is probably note the right endpoint defintion for this action

current frontrunners are:

PATCH v3/routes/:guid/relationships/space -d { "data": {"guid": "space-2-guid" }}
PATCH /v3/routes/:guid/actioins/transfer_owner -d {"guid": "space-2-guid" }

Co-authored-by: Merric de Launey <mdelauney@pivotal.io>
Co-authored-by: Michael Oleske <moleske@pivotal.io>
Co-authored-by: David Alvarado <alvaradoda@vmware.com>
Co-authored-by: Alex Rocha <alexr1@vmware.com>

app/actions/route_transfer_owner.rb

@@ -0,0 +1,30 @@
+require 'repositories/route_event_repository'
+
+module VCAP::CloudController
+  class RouteTransferOwner
+    class Error < ::StandardError
+    end
+
+    class << self
+      def transfer(route, target_space, user_audit_info)
+        return route if target_space.name == route.space.name
+
+        original_space = route.space
+        Route.db.transaction do
+          route.space = target_space
+          route.remove_shared_space(target_space)
+          route.add_shared_space(original_space)
+          route.save
+        end
+        Repositories::RouteEventRepository.new.record_route_transfer_owner(
+          route, user_audit_info, original_space, target_space.guid)
+      end
+
+      private
+
+      def error!(message)
+        raise Error.new(message)
+      end
+    end
+  end
+end

app/controllers/v3/routes_controller.rb

@@ -4,6 +4,7 @@
 require 'messages/routes_list_message'
 require 'messages/route_show_message'
 require 'messages/route_update_message'
+require 'messages/route_transfer_owner_message'
 require 'messages/route_update_destinations_message'
 require 'actions/update_route_destinations'
 require 'decorators/include_route_domain_decorator'
@@ -16,6 +17,7 @@
 require 'actions/route_update'
 require 'actions/route_share'
 require 'actions/route_unshare'
+require 'actions/route_transfer_owner'
 require 'fetchers/app_fetcher'
 require 'fetchers/route_fetcher'
 require 'fetchers/route_destinations_list_fetcher'
@@ -169,14 +171,24 @@ def relationships_shared_routes
 
   def transfer_owner
     FeatureFlag.raise_unless_enabled!(:route_sharing)
+
+    message = RouteTransferOwnerMessage.new(hashed_params[:body])
+    unprocessable!(message.errors.full_messages) unless message.valid?
+
     unauthorized! unless permission_queryer.can_manage_apps_in_active_space?(route.space.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
+    target_space = Space.first(guid: message.guid)
+    target_space_error = if target_space.nil? || !can_read_space?(target_space)
+                           'Ensure the space exists and that you have access to it.'
+                         elsif !permission_queryer.can_manage_apps_in_active_space?(target_space.guid)
+                           'Ensure that you have write permission for the target space.'
+                         elsif !permission_queryer.is_space_active?(target_space.guid)
+                           'The target organization is suspended.'
+                         end
+    unprocessable!("Unable to transfer owner of route '#{route.uri}' to space '#{message.guid}'. #{target_space_error}") unless target_space_error.nil?
+
+    RouteTransferOwner.transfer(route, target_space, user_audit_info)
+
     render status: :ok, json: { status: 'ok' }
   end
 

app/messages/route_transfer_owner_message.rb

@@ -0,0 +1,10 @@
+require 'messages/base_message'
+
+module VCAP::CloudController
+  class RouteTransferOwnerMessage < BaseMessage
+    register_allowed_keys [:guid]
+
+    validates_with NoAdditionalKeysValidator
+    validates :guid, presence: true, string: true, allow_nil: false, allow_blank: false
+  end
+end

app/repositories/route_event_repository.rb

@@ -77,6 +77,24 @@ def record_route_unshare(route, actor_audit_info, target_space_guid)
         )
       end
 
+      def record_route_transfer_owner(route, actor_audit_info, original_space, target_space_guid)
+        Event.create(
+          space:          original_space,
+          type:           'audit.route.transfer-owner',
+          actee:          route.guid,
+          actee_type:     'route',
+          actee_name:     route.host,
+          actor:          actor_audit_info.user_guid,
+          actor_type:     'user',
+          actor_name:     actor_audit_info.user_email,
+          actor_username: actor_audit_info.user_name,
+          timestamp:      Sequel::CURRENT_TIMESTAMP,
+          metadata:       {
+            target_space_guid: target_space_guid
+          }
+        )
+      end
+
       def record_route_delete_request(route, actor_audit_info, recursive)
         Event.create(
           type:              'audit.route.delete-request',

spec/request/routes_spec.rb

@@ -3148,19 +3148,15 @@
         expect(route.shared_spaces).to contain_exactly(target_space_1, target_space_2, target_space_3)
       end
     end
-
-    describe 'errors while unsharing' do
-      # isolation segments?
-    end
   end
 
-  describe 'PATCH /v3/routes/:guid/transfer-owner' do
+  describe 'PATCH /v3/routes/:guid/transfer_owner' 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' => target_space.guid
+        'guid' => target_space.guid
       }
     end
     let(:space_dev_headers) do
@@ -3191,12 +3187,32 @@
       it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
     end
 
+    it 'changes the route owner to the given space and logs an event' do
+      api_call.call(space_dev_headers)
+
+      expect(last_response.status).to eq(200)
+
+      event = VCAP::CloudController::Event.last
+      expect(event.values).to include({
+        type: 'audit.route.transfer-owner',
+        actor: user.guid,
+        actee_type: 'route',
+        actee_name: route.host,
+        space_guid: space.guid,
+        organization_guid: space.organization.guid
+      })
+      expect(event.metadata['target_space_guid']).to eq(target_space.guid)
+
+      route.reload
+      expect(route.space).to eq target_space
+    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
+            'guid' => target_space_guid
           }
         end
 
@@ -3207,19 +3223,19 @@
           expect(parsed_response['errors']).to include(
             include(
               {
-                'detail' => "Unable to transfer owner of route #{route.uri} to space '#{target_space_guid}'. " \
+                '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
+      context 'user does not have read 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
+            'guid' => no_access_target_space.guid
           }
         end
 
@@ -3230,13 +3246,76 @@
           expect(parsed_response['errors']).to include(
             include(
               {
-                'detail' => "Unable to transfer owner of route #{route.uri} to space '#{no_access_target_space.guid}'. "\
+                '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
+      context 'user does not have write access to the target space' do
+        let(:no_write_access_target_space) { VCAP::CloudController::Space.make(organization: org) }
+        let(:request_body) do
+          {
+            'guid' => no_write_access_target_space.guid
+          }
+        end
+
+        before do
+          no_write_access_target_space.add_auditor(user)
+        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_write_access_target_space.guid}'. "\
+                'Ensure that you have write permission for the target space.',
+                'title' => 'CF-UnprocessableEntity'
+              })
+          )
+        end
+      end
+    end
+
+    it 'responds with 404 when the route does not exist' do
+      patch '/v3/routes/some-fake-guid/transfer_owner', request_body.to_json, space_dev_headers
+
+      expect(last_response).to have_status_code(404)
+      expect(parsed_response['errors']).to include(
+        include(
+          {
+            'detail' => 'Route not found',
+            'title' => 'CF-ResourceNotFound'
+          })
+      )
+    end
+
+    describe 'when the request body is invalid' do
+      context 'when there are additional keys' do
+        let(:request_body) do
+          {
+            'guid' => target_space.guid,
+            'fake-key' => 'foo'
+          }
+        end
+
+        it 'should respond with 422' do
+          api_call.call(space_dev_headers)
+
+          expect(last_response.status).to eq(422)
+          expect(parsed_response['errors']).to include(
+            include(
+              {
+                'detail' => "Unknown field(s): 'fake-key'",
+                'title' => 'CF-UnprocessableEntity'
+              })
+          )
+        end
+      end
     end
 
     describe 'when route_sharing flag is disabled' do

spec/unit/actions/route_transfer_owner_spec.rb

@@ -0,0 +1,85 @@
+require 'spec_helper'
+require 'actions/route_transfer_owner'
+
+module VCAP::CloudController
+  RSpec.describe RouteTransferOwner do
+    let(:route_share) { RouteShare.new }
+    let(:route) { Route.make domain: SharedDomain.make, space: original_owning_space }
+    let(:original_owning_space) { Space.make name: 'original_owning_space' }
+    let(:target_space) { Space.make name: 'target_space' }
+    let(:shared_space) { Space.make name: 'shared_space' }
+    let(:user_audit_info) { UserAuditInfo.new(user_guid: 'user-guid-1', user_email: 'user@email.com') }
+
+    describe '#transfer' do
+      before do
+        route_share.create(route, [shared_space], user_audit_info)
+      end
+
+      it 'makes the target space the new owner' do
+        RouteTransferOwner.transfer(route, target_space, user_audit_info)
+        expect(route.space.name).to eq target_space.name
+      end
+
+      context 'route was previously shared with the target space' do
+        before do
+          route_share.create(route, [target_space], user_audit_info)
+        end
+
+        it 'removes the target space from the list of shared spaces' do
+          expect(route.shared_spaces.map(&:name)).to include target_space.name
+          RouteTransferOwner.transfer(route, target_space, user_audit_info)
+          route.reload
+          expect(route.shared_spaces.map(&:name)).not_to include target_space.name
+        end
+      end
+
+      it 'shares the route with the original owning space' do
+        expect(route.shared_spaces.map(&:name)).not_to include original_owning_space.name
+        RouteTransferOwner.transfer(route, target_space, user_audit_info)
+        route.reload
+        expect(route.shared_spaces.map(&:name)).to include original_owning_space.name
+      end
+
+      context 'target space is already the owning space' do
+        it ' it does nothing and succeeds' do
+          expect { RouteTransferOwner.transfer(route, original_owning_space, user_audit_info) }.not_to raise_error
+          expect(route.shared_spaces.map(&:name)).not_to include original_owning_space.name
+          expect(route.space.name).to eq original_owning_space.name
+        end
+      end
+
+      it 'records a transfer event' do
+        expect_any_instance_of(Repositories::RouteEventRepository).to receive(:record_route_transfer_owner).with(
+          route, user_audit_info, target_space.guid)
+
+        RouteTransferOwner.transfer(route, target_space, user_audit_info)
+      end
+
+      context 'when tranfering ownership fails' do
+        before do
+          allow(route).to receive(:save).and_raise('db failure')
+        end
+
+        it 'does not change the owning space' do
+          expect(route.space.name).to eq original_owning_space.name
+          expect {
+            RouteTransferOwner.transfer(route, target_space, user_audit_info)
+          }.to raise_error('db failure')
+          route.reload
+          expect(route.space.name).to eq original_owning_space.name
+        end
+
+        it 'does not change the shared spaces' do
+          expect(route.shared_spaces.length).to eq 1
+          expect(route.shared_spaces.map(&:name)).to include shared_space.name
+          expect {
+            RouteTransferOwner.transfer(route, target_space, user_audit_info)
+          }.to raise_error('db failure')
+          route.reload
+          expect(route.shared_spaces.map(&:name)).to include shared_space.name
+          expect(route.shared_spaces.length).to eq 1
+        end
+      end
+    end
+  end
+end

spec/unit/messages/route_transfer_owner_message_spec.rb

@@ -0,0 +1,42 @@
+require 'spec_helper'
+require 'messages/process_scale_message'
+require 'messages/base_message'
+
+module VCAP::CloudController
+  RSpec.describe RouteTransferOwnerMessage do
+    context 'when unexpected keys are requested' do
+      let(:params) { { guid: 'some-guid', unexpected: 'foo' } }
+
+      it 'is not valid' do
+        message = RouteTransferOwnerMessage.new(params)
+
+        expect(message).not_to be_valid
+        expect(message.errors.full_messages[0]).to include("Unknown field(s): 'unexpected'")
+      end
+    end
+
+    context 'when guid is not a string' do
+      let(:params) { { guid: 5 } }
+
+      it 'is not valid' do
+        message = RouteTransferOwnerMessage.new(params)
+
+        expect(message).not_to be_valid
+        expect(message.errors.count).to eq(1)
+        expect(message.errors[:guid]).to include('must be a string')
+      end
+    end
+
+    context 'when guid is not present' do
+      let(:params) { { guid: '' } }
+
+      it 'is not valid' do
+        message = RouteTransferOwnerMessage.new(params)
+
+        expect(message).not_to be_valid
+        expect(message.errors.count).to eq(1)
+        expect(message.errors[:guid]).to include("can't be blank")
+      end
+    end
+  end
+end