Project members with guest role can't access confidential issues

3 files changed, 37 insertions, 4 deletions

diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index b99e02ba678..e871a103d42 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -132,12 +132,14 @@ describe NotificationService, services: true do
       let(:assignee) { create(:user) }
       let(:non_member) { create(:user) }
       let(:member) { create(:user) }
+      let(:guest) { create(:user) }
       let(:admin) { create(:admin) }
       let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) }
       let(:note) { create(:note_on_issue, noteable: confidential_issue, project: project, note: "#{author.to_reference} #{assignee.to_reference} #{non_member.to_reference} #{member.to_reference} #{admin.to_reference}") }
 
       it 'filters out users that can not read the issue' do
         project.team << [member, :developer]
+        project.team << [guest, :guest]
 
         expect(SentNotification).to receive(:record).with(confidential_issue, any_args).exactly(4).times
 
@@ -146,6 +148,7 @@ describe NotificationService, services: true do
         notification.new_note(note)
 
         should_not_email(non_member)
+        should_not_email(guest)
         should_email(author)
         should_email(assignee)
         should_email(member)
@@ -322,17 +325,20 @@ describe NotificationService, services: true do
         let(:assignee) { create(:user) }
         let(:non_member) { create(:user) }
         let(:member) { create(:user) }
+        let(:guest) { create(:user) }
         let(:admin) { create(:admin) }
         let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignee: assignee) }
 
         it "emails subscribers of the issue's labels that can read the issue" do
           project.team << [member, :developer]
+          project.team << [guest, :guest]
 
           label = create(:label, issues: [confidential_issue])
           label.toggle_subscription(non_member)
           label.toggle_subscription(author)
           label.toggle_subscription(assignee)
           label.toggle_subscription(member)
+          label.toggle_subscription(guest)
           label.toggle_subscription(admin)
 
           ActionMailer::Base.deliveries.clear
@@ -341,6 +347,7 @@ describe NotificationService, services: true do
 
           should_not_email(non_member)
           should_not_email(author)
+          should_not_email(guest)
           should_email(assignee)
           should_email(member)
           should_email(admin)
@@ -490,6 +497,7 @@ describe NotificationService, services: true do
         let(:assignee) { create(:user) }
         let(:non_member) { create(:user) }
         let(:member) { create(:user) }
+        let(:guest) { create(:user) }
         let(:admin) { create(:admin) }
         let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignee: assignee) }
         let!(:label_1) { create(:label, issues: [confidential_issue]) }
@@ -497,11 +505,13 @@ describe NotificationService, services: true do
 
         it "emails subscribers of the issue's labels that can read the issue" do
           project.team << [member, :developer]
+          project.team << [guest, :guest]
 
           label_2.toggle_subscription(non_member)
           label_2.toggle_subscription(author)
           label_2.toggle_subscription(assignee)
           label_2.toggle_subscription(member)
+          label_2.toggle_subscription(guest)
           label_2.toggle_subscription(admin)
 
           ActionMailer::Base.deliveries.clear
@@ -509,6 +519,7 @@ describe NotificationService, services: true do
           notification.relabeled_issue(confidential_issue, [label_2], @u_disabled)
 
           should_not_email(non_member)
+          should_not_email(guest)
           should_email(author)
           should_email(assignee)
           should_email(member)
diff --git a/spec/services/projects/autocomplete_service_spec.rb b/spec/services/projects/autocomplete_service_spec.rb
index 6108c26a78b..0971fec2e9f 100644
--- a/spec/services/projects/autocomplete_service_spec.rb
+++ b/spec/services/projects/autocomplete_service_spec.rb
@@ -33,6 +33,18 @@ describe Projects::AutocompleteService, services: true do
         expect(issues.count).to eq 1
       end
 
+      it 'should not list project confidential issues for project members with guest role' do
+        project.team << [member, :guest]
+
+        autocomplete = described_class.new(project, non_member)
+        issues = autocomplete.issues.map(&:iid)
+
+        expect(issues).to include issue.iid
+        expect(issues).not_to include security_issue_1.iid
+        expect(issues).not_to include security_issue_2.iid
+        expect(issues.count).to eq 1
+      end
+
       it 'should list project confidential issues for author' do
         autocomplete = described_class.new(project, author)
         issues = autocomplete.issues.map(&:iid)
diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index 489c920f19f..549a936b060 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -5,13 +5,15 @@ describe TodoService, services: true do
   let(:assignee) { create(:user) }
   let(:non_member) { create(:user) }
   let(:member) { create(:user) }
+  let(:guest) { create(:user) }
   let(:admin) { create(:admin) }
   let(:john_doe) { create(:user) }
   let(:project) { create(:project) }
-  let(:mentions) { [author, assignee, john_doe, member, non_member, admin].map(&:to_reference).join(' ') }
+  let(:mentions) { [author, assignee, john_doe, member, guest, non_member, admin].map(&:to_reference).join(' ') }
   let(:service) { described_class.new }
 
   before do
+    project.team << [guest, :guest]
     project.team << [author, :developer]
     project.team << [member, :developer]
     project.team << [john_doe, :developer]
@@ -41,18 +43,20 @@ describe TodoService, services: true do
         service.new_issue(issue, author)
 
         should_create_todo(user: member, target: issue, action: Todo::MENTIONED)
+        should_create_todo(user: guest, target: issue, action: Todo::MENTIONED)
         should_not_create_todo(user: author, target: issue, action: Todo::MENTIONED)
         should_not_create_todo(user: john_doe, target: issue, action: Todo::MENTIONED)
         should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED)
       end
 
-      it 'does not create todo for non project members when issue is confidential' do
+      it 'does not create todo if user can not see the issue when issue is confidential' do
         service.new_issue(confidential_issue, john_doe)
 
         should_create_todo(user: assignee, target: confidential_issue, author: john_doe, action: Todo::ASSIGNED)
         should_create_todo(user: author, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
         should_create_todo(user: member, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
         should_create_todo(user: admin, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
+        should_not_create_todo(user: guest, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
         should_not_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
       end
 
@@ -81,6 +85,7 @@ describe TodoService, services: true do
         service.update_issue(issue, author)
 
         should_create_todo(user: member, target: issue, action: Todo::MENTIONED)
+        should_create_todo(user: guest, target: issue, action: Todo::MENTIONED)
         should_create_todo(user: john_doe, target: issue, action: Todo::MENTIONED)
         should_not_create_todo(user: author, target: issue, action: Todo::MENTIONED)
         should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED)
@@ -92,13 +97,14 @@ describe TodoService, services: true do
         expect { service.update_issue(issue, author) }.not_to change(member.todos, :count)
       end
 
-      it 'does not create todo for non project members when issue is confidential' do
+      it 'does not create todo if user can not see the issue when issue is confidential' do
         service.update_issue(confidential_issue, john_doe)
 
         should_create_todo(user: author, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
         should_create_todo(user: assignee, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
         should_create_todo(user: member, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
         should_create_todo(user: admin, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
+        should_not_create_todo(user: guest, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
         should_not_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
       end
 
@@ -192,18 +198,20 @@ describe TodoService, services: true do
         service.new_note(note, john_doe)
 
         should_create_todo(user: member, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
+        should_create_todo(user: guest, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
         should_create_todo(user: author, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
         should_not_create_todo(user: john_doe, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
         should_not_create_todo(user: non_member, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
       end
 
-      it 'does not create todo for non project members when leaving a note on a confidential issue' do
+      it 'does not create todo if user can not see the issue when leaving a note on a confidential issue' do
         service.new_note(note_on_confidential_issue, john_doe)
 
         should_create_todo(user: author, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
         should_create_todo(user: assignee, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
         should_create_todo(user: member, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
         should_create_todo(user: admin, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
+        should_not_create_todo(user: guest, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
         should_not_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
       end
 
@@ -245,6 +253,7 @@ describe TodoService, services: true do
         service.new_merge_request(mr_assigned, author)
 
         should_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED)
+        should_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED)
         should_not_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED)
         should_not_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED)
         should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED)
@@ -256,6 +265,7 @@ describe TodoService, services: true do
         service.update_merge_request(mr_assigned, author)
 
         should_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED)
+        should_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED)
         should_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED)
         should_not_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED)
         should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED)