diff --git a/app/models/concerns/protected_ref_access.rb b/app/models/concerns/protected_ref_access.rb index f0bb1cc359b6991e172454fcdb5e88cf6ea23ce4..a5994b538ce57076071bc9c291e52c0103c19484 100644 --- a/app/models/concerns/protected_ref_access.rb +++ b/app/models/concerns/protected_ref_access.rb @@ -71,6 +71,8 @@ def check_access(current_user) return false if current_user.nil? || no_access? return current_user.admin? if admin_access? + return false if Feature.enabled?(:check_membership_in_protected_ref_access) && !project.member?(current_user) + yield if block_given? user_can_access?(current_user) diff --git a/config/feature_flags/development/check_membership_in_protected_ref_access.yml b/config/feature_flags/development/check_membership_in_protected_ref_access.yml new file mode 100644 index 0000000000000000000000000000000000000000..87e3fc9fcaf10f0ed85e1af7a2f8359214457c4c --- /dev/null +++ b/config/feature_flags/development/check_membership_in_protected_ref_access.yml @@ -0,0 +1,8 @@ +--- +name: check_membership_in_protected_ref_access +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132380 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/425926 +milestone: '16.5' +type: development +group: group::source code +default_enabled: false diff --git a/spec/support/shared_examples/models/concerns/protected_ref_access_examples.rb b/spec/support/shared_examples/models/concerns/protected_ref_access_examples.rb index 0e9200f1fd9346218b29aca712dbbcc881b2c91c..bb438b0082fd4fdd3a3a6e1d5728950b8d976379 100644 --- a/spec/support/shared_examples/models/concerns/protected_ref_access_examples.rb +++ b/spec/support/shared_examples/models/concerns/protected_ref_access_examples.rb @@ -52,7 +52,11 @@ end describe '#check_access' do + let_it_be(:group) { create(:group) } + # Making a project public to avoid false positives tests + let_it_be(:project) { create(:project, :public, group: group) } let_it_be(:current_user) { create(:user) } + let_it_be(:protected_ref) { create(association, project: project) } let(:access_level) { ::Gitlab::Access::DEVELOPER } @@ -71,6 +75,47 @@ it { expect(subject.check_access(nil)).to eq(false) } end + context 'when current_user access exists without membership' do + let(:other_user) { create(:user) } + let(:user_access) do + described_class.new(association => protected_ref, access_level: access_level, user_id: other_user.id) + end + + let(:enable_ff) { false } + + before do + stub_feature_flags(check_membership_in_protected_ref_access: enable_ff) + end + + it 'does not check membership if check_membership_in_protected_ref_access FF is disabled' do + expect(project).not_to receive(:member?).with(other_user) + + user_access.check_access(other_user) + end + + context 'when check_membership_in_protected_ref_access FF is enabled' do + let(:enable_ff) { true } + + it 'does check membership' do + expect(project).to receive(:member?).with(other_user) + + user_access.check_access(other_user) + end + + it 'returns false' do + expect(user_access.check_access(other_user)).to be_falsey + end + + context 'when user has inherited membership' do + let!(:inherited_membership) { create(:group_member, group: group, user: other_user) } + + it do + expect(user_access.check_access(other_user)).to be_truthy + end + end + end + end + context 'when access_level is NO_ACCESS' do let(:access_level) { ::Gitlab::Access::NO_ACCESS }