diff --git a/app/controllers/v3/roles_controller.rb b/app/controllers/v3/roles_controller.rb index f8788176908..a07f6cf2ea5 100644 --- a/app/controllers/v3/roles_controller.rb +++ b/app/controllers/v3/roles_controller.rb @@ -173,8 +173,8 @@ def readable_roles if permission_queryer.can_read_globally? Role else - readable_by_space = { space_id: permission_queryer.readable_spaces_query.select(:id) } - readable_by_org = { organization_id: permission_queryer.readable_orgs_query.select(:id) } + readable_by_space = { space_id: permission_queryer.readable_space_ids_query } + readable_by_org = { organization_id: permission_queryer.readable_org_ids_query } Role.where { Sequel.|(readable_by_space, readable_by_org) } end end diff --git a/lib/cloud_controller/membership.rb b/lib/cloud_controller/membership.rb index 918c11a639e..178d41a1c85 100644 --- a/lib/cloud_controller/membership.rb +++ b/lib/cloud_controller/membership.rb @@ -29,13 +29,19 @@ def authorized_org_guids_subquery(roles) end def authorized_orgs_subquery(roles) + Organization.where(id: authorized_org_ids_subquery(roles)) + end + + def authorized_org_ids_subquery(roles) space_ids_query = member_space_ids(roles) - org_ids_from_space_roles = Space.where(id: space_ids_query).select(:organization_id) if space_ids_query + org_ids_query = member_org_ids(roles) - if org_ids_from_space_roles - Organization.where(id: member_org_ids(roles)).or(id: org_ids_from_space_roles) + org_ids_via_space_roles = Space.where(id: space_ids_query).select(:organization_id) if space_ids_query + + if org_ids_query && org_ids_via_space_roles + org_ids_query.union(org_ids_via_space_roles, from_self: false) else - Organization.where(id: member_org_ids(roles)) + org_ids_query || org_ids_via_space_roles end end @@ -48,11 +54,19 @@ def authorized_space_guids_subquery(roles) end def authorized_spaces_subquery(roles) - org_ids = member_org_ids(roles) - if org_ids - Space.where(id: member_space_ids(roles)).or(organization_id: org_ids).select(:id, :guid) + Space.where(id: authorized_space_ids_subquery(roles)).select(:id, :guid) + end + + def authorized_space_ids_subquery(roles) + space_ids_query = member_space_ids(roles) + org_ids_query = member_org_ids(roles) + + space_ids_via_org_roles = Space.where(organization_id: org_ids_query).select(:id) if org_ids_query + + if space_ids_query && space_ids_via_org_roles + space_ids_query.union(space_ids_via_org_roles, from_self: false) else - Space.where(id: member_space_ids(roles)).select(:id, :guid) + space_ids_query || space_ids_via_org_roles end end diff --git a/lib/cloud_controller/permissions.rb b/lib/cloud_controller/permissions.rb index 92f77d827ed..eb4ec13116e 100644 --- a/lib/cloud_controller/permissions.rb +++ b/lib/cloud_controller/permissions.rb @@ -123,6 +123,14 @@ def readable_orgs_query end end + def readable_org_ids_query + if can_read_globally? + VCAP::CloudController::Organization.select(:id) + else + membership.authorized_org_ids_subquery(ROLES_FOR_ORG_READING) + end + end + def readable_org_guids_for_domains_query if can_read_globally? VCAP::CloudController::Organization.select(:guid) @@ -168,6 +176,12 @@ def readable_spaces_query membership.authorized_spaces_subquery(ROLES_FOR_SPACE_READING) end + def readable_space_ids_query + raise 'must not be called for users that can read globally' if can_read_globally? + + membership.authorized_space_ids_subquery(ROLES_FOR_SPACE_READING) + end + def readable_space_guids_query raise 'must not be called for users that can read globally' if can_read_globally? diff --git a/spec/unit/lib/cloud_controller/permissions_spec.rb b/spec/unit/lib/cloud_controller/permissions_spec.rb index 9a3c92cd9a6..a2efd751a58 100644 --- a/spec/unit/lib/cloud_controller/permissions_spec.rb +++ b/spec/unit/lib/cloud_controller/permissions_spec.rb @@ -163,6 +163,16 @@ module VCAP::CloudController end end + describe '#readable_org_ids_query' do + it 'returns subquery from membership' do + membership = instance_double(Membership) + subquery = instance_double(Sequel::Dataset) + expect(Membership).to receive(:new).with(user).and_return(membership) + expect(membership).to receive(:authorized_org_ids_subquery).with(Permissions::ROLES_FOR_ORG_READING).and_return(subquery) + expect(permissions.readable_org_ids_query).to be(subquery) + end + end + describe '#readable_org_guids_for_domains_query' do context 'when user has valid membership' do let(:membership) { instance_double(Membership) } @@ -388,6 +398,16 @@ module VCAP::CloudController end end + describe '#readable_space_ids_query' do + it 'returns subquery from membership' do + membership = instance_double(Membership) + subquery = instance_double(Sequel::Dataset) + expect(Membership).to receive(:new).with(user).and_return(membership) + expect(membership).to receive(:authorized_space_ids_subquery).with(Permissions::ROLES_FOR_SPACE_READING).and_return(subquery) + expect(permissions.readable_space_ids_query).to be(subquery) + end + end + describe '#readables_space_guids_query' do it 'returns subquery from membership' do membership = instance_double(Membership)