diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 767481d20db351f5fb25e7c0596591f5fbe24f94..0b834112ba04ee8720d025c0a2f4bdbb1222a311 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 @@ -1346,8 +1344,8 @@ 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 - project.team.max_member_access(@user.id) + # NOTE: max_member_access_for_user is cached + project.max_member_access_for_user(@user) end def access_allowed_to?(feature) diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index d3f2d27fa3c85059b285cf1320d7361283c25a99..f521944682af80ced2e319de1c335c3cc593787c 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 diff --git a/spec/finders/resource_milestone_event_finder_spec.rb b/spec/finders/resource_milestone_event_finder_spec.rb index a05059328e3ab0ed7bd0fdd88e0811fac5e8afaa..3ececbd27f0b92afd5255026dd72c6cd9d51f7c4 100644 --- a/spec/finders/resource_milestone_event_finder_spec.rb +++ b/spec/finders/resource_milestone_event_finder_spec.rb @@ -59,8 +59,7 @@ 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(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 c68498173f13f41b2300b74350e4665490ba0853..058a694ea187719756c3c09d1640158c547932df 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 f542c0485e072539fde347a646d9a576c99044b6..59a6f62ecd971f4038835263539f3747f42e28b1 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 2131b386b9f19c88adfc6d2d59c573fcc960d612..b96781a4f0ee2cd68b4267eb6f55ba8e5641ef34 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/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 4129d14125207433a69c856840266014bff8773c..21a15b25dfb8039782a4e1a5849d2bb7a4ed0128 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/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index c9f1b72aa4faf506689b54209effcc1cc2b0c0aa..0f04a875c37290da136ba30c09a3d5c14aeb19f1 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 diff --git a/spec/services/repositories/changelog_service_spec.rb b/spec/services/repositories/changelog_service_spec.rb index 35407c127b33f3f84d27ac354f032a780bc84468..b605281953f03b23ce4bbf1c68ff591f63e659e3 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 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 d98b46047756ffb9f621d582dece06f30485420d..176429f15b7ebebc943565d5337239d29fea7aa4 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 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 3864cc4c8e5f584418ccb5c5f6a89e3c3f07510b..1d834a5b1bb3c88b41dce70dc8f1ec5fb8de771c 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) @@ -484,7 +483,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) diff --git a/spec/support/stub_member_access_level.rb b/spec/support/stub_member_access_level.rb index 62e932ee1fca2f55956de5ef18e0e28485a66374..170335a8e76170e975fb4a4902ec5c41a8aa37aa 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 diff --git a/spec/support_specs/stub_member_access_level_spec.rb b/spec/support_specs/stub_member_access_level_spec.rb index c76bd2ee41789c78db06c7d8e0fdbd969f747506..e7e47e4567e3ecd6d6a52ebe3e192cad42e7e25e 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 @@ -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