From 873cfffe32f4e1e88f2636473e96947d6a684024 Mon Sep 17 00:00:00 2001
From: Darby Frey <dfrey@gitlab.com>
Date: Thu, 13 Jan 2022 15:18:51 -0600
Subject: [PATCH 1/7] Adding Secure Files API

---
 lib/api/api.rb                            |   1 +
 lib/api/ci/secure_files.rb                |  94 ++++++++++
 lib/api/entities/ci/secure_file.rb        |  15 ++
 spec/requests/api/ci/secure_files_spec.rb | 213 ++++++++++++++++++++++
 4 files changed, 323 insertions(+)
 create mode 100644 lib/api/ci/secure_files.rb
 create mode 100644 lib/api/entities/ci/secure_file.rb
 create mode 100644 spec/requests/api/ci/secure_files_spec.rb

diff --git a/lib/api/api.rb b/lib/api/api.rb
index 5984879413f260..bfd070ba6da0e4 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -171,6 +171,7 @@ class API < ::API::Base
       mount ::API::Ci::ResourceGroups
       mount ::API::Ci::Runner
       mount ::API::Ci::Runners
+      mount ::API::Ci::SecureFiles
       mount ::API::Ci::Triggers
       mount ::API::Ci::Variables
       mount ::API::Commits
diff --git a/lib/api/ci/secure_files.rb b/lib/api/ci/secure_files.rb
new file mode 100644
index 00000000000000..6d43292ba4ac19
--- /dev/null
+++ b/lib/api/ci/secure_files.rb
@@ -0,0 +1,94 @@
+# frozen_string_literal: true
+
+module API
+  module Ci
+    class SecureFiles < ::API::Base
+      include PaginationParams
+
+      before do
+        authenticate!
+        authorize! :admin_build, user_project
+      end
+
+      feature_category :pipeline_authoring
+
+      default_format :json
+
+      params do
+        requires :id, type: String, desc: 'The ID of a project'
+      end
+
+      resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
+        desc 'List all Secure Files for a Project'
+        params do
+          use :pagination
+        end
+        route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true
+        get ':id/secure_files' do
+          secure_files = user_project.secure_files
+          present paginate(secure_files), with: Entities::Ci::SecureFile
+        end
+
+        desc 'Get an individual Secure File'
+        params do
+          requires :id, type: Integer, desc: 'The Secure File ID'
+        end
+
+        route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true
+        get ':id/secure_files/:secure_file_id' do
+          secure_file = user_project.secure_files.find(params[:secure_file_id])
+          not_found!('Secure File') unless secure_file
+          present secure_file, with: Entities::Ci::SecureFile
+        end
+
+        desc 'Download a Secure File'
+        route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true
+        get ':id/secure_files/:secure_file_id/download' do
+          secure_file = user_project.secure_files.find(params[:secure_file_id])
+          not_found!('Secure File') unless secure_file
+
+          content_type 'application/octet-stream'
+          env['api.format'] = :binary
+          header['Content-Disposition'] = "attachment; filename=#{secure_file.name}"
+          body secure_file.file.read
+        end
+
+        desc 'Upload a Secure File'
+        params do
+          requires :name, type: String, desc: 'The name of the file'
+          requires :file, types: [Rack::Multipart::UploadedFile, ::API::Validations::Types::WorkhorseFile], desc: 'The secure file file to be uploaded'
+          optional :permissions, type: String, desc: 'The file permissions'
+        end
+
+        route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true
+        post ':id/secure_files' do
+          secure_file = user_project.secure_files.new(
+            name: params[:name],
+            permissions: params[:permissions] || :read_only
+          )
+
+          secure_file.file = params[:file]
+
+          if secure_file.valid?
+            secure_file.save!
+            present secure_file, with: Entities::Ci::SecureFile
+          else
+            render_validation_error!(secure_file)
+          end
+        end
+
+        desc 'Delete an individual Secure File'
+        route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true
+        delete ':id/secure_files/:secure_file_id' do
+          secure_file = user_project.secure_files.find(params[:secure_file_id])
+
+          not_found!('Secure File') unless secure_file
+
+          secure_file.destroy!
+
+          no_content!
+        end
+      end
+    end
+  end
+end
diff --git a/lib/api/entities/ci/secure_file.rb b/lib/api/entities/ci/secure_file.rb
new file mode 100644
index 00000000000000..041c864156b463
--- /dev/null
+++ b/lib/api/entities/ci/secure_file.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module API
+  module Entities
+    module Ci
+      class SecureFile < Grape::Entity
+        expose :id
+        expose :name
+        expose :permissions
+        expose :checksum
+        expose :checksum_algorithm
+      end
+    end
+  end
+end
diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb
new file mode 100644
index 00000000000000..a77c0947a5313b
--- /dev/null
+++ b/spec/requests/api/ci/secure_files_spec.rb
@@ -0,0 +1,213 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe API::Ci::SecureFiles do
+  before do
+    stub_ci_secure_file_object_storage
+  end
+
+  let(:user) { create(:user) }
+  let(:user2) { create(:user) }
+  let!(:project) { create(:project, creator_id: user.id) }
+  let!(:maintainer) { create(:project_member, :maintainer, user: user, project: project) }
+  let!(:developer) { create(:project_member, :developer, user: user2, project: project) }
+  let!(:secure_file) { create(:ci_secure_file, project: project) }
+
+  describe 'GET /projects/:id/secure_files' do
+    context 'authorized user with proper permissions' do
+      it 'returns project secure files' do
+        get api("/projects/#{project.id}/secure_files", user)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to be_a(Array)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'does not return project secure files' do
+        get api("/projects/#{project.id}/secure_files", user2)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'does not return project secure files' do
+        get api("/projects/#{project.id}/secure_files")
+
+        expect(response).to have_gitlab_http_status(:unauthorized)
+      end
+    end
+  end
+
+  describe 'GET /projects/:id/secure_files/:secure_file_id' do
+    context 'authorized user with proper permissions' do
+      it 'returns project secure file details' do
+        get api("/projects/#{project.id}/secure_files/#{secure_file.id}", user)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['name']).to eq(secure_file.name)
+        expect(json_response['permissions']).to eq(secure_file.permissions)
+      end
+
+      it 'responds with 404 Not Found if requesting non-existing secure file' do
+        get api("/projects/#{project.id}/secure_files/99999", user)
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'does not return project secure file details' do
+        get api("/projects/#{project.id}/secure_files/#{secure_file.id}", user2)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'does not return project secure file details' do
+        get api("/projects/#{project.id}/secure_files/#{secure_file.id}")
+
+        expect(response).to have_gitlab_http_status(:unauthorized)
+      end
+    end
+  end
+
+  describe 'POST /projects/:id/secure_files' do
+    context 'authorized user with proper permissions' do
+      it 'creates a secure file' do
+        params = {
+          file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
+          name: 'upload-keystore.jks',
+          permissions: 'execute'
+        }
+
+        expect do
+          post api("/projects/#{project.id}/secure_files", user), 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')
+        expect(json_response['permissions']).to eq('execute')
+        expect(json_response['checksum']).to eq(secure_file.checksum)
+        expect(json_response['checksum_algorithm']).to eq('sha256')
+
+        secure_file = Ci::SecureFile.find(json_response['id'])
+        expect(secure_file.checksum).to eq(
+          Digest::SHA256.hexdigest(fixture_file('ci_secure_files/upload-keystore.jks'))
+        )
+        expect(json_response['id']).to eq(secure_file.id)
+      end
+
+      it 'creates a secure file with read_only permissions by default' do
+        params = {
+          file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
+          name: 'upload-keystore.jks'
+        }
+
+        expect do
+          post api("/projects/#{project.id}/secure_files", user), params: params
+        end.to change {project.secure_files.count}.by(1)
+
+        expect(json_response['permissions']).to eq('read_only')
+      end
+
+      it 'uploads and downloads a secure file' do
+        post_params = {
+          file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
+          name: 'upload-keystore.jks'
+        }
+
+        post api("/projects/#{project.id}/secure_files", user), params: post_params
+
+        secure_file_id = json_response['id']
+
+        get api("/projects/#{project.id}/secure_files/#{secure_file_id}/download", user)
+
+        expect(Base64.encode64(response.body)).to eq(Base64.encode64(fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks').read))
+      end
+
+      it 'returns an error when the file checksum fails to validate' do
+        secure_file.update!(checksum: 'foo')
+
+        get api("/projects/#{project.id}/secure_files/#{secure_file.id}/download", user)
+
+        expect(response.code).to eq("500")
+      end
+
+      it 'returns an error when no file is uploaded' do
+        post_params = {
+          name: 'upload-keystore.jks'
+        }
+
+        post api("/projects/#{project.id}/secure_files", user), params: post_params
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+        expect(json_response['error']).to eq('file is missing')
+      end
+
+      it 'returns an error when the file name is missing' do
+        post_params = {
+          file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks')
+        }
+
+        post api("/projects/#{project.id}/secure_files", user), params: post_params
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+        expect(json_response['error']).to eq('name is missing')
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'does not create a secure file' do
+        post api("/projects/#{project.id}/secure_files", user2)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'does not create a secure file' do
+        post api("/projects/#{project.id}/secure_files")
+
+        expect(response).to have_gitlab_http_status(:unauthorized)
+      end
+    end
+  end
+
+  describe 'DELETE /projects/:id/secure_files/:secure_file_id' do
+    context 'authorized user with proper permissions' do
+      it 'deletes the secure file' do
+        expect do
+          delete api("/projects/#{project.id}/secure_files/#{secure_file.id}", user)
+
+          expect(response).to have_gitlab_http_status(:no_content)
+        end.to change {project.secure_files.count}.by(-1)
+      end
+
+      it 'responds with 404 Not Found if requesting non-existing secure_file' do
+        delete api("/projects/#{project.id}/secure_files/99999", user)
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'does not delete the secure_file' do
+        delete api("/projects/#{project.id}/secure_files/#{secure_file.id}", user2)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'does not delete the secure_file' do
+        delete api("/projects/#{project.id}/secure_files/#{secure_file.id}")
+
+        expect(response).to have_gitlab_http_status(:unauthorized)
+      end
+    end
+  end
+end
-- 
GitLab


From 3485d4d154a7a342820375cacf92ef6180175f69 Mon Sep 17 00:00:00 2001
From: Darby Frey <dfrey@gitlab.com>
Date: Fri, 14 Jan 2022 16:34:04 -0600
Subject: [PATCH 2/7] Adding an API test for unexpected validation errors

---
 lib/api/ci/secure_files.rb                |  2 +-
 spec/requests/api/ci/secure_files_spec.rb | 29 ++++++++++++++++++-----
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/lib/api/ci/secure_files.rb b/lib/api/ci/secure_files.rb
index 6d43292ba4ac19..2f8738c6334662 100644
--- a/lib/api/ci/secure_files.rb
+++ b/lib/api/ci/secure_files.rb
@@ -56,7 +56,7 @@ class SecureFiles < ::API::Base
         desc 'Upload a Secure File'
         params do
           requires :name, type: String, desc: 'The name of the file'
-          requires :file, types: [Rack::Multipart::UploadedFile, ::API::Validations::Types::WorkhorseFile], desc: 'The secure file 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'
         end
 
diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb
index a77c0947a5313b..679c20aa26151a 100644
--- a/spec/requests/api/ci/secure_files_spec.rb
+++ b/spec/requests/api/ci/secure_files_spec.rb
@@ -7,12 +7,12 @@
     stub_ci_secure_file_object_storage
   end
 
-  let(:user) { create(:user) }
-  let(:user2) { create(:user) }
-  let!(:project) { create(:project, creator_id: user.id) }
-  let!(:maintainer) { create(:project_member, :maintainer, user: user, project: project) }
-  let!(:developer) { create(:project_member, :developer, user: user2, project: project) }
-  let!(:secure_file) { create(:ci_secure_file, project: project) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:user2) { create(:user) }
+  let_it_be(:project) { create(:project, creator_id: user.id) }
+  let_it_be(:maintainer) { create(:project_member, :maintainer, user: user, project: project) }
+  let_it_be(:developer) { create(:project_member, :developer, user: user2, project: project) }
+  let_it_be(:secure_file) { create(:ci_secure_file, project: project) }
 
   describe 'GET /projects/:id/secure_files' do
     context 'authorized user with proper permissions' do
@@ -158,6 +158,23 @@
         expect(response).to have_gitlab_http_status(:bad_request)
         expect(json_response['error']).to eq('name is missing')
       end
+
+      it 'retuns an error when an unexpected validation failure happens' do
+        allow_next_instance_of(Ci::SecureFile) do |instance|
+          allow(instance).to receive(:valid?).and_return(false)
+          allow(instance).to receive_message_chain(:errors, :any?).and_return(true)
+          allow(instance).to receive_message_chain(:errors, :messages).and_return(['Error 1', 'Error 2'])
+        end
+
+        post_params = {
+          file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
+          name: 'upload-keystore.jks'
+        }
+
+        post api("/projects/#{project.id}/secure_files", user), params: post_params
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+      end
     end
 
     context 'authorized user with invalid permissions' do
-- 
GitLab


From a2858d4ddc5ffad6b820e4cf085d0cd7693dc158 Mon Sep 17 00:00:00 2001
From: Darby Frey <dfrey@gitlab.com>
Date: Mon, 17 Jan 2022 16:24:03 -0600
Subject: [PATCH 3/7] Adding permissions values to upload endpoint and test
 case

---
 lib/api/ci/secure_files.rb                |  2 +-
 spec/requests/api/ci/secure_files_spec.rb | 18 ++++++++++++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/api/ci/secure_files.rb b/lib/api/ci/secure_files.rb
index 2f8738c6334662..13cd5aaa20fac1 100644
--- a/lib/api/ci/secure_files.rb
+++ b/lib/api/ci/secure_files.rb
@@ -57,7 +57,7 @@ class SecureFiles < ::API::Base
         params do
           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'
-          optional :permissions, type: String, desc: 'The file permissions'
+          optional :permissions, type: String, desc: 'The file permissions', default: 'read_only', values: %w[read_only read_write execute]
         end
 
         route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true
diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb
index 679c20aa26151a..983506028e1dc5 100644
--- a/spec/requests/api/ci/secure_files_spec.rb
+++ b/spec/requests/api/ci/secure_files_spec.rb
@@ -117,7 +117,8 @@
       it 'uploads and downloads a secure file' do
         post_params = {
           file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
-          name: 'upload-keystore.jks'
+          name: 'upload-keystore.jks',
+          permissions: 'read_write'
         }
 
         post api("/projects/#{project.id}/secure_files", user), params: post_params
@@ -159,7 +160,20 @@
         expect(json_response['error']).to eq('name is missing')
       end
 
-      it 'retuns an error when an unexpected validation failure happens' do
+      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'),
+          name: 'upload-keystore.jks',
+          permissions: 'foo'
+        }
+
+        post api("/projects/#{project.id}/secure_files", user), params: post_params
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+        expect(json_response['error']).to eq('permissions does not have a valid value')
+      end
+
+      it 'returns an error when an unexpected validation failure happens' do
         allow_next_instance_of(Ci::SecureFile) do |instance|
           allow(instance).to receive(:valid?).and_return(false)
           allow(instance).to receive_message_chain(:errors, :any?).and_return(true)
-- 
GitLab


From 41a553c1a83658b1d6d23ee6bd88a94ef675a6fa Mon Sep 17 00:00:00 2001
From: Darby Frey <dfrey@gitlab.com>
Date: Mon, 17 Jan 2022 17:19:34 -0600
Subject: [PATCH 4/7] Adding a missed setting in gitlab.yml.example

---
 config/gitlab.yml.example | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index f5755591da731e..632f077f4a6f0c 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -404,6 +404,23 @@ production: &base
         # aws_signature_version: 4 # For creation of signed URLs. Set to 2 if provider does not support v4.
         # path_style: true # Use 'host/bucket_name/object' instead of 'bucket_name.host/object'
 
+  ## CI Secure Files
+  ci_secure_files:
+    enabled: true
+    # storage_path: shared/ci_secure_files
+    object_store:
+      enabled: false
+      remote_directory: ci_secure_files # The bucket name
+      connection:
+        provider: AWS
+        aws_access_key_id: AWS_ACCESS_KEY_ID
+        aws_secret_access_key: AWS_SECRET_ACCESS_KEY
+        region: us-east-1
+        # host: 'localhost' # default: s3.amazonaws.com
+        # endpoint: 'http://127.0.0.1:9000' # default: nil
+        # aws_signature_version: 4 # For creation of signed URLs. Set to 2 if provider does not support v4.
+        # path_style: true # Use 'host/bucket_name/object' instead of 'bucket_name.host/object'
+
   ## GitLab Pages
   pages:
     enabled: false
-- 
GitLab


From b7b5f4c9ef551e550a91fae089ba82d199e4f2b4 Mon Sep 17 00:00:00 2001
From: Darby Frey <dfrey@gitlab.com>
Date: Tue, 18 Jan 2022 15:05:45 -0600
Subject: [PATCH 5/7] Secure Files API clean up, added tests

---
 lib/api/ci/secure_files.rb                |  7 +----
 spec/requests/api/ci/secure_files_spec.rb | 37 +++++++++++++++++++++++
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/lib/api/ci/secure_files.rb b/lib/api/ci/secure_files.rb
index 13cd5aaa20fac1..2e0574e8fc8b2b 100644
--- a/lib/api/ci/secure_files.rb
+++ b/lib/api/ci/secure_files.rb
@@ -37,7 +37,6 @@ class SecureFiles < ::API::Base
         route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true
         get ':id/secure_files/:secure_file_id' do
           secure_file = user_project.secure_files.find(params[:secure_file_id])
-          not_found!('Secure File') unless secure_file
           present secure_file, with: Entities::Ci::SecureFile
         end
 
@@ -45,7 +44,6 @@ class SecureFiles < ::API::Base
         route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true
         get ':id/secure_files/:secure_file_id/download' do
           secure_file = user_project.secure_files.find(params[:secure_file_id])
-          not_found!('Secure File') unless secure_file
 
           content_type 'application/octet-stream'
           env['api.format'] = :binary
@@ -69,8 +67,7 @@ class SecureFiles < ::API::Base
 
           secure_file.file = params[:file]
 
-          if secure_file.valid?
-            secure_file.save!
+          if secure_file.save
             present secure_file, with: Entities::Ci::SecureFile
           else
             render_validation_error!(secure_file)
@@ -82,8 +79,6 @@ class SecureFiles < ::API::Base
         delete ':id/secure_files/:secure_file_id' do
           secure_file = user_project.secure_files.find(params[:secure_file_id])
 
-          not_found!('Secure File') unless secure_file
-
           secure_file.destroy!
 
           no_content!
diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb
index 983506028e1dc5..894261e3cc07eb 100644
--- a/spec/requests/api/ci/secure_files_spec.rb
+++ b/spec/requests/api/ci/secure_files_spec.rb
@@ -75,6 +75,43 @@
     end
   end
 
+  describe 'GET /projects/:id/secure_files/:secure_file_id/download' do
+    context 'authorized user with proper permissions' do
+      it 'returns a secure file' do
+        sample_file = fixture_file('ci_secure_files/upload-keystore.jks')
+        secure_file.file = CarrierWaveStringFile.new(sample_file)
+        secure_file.save!
+
+        get api("/projects/#{project.id}/secure_files/#{secure_file.id}/download", user)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(Base64.encode64(response.body)).to eq(Base64.encode64(sample_file))
+      end
+
+      it 'responds with 404 Not Found if requesting non-existing secure file' do
+        get api("/projects/#{project.id}/secure_files/99999/download", user)
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'does not return project secure file details' do
+        get api("/projects/#{project.id}/secure_files/#{secure_file.id}/download", user2)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'does not return project secure file details' do
+        get api("/projects/#{project.id}/secure_files/#{secure_file.id}/download")
+
+        expect(response).to have_gitlab_http_status(:unauthorized)
+      end
+    end
+  end
+
   describe 'POST /projects/:id/secure_files' do
     context 'authorized user with proper permissions' do
       it 'creates a secure file' do
-- 
GitLab


From 1ead057dabff28ad14b9d1ea0712b5b3cfd4c4ff Mon Sep 17 00:00:00 2001
From: Darby Frey <dfrey@gitlab.com>
Date: Wed, 19 Jan 2022 16:36:07 -0600
Subject: [PATCH 6/7] Adding custom error response for when file upload is too
 large

---
 lib/api/ci/secure_files.rb                |  3 +++
 spec/requests/api/ci/secure_files_spec.rb | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/lib/api/ci/secure_files.rb b/lib/api/ci/secure_files.rb
index 2e0574e8fc8b2b..cfc8df93364bad 100644
--- a/lib/api/ci/secure_files.rb
+++ b/lib/api/ci/secure_files.rb
@@ -60,6 +60,7 @@ class SecureFiles < ::API::Base
 
         route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true
         post ':id/secure_files' do
+  
           secure_file = user_project.secure_files.new(
             name: params[:name],
             permissions: params[:permissions] || :read_only
@@ -67,6 +68,8 @@ class SecureFiles < ::API::Base
 
           secure_file.file = params[:file]
 
+          file_too_large! unless secure_file.file.size < ::Ci::SecureFile::FILE_SIZE_LIMIT.to_i
+
           if secure_file.save
             present secure_file, with: Entities::Ci::SecureFile
           else
diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb
index 894261e3cc07eb..c83d4d94183ab9 100644
--- a/spec/requests/api/ci/secure_files_spec.rb
+++ b/spec/requests/api/ci/secure_files_spec.rb
@@ -226,6 +226,21 @@
 
         expect(response).to have_gitlab_http_status(:bad_request)
       end
+
+      it 'returns a 413 error when the file size is too large' do
+        allow_next_instance_of(Ci::SecureFile) do |instance|
+          allow(instance).to receive_message_chain(:file, :size).and_return(6.megabytes.to_i)
+        end
+
+        post_params = {
+          file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
+          name: 'upload-keystore.jks'
+        }
+
+        post api("/projects/#{project.id}/secure_files", user), params: post_params
+
+        expect(response).to have_gitlab_http_status(:payload_too_large)
+      end
     end
 
     context 'authorized user with invalid permissions' do
-- 
GitLab


From 71730c281d3536d8fe530a39d8368bd71b9995ad Mon Sep 17 00:00:00 2001
From: Darby Frey <dfrey@gitlab.com>
Date: Thu, 20 Jan 2022 09:35:53 -0600
Subject: [PATCH 7/7] Changing API to be behind a feature flag

---
 .../development/ci_secure_files.yml            |  8 ++++++++
 lib/api/ci/secure_files.rb                     |  8 +++++++-
 spec/requests/api/ci/secure_files_spec.rb      | 18 ++++++++++++++++++
 3 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 config/feature_flags/development/ci_secure_files.yml

diff --git a/config/feature_flags/development/ci_secure_files.yml b/config/feature_flags/development/ci_secure_files.yml
new file mode 100644
index 00000000000000..a1aa82fe298432
--- /dev/null
+++ b/config/feature_flags/development/ci_secure_files.yml
@@ -0,0 +1,8 @@
+---
+name: ci_secure_files
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78227
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/350748
+milestone: '14.8'
+type: development
+group: group::incubation
+default_enabled: false
diff --git a/lib/api/ci/secure_files.rb b/lib/api/ci/secure_files.rb
index cfc8df93364bad..715a8b37fae2ba 100644
--- a/lib/api/ci/secure_files.rb
+++ b/lib/api/ci/secure_files.rb
@@ -8,6 +8,7 @@ class SecureFiles < ::API::Base
       before do
         authenticate!
         authorize! :admin_build, user_project
+        feature_flag_enabled?
       end
 
       feature_category :pipeline_authoring
@@ -60,7 +61,6 @@ class SecureFiles < ::API::Base
 
         route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true
         post ':id/secure_files' do
-  
           secure_file = user_project.secure_files.new(
             name: params[:name],
             permissions: params[:permissions] || :read_only
@@ -87,6 +87,12 @@ class SecureFiles < ::API::Base
           no_content!
         end
       end
+
+      helpers do
+        def feature_flag_enabled?
+          service_unavailable! unless Feature.enabled?(:ci_secure_files, user_project, default_enabled: :yaml)
+        end
+      end
     end
   end
 end
diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb
index c83d4d94183ab9..5cf6999f60a1e3 100644
--- a/spec/requests/api/ci/secure_files_spec.rb
+++ b/spec/requests/api/ci/secure_files_spec.rb
@@ -5,6 +5,7 @@
 RSpec.describe API::Ci::SecureFiles do
   before do
     stub_ci_secure_file_object_storage
+    stub_feature_flags(ci_secure_files: true)
   end
 
   let_it_be(:user) { create(:user) }
@@ -15,6 +16,23 @@
   let_it_be(:secure_file) { create(:ci_secure_file, project: project) }
 
   describe 'GET /projects/:id/secure_files' do
+    context 'feature flag' do
+      it 'returns a 503 when the feature flag is disabled' do
+        stub_feature_flags(ci_secure_files: false)
+
+        get api("/projects/#{project.id}/secure_files", user)
+
+        expect(response).to have_gitlab_http_status(:service_unavailable)
+      end
+
+      it 'returns a 200 when the feature flag is enabled' do
+        get api("/projects/#{project.id}/secure_files", user)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to be_a(Array)
+      end
+    end
+
     context 'authorized user with proper permissions' do
       it 'returns project secure files' do
         get api("/projects/#{project.id}/secure_files", user)
-- 
GitLab