root
/
gitlab-ce
mirror of https://gitlab.com/free-releases/gitlab-ce.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

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

This commit is contained in:
GitLab Bot 2021-02-10 23:15:38 +00:00
parent 09cb1f3ef8
commit 08a8aa66ef
13 changed files with 234 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
changelogs/unreleased/security-check-user-access-on-api-mr-read-actions-master.yml Normal file
View File

@ -0,0 +1,5 @@
---
title: Check user access on API merge request read actions
merge_request:
author:
type: security

5
changelogs/unreleased/security-fix-unauthenticated-lint.yml Normal file
View File

@ -0,0 +1,5 @@
---
title: Updates authorization for linting API
merge_request:
author:
type: security

4
lib/api/lint.rb
View File

@ -11,6 +11,8 @@ module API
optional :include_merged_yaml, type: Boolean, desc: 'Whether or not to include merged CI config yaml in the response'
end
post '/lint' do
unauthorized! unless Gitlab::CurrentSettings.signup_enabled? && current_user
result = Gitlab::Ci::YamlProcessor.new(params[:content], user: current_user).execute
status 200
@ -55,7 +57,7 @@ module API
optional :dry_run, type: Boolean, default: false, desc: 'Run pipeline creation simulation, or only do static check.'
end
post ':id/ci/lint' do
authorize! :download_code, user_project
authorize! :create_pipeline, user_project
result = Gitlab::Ci::Lint
.new(project: user_project, current_user: current_user)

2
lib/api/merge_request_approvals.rb
View File

@ -26,6 +26,8 @@ module API
# GET /projects/:id/merge_requests/:merge_request_iid/approvals
desc 'List approvals for merge request'
get 'approvals' do
not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project)
merge_request = find_merge_request_with_access(params[:merge_request_iid])
present_approval(merge_request)

4
lib/api/merge_request_diffs.rb
View File

@ -23,6 +23,8 @@ module API
use :pagination
end
get ":id/merge_requests/:merge_request_iid/versions" do
not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project)
merge_request = find_merge_request_with_access(params[:merge_request_iid])
present paginate(merge_request.merge_request_diffs.order_id_desc), with: Entities::MergeRequestDiff
@ -39,6 +41,8 @@ module API
end
get ":id/merge_requests/:merge_request_iid/versions/:version_id" do
not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project)
merge_request = find_merge_request_with_access(params[:merge_request_iid])
present merge_request.merge_request_diffs.find(params[:version_id]), with: Entities::MergeRequestDiffFull

11
lib/api/merge_requests.rb
View File

@ -246,6 +246,8 @@ module API
success Entities::MergeRequest
end
get ':id/merge_requests/:merge_request_iid' do
not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project)
merge_request = find_merge_request_with_access(params[:merge_request_iid])
present merge_request,
@ -262,7 +264,10 @@ module API
success Entities::UserBasic
end
get ':id/merge_requests/:merge_request_iid/participants' do
not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project)
merge_request = find_merge_request_with_access(params[:merge_request_iid])
participants = ::Kaminari.paginate_array(merge_request.participants)
present paginate(participants), with: Entities::UserBasic
@ -272,6 +277,8 @@ module API
success Entities::Commit
end
get ':id/merge_requests/:merge_request_iid/commits' do
not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project)
merge_request = find_merge_request_with_access(params[:merge_request_iid])
commits =
@ -353,6 +360,8 @@ module API
success Entities::MergeRequestChanges
end
get ':id/merge_requests/:merge_request_iid/changes' do
not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project)
merge_request = find_merge_request_with_access(params[:merge_request_iid])
present merge_request,
@ -368,6 +377,8 @@ module API
get ':id/merge_requests/:merge_request_iid/pipelines' do
pipelines = merge_request_pipelines_with_access
not_found!("Merge Request") unless can?(current_user, :read_merge_request, user_project)
present paginate(pipelines), with: Entities::Ci::PipelineBasic
end

5
lib/api/todos.rb
View File

@ -28,6 +28,11 @@ module API
end
post ":id/#{type}/:#{type_id_str}/todo" do
issuable = instance_exec(params[type_id_str], &finder)
unless can?(current_user, :read_merge_request, issuable.project)
not_found!(type.split("_").map(&:capitalize).join(" "))
end
todo = TodoService.new.mark_todo(issuable, current_user).first
if todo

182
spec/requests/api/lint_spec.rb
View File

@ -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'))
context 'when signup settings are disabled' do
Gitlab::CurrentSettings.signup_enabled = false
context 'when unauthenticated' do
it 'returns authentication error' do
post api('/ci/lint'), params: { content: 'content' }
expect(response).to have_gitlab_http_status(:unauthorized)
end
end
it 'passes validation without warnings or errors' do
post api('/ci/lint'), params: { content: yaml_content }
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 be_an Hash
expect(json_response['status']).to eq('valid')
expect(json_response['warnings']).to eq([])
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 }
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
end
context 'with an invalid .gitlab_ci.yml' do
context 'with invalid syntax' do
let(:yaml_content) { 'invalid content' }
it 'responds with errors about invalid syntax' do
post api('/ci/lint'), params: { content: yaml_content }
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)
expect(json_response['status']).to eq('invalid')
end
end
end
context 'when authenticated' do
let_it_be(:api_user) { create(:user) }
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).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'])
end
it 'outputs expanded yaml content' do
post api('/ci/lint'), params: { content: yaml_content, include_merged_yaml: true }
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to have_key('merged_yaml')
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
end
context 'without the content parameter' do
it 'responds with validation error about missing content' do
post api('/ci/lint')
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(:bad_request)
expect(json_response['error']).to eq('content is missing')
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
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')
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

6
spec/requests/api/merge_request_approvals_spec.rb
View File

@ -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

12
spec/requests/api/merge_request_diffs_spec.rb
View File

@ -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

30
spec/requests/api/merge_requests_spec.rb
View File

@ -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

8
spec/requests/api/todos_spec.rb
View File

@ -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

23
spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb Normal file
View File

@ -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