Prevent service instance sharers from attempting to share with themselves

app/actions/service_instance_share.rb

@@ -3,14 +3,30 @@
 module VCAP::CloudController
   class ServiceInstanceShare
     def create(service_instance, target_spaces, user_audit_info)
+      if service_instance.route_service?
+        raise CloudController::Errors::ApiError.new_from_details('RouteServiceInstanceSharingNotSupported')
+      end
+      unless service_instance.managed_instance?
+        raise CloudController::Errors::ApiError.new_from_details('UserProvidedServiceInstanceSharingNotSupported')
+      end
+
+      unless service_instance.shareable?
+        raise CloudController::Errors::ApiError.new_from_details('ServiceShareIsDisabled', service_instance.service.label)
+      end
+
+      if target_spaces.include?(service_instance.space)
+        raise CloudController::Errors::ApiError.new_from_details('InvalidServiceInstanceSharingTargetSpace')
+      end
+
       ServiceInstance.db.transaction do
         target_spaces.each do |space|
           service_instance.add_shared_space(space)
         end
       end
 
       Repositories::ServiceInstanceShareEventRepository.record_share_event(
-        service_instance, target_spaces.map(&:guid), user_audit_info)
+        service_instance, target_spaces.map(&:guid), user_audit_info
+      )
       service_instance
     end
   end

app/controllers/v3/service_instances_controller.rb

@@ -30,7 +30,7 @@ def share_service_instance
 
     service_instance = ServiceInstance.first(guid: params[:service_instance_guid])
 
-    resource_not_found!(:service_instance) unless service_instance && can_read_space?(service_instance.space)
+    resource_not_found!(:service_instance) unless service_instance && can_read_service_instance?(service_instance)
     unauthorized! unless can_write_space?(service_instance.space)
 
     message = VCAP::CloudController::ToManyRelationshipMessage.create_from_http_request(params[:body])
@@ -52,7 +52,7 @@ def unshare_service_instance
 
     service_instance = ServiceInstance.first(guid: params[:service_instance_guid])
 
-    resource_not_found!(:service_instance) unless service_instance && can_read_space?(service_instance.space)
+    resource_not_found!(:service_instance) unless service_instance && can_read_service_instance?(service_instance)
     unauthorized! unless can_write_space?(service_instance.space)
 
     space_guid = params[:space_guid]
@@ -93,8 +93,16 @@ def check_spaces_exist_and_are_readable!(request_guids, found_spaces)
     end
   end
 
+  def can_read_service_instance?(service_instance)
+    readable_spaces = service_instance.shared_spaces + [service_instance.space]
+
+    readable_spaces.any? do |space|
+      can_read?(space.guid, space.organization_guid)
+    end
+  end
+
   def can_read_space?(space)
-    can_read?(space.guid, space.organization_guid)
+    can_read?(space.guid, space.organization.guid)
   end
 
   def can_write_space?(space)

app/models/services/managed_service_instance.rb

@@ -103,6 +103,10 @@ def route_service?
       service.route_service?
     end
 
+    def shareable?
+      service.shareable?
+    end
+
     def volume_service?
       service.volume_service?
     end

app/models/services/service.rb

@@ -129,6 +129,14 @@ def route_service?
       requires.include?('route_forwarding')
     end
 
+    def shareable?
+      return false if extra.nil?
+      metadata = JSON.parse(extra)
+      metadata && metadata['shareable']
+    rescue JSON::ParserError
+      return false
+    end
+
     def volume_service?
       requires.include?('volume_mount')
     end

app/models/services/service_instance.rb

@@ -184,6 +184,10 @@ def route_service?
       false
     end
 
+    def shareable?
+      false
+    end
+
     def volume_service?
       false
     end

spec/request/service_instances_spec.rb

@@ -168,6 +168,28 @@
       })
       expect(event.metadata['target_space_guids']).to eq([target_space.guid])
     end
+
+    context 'when the service offering has shareable false' do
+      before do
+        service_instance1.service.extra = { shareable: false }.to_json
+        service_instance1.service.save
+      end
+
+      it 'fails to share' do
+        share_request = {
+          'data' => [
+            { 'guid' => target_space.guid }
+          ]
+        }
+
+        post "/v3/service_instances/#{service_instance1.guid}/relationships/shared_spaces", share_request.to_json, admin_header
+
+        expect(last_response.status).to eq(400)
+        parsed_response = MultiJson.load(last_response.body)
+        expect(parsed_response['errors'].first['code']).to eq(390003)
+        expect(parsed_response['errors'].first['title']).to eq('CF-ServiceShareIsDisabled')
+      end
+    end
   end
 
   describe 'DELETE /v3/service_instances/:guid/relationships/shared_spaces/:space-guid' do

spec/support/fakes/blueprints.rb

@@ -205,6 +205,7 @@ module VCAP::CloudController
     active            { true }
     service_broker    { ServiceBroker.make }
     description       { Sham.description } # remove hack
+    extra             { '{"shareable": true}' }
   end
 
   Service.blueprint(:routing) do

spec/unit/actions/service_instance_share_spec.rb

@@ -5,6 +5,7 @@ module VCAP::CloudController
   RSpec.describe ServiceInstanceShare do
     let(:service_instance_share) { ServiceInstanceShare.new }
     let(:service_instance) { ManagedServiceInstance.make }
+    let(:user_provided_service_instance) { UserProvidedServiceInstance.make }
     let(:user_audit_info) { UserAuditInfo.new(user_guid: 'user-guid-1', user_email: 'user@email.com') }
     let(:target_space1) { Space.make }
     let(:target_space2) { Space.make }
@@ -29,6 +30,110 @@ module VCAP::CloudController
         expect(Repositories::ServiceInstanceShareEventRepository).to have_received(:record_share_event).with(
           service_instance, [target_space1.guid, target_space2.guid], user_audit_info)
       end
+
+      context 'when a share already exists' do
+        before do
+          service_instance.add_shared_space(target_space1)
+        end
+
+        it 'is idempotent' do
+          shared_instance = service_instance_share.create(service_instance, [target_space1], user_audit_info)
+          expect(shared_instance.shared_spaces.length).to eq 1
+        end
+      end
+
+      context 'when sharing one space from the list of spaces fails' do
+        before do
+          allow(service_instance).to receive(:add_shared_space).with(target_space1).and_call_original
+          allow(service_instance).to receive(:add_shared_space).with(target_space2).and_raise('db failure')
+        end
+
+        it 'does not share with any spaces' do
+          expect {
+            service_instance_share.create(service_instance, [target_space1, target_space2], user_audit_info)
+          }.to raise_error('db failure')
+
+          instance = ServiceInstance.find(guid: service_instance.guid)
+
+          expect(instance.shared_spaces.length).to eq 0
+        end
+
+        it 'does not audit any share events' do
+          expect(Repositories::ServiceInstanceShareEventRepository).to_not receive(:record_share_event)
+
+          expect {
+            service_instance_share.create(service_instance, [target_space1, target_space2], user_audit_info)
+          }.to raise_error('db failure')
+        end
+      end
+
+      context 'when source space is included in list of target spaces' do
+        it 'does not share with any spaces' do
+          expect {
+            service_instance_share.create(service_instance, [target_space1, service_instance.space], user_audit_info)
+          }.to raise_error(CloudController::Errors::ApiError,
+                           'Service instances cannot be shared into the space where they were created')
+
+          instance = ServiceInstance.find(guid: service_instance.guid)
+
+          expect(instance.shared_spaces.length).to eq 0
+        end
+
+        it 'does not audit any share events' do
+          expect(Repositories::ServiceInstanceShareEventRepository).to_not receive(:record_share_event)
+
+          expect {
+            service_instance_share.create(service_instance, [target_space1, service_instance.space], user_audit_info)
+          }.to raise_error(CloudController::Errors::ApiError,
+                           'Service instances cannot be shared into the space where they were created')
+        end
+      end
+
+      context 'when the service does is not shareable' do
+        before do
+          allow(service_instance).to receive(:shareable?).and_return(false)
+        end
+
+        it 'raises an api error' do
+          expect {
+            service_instance_share.create(service_instance, [target_space1, target_space2], user_audit_info)
+          }.to raise_error(CloudController::Errors::ApiError, /The #{service_instance.service.label} service does not support service instance sharing./)
+        end
+      end
+
+      context 'when the service is user-provided' do
+        it 'raises an api error' do
+          expect {
+            service_instance_share.create(user_provided_service_instance, [target_space1, target_space2], user_audit_info)
+          }.to raise_error(CloudController::Errors::ApiError, /User-provided services cannot be shared/)
+        end
+      end
+
+      context 'when the service is a route service' do
+        context 'and is a managed instance' do
+          before do
+            allow(service_instance).to receive(:route_service?).and_return(true)
+          end
+
+          it 'raises an api error' do
+            expect {
+              service_instance_share.create(service_instance, [target_space1, target_space2], user_audit_info)
+            }.to raise_error(CloudController::Errors::ApiError, /Route services cannot be shared/)
+          end
+        end
+
+        context 'and is a user-provided service instance' do
+          before do
+            allow(user_provided_service_instance).to receive(:route_service?).and_return(true)
+          end
+
+          it 'raises an api error' do
+            expect {
+              service_instance_share.create(user_provided_service_instance, [target_space1, target_space2], user_audit_info)
+            }.to raise_error(CloudController::Errors::ApiError, /Route services cannot be shared/)
+          end
+        end
+      end
     end
   end
 end