From ad5894e248c0653b48e4f209f35a2551f6e46d0c Mon Sep 17 00:00:00 2001 From: blackst0ne Date: Fri, 28 Oct 2016 20:30:58 +1100 Subject: [PATCH 1/2] Stop unauthorized users dragging on milestone page --- CHANGELOG.md | 1 + app/assets/stylesheets/framework/lists.scss | 2 +- .../shared/milestones/_issuable.html.haml | 3 +- spec/features/milestones/milestones_spec.rb | 86 +++++++++++++++++++ 4 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 spec/features/milestones/milestones_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 0989345d230..b233b5d8f50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix extra space on Build sidebar on Firefox !7060 - Fix HipChat notifications rendering (airatshigapov, eisnerd) - Add hover to trash icon in notes !7008 (blackst0ne) + - Stop unauthorized users dragging on milestone page (blackst0ne) - Escape ref and path for relative links !6050 (winniehell) - Simpler arguments passed to named_route on toggle_award_url helper method - Fix: Backup restore doesn't clear cache diff --git a/app/assets/stylesheets/framework/lists.scss b/app/assets/stylesheets/framework/lists.scss index 48e34a0066e..f0eb618773a 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 From 0162c132f4230c61c8d36e4f867d63096c258a6c Mon Sep 17 00:00:00 2001 From: blackst0ne Date: Fri, 28 Oct 2016 20:30:58 +1100 Subject: [PATCH 2/2] Stop unauthorized users dragging on milestone page --- CHANGELOG.md | 1 + app/assets/stylesheets/framework/lists.scss | 2 +- .../shared/milestones/_issuable.html.haml | 3 +- spec/features/milestones/milestones_spec.rb | 86 +++++++++++++++++++ 4 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 spec/features/milestones/milestones_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 56d9d4e2809..5f2b8e9c625 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Add hover to trash icon in notes !7008 (blackst0ne) - Fix sidekiq stats in admin area (blackst0ne) - 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