Merge branch 'disallow-non-members-to-unlock-project-files' into 'master'

Disallow non-members to unlock project files

See merge request gitlab-org/gitlab!74541

ee/app/helpers/path_locks_helper.rb

 # frozen_string_literal: true
 
 module PathLocksHelper
-  def can_unlock?(path_lock, current_user = @current_user, project = @project)
-    can?(current_user, :admin_path_locks, project) || path_lock.user == current_user
+  def can_unlock?(path_lock, current_user = @current_user)
+    can?(current_user, :admin_path_locks, path_lock)
   end
 
   def text_label_for_lock(file_lock, path)

ee/app/policies/path_lock_policy.rb

+# frozen_string_literal: true
+
+class PathLockPolicy < BasePolicy # rubocop:disable Gitlab/NamespacedClass
+  delegate { @subject.project }
+
+  condition(:is_author) { @user && @subject.user == @user }
+  condition(:is_project_member) { @user && @subject.project && @subject.project.team.member?(user) }
+
+  rule { is_author & is_project_member }.enable :admin_path_locks
+end

ee/spec/helpers/path_locks_helper_spec.rb

@@ -3,16 +3,30 @@
 require 'spec_helper'
 
 RSpec.describe PathLocksHelper do
+  let(:user) { create(:user, name: 'John') }
+  let(:project) { create(:project) }
+  let(:path_lock) { create(:path_lock, path: 'app', user: user, project: project) }
+
+  describe '#can_unlock?' do
+    it "returns true if the user has admin_path_locks permission" do
+      allow(self).to receive(:can?).with(user, :admin_path_locks, path_lock).and_return(true)
+
+      expect(can_unlock?(path_lock, user)).to be(true)
+    end
+
+    it "returns false if the user does not have admin_path_locks permission" do
+      allow(self).to receive(:can?).with(user, :admin_path_locks, path_lock).and_return(false)
+
+      expect(can_unlock?(path_lock, user)).to be(false)
+    end
+  end
+
   describe '#text_label_for_lock' do
     it "return correct string for non-nested locks" do
-      user = create :user, name: 'John'
-      path_lock = create :path_lock, path: 'app', user: user
       expect(text_label_for_lock(path_lock, 'app')).to eq('Locked by John')
     end
 
     it "return correct string for nested locks" do
-      user = create :user, name: 'John'
-      path_lock = create :path_lock, path: 'app', user: user
       expect(text_label_for_lock(path_lock, 'app/models')).to eq('John has a lock on "app"')
     end
   end

ee/spec/policies/path_lock_policy_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe PathLockPolicy do
+  let(:project) { create(:project) }
+  let(:maintainer) { create(:user) }
+  let(:developer) { create(:user) }
+  let(:non_member) { create(:user) }
+
+  let(:developer_path_lock) { create(:path_lock, user: developer, project: project) }
+  let(:non_member_path_lock) { create(:path_lock, user: non_member, project: project) }
+
+  before do
+    project.add_maintainer(maintainer)
+    project.add_developer(developer)
+  end
+
+  def permissions(user, path_lock)
+    described_class.new(user, path_lock)
+  end
+
+  it 'disallows non-member from administrating path lock they created' do
+    expect(permissions(non_member, non_member_path_lock)).to be_disallowed(:admin_path_locks)
+  end
+
+  it 'disallows developer from administrating path lock they did not create' do
+    expect(permissions(developer, non_member_path_lock)).to be_disallowed(:admin_path_locks)
+  end
+
+  it 'allows developer to administrating path lock they created' do
+    expect(permissions(developer, developer_path_lock)).to be_allowed(:admin_path_locks)
+  end
+
+  it 'allows maintainer to administrating path lock they did not create' do
+    expect(permissions(maintainer, non_member_path_lock)).to be_allowed(:admin_path_locks)
+    expect(permissions(maintainer, developer_path_lock)).to be_allowed(:admin_path_locks)
+  end
+end