Add latest changes from gitlab-org/gitlab@master

2 files changed, 102 insertions, 34 deletions

diff --git a/spec/policies/issuable_policy_spec.rb b/spec/policies/issuable_policy_spec.rb
index fd7ec5917d6..c02294571ff 100644
--- a/spec/policies/issuable_policy_spec.rb
+++ b/spec/policies/issuable_policy_spec.rb
@@ -31,6 +31,10 @@ RSpec.describe IssuablePolicy, models: true do
         expect(policies).to be_allowed(:resolve_note)
       end
 
+      it 'allows reading confidential notes' do
+        expect(policies).to be_allowed(:read_confidential_notes)
+      end
+
       context 'when user is able to read project' do
         it 'enables user to read and update issuables' do
           expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request, :reopen_merge_request)
@@ -86,6 +90,15 @@ RSpec.describe IssuablePolicy, models: true do
       end
     end
 
+    context 'when user is assignee of issuable' do
+      let(:issue) { create(:issue, project: project, assignees: [user]) }
+      let(:policies) { described_class.new(user, issue) }
+
+      it 'allows reading confidential notes' do
+        expect(policies).to be_allowed(:read_confidential_notes)
+      end
+    end
+
     context 'when discussion is locked for the issuable' do
       let(:issue) { create(:issue, project: project, discussion_locked: true) }
 
@@ -138,6 +151,10 @@ RSpec.describe IssuablePolicy, models: true do
       it 'does not allow timelogs creation' do
         expect(permissions(guest, issue)).to be_disallowed(:create_timelog)
       end
+
+      it 'does not allow reading confidential notes' do
+        expect(permissions(guest, issue)).to be_disallowed(:read_confidential_notes)
+      end
     end
 
     context 'when user is a guest member of the project and the author of the issuable' do
@@ -152,6 +169,10 @@ RSpec.describe IssuablePolicy, models: true do
       it 'allows timelogs creation' do
         expect(permissions(reporter, issue)).to be_allowed(:create_timelog)
       end
+
+      it 'allows reading confidential notes' do
+        expect(permissions(reporter, issue)).to be_allowed(:read_confidential_notes)
+      end
     end
 
     context 'when subject is a Merge Request' do
diff --git a/spec/policies/todo_policy_spec.rb b/spec/policies/todo_policy_spec.rb
index 16435b21666..34ba7bf9276 100644
--- a/spec/policies/todo_policy_spec.rb
+++ b/spec/policies/todo_policy_spec.rb
@@ -3,53 +3,100 @@
 require 'spec_helper'
 
 RSpec.describe TodoPolicy do
-  let_it_be(:author) { create(:user) }
-
-  let_it_be(:user1) { create(:user) }
-  let_it_be(:user2) { create(:user) }
-  let_it_be(:user3) { create(:user) }
+  using RSpec::Parameterized::TableSyntax
 
   let_it_be(:project) { create(:project) }
   let_it_be(:issue) { create(:issue, project: project) }
-
-  let_it_be(:todo1) { create(:todo, author: author, user: user1, issue: issue) }
-  let_it_be(:todo2) { create(:todo, author: author, user: user2, issue: issue) }
-  let_it_be(:todo3) { create(:todo, author: author, user: user2) }
-  let_it_be(:todo4) { create(:todo, author: author, user: user3, issue: issue) }
+  let_it_be(:author) { create(:user) }
 
   def permissions(user, todo)
     described_class.new(user, todo)
   end
 
-  before_all do
-    project.add_developer(user1)
-    project.add_developer(user2)
+  shared_examples 'grants the expected permissions' do |policy|
+    it do
+      if allowed
+        expect(permissions(user, todo)).to be_allowed(policy)
+      else
+        expect(permissions(user, todo)).to be_disallowed(policy)
+      end
+    end
   end
 
   describe 'own_todo' do
-    it 'allows owners to access their own todos if they can read todo target' do
-      [
-        [user1, todo1],
-        [user2, todo2]
-      ].each do |user, todo|
-        expect(permissions(user, todo)).to be_allowed(:read_todo)
-      end
+    let_it_be(:user1) { create(:user) }
+    let_it_be(:user2) { create(:user) }
+    let_it_be(:user3) { create(:user) }
+
+    let_it_be(:todo1) { create(:todo, author: author, user: user1, issue: issue) }
+    let_it_be(:todo2) { create(:todo, author: author, user: user2, issue: issue) }
+    let_it_be(:todo3) { create(:todo, author: author, user: user2) }
+    let_it_be(:todo4) { create(:todo, author: author, user: user3, issue: issue) }
+
+    where(:user, :todo, :allowed) do
+      ref(:user1) | ref(:todo1) | true
+      ref(:user2) | ref(:todo2) | true
+      ref(:user1) | ref(:todo2) | false
+      ref(:user1) | ref(:todo3) | false
+      ref(:user2) | ref(:todo1) | false
+      ref(:user2) | ref(:todo4) | false
+      ref(:user3) | ref(:todo1) | false
+      ref(:user3) | ref(:todo2) | false
+      ref(:user3) | ref(:todo3) | false
+      ref(:user3) | ref(:todo4) | false
+      ref(:user2) | ref(:todo3) | false
     end
 
-    it 'does not allow users to access todos of other users' do
-      [
-        [user1, todo2],
-        [user1, todo3],
-        [user2, todo1],
-        [user2, todo4],
-        [user3, todo1],
-        [user3, todo2],
-        [user3, todo3],
-        [user2, todo3],
-        [user3, todo4]
-      ].each do |user, todo|
-        expect(permissions(user, todo)).to be_disallowed(:read_todo)
-      end
+    before_all do
+      project.add_developer(user1)
+      project.add_developer(user2)
+    end
+
+    with_them do
+      it_behaves_like 'grants the expected permissions', :read_todo
+    end
+  end
+
+  describe 'read_note' do
+    let_it_be(:non_member) { create(:user) }
+    let_it_be(:guest) { create(:user) }
+    let_it_be(:reporter) { create(:user) }
+
+    let_it_be(:note) { create(:note, noteable: issue, project: project) }
+    let_it_be(:internal) { create(:note, :confidential, noteable: issue, project: project) }
+
+    let_it_be(:no_note_todo1) { create(:todo, author: author, user: reporter, issue: issue) }
+    let_it_be(:note_todo1) { create(:todo, note: note, author: author, user: reporter, issue: issue) }
+    let_it_be(:internal_note_todo1) { create(:todo, note: internal, author: author, user: reporter, issue: issue) }
+
+    let_it_be(:no_note_todo2) { create(:todo, author: author, user: guest, issue: issue) }
+    let_it_be(:note_todo2) { create(:todo, note: note, author: author, user: guest, issue: issue) }
+    let_it_be(:internal_note_todo2) { create(:todo, note: internal, author: author, user: guest, issue: issue) }
+
+    let_it_be(:no_note_todo3) { create(:todo, author: author, user: non_member, issue: issue) }
+    let_it_be(:note_todo3) { create(:todo, note: note, author: author, user: non_member, issue: issue) }
+    let_it_be(:internal_note_todo3) { create(:todo, note: internal, author: author, user: non_member, issue: issue) }
+
+    where(:user, :todo, :allowed) do
+      ref(:reporter)   | ref(:no_note_todo1)       | true
+      ref(:reporter)   | ref(:note_todo1)          | true
+      ref(:reporter)   | ref(:internal_note_todo1) | true
+      ref(:guest)      | ref(:no_note_todo2)       | true
+      ref(:guest)      | ref(:note_todo2)          | true
+      ref(:guest)      | ref(:internal_note_todo2) | false
+      ref(:non_member) | ref(:no_note_todo3)       | false
+      ref(:non_member) | ref(:note_todo3)          | false
+      ref(:non_member) | ref(:internal_note_todo3) | false
+    end
+
+    before_all do
+      project.add_guest(guest)
+      project.add_reporter(reporter)
+    end
+
+    with_them do
+      it_behaves_like 'grants the expected permissions', :read_todo
+      it_behaves_like 'grants the expected permissions', :update_todo
     end
   end
 end