From e6c539b9aeaf7f3ade892cfac162f4d22e1d7cc7 Mon Sep 17 00:00:00 2001 From: Diane Russel Date: Wed, 10 Dec 2025 14:52:30 -0500 Subject: [PATCH 01/13] Update ProjectPolicy to use max_member_access_for_user for lookup_access_level --- app/policies/project_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 767481d20db351..f8a12f9c3e58d9 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -1347,7 +1347,7 @@ def lookup_access_level! return ::Gitlab::Access::REPORTER if support_bot? && service_desk_enabled? # NOTE: max_member_access has its own cache - project.team.max_member_access(@user.id) + project.max_member_access_for_user(@user) end def access_allowed_to?(feature) -- GitLab From 7c7fadab30147507b894e3d6654106aa01d58ac3 Mon Sep 17 00:00:00 2001 From: Diane Russel Date: Wed, 10 Dec 2025 15:50:52 -0500 Subject: [PATCH 02/13] Remove comment that no longer applies --- app/policies/project_policy.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index f8a12f9c3e58d9..2032009805edb5 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -465,8 +465,6 @@ class ProjectPolicy < BasePolicy rule { can?(:create_issue) }.enable :create_task - # These abilities are not allowed to admins that are not members of the project, - # that's why they are defined separately. rule { guest & can?(:download_code) }.enable :build_download_code rule { guest & can?(:read_container_image) }.enable :build_read_container_image -- GitLab From 6067f2b777e1bed2494b815dd8afd213fd7cc690 Mon Sep 17 00:00:00 2001 From: Diane Russel Date: Wed, 10 Dec 2025 16:16:06 -0500 Subject: [PATCH 03/13] Fix tests for container_image policies --- spec/policies/project_policy_spec.rb | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index c9f1b72aa4faf5..0f04a875c37290 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -3144,10 +3144,6 @@ def update_access_level(project, feature, state) describe 'container_image policies' do using RSpec::Parameterized::TableSyntax - # These are permissions that admins should not have when the project is private - # or the container registry is private. - let(:admin_excluded_permissions) { [:build_read_container_image] } - let(:anonymous_operations_permissions) { [:read_container_image] } let(:guest_operations_permissions) { anonymous_operations_permissions + [:build_read_container_image] } @@ -3318,13 +3314,7 @@ def update_access_level(project, feature, state) # Overrides `permissions_abilities` defined below to be suitable for container_image policies def permissions_abilities(role) case role - when :admin - if project_visibility == :private || access_level == ProjectFeature::PRIVATE - maintainer_operations_permissions - admin_excluded_permissions - else - maintainer_operations_permissions - end - when :maintainer, :owner + when :admin, :maintainer, :owner maintainer_operations_permissions when :developer developer_operations_permissions -- GitLab From 224181dee5ca351ce66d3b3e96cbc862567e94ce Mon Sep 17 00:00:00 2001 From: Diane Russel Date: Wed, 10 Dec 2025 16:45:18 -0500 Subject: [PATCH 04/13] Fix admin shared examples --- .../shared_examples/policies/project_policy_shared_examples.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/support/shared_examples/policies/project_policy_shared_examples.rb b/spec/support/shared_examples/policies/project_policy_shared_examples.rb index 3864cc4c8e5f58..8eedc637b98062 100644 --- a/spec/support/shared_examples/policies/project_policy_shared_examples.rb +++ b/spec/support/shared_examples/policies/project_policy_shared_examples.rb @@ -484,7 +484,6 @@ expect_allowed(*guest_permissions) expect_allowed(*planner_permissions) expect_allowed(*reporter_permissions) - expect_disallowed(*team_member_reporter_permissions) expect_allowed(*developer_permissions) expect_allowed(*maintainer_permissions) expect_allowed(*admin_permissions) -- GitLab From 1cec85f91c1e19d8279ec096c2d16f755ecb90d8 Mon Sep 17 00:00:00 2001 From: Diane Russel Date: Wed, 10 Dec 2025 17:20:30 -0500 Subject: [PATCH 05/13] Fix organization owner shared examples --- .../shared_examples/policies/project_policy_shared_examples.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/support/shared_examples/policies/project_policy_shared_examples.rb b/spec/support/shared_examples/policies/project_policy_shared_examples.rb index 8eedc637b98062..1d834a5b1bb3c8 100644 --- a/spec/support/shared_examples/policies/project_policy_shared_examples.rb +++ b/spec/support/shared_examples/policies/project_policy_shared_examples.rb @@ -452,7 +452,6 @@ expect_allowed(*guest_permissions) expect_allowed(*planner_permissions) expect_allowed(*reporter_permissions) - expect_disallowed(*team_member_reporter_permissions) expect_allowed(*developer_permissions) expect_allowed(*maintainer_permissions) expect_allowed(*owner_permissions) -- GitLab From 52a67f5ee6c4e4714da12a6808712b1861970dee Mon Sep 17 00:00:00 2001 From: Diane Russel Date: Wed, 10 Dec 2025 19:25:39 -0500 Subject: [PATCH 06/13] Fix test for set_license_information_source --- ee/spec/policies/project_policy_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index d3f2d27fa3c850..f521944682af80 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -5633,7 +5633,7 @@ def create_member_role(member, abilities = member_role_abilities) context 'when admin_security_testing is disabled' do before do - allow(Ability).to receive(:allowed?).with(current_user, :admin_all_resources, :global) + allow(Ability).to receive(:allowed?).and_call_original allow(Ability).to receive(:allowed?).with(current_user, :admin_security_testing, project).and_return(false) end -- GitLab From 5d2ea81c6a7153c1cad45a54b7e8f5bf0f8b8216 Mon Sep 17 00:00:00 2001 From: Diane Russel Date: Fri, 12 Dec 2025 11:02:23 -0500 Subject: [PATCH 07/13] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Jay --- app/policies/project_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 2032009805edb5..0b834112ba04ee 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -1344,7 +1344,7 @@ def lookup_access_level! return ::Gitlab::Access::REPORTER if alert_bot? return ::Gitlab::Access::REPORTER if support_bot? && service_desk_enabled? - # NOTE: max_member_access has its own cache + # NOTE: max_member_access_for_user is cached project.max_member_access_for_user(@user) end -- GitLab From 75412ea1e45dd13e2b44af9780f7a23c4359a638 Mon Sep 17 00:00:00 2001 From: Diane Russel Date: Fri, 12 Dec 2025 13:37:41 -0500 Subject: [PATCH 08/13] Update tests --- spec/finders/resource_milestone_event_finder_spec.rb | 2 +- spec/graphql/resolvers/group_labels_resolver_spec.rb | 2 +- spec/lib/gitlab/ci/config/external/mapper/verifier_spec.rb | 2 +- spec/lib/gitlab/git_access_spec.rb | 2 +- .../features/search/redacted_search_results_shared_examples.rb | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/finders/resource_milestone_event_finder_spec.rb b/spec/finders/resource_milestone_event_finder_spec.rb index a05059328e3ab0..34a3f900a2a105 100644 --- a/spec/finders/resource_milestone_event_finder_spec.rb +++ b/spec/finders/resource_milestone_event_finder_spec.rb @@ -60,7 +60,7 @@ create_event(milestone2, :remove) # 1 milestones + 1 project + 1 user + 4 ability - expect { described_class.new(user, issue).execute }.not_to exceed_query_limit(control).with_threshold(6) + expect { described_class.new(user, issue).execute }.not_to exceed_query_limit(control).with_threshold(10) end end diff --git a/spec/graphql/resolvers/group_labels_resolver_spec.rb b/spec/graphql/resolvers/group_labels_resolver_spec.rb index c68498173f13f4..058a694ea18771 100644 --- a/spec/graphql/resolvers/group_labels_resolver_spec.rb +++ b/spec/graphql/resolvers/group_labels_resolver_spec.rb @@ -79,7 +79,7 @@ Gitlab::SafeRequestStore.ensure_request_store do resolve_labels(group, params).to_a end - end.not_to exceed_query_limit(control) + end.not_to exceed_query_limit(control).with_threshold(1) end end diff --git a/spec/lib/gitlab/ci/config/external/mapper/verifier_spec.rb b/spec/lib/gitlab/ci/config/external/mapper/verifier_spec.rb index f542c0485e0725..59a6f62ecd971f 100644 --- a/spec/lib/gitlab/ci/config/external/mapper/verifier_spec.rb +++ b/spec/lib/gitlab/ci/config/external/mapper/verifier_spec.rb @@ -144,7 +144,7 @@ # We could not reduce the number of projects queries because we need to call project for # the `can_access_local_content?` and `sha` BatchLoaders. expect(projects_queries.values.sum).to eq(2) - expect(access_check_queries.values.sum).to eq(2) + expect(access_check_queries.values.sum).to eq(0) end context 'when a project is missing' do diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 2131b386b9f19c..b96781a4f0ee2c 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -731,7 +731,7 @@ def disable_protocol(protocol) context 'when is not member of the project' do context 'pull code' do - it { expect { pull_access_check }.to raise_forbidden(described_class::ERROR_MESSAGES[:download]) } + it { expect { pull_access_check }.not_to raise_error } end end end diff --git a/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb b/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb index d98b46047756ff..176429f15b7ebe 100644 --- a/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb +++ b/spec/support/shared_examples/features/search/redacted_search_results_shared_examples.rb @@ -253,7 +253,7 @@ def kaminari_array(*objects) end context 'with :with_api_entity_associations' do - it_behaves_like "redaction limits N+1 queries", limit: 15 + it_behaves_like "redaction limits N+1 queries", limit: 17 end end -- GitLab From 5054f29e762d54a718ca8c5ae706c5b9ff9d151a Mon Sep 17 00:00:00 2001 From: Diane Russel Date: Fri, 12 Dec 2025 13:56:38 -0500 Subject: [PATCH 09/13] Refactor test helper to stub max_member_access_for_user --- spec/support/stub_member_access_level.rb | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/spec/support/stub_member_access_level.rb b/spec/support/stub_member_access_level.rb index 62e932ee1fca2f..170335a8e76170 100644 --- a/spec/support/stub_member_access_level.rb +++ b/spec/support/stub_member_access_level.rb @@ -4,42 +4,32 @@ module StubMemberAccessLevel # Stubs access level of a member of +object+. # # The following types are supported: - # * `Project` - stubs `project.team.max_member_access(user.id)` + # * `Project` - stubs `project.max_member_access_for_user(user)` # * `Group` - stubs `group.max_member_access_for_user(user)` # # @example # # stub_member_access_level(project, maintainer: user) - # project.team.max_member_access(user.id) # => Gitlab::Access::MAINTAINER + # project.max_member_access_for_user(user) # => Gitlab::Access::MAINTAINER # # stub_member_access_level(group, developer: user) # group.max_member_access_for_user(user) # => Gitlab::Access::DEVELOPER # # stub_member_access_level(project, reporter: user, guest: [guest1, guest2]) - # project.team.max_member_access(user.id) # => Gitlab::Access::REPORTER - # project.team.max_member_access(guests.first.id) # => Gitlab::Access::GUEST - # project.team.max_member_access(guests.last.id) # => Gitlab::Access::GUEST + # project.max_member_access_for_user(user) # => Gitlab::Access::REPORTER + # project.max_member_access_for_user(guests.first) # => Gitlab::Access::GUEST + # project.max_member_access_for_user(guests.last) # => Gitlab::Access::GUEST # # @param object [Project, Group] Object to be stubbed. # @param access_levels [Hash, Hash] Map of access level to users def stub_member_access_level(object, **access_levels) - expectation = case object - when Project - ->(user) { expect(object.team).to receive(:max_member_access).with(user.id) } - when Group - ->(user) { expect(object).to receive(:max_member_access_for_user).with(user) } - else - raise ArgumentError, - "Stubbing member access level unsupported for #{object.inspect} (#{object.class})" - end - access_levels.each do |access_level, users| access_level = Gitlab::Access.sym_options_with_owner.fetch(access_level) do raise ArgumentError, "Invalid access level #{access_level.inspect}" end Array(users).each do |user| - expectation.call(user).at_least(1).times.and_return(access_level) + expect(object).to receive(:max_member_access_for_user).with(user).at_least(:once).and_return(access_level) end end end -- GitLab From 1ee5860926c38a16914cd97dfe729d42691fda79 Mon Sep 17 00:00:00 2001 From: Diane Russel Date: Fri, 12 Dec 2025 14:06:23 -0500 Subject: [PATCH 10/13] Update tests --- .../users_max_access_level_by_project_preloader_spec.rb | 4 +++- spec/services/repositories/changelog_service_spec.rb | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/models/preloaders/users_max_access_level_by_project_preloader_spec.rb b/spec/models/preloaders/users_max_access_level_by_project_preloader_spec.rb index 4129d141252074..21a15b25dfb803 100644 --- a/spec/models/preloaders/users_max_access_level_by_project_preloader_spec.rb +++ b/spec/models/preloaders/users_max_access_level_by_project_preloader_spec.rb @@ -44,7 +44,9 @@ end end - expect(policy_queries).not_to exceed_query_limit(1) + # Threshold accounts for organization admin privilege checks added when + # aligning ProjectPolicy with GroupPolicy. These are cached per request. + expect(policy_queries).not_to exceed_query_limit(1).with_threshold(2) end end end diff --git a/spec/services/repositories/changelog_service_spec.rb b/spec/services/repositories/changelog_service_spec.rb index 35407c127b33f3..b605281953f03b 100644 --- a/spec/services/repositories/changelog_service_spec.rb +++ b/spec/services/repositories/changelog_service_spec.rb @@ -79,7 +79,7 @@ recorder = ActiveRecord::QueryRecorder.new { service.execute(commit_to_changelog: commit_to_changelog) } changelog = project.repository.blob_at('master', 'CHANGELOG.md')&.data - expect(recorder.count).to eq(15) + expect(recorder.count).to eq(14) expect(changelog).to include('Title 1', 'Title 2') end -- GitLab From e542536140f9b7caff1c34daf5037cdb8cb9c2cb Mon Sep 17 00:00:00 2001 From: Diane Russel Date: Fri, 12 Dec 2025 16:19:16 -0500 Subject: [PATCH 11/13] Remove unnecessary test --- spec/support_specs/stub_member_access_level_spec.rb | 9 --------- 1 file changed, 9 deletions(-) diff --git a/spec/support_specs/stub_member_access_level_spec.rb b/spec/support_specs/stub_member_access_level_spec.rb index c76bd2ee41789c..1a445bf5be0e7f 100644 --- a/spec/support_specs/stub_member_access_level_spec.rb +++ b/spec/support_specs/stub_member_access_level_spec.rb @@ -56,14 +56,5 @@ def access_level_for(user) end end end - - context 'with unsupported object' do - let(:object) { :a_symbol } - - it 'raises an error' do - expect { stub_member_access_level(object) } - .to raise_error(ArgumentError, "Stubbing member access level unsupported for :a_symbol (Symbol)") - end - end end end -- GitLab From f1b4a3a6de4d3f3131c11ee77e5dbf800a7d8574 Mon Sep 17 00:00:00 2001 From: Diane Russel Date: Fri, 12 Dec 2025 16:57:18 -0500 Subject: [PATCH 12/13] Fix test --- spec/support_specs/stub_member_access_level_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support_specs/stub_member_access_level_spec.rb b/spec/support_specs/stub_member_access_level_spec.rb index 1a445bf5be0e7f..e7e47e4567e3ec 100644 --- a/spec/support_specs/stub_member_access_level_spec.rb +++ b/spec/support_specs/stub_member_access_level_spec.rb @@ -42,7 +42,7 @@ it_behaves_like 'access level stubs' do def access_level_for(user) - object.team.max_member_access(user.id) + object.max_member_access_for_user(user) end end end -- GitLab From d56c5c2a03669efb8087b8f364c8ffb1cd8d4946 Mon Sep 17 00:00:00 2001 From: Diane Russel Date: Fri, 12 Dec 2025 22:04:24 -0500 Subject: [PATCH 13/13] Remove old comment --- spec/finders/resource_milestone_event_finder_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/finders/resource_milestone_event_finder_spec.rb b/spec/finders/resource_milestone_event_finder_spec.rb index 34a3f900a2a105..3ececbd27f0b92 100644 --- a/spec/finders/resource_milestone_event_finder_spec.rb +++ b/spec/finders/resource_milestone_event_finder_spec.rb @@ -59,7 +59,6 @@ create_event(milestone2, :add) create_event(milestone2, :remove) - # 1 milestones + 1 project + 1 user + 4 ability expect { described_class.new(user, issue).execute }.not_to exceed_query_limit(control).with_threshold(10) end end -- GitLab