Commit b8b7735f authored by lauraMon's avatar lauraMon

Change authorization policy for /lint

* For /projects/ci/id/lint, change policy to create_pipelinen
* For /ci/lint - enforces user authenication if registration is disabled
* Adds a changelog
parent 3339b689
---
title: Updates authorization for linting API
merge_request:
author:
type: security
...@@ -11,6 +11,8 @@ module API ...@@ -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' optional :include_merged_yaml, type: Boolean, desc: 'Whether or not to include merged CI config yaml in the response'
end end
post '/lint' do post '/lint' do
unauthorized! unless Gitlab::CurrentSettings.signup_enabled? && current_user
result = Gitlab::Ci::YamlProcessor.new(params[:content], user: current_user).execute result = Gitlab::Ci::YamlProcessor.new(params[:content], user: current_user).execute
status 200 status 200
...@@ -55,7 +57,7 @@ module API ...@@ -55,7 +57,7 @@ module API
optional :dry_run, type: Boolean, default: false, desc: 'Run pipeline creation simulation, or only do static check.' optional :dry_run, type: Boolean, default: false, desc: 'Run pipeline creation simulation, or only do static check.'
end end
post ':id/ci/lint' do post ':id/ci/lint' do
authorize! :download_code, user_project authorize! :create_pipeline, user_project
result = Gitlab::Ci::Lint result = Gitlab::Ci::Lint
.new(project: user_project, current_user: current_user) .new(project: user_project, current_user: current_user)
......
...@@ -4,91 +4,136 @@ require 'spec_helper' ...@@ -4,91 +4,136 @@ require 'spec_helper'
RSpec.describe API::Lint do RSpec.describe API::Lint do
describe 'POST /ci/lint' do describe 'POST /ci/lint' do
context 'with valid .gitlab-ci.yaml content' do context 'when signup settings are disabled' do
let(:yaml_content) do Gitlab::CurrentSettings.signup_enabled = false
File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml'))
end
it 'passes validation without warnings or errors' do context 'when unauthenticated' do
post api('/ci/lint'), params: { content: yaml_content } it 'returns authentication error' do
post api('/ci/lint'), params: { content: 'content' }
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:unauthorized)
expect(json_response).to be_an Hash end
expect(json_response['status']).to eq('valid')
expect(json_response['warnings']).to eq([])
expect(json_response['errors']).to eq([])
end end
it 'outputs expanded yaml content' do context 'when authenticated' do
post api('/ci/lint'), params: { content: yaml_content, include_merged_yaml: true } it 'returns unauthorized error' do
post api('/ci/lint'), params: { content: 'content' }
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:unauthorized)
expect(json_response).to have_key('merged_yaml') end
end end
end end
context 'with valid .gitlab-ci.yaml with warnings' do context 'when signup settings are enabled' do
let(:yaml_content) { { job: { script: 'ls', rules: [{ when: 'always' }] } }.to_yaml } Gitlab::CurrentSettings.signup_enabled = true
it 'passes validation but returns warnings' do context 'when unauthenticated' do
post api('/ci/lint'), params: { content: yaml_content } it 'returns authentication error' do
post api('/ci/lint'), params: { content: 'content' }
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:unauthorized)
expect(json_response['status']).to eq('valid') end
expect(json_response['warnings']).not_to be_empty end
expect(json_response['status']).to eq('valid')
expect(json_response['errors']).to eq([]) 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
end end
context 'with an invalid .gitlab_ci.yml' do context 'when authenticated' do
context 'with invalid syntax' do let_it_be(:api_user) { create(:user) }
let(:yaml_content) { 'invalid content' }
it 'responds with errors about invalid syntax' do context 'with valid .gitlab-ci.yaml content' do
post api('/ci/lint'), params: { content: yaml_content } 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(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['warnings']).to eq([])
expect(json_response['errors']).to eq(['Invalid configuration format']) expect(json_response['errors']).to eq([])
end end
it 'outputs expanded yaml content' do 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(response).to have_gitlab_http_status(:ok)
expect(json_response).to have_key('merged_yaml') expect(json_response).to have_key('merged_yaml')
end end
end end
context 'with invalid configuration' do context 'with valid .gitlab-ci.yaml with warnings' do
let(:yaml_content) { '{ image: "ruby:2.7", services: ["postgres"], invalid }' } let(:yaml_content) { { job: { script: 'ls', rules: [{ when: 'always' }] } }.to_yaml }
it 'responds with errors about invalid configuration' do it 'passes validation but returns warnings' do
post api('/ci/lint'), params: { content: yaml_content } post api('/ci/lint', api_user), params: { content: yaml_content }
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response['status']).to eq('invalid') expect(json_response['status']).to eq('valid')
expect(json_response['warnings']).to eq([]) expect(json_response['warnings']).not_to be_empty
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['errors']).to eq([])
end end
end
it 'outputs expanded yaml content' do context 'with an invalid .gitlab_ci.yml' do
post api('/ci/lint'), params: { content: yaml_content, include_merged_yaml: true } context 'with invalid syntax' do
let(:yaml_content) { 'invalid content' }
expect(response).to have_gitlab_http_status(:ok) it 'responds with errors about invalid syntax' do
expect(json_response).to have_key('merged_yaml') 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 end
end
context 'without the content parameter' do context 'without the content parameter' do
it 'responds with validation error about missing content' do it 'responds with validation error about missing content' do
post api('/ci/lint') post api('/ci/lint', api_user)
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['error']).to eq('content is missing') expect(json_response['error']).to eq('content is missing')
end
end end
end end
end end
...@@ -364,6 +409,18 @@ RSpec.describe API::Lint do ...@@ -364,6 +409,18 @@ RSpec.describe API::Lint do
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
end 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 end
context 'when authenticated as non-member' do context 'when authenticated as non-member' do
...@@ -387,13 +444,10 @@ RSpec.describe API::Lint do ...@@ -387,13 +444,10 @@ RSpec.describe API::Lint do
context 'when running as dry run' do context 'when running as dry run' do
let(:dry_run) { true } let(:dry_run) { true }
it 'returns pipeline creation error' do it 'returns authentication error' do
ci_lint ci_lint
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:forbidden)
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'])
end end
end end
...@@ -410,7 +464,11 @@ RSpec.describe API::Lint do ...@@ -410,7 +464,11 @@ RSpec.describe API::Lint do
) )
end 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 end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment