diff --git a/app/actions/service_instance_share.rb b/app/actions/service_instance_share.rb index fae64b13749..86dc7790677 100644 --- a/app/actions/service_instance_share.rb +++ b/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 diff --git a/spec/unit/actions/service_instance_share_spec.rb b/spec/unit/actions/service_instance_share_spec.rb index f1db26c3122..f6bd2f7823f 100644 --- a/spec/unit/actions/service_instance_share_spec.rb +++ b/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