Skip to content

Commit

Permalink
Filter out space and organization names based on permissions
Browse files Browse the repository at this point in the history
When a user has access to a shared service instance (i.e. read
permissions on any of the shared spaces), the guids of all shared spaces
are visible, but only those space and organization names the user is
allowed to read based on the given roles.

Example:

  DEVELOPER who is space developer in SPACE_1, SPACE_2 and SPACE_3
  shared SERVICE_INSTANCE from SPACE_1 to SPACE_2 and SPACE_3. For each
  space there is a dedicated space auditor (AUDITOR_1, AUDITOR_2 and
  AUDITOR_3).

  SPACE_1
  -------
    DEVELOPER (space developer)
    AUDITOR_1 (space auditor)
    SERVICE_INSTANCE

  SPACE_2
  -------
    DEVELOPER (space developer)
    AUDITOR_2 (space auditor)
    shared SERVICE_INSTANCE

  SPACE_3
  -------
    DEVELOPER (space developer)
    AUDITOR_3 (space auditor)
    shared SERVICE_INSTANCE

Original behavior (before PR #3931):

  - AUDITOR_1 can see SPACE_2.guid + name and SPACE_3.guid + name
    => SPACE_2.name and SPACE_3.name should not be readable
  - AUDITOR_2 cannot see shared spaces
    => shared spaces should be readable
  - AUDITOR_3 cannot see shared spaces
    => shared spaces should be readable

Changed behavior (with PR #3931):

  - AUDITOR_1 can see SPACE_2.guid + name and SPACE_3.guid + name
    => SPACE_2.name and SPACE_3.name should not be readable
  - AUDITOR_2 can see SPACE_2.guid + name and SPACE_3.guid + name
    => SPACE_3.name should not be readable
  - AUDITOR_3 can see SPACE_2.guid + name and SPACE_3.guid + name
    => SPACE_2.name should not be readable

New behavior (this change):

  - AUDITOR_1 can see SPACE_2.guid and SPACE_3.guid
  - AUDITOR_2 can see SPACE_2.guid + name and SPACE_3.guid
  - AUDITOR_3 can see SPACE_2.guid and SPACE_3.guid + name
  • Loading branch information
philippthun committed Sep 10, 2024
1 parent ef859cb commit ddc3801
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 2 deletions.
14 changes: 13 additions & 1 deletion app/decorators/field_service_instance_organization_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 12 additions & 1 deletion app/decorators/field_service_instance_space_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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] =
{
Expand All @@ -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
23 changes: 23 additions & 0 deletions spec/request/service_instances_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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] })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'] }) }

Expand Down

0 comments on commit ddc3801

Please sign in to comment.