Add latest changes from gitlab-org/security/gitlab@13-8-stable-ee

6 files changed, 192 insertions, 55 deletions

diff --git a/spec/requests/api/lint_spec.rb b/spec/requests/api/lint_spec.rb
index 2653653c896..2316e702c3e 100644
--- a/spec/requests/api/lint_spec.rb
+++ b/spec/requests/api/lint_spec.rb
@@ -4,91 +4,136 @@ require 'spec_helper'
 
 RSpec.describe API::Lint do
   describe 'POST /ci/lint' do
-    context 'with valid .gitlab-ci.yaml content' do
-      let(:yaml_content) do
-        File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml'))
-      end
+    context 'when signup settings are disabled' do
+      Gitlab::CurrentSettings.signup_enabled = false
 
-      it 'passes validation without warnings or errors' do
-        post api('/ci/lint'), params: { content: yaml_content }
+      context 'when unauthenticated' do
+        it 'returns authentication error' do
+          post api('/ci/lint'), params: { content: 'content' }
 
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response).to be_an Hash
-        expect(json_response['status']).to eq('valid')
-        expect(json_response['warnings']).to eq([])
-        expect(json_response['errors']).to eq([])
+          expect(response).to have_gitlab_http_status(:unauthorized)
+        end
       end
 
-      it 'outputs expanded yaml content' do
-        post api('/ci/lint'), params: { content: yaml_content, include_merged_yaml: true }
+      context 'when authenticated' do
+        it 'returns unauthorized error' do
+          post api('/ci/lint'), params: { content: 'content' }
 
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response).to have_key('merged_yaml')
+          expect(response).to have_gitlab_http_status(:unauthorized)
+        end
       end
     end
 
-    context 'with valid .gitlab-ci.yaml with warnings' do
-      let(:yaml_content) { { job: { script: 'ls', rules: [{ when: 'always' }] } }.to_yaml }
+    context 'when signup settings are enabled' do
+      Gitlab::CurrentSettings.signup_enabled = true
 
-      it 'passes validation but returns warnings' do
-        post api('/ci/lint'), params: { content: yaml_content }
+      context 'when unauthenticated' do
+        it 'returns authentication error' do
+          post api('/ci/lint'), params: { content: 'content' }
 
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response['status']).to eq('valid')
-        expect(json_response['warnings']).not_to be_empty
-        expect(json_response['status']).to eq('valid')
-        expect(json_response['errors']).to eq([])
+          expect(response).to have_gitlab_http_status(:unauthorized)
+        end
+      end
+
+      context 'when authenticated' do
+        let_it_be(:api_user) { create(:user) }
+        it 'returns authentication success' do
+          post api('/ci/lint', api_user), params: { content: 'content' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+        end
       end
     end
 
-    context 'with an invalid .gitlab_ci.yml' do
-      context 'with invalid syntax' do
-        let(:yaml_content) { 'invalid content' }
+    context 'when authenticated' do
+      let_it_be(:api_user) { create(:user) }
 
-        it 'responds with errors about invalid syntax' do
-          post api('/ci/lint'), params: { content: yaml_content }
+      context 'with valid .gitlab-ci.yaml content' do
+        let(:yaml_content) do
+          File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml'))
+        end
+
+        it 'passes validation without warnings or errors' do
+          post api('/ci/lint', api_user), params: { content: yaml_content }
 
           expect(response).to have_gitlab_http_status(:ok)
-          expect(json_response['status']).to eq('invalid')
+          expect(json_response).to be_an Hash
+          expect(json_response['status']).to eq('valid')
           expect(json_response['warnings']).to eq([])
-          expect(json_response['errors']).to eq(['Invalid configuration format'])
+          expect(json_response['errors']).to eq([])
         end
 
         it 'outputs expanded yaml content' do
-          post api('/ci/lint'), params: { content: yaml_content, include_merged_yaml: true }
+          post api('/ci/lint', api_user), params: { content: yaml_content, include_merged_yaml: true }
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(json_response).to have_key('merged_yaml')
         end
       end
 
-      context 'with invalid configuration' do
-        let(:yaml_content) { '{ image: "ruby:2.7",  services: ["postgres"], invalid }' }
+      context 'with valid .gitlab-ci.yaml with warnings' do
+        let(:yaml_content) { { job: { script: 'ls', rules: [{ when: 'always' }] } }.to_yaml }
 
-        it 'responds with errors about invalid configuration' do
-          post api('/ci/lint'), params: { content: yaml_content }
+        it 'passes validation but returns warnings' do
+          post api('/ci/lint', api_user), params: { content: yaml_content }
 
           expect(response).to have_gitlab_http_status(:ok)
-          expect(json_response['status']).to eq('invalid')
-          expect(json_response['warnings']).to eq([])
-          expect(json_response['errors']).to eq(['jobs invalid config should implement a script: or a trigger: keyword', 'jobs config should contain at least one visible job'])
+          expect(json_response['status']).to eq('valid')
+          expect(json_response['warnings']).not_to be_empty
+          expect(json_response['status']).to eq('valid')
+          expect(json_response['errors']).to eq([])
         end
+      end
 
-        it 'outputs expanded yaml content' do
-          post api('/ci/lint'), params: { content: yaml_content, include_merged_yaml: true }
+      context 'with an invalid .gitlab_ci.yml' do
+        context 'with invalid syntax' do
+          let(:yaml_content) { 'invalid content' }
 
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(json_response).to have_key('merged_yaml')
+          it 'responds with errors about invalid syntax' do
+            post api('/ci/lint', api_user), params: { content: yaml_content }
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response['status']).to eq('invalid')
+            expect(json_response['warnings']).to eq([])
+            expect(json_response['errors']).to eq(['Invalid configuration format'])
+          end
+
+          it 'outputs expanded yaml content' do
+            post api('/ci/lint', api_user), params: { content: yaml_content, include_merged_yaml: true }
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response).to have_key('merged_yaml')
+          end
+        end
+
+        context 'with invalid configuration' do
+          let(:yaml_content) { '{ image: "ruby:2.7",  services: ["postgres"] }' }
+
+          it 'responds with errors about invalid configuration' do
+            post api('/ci/lint', api_user), params: { content: yaml_content }
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response['status']).to eq('invalid')
+            expect(json_response['warnings']).to eq([])
+            expect(json_response['errors']).to eq(['jobs config should contain at least one visible job'])
+          end
+
+          it 'outputs expanded yaml content' do
+            post api('/ci/lint', api_user), params: { content: yaml_content, include_merged_yaml: true }
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response).to have_key('merged_yaml')
+          end
         end
       end
-    end
 
-    context 'without the content parameter' do
-      it 'responds with validation error about missing content' do
-        post api('/ci/lint')
+      context 'without the content parameter' do
+        it 'responds with validation error about missing content' do
+          post api('/ci/lint', api_user)
 
-        expect(response).to have_gitlab_http_status(:bad_request)
-        expect(json_response['error']).to eq('content is missing')
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(json_response['error']).to eq('content is missing')
+        end
       end
     end
   end
@@ -364,6 +409,18 @@ RSpec.describe API::Lint do
 
         expect(response).to have_gitlab_http_status(:not_found)
       end
+
+      context 'when project is public' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+        end
+
+        it 'returns authentication error' do
+          ci_lint
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
+      end
     end
 
     context 'when authenticated as non-member' do
@@ -387,13 +444,10 @@ RSpec.describe API::Lint do
         context 'when running as dry run' do
           let(:dry_run) { true }
 
-          it 'returns pipeline creation error' do
+          it 'returns authentication error' do
             ci_lint
 
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(json_response['merged_yaml']).to eq(nil)
-            expect(json_response['valid']).to eq(false)
-            expect(json_response['errors']).to eq(['Insufficient permissions to create a new pipeline'])
+            expect(response).to have_gitlab_http_status(:forbidden)
           end
         end
 
@@ -410,7 +464,11 @@ RSpec.describe API::Lint do
             )
           end
 
-          it_behaves_like 'valid project config'
+          it 'returns authentication error' do
+            ci_lint
+
+            expect(response).to have_gitlab_http_status(:forbidden)
+          end
         end
       end
     end
diff --git a/spec/requests/api/merge_request_approvals_spec.rb b/spec/requests/api/merge_request_approvals_spec.rb
index fad5c3fb60e..b18f3017e03 100644
--- a/spec/requests/api/merge_request_approvals_spec.rb
+++ b/spec/requests/api/merge_request_approvals_spec.rb
@@ -21,6 +21,12 @@ RSpec.describe API::MergeRequestApprovals do
 
       expect(response).to have_gitlab_http_status(:ok)
     end
+
+    context 'when merge request author has only guest access' do
+      it_behaves_like 'rejects user from accessing merge request info' do
+        let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/approvals" }
+      end
+    end
   end
 
   describe 'POST :id/merge_requests/:merge_request_iid/approve' do
diff --git a/spec/requests/api/merge_request_diffs_spec.rb b/spec/requests/api/merge_request_diffs_spec.rb
index 2e6cbe7bee7..971fb5e991c 100644
--- a/spec/requests/api/merge_request_diffs_spec.rb
+++ b/spec/requests/api/merge_request_diffs_spec.rb
@@ -35,6 +35,12 @@ RSpec.describe API::MergeRequestDiffs, 'MergeRequestDiffs' do
       get api("/projects/#{project.id}/merge_requests/0/versions", user)
       expect(response).to have_gitlab_http_status(:not_found)
     end
+
+    context 'when merge request author has only guest access' do
+      it_behaves_like 'rejects user from accessing merge request info' do
+        let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/versions" }
+      end
+    end
   end
 
   describe 'GET /projects/:id/merge_requests/:merge_request_iid/versions/:version_id' do
@@ -63,5 +69,11 @@ RSpec.describe API::MergeRequestDiffs, 'MergeRequestDiffs' do
       get api("/projects/#{project.id}/merge_requests/#{non_existing_record_iid}/versions/#{merge_request_diff.id}", user)
       expect(response).to have_gitlab_http_status(:not_found)
     end
+
+    context 'when merge request author has only guest access' do
+      it_behaves_like 'rejects user from accessing merge request info' do
+        let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/versions/#{merge_request_diff.id}" }
+      end
+    end
   end
 end
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 3a3eae73932..a04867658e8 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -1226,6 +1226,12 @@ RSpec.describe API::MergeRequests do
       end
     end
 
+    context 'when merge request author has only guest access' do
+      it_behaves_like 'rejects user from accessing merge request info' do
+        let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}" }
+      end
+    end
+
     context 'merge_request_metrics' do
       let(:pipeline) { create(:ci_empty_pipeline) }
 
@@ -1402,6 +1408,12 @@ RSpec.describe API::MergeRequests do
     it_behaves_like 'issuable participants endpoint' do
       let(:entity) { create(:merge_request, :simple, milestone: milestone1, author: user, assignees: [user], source_project: project, target_project: project, source_branch: 'markdown', title: "Test", created_at: base_time) }
     end
+
+    context 'when merge request author has only guest access' do
+      it_behaves_like 'rejects user from accessing merge request info' do
+        let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/participants" }
+      end
+    end
   end
 
   describe 'GET /projects/:id/merge_requests/:merge_request_iid/commits' do
@@ -1427,6 +1439,12 @@ RSpec.describe API::MergeRequests do
 
       expect(response).to have_gitlab_http_status(:not_found)
     end
+
+    context 'when merge request author has only guest access' do
+      it_behaves_like 'rejects user from accessing merge request info' do
+        let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/commits" }
+      end
+    end
   end
 
   describe 'GET /projects/:id/merge_requests/:merge_request_iid/:context_commits' do
@@ -1502,6 +1520,12 @@ RSpec.describe API::MergeRequests do
       expect(response).to have_gitlab_http_status(:not_found)
     end
 
+    context 'when merge request author has only guest access' do
+      it_behaves_like 'rejects user from accessing merge request info' do
+        let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/changes" }
+      end
+    end
+
     it_behaves_like 'find an existing merge request'
     it_behaves_like 'accesses diffs via raw_diffs'
 
@@ -1591,6 +1615,12 @@ RSpec.describe API::MergeRequests do
         expect(response).to have_gitlab_http_status(:forbidden)
       end
     end
+
+    context 'when merge request author has only guest access' do
+      it_behaves_like 'rejects user from accessing merge request info' do
+        let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/pipelines" }
+      end
+    end
   end
 
   describe 'POST /projects/:id/merge_requests/:merge_request_iid/pipelines' do
diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb
index eaffa49fc9d..00de1ef5964 100644
--- a/spec/requests/api/todos_spec.rb
+++ b/spec/requests/api/todos_spec.rb
@@ -331,6 +331,14 @@ RSpec.describe API::Todos do
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end
+
+    it 'returns an error if the issuable author does not have access' do
+      project_1.add_guest(issuable.author)
+
+      post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.iid}/todo", issuable.author)
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
   end
 
   describe 'POST :id/issuable_type/:issueable_id/todo' do
diff --git a/spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb b/spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb
new file mode 100644
index 00000000000..e6f9e5a434c
--- /dev/null
+++ b/spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'rejects user from accessing merge request info' do
+  let(:project) { create(:project, :private) }
+  let(:merge_request) do
+    create(:merge_request,
+      author: user,
+      source_project: project,
+      target_project: project
+    )
+  end
+
+  before do
+    project.add_guest(user)
+  end
+
+  it 'returns a 404 error' do
+    get api(url, user)
+
+    expect(response).to have_gitlab_http_status(:not_found)
+    expect(json_response['message']).to eq('404 Merge Request Not Found')
+  end
+end