diff --git a/app/controllers/v3/service_offerings_controller.rb b/app/controllers/v3/service_offerings_controller.rb index d4a30b1cee7..87df85530ae 100644 --- a/app/controllers/v3/service_offerings_controller.rb +++ b/app/controllers/v3/service_offerings_controller.rb @@ -34,8 +34,8 @@ def index else ServiceOfferingListFetcher.fetch( message, - readable_org_guids: permission_queryer.readable_org_guids, - readable_space_guids: permission_queryer.readable_space_scoped_space_guids, + readable_orgs: permission_queryer.readable_orgs, + readable_spaces: permission_queryer.readable_space_scoped_spaces, eager_loaded_associations: Presenters::V3::ServiceOfferingPresenter.associated_resources, ) end diff --git a/app/controllers/v3/service_plans_controller.rb b/app/controllers/v3/service_plans_controller.rb index 63c98316b0e..6246c660ac3 100644 --- a/app/controllers/v3/service_plans_controller.rb +++ b/app/controllers/v3/service_plans_controller.rb @@ -54,8 +54,8 @@ def index ServicePlanListFetcher.fetch( message, eager_loaded_associations: Presenters::V3::ServicePlanPresenter.associated_resources, - readable_org_guids: permission_queryer.readable_org_guids, - readable_space_guids: permission_queryer.readable_space_scoped_space_guids, + readable_orgs: permission_queryer.readable_orgs, + readable_spaces: permission_queryer.readable_space_scoped_spaces, ) end diff --git a/app/fetchers/base_service_list_fetcher.rb b/app/fetchers/base_service_list_fetcher.rb index f1a95a14153..03d957ce5cd 100644 --- a/app/fetchers/base_service_list_fetcher.rb +++ b/app/fetchers/base_service_list_fetcher.rb @@ -7,16 +7,16 @@ class BaseServiceListFetcher < BaseListFetcher class << self private - def select_readable(dataset, message, omniscient: false, readable_space_guids: [], readable_org_guids: []) - if readable_org_guids.any? + def select_readable(dataset, message, omniscient: false, readable_spaces: [], readable_orgs: []) + if readable_orgs.any? dataset = join_service_plans(dataset) - dataset = join_plan_orgs(dataset) - dataset = join_broker_spaces(dataset) + dataset = join_plan_org_visibilities(dataset) + dataset = join_service_brokers(dataset) dataset = dataset.where do (Sequel[:service_plans][:public] =~ true) | - (Sequel[:plan_orgs][:guid] =~ readable_org_guids) | - (Sequel[:broker_spaces][:guid] =~ readable_space_guids) + (Sequel[:service_plan_visibilities][:organization_id] =~ readable_orgs.map(&:id)) | + (Sequel[:service_brokers][:space_id] =~ readable_spaces.map(&:id)) end elsif !omniscient dataset = join_service_plans(dataset) @@ -27,7 +27,7 @@ def select_readable(dataset, message, omniscient: false, readable_space_guids: [ dataset = filter_spaces( dataset, filtered_space_guids: message.space_guids, - readable_space_guids: readable_space_guids, + readable_space_guids: readable_spaces.map(&:guid), omniscient: omniscient, ) end @@ -119,9 +119,13 @@ def join_broker_orgs(dataset) join(dataset, :left, Sequel[:organizations].as(:broker_orgs), id: Sequel[:broker_spaces][:organization_id]) end - def join_plan_orgs(dataset) + def join_plan_org_visibilities(dataset) dataset = join_service_plans(dataset) - dataset = join(dataset, :left, :service_plan_visibilities, service_plan_id: Sequel[:service_plans][:id]) + join(dataset, :left, :service_plan_visibilities, service_plan_id: Sequel[:service_plans][:id]) + end + + def join_plan_orgs(dataset) + dataset = join_plan_org_visibilities(dataset) join(dataset, :left, Sequel[:organizations].as(:plan_orgs), id: Sequel[:service_plan_visibilities][:organization_id]) end diff --git a/app/fetchers/service_offering_list_fetcher.rb b/app/fetchers/service_offering_list_fetcher.rb index 37bf09299d3..c6c12946de9 100644 --- a/app/fetchers/service_offering_list_fetcher.rb +++ b/app/fetchers/service_offering_list_fetcher.rb @@ -3,13 +3,13 @@ module VCAP::CloudController class ServiceOfferingListFetcher < BaseServiceListFetcher class << self - def fetch(message, omniscient: false, readable_space_guids: [], readable_org_guids: [], eager_loaded_associations: []) + def fetch(message, omniscient: false, readable_spaces: [], readable_orgs: [], eager_loaded_associations: []) dataset = select_readable( Service.dataset.eager(eager_loaded_associations), message, omniscient: omniscient, - readable_org_guids: readable_org_guids, - readable_space_guids: readable_space_guids, + readable_orgs: readable_orgs, + readable_spaces: readable_spaces, ) filter(message, dataset).select_all(:services).distinct diff --git a/app/fetchers/service_plan_list_fetcher.rb b/app/fetchers/service_plan_list_fetcher.rb index fc2c1950cf7..3eea5931531 100644 --- a/app/fetchers/service_plan_list_fetcher.rb +++ b/app/fetchers/service_plan_list_fetcher.rb @@ -3,13 +3,13 @@ module VCAP::CloudController class ServicePlanListFetcher < BaseServiceListFetcher class << self - def fetch(message, omniscient: false, readable_space_guids: [], readable_org_guids: [], eager_loaded_associations: []) + def fetch(message, omniscient: false, readable_spaces: [], readable_orgs: [], eager_loaded_associations: []) dataset = select_readable( ServicePlan.dataset.eager(eager_loaded_associations), message, omniscient: omniscient, - readable_org_guids: readable_org_guids, - readable_space_guids: readable_space_guids, + readable_orgs: readable_orgs, + readable_spaces: readable_spaces, ) filter(message, dataset).select_all(:service_plans).distinct diff --git a/lib/cloud_controller/membership.rb b/lib/cloud_controller/membership.rb index 8e6b04f2dbe..805649f66bb 100644 --- a/lib/cloud_controller/membership.rb +++ b/lib/cloud_controller/membership.rb @@ -17,11 +17,17 @@ def initialize(user) end def has_any_roles?(roles, space_guid=nil, org_guid=nil) - space_ids = Space.where(guid: space_guid).select(:id) - return true if SpaceRole.where(type: space_roles(roles), user_id: @user.id, space_id: space_ids).any? + if space_guid && space_role?(roles) + space_id = Space.where(guid: space_guid).select(:id) + return true unless SpaceRole.where(type: space_roles(roles), user_id: @user.id, space_id: space_id).empty? + end - org_ids = Organization.where(guid: org_guid).select(:id) - OrganizationRole.where(type: org_roles(roles), user_id: @user.id, organization_id: org_ids).any? + if org_guid && org_role?(roles) + org_id = Organization.where(guid: org_guid).select(:id) + return true unless OrganizationRole.where(type: org_roles(roles), user_id: @user.id, organization_id: org_id).empty? + end + + false end def org_guids_for_roles(roles) @@ -29,14 +35,16 @@ def org_guids_for_roles(roles) end def org_guids_for_roles_subquery(roles) - org_role_dataset = OrganizationRole.where(type: org_roles(roles), user_id: @user.id) - org_guid_dataset = Organization.where(id: org_role_dataset.select(:organization_id)).select(:guid) - - space_role_dataset = SpaceRole.where(type: space_roles(roles), user_id: @user.id) - spaces_dataset = Space.where(id: space_role_dataset.select(:space_id)) - org_guids_for_space_roles_dataset = Organization.where(id: spaces_dataset.select(:organization_id)).select(:guid) + orgs_for_roles_subquery(roles).select(:guid) + end - org_guids_for_space_roles_dataset.union(org_guid_dataset) + def orgs_for_roles_subquery(roles) + org_ids = org_ids_for_org_roles(roles) + space_ids = space_ids_for_space_roles(roles) + org_ids_from_space_roles = if space_ids + Space.where(id: space_ids).select(:organization_id) + end + Organization.where(id: org_ids).or(id: org_ids_from_space_roles).select(:id, :guid) end def space_guids_for_roles(roles) @@ -44,13 +52,13 @@ def space_guids_for_roles(roles) end def space_guids_for_roles_subquery(roles) - space_role_dataset = SpaceRole.where(type: space_roles(roles), user_id: @user.id) - space_guid_dataset = Space.where(id: space_role_dataset.select(:space_id)).select(:guid) - - org_role_dataset = OrganizationRole.where(type: org_roles(roles), user_id: @user.id) - space_guids_for_org_roles_dataset = Space.where(organization_id: org_role_dataset.select(:organization_id)).select(:guid) + spaces_for_roles_subquery(roles).select(:guid) + end - space_guids_for_org_roles_dataset.union(space_guid_dataset) + def spaces_for_roles_subquery(roles) + space_ids = space_ids_for_space_roles(roles) + org_ids = org_ids_for_org_roles(roles) + Space.where(id: space_ids).or(organization_id: org_ids).select(:id, :guid) end private @@ -62,5 +70,25 @@ def space_roles(roles) def org_roles(roles) Array(roles) & ORG_ROLES end + + def space_role?(roles) + space_roles(roles).any? + end + + def org_role?(roles) + org_roles(roles).any? + end + + def space_ids_for_space_roles(roles) + if space_role?(roles) + SpaceRole.where(type: space_roles(roles), user_id: @user.id).select(:space_id) + end + end + + def org_ids_for_org_roles(roles) + if org_role?(roles) + OrganizationRole.where(type: org_roles(roles), user_id: @user.id).select(:organization_id) + end + end end end diff --git a/lib/cloud_controller/permissions.rb b/lib/cloud_controller/permissions.rb index 9521f6e6ca2..e8beba0a955 100644 --- a/lib/cloud_controller/permissions.rb +++ b/lib/cloud_controller/permissions.rb @@ -122,6 +122,14 @@ def readable_org_guids_query end end + def readable_orgs + if can_read_globally? + VCAP::CloudController::Organization.select(:id, :guid).all + else + membership.orgs_for_roles_subquery(ROLES_FOR_ORG_READING).all + end + end + def readable_org_guids_for_domains_query if can_read_globally? VCAP::CloudController::Organization.select(:guid) @@ -241,6 +249,14 @@ def readable_space_scoped_space_guids end end + def readable_space_scoped_spaces + if can_read_globally? + VCAP::CloudController::Space.select(:id, :guid).all + else + membership.spaces_for_roles_subquery(SPACE_ROLES).all + end + end + def can_read_route?(space_guid, org_guid) return true if can_read_globally? diff --git a/spec/unit/fetchers/service_offering_list_fetcher_spec.rb b/spec/unit/fetchers/service_offering_list_fetcher_spec.rb index 1a1c0f185d9..2d49e2ad259 100644 --- a/spec/unit/fetchers/service_offering_list_fetcher_spec.rb +++ b/spec/unit/fetchers/service_offering_list_fetcher_spec.rb @@ -65,12 +65,12 @@ module VCAP::CloudController end end - context 'when `readable_org_guids` are specified' do + context 'when `readable_orgs` are specified' do it 'includes public plans and ones for those orgs' do service_offerings = fetcher.fetch( message, - readable_org_guids: [org_1.guid, org_3.guid], - readable_space_guids: [], + readable_orgs: [org_1, org_3], + readable_spaces: [], ).all expect(service_offerings).to contain_exactly( @@ -83,12 +83,12 @@ module VCAP::CloudController end end - context 'when both `readable_space_guids` and `readable_org_guids` are specified' do + context 'when both `readable_spaces` and `readable_orgs` are specified' do it 'includes public plans, ones for those spaces and ones for those orgs' do service_offerings = fetcher.fetch( message, - readable_space_guids: [space_3.guid], - readable_org_guids: [org_3.guid], + readable_spaces: [space_3], + readable_orgs: [org_3], ).all expect(service_offerings).to contain_exactly( @@ -152,8 +152,8 @@ module VCAP::CloudController service_offerings = ServiceOfferingListFetcher.fetch( message, - readable_org_guids: [org_1.guid], - readable_space_guids: [], + readable_orgs: [org_1], + readable_spaces: [], ).all expect(service_offerings).to contain_exactly(org_restricted_offering_1, public_offering) @@ -168,8 +168,8 @@ module VCAP::CloudController service_offerings = ServiceOfferingListFetcher.fetch( message, - readable_org_guids: [], - readable_space_guids: [], + readable_orgs: [], + readable_spaces: [], ).all expect(service_offerings).to contain_exactly(public_offering) @@ -187,8 +187,8 @@ module VCAP::CloudController service_offerings = ServiceOfferingListFetcher.fetch( message, - readable_org_guids: [org_1.guid], - readable_space_guids: [space_1.guid], + readable_orgs: [org_1], + readable_spaces: [space_1], ).all expect(service_offerings).to contain_exactly(public_offering, org_restricted_offering_1, space_scoped_offering_1) @@ -222,8 +222,8 @@ module VCAP::CloudController }.with_indifferent_access) service_offerings = ServiceOfferingListFetcher.fetch( message, - readable_org_guids: [org_1.guid], - readable_space_guids: [space_1.guid], + readable_orgs: [org_1], + readable_spaces: [space_1], ).all expect(service_offerings).to contain_exactly(space_scoped_offering_1, org_restricted_offering_1, public_offering) end @@ -236,8 +236,8 @@ module VCAP::CloudController }.with_indifferent_access) service_offerings = ServiceOfferingListFetcher.fetch( message, - readable_org_guids: [org_1.guid, org_2.guid], - readable_space_guids: [space_1.guid], + readable_orgs: [org_1, org_2], + readable_spaces: [space_1], ).all expect(service_offerings).to contain_exactly(space_scoped_offering_1, org_restricted_offering_1, public_offering) end @@ -251,8 +251,8 @@ module VCAP::CloudController service_offerings = ServiceOfferingListFetcher.fetch( message, - readable_org_guids: [], - readable_space_guids: [], + readable_orgs: [], + readable_spaces: [], ).all expect(service_offerings).to contain_exactly(public_offering) @@ -289,8 +289,8 @@ module VCAP::CloudController }.with_indifferent_access) service_offerings = ServiceOfferingListFetcher.fetch( message, - readable_org_guids: [org_1.guid, org_2.guid], - readable_space_guids: [space_1.guid], + readable_orgs: [org_1, org_2], + readable_spaces: [space_1], ).all expect(service_offerings).to contain_exactly(public_offering) end diff --git a/spec/unit/fetchers/service_plan_list_fetcher_spec.rb b/spec/unit/fetchers/service_plan_list_fetcher_spec.rb index f9dec2b82b1..9bbe721a88a 100644 --- a/spec/unit/fetchers/service_plan_list_fetcher_spec.rb +++ b/spec/unit/fetchers/service_plan_list_fetcher_spec.rb @@ -78,12 +78,12 @@ module VCAP::CloudController end end - context 'when `readable_org_guids` are specified' do + context 'when `readable_org` are specified' do it 'includes public plans and ones for those orgs' do service_plans = fetcher.fetch( message, - readable_org_guids: [org_1.guid, org_3.guid], - readable_space_guids: [], + readable_orgs: [org_1, org_3], + readable_spaces: [], ).all expect(service_plans).to contain_exactly( @@ -96,12 +96,12 @@ module VCAP::CloudController end end - context 'when both `readable_space_guids` and `readable_org_guids` are specified' do + context 'when both `readable_spaces` and `readable_orgs` are specified' do it 'includes public plans, ones for those spaces and ones for those orgs' do service_plans = fetcher.fetch( message, - readable_space_guids: [space_3.guid], - readable_org_guids: [org_3.guid], + readable_spaces: [space_3], + readable_orgs: [org_3], ).all expect(service_plans).to contain_exactly( @@ -165,8 +165,8 @@ module VCAP::CloudController service_plans = fetcher.fetch( message, - readable_org_guids: [org_1.guid], - readable_space_guids: [], + readable_orgs: [org_1], + readable_spaces: [], ).all expect(service_plans).to contain_exactly(org_restricted_plan_1, public_plan) @@ -181,8 +181,8 @@ module VCAP::CloudController service_plans = fetcher.fetch( message, - readable_org_guids: [], - readable_space_guids: [], + readable_orgs: [], + readable_spaces: [], ).all expect(service_plans).to contain_exactly(public_plan) @@ -200,8 +200,8 @@ module VCAP::CloudController service_plans = fetcher.fetch( message, - readable_org_guids: [org_1.guid], - readable_space_guids: [space_1.guid], + readable_orgs: [org_1], + readable_spaces: [space_1], ).all expect(service_plans).to contain_exactly(public_plan, org_restricted_plan_1, space_scoped_plan_1) @@ -235,8 +235,8 @@ module VCAP::CloudController }.with_indifferent_access) service_plans = fetcher.fetch( message, - readable_org_guids: [org_1.guid], - readable_space_guids: [space_1.guid], + readable_orgs: [org_1], + readable_spaces: [space_1], ).all expect(service_plans).to contain_exactly(space_scoped_plan_1, org_restricted_plan_1, public_plan) end @@ -249,8 +249,8 @@ module VCAP::CloudController }.with_indifferent_access) service_plans = fetcher.fetch( message, - readable_org_guids: [org_1.guid, org_2.guid], - readable_space_guids: [space_1.guid], + readable_orgs: [org_1, org_2], + readable_spaces: [space_1], ).all expect(service_plans).to contain_exactly(space_scoped_plan_1, org_restricted_plan_1, public_plan) end @@ -264,8 +264,8 @@ module VCAP::CloudController service_plans = fetcher.fetch( message, - readable_org_guids: [], - readable_space_guids: [], + readable_orgs: [], + readable_spaces: [], ).all expect(service_plans).to contain_exactly(public_plan) @@ -302,8 +302,8 @@ module VCAP::CloudController }.with_indifferent_access) service_plans = fetcher.fetch( message, - readable_org_guids: [org_1.guid, org_2.guid], - readable_space_guids: [space_1.guid], + readable_orgs: [org_1, org_2], + readable_spaces: [space_1], ).all expect(service_plans).to contain_exactly(public_plan) end @@ -464,7 +464,7 @@ module VCAP::CloudController context 'when org user' do let(:org_1) { Organization.make } let(:space_1) { Space.make(organization: org_1) } - let(:service_plans) { fetcher.fetch(message, omniscient: false, readable_space_guids: [space_1.guid], readable_org_guids: [org_1.guid]).all } + let(:service_plans) { fetcher.fetch(message, omniscient: false, readable_spaces: [space_1], readable_orgs: [org_1]).all } it_behaves_like 'filtered service plans fetcher' end