diff options
Diffstat (limited to 'spec/models/group_spec.rb')
| -rw-r--r-- | spec/models/group_spec.rb | 337 |
1 files changed, 330 insertions, 7 deletions
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index e79b54b4674..24d09d1c035 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -25,7 +25,6 @@ RSpec.describe Group do it { is_expected.to have_many(:clusters).class_name('Clusters::Cluster') } it { is_expected.to have_many(:container_repositories) } it { is_expected.to have_many(:milestones) } - it { is_expected.to have_many(:iterations) } it { is_expected.to have_many(:group_deploy_keys) } it { is_expected.to have_many(:services) } it { is_expected.to have_one(:dependency_proxy_setting) } @@ -65,6 +64,59 @@ RSpec.describe Group do it { is_expected.to validate_presence_of :two_factor_grace_period } it { is_expected.to validate_numericality_of(:two_factor_grace_period).is_greater_than_or_equal_to(0) } + context 'validating the parent of a group' do + context 'when the group has no parent' do + it 'allows a group to have no parent associated with it' do + group = build(:group) + + expect(group).to be_valid + end + end + + context 'when the group has a parent' do + it 'does not allow a group to have a namespace as its parent' do + group = build(:group, parent: build(:namespace)) + + expect(group).not_to be_valid + expect(group.errors[:parent_id].first).to eq('a group cannot have a user namespace as its parent') + end + + it 'allows a group to have another group as its parent' do + group = build(:group, parent: build(:group)) + + expect(group).to be_valid + end + end + + context 'when the feature flag `validate_namespace_parent_type` is disabled' do + before do + stub_feature_flags(validate_namespace_parent_type: false) + end + + context 'when the group has no parent' do + it 'allows a group to have no parent associated with it' do + group = build(:group) + + expect(group).to be_valid + end + end + + context 'when the group has a parent' do + it 'allows a group to have a namespace as its parent' do + group = build(:group, parent: build(:namespace)) + + expect(group).to be_valid + end + + it 'allows a group to have another group as its parent' do + group = build(:group, parent: build(:group)) + + expect(group).to be_valid + end + end + end + end + describe 'path validation' do it 'rejects paths reserved on the root namespace when the group has no parent' do group = build(:group, path: 'api') @@ -513,6 +565,42 @@ RSpec.describe Group do end end + describe '#last_blocked_owner?' do + let(:blocked_user) { create(:user, :blocked) } + + before do + group.add_user(blocked_user, GroupMember::OWNER) + end + + it { expect(group.last_blocked_owner?(blocked_user)).to be_truthy } + + context 'with another active owner' do + before do + group.add_user(create(:user), GroupMember::OWNER) + end + + it { expect(group.last_blocked_owner?(blocked_user)).to be_falsy } + end + + context 'with 2 blocked owners' do + before do + group.add_user(create(:user, :blocked), GroupMember::OWNER) + end + + it { expect(group.last_blocked_owner?(blocked_user)).to be_falsy } + end + + context 'with owners from a parent' do + before do + parent_group = create(:group) + create(:group_member, :owner, group: parent_group) + group.update(parent: parent_group) + end + + it { expect(group.last_blocked_owner?(blocked_user)).to be_falsy } + end + end + describe '#lfs_enabled?' do context 'LFS enabled globally' do before do @@ -729,8 +817,16 @@ RSpec.describe Group do context 'evaluating admin access level' 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) + context 'when admin mode is enabled', :enable_admin_mode do + it 'returns OWNER by default' do + expect(group.max_member_access_for_user(admin)).to eq(Gitlab::Access::OWNER) + end + end + + context 'when admin mode is disabled' do + it 'returns NO_ACCESS' do + expect(group.max_member_access_for_user(admin)).to eq(Gitlab::Access::NO_ACCESS) + end end it 'returns NO_ACCESS when only concrete membership should be considered' do @@ -740,6 +836,33 @@ RSpec.describe Group do end end + describe '#direct_members' do + let_it_be(:group) { create(:group, :nested) } + let_it_be(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } + let_it_be(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } + + it 'does not return members of the parent' do + expect(group.direct_members).not_to include(maintainer) + end + + it 'returns the direct member of the group' do + expect(group.direct_members).to include(developer) + end + + context 'group sharing' do + let!(:shared_group) { create(:group) } + + before do + create(:group_group_link, shared_group: shared_group, shared_with_group: group) + end + + it 'does not return members of the shared_with group' do + expect(shared_group.direct_members).not_to( + include(developer)) + end + end + end + describe '#members_with_parents' do let!(:group) { create(:group, :nested) } let!(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } @@ -932,6 +1055,65 @@ RSpec.describe Group do end end + describe '#refresh_members_authorized_projects' do + let_it_be(:group) { create(:group, :nested) } + let_it_be(:parent_group_user) { create(:user) } + let_it_be(:group_user) { create(:user) } + + before do + group.parent.add_maintainer(parent_group_user) + group.add_developer(group_user) + end + + context 'users for which authorizations refresh is executed' do + it 'processes authorizations refresh for all members of the group' do + expect(UserProjectAccessChangedService).to receive(:new).with(contain_exactly(group_user.id, parent_group_user.id)).and_call_original + + group.refresh_members_authorized_projects + end + + context 'when explicitly specified to run only for direct members' do + it 'processes authorizations refresh only for direct members of the group' do + expect(UserProjectAccessChangedService).to receive(:new).with(contain_exactly(group_user.id)).and_call_original + + group.refresh_members_authorized_projects(direct_members_only: true) + end + end + end + end + + describe '#users_ids_of_direct_members' do + let_it_be(:group) { create(:group, :nested) } + let_it_be(:parent_group_user) { create(:user) } + let_it_be(:group_user) { create(:user) } + + before do + group.parent.add_maintainer(parent_group_user) + group.add_developer(group_user) + end + + it 'does not return user ids of the members of the parent' do + expect(group.users_ids_of_direct_members).not_to include(parent_group_user.id) + end + + it 'returns the user ids of the direct member of the group' do + expect(group.users_ids_of_direct_members).to include(group_user.id) + end + + context 'group sharing' do + let!(:shared_group) { create(:group) } + + before do + create(:group_group_link, shared_group: shared_group, shared_with_group: group) + end + + it 'does not return the user ids of members of the shared_with group' do + expect(shared_group.users_ids_of_direct_members).not_to( + include(group_user.id)) + end + end + end + describe '#user_ids_for_project_authorizations' do it 'returns the user IDs for which to refresh authorizations' do maintainer = create(:user) @@ -959,6 +1141,29 @@ RSpec.describe Group do include(group_user.id)) end end + + context 'distinct user ids' do + let_it_be(:subgroup) { create(:group, :nested) } + let_it_be(:user) { create(:user) } + let_it_be(:shared_with_group) { create(:group) } + let_it_be(:other_subgroup_user) { create(:user) } + + before do + create(:group_group_link, shared_group: subgroup, shared_with_group: shared_with_group) + subgroup.add_maintainer(other_subgroup_user) + + # `user` is added as a direct member of the parent group, the subgroup + # and another group shared with the subgroup. + subgroup.parent.add_maintainer(user) + subgroup.add_developer(user) + shared_with_group.add_guest(user) + end + + it 'returns only distinct user ids of users for which to refresh authorizations' do + expect(subgroup.user_ids_for_project_authorizations).to( + contain_exactly(user.id, other_subgroup_user.id)) + end + end end describe '#update_two_factor_requirement' do @@ -1149,9 +1354,10 @@ RSpec.describe Group do describe '#ci_variables_for' do let(:project) { create(:project, group: group) } + let(:environment_scope) { '*' } let!(:ci_variable) do - create(:ci_group_variable, value: 'secret', group: group) + create(:ci_group_variable, value: 'secret', group: group, environment_scope: environment_scope) end let!(:protected_variable) do @@ -1160,13 +1366,16 @@ RSpec.describe Group do subject { group.ci_variables_for('ref', project) } - it 'memoizes the result by ref', :request_store do + it 'memoizes the result by ref and environment', :request_store do + scoped_variable = create(:ci_group_variable, value: 'secret', group: group, environment_scope: 'scoped') + expect(project).to receive(:protected_for?).with('ref').once.and_return(true) - expect(project).to receive(:protected_for?).with('other').once.and_return(false) + expect(project).to receive(:protected_for?).with('other').twice.and_return(false) 2.times do - expect(group.ci_variables_for('ref', project)).to contain_exactly(ci_variable, protected_variable) + expect(group.ci_variables_for('ref', project, environment: 'production')).to contain_exactly(ci_variable, protected_variable) expect(group.ci_variables_for('other', project)).to contain_exactly(ci_variable) + expect(group.ci_variables_for('other', project, environment: 'scoped')).to contain_exactly(ci_variable, scoped_variable) end end @@ -1203,6 +1412,120 @@ RSpec.describe Group do it_behaves_like 'ref is protected' end + context 'when environment name is specified' do + let(:environment) { 'review/name' } + + subject do + group.ci_variables_for('ref', project, environment: environment) + end + + context 'when environment scope is exactly matched' do + let(:environment_scope) { 'review/name' } + + it { is_expected.to contain_exactly(ci_variable) } + end + + context 'when environment scope is matched by wildcard' do + let(:environment_scope) { 'review/*' } + + it { is_expected.to contain_exactly(ci_variable) } + end + + context 'when environment scope does not match' do + let(:environment_scope) { 'review/*/special' } + + it { is_expected.not_to contain_exactly(ci_variable) } + end + + context 'when environment scope has _' do + let(:environment_scope) { '*_*' } + + it 'does not treat it as wildcard' do + is_expected.not_to contain_exactly(ci_variable) + end + + context 'when environment name contains underscore' do + let(:environment) { 'foo_bar/test' } + let(:environment_scope) { 'foo_bar/*' } + + it 'matches literally for _' do + is_expected.to contain_exactly(ci_variable) + end + end + end + + # The environment name and scope cannot have % at the moment, + # but we're considering relaxing it and we should also make sure + # it doesn't break in case some data sneaked in somehow as we're + # not checking this integrity in database level. + context 'when environment scope has %' do + it 'does not treat it as wildcard' do + ci_variable.update_attribute(:environment_scope, '*%*') + + is_expected.not_to contain_exactly(ci_variable) + end + + context 'when environment name contains a percent' do + let(:environment) { 'foo%bar/test' } + + it 'matches literally for %' do + ci_variable.update(environment_scope: 'foo%bar/*') + + is_expected.to contain_exactly(ci_variable) + end + end + end + + context 'when variables with the same name have different environment scopes' do + let!(:partially_matched_variable) do + create(:ci_group_variable, + key: ci_variable.key, + value: 'partial', + environment_scope: 'review/*', + group: group) + end + + let!(:perfectly_matched_variable) do + create(:ci_group_variable, + key: ci_variable.key, + value: 'prefect', + environment_scope: 'review/name', + group: group) + end + + it 'puts variables matching environment scope more in the end' do + is_expected.to eq( + [ci_variable, + partially_matched_variable, + perfectly_matched_variable]) + end + end + + context 'when :scoped_group_variables feature flag is disabled' do + before do + stub_feature_flags(scoped_group_variables: false) + end + + context 'when environment scope is exactly matched' do + let(:environment_scope) { 'review/name' } + + it { is_expected.to contain_exactly(ci_variable) } + end + + context 'when environment scope is partially matched' do + let(:environment_scope) { 'review/*' } + + it { is_expected.to contain_exactly(ci_variable) } + end + + context 'when environment scope does not match' do + let(:environment_scope) { 'review/*/special' } + + it { is_expected.to contain_exactly(ci_variable) } + end + end + end + context 'when group has children' do let(:group_child) { create(:group, parent: group) } let(:group_child_2) { create(:group, parent: group_child) } |