diff options
Diffstat (limited to 'app')
31 files changed, 217 insertions, 87 deletions
diff --git a/app/assets/javascripts/create_item_dropdown.js b/app/assets/javascripts/create_item_dropdown.js index 1472adf458b..b39720c6094 100644 --- a/app/assets/javascripts/create_item_dropdown.js +++ b/app/assets/javascripts/create_item_dropdown.js @@ -1,4 +1,3 @@ -import { escape } from 'lodash'; import initDeprecatedJQueryDropdown from '~/deprecated_jquery_dropdown'; export default class CreateItemDropdown { @@ -37,14 +36,14 @@ export default class CreateItemDropdown { }, selectable: true, toggleLabel(selected) { - return selected && 'id' in selected ? escape(selected.title) : this.defaultToggleLabel; + return selected && 'id' in selected ? selected.title : this.defaultToggleLabel; }, fieldName: this.fieldName, text(item) { - return escape(item.text); + return item.text; }, id(item) { - return escape(item.id); + return item.id; }, onFilter: this.toggleCreateNewButton.bind(this), clicked: (options) => { diff --git a/app/controllers/jira_connect/users_controller.rb b/app/controllers/jira_connect/users_controller.rb index 569dc42fed3..a37c68de299 100644 --- a/app/controllers/jira_connect/users_controller.rb +++ b/app/controllers/jira_connect/users_controller.rb @@ -5,7 +5,17 @@ class JiraConnect::UsersController < ApplicationController layout 'signup_onboarding' + before_action :verify_return_to_url, only: [:show] + def show @jira_app_link = params.delete(:return_to) end + + private + + def verify_return_to_url + return unless params[:return_to].present? + + params.delete(:return_to) unless Integrations::Jira.valid_jira_cloud_url?(params[:return_to]) + end end diff --git a/app/graphql/mutations/packages/destroy.rb b/app/graphql/mutations/packages/destroy.rb index 979a54da6bd..81fa53fc116 100644 --- a/app/graphql/mutations/packages/destroy.rb +++ b/app/graphql/mutations/packages/destroy.rb @@ -15,7 +15,7 @@ module Mutations def resolve(id:) package = authorized_find!(id: id) - result = ::Packages::DestroyPackageService.new(container: package, current_user: current_user).execute + result = ::Packages::MarkPackageForDestructionService.new(container: package, current_user: current_user).execute errors = result.error? ? Array.wrap(result[:message]) : [] diff --git a/app/graphql/mutations/packages/destroy_file.rb b/app/graphql/mutations/packages/destroy_file.rb index 35a486666d5..4aa33b3504c 100644 --- a/app/graphql/mutations/packages/destroy_file.rb +++ b/app/graphql/mutations/packages/destroy_file.rb @@ -15,7 +15,7 @@ module Mutations def resolve(id:) package_file = authorized_find!(id: id) - if package_file.destroy + if package_file.update(status: :pending_destruction) return { errors: [] } end diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index f4067552f55..d49244f6b65 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -438,6 +438,12 @@ module Types ::Security::CiConfiguration::SastParserService.new(object).configuration end + def service_desk_address + return unless Ability.allowed?(current_user, :admin_issue, project) + + object.service_desk_address + end + def tag_list object.topic_list end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index b69c0199c70..3c9f7c4dd7f 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -10,6 +10,7 @@ class ApplicationSetting < ApplicationRecord ignore_columns %i[elasticsearch_shards elasticsearch_replicas], remove_with: '14.4', remove_after: '2021-09-22' ignore_columns %i[static_objects_external_storage_auth_token], remove_with: '14.9', remove_after: '2022-03-22' + ignore_column %i[max_package_files_for_package_destruction], remove_with: '14.9', remove_after: '2022-03-22' INSTANCE_REVIEW_MIN_USERS = 50 GRAFANA_URL_ERROR_MESSAGE = 'Please check your Grafana URL setting in ' \ diff --git a/app/models/concerns/integrations/enable_ssl_verification.rb b/app/models/concerns/integrations/enable_ssl_verification.rb new file mode 100644 index 00000000000..11dc8a76a2b --- /dev/null +++ b/app/models/concerns/integrations/enable_ssl_verification.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Integrations + module EnableSslVerification + extend ActiveSupport::Concern + + prepended do + boolean_accessor :enable_ssl_verification + end + + def initialize_properties + super + + self.enable_ssl_verification = true if new_record? && enable_ssl_verification.nil? + end + + def fields + super.tap do |fields| + url_index = fields.index { |field| field[:name].ends_with?('_url') } + insert_index = url_index ? url_index + 1 : -1 + + fields.insert(insert_index, { + type: 'checkbox', + name: 'enable_ssl_verification', + title: s_('Integrations|SSL verification'), + checkbox_label: s_('Integrations|Enable SSL verification'), + help: s_('Integrations|Clear if using a self-signed certificate.') + }) + end + end + end +end diff --git a/app/models/concerns/integrations/has_web_hook.rb b/app/models/concerns/integrations/has_web_hook.rb index dabe7152b18..bc28c32695c 100644 --- a/app/models/concerns/integrations/has_web_hook.rb +++ b/app/models/concerns/integrations/has_web_hook.rb @@ -15,7 +15,11 @@ module Integrations # Return whether the webhook should use SSL verification. def hook_ssl_verification - true + if respond_to?(:enable_ssl_verification) + enable_ssl_verification + else + true + end end # Create or update the webhook, raising an exception if it cannot be saved. diff --git a/app/models/integrations/bamboo.rb b/app/models/integrations/bamboo.rb index 0774b84b69f..57767c63cf4 100644 --- a/app/models/integrations/bamboo.rb +++ b/app/models/integrations/bamboo.rb @@ -4,6 +4,7 @@ module Integrations class Bamboo < BaseCi include ActionView::Helpers::UrlHelper include ReactivelyCached + prepend EnableSslVerification prop_accessor :bamboo_url, :build_key, :username, :password @@ -162,7 +163,7 @@ module Integrations end def build_get_params(query_params) - params = { verify: false, query: query_params } + params = { verify: enable_ssl_verification, query: query_params } return params if username.blank? && password.blank? query_params[:os_authType] = 'basic' diff --git a/app/models/integrations/buildkite.rb b/app/models/integrations/buildkite.rb index 9fad3a42647..90593d78a5d 100644 --- a/app/models/integrations/buildkite.rb +++ b/app/models/integrations/buildkite.rb @@ -137,7 +137,7 @@ module Integrations end def request_options - { verify: false, extra_log_info: { project_id: project_id } } + { extra_log_info: { project_id: project_id } } end end end diff --git a/app/models/integrations/drone_ci.rb b/app/models/integrations/drone_ci.rb index 856d14c022d..3c18e5d8732 100644 --- a/app/models/integrations/drone_ci.rb +++ b/app/models/integrations/drone_ci.rb @@ -5,10 +5,12 @@ module Integrations include HasWebHook include PushDataValidations include ReactivelyCached + prepend EnableSslVerification extend Gitlab::Utils::Override + DRONE_SAAS_HOSTNAME = 'cloud.drone.io' + prop_accessor :drone_url, :token - boolean_accessor :enable_ssl_verification validates :drone_url, presence: true, public_url: true, if: :activated? validates :token, presence: true, if: :activated? @@ -95,8 +97,7 @@ module Integrations def fields [ { type: 'text', name: 'token', help: s_('ProjectService|Token for the Drone project.'), required: true }, - { type: 'text', name: 'drone_url', title: s_('ProjectService|Drone server URL'), placeholder: 'http://drone.example.com', required: true }, - { type: 'checkbox', name: 'enable_ssl_verification', title: "Enable SSL verification" } + { type: 'text', name: 'drone_url', title: s_('ProjectService|Drone server URL'), placeholder: 'http://drone.example.com', required: true } ] end @@ -105,15 +106,24 @@ module Integrations [drone_url, "/hook", "?owner=#{project.namespace.full_path}", "&name=#{project.path}", "&access_token=#{token}"].join end - override :hook_ssl_verification - def hook_ssl_verification - !!enable_ssl_verification - end - override :update_web_hook! def update_web_hook! # If using a service template, project may not be available super if project end + + def enable_ssl_verification + original_value = Gitlab::Utils.to_boolean(properties['enable_ssl_verification']) + original_value.nil? ? (new_record? || url_is_saas?) : original_value + end + + private + + def url_is_saas? + parsed_url = Addressable::URI.parse(drone_url) + parsed_url&.scheme == 'https' && parsed_url.hostname == DRONE_SAAS_HOSTNAME + rescue Addressable::URI::InvalidURIError + false + end end end diff --git a/app/models/integrations/jenkins.rb b/app/models/integrations/jenkins.rb index e5c1d5ad0d7..5ea92170c26 100644 --- a/app/models/integrations/jenkins.rb +++ b/app/models/integrations/jenkins.rb @@ -4,6 +4,7 @@ module Integrations class Jenkins < BaseCi include HasWebHook include ActionView::Helpers::UrlHelper + prepend EnableSslVerification extend Gitlab::Utils::Override prop_accessor :jenkins_url, :project_name, :username, :password diff --git a/app/models/integrations/jira.rb b/app/models/integrations/jira.rb index 816f5cbe177..966ad07afad 100644 --- a/app/models/integrations/jira.rb +++ b/app/models/integrations/jira.rb @@ -56,6 +56,12 @@ module Integrations @reference_pattern ||= /(?<issue>\b#{Gitlab::Regex.jira_issue_key_regex})/ end + def self.valid_jira_cloud_url?(url) + return false unless url.present? + + !!URI(url).hostname&.end_with?(JIRA_CLOUD_HOST) + end + def initialize_properties {} end @@ -565,7 +571,7 @@ module Integrations end def jira_cloud? - server_info['deploymentType'] == 'Cloud' || URI(client_url).hostname.end_with?(JIRA_CLOUD_HOST) + server_info['deploymentType'] == 'Cloud' || self.class.valid_jira_cloud_url?(client_url) end def set_deployment_type_from_url @@ -578,7 +584,7 @@ module Integrations # we can only assume it's either Cloud or Server # based on the URL being *.atlassian.net - if URI(client_url).hostname.end_with?(JIRA_CLOUD_HOST) + if self.class.valid_jira_cloud_url?(client_url) data_fields.deployment_cloud! else data_fields.deployment_server! diff --git a/app/models/integrations/mock_ci.rb b/app/models/integrations/mock_ci.rb index 7359be83d4f..568fb609a44 100644 --- a/app/models/integrations/mock_ci.rb +++ b/app/models/integrations/mock_ci.rb @@ -3,6 +3,8 @@ # For an example companion mocking service, see https://gitlab.com/gitlab-org/gitlab-mock-ci-service module Integrations class MockCi < BaseCi + prepend EnableSslVerification + ALLOWED_STATES = %w[failed canceled running pending success success-with-warnings skipped not_found].freeze prop_accessor :mock_service_url @@ -55,7 +57,7 @@ module Integrations # # => 'running' # def commit_status(sha, ref) - response = Gitlab::HTTP.get(commit_status_path(sha), verify: false, use_read_total_timeout: true) + response = Gitlab::HTTP.get(commit_status_path(sha), verify: enable_ssl_verification, use_read_total_timeout: true) read_commit_status(response) rescue Errno::ECONNREFUSED :error @@ -68,19 +70,16 @@ module Integrations end def read_commit_status(response) - return :error unless response.code == 200 || response.code == 404 - - status = if response.code == 404 - 'pending' - else - response['status'] - end + return :pending if response.code == 404 + return :error unless response.code == 200 - if status.present? && ALLOWED_STATES.include?(status) - status - else - :error + begin + status = Gitlab::Json.parse(response.body).try(:fetch, 'status', nil) + return status if ALLOWED_STATES.include?(status) + rescue JSON::ParserError end + + :error end def testable? diff --git a/app/models/integrations/teamcity.rb b/app/models/integrations/teamcity.rb index 008b591c304..f0f83f118d7 100644 --- a/app/models/integrations/teamcity.rb +++ b/app/models/integrations/teamcity.rb @@ -4,6 +4,9 @@ module Integrations class Teamcity < BaseCi include PushDataValidations include ReactivelyCached + prepend EnableSslVerification + + TEAMCITY_SAAS_HOSTNAME = /\A[^\.]+\.teamcity\.com\z/i.freeze prop_accessor :teamcity_url, :build_type, :username, :password @@ -104,8 +107,20 @@ module Integrations end end + def enable_ssl_verification + original_value = Gitlab::Utils.to_boolean(properties['enable_ssl_verification']) + original_value.nil? ? (new_record? || url_is_saas?) : original_value + end + private + def url_is_saas? + parsed_url = Addressable::URI.parse(teamcity_url) + parsed_url&.scheme == 'https' && parsed_url.hostname.match?(TEAMCITY_SAAS_HOSTNAME) + rescue Addressable::URI::InvalidURIError + false + end + def execute_push(data) branch = Gitlab::Git.ref_name(data[:ref]) post_to_build_queue(data, branch) if push_valid?(data) @@ -155,7 +170,7 @@ module Integrations end def get_path(path) - Gitlab::HTTP.try_get(build_url(path), verify: false, basic_auth: basic_auth, extra_log_info: { project_id: project_id }, use_read_total_timeout: true) + Gitlab::HTTP.try_get(build_url(path), verify: enable_ssl_verification, basic_auth: basic_auth, extra_log_info: { project_id: project_id }, use_read_total_timeout: true) end def post_to_build_queue(data, branch) @@ -165,6 +180,7 @@ module Integrations "<buildType id=#{build_type.encode(xml: :attr)}/>"\ '</build>', headers: { 'Content-type' => 'application/xml' }, + verify: enable_ssl_verification, basic_auth: basic_auth, use_read_total_timeout: true ) diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb index 52dd0aba43b..1b2c3e25283 100644 --- a/app/models/packages/package.rb +++ b/app/models/packages/package.rb @@ -25,7 +25,7 @@ class Packages::Package < ApplicationRecord terraform_module: 12 } - enum status: { default: 0, hidden: 1, processing: 2, error: 3 } + enum status: { default: 0, hidden: 1, processing: 2, error: 3, pending_destruction: 4 } belongs_to :project belongs_to :creator, class_name: 'User' @@ -304,6 +304,12 @@ class Packages::Package < ApplicationRecord build_infos.find_or_create_by!(pipeline: build.pipeline) end + def mark_package_files_for_destruction + return unless pending_destruction? + + ::Packages::MarkPackageFilesForDestructionWorker.perform_async(id) + end + private def composer_tag_version? diff --git a/app/models/packages/package_file.rb b/app/models/packages/package_file.rb index 072ff4a3a5f..190081c4e8e 100644 --- a/app/models/packages/package_file.rb +++ b/app/models/packages/package_file.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true class Packages::PackageFile < ApplicationRecord + include EachBatch include UpdateProjectStatistics include FileStoreMounter include Packages::Installable diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb index 7b13109dbc4..a3c9db90b5d 100644 --- a/app/models/system_note_metadata.rb +++ b/app/models/system_note_metadata.rb @@ -10,7 +10,7 @@ class SystemNoteMetadata < ApplicationRecord # in the same project (i.e. with the same permissions) TYPES_WITH_CROSS_REFERENCES = %w[ commit cross_reference - close duplicate + closed duplicate moved merge label milestone relate unrelate diff --git a/app/services/concerns/protected_ref_name_sanitizer.rb b/app/services/concerns/protected_ref_name_sanitizer.rb deleted file mode 100644 index 3966c410fec..00000000000 --- a/app/services/concerns/protected_ref_name_sanitizer.rb +++ /dev/null @@ -1,12 +0,0 @@ -# frozen_string_literal: true - -module ProtectedRefNameSanitizer - def sanitize_name(name) - name = CGI.unescapeHTML(name) - name = Sanitize.fragment(name) - - # Sanitize.fragment escapes HTML chars, so unescape again to allow names - # like `feature->master` - CGI.unescapeHTML(name) - end -end diff --git a/app/services/packages/mark_package_files_for_destruction_service.rb b/app/services/packages/mark_package_files_for_destruction_service.rb new file mode 100644 index 00000000000..3672b44b409 --- /dev/null +++ b/app/services/packages/mark_package_files_for_destruction_service.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Packages + # WARNING: ensure that permissions are verified before using this service. + class MarkPackageFilesForDestructionService + BATCH_SIZE = 500 + + def initialize(package_files) + @package_files = package_files + end + + def execute + @package_files.each_batch(of: BATCH_SIZE) do |batched_package_files| + batched_package_files.update_all(status: :pending_destruction) + end + + service_response_success('Package files are now pending destruction') + end + + private + + def service_response_success(message) + ServiceResponse.success(message: message) + end + end +end diff --git a/app/services/packages/destroy_package_service.rb b/app/services/packages/mark_package_for_destruction_service.rb index 697f1fa3ac8..3417febe79a 100644 --- a/app/services/packages/destroy_package_service.rb +++ b/app/services/packages/mark_package_for_destruction_service.rb @@ -1,19 +1,20 @@ # frozen_string_literal: true module Packages - class DestroyPackageService < BaseContainerService + class MarkPackageForDestructionService < BaseContainerService alias_method :package, :container def execute return service_response_error("You don't have access to this package", 403) unless user_can_delete_package? - package.destroy! + package.pending_destruction! + package.mark_package_files_for_destruction package.sync_maven_metadata(current_user) - service_response_success('Package was successfully deleted') + service_response_success('Package was successfully marked as pending destruction') rescue StandardError - service_response_error('Failed to remove the package', 400) + service_response_error('Failed to mark the package as pending destruction', 400) end private diff --git a/app/services/protected_branches/base_service.rb b/app/services/protected_branches/base_service.rb index 1ab3ccfcaae..f48e02ab4b5 100644 --- a/app/services/protected_branches/base_service.rb +++ b/app/services/protected_branches/base_service.rb @@ -2,8 +2,6 @@ module ProtectedBranches class BaseService < ::BaseService - include ProtectedRefNameSanitizer - # current_user - The user that performs the action # params - A hash of parameters def initialize(project, current_user = nil, params = {}) @@ -15,14 +13,5 @@ module ProtectedBranches def after_execute(*) # overridden in EE::ProtectedBranches module end - - private - - def filtered_params - return unless params - - params[:name] = sanitize_name(params[:name]) if params[:name].present? - params - end end end diff --git a/app/services/protected_branches/create_service.rb b/app/services/protected_branches/create_service.rb index ea494dd4426..dada449989a 100644 --- a/app/services/protected_branches/create_service.rb +++ b/app/services/protected_branches/create_service.rb @@ -21,7 +21,7 @@ module ProtectedBranches end def protected_branch - @protected_branch ||= project.protected_branches.new(filtered_params) + @protected_branch ||= project.protected_branches.new(params) end end end diff --git a/app/services/protected_branches/update_service.rb b/app/services/protected_branches/update_service.rb index 40e9a286af9..1e70f2d9793 100644 --- a/app/services/protected_branches/update_service.rb +++ b/app/services/protected_branches/update_service.rb @@ -8,7 +8,7 @@ module ProtectedBranches old_merge_access_levels = protected_branch.merge_access_levels.map(&:clone) old_push_access_levels = protected_branch.push_access_levels.map(&:clone) - if protected_branch.update(filtered_params) + if protected_branch.update(params) after_execute(protected_branch: protected_branch, old_merge_access_levels: old_merge_access_levels, old_push_access_levels: old_push_access_levels) end diff --git a/app/services/protected_tags/base_service.rb b/app/services/protected_tags/base_service.rb deleted file mode 100644 index e0181815f0f..00000000000 --- a/app/services/protected_tags/base_service.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -module ProtectedTags - class BaseService < ::BaseService - include ProtectedRefNameSanitizer - - private - - def filtered_params - return unless params - - params[:name] = sanitize_name(params[:name]) if params[:name].present? - params - end - end -end diff --git a/app/services/protected_tags/create_service.rb b/app/services/protected_tags/create_service.rb index 7d2b583a295..65303f21a4a 100644 --- a/app/services/protected_tags/create_service.rb +++ b/app/services/protected_tags/create_service.rb @@ -1,13 +1,13 @@ # frozen_string_literal: true module ProtectedTags - class CreateService < ProtectedTags::BaseService + class CreateService < ::BaseService attr_reader :protected_tag def execute raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project) - project.protected_tags.create(filtered_params) + project.protected_tags.create(params) end end end diff --git a/app/services/protected_tags/update_service.rb b/app/services/protected_tags/update_service.rb index e337ec39898..283aa8882c5 100644 --- a/app/services/protected_tags/update_service.rb +++ b/app/services/protected_tags/update_service.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true module ProtectedTags - class UpdateService < ProtectedTags::BaseService + class UpdateService < ::BaseService def execute(protected_tag) raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project) - protected_tag.update(filtered_params) + protected_tag.update(params) protected_tag end end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 14fa6599073..239b66bdeb0 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -1384,6 +1384,15 @@ :weight: 1 :idempotent: true :tags: [] +- :name: package_cleanup:packages_mark_package_files_for_destruction + :worker_name: Packages::MarkPackageFilesForDestructionWorker + :feature_category: :package_registry + :has_external_dependencies: + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: package_repositories:packages_debian_generate_distribution :worker_name: Packages::Debian::GenerateDistributionWorker :feature_category: :package_registry diff --git a/app/workers/concerns/packages/cleanup_artifact_worker.rb b/app/workers/concerns/packages/cleanup_artifact_worker.rb index db6c7330ea3..d4ad023b4a8 100644 --- a/app/workers/concerns/packages/cleanup_artifact_worker.rb +++ b/app/workers/concerns/packages/cleanup_artifact_worker.rb @@ -9,11 +9,15 @@ module Packages def perform_work return unless artifact - log_metadata(artifact) + artifact.transaction do + log_metadata(artifact) - artifact.destroy! - rescue StandardError - artifact&.error! + artifact.destroy! + rescue StandardError + artifact&.error! + end + + after_destroy end def remaining_work_count @@ -34,6 +38,10 @@ module Packages raise NotImplementedError end + def after_destroy + # no op + end + def artifact strong_memoize(:artifact) do model.transaction do diff --git a/app/workers/packages/cleanup_package_file_worker.rb b/app/workers/packages/cleanup_package_file_worker.rb index cb2b2a12c5e..f188017ee7a 100644 --- a/app/workers/packages/cleanup_package_file_worker.rb +++ b/app/workers/packages/cleanup_package_file_worker.rb @@ -19,6 +19,14 @@ module Packages private + def after_destroy + pkg = artifact.package + + pkg.transaction do + pkg.destroy if model.for_package_ids(pkg.id).empty? + end + end + def model Packages::PackageFile end diff --git a/app/workers/packages/mark_package_files_for_destruction_worker.rb b/app/workers/packages/mark_package_files_for_destruction_worker.rb new file mode 100644 index 00000000000..d8e52202841 --- /dev/null +++ b/app/workers/packages/mark_package_files_for_destruction_worker.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Packages + class MarkPackageFilesForDestructionWorker + include ApplicationWorker + + data_consistency :sticky + queue_namespace :package_cleanup + feature_category :package_registry + urgency :low + deduplicate :until_executing, including_scheduled: true + idempotent! + + sidekiq_options retry: 3 + + def perform(package_id) + package = ::Packages::Package.find_by_id(package_id) + + return unless package + + ::Packages::MarkPackageFilesForDestructionService.new(package.package_files) + .execute + end + end +end |