Merge branch 'security-65756-ex-admin-attacker-can-comment-in-internalsecurity-65756-ex-admin-attacker-can-comment-in-internal-12-4' into '12-4-stable'

Improper access control allows the attacker to comment in internal commit after they are no longer admin

See merge request gitlab/gitlabhq!3497

1 files changed, 36 insertions, 12 deletions

diff --git a/spec/policies/commit_policy_spec.rb b/spec/policies/commit_policy_spec.rb
index 41f6fb08426..40183f51e9e 100644
--- a/spec/policies/commit_policy_spec.rb
+++ b/spec/policies/commit_policy_spec.rb
@@ -8,28 +8,42 @@ describe CommitPolicy do
     let(:commit) { project.repository.head_commit }
     let(:policy) { described_class.new(user, commit) }
 
+    shared_examples 'can read commit and create a note' do
+      it 'can read commit' do
+        expect(policy).to be_allowed(:read_commit)
+      end
+
+      it 'can create a note' do
+        expect(policy).to be_allowed(:create_note)
+      end
+    end
+
+    shared_examples 'cannot read commit nor create a note' do
+      it 'can not read commit' do
+        expect(policy).to be_disallowed(:read_commit)
+      end
+
+      it 'can not create a note' do
+        expect(policy).to be_disallowed(:create_note)
+      end
+    end
+
     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
+      it_behaves_like 'can read commit and create a note'
 
       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
+        it_behaves_like 'cannot read commit nor create a note'
 
         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
+          it_behaves_like 'can read commit and create a note'
         end
       end
     end
@@ -37,9 +51,7 @@ describe CommitPolicy do
     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
+      it_behaves_like 'cannot read commit nor create a note'
 
       context 'when the user is a project member' do
         before do
@@ -50,6 +62,18 @@ describe CommitPolicy do
           expect(policy).to be_allowed(:read_commit)
         end
       end
+
+      context 'when the user is a guest' do
+        before do
+          project.add_guest(user)
+        end
+
+        it_behaves_like 'cannot read commit nor create a note'
+
+        it 'cannot download code' do
+          expect(policy).to be_disallowed(:download_code)
+        end
+      end
     end
   end
 end