${s__('AdminUsers|You can unban their account in the future. Their data remains intact.')}
diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb index b663a75f04a57f62384ac10fc899080038add747..1477f8e0aacfa7a4beb75eb4d922fddcef467beb 100644 --- a/app/controllers/profiles/notifications_controller.rb +++ b/app/controllers/profiles/notifications_controller.rb @@ -45,7 +45,7 @@ def project_notifications_with_preloaded_associations projects = project_notifications.map(&:source) ActiveRecord::Associations::Preloader.new( records: projects, - associations: { namespace: [:route, :owner], group: [] } + associations: { namespace: [:route, :owner], group: [], creator: [] } ).call Preloaders::UserMaxAccessLevelInProjectsPreloader.new(projects, current_user).execute diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 57a9538db150f952897136a405a7baebcd2d8f93..e6ee4355fd496133bb0fb82222537de29f3ad503 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -53,6 +53,10 @@ def execute init_collection end + if Feature.enabled?(:hide_projects_of_banned_users) + collection = without_created_and_owned_by_banned_user(collection) + end + use_cte = params.delete(:use_cte) collection = Project.wrap_with_cte(collection) if use_cte collection = filter_projects(collection) @@ -282,6 +286,12 @@ def finder_params { min_access_level: params[:min_access_level] } end + + def without_created_and_owned_by_banned_user(projects) + return projects if current_user&.can?(:admin_all_resources) + + projects.without_created_and_owned_by_banned_user + end end ProjectsFinder.prepend_mod_with('ProjectsFinder') diff --git a/app/models/project.rb b/app/models/project.rb index 452a5c8973c3952435b4c973fa4821b98db80fa7..c68755bd42c105f7145de58675c9c379660919bb 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -904,6 +904,16 @@ def self.inactive scope :for_group_and_its_ancestor_groups, ->(group) { where(namespace_id: group.self_and_ancestors.select(:id)) } scope :is_importing, -> { with_import_state.where(import_state: { status: %w[started scheduled] }) } + scope :without_created_and_owned_by_banned_user, -> do + where_not_exists( + Users::BannedUser.joins( + 'INNER JOIN project_authorizations ON project_authorizations.user_id = banned_users.user_id' + ).where('projects.creator_id = banned_users.user_id') + .where('project_authorizations.project_id = projects.id') + .where(project_authorizations: { access_level: Gitlab::Access::OWNER }) + ) + end + class << self # Searches for a list of projects based on the query given in `query`. # @@ -3167,6 +3177,10 @@ def pending_delete_or_hidden? pending_delete? || hidden? end + def created_and_owned_by_banned_user? + creator.banned? && team.max_member_access(creator.id) == Gitlab::Access::OWNER + end + def content_editor_on_issues_feature_flag_enabled? group&.content_editor_on_issues_feature_flag_enabled? || Feature.enabled?(:content_editor_on_issues, self) end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index c70dc2887100c30347128a7bc5018cd37a2f4a93..3ba2b23929f1ff4abe53cf21fe631d908fea0870 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -259,6 +259,10 @@ class ProjectPolicy < BasePolicy condition(:namespace_catalog_available) { namespace_catalog_available? } + condition(:created_and_owned_by_banned_user, scope: :subject) do + Feature.enabled?(:hide_projects_of_banned_users) && @subject.created_and_owned_by_banned_user? + end + # `:read_project` may be prevented in EE, but `:read_project_for_iids` should # not. rule { guest | admin }.enable :read_project_for_iids @@ -909,6 +913,10 @@ class ProjectPolicy < BasePolicy enable :read_model_experiments end + rule { ~admin & created_and_owned_by_banned_user }.policy do + prevent :read_project + end + private def user_is_user? diff --git a/config/feature_flags/development/hide_projects_of_banned_users.yml b/config/feature_flags/development/hide_projects_of_banned_users.yml new file mode 100644 index 0000000000000000000000000000000000000000..374782c359a3e1e62bc27558b3394f50054c8ea9 --- /dev/null +++ b/config/feature_flags/development/hide_projects_of_banned_users.yml @@ -0,0 +1,8 @@ +--- +name: hide_projects_of_banned_users +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121488 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/412621 +milestone: '16.2' +type: development +group: group::anti-abuse +default_enabled: false diff --git a/doc/user/admin_area/moderate_users.md b/doc/user/admin_area/moderate_users.md index ee6d360ac1d79439c14eec7351b730726b9ba329..f7a8150dacf9d8288bbde87f3215e84eb83b3d17 100644 --- a/doc/user/admin_area/moderate_users.md +++ b/doc/user/admin_area/moderate_users.md @@ -229,8 +229,9 @@ Users can also be activated using the [GitLab API](../../api/users.md#activate-u > - Ban and unban users [generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/327353) in GitLab 14.8. Feature flag `ban_user_feature_flag` removed. > - Hiding merge requests of banned users [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107836) in GitLab 15.8 [with a flag](../../administration/feature_flags.md) named `hide_merge_requests_from_banned_users`. Disabled by default. > - Hiding comments of banned users [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112973) in GitLab 15.11 [with a flag](../../administration/feature_flags.md) named `hidden_notes`. Disabled by default. +> - Hiding projects of banned users [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121488) in GitLab 16.2 [with a flag](../../administration/feature_flags.md) named `hide_projects_of_banned_users`. Disabled by default. -GitLab administrators can ban and unban users. Banned users are blocked, and their issues, merge requests, and comments are hidden. +GitLab administrators can ban and unban users. Banned users are blocked, and their projects, issues, merge requests, and comments are hidden. ### Ban a user diff --git a/ee/lib/gitlab/elastic/search_results.rb b/ee/lib/gitlab/elastic/search_results.rb index 17614606c964d6bed635153f9c49dbfff652d1c9..fa3b9c5f1a8c9755ab5a0c2be462d4e54a3c25f4 100644 --- a/ee/lib/gitlab/elastic/search_results.rb +++ b/ee/lib/gitlab/elastic/search_results.rb @@ -30,7 +30,7 @@ def objects(scope, page: 1, per_page: DEFAULT_PER_PAGE, preload_method: nil) case scope when 'projects' - eager_load(projects, page, per_page, preload_method, [:route, :namespace, :topics]) + eager_load(projects, page, per_page, preload_method, [:route, :namespace, :topics, :creator]) when 'issues' eager_load(issues, page, per_page, preload_method, project: [:route, :namespace], labels: [], timelogs: [], assignees: []) when 'merge_requests' diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fbe56269f88fb3f0824c05ed45d89d02bce73c06..457e9b493a1a50a3a237f27b7e07882634bd6764 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3849,9 +3849,6 @@ msgstr "" msgid "AdminUsers|Is using seat" msgstr "" -msgid "AdminUsers|Issues and merge requests authored by this user are hidden from other users." -msgstr "" - msgid "AdminUsers|It's you!" msgstr "" @@ -3897,6 +3894,9 @@ msgstr "" msgid "AdminUsers|Private profile" msgstr "" +msgid "AdminUsers|Projects, issues, merge requests, and comments of this user are hidden from other users." +msgstr "" + msgid "AdminUsers|Reactivating a user will:" msgstr "" diff --git a/spec/factories/project_authorizations.rb b/spec/factories/project_authorizations.rb index ffdf5576f842f48a0f6ebee51bed76242ec1b1f1..1726da55c994bb02bf61232eae93ed0089246bbd 100644 --- a/spec/factories/project_authorizations.rb +++ b/spec/factories/project_authorizations.rb @@ -6,4 +6,8 @@ project access_level { Gitlab::Access::REPORTER } end + + trait :owner do + access_level { Gitlab::Access::OWNER } + end end diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index 3d108951c64efcb1475871a2b56621e824340449..a795df4dec6a1e6c3f43fce2a87679edca90fbd4 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ProjectsFinder do +RSpec.describe ProjectsFinder, feature_category: :groups_and_projects do include AdminModeHelper describe '#execute' do @@ -25,6 +25,12 @@ create(:project, :private, name: 'D', path: 'D') end + let_it_be(:banned_user_project) do + create(:project, :public, name: 'Project created by a banned user', creator: create(:user, :banned)).tap do |p| + create(:project_authorization, :owner, user: p.creator, project: p) + end + end + let(:params) { {} } let(:current_user) { user } let(:project_ids_relation) { nil } @@ -488,16 +494,32 @@ describe 'with admin user' do let(:user) { create(:admin) } - context 'admin mode enabled' do + context 'with admin mode enabled' do before do enable_admin_mode!(current_user) end - it { is_expected.to match_array([public_project, internal_project, private_project, shared_project]) } + it do + is_expected.to match_array([ + public_project, + internal_project, + private_project, + shared_project, + banned_user_project + ]) + end end - context 'admin mode disabled' do + context 'with admin mode disabled' do it { is_expected.to match_array([public_project, internal_project]) } + + context 'when hide_projects_of_banned_users FF is disabled' do + before do + stub_feature_flags(hide_projects_of_banned_users: false) + end + + it { is_expected.to match_array([public_project, internal_project, banned_user_project]) } + end end end end diff --git a/spec/graphql/resolvers/users/participants_resolver_spec.rb b/spec/graphql/resolvers/users/participants_resolver_spec.rb index 63a14daabba72d01535b1640a2b316fd269012fb..22111626c5b033f223d6cd46cd2c6cde7b48ecb9 100644 --- a/spec/graphql/resolvers/users/participants_resolver_spec.rb +++ b/spec/graphql/resolvers/users/participants_resolver_spec.rb @@ -137,7 +137,8 @@ # 1 extra query per source (3 emojis + 2 notes) to fetch participables collection # 2 extra queries to load work item widgets collection - expect { query.call }.not_to exceed_query_limit(control_count).with_threshold(7) + # 1 extra query to load the project creator to check if they are banned + expect { query.call }.not_to exceed_query_limit(control_count).with_threshold(8) end it 'does not execute N+1 for system note metadata relation' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f44331521e971f572f92c3c31a193d8964ed1733..9dad1032c578a02c836a8622b5f4d2d1ce00417b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -9028,6 +9028,67 @@ def has_external_wiki end end + describe '.without_created_and_owned_by_banned_user' do + let_it_be(:other_project) { create(:project) } + + subject(:results) { described_class.without_created_and_owned_by_banned_user } + + context 'when project creator is not banned' do + let_it_be(:project_of_active_user) { create(:project, creator: create(:user)) } + + it 'includes the project' do + expect(results).to match_array([other_project, project_of_active_user]) + end + end + + context 'when project creator is banned' do + let_it_be(:banned_user) { create(:user, :banned) } + let_it_be(:project_of_banned_user) { create(:project, creator: banned_user) } + + context 'when project creator is also an owner' do + let_it_be(:project_auth) do + project = project_of_banned_user + create(:project_authorization, :owner, user: project.creator, project: project) + end + + it 'excludes the project' do + expect(results).to match_array([other_project]) + end + end + + context 'when project creator is not an owner' do + it 'includes the project' do + expect(results).to match_array([other_project, project_of_banned_user]) + end + end + end + end + + describe '#created_and_owned_by_banned_user?' do + subject { project.created_and_owned_by_banned_user? } + + context 'when creator is banned' do + let_it_be(:creator) { create(:user, :banned) } + let_it_be(:project) { create(:project, creator: creator) } + + it { is_expected.to eq false } + + context 'when creator is an owner' do + let_it_be(:project_auth) do + create(:project_authorization, :owner, user: project.creator, project: project) + end + + it { is_expected.to eq true } + end + end + + context 'when creator is not banned' do + let_it_be(:project) { create(:project) } + + it { is_expected.to eq false } + end + end + it_behaves_like 'something that has web-hooks' do let_it_be_with_reload(:object) { create(:project) } diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index ee8d811971a07dc00f35bb883b2261040bf15cab..1cffad012341ee94796e1afba13e222b1854ff76 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -3309,6 +3309,32 @@ def permissions_abilities(role) end end + describe 'when project is created and owned by a banned user' do + let_it_be(:project) { create(:project, :public) } + + let(:current_user) { guest } + + before do + allow(project).to receive(:created_and_owned_by_banned_user?).and_return(true) + end + + it { expect_disallowed(:read_project) } + + context 'when current user is an admin', :enable_admin_mode do + let(:current_user) { admin } + + it { expect_allowed(:read_project) } + end + + context 'when hide_projects_of_banned_users FF is disabled' do + before do + stub_feature_flags(hide_projects_of_banned_users: false) + end + + it { expect_allowed(:read_project) } + end + end + private def project_subject(project_type)