diff options
author | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-05-28 13:16:01 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-05-28 13:16:01 +0000 |
commit | 90b7724ed1d7c092c4d4ccba82872ee67f1bf7f8 (patch) | |
tree | 27cf1a985e51c83819869150c861cfc26b3c8ab3 | |
parent | aa821670b9c71b5feb6c29048248cf2f12ee9a03 (diff) | |
parent | a175f1a109ca114f2fb27c3379885d352a7c35fd (diff) | |
download | gitlab-ce-90b7724ed1d7c092c4d4ccba82872ee67f1bf7f8.tar.gz |
Merge branch 'security-unsubscribing-from-issue-11-10' into '11-10-stable'
Hide issue title on unsubscribe for anonymous users
See merge request gitlab/gitlabhq!3100
4 files changed, 111 insertions, 11 deletions
diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 5318ab4ddef..c08b8d41ba9 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -93,4 +93,8 @@ module NotificationsHelper s_(event.to_s.humanize) end end + + def show_unsubscribe_title?(noteable) + can?(current_user, "read_#{noteable.to_ability_name}".to_sym, noteable) + end end diff --git a/app/views/sent_notifications/unsubscribe.html.haml b/app/views/sent_notifications/unsubscribe.html.haml index ca392e1adfc..22fcfcda297 100644 --- a/app/views/sent_notifications/unsubscribe.html.haml +++ b/app/views/sent_notifications/unsubscribe.html.haml @@ -1,6 +1,6 @@ - noteable = @sent_notification.noteable - noteable_type = @sent_notification.noteable_type.titleize.downcase -- noteable_text = %(#{noteable.title} (#{noteable.to_reference})) +- noteable_text = show_unsubscribe_title?(noteable) ? %(#{noteable.title} (#{noteable.to_reference})) : %(#{noteable.to_reference}) - page_title _("Unsubscribe"), noteable_text, noteable_type.pluralize, @sent_notification.project.full_name %h3.page-title diff --git a/changelogs/unreleased/security-unsubscribing-from-issue.yml b/changelogs/unreleased/security-unsubscribing-from-issue.yml new file mode 100644 index 00000000000..3a33a457c69 --- /dev/null +++ b/changelogs/unreleased/security-unsubscribing-from-issue.yml @@ -0,0 +1,5 @@ +--- +title: Hide confidential issue title on unsubscribe for anonymous users +merge_request: +author: +type: security diff --git a/spec/controllers/sent_notifications_controller_spec.rb b/spec/controllers/sent_notifications_controller_spec.rb index 75c91dd8607..89857a9d21b 100644 --- a/spec/controllers/sent_notifications_controller_spec.rb +++ b/spec/controllers/sent_notifications_controller_spec.rb @@ -1,16 +1,34 @@ +# frozen_string_literal: true + require 'rails_helper' describe SentNotificationsController do let(:user) { create(:user) } - let(:project) { create(:project) } - let(:sent_notification) { create(:sent_notification, project: project, noteable: issue, recipient: user) } + let(:project) { create(:project, :public) } + let(:private_project) { create(:project, :private) } + let(:sent_notification) { create(:sent_notification, project: target_project, noteable: noteable, recipient: user) } let(:issue) do - create(:issue, project: project, author: user) do |issue| - issue.subscriptions.create(user: user, project: project, subscribed: true) + create(:issue, project: target_project) do |issue| + issue.subscriptions.create(user: user, project: target_project, subscribed: true) + end + end + + let(:confidential_issue) do + create(:issue, project: target_project, confidential: true) do |issue| + issue.subscriptions.create(user: user, project: target_project, subscribed: true) end end + let(:merge_request) do + create(:merge_request, source_project: target_project, target_project: target_project) do |mr| + mr.subscriptions.create(user: user, project: target_project, subscribed: true) + end + end + + let(:noteable) { issue } + let(:target_project) { project } + describe 'GET unsubscribe' do context 'when the user is not logged in' do context 'when the force param is passed' do @@ -32,20 +50,93 @@ describe SentNotificationsController do end context 'when the force param is not passed' do + render_views + before do get(:unsubscribe, params: { id: sent_notification.reply_key }) end - it 'does not unsubscribe the user' do - expect(issue.subscribed?(user, project)).to be_truthy + shared_examples 'unsubscribing as anonymous' do + it 'does not unsubscribe the user' do + expect(noteable.subscribed?(user, target_project)).to be_truthy + end + + it 'does not set the flash message' do + expect(controller).not_to set_flash[:notice] + end + + it 'renders unsubscribe page' do + expect(response.status).to eq(200) + expect(response).to render_template :unsubscribe + end end - it 'does not set the flash message' do - expect(controller).not_to set_flash[:notice] + context 'when project is public' do + context 'when unsubscribing from issue' do + let(:noteable) { issue } + + it 'shows issue title' do + expect(response.body).to include(issue.title) + end + + it_behaves_like 'unsubscribing as anonymous' + end + + context 'when unsubscribing from confidential issue' do + let(:noteable) { confidential_issue } + + it 'does not show issue title' do + expect(response.body).not_to include(confidential_issue.title) + expect(response.body).to include(confidential_issue.to_reference) + end + + it_behaves_like 'unsubscribing as anonymous' + end + + context 'when unsubscribing from merge request' do + let(:noteable) { merge_request } + + it 'shows merge request title' do + expect(response.body).to include(merge_request.title) + end + + it_behaves_like 'unsubscribing as anonymous' + end end - it 'redirects to the login page' do - expect(response).to render_template :unsubscribe + context 'when project is not public' do + let(:target_project) { private_project } + + context 'when unsubscribing from issue' do + let(:noteable) { issue } + + it 'shows issue title' do + expect(response.body).not_to include(issue.title) + end + + it_behaves_like 'unsubscribing as anonymous' + end + + context 'when unsubscribing from confidential issue' do + let(:noteable) { confidential_issue } + + it 'does not show issue title' do + expect(response.body).not_to include(confidential_issue.title) + expect(response.body).to include(confidential_issue.to_reference) + end + + it_behaves_like 'unsubscribing as anonymous' + end + + context 'when unsubscribing from merge request' do + let(:noteable) { merge_request } + + it 'shows merge request title' do + expect(response.body).not_to include(merge_request.title) + end + + it_behaves_like 'unsubscribing as anonymous' + end end end end |