summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2022-02-03 11:35:56 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2022-02-03 11:35:56 +0000
commit33bbb6aa7b6369fea0037f3d8a9243824e48f64f (patch)
tree18ae1428e70ddcfe1115f355ebdad6ad6f0a6e56
parent41fd6d4d38aaef723e501ff3ab38ae63e31d4efb (diff)
downloadgitlab-ce-33bbb6aa7b6369fea0037f3d8a9243824e48f64f.tar.gz
Add latest changes from gitlab-org/security/gitlab@14-7-stable-ee
-rw-r--r--app/assets/javascripts/create_item_dropdown.js7
-rw-r--r--app/controllers/jira_connect/users_controller.rb10
-rw-r--r--app/graphql/mutations/packages/destroy.rb2
-rw-r--r--app/graphql/mutations/packages/destroy_file.rb2
-rw-r--r--app/graphql/types/project_type.rb6
-rw-r--r--app/models/application_setting.rb1
-rw-r--r--app/models/concerns/integrations/enable_ssl_verification.rb32
-rw-r--r--app/models/concerns/integrations/has_web_hook.rb6
-rw-r--r--app/models/integrations/bamboo.rb3
-rw-r--r--app/models/integrations/buildkite.rb2
-rw-r--r--app/models/integrations/drone_ci.rb26
-rw-r--r--app/models/integrations/jenkins.rb1
-rw-r--r--app/models/integrations/jira.rb10
-rw-r--r--app/models/integrations/mock_ci.rb23
-rw-r--r--app/models/integrations/teamcity.rb18
-rw-r--r--app/models/packages/package.rb8
-rw-r--r--app/models/packages/package_file.rb1
-rw-r--r--app/models/system_note_metadata.rb2
-rw-r--r--app/services/concerns/protected_ref_name_sanitizer.rb12
-rw-r--r--app/services/packages/mark_package_files_for_destruction_service.rb26
-rw-r--r--app/services/packages/mark_package_for_destruction_service.rb (renamed from app/services/packages/destroy_package_service.rb)9
-rw-r--r--app/services/protected_branches/base_service.rb11
-rw-r--r--app/services/protected_branches/create_service.rb2
-rw-r--r--app/services/protected_branches/update_service.rb2
-rw-r--r--app/services/protected_tags/base_service.rb16
-rw-r--r--app/services/protected_tags/create_service.rb4
-rw-r--r--app/services/protected_tags/update_service.rb4
-rw-r--r--app/workers/all_queues.yml9
-rw-r--r--app/workers/concerns/packages/cleanup_artifact_worker.rb16
-rw-r--r--app/workers/packages/cleanup_package_file_worker.rb8
-rw-r--r--app/workers/packages/mark_package_files_for_destruction_worker.rb25
-rw-r--r--db/migrate/20220113135449_add_package_files_limit_to_application_settings.rb7
-rw-r--r--db/migrate/20220113135924_add_application_settings_package_files_limit_constraints.rb15
-rw-r--r--db/schema_migrations/202201131354491
-rw-r--r--db/schema_migrations/202201131359241
-rw-r--r--db/structure.sql2
-rw-r--r--doc/api/graphql/reference/index.md1
-rw-r--r--doc/api/integrations.md6
-rw-r--r--lib/api/helpers/integrations_helpers.rb20
-rw-r--r--lib/api/package_files.rb4
-rw-r--r--lib/api/project_packages.rb2
-rw-r--r--lib/gitlab/url_blocker.rb8
-rw-r--r--locale/gitlab.pot9
-rw-r--r--spec/factories/packages/packages.rb4
-rw-r--r--spec/features/issues/notes_on_issues_spec.rb58
-rw-r--r--spec/features/protected_branches_spec.rb11
-rw-r--r--spec/frontend/create_item_dropdown_spec.js11
-rw-r--r--spec/graphql/types/packages/package_status_enum_spec.rb2
-rw-r--r--spec/graphql/types/project_type_spec.rb41
-rw-r--r--spec/lib/gitlab/url_blocker_spec.rb18
-rw-r--r--spec/models/concerns/integrations/enable_ssl_verification_spec.rb23
-rw-r--r--spec/models/integrations/bamboo_spec.rb2
-rw-r--r--spec/models/integrations/drone_ci_spec.rb48
-rw-r--r--spec/models/integrations/jenkins_spec.rb4
-rw-r--r--spec/models/integrations/jira_spec.rb17
-rw-r--r--spec/models/integrations/mock_ci_spec.rb73
-rw-r--r--spec/models/integrations/teamcity_spec.rb60
-rw-r--r--spec/models/packages/package_spec.rb24
-rw-r--r--spec/requests/api/graphql/mutations/packages/destroy_file_spec.rb15
-rw-r--r--spec/requests/api/graphql/mutations/packages/destroy_spec.rb21
-rw-r--r--spec/requests/api/graphql/project/cluster_agents_spec.rb2
-rw-r--r--spec/requests/api/package_files_spec.rb18
-rw-r--r--spec/requests/api/project_packages_spec.rb16
-rw-r--r--spec/requests/jira_connect/users_controller_spec.rb35
-rw-r--r--spec/services/packages/mark_package_files_for_destruction_service_spec.rb56
-rw-r--r--spec/services/packages/mark_package_for_destruction_service_spec.rb (renamed from spec/services/packages/destroy_package_service_spec.rb)20
-rw-r--r--spec/services/protected_branches/create_service_spec.rb36
-rw-r--r--spec/services/protected_branches/update_service_spec.rb33
-rw-r--r--spec/services/protected_tags/create_service_spec.rb36
-rw-r--r--spec/services/protected_tags/update_service_spec.rb33
-rw-r--r--spec/support/shared_contexts/models/concerns/integrations/enable_ssl_verification_shared_context.rb47
-rw-r--r--spec/support/shared_examples/models/integrations/has_web_hook_shared_examples.rb10
-rw-r--r--spec/workers/every_sidekiq_worker_spec.rb1
-rw-r--r--spec/workers/packages/cleanup_package_file_worker_spec.rb12
-rw-r--r--spec/workers/packages/mark_package_files_for_destruction_worker_spec.rb55
75 files changed, 950 insertions, 272 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
diff --git a/db/migrate/20220113135449_add_package_files_limit_to_application_settings.rb b/db/migrate/20220113135449_add_package_files_limit_to_application_settings.rb
new file mode 100644
index 00000000000..6d3deacdda3
--- /dev/null
+++ b/db/migrate/20220113135449_add_package_files_limit_to_application_settings.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddPackageFilesLimitToApplicationSettings < Gitlab::Database::Migration[1.0]
+ def change
+ add_column :application_settings, :max_package_files_for_package_destruction, :smallint, default: 100, null: false
+ end
+end
diff --git a/db/migrate/20220113135924_add_application_settings_package_files_limit_constraints.rb b/db/migrate/20220113135924_add_application_settings_package_files_limit_constraints.rb
new file mode 100644
index 00000000000..65fbccbd1ae
--- /dev/null
+++ b/db/migrate/20220113135924_add_application_settings_package_files_limit_constraints.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class AddApplicationSettingsPackageFilesLimitConstraints < Gitlab::Database::Migration[1.0]
+ CONSTRAINT_NAME = 'app_settings_max_package_files_for_package_destruction_positive'
+
+ disable_ddl_transaction!
+
+ def up
+ add_check_constraint :application_settings, 'max_package_files_for_package_destruction > 0', CONSTRAINT_NAME
+ end
+
+ def down
+ remove_check_constraint :application_settings, CONSTRAINT_NAME
+ end
+end
diff --git a/db/schema_migrations/20220113135449 b/db/schema_migrations/20220113135449
new file mode 100644
index 00000000000..57e6ede94b5
--- /dev/null
+++ b/db/schema_migrations/20220113135449
@@ -0,0 +1 @@
+46796379175ce9343907234d3ae14a417442c7c5ebbfcf6987db1d759eca2c3a \ No newline at end of file
diff --git a/db/schema_migrations/20220113135924 b/db/schema_migrations/20220113135924
new file mode 100644
index 00000000000..41328a43c37
--- /dev/null
+++ b/db/schema_migrations/20220113135924
@@ -0,0 +1 @@
+2a230758c13111c9e3738794008c31a3608dda2f0d071fbde0ad3cd782d29162 \ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index fff600c392a..47e111bf48f 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -10489,12 +10489,14 @@ CREATE TABLE application_settings (
container_registry_import_max_step_duration integer DEFAULT 300 NOT NULL,
container_registry_import_target_plan text DEFAULT 'free'::text NOT NULL,
container_registry_import_created_before timestamp with time zone DEFAULT '2022-01-23 00:00:00+00'::timestamp with time zone NOT NULL,
+ max_package_files_for_package_destruction smallint DEFAULT 100 NOT NULL,
runner_token_expiration_interval integer,
group_runner_token_expiration_interval integer,
project_runner_token_expiration_interval integer,
CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)),
CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)),
+ CONSTRAINT app_settings_max_package_files_for_package_destruction_positive CHECK ((max_package_files_for_package_destruction > 0)),
CONSTRAINT app_settings_p_cleanup_package_file_worker_capacity_positive CHECK ((packages_cleanup_package_file_worker_capacity >= 0)),
CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)),
CONSTRAINT app_settings_yaml_max_depth_positive CHECK ((max_yaml_depth > 0)),
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 3148fd1a6df..d7bf89c67ce 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -17301,6 +17301,7 @@ Values for sorting package.
| <a id="packagestatusdefault"></a>`DEFAULT` | Packages with a default status. |
| <a id="packagestatuserror"></a>`ERROR` | Packages with a error status. |
| <a id="packagestatushidden"></a>`HIDDEN` | Packages with a hidden status. |
+| <a id="packagestatuspending_destruction"></a>`PENDING_DESTRUCTION` | Packages with a pending_destruction status. |
| <a id="packagestatusprocessing"></a>`PROCESSING` | Packages with a processing status. |
### `PackageTypeEnum`
diff --git a/doc/api/integrations.md b/doc/api/integrations.md
index dd5a8c568fb..4719fe69c7b 100644
--- a/doc/api/integrations.md
+++ b/doc/api/integrations.md
@@ -165,6 +165,7 @@ Parameters:
| Parameter | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `bamboo_url` | string | true | Bamboo root URL. For example, `https://bamboo.example.com`. |
+| `enable_ssl_verification` | boolean | false | Enable SSL verification. Defaults to true (enabled). |
| `build_key` | string | true | Bamboo build plan key like KEY |
| `username` | string | true | A user with API access, if applicable |
| `password` | string | true | Password of the user |
@@ -521,7 +522,7 @@ Parameters:
| --------- | ---- | -------- | ----------- |
| `token` | string | true | Drone CI project specific token |
| `drone_url` | string | true | `http://drone.example.com` |
-| `enable_ssl_verification` | boolean | false | Enable SSL verification |
+| `enable_ssl_verification` | boolean | false | Enable SSL verification. Defaults to true (enabled). |
| `push_events` | boolean | false | Enable notifications for push events |
| `merge_requests_events` | boolean | false | Enable notifications for merge request events |
| `tag_push_events` | boolean | false | Enable notifications for tag push events |
@@ -1396,6 +1397,7 @@ Parameters:
| Parameter | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `teamcity_url` | string | true | TeamCity root URL. For example, `https://teamcity.example.com` |
+| `enable_ssl_verification` | boolean | false | Enable SSL verification. Defaults to true (enabled). |
| `build_type` | string | true | Build configuration ID |
| `username` | string | true | A user with permissions to trigger a manual build |
| `password` | string | true | The password of the user |
@@ -1434,6 +1436,7 @@ Parameters:
| Parameter | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `jenkins_url` | string | true | Jenkins URL like `http://jenkins.example.com`. |
+| `enable_ssl_verification` | boolean | false | Enable SSL verification. Defaults to true (enabled). |
| `project_name` | string | true | The URL-friendly project name. Example: `my_project_name`. |
| `username` | string | false | Username for authentication with the Jenkins server, if authentication is required by the server. |
| `password` | string | false | Password for authentication with the Jenkins server, if authentication is required by the server. |
@@ -1513,6 +1516,7 @@ Parameters:
| Parameter | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `mock_service_url` | string | true | `http://localhost:4004` |
+| `enable_ssl_verification` | boolean | false | Enable SSL verification. Defaults to true (enabled). |
### Disable MockCI integration
diff --git a/lib/api/helpers/integrations_helpers.rb b/lib/api/helpers/integrations_helpers.rb
index 3af0dd4c532..72b16a23dd6 100644
--- a/lib/api/helpers/integrations_helpers.rb
+++ b/lib/api/helpers/integrations_helpers.rb
@@ -197,6 +197,12 @@ module API
desc: 'Bamboo root URL like https://bamboo.example.com'
},
{
+ required: false,
+ name: :enable_ssl_verification,
+ type: Boolean,
+ desc: 'Enable SSL verification'
+ },
+ {
required: true,
name: :build_key,
type: String,
@@ -368,7 +374,7 @@ module API
required: false,
name: :enable_ssl_verification,
type: Boolean,
- desc: 'Enable SSL verification for communication'
+ desc: 'Enable SSL verification'
}
],
'emails-on-push' => [
@@ -468,6 +474,12 @@ module API
desc: 'Jenkins root URL like https://jenkins.example.com'
},
{
+ required: false,
+ name: :enable_ssl_verification,
+ type: Boolean,
+ desc: 'Enable SSL verification'
+ },
+ {
required: true,
name: :project_name,
type: String,
@@ -749,6 +761,12 @@ module API
desc: 'TeamCity root URL like https://teamcity.example.com'
},
{
+ required: false,
+ name: :enable_ssl_verification,
+ type: Boolean,
+ desc: 'Enable SSL verification'
+ },
+ {
required: true,
name: :build_type,
type: String,
diff --git a/lib/api/package_files.rb b/lib/api/package_files.rb
index 5e421da2c55..3bf47fe1e8b 100644
--- a/lib/api/package_files.rb
+++ b/lib/api/package_files.rb
@@ -65,7 +65,9 @@ module API
not_found! unless package_file
- destroy_conditionally!(package_file)
+ destroy_conditionally!(package_file) do |package_file|
+ package_file.pending_destruction!
+ end
end
end
end
diff --git a/lib/api/project_packages.rb b/lib/api/project_packages.rb
index 54c0a0628a7..c997afea865 100644
--- a/lib/api/project_packages.rb
+++ b/lib/api/project_packages.rb
@@ -71,7 +71,7 @@ module API
.new(user_project, params[:package_id]).execute
destroy_conditionally!(package) do |package|
- ::Packages::DestroyPackageService.new(container: package, current_user: current_user).execute
+ ::Packages::MarkPackageForDestructionService.new(container: package, current_user: current_user).execute
end
end
end
diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb
index f092e03046a..48228ede684 100644
--- a/lib/gitlab/url_blocker.rb
+++ b/lib/gitlab/url_blocker.rb
@@ -148,9 +148,17 @@ module Gitlab
unless allow_local_network
validate_local_network(address_info)
validate_link_local(address_info)
+ validate_shared_address(address_info)
end
end
+ def validate_shared_address(addrs_info)
+ netmask = IPAddr.new('100.64.0.0/10')
+ return unless addrs_info.any? { |addr| netmask.include?(addr.ip_address) }
+
+ raise BlockedUrlError, "Requests to the shared address space are not allowed"
+ end
+
def get_port(uri)
uri.port || uri.default_port
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 8f5be8ace27..150f6214e95 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -19142,6 +19142,9 @@ msgstr ""
msgid "Integrations|Browser limitations"
msgstr ""
+msgid "Integrations|Clear if using a self-signed certificate."
+msgstr ""
+
msgid "Integrations|Comment detail:"
msgstr ""
@@ -19172,6 +19175,9 @@ msgstr ""
msgid "Integrations|Enable GitLab.com slash commands in a Slack workspace."
msgstr ""
+msgid "Integrations|Enable SSL verification"
+msgstr ""
+
msgid "Integrations|Enable comments"
msgstr ""
@@ -19250,6 +19256,9 @@ msgstr ""
msgid "Integrations|Return to GitLab for Jira"
msgstr ""
+msgid "Integrations|SSL verification"
+msgstr ""
+
msgid "Integrations|Save settings?"
msgstr ""
diff --git a/spec/factories/packages/packages.rb b/spec/factories/packages/packages.rb
index 153518f4cd3..90c68074a3b 100644
--- a/spec/factories/packages/packages.rb
+++ b/spec/factories/packages/packages.rb
@@ -20,6 +20,10 @@ FactoryBot.define do
status { :error }
end
+ trait :pending_destruction do
+ status { :pending_destruction }
+ end
+
factory :maven_package do
maven_metadatum
diff --git a/spec/features/issues/notes_on_issues_spec.rb b/spec/features/issues/notes_on_issues_spec.rb
index be85d73d777..4e98062e8b2 100644
--- a/spec/features/issues/notes_on_issues_spec.rb
+++ b/spec/features/issues/notes_on_issues_spec.rb
@@ -91,4 +91,62 @@ RSpec.describe 'Create notes on issues', :js do
expect(page).to have_selector '.gfm-project_member.current-user', text: user.username
end
+
+ shared_examples "when reference belongs to a private project" do
+ let(:project) { create(:project, :private, :repository) }
+ let(:issue) { create(:issue, project: project) }
+
+ before do
+ sign_in(user)
+ end
+
+ context 'when the user does not have permission to see the reference' do
+ before do
+ project.add_guest(user)
+ end
+
+ it 'does not show the user the reference' do
+ visit project_issue_path(project, issue)
+
+ expect(page).not_to have_content('closed via')
+ end
+ end
+
+ context 'when the user has permission to see the reference' do
+ before do
+ project.add_developer(user)
+ end
+
+ it 'shows the user the reference' do
+ visit project_issue_path(project, issue)
+
+ page.within('div#notes li.note .system-note-message') do
+ expect(page).to have_content('closed via')
+ expect(page.find('a')).to have_content(reference_content)
+ end
+ end
+ end
+ end
+
+ context 'when the issue is closed via a merge request' do
+ it_behaves_like "when reference belongs to a private project" do
+ let(:reference) { create(:merge_request, source_project: project) }
+ let(:reference_content) { reference.to_reference }
+
+ before do
+ create(:resource_state_event, issue: issue, state: :closed, created_at: '2020-02-05', source_merge_request: reference)
+ end
+ end
+ end
+
+ context 'when the issue is closed via a commit' do
+ it_behaves_like "when reference belongs to a private project" do
+ let(:reference) { create(:commit, project: project) }
+ let(:reference_content) { reference.short_sha }
+
+ before do
+ create(:resource_state_event, issue: issue, state: :closed, created_at: '2020-02-05', source_commit: reference.id)
+ end
+ end
+ end
end
diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb
index 4278efc5a8f..389a51a10e0 100644
--- a/spec/features/protected_branches_spec.rb
+++ b/spec/features/protected_branches_spec.rb
@@ -38,6 +38,17 @@ RSpec.describe 'Protected Branches', :js do
sign_in(user)
end
+ it 'allows to create a protected branch with name containing HTML tags' do
+ visit project_protected_branches_path(project)
+ set_defaults
+ set_protected_branch_name('foo<b>bar<\b>')
+ click_on "Protect"
+
+ within(".protected-branches-list") { expect(page).to have_content('foo<b>bar<\b>') }
+ expect(ProtectedBranch.count).to eq(1)
+ expect(ProtectedBranch.last.name).to eq('foo<b>bar<\b>')
+ end
+
describe 'Delete protected branch' do
before do
create(:protected_branch, project: project, name: 'fix')
diff --git a/spec/frontend/create_item_dropdown_spec.js b/spec/frontend/create_item_dropdown_spec.js
index 56c09cd731e..143ccb9b930 100644
--- a/spec/frontend/create_item_dropdown_spec.js
+++ b/spec/frontend/create_item_dropdown_spec.js
@@ -17,6 +17,11 @@ const DROPDOWN_ITEM_DATA = [
id: 'three',
text: 'three',
},
+ {
+ title: '<b>four</b>title',
+ id: '<b>four</b>id',
+ text: '<b>four</b>text',
+ },
];
describe('CreateItemDropdown', () => {
@@ -63,6 +68,10 @@ describe('CreateItemDropdown', () => {
const $itemEls = $wrapperEl.find('.js-dropdown-content a');
expect($itemEls.length).toEqual(DROPDOWN_ITEM_DATA.length);
+
+ DROPDOWN_ITEM_DATA.forEach((dataItem, i) => {
+ expect($($itemEls[i]).text()).toEqual(dataItem.text);
+ });
});
});
@@ -177,7 +186,7 @@ describe('CreateItemDropdown', () => {
const $itemEls = $wrapperEl.find('.js-dropdown-content a');
expect($itemEls.length).toEqual(1 + DROPDOWN_ITEM_DATA.length);
- expect($($itemEls[3]).text()).toEqual('new-item-text');
+ expect($($itemEls[DROPDOWN_ITEM_DATA.length]).text()).toEqual('new-item-text');
expect($wrapperEl.find('.dropdown-toggle-text').text()).toEqual('new-item-title');
});
});
diff --git a/spec/graphql/types/packages/package_status_enum_spec.rb b/spec/graphql/types/packages/package_status_enum_spec.rb
index 71d05da35ea..2c79c92498b 100644
--- a/spec/graphql/types/packages/package_status_enum_spec.rb
+++ b/spec/graphql/types/packages/package_status_enum_spec.rb
@@ -4,6 +4,6 @@ require 'spec_helper'
RSpec.describe GitlabSchema.types['PackageStatus'] do
it 'exposes all package statuses' do
- expect(described_class.values.keys).to contain_exactly(*%w[DEFAULT HIDDEN PROCESSING ERROR])
+ expect(described_class.values.keys).to contain_exactly(*%w[DEFAULT HIDDEN PROCESSING ERROR PENDING_DESTRUCTION])
end
end
diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb
index cd216232569..961e12288d4 100644
--- a/spec/graphql/types/project_type_spec.rb
+++ b/spec/graphql/types/project_type_spec.rb
@@ -595,4 +595,45 @@ RSpec.describe GitlabSchema.types['Project'] do
expect(cluster_agent.agent_tokens.size).to be(count)
end
end
+
+ describe 'service_desk_address' do
+ let(:user) { create(:user) }
+ let(:query) do
+ %(
+ query {
+ project(fullPath: "#{project.full_path}") {
+ id
+ serviceDeskAddress
+ }
+ }
+ )
+ end
+
+ subject { GitlabSchema.execute(query, context: { current_user: user }).as_json }
+
+ before do
+ allow(::Gitlab::ServiceDeskEmail).to receive(:enabled?) { true }
+ allow(::Gitlab::ServiceDeskEmail).to receive(:address_for_key) { 'address-suffix@example.com' }
+ end
+
+ context 'when a user can admin issues' do
+ let(:project) { create(:project, :public, :service_desk_enabled) }
+
+ before do
+ project.add_reporter(user)
+ end
+
+ it 'is present' do
+ expect(subject.dig('data', 'project', 'serviceDeskAddress')).to be_present
+ end
+ end
+
+ context 'when a user can not admin issues' do
+ let(:project) { create(:project, :public, :service_desk_disabled) }
+
+ it 'is empty' do
+ expect(subject.dig('data', 'project', 'serviceDeskAddress')).to be_blank
+ end
+ end
+ end
end
diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb
index 0713475d59b..5b77290ce2e 100644
--- a/spec/lib/gitlab/url_blocker_spec.rb
+++ b/spec/lib/gitlab/url_blocker_spec.rb
@@ -279,6 +279,8 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do
end
context 'when allow_local_network is' do
+ let(:shared_address_space_ips) { ['100.64.0.0', '100.64.127.127', '100.64.255.255'] }
+
let(:local_ips) do
[
'192.168.1.2',
@@ -292,7 +294,8 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do
'[::ffff:ac10:20]',
'[feef::1]',
'[fee2::]',
- '[fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa]'
+ '[fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa]',
+ *shared_address_space_ips
]
end
@@ -385,18 +388,7 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do
'127.0.0.1',
'127.0.0.2',
'192.168.1.1',
- '192.168.1.2',
- '0:0:0:0:0:ffff:192.168.1.2',
- '::ffff:c0a8:102',
- '10.0.0.2',
- '0:0:0:0:0:ffff:10.0.0.2',
- '::ffff:a00:2',
- '172.16.0.2',
- '0:0:0:0:0:ffff:172.16.0.2',
- '::ffff:ac10:20',
- 'feef::1',
- 'fee2::',
- 'fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa',
+ *local_ips,
'0:0:0:0:0:ffff:169.254.169.254',
'::ffff:a9fe:a9fe',
'::ffff:169.254.168.100',
diff --git a/spec/models/concerns/integrations/enable_ssl_verification_spec.rb b/spec/models/concerns/integrations/enable_ssl_verification_spec.rb
new file mode 100644
index 00000000000..802e950c0c2
--- /dev/null
+++ b/spec/models/concerns/integrations/enable_ssl_verification_spec.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Integrations::EnableSslVerification do
+ let(:described_class) do
+ Class.new(Integration) do
+ prepend Integrations::EnableSslVerification
+
+ def fields
+ [
+ { name: 'main_url' },
+ { name: 'other_url' },
+ { name: 'username' }
+ ]
+ end
+ end
+ end
+
+ let(:integration) { described_class.new }
+
+ include_context Integrations::EnableSslVerification
+end
diff --git a/spec/models/integrations/bamboo_spec.rb b/spec/models/integrations/bamboo_spec.rb
index 60ff6685c3d..b5684d153f2 100644
--- a/spec/models/integrations/bamboo_spec.rb
+++ b/spec/models/integrations/bamboo_spec.rb
@@ -23,6 +23,8 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching do
)
end
+ include_context Integrations::EnableSslVerification
+
describe 'Validations' do
context 'when active' do
before do
diff --git a/spec/models/integrations/drone_ci_spec.rb b/spec/models/integrations/drone_ci_spec.rb
index 062e23d628e..dd64dcfc52c 100644
--- a/spec/models/integrations/drone_ci_spec.rb
+++ b/spec/models/integrations/drone_ci_spec.rb
@@ -5,6 +5,8 @@ require 'spec_helper'
RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do
include ReactiveCachingHelpers
+ subject(:integration) { described_class.new }
+
describe 'validations' do
context 'active' do
before do
@@ -59,6 +61,52 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do
end
end
+ include_context Integrations::EnableSslVerification do
+ describe '#enable_ssl_verification' do
+ before do
+ allow(integration).to receive(:new_record?).and_return(false)
+ end
+
+ it 'returns true for a known hostname' do
+ integration.drone_url = 'https://cloud.drone.io'
+
+ expect(integration.enable_ssl_verification).to be(true)
+ end
+
+ it 'returns true for new records' do
+ allow(integration).to receive(:new_record?).and_return(true)
+ integration.drone_url = 'http://example.com'
+
+ expect(integration.enable_ssl_verification).to be(true)
+ end
+
+ it 'returns false for an unknown hostname' do
+ integration.drone_url = 'https://example.com'
+
+ expect(integration.enable_ssl_verification).to be(false)
+ end
+
+ it 'returns false for a HTTP URL' do
+ integration.drone_url = 'http://cloud.drone.io'
+
+ expect(integration.enable_ssl_verification).to be(false)
+ end
+
+ it 'returns false for an invalid URL' do
+ integration.drone_url = 'https://example.com:foo'
+
+ expect(integration.enable_ssl_verification).to be(false)
+ end
+
+ it 'returns the persisted value if present' do
+ integration.drone_url = 'https://cloud.drone.io'
+ integration.enable_ssl_verification = false
+
+ expect(integration.enable_ssl_verification).to be(false)
+ end
+ end
+ end
+
it_behaves_like Integrations::HasWebHook do
include_context :drone_ci_integration
diff --git a/spec/models/integrations/jenkins_spec.rb b/spec/models/integrations/jenkins_spec.rb
index 9286d026290..3d6393f2793 100644
--- a/spec/models/integrations/jenkins_spec.rb
+++ b/spec/models/integrations/jenkins_spec.rb
@@ -24,6 +24,10 @@ RSpec.describe Integrations::Jenkins do
let(:jenkins_authorization) { "Basic " + ::Base64.strict_encode64(jenkins_username + ':' + jenkins_password) }
+ include_context Integrations::EnableSslVerification do
+ let(:integration) { described_class.new(jenkins_params) }
+ end
+
it_behaves_like Integrations::HasWebHook do
let(:integration) { described_class.new(jenkins_params) }
let(:hook_url) { "http://#{ERB::Util.url_encode jenkins_username}:#{ERB::Util.url_encode jenkins_password}@jenkins.example.com/project/my_project" }
diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb
index e80fa6e3b70..6ce84c28044 100644
--- a/spec/models/integrations/jira_spec.rb
+++ b/spec/models/integrations/jira_spec.rb
@@ -130,6 +130,23 @@ RSpec.describe Integrations::Jira do
end
end
+ describe '.valid_jira_cloud_url?' do
+ using RSpec::Parameterized::TableSyntax
+
+ where(:url, :result) do
+ 'https://abc.atlassian.net' | true
+ 'abc.atlassian.net' | false # This is how it behaves currently, but we may need to consider adding scheme if missing
+ 'https://somethingelse.com' | false
+ nil | false
+ end
+
+ with_them do
+ specify do
+ expect(described_class.valid_jira_cloud_url?(url)).to eq(result)
+ end
+ end
+ end
+
describe '#create' do
let(:params) do
{
diff --git a/spec/models/integrations/mock_ci_spec.rb b/spec/models/integrations/mock_ci_spec.rb
new file mode 100644
index 00000000000..d29c63b3a97
--- /dev/null
+++ b/spec/models/integrations/mock_ci_spec.rb
@@ -0,0 +1,73 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Integrations::MockCi do
+ let_it_be(:project) { build(:project) }
+
+ subject(:integration) { described_class.new(project: project, mock_service_url: generate(:url)) }
+
+ include_context Integrations::EnableSslVerification
+
+ describe '#commit_status' do
+ let(:sha) { generate(:sha) }
+
+ def stub_request(*args)
+ WebMock.stub_request(:get, integration.commit_status_path(sha)).to_return(*args)
+ end
+
+ def commit_status
+ integration.commit_status(sha, 'master')
+ end
+
+ it 'returns allowed states' do
+ described_class::ALLOWED_STATES.each do |state|
+ stub_request(status: 200, body: { status: state }.to_json)
+
+ expect(commit_status).to eq(state)
+ end
+ end
+
+ it 'returns :pending for 404 responses' do
+ stub_request(status: 404)
+
+ expect(commit_status).to eq(:pending)
+ end
+
+ it 'returns :error for responses other than 200 or 404' do
+ stub_request(status: 500)
+
+ expect(commit_status).to eq(:error)
+ end
+
+ it 'returns :error for unknown states' do
+ stub_request(status: 200, body: { status: 'unknown' }.to_json)
+
+ expect(commit_status).to eq(:error)
+ end
+
+ it 'returns :error for invalid JSON' do
+ stub_request(status: 200, body: '')
+
+ expect(commit_status).to eq(:error)
+ end
+
+ it 'returns :error for non-hash JSON responses' do
+ stub_request(status: 200, body: 23.to_json)
+
+ expect(commit_status).to eq(:error)
+ end
+
+ it 'returns :error for JSON responses without a status' do
+ stub_request(status: 200, body: { foo: :bar }.to_json)
+
+ expect(commit_status).to eq(:error)
+ end
+
+ it 'returns :error when connection is refused' do
+ stub_request(status: 500).to_raise(Errno::ECONNREFUSED)
+
+ expect(commit_status).to eq(:error)
+ end
+ end
+end
diff --git a/spec/models/integrations/teamcity_spec.rb b/spec/models/integrations/teamcity_spec.rb
index 0713141ea08..e1f4e577503 100644
--- a/spec/models/integrations/teamcity_spec.rb
+++ b/spec/models/integrations/teamcity_spec.rb
@@ -6,8 +6,8 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do
include ReactiveCachingHelpers
include StubRequests
- let(:teamcity_url) { 'http://gitlab.com/teamcity' }
- let(:teamcity_full_url) { 'http://gitlab.com/teamcity/httpAuth/app/rest/builds/branch:unspecified:any,revision:123' }
+ let(:teamcity_url) { 'https://gitlab.teamcity.com' }
+ let(:teamcity_full_url) { 'https://gitlab.teamcity.com/httpAuth/app/rest/builds/branch:unspecified:any,revision:123' }
let(:project) { create(:project) }
subject(:integration) do
@@ -22,6 +22,52 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do
)
end
+ include_context Integrations::EnableSslVerification do
+ describe '#enable_ssl_verification' do
+ before do
+ allow(integration).to receive(:new_record?).and_return(false)
+ end
+
+ it 'returns true for a known hostname' do
+ integration.teamcity_url = 'https://example.teamcity.com'
+
+ expect(integration.enable_ssl_verification).to be(true)
+ end
+
+ it 'returns true for new records' do
+ allow(integration).to receive(:new_record?).and_return(true)
+ integration.teamcity_url = 'http://example.com'
+
+ expect(integration.enable_ssl_verification).to be(true)
+ end
+
+ it 'returns false for an unknown hostname' do
+ integration.teamcity_url = 'https://sub.example.teamcity.com'
+
+ expect(integration.enable_ssl_verification).to be(false)
+ end
+
+ it 'returns false for a HTTP URL' do
+ integration.teamcity_url = 'http://example.teamcity.com'
+
+ expect(integration.enable_ssl_verification).to be(false)
+ end
+
+ it 'returns false for an invalid URL' do
+ integration.teamcity_url = 'https://example.com:foo'
+
+ expect(integration.enable_ssl_verification).to be(false)
+ end
+
+ it 'returns the persisted value if present' do
+ integration.teamcity_url = 'https://example.teamcity.com'
+ integration.enable_ssl_verification = false
+
+ expect(integration.enable_ssl_verification).to be(false)
+ end
+ end
+ end
+
describe 'Validations' do
context 'when integration is active' do
before do
@@ -140,22 +186,22 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do
it 'returns a specific URL when status is 500' do
stub_request(status: 500)
- is_expected.to eq('http://gitlab.com/teamcity/viewLog.html?buildTypeId=foo')
+ is_expected.to eq("#{teamcity_url}/viewLog.html?buildTypeId=foo")
end
it 'returns a build URL when teamcity_url has no trailing slash' do
stub_request(body: %q({"build":{"id":"666"}}))
- is_expected.to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
+ is_expected.to eq("#{teamcity_url}/viewLog.html?buildId=666&buildTypeId=foo")
end
context 'teamcity_url has trailing slash' do
- let(:teamcity_url) { 'http://gitlab.com/teamcity/' }
+ let(:teamcity_url) { 'https://gitlab.teamcity.com/' }
it 'returns a build URL' do
stub_request(body: %q({"build":{"id":"666"}}))
- is_expected.to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
+ is_expected.to eq('https://gitlab.teamcity.com/viewLog.html?buildId=666&buildTypeId=foo')
end
end
@@ -299,7 +345,7 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do
end
def stub_post_to_build_queue(branch:)
- teamcity_full_url = 'http://gitlab.com/teamcity/httpAuth/app/rest/buildQueue'
+ teamcity_full_url = "#{teamcity_url}/httpAuth/app/rest/buildQueue"
body ||= %Q(<build branchName=\"#{branch}\"><buildType id=\"foo\"/></build>)
auth = %w(mic password)
diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb
index 122340f7bec..0cb92f81da2 100644
--- a/spec/models/packages/package_spec.rb
+++ b/spec/models/packages/package_spec.rb
@@ -1201,6 +1201,30 @@ RSpec.describe Packages::Package, type: :model do
end
end
+ describe '#mark_package_files_for_destruction' do
+ let_it_be(:package) { create(:npm_package, :pending_destruction) }
+
+ subject { package.mark_package_files_for_destruction }
+
+ it 'enqueues a sync worker job' do
+ expect(::Packages::MarkPackageFilesForDestructionWorker)
+ .to receive(:perform_async).with(package.id)
+
+ subject
+ end
+
+ context 'for a package non pending destruction' do
+ let_it_be(:package) { create(:npm_package) }
+
+ it 'does not enqueues a sync worker job' do
+ expect(::Packages::MarkPackageFilesForDestructionWorker)
+ .not_to receive(:perform_async).with(package.id)
+
+ subject
+ end
+ end
+ end
+
describe '#create_build_infos!' do
let_it_be(:package) { create(:package) }
let_it_be(:pipeline) { create(:ci_pipeline) }
diff --git a/spec/requests/api/graphql/mutations/packages/destroy_file_spec.rb b/spec/requests/api/graphql/mutations/packages/destroy_file_spec.rb
index 7be629f8f4b..cd25aba9e00 100644
--- a/spec/requests/api/graphql/mutations/packages/destroy_file_spec.rb
+++ b/spec/requests/api/graphql/mutations/packages/destroy_file_spec.rb
@@ -24,19 +24,16 @@ RSpec.describe 'Destroying a package file' do
let(:mutation_response) { graphql_mutation_response(:destroyPackageFile) }
shared_examples 'destroying the package file' do
- it 'destroy the package file' do
- expect { mutation_request }.to change { ::Packages::PackageFile.count }.by(-1)
+ it 'marks the package file as pending destruction' do
+ expect { mutation_request }.to change { ::Packages::PackageFile.pending_destruction.count }.by(1)
end
it_behaves_like 'returning response status', :success
end
shared_examples 'denying the mutation request' do
- it 'does not destroy the package file' do
- expect(::Packages::PackageFile)
- .not_to receive(:destroy)
-
- expect { mutation_request }.not_to change { ::Packages::PackageFile.count }
+ it 'does not mark the package file as pending destruction' do
+ expect { mutation_request }.not_to change { ::Packages::PackageFile.pending_destruction.count }
expect(mutation_response).to be_nil
end
@@ -71,7 +68,7 @@ RSpec.describe 'Destroying a package file' do
it_behaves_like 'denying the mutation request'
end
- context 'when an error occures' do
+ context 'when an error occurs' do
let(:error_messages) { ['some error'] }
before do
@@ -80,7 +77,7 @@ RSpec.describe 'Destroying a package file' do
it 'returns the errors in the response' do
allow_next_found_instance_of(::Packages::PackageFile) do |package_file|
- allow(package_file).to receive(:destroy).and_return(false)
+ allow(package_file).to receive(:update).with(status: :pending_destruction).and_return(false)
allow(package_file).to receive_message_chain(:errors, :full_messages).and_return(error_messages)
end
diff --git a/spec/requests/api/graphql/mutations/packages/destroy_spec.rb b/spec/requests/api/graphql/mutations/packages/destroy_spec.rb
index e5ced419ecf..2340a6a36d8 100644
--- a/spec/requests/api/graphql/mutations/packages/destroy_spec.rb
+++ b/spec/requests/api/graphql/mutations/packages/destroy_spec.rb
@@ -24,22 +24,27 @@ RSpec.describe 'Destroying a package' do
let(:mutation_response) { graphql_mutation_response(:destroyPackage) }
shared_examples 'destroying the package' do
- it 'destroy the package' do
- expect(::Packages::DestroyPackageService)
+ it 'marks the package as pending destruction' do
+ expect(::Packages::MarkPackageForDestructionService)
.to receive(:new).with(container: package, current_user: user).and_call_original
+ expect_next_found_instance_of(::Packages::Package) do |package|
+ expect(package).to receive(:mark_package_files_for_destruction)
+ end
- expect { mutation_request }.to change { ::Packages::Package.count }.by(-1)
+ expect { mutation_request }
+ .to change { ::Packages::Package.pending_destruction.count }.by(1)
end
it_behaves_like 'returning response status', :success
end
shared_examples 'denying the mutation request' do
- it 'does not destroy the package' do
- expect(::Packages::DestroyPackageService)
+ it 'does not mark the package as pending destruction' do
+ expect(::Packages::MarkPackageForDestructionService)
.not_to receive(:new).with(container: package, current_user: user)
- expect { mutation_request }.not_to change { ::Packages::Package.count }
+ expect { mutation_request }
+ .to not_change { ::Packages::Package.pending_destruction.count }
expect(mutation_response).to be_nil
end
@@ -81,12 +86,12 @@ RSpec.describe 'Destroying a package' do
it 'returns the errors in the response' do
allow_next_found_instance_of(::Packages::Package) do |package|
- allow(package).to receive(:destroy!).and_raise(StandardError)
+ allow(package).to receive(:pending_destruction!).and_raise(StandardError)
end
mutation_request
- expect(mutation_response['errors']).to eq(['Failed to remove the package'])
+ expect(mutation_response['errors']).to match_array(['Failed to mark the package as pending destruction'])
end
end
end
diff --git a/spec/requests/api/graphql/project/cluster_agents_spec.rb b/spec/requests/api/graphql/project/cluster_agents_spec.rb
index 585126f3849..c9900fea277 100644
--- a/spec/requests/api/graphql/project/cluster_agents_spec.rb
+++ b/spec/requests/api/graphql/project/cluster_agents_spec.rb
@@ -126,7 +126,7 @@ RSpec.describe 'Project.cluster_agents' do
})
end
- it 'preloads associations to prevent N+1 queries' do
+ it 'preloads associations to prevent N+1 queries', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/350868' do
user = create(:user)
token = create(:cluster_agent_token, agent: agents.second)
create(:agent_activity_event, agent: agents.second, agent_token: token, user: user)
diff --git a/spec/requests/api/package_files_spec.rb b/spec/requests/api/package_files_spec.rb
index 7a6b1599154..a7e6a97fd0e 100644
--- a/spec/requests/api/package_files_spec.rb
+++ b/spec/requests/api/package_files_spec.rb
@@ -114,14 +114,14 @@ RSpec.describe API::PackageFiles do
let(:user) { nil }
it 'returns 403 for non authenticated user', :aggregate_failures do
- expect { api_request }.not_to change { package.package_files.count }
+ expect { api_request }.not_to change { package.package_files.pending_destruction.count }
expect(response).to have_gitlab_http_status(:forbidden)
end
end
it 'returns 403 for a user without access to the project', :aggregate_failures do
- expect { api_request }.not_to change { package.package_files.count }
+ expect { api_request }.not_to change { package.package_files.pending_destruction.count }
expect(response).to have_gitlab_http_status(:forbidden)
end
@@ -131,7 +131,7 @@ RSpec.describe API::PackageFiles do
let_it_be_with_refind(:project) { create(:project, :private) }
it 'returns 404 for a user without access to the project', :aggregate_failures do
- expect { api_request }.not_to change { package.package_files.count }
+ expect { api_request }.not_to change { package.package_files.pending_destruction.count }
expect(response).to have_gitlab_http_status(:not_found)
end
@@ -139,7 +139,7 @@ RSpec.describe API::PackageFiles do
it 'returns 403 for a user without enough permissions', :aggregate_failures do
project.add_developer(user)
- expect { api_request }.not_to change { package.package_files.count }
+ expect { api_request }.not_to change { package.package_files.pending_destruction.count }
expect(response).to have_gitlab_http_status(:forbidden)
end
@@ -147,7 +147,7 @@ RSpec.describe API::PackageFiles do
it 'returns 204', :aggregate_failures do
project.add_maintainer(user)
- expect { api_request }.to change { package.package_files.count }.by(-1)
+ expect { api_request }.to change { package.package_files.pending_destruction.count }.by(1)
expect(response).to have_gitlab_http_status(:no_content)
end
@@ -156,7 +156,7 @@ RSpec.describe API::PackageFiles do
let(:user) { nil }
it 'returns 404 for non authenticated user', :aggregate_failures do
- expect { api_request }.not_to change { package.package_files.count }
+ expect { api_request }.not_to change { package.package_files.pending_destruction.count }
expect(response).to have_gitlab_http_status(:not_found)
end
@@ -168,7 +168,7 @@ RSpec.describe API::PackageFiles do
it 'returns 404 when the package file does not exist', :aggregate_failures do
project.add_maintainer(user)
- expect { api_request }.not_to change { package.package_files.count }
+ expect { api_request }.not_to change { package.package_files.pending_destruction.count }
expect(response).to have_gitlab_http_status(:not_found)
end
@@ -182,7 +182,7 @@ RSpec.describe API::PackageFiles do
end
it 'can not be accessed', :aggregate_failures do
- expect { api_request }.not_to change { package.package_files.count }
+ expect { api_request }.not_to change { package.package_files.pending_destruction.count }
expect(response).to have_gitlab_http_status(:not_found)
end
@@ -193,7 +193,7 @@ RSpec.describe API::PackageFiles do
end
it 'can be accessed', :aggregate_failures do
- expect { api_request }.to change { package.package_files.count }.by(-1)
+ expect { api_request }.not_to change { package.package_files.pending_destruction.count }
expect(response).to have_gitlab_http_status(:no_content)
end
diff --git a/spec/requests/api/project_packages_spec.rb b/spec/requests/api/project_packages_spec.rb
index 9b7538547f6..5f4b8899a33 100644
--- a/spec/requests/api/project_packages_spec.rb
+++ b/spec/requests/api/project_packages_spec.rb
@@ -293,13 +293,13 @@ RSpec.describe API::ProjectPackages do
context 'without the need for a license' do
context 'project is public' do
it 'returns 403 for non authenticated user' do
- delete api(package_url)
+ expect { delete api(package_url) }.not_to change { ::Packages::Package.pending_destruction.count }
expect(response).to have_gitlab_http_status(:forbidden)
end
it 'returns 403 for a user without access to the project' do
- delete api(package_url, user)
+ expect { delete api(package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count }
expect(response).to have_gitlab_http_status(:forbidden)
end
@@ -313,13 +313,13 @@ RSpec.describe API::ProjectPackages do
end
it 'returns 404 for non authenticated user' do
- delete api(package_url)
+ expect { delete api(package_url) }.not_to change { ::Packages::Package.pending_destruction.count }
expect(response).to have_gitlab_http_status(:not_found)
end
it 'returns 404 for a user without access to the project' do
- delete api(package_url, user)
+ expect { delete api(package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count }
expect(response).to have_gitlab_http_status(:not_found)
end
@@ -327,7 +327,7 @@ RSpec.describe API::ProjectPackages do
it 'returns 404 when the package does not exist' do
project.add_maintainer(user)
- delete api(no_package_url, user)
+ expect { delete api(no_package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count }
expect(response).to have_gitlab_http_status(:not_found)
end
@@ -335,7 +335,7 @@ RSpec.describe API::ProjectPackages do
it 'returns 404 for the package from a different project' do
project.add_maintainer(user)
- delete api(wrong_package_url, user)
+ expect { delete api(wrong_package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count }
expect(response).to have_gitlab_http_status(:not_found)
end
@@ -343,7 +343,7 @@ RSpec.describe API::ProjectPackages do
it 'returns 403 for a user without enough permissions' do
project.add_developer(user)
- delete api(package_url, user)
+ expect { delete api(package_url, user) }.not_to change { ::Packages::Package.pending_destruction.count }
expect(response).to have_gitlab_http_status(:forbidden)
end
@@ -351,7 +351,7 @@ RSpec.describe API::ProjectPackages do
it 'returns 204' do
project.add_maintainer(user)
- delete api(package_url, user)
+ expect { delete api(package_url, user) }.to change { ::Packages::Package.pending_destruction.count }.by(1)
expect(response).to have_gitlab_http_status(:no_content)
end
diff --git a/spec/requests/jira_connect/users_controller_spec.rb b/spec/requests/jira_connect/users_controller_spec.rb
new file mode 100644
index 00000000000..c648d28c1bc
--- /dev/null
+++ b/spec/requests/jira_connect/users_controller_spec.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe JiraConnect::UsersController do
+ describe 'GET /-/jira_connect/users' do
+ let_it_be(:user) { create(:user) }
+
+ before do
+ sign_in(user)
+ end
+
+ context 'with a valid host' do
+ let(:return_to) { 'https://testcompany.atlassian.net/plugins/servlet/ac/gitlab-jira-connect-staging.gitlab.com/gitlab-configuration' }
+
+ it 'includes a return url' do
+ get '/-/jira_connect/users', params: { return_to: return_to }
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(response.body).to include('Return to GitLab')
+ end
+ end
+
+ context 'with an invalid host' do
+ let(:return_to) { 'https://evil.com' }
+
+ it 'does not include a return url' do
+ get '/-/jira_connect/users', params: { return_to: return_to }
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(response.body).not_to include('Return to GitLab')
+ end
+ end
+ end
+end
diff --git a/spec/services/packages/mark_package_files_for_destruction_service_spec.rb b/spec/services/packages/mark_package_files_for_destruction_service_spec.rb
new file mode 100644
index 00000000000..a836de1f7f6
--- /dev/null
+++ b/spec/services/packages/mark_package_files_for_destruction_service_spec.rb
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::MarkPackageFilesForDestructionService, :aggregate_failures do
+ let(:service) { described_class.new(package_files) }
+
+ describe '#execute', :aggregate_failures do
+ subject { service.execute }
+
+ shared_examples 'executing successfully' do
+ it 'marks package files for destruction' do
+ expect { subject }
+ .to change { ::Packages::PackageFile.pending_destruction.count }.by(package_files.size)
+ end
+
+ it 'executes successfully' do
+ expect(subject).to be_success
+ expect(subject.message).to eq('Package files are now pending destruction')
+ end
+ end
+
+ context 'with no package files' do
+ let_it_be(:package_files) { ::Packages::PackageFile.none }
+
+ it_behaves_like 'executing successfully'
+ end
+
+ context 'with a single package file' do
+ let_it_be(:package_file) { create(:package_file) }
+ let_it_be(:package_files) { ::Packages::PackageFile.id_in(package_file.id) }
+
+ it_behaves_like 'executing successfully'
+ end
+
+ context 'with many package files' do
+ let_it_be(:package_files) { ::Packages::PackageFile.id_in(create_list(:package_file, 3).map(&:id)) }
+
+ it_behaves_like 'executing successfully'
+ end
+
+ context 'with an error during the update' do
+ let_it_be(:package_files) { ::Packages::PackageFile.none }
+
+ before do
+ expect(package_files).to receive(:each_batch).and_raise('error!')
+ end
+
+ it 'raises the error' do
+ expect { subject }
+ .to raise_error('error!')
+ .and not_change { ::Packages::PackageFile.pending_destruction.count }
+ end
+ end
+ end
+end
diff --git a/spec/services/packages/destroy_package_service_spec.rb b/spec/services/packages/mark_package_for_destruction_service_spec.rb
index 92db8da968c..125ec53ad61 100644
--- a/spec/services/packages/destroy_package_service_spec.rb
+++ b/spec/services/packages/mark_package_for_destruction_service_spec.rb
@@ -2,10 +2,9 @@
require 'spec_helper'
-RSpec.describe Packages::DestroyPackageService do
+RSpec.describe Packages::MarkPackageForDestructionService do
let_it_be(:user) { create(:user) }
-
- let!(:package) { create(:npm_package) }
+ let_it_be_with_reload(:package) { create(:npm_package) }
describe '#execute' do
subject(:service) { described_class.new(container: package, current_user: user) }
@@ -15,10 +14,11 @@ RSpec.describe Packages::DestroyPackageService do
package.project.add_maintainer(user)
end
- context 'when the destroy is successfull' do
- it 'destroy the package' do
+ context 'when it is successful' do
+ it 'marks the package and package files as pending destruction' do
expect(package).to receive(:sync_maven_metadata).and_call_original
- expect { service.execute }.to change { Packages::Package.count }.by(-1)
+ expect(package).to receive(:mark_package_files_for_destruction).and_call_original
+ expect { service.execute }.to change { package.status }.from('default').to('pending_destruction')
end
it 'returns a success ServiceResponse' do
@@ -26,13 +26,13 @@ RSpec.describe Packages::DestroyPackageService do
expect(response).to be_a(ServiceResponse)
expect(response).to be_success
- expect(response.message).to eq("Package was successfully deleted")
+ expect(response.message).to eq("Package was successfully marked as pending destruction")
end
end
- context 'when the destroy is not successful' do
+ context 'when it is not successful' do
before do
- allow(package).to receive(:destroy!).and_raise(StandardError, "test")
+ allow(package).to receive(:pending_destruction!).and_raise(StandardError, "test")
end
it 'returns an error ServiceResponse' do
@@ -41,7 +41,7 @@ RSpec.describe Packages::DestroyPackageService do
expect(package).not_to receive(:sync_maven_metadata)
expect(response).to be_a(ServiceResponse)
expect(response).to be_error
- expect(response.message).to eq("Failed to remove the package")
+ expect(response.message).to eq("Failed to mark the package as pending destruction")
expect(response.status).to eq(:error)
end
end
diff --git a/spec/services/protected_branches/create_service_spec.rb b/spec/services/protected_branches/create_service_spec.rb
index 0bea3edf203..3ac42d41377 100644
--- a/spec/services/protected_branches/create_service_spec.rb
+++ b/spec/services/protected_branches/create_service_spec.rb
@@ -24,38 +24,14 @@ RSpec.describe ProtectedBranches::CreateService do
expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER])
end
- context 'when name has escaped HTML' do
- let(:name) { 'feature-&gt;test' }
+ context 'when protecting a branch with a name that contains HTML tags' do
+ let(:name) { 'foo<b>bar<\b>' }
- it 'creates the new protected branch matching the unescaped version' do
- expect { service.execute }.to change(ProtectedBranch, :count).by(1)
- expect(project.protected_branches.last.name).to eq('feature->test')
- end
-
- context 'and name contains HTML tags' do
- let(:name) { '&lt;b&gt;master&lt;/b&gt;' }
-
- it 'creates the new protected branch with sanitized name' do
- expect { service.execute }.to change(ProtectedBranch, :count).by(1)
- expect(project.protected_branches.last.name).to eq('master')
- end
-
- context 'and contains unsafe HTML' do
- let(:name) { '&lt;script&gt;alert(&#39;foo&#39;);&lt;/script&gt;' }
+ subject(:service) { described_class.new(project, user, params) }
- it 'does not create the new protected branch' do
- expect { service.execute }.not_to change(ProtectedBranch, :count)
- end
- end
- end
-
- context 'when name contains unescaped HTML tags' do
- let(:name) { '<b>master</b>' }
-
- it 'creates the new protected branch with sanitized name' do
- expect { service.execute }.to change(ProtectedBranch, :count).by(1)
- expect(project.protected_branches.last.name).to eq('master')
- end
+ it 'creates a new protected branch' do
+ expect { service.execute }.to change(ProtectedBranch, :count).by(1)
+ expect(project.protected_branches.last.name).to eq(name)
end
end
diff --git a/spec/services/protected_branches/update_service_spec.rb b/spec/services/protected_branches/update_service_spec.rb
index 3d9b77dcfc0..4405af35c37 100644
--- a/spec/services/protected_branches/update_service_spec.rb
+++ b/spec/services/protected_branches/update_service_spec.rb
@@ -18,35 +18,14 @@ RSpec.describe ProtectedBranches::UpdateService do
expect(result.reload.name).to eq(params[:name])
end
- context 'when name has escaped HTML' do
- let(:new_name) { 'feature-&gt;test' }
+ context 'when updating name of a protected branch to one that contains HTML tags' do
+ let(:new_name) { 'foo<b>bar<\b>' }
+ let(:result) { service.execute(protected_branch) }
- it 'updates protected branch name with unescaped HTML' do
- expect(result.reload.name).to eq('feature->test')
- end
-
- context 'and name contains HTML tags' do
- let(:new_name) { '&lt;b&gt;master&lt;/b&gt;' }
-
- it 'updates protected branch name with sanitized name' do
- expect(result.reload.name).to eq('master')
- end
-
- context 'and contains unsafe HTML' do
- let(:new_name) { '&lt;script&gt;alert(&#39;foo&#39;);&lt;/script&gt;' }
-
- it 'does not update the protected branch' do
- expect(result.reload.name).to eq(protected_branch.name)
- end
- end
- end
- end
-
- context 'when name contains unescaped HTML tags' do
- let(:new_name) { '<b>master</b>' }
+ subject(:service) { described_class.new(project, user, params) }
- it 'updates protected branch name with sanitized name' do
- expect(result.reload.name).to eq('master')
+ it 'updates a protected branch' do
+ expect(result.reload.name).to eq(new_name)
end
end
diff --git a/spec/services/protected_tags/create_service_spec.rb b/spec/services/protected_tags/create_service_spec.rb
index 31059d17f10..a0b99b595e3 100644
--- a/spec/services/protected_tags/create_service_spec.rb
+++ b/spec/services/protected_tags/create_service_spec.rb
@@ -22,38 +22,14 @@ RSpec.describe ProtectedTags::CreateService do
expect(project.protected_tags.last.create_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER])
end
- context 'when name has escaped HTML' do
- let(:name) { 'tag-&gt;test' }
+ context 'protecting a tag with a name that contains HTML tags' do
+ let(:name) { 'foo<b>bar<\b>' }
- it 'creates the new protected tag matching the unescaped version' do
- expect { service.execute }.to change(ProtectedTag, :count).by(1)
- expect(project.protected_tags.last.name).to eq('tag->test')
- end
-
- context 'and name contains HTML tags' do
- let(:name) { '&lt;b&gt;tag&lt;/b&gt;' }
-
- it 'creates the new protected tag with sanitized name' do
- expect { service.execute }.to change(ProtectedTag, :count).by(1)
- expect(project.protected_tags.last.name).to eq('tag')
- end
-
- context 'and contains unsafe HTML' do
- let(:name) { '&lt;script&gt;alert(&#39;foo&#39;);&lt;/script&gt;' }
+ subject(:service) { described_class.new(project, user, params) }
- it 'does not create the new protected tag' do
- expect { service.execute }.not_to change(ProtectedTag, :count)
- end
- end
- end
-
- context 'when name contains unescaped HTML tags' do
- let(:name) { '<b>tag</b>' }
-
- it 'creates the new protected tag with sanitized name' do
- expect { service.execute }.to change(ProtectedTag, :count).by(1)
- expect(project.protected_tags.last.name).to eq('tag')
- end
+ it 'creates a new protected tag' do
+ expect { service.execute }.to change(ProtectedTag, :count).by(1)
+ expect(project.protected_tags.last.name).to eq(name)
end
end
end
diff --git a/spec/services/protected_tags/update_service_spec.rb b/spec/services/protected_tags/update_service_spec.rb
index 8d301dcd825..4b6e726bb6e 100644
--- a/spec/services/protected_tags/update_service_spec.rb
+++ b/spec/services/protected_tags/update_service_spec.rb
@@ -18,35 +18,14 @@ RSpec.describe ProtectedTags::UpdateService do
expect(result.reload.name).to eq(params[:name])
end
- context 'when name has escaped HTML' do
- let(:new_name) { 'tag-&gt;test' }
+ context 'when updating protected tag with a name that contains HTML tags' do
+ let(:new_name) { 'foo<b>bar<\b>' }
+ let(:result) { service.execute(protected_tag) }
- it 'updates protected tag name with unescaped HTML' do
- expect(result.reload.name).to eq('tag->test')
- end
-
- context 'and name contains HTML tags' do
- let(:new_name) { '&lt;b&gt;tag&lt;/b&gt;' }
-
- it 'updates protected tag name with sanitized name' do
- expect(result.reload.name).to eq('tag')
- end
-
- context 'and contains unsafe HTML' do
- let(:new_name) { '&lt;script&gt;alert(&#39;foo&#39;);&lt;/script&gt;' }
-
- it 'does not update the protected tag' do
- expect(result.reload.name).to eq(protected_tag.name)
- end
- end
- end
- end
-
- context 'when name contains unescaped HTML tags' do
- let(:new_name) { '<b>tag</b>' }
+ subject(:service) { described_class.new(project, user, params) }
- it 'updates protected tag name with sanitized name' do
- expect(result.reload.name).to eq('tag')
+ it 'updates a protected tag' do
+ expect(result.reload.name).to eq(new_name)
end
end
diff --git a/spec/support/shared_contexts/models/concerns/integrations/enable_ssl_verification_shared_context.rb b/spec/support/shared_contexts/models/concerns/integrations/enable_ssl_verification_shared_context.rb
new file mode 100644
index 00000000000..c698e06c2a2
--- /dev/null
+++ b/spec/support/shared_contexts/models/concerns/integrations/enable_ssl_verification_shared_context.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+RSpec.shared_context Integrations::EnableSslVerification do
+ # This is added to the global setup, to make sure all calls to
+ # `Gitlab::HTTP` in the main model spec are passing the `verify:` option.
+ before do
+ allow(Gitlab::HTTP).to receive(:perform_request)
+ .with(anything, anything, include(verify: true))
+ .and_call_original
+ end
+
+ describe 'accessors' do
+ it { is_expected.to respond_to(:enable_ssl_verification) }
+ it { is_expected.to respond_to(:enable_ssl_verification?) }
+ end
+
+ describe '#initialize_properties' do
+ it 'enables the setting by default' do
+ expect(integration.enable_ssl_verification).to be(true)
+ end
+
+ it 'does not enable the setting if the record is already persisted' do
+ allow(integration).to receive(:new_record?).and_return(false)
+
+ integration.enable_ssl_verification = false
+ integration.send(:initialize_properties)
+
+ expect(integration.enable_ssl_verification).to be(false)
+ end
+
+ it 'does not enable the setting if a custom value was set' do
+ integration = described_class.new(enable_ssl_verification: false)
+
+ expect(integration.enable_ssl_verification).to be(false)
+ end
+ end
+
+ describe '#fields' do
+ it 'inserts the checkbox field after the first URL field, or at the end' do
+ names = integration.fields.pluck(:name)
+ url_index = names.index { |name| name.ends_with?('_url') }
+ insert_index = url_index ? url_index + 1 : names.size - 1
+
+ expect(names.index('enable_ssl_verification')).to eq insert_index
+ end
+ end
+end
diff --git a/spec/support/shared_examples/models/integrations/has_web_hook_shared_examples.rb b/spec/support/shared_examples/models/integrations/has_web_hook_shared_examples.rb
index 1fa340a0cf4..ae72cb6ec5d 100644
--- a/spec/support/shared_examples/models/integrations/has_web_hook_shared_examples.rb
+++ b/spec/support/shared_examples/models/integrations/has_web_hook_shared_examples.rb
@@ -37,6 +37,16 @@ RSpec.shared_examples Integrations::HasWebHook do
it 'returns a boolean' do
expect(integration.hook_ssl_verification).to be_in([true, false])
end
+
+ it 'delegates to #enable_ssl_verification if the concern is included' do
+ next unless integration.is_a?(Integrations::EnableSslVerification)
+
+ [true, false].each do |value|
+ integration.enable_ssl_verification = value
+
+ expect(integration.hook_ssl_verification).to be(value)
+ end
+ end
end
describe '#update_web_hook!' do
diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb
index bb4e2981070..4f9c207f976 100644
--- a/spec/workers/every_sidekiq_worker_spec.rb
+++ b/spec/workers/every_sidekiq_worker_spec.rb
@@ -364,6 +364,7 @@ RSpec.describe 'Every Sidekiq worker' do
'Packages::CleanupPackageFileWorker' => 0,
'Packages::Composer::CacheUpdateWorker' => false,
'Packages::Go::SyncPackagesWorker' => 3,
+ 'Packages::MarkPackageFilesForDestructionWorker' => 3,
'Packages::Maven::Metadata::SyncWorker' => 3,
'Packages::Nuget::ExtractionWorker' => 3,
'Packages::Rubygems::ExtractionWorker' => 3,
diff --git a/spec/workers/packages/cleanup_package_file_worker_spec.rb b/spec/workers/packages/cleanup_package_file_worker_spec.rb
index b423c4d3f06..33f89826312 100644
--- a/spec/workers/packages/cleanup_package_file_worker_spec.rb
+++ b/spec/workers/packages/cleanup_package_file_worker_spec.rb
@@ -23,6 +23,7 @@ RSpec.describe Packages::CleanupPackageFileWorker do
expect(worker).to receive(:log_extra_metadata_on_done).twice
expect { subject }.to change { Packages::PackageFile.count }.by(-1)
+ .and not_change { Packages::Package.count }
end
end
@@ -38,6 +39,17 @@ RSpec.describe Packages::CleanupPackageFileWorker do
expect(package_file.reload).to be_error
end
end
+
+ context 'removing the last package file' do
+ let_it_be(:package_file) { create(:package_file, :pending_destruction, package: package) }
+
+ it 'deletes the package file and the package' do
+ expect(worker).to receive(:log_extra_metadata_on_done).twice
+
+ expect { subject }.to change { Packages::PackageFile.count }.by(-1)
+ .and change { Packages::Package.count }.by(-1)
+ end
+ end
end
describe '#max_running_jobs' do
diff --git a/spec/workers/packages/mark_package_files_for_destruction_worker_spec.rb b/spec/workers/packages/mark_package_files_for_destruction_worker_spec.rb
new file mode 100644
index 00000000000..15d9e4c347b
--- /dev/null
+++ b/spec/workers/packages/mark_package_files_for_destruction_worker_spec.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::MarkPackageFilesForDestructionWorker, :aggregate_failures do
+ describe '#perform' do
+ let_it_be(:package) { create(:package) }
+ let_it_be(:package_files) { create_list(:package_file, 3, package: package) }
+
+ let(:worker) { described_class.new }
+ let(:job_args) { [package.id] }
+
+ subject { worker.perform(*job_args) }
+
+ context 'with a valid package id' do
+ it_behaves_like 'an idempotent worker'
+
+ it 'marks all package files as pending_destruction' do
+ package_files_count = package.package_files.count
+
+ expect(package.package_files.pending_destruction.count).to eq(0)
+ expect(package.package_files.default.count).to eq(package_files_count)
+
+ subject
+
+ expect(package.package_files.default.count).to eq(0)
+ expect(package.package_files.pending_destruction.count).to eq(package_files_count)
+ end
+ end
+
+ context 'with an invalid package id' do
+ let(:job_args) { [non_existing_record_id] }
+
+ it_behaves_like 'an idempotent worker'
+
+ it 'marks no packag files' do
+ expect(::Packages::MarkPackageFilesForDestructionService).not_to receive(:new)
+
+ expect { subject }.not_to change { ::Packages::PackageFile.pending_destruction.count }
+ end
+ end
+
+ context 'with a nil package id' do
+ let(:job_args) { [nil] }
+
+ it_behaves_like 'an idempotent worker'
+
+ it 'marks no packag files' do
+ expect(::Packages::MarkPackageFilesForDestructionService).not_to receive(:new)
+
+ expect { subject }.not_to change { ::Packages::PackageFile.pending_destruction.count }
+ end
+ end
+ end
+end