diff --git a/app/services/packages/create_package_service.rb b/app/services/packages/create_package_service.rb index 397a5f74e0af07a8ea1b566f47796d3b9f86ae05..e3b0ad218e2b04491c53c29dc12e1acda40fbd79 100644 --- a/app/services/packages/create_package_service.rb +++ b/app/services/packages/create_package_service.rb @@ -10,6 +10,7 @@ module Packages .with_package_type(package_type) .safe_find_or_create_by!(name: name, version: version) do |pkg| pkg.creator = package_creator + yield pkg if block_given? end end diff --git a/app/services/packages/generic/create_package_file_service.rb b/app/services/packages/generic/create_package_file_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..4d49c63799f5b2b54bda0cfb31b7cbbd93be7b21 --- /dev/null +++ b/app/services/packages/generic/create_package_file_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Packages + module Generic + class CreatePackageFileService < BaseService + def execute + ::Packages::Package.transaction do + create_package_file(find_or_create_package) + end + end + + private + + def find_or_create_package + package_params = { + name: params[:package_name], + version: params[:package_version], + build: params[:build] + } + + ::Packages::Generic::FindOrCreatePackageService + .new(project, current_user, package_params) + .execute + end + + def create_package_file(package) + file_params = { + file: params[:file], + size: params[:file].size, + file_sha256: params[:file].sha256, + file_name: params[:file_name] + } + + ::Packages::CreatePackageFileService.new(package, file_params).execute + end + end + end +end diff --git a/app/services/packages/generic/find_or_create_package_service.rb b/app/services/packages/generic/find_or_create_package_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..8a8459d167eff50b5ddb4abfbacd56ff6a42f43e --- /dev/null +++ b/app/services/packages/generic/find_or_create_package_service.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Packages + module Generic + class FindOrCreatePackageService < ::Packages::CreatePackageService + def execute + find_or_create_package!(::Packages::Package.package_types['generic']) do |package| + if params[:build].present? + package.build_info = Packages::BuildInfo.new(pipeline: params[:build].pipeline) + end + end + end + end + end +end diff --git a/lib/api/generic_packages.rb b/lib/api/generic_packages.rb index 98b8a40c7c96770f300bd31dea6da0aed390732c..a24580b358a2ebb07e3684683a1479ba8b394a6f 100644 --- a/lib/api/generic_packages.rb +++ b/lib/api/generic_packages.rb @@ -2,6 +2,11 @@ module API class GenericPackages < Grape::API::Instance + GENERIC_PACKAGES_REQUIREMENTS = { + package_name: API::NO_SLASH_URL_PART_REGEX, + file_name: API::NO_SLASH_URL_PART_REGEX + }.freeze + before do require_packages_enabled! authenticate! @@ -17,17 +22,71 @@ module API route_setting :authentication, job_token_allowed: true namespace ':id/packages/generic' do - get 'ping' do - :pong + namespace ':package_name/*package_version/:file_name', requirements: GENERIC_PACKAGES_REQUIREMENTS do + desc 'Workhorse authorize generic package file' do + detail 'This feature was introduced in GitLab 13.5' + end + + route_setting :authentication, job_token_allowed: true + + params do + requires :package_name, type: String, desc: 'Package name' + requires :package_version, type: String, desc: 'Package version', regexp: Gitlab::Regex.generic_package_version_regex + requires :file_name, type: String, desc: 'Package file name', regexp: Gitlab::Regex.generic_package_file_name_regex, file_path: true + end + + put 'authorize' do + authorize_workhorse!(subject: project, maximum_size: project.actual_limits.generic_packages_max_file_size) + end + + desc 'Upload package file' do + detail 'This feature was introduced in GitLab 13.5' + end + + params do + requires :package_name, type: String, desc: 'Package name' + requires :package_version, type: String, desc: 'Package version', regexp: Gitlab::Regex.generic_package_version_regex + requires :file_name, type: String, desc: 'Package file name', regexp: Gitlab::Regex.generic_package_file_name_regex, file_path: true + requires :file, type: ::API::Validations::Types::WorkhorseFile, desc: 'The package file to be published (generated by Multipart middleware)' + end + + route_setting :authentication, job_token_allowed: true + + put do + authorize_upload!(project) + bad_request!('File is too large') if max_file_size_exceeded? + + track_event('push_package') + + create_package_file_params = declared_params.merge(build: current_authenticated_job) + ::Packages::Generic::CreatePackageFileService + .new(project, current_user, create_package_file_params) + .execute + + created! + rescue ObjectStorage::RemoteStoreError => e + Gitlab::ErrorTracking.track_exception(e, extra: { file_name: params[:file_name], project_id: project.id }) + + forbidden! + end end end end helpers do include ::API::Helpers::PackagesHelpers + include ::API::Helpers::Packages::BasicAuthHelpers def require_generic_packages_available! - not_found! unless Feature.enabled?(:generic_packages, user_project) + not_found! unless Feature.enabled?(:generic_packages, project) + end + + def project + authorized_user_project + end + + def max_file_size_exceeded? + project.actual_limits.exceeded?(:generic_packages_max_file_size, params[:file].size) end end end diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 8e23ac6aca5e0e94992cc9fedfb973a5053c8d15..7df8b69e843ade4a2c685fb3307a6b6b17261ba9 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -103,6 +103,10 @@ module Gitlab def generic_package_version_regex /\A\d+\.\d+\.\d+\z/ end + + def generic_package_file_name_regex + maven_file_name_regex + end end extend self diff --git a/spec/fixtures/packages/generic/myfile.tar.gz b/spec/fixtures/packages/generic/myfile.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..c71b1fef23d8031b20ec67d2ac7bc4dfff2dfe20 Binary files /dev/null and b/spec/fixtures/packages/generic/myfile.tar.gz differ diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index 88c3315150b2e279dc4478765eed945246a60acc..0a6fbeee67207134b6db2e55cbe5523683276844 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -434,4 +434,18 @@ RSpec.describe Gitlab::Regex do it { is_expected.not_to match('%2e%2e%2f1.2.3') } it { is_expected.not_to match('') } end + + describe '.generic_package_file_name_regex' do + subject { described_class.generic_package_file_name_regex } + + it { is_expected.to match('123') } + it { is_expected.to match('foo') } + it { is_expected.to match('foo.bar.baz-2.0-20190901.47283-1.jar') } + it { is_expected.not_to match('../../foo') } + it { is_expected.not_to match('..\..\foo') } + it { is_expected.not_to match('%2f%2e%2e%2f%2essh%2fauthorized_keys') } + it { is_expected.not_to match('$foo/bar') } + it { is_expected.not_to match('my file name') } + it { is_expected.not_to match('!!()()') } + end end diff --git a/spec/requests/api/generic_packages_spec.rb b/spec/requests/api/generic_packages_spec.rb index ed852fe75c712339bf2c3c196e69a57af5c96e40..bebeed9402e80bfa5ee0bbf0a4c5f27fbf22228b 100644 --- a/spec/requests/api/generic_packages_spec.rb +++ b/spec/requests/api/generic_packages_spec.rb @@ -4,79 +4,268 @@ require 'spec_helper' RSpec.describe API::GenericPackages do let_it_be(:personal_access_token) { create(:personal_access_token) } - let_it_be(:project) { create(:project) } + let_it_be(:project, reload: true) { create(:project) } + let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } + let(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } + let(:user) { personal_access_token.user } + let(:ci_build) { create(:ci_build, :running, user: user) } - describe 'GET /api/v4/projects/:id/packages/generic/ping' do - let(:user) { personal_access_token.user } - let(:auth_token) { personal_access_token.token } + def auth_header + return {} if user_role == :anonymous - before do - project.add_developer(user) + case authenticate_with + when :personal_access_token + personal_access_token_header + when :job_token + job_token_header + when :invalid_personal_access_token + personal_access_token_header('wrong token') + when :invalid_job_token + job_token_header('wrong token') end + end - context 'packages feature is disabled' do - it 'responds with 404 Not Found' do - stub_packages_setting(enabled: false) + def personal_access_token_header(value = nil) + { Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => value || personal_access_token.token } + end - ping(personal_access_token: auth_token) + def job_token_header(value = nil) + { Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER => value || ci_build.token } + end - expect(response).to have_gitlab_http_status(:not_found) + describe 'PUT /api/v4/projects/:id/packages/generic/mypackage/0.0.1/myfile.tar.gz/authorize' do + context 'with valid project' do + using RSpec::Parameterized::TableSyntax + + where(:project_visibility, :user_role, :member?, :authenticate_with, :expected_status) do + 'PUBLIC' | :developer | true | :personal_access_token | :success + 'PUBLIC' | :guest | true | :personal_access_token | :forbidden + 'PUBLIC' | :developer | true | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :guest | true | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :developer | false | :personal_access_token | :forbidden + 'PUBLIC' | :guest | false | :personal_access_token | :forbidden + 'PUBLIC' | :developer | false | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :guest | false | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :anonymous | false | :none | :unauthorized + 'PRIVATE' | :developer | true | :personal_access_token | :success + 'PRIVATE' | :guest | true | :personal_access_token | :forbidden + 'PRIVATE' | :developer | true | :invalid_personal_access_token | :unauthorized + 'PRIVATE' | :guest | true | :invalid_personal_access_token | :unauthorized + 'PRIVATE' | :developer | false | :personal_access_token | :not_found + 'PRIVATE' | :guest | false | :personal_access_token | :not_found + 'PRIVATE' | :developer | false | :invalid_personal_access_token | :unauthorized + 'PRIVATE' | :guest | false | :invalid_personal_access_token | :unauthorized + 'PRIVATE' | :anonymous | false | :none | :unauthorized + 'PUBLIC' | :developer | true | :job_token | :success + 'PUBLIC' | :developer | true | :invalid_job_token | :unauthorized + 'PUBLIC' | :developer | false | :job_token | :forbidden + 'PUBLIC' | :developer | false | :invalid_job_token | :unauthorized + 'PRIVATE' | :developer | true | :job_token | :success + 'PRIVATE' | :developer | true | :invalid_job_token | :unauthorized + 'PRIVATE' | :developer | false | :job_token | :not_found + 'PRIVATE' | :developer | false | :invalid_job_token | :unauthorized + end + + with_them do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility, false)) + project.send("add_#{user_role}", user) if member? && user_role != :anonymous + end + + it "responds with #{params[:expected_status]}" do + headers = workhorse_header.merge(auth_header) + url = "/projects/#{project.id}/packages/generic/mypackage/0.0.1/myfile.tar.gz/authorize" + + put api(url), headers: headers + + expect(response).to have_gitlab_http_status(expected_status) + end end end + it 'rejects a malicious request' do + project.add_developer(user) + headers = workhorse_header.merge(personal_access_token_header) + url = "/projects/#{project.id}/packages/generic/mypackage/0.0.1/%2e%2e%2f.ssh%2fauthorized_keys/authorize" + + put api(url), headers: headers + + expect(response).to have_gitlab_http_status(:bad_request) + end + context 'generic_packages feature flag is disabled' do it 'responds with 404 Not Found' do stub_feature_flags(generic_packages: false) + project.add_developer(user) + headers = workhorse_header.merge(personal_access_token_header) + url = "/projects/#{project.id}/packages/generic/mypackage/0.0.1/myfile.tar.gz/authorize" - ping(personal_access_token: auth_token) + put api(url), headers: headers expect(response).to have_gitlab_http_status(:not_found) end end + end - context 'generic_packages feature flag is enabled' do - before do - stub_feature_flags(generic_packages: true) + describe 'PUT /api/v4/projects/:id/packages/generic/mypackage/0.0.1/myfile.tar.gz' do + include WorkhorseHelpers + + let(:file_upload) { fixture_file_upload('spec/fixtures/packages/generic/myfile.tar.gz') } + let(:params) { { file: file_upload } } + + context 'authentication' do + using RSpec::Parameterized::TableSyntax + + where(:project_visibility, :user_role, :member?, :authenticate_with, :expected_status) do + 'PUBLIC' | :guest | true | :personal_access_token | :forbidden + 'PUBLIC' | :developer | true | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :guest | true | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :developer | false | :personal_access_token | :forbidden + 'PUBLIC' | :guest | false | :personal_access_token | :forbidden + 'PUBLIC' | :developer | false | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :guest | false | :invalid_personal_access_token | :unauthorized + 'PUBLIC' | :anonymous | false | :none | :unauthorized + 'PRIVATE' | :guest | true | :personal_access_token | :forbidden + 'PRIVATE' | :developer | true | :invalid_personal_access_token | :unauthorized + 'PRIVATE' | :guest | true | :invalid_personal_access_token | :unauthorized + 'PRIVATE' | :developer | false | :personal_access_token | :not_found + 'PRIVATE' | :guest | false | :personal_access_token | :not_found + 'PRIVATE' | :developer | false | :invalid_personal_access_token | :unauthorized + 'PRIVATE' | :guest | false | :invalid_personal_access_token | :unauthorized + 'PRIVATE' | :anonymous | false | :none | :unauthorized + 'PUBLIC' | :developer | true | :invalid_job_token | :unauthorized + 'PUBLIC' | :developer | false | :job_token | :forbidden + 'PUBLIC' | :developer | false | :invalid_job_token | :unauthorized + 'PRIVATE' | :developer | true | :invalid_job_token | :unauthorized + 'PRIVATE' | :developer | false | :job_token | :not_found + 'PRIVATE' | :developer | false | :invalid_job_token | :unauthorized end - context 'authenticating using personal access token' do - it 'responds with 200 OK when valid personal access token is provided' do - ping(personal_access_token: auth_token) + with_them do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility, false)) + project.send("add_#{user_role}", user) if member? && user_role != :anonymous + end + + it "responds with #{params[:expected_status]}" do + headers = workhorse_header.merge(auth_header) - expect(response).to have_gitlab_http_status(:ok) + upload_file(params, headers) + + expect(response).to have_gitlab_http_status(expected_status) end + end + end - it 'responds with 401 Unauthorized when invalid personal access token provided' do - ping(personal_access_token: 'invalid-token') + context 'when user can upload packages and has valid credentials' do + before do + project.add_developer(user) + end + + it 'creates package and package file when valid personal access token is used' do + headers = workhorse_header.merge(personal_access_token_header) + + expect { upload_file(params, headers) } + .to change { project.packages.generic.count }.by(1) + .and change { Packages::PackageFile.count }.by(1) - expect(response).to have_gitlab_http_status(:unauthorized) + aggregate_failures do + expect(response).to have_gitlab_http_status(:created) + + package = project.packages.generic.last + expect(package.name).to eq('mypackage') + expect(package.version).to eq('0.0.1') + expect(package.build_info).to be_nil + + package_file = package.package_files.last + expect(package_file.file_name).to eq('myfile.tar.gz') end end - context 'authenticating using job token' do - it 'responds with 200 OK when valid job token is provided' do - job_token = create(:ci_build, :running, user: user).token + it 'creates package, package file, and package build info when valid job token is used' do + headers = workhorse_header.merge(job_token_header) + + expect { upload_file(params, headers) } + .to change { project.packages.generic.count }.by(1) + .and change { Packages::PackageFile.count }.by(1) + + aggregate_failures do + expect(response).to have_gitlab_http_status(:created) - ping(job_token: job_token) + package = project.packages.generic.last + expect(package.name).to eq('mypackage') + expect(package.version).to eq('0.0.1') + expect(package.build_info.pipeline).to eq(ci_build.pipeline) - expect(response).to have_gitlab_http_status(:ok) + package_file = package.package_files.last + expect(package_file.file_name).to eq('myfile.tar.gz') end + end + + context 'event tracking' do + subject { upload_file(params, workhorse_header.merge(personal_access_token_header)) } + + it_behaves_like 'a gitlab tracking event', described_class.name, 'push_package' + end + + it 'rejects request without a file from workhorse' do + headers = workhorse_header.merge(personal_access_token_header) + upload_file({}, headers) + + expect(response).to have_gitlab_http_status(:bad_request) + end - it 'responds with 401 Unauthorized when invalid job token provided' do - ping(job_token: 'invalid-token') + it 'rejects request without an auth token' do + upload_file(params, workhorse_header) - expect(response).to have_gitlab_http_status(:unauthorized) + expect(response).to have_gitlab_http_status(:unauthorized) + end + + it 'rejects request without workhorse rewritten fields' do + headers = workhorse_header.merge(personal_access_token_header) + upload_file(params, headers, send_rewritten_field: false) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'rejects request if file size is too large' do + allow_next_instance_of(UploadedFile) do |uploaded_file| + allow(uploaded_file).to receive(:size).and_return(project.actual_limits.generic_packages_max_file_size + 1) end + + headers = workhorse_header.merge(personal_access_token_header) + upload_file(params, headers) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'rejects request without workhorse header' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).once + + upload_file(params, personal_access_token_header) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + it 'rejects a malicious request' do + headers = workhorse_header.merge(personal_access_token_header) + upload_file(params, headers, file_name: '%2e%2e%2f.ssh%2fauthorized_keys') + + expect(response).to have_gitlab_http_status(:bad_request) end end - def ping(personal_access_token: nil, job_token: nil) - headers = { - Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => personal_access_token.presence, - Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER => job_token.presence - }.compact + def upload_file(params, request_headers, send_rewritten_field: true, file_name: 'myfile.tar.gz') + url = "/projects/#{project.id}/packages/generic/mypackage/0.0.1/#{file_name}" - get api('/projects/%d/packages/generic/ping' % project.id), headers: headers + workhorse_finalize( + api(url), + method: :put, + file_key: :file, + params: params, + headers: request_headers, + send_rewritten_field: send_rewritten_field + ) end end end diff --git a/spec/services/packages/generic/create_package_file_service_spec.rb b/spec/services/packages/generic/create_package_file_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..0ae109ef996e7b942326ef8b7582f840501b1270 --- /dev/null +++ b/spec/services/packages/generic/create_package_file_service_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Generic::CreatePackageFileService do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + + describe '#execute' do + let(:sha256) { '440e5e148a25331bbd7991575f7d54933c0ebf6cc735a18ee5066ac1381bb590' } + let(:temp_file) { Tempfile.new("test") } + let(:file) { UploadedFile.new(temp_file.path, sha256: sha256) } + let(:package) { create(:generic_package, project: project) } + let(:params) do + { + package_name: 'mypackage', + package_version: '0.0.1', + file: file, + file_name: 'myfile.tar.gz.1' + } + end + + before do + FileUtils.touch(temp_file) + end + + after do + FileUtils.rm_f(temp_file) + end + + it 'creates package file' do + package_service = double + package_params = { + name: params[:package_name], + version: params[:package_version], + build: params[:build] + } + expect(::Packages::Generic::FindOrCreatePackageService).to receive(:new).with(project, user, package_params).and_return(package_service) + expect(package_service).to receive(:execute).and_return(package) + + service = described_class.new(project, user, params) + + expect { service.execute }.to change { package.package_files.count }.by(1) + + package_file = package.package_files.last + aggregate_failures do + expect(package_file.package).to eq(package) + expect(package_file.file_name).to eq('myfile.tar.gz.1') + expect(package_file.size).to eq(file.size) + expect(package_file.file_sha256).to eq(sha256) + end + end + end +end diff --git a/spec/services/packages/generic/find_or_create_package_service_spec.rb b/spec/services/packages/generic/find_or_create_package_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..5a9b8b032792aae22c350ac446c1736c2c17572c --- /dev/null +++ b/spec/services/packages/generic/find_or_create_package_service_spec.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Generic::FindOrCreatePackageService do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:ci_build) { create(:ci_build, :running, user: user) } + + let(:params) do + { + name: 'mypackage', + version: '0.0.1' + } + end + + describe '#execute' do + context 'when packages does not exist yet' do + it 'creates package' do + service = described_class.new(project, user, params) + + expect { service.execute }.to change { project.packages.generic.count }.by(1) + + package = project.packages.generic.last + + aggregate_failures do + expect(package.creator).to eq(user) + expect(package.name).to eq('mypackage') + expect(package.version).to eq('0.0.1') + expect(package.build_info).to be_nil + end + end + + it 'creates package and package build info when build is provided' do + service = described_class.new(project, user, params.merge(build: ci_build)) + + expect { service.execute }.to change { project.packages.generic.count }.by(1) + + package = project.packages.generic.last + + aggregate_failures do + expect(package.creator).to eq(user) + expect(package.name).to eq('mypackage') + expect(package.version).to eq('0.0.1') + expect(package.build_info.pipeline).to eq(ci_build.pipeline) + end + end + end + + context 'when packages already exists' do + let!(:package) { project.packages.generic.create!(params) } + + context 'when package was created manually' do + it 'finds the package and does not create package build info even if build is provided' do + service = described_class.new(project, user, params.merge(build: ci_build)) + + expect do + found_package = service.execute + + expect(found_package).to eq(package) + end.not_to change { project.packages.generic.count } + + expect(package.reload.build_info).to be_nil + end + end + + context 'when package was created by pipeline' do + let(:pipeline) { create(:ci_pipeline, project: project) } + + before do + package.create_build_info!(pipeline: pipeline) + end + + it 'finds the package and does not change package build info even if build is provided' do + service = described_class.new(project, user, params.merge(build: ci_build)) + + expect do + found_package = service.execute + + expect(found_package).to eq(package) + end.not_to change { project.packages.generic.count } + + expect(package.reload.build_info.pipeline).to eq(pipeline) + end + end + end + end +end