From 682f6ac9298a60638dca29e53be6c0aacfbe79c7 Mon Sep 17 00:00:00 2001
From: Alex Pooley <apooley@gitlab.com>
Date: Fri, 29 Mar 2024 00:06:00 +0000
Subject: [PATCH] Fixes to specs
Bump query counts
Reset admin factory
---
app/models/group.rb | 6 ++-
app/models/user.rb | 14 ++++++
app/policies/group_policy.rb | 16 +++++-
ee/spec/features/issues/issue_actions_spec.rb | 2 +-
ee/spec/models/ee/event_spec.rb | 2 +-
.../add_on_purchase_policy_spec.rb | 2 +-
spec/controllers/groups_controller_spec.rb | 2 +-
spec/features/groups_spec.rb | 2 +-
spec/features/projects/new_project_spec.rb | 2 +-
.../resolvers/project_issues_resolver_spec.rb | 6 +--
.../resolvers/work_items_resolver_spec.rb | 4 +-
.../project/relation_factory_spec.rb | 2 +-
.../import_export/project/tree_saver_spec.rb | 2 +-
spec/models/group_spec.rb | 17 ++++++-
spec/models/user_spec.rb | 50 +++++++++++++++++++
spec/policies/group_policy_spec.rb | 14 ++++++
spec/policies/upload_policy_spec.rb | 2 +-
.../project_export_presenter_spec.rb | 2 +-
spec/requests/api/user_runners_spec.rb | 2 +-
.../ci/runners/create_runner_service_spec.rb | 2 +-
.../create_service_spec.rb | 2 +-
spec/support/matchers/access_matchers.rb | 4 +-
.../access_matchers_for_controller.rb | 2 +-
.../runners_resolver_shared_context.rb | 2 +-
.../policies/group_policy_shared_context.rb | 10 +++-
.../models/member_shared_examples.rb | 4 +-
.../requests/admin_mode_shared_examples.rb | 2 +-
27 files changed, 147 insertions(+), 30 deletions(-)
diff --git a/app/models/group.rb b/app/models/group.rb
index 5bad84d74e569..707b15823ef97 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -715,7 +715,11 @@ def users_count
# @param only_concrete_membership [Bool] whether require admin concrete membership status
def max_member_access_for_user(user, only_concrete_membership: false)
return GroupMember::NO_ACCESS unless user
- return GroupMember::OWNER if user.can_admin_all_resources? && !only_concrete_membership
+
+ unless only_concrete_membership
+ return GroupMember::OWNER if user.can_admin_all_resources?
+ return GroupMember::OWNER if user.can_admin_organization?(organization_id)
+ end
max_member_access(user)
end
diff --git a/app/models/user.rb b/app/models/user.rb
index eff607fc1a042..dddca0e1f9e67 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -2059,6 +2059,20 @@ def can_admin_all_resources?
can?(:admin_all_resources)
end
+ def owns_organization?(organization)
+ return false unless organization
+
+ organization_id = organization.is_a?(Integer) ? organization : organization.id
+
+ organization_users.where(organization_id: organization_id).owner.exists?
+ end
+
+ def can_admin_organization?(organization)
+ strong_memoize_with(:can_admin_organization, organization) do
+ owns_organization?(organization)
+ end
+ end
+
def update_two_factor_requirement
periods = expanded_groups_requiring_two_factor_authentication.pluck(:two_factor_grace_period)
diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb
index 8c2664d638a3e..c145b44bbccfd 100644
--- a/app/policies/group_policy.rb
+++ b/app/policies/group_policy.rb
@@ -32,6 +32,15 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
group_projects_for(user: @user, group: @subject).any?
end
+ desc "User owns the group's organization"
+ condition(:organization_owner) do
+ if @user.is_a?(User)
+ @user.owns_organization?(@subject.organization_id)
+ else
+ false
+ end
+ end
+
with_options scope: :subject, score: 0
condition(:request_access_enabled) { @subject.request_access_enabled }
@@ -124,8 +133,11 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
enable :award_emoji
end
- rule { admin }.policy do
+ rule { admin | organization_owner }.policy do
enable :read_group
+ end
+
+ rule { admin }.policy do
enable :update_max_artifacts_size
end
@@ -289,7 +301,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
prevent :import_projects
end
- rule { owner | admin }.policy do
+ rule { owner | admin | organization_owner }.policy do
enable :owner_access
enable :read_statistics
end
diff --git a/ee/spec/features/issues/issue_actions_spec.rb b/ee/spec/features/issues/issue_actions_spec.rb
index 786baa332bc26..e77eaef3703c3 100644
--- a/ee/spec/features/issues/issue_actions_spec.rb
+++ b/ee/spec/features/issues/issue_actions_spec.rb
@@ -33,7 +33,7 @@
visit project_issue_path(project, issue)
# TODO: restore threshold after epic-work item sync
# issue: https://gitlab.com/gitlab-org/gitlab/-/issues/438295
- allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(107)
+ allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(108)
end
it 'clicking "Promote to epic" creates and redirects user to epic' do
diff --git a/ee/spec/models/ee/event_spec.rb b/ee/spec/models/ee/event_spec.rb
index 6e27c5188e2f7..6c2d82e59f950 100644
--- a/ee/spec/models/ee/event_spec.rb
+++ b/ee/spec/models/ee/event_spec.rb
@@ -9,7 +9,7 @@
let_it_be(:guest) { create(:user) }
let_it_be(:reporter) { create(:user) }
let_it_be(:author) { create(:author) }
- let_it_be(:admin) { create(:admin) }
+ let_it_be(:admin) { create(:admin, :without_default_org) }
let(:users) { [non_member, member, reporter, guest, author, admin] }
diff --git a/ee/spec/policies/gitlab_subscriptions/add_on_purchase_policy_spec.rb b/ee/spec/policies/gitlab_subscriptions/add_on_purchase_policy_spec.rb
index e57e0cbf8df01..d5d2e94ab50d0 100644
--- a/ee/spec/policies/gitlab_subscriptions/add_on_purchase_policy_spec.rb
+++ b/ee/spec/policies/gitlab_subscriptions/add_on_purchase_policy_spec.rb
@@ -7,7 +7,7 @@
using RSpec::Parameterized::TableSyntax
let_it_be(:group) { create(:group) }
- let_it_be(:admin) { create(:admin) }
+ let_it_be(:admin) { create(:admin, :without_default_org) }
let_it_be(:owner) { create(:user) }
let_it_be(:maintainer) { create(:user) }
let_it_be(:developer) { create(:user) }
diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index 01bb3761a2575..c0c07d51896c1 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -10,7 +10,7 @@
let_it_be_with_refind(:project) { create(:project, namespace: group) }
let_it_be(:user) { create(:user) }
let_it_be(:admin_with_admin_mode) { create(:admin) }
- let_it_be(:admin_without_admin_mode) { create(:admin) }
+ let_it_be(:admin_without_admin_mode) { create(:admin, :without_default_org) }
let_it_be(:group_member) { create(:group_member, group: group, user: user) }
let_it_be(:owner) { group.add_owner(create(:user)).user }
let_it_be(:maintainer) { group.add_maintainer(create(:user)).user }
diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb
index 8d20ee90477ac..39d8033a7773a 100644
--- a/spec/features/groups_spec.rb
+++ b/spec/features/groups_spec.rb
@@ -248,7 +248,7 @@
let_it_be(:group) { create(:group, path: 'foo') }
context 'as admin' do
- let(:user) { create(:admin) }
+ let(:user) { create(:admin, :without_default_org) }
before do
visit new_group_path(parent_id: group.id, anchor: 'create-group-pane')
diff --git a/spec/features/projects/new_project_spec.rb b/spec/features/projects/new_project_spec.rb
index d6b27d8c618fa..cf9c0012e29c0 100644
--- a/spec/features/projects/new_project_spec.rb
+++ b/spec/features/projects/new_project_spec.rb
@@ -92,7 +92,7 @@
end
context 'as an admin' do
- let(:user) { create(:admin) }
+ let(:user) { create(:admin, :without_default_org) }
it_behaves_like 'shows correct navigation'
diff --git a/spec/graphql/resolvers/project_issues_resolver_spec.rb b/spec/graphql/resolvers/project_issues_resolver_spec.rb
index 593489290e006..d47407cc0f27c 100644
--- a/spec/graphql/resolvers/project_issues_resolver_spec.rb
+++ b/spec/graphql/resolvers/project_issues_resolver_spec.rb
@@ -607,13 +607,13 @@
end
it 'finds a specific issue with iid', :request_store do
- result = batch_sync(max_queries: 8) { resolve_issues(iid: issue1.iid).to_a }
+ result = batch_sync(max_queries: 11) { resolve_issues(iid: issue1.iid).to_a }
expect(result).to contain_exactly(issue1)
end
it 'batches queries that only include IIDs', :request_store do
- result = batch_sync(max_queries: 8) do
+ result = batch_sync(max_queries: 11) do
[issue1, issue2]
.map { |issue| resolve_issues(iid: issue.iid.to_s) }
.flat_map(&:to_a)
@@ -623,7 +623,7 @@
end
it 'finds a specific issue with iids', :request_store do
- result = batch_sync(max_queries: 8) do
+ result = batch_sync(max_queries: 11) do
resolve_issues(iids: [issue1.iid]).to_a
end
diff --git a/spec/graphql/resolvers/work_items_resolver_spec.rb b/spec/graphql/resolvers/work_items_resolver_spec.rb
index 4addb3e7da128..aee166d4e97b2 100644
--- a/spec/graphql/resolvers/work_items_resolver_spec.rb
+++ b/spec/graphql/resolvers/work_items_resolver_spec.rb
@@ -113,7 +113,7 @@
end
it 'batches queries that only include IIDs', :request_store do
- result = batch_sync(max_queries: 9) do
+ result = batch_sync(max_queries: 11) do
[item1, item2]
.map { |item| resolve_items(iid: item.iid.to_s) }
.flat_map(&:to_a)
@@ -123,7 +123,7 @@
end
it 'finds a specific item with iids', :request_store do
- result = batch_sync(max_queries: 9) do
+ result = batch_sync(max_queries: 11) do
resolve_items(iids: [item1.iid]).to_a
end
diff --git a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb
index 8d16ede0d2a76..70329fc3b70c8 100644
--- a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb
+++ b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb
@@ -6,7 +6,7 @@
let(:group) { create(:group).tap { |g| g.add_maintainer(importer_user) } }
let(:project) { create(:project, :repository, group: group) }
let(:members_mapper) { double('members_mapper').as_null_object }
- let(:admin) { create(:admin) }
+ let(:admin) { create(:admin, :without_default_org) }
let(:importer_user) { admin }
let(:excluded_keys) { [] }
let(:additional_relation_attributes) { {} }
diff --git a/spec/lib/gitlab/import_export/project/tree_saver_spec.rb b/spec/lib/gitlab/import_export/project/tree_saver_spec.rb
index 5747194a7d051..6e5b9b7027eae 100644
--- a/spec/lib/gitlab/import_export/project/tree_saver_spec.rb
+++ b/spec/lib/gitlab/import_export/project/tree_saver_spec.rb
@@ -393,7 +393,7 @@
end
context 'as admin' do
- let(:user) { create(:admin) }
+ let(:user) { create(:admin, :without_default_org) }
before do
project_tree_saver.save # rubocop:disable Rails/SaveBang
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 9d3cca8f15777..7dc546f130002 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -1921,7 +1921,7 @@ def setup_group_members(group)
end
context 'evaluating admin access level' do
- let_it_be(:admin) { create(:admin) }
+ let_it_be(:admin) { create(:admin, :without_default_org) }
context 'when admin mode is enabled', :enable_admin_mode do
it 'returns OWNER by default' do
@@ -1941,6 +1941,21 @@ def setup_group_members(group)
end
end
+ context 'when organization owner' do
+ let_it_be(:admin) { create(:admin) }
+
+ it 'returns OWNER by default' do
+ expect(group.max_member_access_for_user(admin)).to eq(Gitlab::Access::OWNER)
+ end
+
+ context 'when only concrete members' do
+ it 'returns NO_ACCESS' do
+ expect(group.max_member_access_for_user(admin, only_concrete_membership: true))
+ .to eq(Gitlab::Access::NO_ACCESS)
+ end
+ end
+ end
+
context 'group shared with another group' do
let_it_be(:parent_group_user) { create(:user) }
let_it_be(:child_group_user) { create(:user) }
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index fdbcb14654fa0..ea1a4ff921cc2 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -5683,6 +5683,56 @@ def add_user(access)
end
end
+ shared_examples 'organization owner' do
+ let!(:org_user) { create(:organization_user, organization: organization, user: user, access_level: access_level) }
+
+ context 'when user is the owner of the organization' do
+ let(:access_level) { Gitlab::Access::OWNER }
+
+ it { is_expected.to be_truthy }
+ end
+
+ context 'when user is not the owner of the organization' do
+ let(:access_level) { Gitlab::Access::GUEST }
+
+ it { is_expected.to be_falsey }
+ end
+ end
+
+ describe '#can_admin_organization?' do
+ let(:user) { create(:user) }
+ let(:organization) { create(:organization) }
+
+ subject { user.can_admin_organization?(organization) }
+
+ it_behaves_like 'organization owner'
+ end
+
+ describe '#owns_organization?' do
+ let_it_be(:user) { create(:user) }
+ let_it_be(:organization) { create(:organization) }
+
+ subject { user.owns_organization?(organization_param) }
+
+ context 'when passed organization object' do
+ let(:organization_param) { organization }
+
+ it_behaves_like 'organization owner'
+ end
+
+ context 'when passed organization id' do
+ let(:organization_param) { organization.id }
+
+ it_behaves_like 'organization owner'
+ end
+
+ context 'when passed nil' do
+ let(:organization_param) { nil }
+
+ it { is_expected.to be_falsey }
+ end
+ end
+
describe '#update_two_factor_requirement' do
let(:user) { create :user }
diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb
index f9712b9735dd5..a5fc4bba4fb51 100644
--- a/spec/policies/group_policy_spec.rb
+++ b/spec/policies/group_policy_spec.rb
@@ -241,6 +241,20 @@
end
end
+ context 'organization owner' do
+ let(:current_user) { organization_owner }
+
+ specify do
+ expect_allowed(*public_permissions)
+ expect_allowed(*guest_permissions)
+ expect_allowed(*reporter_permissions)
+ expect_allowed(*developer_permissions)
+ expect_allowed(*maintainer_permissions)
+ expect_allowed(*owner_permissions)
+ expect_allowed(*admin_permissions)
+ end
+ end
+
context 'migration bot' do
let_it_be(:migration_bot) { Users::Internal.migration_bot }
let_it_be(:current_user) { migration_bot }
diff --git a/spec/policies/upload_policy_spec.rb b/spec/policies/upload_policy_spec.rb
index 1169df0b30068..0c2aebcae250c 100644
--- a/spec/policies/upload_policy_spec.rb
+++ b/spec/policies/upload_policy_spec.rb
@@ -9,7 +9,7 @@
let_it_be(:developer) { create(:user).tap { |user| group.add_developer(user) } }
let_it_be(:maintainer) { create(:user).tap { |user| group.add_maintainer(user) } }
let_it_be(:owner) { create(:user).tap { |user| group.add_owner(user) } }
- let_it_be(:admin) { create(:admin) }
+ let_it_be(:admin) { create(:admin, :without_default_org) }
let_it_be(:non_member_user) { create(:user) }
let(:upload_permissions) { [:read_upload, :destroy_upload] }
diff --git a/spec/presenters/projects/import_export/project_export_presenter_spec.rb b/spec/presenters/projects/import_export/project_export_presenter_spec.rb
index d5776ba2323e9..277df5d4277ad 100644
--- a/spec/presenters/projects/import_export/project_export_presenter_spec.rb
+++ b/spec/presenters/projects/import_export/project_export_presenter_spec.rb
@@ -84,7 +84,7 @@
end
context 'as admin' do
- let(:user) { create(:admin) }
+ let(:user) { create(:admin, :without_default_org) }
context 'when admin mode is enabled', :enable_admin_mode do
it 'exports group members as admin' do
diff --git a/spec/requests/api/user_runners_spec.rb b/spec/requests/api/user_runners_spec.rb
index 412b2c48f3fae..18454d3dac8dc 100644
--- a/spec/requests/api/user_runners_spec.rb
+++ b/spec/requests/api/user_runners_spec.rb
@@ -3,7 +3,7 @@
require 'spec_helper'
RSpec.describe API::UserRunners, :aggregate_failures, feature_category: :fleet_visibility do
- let_it_be(:admin) { create(:admin) }
+ let_it_be(:admin) { create(:admin, :without_default_org) }
let_it_be(:user, reload: true) { create(:user, username: 'user.withdot') }
describe 'POST /user/runners' do
diff --git a/spec/services/ci/runners/create_runner_service_spec.rb b/spec/services/ci/runners/create_runner_service_spec.rb
index eaba7b9e4db54..bde59f1489e2e 100644
--- a/spec/services/ci/runners/create_runner_service_spec.rb
+++ b/spec/services/ci/runners/create_runner_service_spec.rb
@@ -7,7 +7,7 @@
let(:runner) { execute.payload[:runner] }
- let_it_be(:admin) { create(:admin) }
+ let_it_be(:admin) { create(:admin, :without_default_org) }
let_it_be(:non_admin_user) { create(:user) }
let_it_be(:anonymous) { nil }
let_it_be(:group_owner) { create(:user) }
diff --git a/spec/services/resource_access_tokens/create_service_spec.rb b/spec/services/resource_access_tokens/create_service_spec.rb
index e0f3f9e6109bf..6810f2f0209d3 100644
--- a/spec/services/resource_access_tokens/create_service_spec.rb
+++ b/spec/services/resource_access_tokens/create_service_spec.rb
@@ -62,7 +62,7 @@
end
context 'when created by an admin' do
- let(:user) { create(:admin) }
+ let(:user) { create(:admin, :without_default_org) }
context 'when admin mode is enabled', :enable_admin_mode do
it_behaves_like 'creates a user that has their email confirmed'
diff --git a/spec/support/matchers/access_matchers.rb b/spec/support/matchers/access_matchers.rb
index 1b460fbdbf7ff..741fed412c6bc 100644
--- a/spec/support/matchers/access_matchers.rb
+++ b/spec/support/matchers/access_matchers.rb
@@ -11,7 +11,9 @@ module AccessMatchers
def emulate_user(user_type_or_trait, membership = nil)
case user_type_or_trait
- when :user, :admin
+ when :admin
+ login_as(create(user_type_or_trait, :without_default_org))
+ when :user
login_as(create(user_type_or_trait))
when :external, :auditor
login_as(create(:user, user_type_or_trait))
diff --git a/spec/support/matchers/access_matchers_for_controller.rb b/spec/support/matchers/access_matchers_for_controller.rb
index 401bf6c196e84..2188128d60fe7 100644
--- a/spec/support/matchers/access_matchers_for_controller.rb
+++ b/spec/support/matchers/access_matchers_for_controller.rb
@@ -13,7 +13,7 @@ module AccessMatchersForController
def emulate_user(role, membership = nil)
case role
when :admin
- user = create(:admin)
+ user = create(:admin, :without_default_org)
sign_in(user)
when :user
user = create(:user)
diff --git a/spec/support/shared_contexts/graphql/resolvers/runners_resolver_shared_context.rb b/spec/support/shared_contexts/graphql/resolvers/runners_resolver_shared_context.rb
index 2dbb903a2724c..b24ad8a535d43 100644
--- a/spec/support/shared_contexts/graphql/resolvers/runners_resolver_shared_context.rb
+++ b/spec/support/shared_contexts/graphql/resolvers/runners_resolver_shared_context.rb
@@ -3,7 +3,7 @@
require 'spec_helper'
RSpec.shared_context 'runners resolver setup' do
- let_it_be(:user) { create_default(:user, :admin) }
+ let_it_be(:user) { create_default(:user, :admin, :without_default_org) }
let_it_be(:group) { create(:group, :public) }
let_it_be(:subgroup) { create(:group, :public, parent: group) }
let_it_be(:project) { create(:project, :public, group: group) }
diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb
index 565de038f51c2..866c96887d0c3 100644
--- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb
+++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb
@@ -6,9 +6,15 @@
let_it_be(:developer) { create(:user) }
let_it_be(:maintainer) { create(:user) }
let_it_be(:owner) { create(:user) }
- let_it_be(:admin) { create(:admin) }
+ let_it_be(:admin) { create(:admin, :without_default_org) }
let_it_be(:non_group_member) { create(:user) }
- let_it_be(:group, refind: true) { create(:group, :private, :owner_subgroup_creation_only) }
+
+ let_it_be(:organization) { create(:organization) }
+ let_it_be(:organization_owner) { create(:organization_user, :owner, organization: organization).user }
+
+ let_it_be(:group, refind: true) do
+ create(:group, :private, :owner_subgroup_creation_only, organization: organization)
+ end
let(:public_permissions) do
%i[
diff --git a/spec/support/shared_examples/models/member_shared_examples.rb b/spec/support/shared_examples/models/member_shared_examples.rb
index 43b60fad67d69..757ee5950e80d 100644
--- a/spec/support/shared_examples/models/member_shared_examples.rb
+++ b/spec/support/shared_examples/models/member_shared_examples.rb
@@ -112,7 +112,7 @@
end
RSpec.shared_examples_for "member creation" do
- let_it_be(:admin) { create(:admin) }
+ let_it_be(:admin) { create(:admin, :without_default_org) }
it 'returns a Member object', :aggregate_failures do
member = described_class.add_member(source, user, :maintainer)
@@ -385,7 +385,7 @@
end
RSpec.shared_examples_for "bulk member creation" do
- let_it_be(:admin) { create(:admin) }
+ let_it_be(:admin) { create(:admin, :without_default_org) }
let_it_be(:user1) { create(:user) }
let_it_be(:user2) { create(:user) }
diff --git a/spec/support/shared_examples/requests/admin_mode_shared_examples.rb b/spec/support/shared_examples/requests/admin_mode_shared_examples.rb
index 4f198dfb740de..475e9bf6fe265 100644
--- a/spec/support/shared_examples/requests/admin_mode_shared_examples.rb
+++ b/spec/support/shared_examples/requests/admin_mode_shared_examples.rb
@@ -55,7 +55,7 @@
end
RSpec.shared_examples 'when admin' do
- let_it_be(:current_user) { create(:admin) }
+ let_it_be(:current_user) { create(:admin, :without_default_org) }
it_behaves_like 'makes request' do
let(:status) { success_status_code }
--
GitLab