From deb229258b6e659695dfc5df0de6f378ecc36cad Mon Sep 17 00:00:00 2001 From: Manoj M J <mmj@gitlab.com> Date: Tue, 28 Sep 2021 12:28:35 +0530 Subject: [PATCH] Value of `lock_memberships_to_ldap` should not affect authorizations This change removes the check on the value of `lock_memberships_to_ldap ` setting that was previously being done while refreshing authorizations via the `EffectiveAccessLevelFinder` finder. Changelog: fixed --- .../members/effective_access_level_finder.rb | 2 +- .../effective_access_level_finder_spec.rb | 20 +++++++++- .../lib/gitlab/project_authorizations_spec.rb | 37 +++++++++++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/app/finders/projects/members/effective_access_level_finder.rb b/app/finders/projects/members/effective_access_level_finder.rb index c1e3842a9e4fa..d238679f2fbff 100644 --- a/app/finders/projects/members/effective_access_level_finder.rb +++ b/app/finders/projects/members/effective_access_level_finder.rb @@ -99,7 +99,7 @@ def project_owner_acting_as_maintainer end def include_membership_from_project_group_shares? - project.allowed_to_share_with_group? && project.project_group_links.any? + !project.namespace.share_with_group_lock && project.project_group_links.any? end # methods for `select` options diff --git a/spec/finders/projects/members/effective_access_level_finder_spec.rb b/spec/finders/projects/members/effective_access_level_finder_spec.rb index 1112dbd0d6ebd..33fbb5aca3045 100644 --- a/spec/finders/projects/members/effective_access_level_finder_spec.rb +++ b/spec/finders/projects/members/effective_access_level_finder_spec.rb @@ -194,6 +194,7 @@ context 'for a project that is shared with other group(s)' do let_it_be(:shared_with_group) { create(:group) } let_it_be(:user_from_shared_with_group) { create(:user) } + let_it_be(:project) { create(:project, group: create(:group)) } before do create(:project_group_link, :developer, project: project, group: shared_with_group) @@ -211,9 +212,24 @@ ) end - context 'when the group containing the project has forbidden group shares for any of its projects' do - let_it_be(:project) { create(:project, group: create(:group)) } + context 'even when the `lock_memberships_to_ldap` setting has been turned ON' do + before do + stub_application_setting(lock_memberships_to_ldap: true) + end + it 'includes the least among the specified access levels' do + expect(subject).to( + include( + hash_including( + 'user_id' => user_from_shared_with_group.id, + 'access_level' => Gitlab::Access::DEVELOPER + ) + ) + ) + end + end + + context 'when the group containing the project has forbidden group shares for any of its projects' do before do project.namespace.update!(share_with_group_lock: true) end diff --git a/spec/lib/gitlab/project_authorizations_spec.rb b/spec/lib/gitlab/project_authorizations_spec.rb index d2b41ee31d9b4..1606693419487 100644 --- a/spec/lib/gitlab/project_authorizations_spec.rb +++ b/spec/lib/gitlab/project_authorizations_spec.rb @@ -204,6 +204,43 @@ def map_access_levels(rows) end end + context 'with shared projects' do + let_it_be(:shared_with_group) { create(:group) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, group: create(:group)) } + + let(:mapping) { map_access_levels(authorizations) } + + before do + create(:project_group_link, :developer, project: project, group: shared_with_group) + shared_with_group.add_maintainer(user) + end + + it 'creates proper authorizations' do + expect(mapping[project.id]).to eq(Gitlab::Access::DEVELOPER) + end + + context 'even when the `lock_memberships_to_ldap` setting has been turned ON' do + before do + stub_application_setting(lock_memberships_to_ldap: true) + end + + it 'creates proper authorizations' do + expect(mapping[project.id]).to eq(Gitlab::Access::DEVELOPER) + end + end + + context 'when the group containing the project has forbidden group shares for any of its projects' do + before do + project.namespace.update!(share_with_group_lock: true) + end + + it 'does not create authorizations' do + expect(mapping[project.id]).to be_nil + end + end + end + context 'with shared groups' do let(:parent_group_user) { create(:user) } let(:group_user) { create(:user) } -- GitLab