Commit 9364a950 authored by Stan Hu's avatar Stan Hu

Merge branch 'secure-files-upload-checksum' into 'master'

Adding upload checksum to Secure Files API

See merge request gitlab-org/gitlab!83192
parents 2d9b581c e3ade867
...@@ -11,10 +11,13 @@ module Ci ...@@ -11,10 +11,13 @@ module Ci
self.limit_scope = :project self.limit_scope = :project
self.limit_name = 'project_ci_secure_files' self.limit_name = 'project_ci_secure_files'
attr_accessor :file_checksum
belongs_to :project, optional: false belongs_to :project, optional: false
validates :file, presence: true, file_size: { maximum: FILE_SIZE_LIMIT } validates :file, presence: true, file_size: { maximum: FILE_SIZE_LIMIT }
validates :checksum, :file_store, :name, :permissions, :project_id, presence: true validates :checksum, :file_store, :name, :permissions, :project_id, presence: true
validate :validate_upload_checksum, on: :create
before_validation :assign_checksum before_validation :assign_checksum
...@@ -33,5 +36,11 @@ module Ci ...@@ -33,5 +36,11 @@ module Ci
def assign_checksum def assign_checksum
self.checksum = file.checksum if file.present? && file_changed? self.checksum = file.checksum if file.present? && file_changed?
end end
def validate_upload_checksum
unless self.file_checksum.nil?
errors.add(:file_checksum, _("Secure Files|File did not match the provided checksum")) unless self.file_checksum == self.checksum
end
end
end end
end end
...@@ -102,10 +102,11 @@ POST /projects/:project_id/secure_files ...@@ -102,10 +102,11 @@ POST /projects/:project_id/secure_files
Supported attributes: Supported attributes:
| Attribute | Type | Required | Description | | Attribute | Type | Required | Description |
|---------------|----------------|------------------------|-------------| |-----------------|----------------|------------------------|-------------|
| `project_id` | integer/string | **{check-circle}** Yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user. | | `project_id` | integer/string | **{check-circle}** Yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user. |
| `name` | string | **{check-circle}** Yes | The `name` of the file being uploaded. | | `name` | string | **{check-circle}** Yes | The `name` of the file being uploaded. |
| `file` | file | **{check-circle}** Yes | The `file` being uploaded. | | `file` | file | **{check-circle}** Yes | The `file` being uploaded. |
| `file_checksum` | file | **{dotted-circle}** No | An optional sha256 checksum of the file to be uploaded. If provided, the checksum must match the uploaded file, or the upload will fail to validate. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/355653) in GitLab 14.10. |
| `permissions` | string | **{dotted-circle}** No | The file is created with the specified permissions when created in the CI/CD job. Available types are: `read_only` (default), `read_write`, and `execute`. | | `permissions` | string | **{dotted-circle}** No | The file is created with the specified permissions when created in the CI/CD job. Available types are: `read_only` (default), `read_write`, and `execute`. |
Example request: Example request:
......
...@@ -62,12 +62,14 @@ module API ...@@ -62,12 +62,14 @@ module API
requires :name, type: String, desc: 'The name of the file' requires :name, type: String, desc: 'The name of the file'
requires :file, types: [Rack::Multipart::UploadedFile, ::API::Validations::Types::WorkhorseFile], desc: 'The secure file to be uploaded' requires :file, types: [Rack::Multipart::UploadedFile, ::API::Validations::Types::WorkhorseFile], desc: 'The secure file to be uploaded'
optional :permissions, type: String, desc: 'The file permissions', default: 'read_only', values: %w[read_only read_write execute] optional :permissions, type: String, desc: 'The file permissions', default: 'read_only', values: %w[read_only read_write execute]
optional :file_checksum, type: String, desc: 'An optional sha256 checksum of the file to be uploaded'
end end
route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true
post ':id/secure_files' do post ':id/secure_files' do
secure_file = user_project.secure_files.new( secure_file = user_project.secure_files.new(
name: params[:name], name: params[:name],
permissions: params[:permissions] || :read_only permissions: params[:permissions] || :read_only,
file_checksum: params[:file_checksum]
) )
secure_file.file = params[:file] secure_file.file = params[:file]
......
...@@ -32856,6 +32856,9 @@ msgstr "" ...@@ -32856,6 +32856,9 @@ msgstr ""
msgid "Secure Files" msgid "Secure Files"
msgstr "" msgstr ""
msgid "Secure Files|File did not match the provided checksum"
msgstr ""
msgid "Secure token that identifies an external storage request." msgid "Secure token that identifies an external storage request."
msgstr "" msgstr ""
......
...@@ -232,6 +232,22 @@ RSpec.describe API::Ci::SecureFiles do ...@@ -232,6 +232,22 @@ RSpec.describe API::Ci::SecureFiles do
expect(Base64.encode64(response.body)).to eq(Base64.encode64(fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks').read)) expect(Base64.encode64(response.body)).to eq(Base64.encode64(fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks').read))
end end
it 'uploads and validates a secure file with a provided checksum' do
params = {
file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
name: 'upload-keystore.jks',
permissions: 'execute',
file_checksum: Digest::SHA256.hexdigest(File.read(fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks')))
}
expect do
post api("/projects/#{project.id}/secure_files", maintainer), params: params
end.to change {project.secure_files.count}.by(1)
expect(response).to have_gitlab_http_status(:created)
expect(json_response['name']).to eq('upload-keystore.jks')
end
it 'returns an error when the file checksum fails to validate' do it 'returns an error when the file checksum fails to validate' do
secure_file.update!(checksum: 'foo') secure_file.update!(checksum: 'foo')
...@@ -242,6 +258,22 @@ RSpec.describe API::Ci::SecureFiles do ...@@ -242,6 +258,22 @@ RSpec.describe API::Ci::SecureFiles do
expect(response.code).to eq("500") expect(response.code).to eq("500")
end end
it 'returns an error when the user provided file checksum fails to validate' do
post_params = {
file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
name: 'upload-keystore.jks',
permissions: 'read_write',
file_checksum: 'foo'
}
expect do
post api("/projects/#{project.id}/secure_files", maintainer), params: post_params
end.not_to change { project.secure_files.count }
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['message']['file_checksum']).to include(_("Secure Files|File did not match the provided checksum"))
end
it 'returns an error when no file is uploaded' do it 'returns an error when no file is uploaded' do
post_params = { post_params = {
name: 'upload-keystore.jks' name: 'upload-keystore.jks'
......
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