From 08a8aa66ef41708976c27734587fc06e489a134f Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 10 Feb 2021 23:15:38 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@13-8-stable-ee --- spec/requests/api/lint_spec.rb | 168 ++++++++++++++------- spec/requests/api/merge_request_approvals_spec.rb | 6 + spec/requests/api/merge_request_diffs_spec.rb | 12 ++ spec/requests/api/merge_requests_spec.rb | 30 ++++ spec/requests/api/todos_spec.rb | 8 + .../requests/api/merge_requests_shared_examples.rb | 23 +++ 6 files changed, 192 insertions(+), 55 deletions(-) create mode 100644 spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb (limited to 'spec') 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 -- cgit v1.2.3