Adding resource block for admin APIs, updated API specs

lib/api/ci/secure_files.rb

@@ -52,44 +52,44 @@ module API
           body secure_file.file.read
         end
 
-        # Additional authorization check for admin endpoints
-        # All APIs defined below this block will require admin level permissions
-        before do
-          authorize! :admin_secure_files, user_project
-        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 to be uploaded'
-          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
-        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]
-
-          file_too_large! unless secure_file.file.size < ::Ci::SecureFile::FILE_SIZE_LIMIT.to_i
+        resource do
+          before do
+            authorize! :admin_secure_files, user_project
+          end
 
-          if secure_file.save
-            present secure_file, with: Entities::Ci::SecureFile
-          else
-            render_validation_error!(secure_file)
+          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 to be uploaded'
+            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
+          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]
+
+            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
+              render_validation_error!(secure_file)
+            end
           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])
+          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])
 
-          ::Ci::DestroySecureFileService.new(user_project, current_user).execute(secure_file)
+            ::Ci::DestroySecureFileService.new(user_project, current_user).execute(secure_file)
 
-          no_content!
+            no_content!
+          end
         end
       end
 

spec/requests/api/ci/secure_files_spec.rb

@@ -235,7 +235,9 @@ RSpec.describe API::Ci::SecureFiles do
       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", maintainer)
+        expect do
+          get api("/projects/#{project.id}/secure_files/#{secure_file.id}/download", maintainer)
+        end.not_to change { project.secure_files.count }
 
         expect(response.code).to eq("500")
       end
@@ -245,7 +247,9 @@ RSpec.describe API::Ci::SecureFiles do
           name: 'upload-keystore.jks'
         }
 
-        post api("/projects/#{project.id}/secure_files", maintainer), params: post_params
+        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['error']).to eq('file is missing')
@@ -256,7 +260,9 @@ RSpec.describe API::Ci::SecureFiles do
           file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks')
         }
 
-        post api("/projects/#{project.id}/secure_files", maintainer), params: post_params
+        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['error']).to eq('name is missing')
@@ -269,7 +275,9 @@ RSpec.describe API::Ci::SecureFiles do
           permissions: 'foo'
         }
 
-        post api("/projects/#{project.id}/secure_files", maintainer), params: post_params
+        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['error']).to eq('permissions does not have a valid value')
@@ -287,7 +295,9 @@ RSpec.describe API::Ci::SecureFiles do
           name: 'upload-keystore.jks'
         }
 
-        post api("/projects/#{project.id}/secure_files", maintainer), params: post_params
+        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)
       end
@@ -302,7 +312,9 @@ RSpec.describe API::Ci::SecureFiles do
           name: 'upload-keystore.jks'
         }
 
-        post api("/projects/#{project.id}/secure_files", maintainer), params: post_params
+        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(:payload_too_large)
       end
@@ -310,7 +322,9 @@ RSpec.describe API::Ci::SecureFiles do
 
     context 'authenticated user with read permissions' do
       it 'does not create a secure file' do
-        post api("/projects/#{project.id}/secure_files", developer)
+        expect do
+          post api("/projects/#{project.id}/secure_files", developer)
+        end.not_to change { project.secure_files.count }
 
         expect(response).to have_gitlab_http_status(:forbidden)
       end
@@ -318,7 +332,9 @@ RSpec.describe API::Ci::SecureFiles do
 
     context 'authenticated user with no permissions' do
       it 'does not create a secure file' do
-        post api("/projects/#{project.id}/secure_files", anonymous)
+        expect do
+          post api("/projects/#{project.id}/secure_files", anonymous)
+        end.not_to change { project.secure_files.count }
 
         expect(response).to have_gitlab_http_status(:not_found)
       end
@@ -326,7 +342,9 @@ RSpec.describe API::Ci::SecureFiles do
 
     context 'unauthenticated user' do
       it 'does not create a secure file' do
-        post api("/projects/#{project.id}/secure_files")
+        expect do
+          post api("/projects/#{project.id}/secure_files")
+        end.not_to change { project.secure_files.count }
 
         expect(response).to have_gitlab_http_status(:unauthorized)
       end
@@ -340,11 +358,13 @@ RSpec.describe API::Ci::SecureFiles do
           delete api("/projects/#{project.id}/secure_files/#{secure_file.id}", maintainer)
 
           expect(response).to have_gitlab_http_status(:no_content)
-        end.to change {project.secure_files.count}.by(-1)
+        end.to change { project.secure_files.count }
       end
 
       it 'responds with 404 Not Found if requesting non-existing secure_file' do
-        delete api("/projects/#{project.id}/secure_files/#{non_existing_record_id}", maintainer)
+        expect do
+          delete api("/projects/#{project.id}/secure_files/#{non_existing_record_id}", maintainer)
+        end.not_to change { project.secure_files.count }
 
         expect(response).to have_gitlab_http_status(:not_found)
       end
@@ -352,7 +372,9 @@ RSpec.describe API::Ci::SecureFiles do
 
     context 'authenticated user with read permissions' do
       it 'does not delete the secure_file' do
-        delete api("/projects/#{project.id}/secure_files/#{secure_file.id}", developer)
+        expect do
+          delete api("/projects/#{project.id}/secure_files/#{secure_file.id}", developer)
+        end.not_to change { project.secure_files.count }
 
         expect(response).to have_gitlab_http_status(:forbidden)
       end
@@ -360,7 +382,9 @@ RSpec.describe API::Ci::SecureFiles do
 
     context 'authenticated user with no permissions' do
       it 'does not delete the secure_file' do
-        delete api("/projects/#{project.id}/secure_files/#{secure_file.id}", anonymous)
+        expect do
+          delete api("/projects/#{project.id}/secure_files/#{secure_file.id}", anonymous)
+        end.not_to change { project.secure_files.count }
 
         expect(response).to have_gitlab_http_status(:not_found)
       end
@@ -368,7 +392,9 @@ RSpec.describe API::Ci::SecureFiles do
 
     context 'unauthenticated user' do
       it 'does not delete the secure_file' do
-        delete api("/projects/#{project.id}/secure_files/#{secure_file.id}")
+        expect do
+          delete api("/projects/#{project.id}/secure_files/#{secure_file.id}")
+        end.not_to change { project.secure_files.count }
 
         expect(response).to have_gitlab_http_status(:unauthorized)
       end