Prevent service instance sharing to space where service access disabled

app/actions/service_instance_share.rb

@@ -3,18 +3,9 @@
 module VCAP::CloudController
   class ServiceInstanceShare
     def create(service_instance, target_spaces, user_audit_info)
-      supported_service_type?(service_instance)
-      service_instance_shareable?(service_instance)
-
-      if target_spaces.include?(service_instance.space)
-        raise CloudController::Errors::ApiError.new_from_details('InvalidServiceInstanceSharingTargetSpace')
-      end
-
-      target_spaces.each do |space|
-        if space.service_instances.map(&:name).include?(service_instance.name)
-          raise CloudController::Errors::ApiError.new_from_details('SharedServiceInstanceNameTaken', service_instance.name, space.name)
-        end
-      end
+      supported_service_type!(service_instance)
+      service_instance_shareable!(service_instance)
+      valid_target_spaces!(service_instance, target_spaces)
 
       ServiceInstance.db.transaction do
         target_spaces.each do |space|
@@ -30,7 +21,37 @@ def create(service_instance, target_spaces, user_audit_info)
 
     private
 
-    def supported_service_type?(service_instance)
+    def valid_target_spaces!(service_instance, target_spaces)
+      no_sharing_to_self!(service_instance, target_spaces)
+
+      target_spaces.each do |space|
+        plan_visibility!(service_instance, space)
+        name_uniqueness!(service_instance, space)
+      end
+    end
+
+    def plan_visibility!(service_instance, space)
+      visible_plans = ServicePlan.organization_visible(space.organization)
+
+      if !visible_plans.include?(service_instance.service_plan)
+        error_msg = "Access to service #{service_instance.service.label} and plan #{service_instance.service_plan.name} is not enabled in #{space.organization.name}/#{space.name}"
+        raise CloudController::Errors::ApiError.new_from_details('UnprocessableEntity', error_msg)
+      end
+    end
+
+    def name_uniqueness!(service_instance, space)
+      if space.service_instances.map(&:name).include?(service_instance.name)
+        raise CloudController::Errors::ApiError.new_from_details('SharedServiceInstanceNameTaken', service_instance.name, space.name)
+      end
+    end
+
+    def no_sharing_to_self!(service_instance, spaces)
+      if spaces.include?(service_instance.space)
+        raise CloudController::Errors::ApiError.new_from_details('InvalidServiceInstanceSharingTargetSpace')
+      end
+    end
+
+    def supported_service_type!(service_instance)
       if service_instance.route_service?
         raise CloudController::Errors::ApiError.new_from_details('RouteServiceInstanceSharingNotSupported')
       end
@@ -40,7 +61,7 @@ def supported_service_type?(service_instance)
       end
     end
 
-    def service_instance_shareable?(service_instance)
+    def service_instance_shareable!(service_instance)
       unless service_instance.shareable?
         raise CloudController::Errors::ApiError.new_from_details('ServiceShareIsDisabled', service_instance.service.label)
       end

spec/unit/actions/service_instance_share_spec.rb

@@ -147,6 +147,61 @@ module VCAP::CloudController
           end
         end
       end
+
+      context 'when the service plan is private' do
+        let(:service_plan) { ServicePlan.make(public: false) }
+        let(:service_instance) { ManagedServiceInstance.make(service_plan: service_plan) }
+
+        it 'raises an api error' do
+          error_msg = "Access to service #{service_instance.service.label} and plan #{service_instance.service_plan.name} is not " \
+            "enabled in #{target_space1.organization.name}/#{target_space1.name}"
+          expect {
+            service_instance_share.create(service_instance, [target_space1], user_audit_info)
+          }.to raise_error(CloudController::Errors::ApiError, error_msg)
+        end
+
+        context 'and access has been enabled for the target org' do
+          before do
+            ServicePlanVisibility.make(organization: target_space1.organization, service_plan: service_instance.service_plan)
+          end
+
+          it 'creates the share' 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 'and when the source org has service plan access disabled but the target org has service plan access enabled' do
+          let(:source_org) { Organization.make }
+          let(:space) { Space.make(organization: source_org) }
+          let(:service_instance) { ManagedServiceInstance.make(service_plan: service_plan, space: space) }
+
+          before do
+            ServicePlanVisibility.make(organization: target_space1.organization, service_plan: service_instance.service_plan)
+          end
+
+          it 'creates the share' 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 'and when source org has had service plan access enabled and the target org has service plan access enabled' do
+          let(:source_org) { Organization.make }
+          let(:space) { Space.make(organization: source_org) }
+          let(:service_instance) { ManagedServiceInstance.make(service_plan: service_plan, space: space) }
+
+          before do
+            ServicePlanVisibility.make(organization: target_space1.organization, service_plan: service_instance.service_plan)
+            ServicePlanVisibility.make(organization: source_org, service_plan: service_instance.service_plan)
+          end
+
+          it 'creates the share' 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
+      end
     end
   end
 end