Commit c1dbae90 authored by Grzegorz Bizon's avatar Grzegorz Bizon

Merge branch 'fix-forbidden-for-build-api-for-deleted-project' into 'master'

Give forbidden if project for the build was deleted

I guess we don't need a change log entry because this is just for an internal corner case fix.

Closes #25309

See merge request !8091
parents f8262e85 18c9fc42
......@@ -41,7 +41,7 @@ module Ci
put ":id" do
authenticate_runner!
build = Ci::Build.where(runner_id: current_runner.id).running.find(params[:id])
forbidden!('Build has been erased!') if build.erased?
validate_build!(build)
update_runner_info
......@@ -71,9 +71,7 @@ module Ci
# PATCH /builds/:id/trace.txt
patch ":id/trace.txt" do
build = Ci::Build.find_by_id(params[:id])
not_found! unless build
authenticate_build_token!(build)
forbidden!('Build has been erased!') if build.erased?
authenticate_build!(build)
error!('400 Missing header Content-Range', 400) unless request.headers.has_key?('Content-Range')
content_range = request.headers['Content-Range']
......@@ -104,8 +102,7 @@ module Ci
Gitlab::Workhorse.verify_api_request!(headers)
not_allowed! unless Gitlab.config.artifacts.enabled
build = Ci::Build.find_by_id(params[:id])
not_found! unless build
authenticate_build_token!(build)
authenticate_build!(build)
forbidden!('build is not running') unless build.running?
if params[:filesize]
......@@ -142,10 +139,8 @@ module Ci
require_gitlab_workhorse!
not_allowed! unless Gitlab.config.artifacts.enabled
build = Ci::Build.find_by_id(params[:id])
not_found! unless build
authenticate_build_token!(build)
authenticate_build!(build)
forbidden!('Build is not running!') unless build.running?
forbidden!('Build has been erased!') if build.erased?
artifacts_upload_path = ArtifactUploader.artifacts_upload_path
artifacts = uploaded_file(:file, artifacts_upload_path)
......@@ -176,8 +171,7 @@ module Ci
# GET /builds/:id/artifacts
get ":id/artifacts" do
build = Ci::Build.find_by_id(params[:id])
not_found! unless build
authenticate_build_token!(build)
authenticate_build!(build)
artifacts_file = build.artifacts_file
unless artifacts_file.file_storage?
......@@ -202,8 +196,7 @@ module Ci
# DELETE /builds/:id/artifacts
delete ":id/artifacts" do
build = Ci::Build.find_by_id(params[:id])
not_found! unless build
authenticate_build_token!(build)
authenticate_build!(build)
build.erase_artifacts!
end
......
......@@ -13,9 +13,20 @@ module Ci
forbidden! unless current_runner
end
def authenticate_build_token!(build)
def authenticate_build!(build)
validate_build!(build) do
forbidden! unless build_token_valid?(build)
end
end
def validate_build!(build)
not_found! unless build
yield if block_given?
forbidden!('Project has been deleted!') unless build.project
forbidden!('Build has been erased!') if build.erased?
end
def runner_registration_token_valid?
ActiveSupport::SecurityUtils.variable_size_secure_compare(
......
......@@ -249,7 +249,11 @@ describe Ci::API::Builds do
end
describe 'PATCH /builds/:id/trace.txt' do
let(:build) { create(:ci_build, :pending, :trace, runner_id: runner.id) }
let(:build) do
attributes = { runner_id: runner.id, pipeline: pipeline }
create(:ci_build, :running, :trace, attributes)
end
let(:headers) { { Ci::API::Helpers::BUILD_TOKEN_HEADER => build.token, 'Content-Type' => 'text/plain' } }
let(:headers_with_range) { headers.merge({ 'Content-Range' => '11-20' }) }
let(:update_interval) { 10.seconds.to_i }
......@@ -276,7 +280,6 @@ describe Ci::API::Builds do
end
before do
build.run!
initial_patch_the_trace
end
......@@ -329,6 +332,19 @@ describe Ci::API::Builds do
end
end
end
context 'when project for the build has been deleted' do
let(:build) do
attributes = { runner_id: runner.id, pipeline: pipeline }
create(:ci_build, :running, :trace, attributes) do |build|
build.project.update(pending_delete: true)
end
end
it 'responds with forbidden' do
expect(response.status).to eq(403)
end
end
end
context 'when Runner makes a force-patch' do
......
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