diff --git a/app/models/group.rb b/app/models/group.rb index 5bad84d74e5697755fe6410432d42e47b4fb3352..707b15823ef97b32a09db6a0c5adb49f2d13196a 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 eff607fc1a0429ad4f3155be5d8c20eb9eb9c038..dddca0e1f9e67243cc3e9848ac92933d8efcc1fe 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 8c2664d638a3e8c61ce7494aff4e16191c072fb0..c145b44bbccfdb61177d14d26cdd8cd8a6a1e479 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 786baa332bc265fb6b60ce16ccb96b8c4a604cc4..e77eaef3703c3ae633f546a70cbe3294f484c79c 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 6e27c5188e2f7a6656afa6b231b323c78f79d99f..6c2d82e59f95082a5d55350237b14d8b2646c05f 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 e57e0cbf8df0103004885436bcd3929b39620de2..d5d2e94ab50d0e4960440c858ea095671bf656d4 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 01bb3761a25755a240e230de414ee7b2581eb778..c0c07d51896c1ff51ed8a915454824e20f5c805b 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 8d20ee90477acb8f038e998fe4d4f852cddc3f10..39d8033a7773a609f621209c99e572729def415a 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 d6b27d8c618fafd80c7ce9b162266df6c2c30ee5..cf9c0012e29c0c15e677297bb27ed16fd80e6e72 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 593489290e006cd4ae4d9a8fd15c8564a20f2222..d47407cc0f27cce1ecdeb9a27e2bc2534bf45d11 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 4addb3e7da1288a679050bcab20597f919fb3053..aee166d4e97b20d27f128062ece5bb246b32c670 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 8d16ede0d2a7665d1713d7b99ca3e86f7639e3da..70329fc3b70c832643ee529659ccd87097c224c7 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 5747194a7d0517434457ecdcd595eefa92ee2f00..6e5b9b7027eaea1477dc607513a674103dc888e3 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 9d3cca8f157772f410a70cb74b316da9e3bdda18..7dc546f130002b45b8df92e1c3973b4760ff2093 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 fdbcb14654fa09e3069d899953ecd1f9bc2525e5..ea1a4ff921cc2f96f9dd72e8188b476f02ba69f5 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 f9712b9735dd545ff416f251709edc8d672d89b9..a5fc4bba4fb5121c93f4ba4425012b5500e35e7b 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 1169df0b3006830e915ac14eff996f25ba4a9ef3..0c2aebcae250cb5696de5f0e2c2b0ec928ec1f1d 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 d5776ba2323e913b87aed5689c7e79131bdaad12..277df5d4277ad355de6121400f8d64fc1820a818 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 412b2c48f3fae491337f9798b526d58cc3ab6f50..18454d3dac8dc77f428ee1007acde73b1188554d 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 eaba7b9e4db54e1656778ea9149a32536bb697c2..bde59f1489e2ed3edc49168729234749354de49a 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 e0f3f9e6109bf677774e1d4be2a782482d3f1a3d..6810f2f0209d3669d8c537a80101f3f8216dd7c7 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 1b460fbdbf7ff5ecfb015a9ac94020918c422866..741fed412c6bc1d37a7d27860d471ed335c5a25f 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 401bf6c196e842c9830bbfc014946452403f1b24..2188128d60fe7227ced35ef35bfd456638d0c372 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 2dbb903a2724c1b857b0839b63418410714b770d..b24ad8a535d430bdf9dcaa49ff4cdd1586ce3736 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 565de038f51c24c5f70a63cd552ddc31ec837e0b..866c96887d0c3d4bb319c2cbfb6ab1fb4fbd4f8b 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 43b60fad67d698cb534718b5416f54bed4cb0551..757ee5950e80dec68f7327894e8d8614bc3f0108 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 4f198dfb740dede8e28c1341e9360648ceda456d..475e9bf6fe265c8b15646857353333a2596559a5 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 }