diff --git a/app/models/ci/secure_file.rb b/app/models/ci/secure_file.rb index 1509344415d4327ebe1af49cef8bf30178992c44..6a26a5341aadfb0742939c0e6dfe62d16149edca 100644 --- a/app/models/ci/secure_file.rb +++ b/app/models/ci/secure_file.rb @@ -15,6 +15,7 @@ class SecureFile < Ci::ApplicationRecord validates :file, presence: true, file_size: { maximum: FILE_SIZE_LIMIT } validates :checksum, :file_store, :name, :permissions, :project_id, presence: true + validates :name, uniqueness: { scope: :project } after_initialize :generate_key_data before_validation :assign_checksum diff --git a/doc/api/secure_files.md b/doc/api/secure_files.md index 8775e9a5f46d2072292b04be5bf4e52b9145ebca..7d30cc0c4c780e8febcc17827b3426fdcdea9376 100644 --- a/doc/api/secure_files.md +++ b/doc/api/secure_files.md @@ -103,7 +103,7 @@ Supported attributes: | 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. | -| `name` | string | **{check-circle}** Yes | The `name` of the file being uploaded. | +| `name` | string | **{check-circle}** Yes | The `name` of the file being uploaded. The file name must be unique within the project. | | `file` | file | **{check-circle}** Yes | The `file` being uploaded (5 MB limit). | | `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`. | diff --git a/spec/models/ci/secure_file_spec.rb b/spec/models/ci/secure_file_spec.rb index 4382385aaf5a06a473b724c5e9c3a020eb510538..f92db3fe8db5190a4d42055caf9d862c167cc9c4 100644 --- a/spec/models/ci/secure_file_spec.rb +++ b/spec/models/ci/secure_file_spec.rb @@ -3,14 +3,14 @@ require 'spec_helper' RSpec.describe Ci::SecureFile do - let(:sample_file) { fixture_file('ci_secure_files/upload-keystore.jks') } - - subject { create(:ci_secure_file) } - before do stub_ci_secure_file_object_storage end + let(:sample_file) { fixture_file('ci_secure_files/upload-keystore.jks') } + + subject { create(:ci_secure_file, file: CarrierWaveStringFile.new(sample_file)) } + it { is_expected.to be_a FileStoreMounter } it { is_expected.to belong_to(:project).required } @@ -27,6 +27,26 @@ it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_presence_of(:permissions) } it { is_expected.to validate_presence_of(:project_id) } + context 'unique filename' do + let_it_be(:project1) { create(:project) } + + it 'ensures the file name is unique within a given project' do + file1 = create(:ci_secure_file, project: project1, name: 'file1') + expect do + create(:ci_secure_file, project: project1, name: 'file1') + end.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Name has already been taken') + + expect(project1.secure_files.where(name: 'file1').count).to be 1 + expect(project1.secure_files.find_by(name: 'file1').id).to eq(file1.id) + end + + it 'allows duplicate file names in different projects' do + create(:ci_secure_file, project: project1) + expect do + create(:ci_secure_file, project: create(:project)) + end.not_to raise_error + end + end end describe '#permissions' do @@ -37,8 +57,6 @@ describe '#checksum' do it 'computes SHA256 checksum on the file before encrypted' do - subject.file = CarrierWaveStringFile.new(sample_file) - subject.save! expect(subject.checksum).to eq(Digest::SHA256.hexdigest(sample_file)) end end @@ -51,8 +69,6 @@ describe '#file' do it 'returns the saved file' do - subject.file = CarrierWaveStringFile.new(sample_file) - subject.save! expect(Base64.encode64(subject.file.read)).to eq(Base64.encode64(sample_file)) end end diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb index 56838c4f6fd7969762b05358da519ca65d8718b3..d699fe02ba4aa328abd0318de4a1b72245ba709e 100644 --- a/spec/requests/api/ci/secure_files_spec.rb +++ b/spec/requests/api/ci/secure_files_spec.rb @@ -293,6 +293,20 @@ expect(json_response['error']).to eq('name is missing') end + it 'returns an error when the file name has already been used' do + post_params = { + name: secure_file.name, + file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks') + } + + 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']['name']).to include('has already been taken') + end + it 'returns an error when an unexpected permission is supplied' do post_params = { file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),