Merge branch 'bugfix/dragging_milestones' into 'master'

Stop unauthorized users dragging on milestone page

Closes #13670

See merge request !7113

4 files changed, 90 insertions, 2 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5bcbd309f19..a9e24717e81 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@ Please view this file on the master branch, on stable branches it's out of date.
   - Fix sidekiq stats in admin area (blackst0ne)
   - API: Fix booleans not recognized as such when using the `to_boolean` helper
   - Removed delete branch tooltip !6954
+  - Stop unauthorized users dragging on milestone page (blackst0ne)
   - Escape ref and path for relative links !6050 (winniehell)
   - Fixed link typo on /help/ui to Alerts section. !6915 (Sam Rose)
   - Fix filtering of milestones with quotes in title (airatshigapov)
diff --git a/app/assets/stylesheets/framework/lists.scss b/app/assets/stylesheets/framework/lists.scss
index 76de3abe808..78464af94bd 100644
--- a/app/assets/stylesheets/framework/lists.scss
+++ b/app/assets/stylesheets/framework/lists.scss
@@ -38,7 +38,7 @@
 
     &.smoke { background-color: $background-color; }
 
-    &:hover {
+    &:not(.ui-sort-disabled):hover {
       background: $row-hover;
     }
 
diff --git a/app/views/shared/milestones/_issuable.html.haml b/app/views/shared/milestones/_issuable.html.haml
index 3c03c220ddd..9e1b0379428 100644
--- a/app/views/shared/milestones/_issuable.html.haml
+++ b/app/views/shared/milestones/_issuable.html.haml
@@ -3,8 +3,9 @@
 - assignee = issuable.assignee
 - issuable_type = issuable.class.table_name
 - base_url_args = [project.namespace.becomes(Namespace), project, issuable_type]
+- can_update = can?(current_user, :"update_#{issuable.to_ability_name}", issuable)
 
-%li{ id: dom_id(issuable, 'sortable'),  class: "issuable-row", 'data-iid' => issuable.iid, 'data-url' => polymorphic_path(issuable) }
+%li{ id: dom_id(issuable, 'sortable'),  class: "issuable-row #{'ui-sort-disabled' unless can_update}", 'data-iid' => issuable.iid, 'data-url' => polymorphic_path(issuable) }
   %span
     - if show_project_name
       %strong #{project.name} ·
diff --git a/spec/features/milestones/milestones_spec.rb b/spec/features/milestones/milestones_spec.rb
new file mode 100644
index 00000000000..8b603f51545
--- /dev/null
+++ b/spec/features/milestones/milestones_spec.rb
@@ -0,0 +1,86 @@
+require 'rails_helper'
+
+describe 'Milestone draggable', feature: true, js: true do
+  let(:milestone) { create(:milestone, project: project, title: 8.14) }
+  let(:project)   { create(:empty_project, :public) }
+  let(:user)      { create(:user) }
+
+  context 'issues' do
+    let(:issue)        { page.find_by_id('issues-list-unassigned').find('li') }
+    let(:issue_target) { page.find_by_id('issues-list-ongoing') }
+
+    it 'does not allow guest to drag issue' do
+      create_and_drag_issue
+
+      expect(issue_target).not_to have_selector('.issuable-row')
+    end
+
+    it 'does not allow authorized user to drag issue' do
+      login_as(user)
+      create_and_drag_issue
+
+      expect(issue_target).not_to have_selector('.issuable-row')
+    end
+
+    it 'allows author to drag issue' do
+      login_as(user)
+      create_and_drag_issue(author: user)
+
+      expect(issue_target).to have_selector('.issuable-row')
+    end
+
+    it 'allows admin to drag issue' do
+      login_as(:admin)
+      create_and_drag_issue
+
+      expect(issue_target).to have_selector('.issuable-row')
+    end
+  end
+
+  context 'merge requests' do
+    let(:merge_request)        { page.find_by_id('merge_requests-list-unassigned').find('li') }
+    let(:merge_request_target) { page.find_by_id('merge_requests-list-ongoing') }
+
+    it 'does not allow guest to drag merge request' do
+      create_and_drag_merge_request
+
+      expect(merge_request_target).not_to have_selector('.issuable-row')
+    end
+
+    it 'does not allow authorized user to drag merge request' do
+      login_as(user)
+      create_and_drag_merge_request
+
+      expect(merge_request_target).not_to have_selector('.issuable-row')
+    end
+
+    it 'allows author to drag merge request' do
+      login_as(user)
+      create_and_drag_merge_request(author: user)
+
+      expect(merge_request_target).to have_selector('.issuable-row')
+    end
+
+    it 'allows admin to drag merge request' do
+      login_as(:admin)
+      create_and_drag_merge_request
+
+      expect(merge_request_target).to have_selector('.issuable-row')
+    end
+  end
+
+  def create_and_drag_issue(params = {})
+    create(:issue, params.merge(title: 'Foo', project: project, milestone: milestone))
+
+    visit namespace_project_milestone_path(project.namespace, project, milestone)
+    issue.drag_to(issue_target)
+  end
+
+  def create_and_drag_merge_request(params = {})
+    create(:merge_request, params.merge(title: 'Foo', source_project: project, target_project: project, milestone: milestone))
+
+    visit namespace_project_milestone_path(project.namespace, project, milestone)
+    page.find("a[href='#tab-merge-requests']").click
+    merge_request.drag_to(merge_request_target)
+  end
+end