diff --git a/app/controllers/internal/log_access_controller.rb b/app/controllers/internal/log_access_controller.rb index b2a975a38ba..84b22daf3d5 100644 --- a/app/controllers/internal/log_access_controller.rb +++ b/app/controllers/internal/log_access_controller.rb @@ -6,18 +6,20 @@ class LogAccessController < RestController::BaseController def lookup(guid) check_read_permissions! - if roles.admin? || roles.admin_read_only? || roles.global_auditor? + if permission_queryer.can_read_globally? found = LogAccessFetcher.new.app_exists?(guid) else - allowed_space_guids = membership.authorized_space_guids_subquery(Permissions::ROLES_FOR_SPACE_READING) + allowed_space_guids = permission_queryer.readable_space_guids_query found = LogAccessFetcher.new.app_exists_by_space?(guid, allowed_space_guids) end found ? HTTP::OK : HTTP::NOT_FOUND end - def membership - @membership ||= Membership.new(current_user) + private + + def permission_queryer + @permission_queryer ||= Permissions.new(SecurityContext.current_user) end end end diff --git a/app/controllers/v3/mixins/service_permissions.rb b/app/controllers/v3/mixins/service_permissions.rb index 31babd367c9..f3110eaa3c8 100644 --- a/app/controllers/v3/mixins/service_permissions.rb +++ b/app/controllers/v3/mixins/service_permissions.rb @@ -16,7 +16,7 @@ def visible_in_readable_orgs?(service_plans) end def writable_space_scoped?(space) - space && space.has_developer?(current_user) + space && permission_queryer.can_write_to_active_space?(space.id) end def current_user_can_write?(resource) @@ -24,7 +24,7 @@ def current_user_can_write?(resource) end def visible_space_scoped?(space) - current_user && space && (space.has_member?(current_user) || space.has_supporter?(current_user)) + current_user && space && permission_queryer.can_read_from_space_as_space_member?(space.id) end def visible_to_current_user?(service: nil, plan: nil) diff --git a/app/controllers/v3/roles_controller.rb b/app/controllers/v3/roles_controller.rb index 92d1fcbec5c..f8788176908 100644 --- a/app/controllers/v3/roles_controller.rb +++ b/app/controllers/v3/roles_controller.rb @@ -166,29 +166,19 @@ def create_uaa_shadow_user(message) end def readable_users - current_user.readable_users(permission_queryer.can_read_globally?) + permission_queryer.readable_users_query end def readable_roles if permission_queryer.can_read_globally? Role else - # This rather complex filter should be applied to the overall Role dataset and is thus using the (virtual row) - # block syntax. - readable_by_space = { space_id: visible_space_ids } - readable_by_org = { organization_id: visible_org_ids } + readable_by_space = { space_id: permission_queryer.readable_spaces_query.select(:id) } + readable_by_org = { organization_id: permission_queryer.readable_orgs_query.select(:id) } Role.where { Sequel.|(readable_by_space, readable_by_org) } end end - def visible_space_ids - Space.user_visibility_filter(current_user)[:spaces__id] - end - - def visible_org_ids - Organization.user_visibility_filter(current_user)[:id] - end - def unprocessable_space! unprocessable!('Invalid space. Ensure that the space exists and you have access to it.') end diff --git a/app/controllers/v3/users_controller.rb b/app/controllers/v3/users_controller.rb index bda028dc641..a79a101ba56 100644 --- a/app/controllers/v3/users_controller.rb +++ b/app/controllers/v3/users_controller.rb @@ -93,13 +93,11 @@ def destroy private def fetch_readable_users(message) - admin_roles = permission_queryer.can_read_globally? - UserListFetcher.fetch_all(message, current_user.readable_users(admin_roles)) + UserListFetcher.fetch_all(message, permission_queryer.readable_users_query) end def fetch_user_if_readable(desired_guid) - readable_users = current_user.readable_users(permission_queryer.can_read_globally?) - readable_users.first(guid: desired_guid) + permission_queryer.readable_users_query.first(guid: desired_guid) end def user_not_found! diff --git a/lib/cloud_controller/membership.rb b/lib/cloud_controller/membership.rb index 25a7da1d3c7..918c11a639e 100644 --- a/lib/cloud_controller/membership.rb +++ b/lib/cloud_controller/membership.rb @@ -68,6 +68,17 @@ def member_org_ids(roles, extra_filters={}) end&.select(:organization_id) end + def visible_user_ids_in_orgs(org_roles) + org_ids = member_org_ids(org_roles) + return nil unless org_ids + + OrganizationUser.where(organization_id: org_ids).select(:user_id). + union(OrganizationManager.where(organization_id: org_ids).select(:user_id), from_self: false). + union(OrganizationAuditor.where(organization_id: org_ids).select(:user_id), from_self: false). + union(OrganizationBillingManager.where(organization_id: org_ids).select(:user_id), from_self: false). + select(:user_id) + end + private def role_applies_to_space?(roles, space_id) diff --git a/lib/cloud_controller/permissions.rb b/lib/cloud_controller/permissions.rb index 319a0a300a4..486bb755459 100644 --- a/lib/cloud_controller/permissions.rb +++ b/lib/cloud_controller/permissions.rb @@ -98,7 +98,7 @@ def can_write_globally? end def is_org_manager? - VCAP::CloudController::OrganizationManager.where(user_id: @user.id).any? + membership.authorized_org_guids_subquery(VCAP::CloudController::Membership::ORG_MANAGER).any? end def readable_org_guids @@ -178,6 +178,10 @@ def can_read_from_space?(space_id, org_id) can_read_globally? || membership.role_applies?(ROLES_FOR_SPACE_READING, space_id, org_id) end + def can_read_from_space_as_space_member?(space_id) + can_read_globally? || membership.role_applies?(SPACE_ROLES, space_id) + end + def can_download_droplet?(space_id, org_id) can_read_globally? || membership.role_applies?(ROLES_FOR_DROPLET_DOWLOAD, space_id, org_id) end @@ -236,11 +240,8 @@ def readable_space_scoped_spaces_query def can_read_route?(space_id) return true if can_read_globally? - space = VCAP::CloudController::Space.where(id: space_id).first - - space.has_member?(@user) || space.has_supporter?(@user) || - @user.managed_organizations.map(&:id).include?(space.organization_id) || - @user.audited_organizations.map(&:id).include?(space.organization_id) + org_id = VCAP::CloudController::Space.where(id: space_id).get(:organization_id) + membership.role_applies?(ROLES_FOR_ROUTE_READING, space_id, org_id) end def space_guids_with_readable_routes_query @@ -260,11 +261,34 @@ def can_read_system_environment_variables?(space_id, org_id) end def readable_app_guids - VCAP::CloudController::AppModel.user_visible(@user, can_read_globally?).select_map(:guid) + if can_read_globally? + VCAP::CloudController::AppModel.select_map(:guid) + elsif @user + visible_space_guids = membership.authorized_space_guids_subquery(ROLES_FOR_SPACE_READING) + VCAP::CloudController::AppModel.where(space_guid: visible_space_guids).select_map(:guid) + else + [] + end end def readable_space_quota_guids - VCAP::CloudController::SpaceQuotaDefinition.user_visible(@user, can_read_globally?).map(&:guid) + if can_read_globally? + VCAP::CloudController::SpaceQuotaDefinition.select_map(:guid) + elsif @user + visible_space_ids = membership.authorized_spaces_subquery(SPACE_ROLES).select(:id) + org_manager_org_ids = membership.authorized_orgs_subquery(VCAP::CloudController::Membership::ORG_MANAGER).select(:id) + + VCAP::CloudController::SpaceQuotaDefinition.where( + Sequel.or([ + [:id, VCAP::CloudController::Space.where(id: visible_space_ids). + exclude(space_quota_definition_id: nil). + select(:space_quota_definition_id)], + [:organization_id, org_manager_org_ids] + ]) + ).select_map(:guid) + else + [] + end end def readable_security_group_guids @@ -272,13 +296,40 @@ def readable_security_group_guids end def readable_security_group_guids_query - VCAP::CloudController::SecurityGroup.user_visible(@user, can_read_globally?).select(:guid) + if can_read_globally? + VCAP::CloudController::SecurityGroup.dataset.select(:guid) + elsif @user + visible_space_ids = membership.authorized_spaces_subquery(ROLES_FOR_SPACE_READING).select(:id) + + VCAP::CloudController::SecurityGroup.where( + Sequel.or([ + [:running_default, true], + [:staging_default, true], + [:id, VCAP::CloudController::SecurityGroupsSpace.where(space_id: visible_space_ids).select(:security_group_id). + union( + VCAP::CloudController::StagingSecurityGroupsSpace.where(staging_space_id: visible_space_ids).select(:staging_security_group_id), + from_self: false + )] + ]) + ).select(:guid) + else + VCAP::CloudController::SecurityGroup.where(id: nil).select(:guid) + end end def can_update_build_state? can_write_globally? || roles.build_state_updater? end + def readable_users_query + if can_read_globally? + VCAP::CloudController::User.dataset + else + visible_user_ids = membership.visible_user_ids_in_orgs(ROLES_FOR_ORG_READING) + VCAP::CloudController::User.where(id: visible_user_ids).or(id: @user.id) + end + end + def readable_event_dataset return VCAP::CloudController::Event.dataset if can_read_globally? diff --git a/spec/unit/lib/cloud_controller/membership_spec.rb b/spec/unit/lib/cloud_controller/membership_spec.rb index e8ff54d58ec..3c0ee4dc05b 100644 --- a/spec/unit/lib/cloud_controller/membership_spec.rb +++ b/spec/unit/lib/cloud_controller/membership_spec.rb @@ -489,6 +489,54 @@ module VCAP::CloudController end end + describe '#visible_user_ids_in_orgs' do + let(:org_roles) { Permissions::ROLES_FOR_ORG_READING } + + context 'when the user has an org role' do + let(:other_user) { User.make } + let(:manager_user) { User.make } + let(:auditor_user) { User.make } + let(:billing_user) { User.make } + + before do + organization.add_user(user) + organization.add_user(other_user) + organization.add_manager(manager_user) + organization.add_auditor(auditor_user) + organization.add_billing_manager(billing_user) + end + + it 'returns user ids from all org role types in the org' do + result = membership.visible_user_ids_in_orgs(org_roles) + user_ids = result.select_map(:user_id) + expect(user_ids).to include(user.id, other_user.id, manager_user.id, auditor_user.id, billing_user.id) + end + + it 'does not include users from other orgs' do + other_org = Organization.make + unrelated_user = User.make + other_org.add_user(unrelated_user) + + user_ids = membership.visible_user_ids_in_orgs(org_roles).select_map(:user_id) + expect(user_ids).not_to include(unrelated_user.id) + end + end + + context 'when the user has no org role' do + it 'returns an empty result set' do + result = membership.visible_user_ids_in_orgs(org_roles) + expect(result.select_map(:user_id)).to be_empty + end + end + + context 'when roles contain only space roles (no org subqueries)' do + it 'returns nil' do + result = membership.visible_user_ids_in_orgs([Membership::SPACE_DEVELOPER]) + expect(result).to be_nil + end + end + end + describe '#authorized_space_guids' do let(:user) { User.make } diff --git a/spec/unit/lib/cloud_controller/permissions_spec.rb b/spec/unit/lib/cloud_controller/permissions_spec.rb index 57fa1739f2b..9a3c92cd9a6 100644 --- a/spec/unit/lib/cloud_controller/permissions_spec.rb +++ b/spec/unit/lib/cloud_controller/permissions_spec.rb @@ -1157,6 +1157,118 @@ module VCAP::CloudController end end + describe '#readable_users_query' do + context 'when user is an admin' do + it 'returns all users' do + set_current_user(user, { admin: true }) + other_user = User.make + + result = permissions.readable_users_query + guids = result.select_map(:guid) + expect(guids).to include(user.guid) + expect(guids).to include(other_user.guid) + end + end + + context 'when user is a read-only admin' do + it 'returns all users' do + set_current_user(user, { admin_read_only: true }) + other_user = User.make + + guids = permissions.readable_users_query.select_map(:guid) + expect(guids).to include(user.guid) + expect(guids).to include(other_user.guid) + end + end + + context 'when user has an org membership' do + let(:other_user) { User.make } + let(:unrelated_user) { User.make } + let(:other_org) { Organization.make } + + before do + org.add_user(user) + org.add_user(other_user) + other_org.add_user(unrelated_user) + end + + it 'returns users from the same org plus self' do + guids = permissions.readable_users_query.select_map(:guid) + expect(guids).to include(user.guid) + expect(guids).to include(other_user.guid) + expect(guids).not_to include(unrelated_user.guid) + end + end + + context 'when user has no org membership' do + it 'returns only the current user' do + guids = permissions.readable_users_query.select_map(:guid) + expect(guids).to contain_exactly(user.guid) + end + end + end + + describe '#can_read_from_space_as_space_member?' do + context 'user has no membership' do + context 'and user is an admin' do + it 'returns true' do + set_current_user(user, { admin: true }) + expect(permissions.can_read_from_space_as_space_member?(space.id)).to be true + end + end + + context 'and user is a read-only admin' do + it 'returns true' do + set_current_user(user, { admin_read_only: true }) + expect(permissions.can_read_from_space_as_space_member?(space.id)).to be true + end + end + + context 'and user is a global auditor' do + it 'returns true' do + set_current_user_as_global_auditor + expect(permissions.can_read_from_space_as_space_member?(space.id)).to be true + end + end + + context 'and user is not an admin' do + it 'returns false' do + set_current_user(user) + expect(permissions.can_read_from_space_as_space_member?(space.id)).to be false + end + end + end + + context 'user has valid membership' do + before { org.add_user(user) } + + it 'returns true for space developer' do + space.add_developer(user) + expect(permissions.can_read_from_space_as_space_member?(space.id)).to be true + end + + it 'returns true for space manager' do + space.add_manager(user) + expect(permissions.can_read_from_space_as_space_member?(space.id)).to be true + end + + it 'returns true for space auditor' do + space.add_auditor(user) + expect(permissions.can_read_from_space_as_space_member?(space.id)).to be true + end + + it 'returns true for space supporter' do + space.add_supporter(user) + expect(permissions.can_read_from_space_as_space_member?(space.id)).to be true + end + + it 'returns false for org manager (not a space member role)' do + org.add_manager(user) + expect(permissions.can_read_from_space_as_space_member?(space.id)).to be false + end + end + end + describe '#readable_space_quota_guids' do let(:org1) { Organization.make } let(:org2) { Organization.make }