Forbid creating discussions for users with restricted access

3 files changed, 129 insertions, 49 deletions

diff --git a/spec/policies/commit_policy_spec.rb b/spec/policies/commit_policy_spec.rb
new file mode 100644
index 00000000000..2259693cf01
--- /dev/null
+++ b/spec/policies/commit_policy_spec.rb
@@ -0,0 +1,53 @@
+require 'spec_helper'
+
+describe CommitPolicy do
+  describe '#rules' do
+    let(:user) { create(:user) }
+    let(:commit) { project.repository.head_commit }
+    let(:policy) { described_class.new(user, commit) }
+
+    context 'when project is public' do
+      let(:project) { create(:project, :public, :repository) }
+
+      it 'can read commit and create a note' do
+        expect(policy).to be_allowed(:read_commit)
+      end
+
+      context 'when repository access level is private' do
+        let(:project) { create(:project, :public, :repository, :repository_private) }
+
+        it 'can not read commit and create a note' do
+          expect(policy).to be_disallowed(:read_commit)
+        end
+
+        context 'when the user is a project member' do
+          before do
+            project.add_developer(user)
+          end
+
+          it 'can read commit and create a note' do
+            expect(policy).to be_allowed(:read_commit)
+          end
+        end
+      end
+    end
+
+    context 'when project is private' do
+      let(:project) { create(:project, :private, :repository) }
+
+      it 'can not read commit and create a note' do
+        expect(policy).to be_disallowed(:read_commit)
+      end
+
+      context 'when the user is a project member' do
+        before do
+          project.add_developer(user)
+        end
+
+        it 'can read commit and create a note' do
+          expect(policy).to be_allowed(:read_commit)
+        end
+      end
+    end
+  end
+end
diff --git a/spec/policies/note_policy_spec.rb b/spec/policies/note_policy_spec.rb
index 0e848c74659..4be7a0266d1 100644
--- a/spec/policies/note_policy_spec.rb
+++ b/spec/policies/note_policy_spec.rb
@@ -1,28 +1,15 @@
 require 'spec_helper'
 
-describe NotePolicy, mdoels: true do
+describe NotePolicy do
   describe '#rules' do
     let(:user) { create(:user) }
     let(:project) { create(:project, :public) }
     let(:issue) { create(:issue, project: project) }
-
-    def policies(noteable = nil)
-      return @policies if @policies
-
-      noteable ||= issue
-      note = if noteable.is_a?(Commit)
-               create(:note_on_commit, commit_id: noteable.id, author: user, project: project)
-             else
-               create(:note, noteable: noteable, author: user, project: project)
-             end
-
-      @policies = described_class.new(user, note)
-    end
+    let(:noteable) { issue }
+    let(:policy) { described_class.new(user, note) }
+    let(:note) { create(:note, noteable: noteable, author: user, project: project) }
 
     shared_examples_for 'a discussion with a private noteable' do
-      let(:noteable) { issue }
-      let(:policy) { policies(noteable) }
-
       context 'when the note author can no longer see the noteable' do
         it 'can not edit nor read the note' do
           expect(policy).to be_disallowed(:admin_note)
@@ -46,12 +33,21 @@ describe NotePolicy, mdoels: true do
       end
     end
 
-    context 'when the project is private' do
-      let(:project) { create(:project, :private, :repository) }
+    context 'when the noteable is a commit' do
+      let(:commit) { project.repository.head_commit }
+      let(:note) { create(:note_on_commit, commit_id: commit.id, author: user, project: project) }
+
+      context 'when the project is private' do
+        let(:project) { create(:project, :private, :repository) }
+
+        it_behaves_like 'a discussion with a private noteable'
+      end
 
-      context 'when the noteable is a commit' do
-        it_behaves_like 'a discussion with a private noteable' do
-          let(:noteable) { project.repository.head_commit }
+      context 'when the project is public' do
+        context 'when repository access level is private' do
+          let(:project) { create(:project, :public, :repository, :repository_private) }
+
+          it_behaves_like 'a discussion with a private noteable'
         end
       end
     end
@@ -59,44 +55,44 @@ describe NotePolicy, mdoels: true do
     context 'when the project is public' do
       context 'when the note author is not a project member' do
         it 'can edit a note' do
-          expect(policies).to be_allowed(:admin_note)
-          expect(policies).to be_allowed(:resolve_note)
-          expect(policies).to be_allowed(:read_note)
+          expect(policy).to be_allowed(:admin_note)
+          expect(policy).to be_allowed(:resolve_note)
+          expect(policy).to be_allowed(:read_note)
         end
       end
 
       context 'when the noteable is a project snippet' do
-        it 'can edit note' do
-          policies = policies(create(:project_snippet, :public, project: project))
+        let(:noteable) { create(:project_snippet, :public, project: project) }
 
-          expect(policies).to be_allowed(:admin_note)
-          expect(policies).to be_allowed(:resolve_note)
-          expect(policies).to be_allowed(:read_note)
+        it 'can edit note' do
+          expect(policy).to be_allowed(:admin_note)
+          expect(policy).to be_allowed(:resolve_note)
+          expect(policy).to be_allowed(:read_note)
         end
 
         context 'when it is private' do
-          it_behaves_like 'a discussion with a private noteable' do
-            let(:noteable) { create(:project_snippet, :private, project: project) }
-          end
+          let(:noteable) { create(:project_snippet, :private, project: project) }
+
+          it_behaves_like 'a discussion with a private noteable'
         end
       end
 
       context 'when the noteable is a personal snippet' do
-        it 'can edit note' do
-          policies = policies(create(:personal_snippet, :public))
+        let(:noteable) { create(:personal_snippet, :public) }
 
-          expect(policies).to be_allowed(:admin_note)
-          expect(policies).to be_allowed(:resolve_note)
-          expect(policies).to be_allowed(:read_note)
+        it 'can edit note' do
+          expect(policy).to be_allowed(:admin_note)
+          expect(policy).to be_allowed(:resolve_note)
+          expect(policy).to be_allowed(:read_note)
         end
 
         context 'when it is private' do
-          it 'can not edit nor read the note' do
-            policies = policies(create(:personal_snippet, :private))
+          let(:noteable) { create(:personal_snippet, :private) }
 
-            expect(policies).to be_disallowed(:admin_note)
-            expect(policies).to be_disallowed(:resolve_note)
-            expect(policies).to be_disallowed(:read_note)
+          it 'can not edit nor read the note' do
+            expect(policy).to be_disallowed(:admin_note)
+            expect(policy).to be_disallowed(:resolve_note)
+            expect(policy).to be_disallowed(:read_note)
           end
         end
       end
@@ -120,20 +116,20 @@ describe NotePolicy, mdoels: true do
           end
 
           it 'can edit a note' do
-            expect(policies).to be_allowed(:admin_note)
-            expect(policies).to be_allowed(:resolve_note)
-            expect(policies).to be_allowed(:read_note)
+            expect(policy).to be_allowed(:admin_note)
+            expect(policy).to be_allowed(:resolve_note)
+            expect(policy).to be_allowed(:read_note)
           end
         end
 
         context 'when the note author is not a project member' do
           it 'can not edit a note' do
-            expect(policies).to be_disallowed(:admin_note)
-            expect(policies).to be_disallowed(:resolve_note)
+            expect(policy).to be_disallowed(:admin_note)
+            expect(policy).to be_disallowed(:resolve_note)
           end
 
           it 'can read a note' do
-            expect(policies).to be_allowed(:read_note)
+            expect(policy).to be_allowed(:read_note)
           end
         end
       end
diff --git a/spec/support/shared_examples/requests/api/discussions.rb b/spec/support/shared_examples/requests/api/discussions.rb
index b6aeb30d69c..0684569bb08 100644
--- a/spec/support/shared_examples/requests/api/discussions.rb
+++ b/spec/support/shared_examples/requests/api/discussions.rb
@@ -86,6 +86,37 @@ shared_examples 'discussions API' do |parent_type, noteable_type, id_name|
         expect(response).to have_gitlab_http_status(404)
       end
     end
+
+    context 'when a project is public with private repo access' do
+      let!(:parent) { create(:project, :public, :repository, :repository_private, :snippets_private) }
+      let!(:user_without_access) { create(:user) }
+
+      context 'when user is not a team member of private repo' do
+        before do
+          project.team.truncate
+        end
+
+        context "creating a new note" do
+          before do
+            post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user_without_access), body: 'hi!'
+          end
+
+          it 'raises 404 error' do
+            expect(response).to have_gitlab_http_status(404)
+          end
+        end
+
+        context "fetching a discussion" do
+          before do
+            get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions/#{note.discussion_id}", user_without_access)
+          end
+
+          it 'raises 404 error' do
+            expect(response).to have_gitlab_http_status(404)
+          end
+        end
+      end
+    end
   end
 
   describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes" do