Include shared service instances in /v2/spaces/:guid/service_instances

app/access/service_instance_access.rb

@@ -4,13 +4,13 @@ def create?(service_instance, params=nil)
       return true if admin_user?
       FeatureFlag.raise_unless_enabled!(:service_instance_creation)
       return false if service_instance.in_suspended_org?
-      service_instance.space.has_developer?(context.user) && allowed?(service_instance)
+      service_instance.space&.has_developer?(context.user) && allowed?(service_instance)
     end
 
     def read_for_update?(service_instance, params=nil)
       return true if admin_user?
       return false if service_instance.in_suspended_org?
-      service_instance.space.has_developer?(context.user)
+      service_instance.space&.has_developer?(context.user)
     end
 
     def update?(service_instance, params=nil)
@@ -20,21 +20,20 @@ def update?(service_instance, params=nil)
     def delete?(service_instance)
       return true if admin_user?
       return false if service_instance.in_suspended_org?
-      service_instance.space.has_developer?(context.user)
+      service_instance.space&.has_developer?(context.user)
     end
 
     def manage_permissions?(service_instance)
       return true if admin_user?
-      service_instance.space.has_developer?(context.user)
+      service_instance.space&.has_developer?(context.user)
     end
 
     def manage_permissions_with_token?(service_instance)
       read_with_token?(service_instance) || has_read_permissions_scope?
     end
 
     def read_permissions?(service_instance)
-      return true if admin_user? || admin_read_only_user?
-      service_instance.space.has_member?(context.user) || service_instance.space.organization.managers.include?(context.user)
+      admin_user? || admin_read_only_user? || object_is_visible_to_user?(service_instance, context.user)
     end
 
     def read_permissions_with_token?(service_instance)
@@ -43,7 +42,7 @@ def read_permissions_with_token?(service_instance)
 
     def read_env?(service_instance)
       return true if admin_user? || admin_read_only_user?
-      service_instance.space.has_developer?(context.user)
+      service_instance.space&.has_developer?(context.user)
     end
 
     def read_env_with_token?(service_instance)
@@ -64,7 +63,7 @@ def allowed?(service_instance)
     end
 
     def purge?(service_instance)
-      admin_user? || (service_instance.space.has_developer?(context.user) && service_instance.service_broker.private?)
+      admin_user? || (service_instance.space&.has_developer?(context.user) && service_instance.service_broker.private?)
     end
 
     def purge_with_token?(instance)

app/controllers/runtime/spaces_controller.rb

@@ -138,20 +138,18 @@ def enumerate_services(guid)
     def enumerate_service_instances(guid)
       space = find_guid_and_validate_access(:read, guid)
 
-      if params['return_user_provided_service_instances'] == 'true'
-        model_class = ServiceInstance
-        relation_name = :service_instances
-      else
-        model_class = ManagedServiceInstance
-        relation_name = :managed_service_instances
-      end
+      model_class = params['return_user_provided_service_instances'] == 'true' ? ServiceInstance : ManagedServiceInstance
 
       service_instances = Query.filtered_dataset_from_query_params(
         model_class,
-        space.user_visible_relationship_dataset(relation_name, @access_context.user, @access_context.admin_override),
+        model_class.user_visible(@access_context.user, @access_context.admin_override),
         ServiceInstancesController.query_parameters,
         @opts)
-      service_instances.filter(space: space)
+
+      service_instances = service_instances.filter(Sequel.or([
+        [:space, space],
+        [:shared_spaces, space]
+      ]))
 
       collection_renderer.render_json(
         ServiceInstancesController,

app/models/services/service_instance.rb

@@ -68,6 +68,10 @@ def self.user_visibility_filter(user)
         [:space, user.spaces_dataset],
         [:space, user.audited_spaces_dataset],
         [:space, user.managed_spaces_dataset],
+        [:shared_spaces, user.spaces_dataset],
+        [:shared_spaces, user.managed_spaces_dataset],
+        [:shared_spaces, user.audited_spaces_dataset],
+        [:shared_spaces, managed_organizations_spaces_dataset(user.managed_organizations_dataset)],
       ])
     end
 
@@ -134,7 +138,7 @@ def credentials_with_serialization
     alias_method_chain :credentials, 'serialization'
 
     def in_suspended_org?
-      space.in_suspended_org?
+      space&.in_suspended_org?
     end
 
     def after_create

spec/unit/access/service_instance_access_spec.rb

@@ -149,6 +149,63 @@ module VCAP::CloudController
       end
     end
 
+    context 'space developer in a space that the service instance has been shared into' do
+      before do
+        org.add_user(user)
+        target_space = VCAP::CloudController::Space.make(organization: org)
+        target_space.add_developer(user)
+        service_instance.add_shared_space(target_space)
+      end
+
+      context 'when the space of the service instance is visible' do
+        it_behaves_like :read_only_access do
+          let(:object) { service_instance }
+        end
+
+        it 'does NOT allow the user to have manage permissions of the service instance' do
+          expect(subject).to_not allow_op_on_object(:manage_permissions, service_instance)
+        end
+
+        it 'allows the user to have read permissions of the service instance' do
+          expect(subject).to allow_op_on_object(:read_permissions, service_instance)
+        end
+
+        it 'does NOT allow the user to read default credentials of the service instance' do
+          expect(subject).not_to allow_op_on_object(:read_env, service_instance)
+        end
+
+        it 'returns false for purge' do
+          expect(subject).not_to allow_op_on_object(:purge, service_instance)
+        end
+      end
+
+      context 'when the space of the service instance is not visible' do
+        before do
+          service_instance.space = nil
+        end
+
+        it_behaves_like :read_only_access do
+          let(:object) { service_instance }
+        end
+
+        it 'does NOT allow the user to have manage permissions of the service instance' do
+          expect(subject).to_not allow_op_on_object(:manage_permissions, service_instance)
+        end
+
+        it 'allows the user to have read permissions of the service instance' do
+          expect(subject).to allow_op_on_object(:read_permissions, service_instance)
+        end
+
+        it 'does NOT allow the user to read default credentials of the service instance' do
+          expect(subject).not_to allow_op_on_object(:read_env, service_instance)
+        end
+
+        it 'returns false for purge' do
+          expect(subject).not_to allow_op_on_object(:purge, service_instance)
+        end
+      end
+    end
+
     context 'organization manager (defensive)' do
       before { org.add_manager(user) }
 

spec/unit/controllers/runtime/spaces_controller_spec.rb

@@ -240,6 +240,39 @@ def decoded_guids
         end
       end
 
+      describe 'shared service instances' do
+        context 'when a service instance has been shared from another space' do
+          let(:shared_service_instance) { ManagedServiceInstance.make(space: Space.make) }
+
+          before do
+            shared_service_instance.add_shared_space(space)
+          end
+
+          it 'returns the shared service instance' do
+            get "v2/spaces/#{space.guid}/service_instances"
+
+            guids = decoded_response.fetch('resources').map { |service| service.fetch('metadata').fetch('guid') }
+            expect(guids).to include(shared_service_instance.guid)
+          end
+        end
+
+        context 'when a service instance has been shared between two spaces that are not the queried space' do
+          let(:other_space) { make_space_for_user(developer) }
+          let(:irrelevant_shared_service_instance) { ManagedServiceInstance.make(space: Space.make) }
+
+          before do
+            irrelevant_shared_service_instance.add_shared_space(other_space)
+          end
+
+          it 'does not return the irrelevant shared service instance' do
+            get "v2/spaces/#{space.guid}/service_instances"
+
+            guids = decoded_response.fetch('resources').map { |service| service.fetch('metadata').fetch('guid') }
+            expect(guids).not_to include(irrelevant_shared_service_instance.guid)
+          end
+        end
+      end
+
       context 'when there are provided service instances' do
         let!(:user_provided_service_instance) { UserProvidedServiceInstance.make(space: space) }
         let!(:managed_service_instance) { ManagedServiceInstance.make(space: space) }

spec/unit/models/services/service_instance_spec.rb

@@ -298,6 +298,14 @@ module VCAP::CloudController
           expect(service_instance).not_to be_in_suspended_org
         end
       end
+
+      context 'when the service instance space is not visible' do
+        let(:space) { nil }
+
+        it 'is false' do
+          expect(service_instance).not_to be_in_suspended_org
+        end
+      end
     end
 
     describe '#to_hash' do
@@ -326,5 +334,103 @@ module VCAP::CloudController
         expect(service_instance.to_hash(opts)['credentials']).to eq({ redacted_message: '[PRIVATE DATA HIDDEN]' })
       end
     end
+
+    describe '#user_visibility_filter' do
+      let(:developer)     { make_developer_for_space(service_instance.space) }
+      let(:auditor)       { make_auditor_for_space(service_instance.space) }
+      let(:user)          { make_user_for_space(service_instance.space) }
+      let(:org_manager)   { make_manager_for_org(service_instance.space.organization) }
+      let(:space_manager) { make_manager_for_space(service_instance.space) }
+
+      context 'when a user is an org manager where the instance was created' do
+        it 'the service instance is visible' do
+          filter = ServiceInstance.user_visibility_filter(org_manager)
+          expect(ServiceInstance.filter(filter).all.length).to eq(1)
+        end
+      end
+
+      context 'when a user is a space developer in the space the instance was created' do
+        it 'the service instance is visible' do
+          filter = ServiceInstance.user_visibility_filter(developer)
+          expect(ServiceInstance.filter(filter).all.length).to eq(1)
+        end
+      end
+
+      context 'when a user is a space auditor in the space the instance was created' do
+        it 'the service instance is visible' do
+          filter = ServiceInstance.user_visibility_filter(auditor)
+          expect(ServiceInstance.filter(filter).all.length).to eq(1)
+        end
+      end
+
+      context 'when a user is a space manager in the space the instance was created' do
+        it 'the service instance is visible' do
+          filter = ServiceInstance.user_visibility_filter(space_manager)
+          expect(ServiceInstance.filter(filter).all.length).to eq(1)
+        end
+      end
+
+      context 'when a user does not have access to the originating space' do
+        it 'the service instance is not visible' do
+          filter = ServiceInstance.user_visibility_filter(user)
+          expect(ServiceInstance.filter(filter).all.length).to eq(0)
+        end
+      end
+
+      context 'when the service instance is shared' do
+        let(:target_space)     { VCAP::CloudController::Space.make }
+        let(:target_space_dev) { make_developer_for_space(target_space) }
+        let(:target_org_user) { make_user_for_org(target_space.organization) }
+        let(:target_space_auditor) { make_auditor_for_space(target_space) }
+        let(:target_space_manager) { make_manager_for_space(target_space) }
+        let(:target_space_org_manager) { make_manager_for_org(target_space.organization) }
+
+        before do
+          service_instance.add_shared_space(target_space)
+        end
+
+        context 'when a user is a space developer in the target space' do
+          it 'the service instance is visible' do
+            filter = ServiceInstance.user_visibility_filter(target_space_dev)
+            expect(ServiceInstance.filter(filter).all.length).to eq(1)
+          end
+        end
+
+        context 'when a user is a space developer in the source space' do
+          it 'the service instance is visible' do
+            filter = ServiceInstance.user_visibility_filter(developer)
+            expect(ServiceInstance.filter(filter).all.length).to eq(1)
+          end
+        end
+
+        context 'when a user is a space auditor in the target space' do
+          it 'the service instance is visible' do
+            filter = ServiceInstance.user_visibility_filter(target_space_auditor)
+            expect(ServiceInstance.filter(filter).all.length).to eq(1)
+          end
+        end
+
+        context 'when a user is a space manager in the target space' do
+          it 'the service instance is visible' do
+            filter = ServiceInstance.user_visibility_filter(target_space_manager)
+            expect(ServiceInstance.filter(filter).all.length).to eq(1)
+          end
+        end
+
+        context 'when a user is a org manager in the target space' do
+          it 'the service instance is visible' do
+            filter = ServiceInstance.user_visibility_filter(target_space_org_manager)
+            expect(ServiceInstance.filter(filter).all.length).to eq(1)
+          end
+        end
+
+        context 'when a user does not have access to the target space' do
+          it 'the service instance is not visible' do
+            filter = ServiceInstance.user_visibility_filter(target_org_user)
+            expect(ServiceInstance.filter(filter).all.length).to eq(0)
+          end
+        end
+      end
+    end
   end
 end