diff --git a/app/decorators/field_service_instance_organization_decorator.rb b/app/decorators/field_service_instance_organization_decorator.rb index db1cde83847..d6532bb17ac 100644 --- a/app/decorators/field_service_instance_organization_decorator.rb +++ b/app/decorators/field_service_instance_organization_decorator.rb @@ -14,17 +14,29 @@ def initialize(fields) def decorate(hash, resources) hash[:included] ||= {} + spaces = resources.map { |r| r.try(:space) || r }.uniq orgs = spaces.map(&:organization).uniq + all_orgs_readable, readable_org_guids = permissions(orgs) hash[:included][:organizations] = orgs.sort_by { |o| [o.created_at, o.guid] }.map do |org| org_view = {} - org_view[:name] = org.name if @fields.include?('name') org_view[:guid] = org.guid if @fields.include?('guid') + org_view[:name] = org.name if @fields.include?('name') && (all_orgs_readable || readable_org_guids.include?(org.guid)) org_view end hash end + + private + + # This method is used to check if the user has permissions to read the organizations and display their names. + def permissions(orgs) + permission_queryer = Permissions.new(SecurityContext.current_user) + return [true, nil] if permission_queryer.can_read_globally? + + [false, Organization.where(guid: orgs.map(&:guid)).where(guid: permission_queryer.readable_org_guids_query).select_map(:guid)] + end end end diff --git a/app/decorators/field_service_instance_space_decorator.rb b/app/decorators/field_service_instance_space_decorator.rb index 1eb2f8709c4..4e5544bfbb2 100644 --- a/app/decorators/field_service_instance_space_decorator.rb +++ b/app/decorators/field_service_instance_space_decorator.rb @@ -16,11 +16,12 @@ def decorate(hash, resources) hash[:included] ||= {} spaces = resources.map { |r| r.try(:space) || r }.uniq + all_spaces_readable, readable_space_guids = permissions(spaces) hash[:included][:spaces] = spaces.sort_by { |s| [s.created_at, s.guid] }.map do |space| temp = {} temp[:guid] = space.guid if @fields.include?('guid') - temp[:name] = space.name if @fields.include?('name') + temp[:name] = space.name if @fields.include?('name') && (all_spaces_readable || readable_space_guids.include?(space.guid)) if @fields.include?('relationships.organization') temp[:relationships] = { @@ -36,5 +37,15 @@ def decorate(hash, resources) hash end + + private + + # This method is used to check if the user has permissions to read the spaces and display their names. + def permissions(spaces) + permission_queryer = Permissions.new(SecurityContext.current_user) + return [true, nil] if permission_queryer.can_read_globally? + + [false, Space.where(guid: spaces.map(&:guid)).where(guid: permission_queryer.readable_space_guids_query).select_map(:guid)] + end end end diff --git a/spec/request/service_instances_spec.rb b/spec/request/service_instances_spec.rb index bfba2f55ed5..65561bfe5a9 100644 --- a/spec/request/service_instances_spec.rb +++ b/spec/request/service_instances_spec.rb @@ -4095,6 +4095,29 @@ def check_filtered_instances(*instances) expect({ included: parsed_response['included'] }).to match_json_response({ included: }) end + context 'when the user can read from the originating space only' do + before do + set_current_user_as_role(role: 'space_developer', org: space.organization, space: space, user: user) + end + + it 'does not include space and organization names of the shared space' do + get "/v3/service_instances/#{instance.guid}/relationships/shared_spaces?fields[space]=name,guid,relationships.organization&fields[space.organization]=name,guid", nil, + user_header + expect(last_response).to have_status_code(200) + + included = { + spaces: [ + { guid: other_space.guid, relationships: { organization: { data: { guid: other_space.organization.guid } } } } + ], + organizations: [ + { guid: other_space.organization.guid } + ] + } + + expect({ included: parsed_response['included'] }).to match_json_response({ included: }) + end + end + it 'fails for invalid resources' do get "/v3/service_instances/#{instance.guid}/relationships/shared_spaces?fields[fruit]=name", nil, admin_headers expect(last_response).to have_status_code(400) diff --git a/spec/unit/decorators/field_service_instance_organization_decorator_spec.rb b/spec/unit/decorators/field_service_instance_organization_decorator_spec.rb index 1acb5659c05..49b6fdf02eb 100644 --- a/spec/unit/decorators/field_service_instance_organization_decorator_spec.rb +++ b/spec/unit/decorators/field_service_instance_organization_decorator_spec.rb @@ -14,6 +14,10 @@ module VCAP::CloudController let(:service_instance_1) { ManagedServiceInstance.make(space: space1) } let(:service_instance_2) { UserProvidedServiceInstance.make(space: space2) } + before do + allow(Permissions).to receive(:new).and_return(double(can_read_globally?: true)) + end + it 'decorated the given hash with orgs names from service instances' do undecorated_hash = { foo: 'bar', included: { monkeys: %w[zach greg] } } decorator = described_class.new({ 'space.organization': %w[name foo] }) diff --git a/spec/unit/decorators/field_service_instance_space_decorator_spec.rb b/spec/unit/decorators/field_service_instance_space_decorator_spec.rb index 5f4d25f84d4..c5995b2d93e 100644 --- a/spec/unit/decorators/field_service_instance_space_decorator_spec.rb +++ b/spec/unit/decorators/field_service_instance_space_decorator_spec.rb @@ -14,6 +14,10 @@ module VCAP::CloudController let(:service_instance_1) { ManagedServiceInstance.make(space: space1) } let(:service_instance_2) { UserProvidedServiceInstance.make(space: space2) } + before do + allow(Permissions).to receive(:new).and_return(double(can_read_globally?: true)) + end + context 'when space guid, name and relationship.organizations are requested' do let(:decorator) { described_class.new({ space: ['relationships.organization', 'guid', 'name'] }) }