diff options
Diffstat (limited to 'app/services')
137 files changed, 1976 insertions, 736 deletions
diff --git a/app/services/achievements/destroy_user_achievement_service.rb b/app/services/achievements/destroy_user_achievement_service.rb new file mode 100644 index 00000000000..3beaed646e3 --- /dev/null +++ b/app/services/achievements/destroy_user_achievement_service.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Achievements + class DestroyUserAchievementService + attr_reader :current_user, :user_achievement + + def initialize(current_user, user_achievement) + @current_user = current_user + @user_achievement = user_achievement + end + + def execute + return error_no_permissions unless allowed? + + user_achievement.delete + ServiceResponse.success(payload: user_achievement) + end + + private + + def allowed? + current_user&.can?(:destroy_user_achievement, user_achievement) + end + + def error_no_permissions + error('You have insufficient permissions to delete this user achievement') + end + + def error(message) + ServiceResponse.error(message: Array(message)) + end + end +end diff --git a/app/services/admin/abuse_report_update_service.rb b/app/services/admin/abuse_report_update_service.rb index 5b2ad27ede4..12cf8bf14a8 100644 --- a/app/services/admin/abuse_report_update_service.rb +++ b/app/services/admin/abuse_report_update_service.rb @@ -17,8 +17,8 @@ module Admin result = perform_action if result[:status] == :success - close_report_and_record_event - ServiceResponse.success + event = close_report_and_record_event + ServiceResponse.success(message: event.success_message) else ServiceResponse.error(message: result[:message]) end @@ -58,6 +58,8 @@ module Admin end def close_report + return error('Report already closed') if abuse_report.closed? + abuse_report.closed! success end diff --git a/app/services/admin/plan_limits/update_service.rb b/app/services/admin/plan_limits/update_service.rb new file mode 100644 index 00000000000..a71d1f14112 --- /dev/null +++ b/app/services/admin/plan_limits/update_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Admin + module PlanLimits + class UpdateService < ::BaseService + def initialize(params = {}, current_user:, plan:) + @current_user = current_user + @params = params + @plan = plan + end + + def execute + return error(_('Access denied'), :forbidden) unless can_update? + + if plan.actual_limits.update(parsed_params) + success + else + error(plan.actual_limits.errors.full_messages, :bad_request) + end + end + + private + + attr_accessor :current_user, :params, :plan + + def can_update? + current_user.can_admin_all_resources? + end + + # Overridden in EE + def parsed_params + params + end + end + end +end + +Admin::PlanLimits::UpdateService.prepend_mod_with('Admin::PlanLimits::UpdateService') diff --git a/app/services/alert_management/http_integrations/base_service.rb b/app/services/alert_management/http_integrations/base_service.rb new file mode 100644 index 00000000000..980f18631c0 --- /dev/null +++ b/app/services/alert_management/http_integrations/base_service.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module AlertManagement + module HttpIntegrations + class BaseService < BaseProjectService + # @param project [Project] + # @param current_user [User] + # @param params [Hash] + def initialize(project, current_user, params) + @response = nil + + super(project: project, current_user: current_user, params: params.with_indifferent_access) + end + + private + + def allowed? + current_user&.can?(:admin_operations, project) + end + + def too_many_integrations?(integration) + AlertManagement::HttpIntegration + .for_project(integration.project_id) + .for_type(integration.type_identifier) + .id_not_in(integration.id) + .any? + end + + def permitted_params + params.slice(*permitted_params_keys) + end + + # overriden in EE + def permitted_params_keys + %i[name active type_identifier] + end + + def error(message) + ServiceResponse.error(message: message) + end + + def success(integration) + ServiceResponse.success(payload: { integration: integration.reset }) + end + + def error_multiple_integrations + error(_('Multiple integrations of a single type are not supported for this project')) + end + + def error_on_save(integration) + error(integration.errors.full_messages.to_sentence) + end + end + end +end + +::AlertManagement::HttpIntegrations::BaseService.prepend_mod diff --git a/app/services/alert_management/http_integrations/create_service.rb b/app/services/alert_management/http_integrations/create_service.rb index 1abe0548c45..17e39577c29 100644 --- a/app/services/alert_management/http_integrations/create_service.rb +++ b/app/services/alert_management/http_integrations/create_service.rb @@ -2,68 +2,34 @@ module AlertManagement module HttpIntegrations - class CreateService - # @param project [Project] - # @param current_user [User] - # @param params [Hash] - def initialize(project, current_user, params) - @project = project - @current_user = current_user - @params = params.with_indifferent_access - end - + class CreateService < BaseService def execute return error_no_permissions unless allowed? - return error_multiple_integrations unless creation_allowed? - - integration = project.alert_management_http_integrations.create(permitted_params) - return error_in_create(integration) unless integration.valid? - - success(integration) - end - private + ::AlertManagement::HttpIntegration.transaction do + integration = project.alert_management_http_integrations.build(permitted_params) - attr_reader :project, :current_user, :params + if integration.save + @response = success(integration) - def allowed? - current_user&.can?(:admin_operations, project) - end + if too_many_integrations?(integration) + @response = error_multiple_integrations - def creation_allowed? - project.alert_management_http_integrations.empty? - end - - def permitted_params - params.slice(*permitted_params_keys) - end + raise ActiveRecord::Rollback + end + else + @response = error_on_save(integration) + end + end - # overriden in EE - def permitted_params_keys - %i[name active] + @response end - def error(message) - ServiceResponse.error(message: message) - end - - def success(integration) - ServiceResponse.success(payload: { integration: integration }) - end + private def error_no_permissions error(_('You have insufficient permissions to create an HTTP integration for this project')) end - - def error_multiple_integrations - error(_('Multiple HTTP integrations are not supported for this project')) - end - - def error_in_create(integration) - error(integration.errors.full_messages.to_sentence) - end end end end - -::AlertManagement::HttpIntegrations::CreateService.prepend_mod_with('AlertManagement::HttpIntegrations::CreateService') diff --git a/app/services/alert_management/http_integrations/destroy_service.rb b/app/services/alert_management/http_integrations/destroy_service.rb index aeb3f6cb807..1bd73ca46e4 100644 --- a/app/services/alert_management/http_integrations/destroy_service.rb +++ b/app/services/alert_management/http_integrations/destroy_service.rb @@ -12,6 +12,7 @@ module AlertManagement def execute return error_no_permissions unless allowed? + return error_legacy_prometheus unless destroy_allowed? if integration.destroy success @@ -28,6 +29,12 @@ module AlertManagement current_user&.can?(:admin_operations, integration) end + # Prevents downtime while migrating from Integrations::Prometheus. + # Remove with https://gitlab.com/gitlab-org/gitlab/-/issues/409734 + def destroy_allowed? + !(integration.legacy? && integration.prometheus?) + end + def error(message) ServiceResponse.error(message: message) end @@ -39,6 +46,10 @@ module AlertManagement def error_no_permissions error(_('You have insufficient permissions to remove this HTTP integration')) end + + def error_legacy_prometheus + error(_('Legacy Prometheus integrations cannot currently be removed')) + end end end end diff --git a/app/services/alert_management/http_integrations/update_service.rb b/app/services/alert_management/http_integrations/update_service.rb index 8662f966a2e..f7a079576e4 100644 --- a/app/services/alert_management/http_integrations/update_service.rb +++ b/app/services/alert_management/http_integrations/update_service.rb @@ -2,51 +2,48 @@ module AlertManagement module HttpIntegrations - class UpdateService + class UpdateService < BaseService # @param integration [AlertManagement::HttpIntegration] # @param current_user [User] # @param params [Hash] def initialize(integration, current_user, params) @integration = integration - @current_user = current_user - @params = params.with_indifferent_access + + super(integration.project, current_user, params) end def execute return error_no_permissions unless allowed? - params[:token] = nil if params.delete(:regenerate_token) + integration.transaction do + if integration.update(permitted_params.merge(token_params)) + @response = success(integration) + + if type_update? && too_many_integrations?(integration) + @response = error_multiple_integrations - if integration.update(permitted_params) - success - else - error(integration.errors.full_messages.to_sentence) + raise ActiveRecord::Rollback + end + else + @response = error_on_save(integration) + end end + + @response end private - attr_reader :integration, :current_user, :params + attr_reader :integration - def allowed? - current_user&.can?(:admin_operations, integration) - end + def token_params + return {} unless params[:regenerate_token] - def permitted_params - params.slice(*permitted_params_keys) + { token: nil } end - # overriden in EE - def permitted_params_keys - %i[name active token] - end - - def error(message) - ServiceResponse.error(message: message) - end - - def success - ServiceResponse.success(payload: { integration: integration.reset }) + def type_update? + params[:type_identifier].present? end def error_no_permissions @@ -55,5 +52,3 @@ module AlertManagement end end end - -::AlertManagement::HttpIntegrations::UpdateService.prepend_mod_with('AlertManagement::HttpIntegrations::UpdateService') diff --git a/app/services/alert_management/process_prometheus_alert_service.rb b/app/services/alert_management/process_prometheus_alert_service.rb index e0594247975..556f04e8786 100644 --- a/app/services/alert_management/process_prometheus_alert_service.rb +++ b/app/services/alert_management/process_prometheus_alert_service.rb @@ -6,9 +6,10 @@ module AlertManagement include ::AlertManagement::AlertProcessing include ::AlertManagement::Responses - def initialize(project, payload) + def initialize(project, payload, integration: nil) @project = project @payload = payload + @integration = integration end def execute @@ -24,7 +25,7 @@ module AlertManagement private - attr_reader :project, :payload + attr_reader :project, :payload, :integration override :incoming_payload def incoming_payload @@ -32,6 +33,7 @@ module AlertManagement Gitlab::AlertManagement::Payload.parse( project, payload, + integration: integration, monitoring_tool: Gitlab::AlertManagement::Payload::MONITORING_TOOLS[:prometheus] ) end diff --git a/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb b/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb index d18f2935d92..2bbb8f925a4 100644 --- a/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb +++ b/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb @@ -11,9 +11,14 @@ module AutoMerge end def process(merge_request) + logger.info("Processing Automerge") return unless merge_request.actual_head_pipeline_success? + + logger.info("Pipeline Success") return unless merge_request.mergeable? + logger.info("Merge request mergeable") + merge_request.merge_async(merge_request.merge_user_id, merge_request.merge_params) end @@ -40,5 +45,9 @@ module AutoMerge def notify(merge_request) notification_service.async.merge_when_pipeline_succeeds(merge_request, current_user) if merge_request.saved_change_to_auto_merge_enabled? end + + def logger + @logger ||= Gitlab::AppLogger + end end end diff --git a/app/services/boards/issues/create_service.rb b/app/services/boards/issues/create_service.rb index 77e297b6b11..04b5d826416 100644 --- a/app/services/boards/issues/create_service.rb +++ b/app/services/boards/issues/create_service.rb @@ -32,7 +32,7 @@ module Boards def create_issue(params) # NOTE: We are intentionally not doing a spam/CAPTCHA check for issues created via boards. # See https://gitlab.com/gitlab-org/gitlab/-/issues/29400#note_598479184 for more context. - ::Issues::CreateService.new(container: project, current_user: current_user, params: params, spam_params: nil).execute + ::Issues::CreateService.new(container: project, current_user: current_user, params: params, perform_spam_check: false).execute end end end diff --git a/app/services/bulk_imports/archive_extraction_service.rb b/app/services/bulk_imports/archive_extraction_service.rb index fec8fd0e1f5..4485b19035b 100644 --- a/app/services/bulk_imports/archive_extraction_service.rb +++ b/app/services/bulk_imports/archive_extraction_service.rb @@ -41,11 +41,11 @@ module BulkImports attr_reader :tmpdir, :filename, :filepath def validate_filepath - Gitlab::Utils.check_path_traversal!(filepath) + Gitlab::PathTraversal.check_path_traversal!(filepath) end def validate_tmpdir - Gitlab::Utils.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir]) + Gitlab::PathTraversal.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir]) end def validate_symlink diff --git a/app/services/bulk_imports/create_service.rb b/app/services/bulk_imports/create_service.rb index 4c9c59ac504..636c636255f 100644 --- a/app/services/bulk_imports/create_service.rb +++ b/app/services/bulk_imports/create_service.rb @@ -118,9 +118,10 @@ module BulkImports end client.get("/#{entity_type}/#{source_entity_identifier}/export_relations/status") + rescue BulkImports::NetworkError => e # the source instance will return a 404 if the feature is disabled as the endpoint won't be available - rescue Gitlab::HTTP::BlockedUrlError - rescue BulkImports::NetworkError + return if e.cause.is_a?(Gitlab::HTTP::BlockedUrlError) + raise ::BulkImports::Error.setting_not_enabled end diff --git a/app/services/bulk_imports/file_decompression_service.rb b/app/services/bulk_imports/file_decompression_service.rb index 41616fc1c75..94573f6bb13 100644 --- a/app/services/bulk_imports/file_decompression_service.rb +++ b/app/services/bulk_imports/file_decompression_service.rb @@ -41,11 +41,11 @@ module BulkImports attr_reader :tmpdir, :filename, :filepath, :decompressed_filename, :decompressed_filepath def validate_filepath - Gitlab::Utils.check_path_traversal!(filepath) + Gitlab::PathTraversal.check_path_traversal!(filepath) end def validate_tmpdir - Gitlab::Utils.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir]) + Gitlab::PathTraversal.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir]) end def validate_decompressed_file_size diff --git a/app/services/bulk_imports/file_download_service.rb b/app/services/bulk_imports/file_download_service.rb index ee499c782b4..ef7e0ae8258 100644 --- a/app/services/bulk_imports/file_download_service.rb +++ b/app/services/bulk_imports/file_download_service.rb @@ -99,7 +99,7 @@ module BulkImports end def validate_tmpdir - Gitlab::Utils.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir]) + Gitlab::PathTraversal.check_allowed_absolute_path!(tmpdir, [Dir.tmpdir]) end def filepath diff --git a/app/services/ci/cancel_pipeline_service.rb b/app/services/ci/cancel_pipeline_service.rb new file mode 100644 index 00000000000..b5c8c00273e --- /dev/null +++ b/app/services/ci/cancel_pipeline_service.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +module Ci + # Cancel a pipelines cancelable jobs and optionally it's child pipelines cancelable jobs + class CancelPipelineService + include Gitlab::OptimisticLocking + include Gitlab::Allowable + + ## + # @cascade_to_children - if true cancels all related child pipelines for parent child pipelines + # @auto_canceled_by_pipeline_id - store the pipeline_id of the pipeline that triggered cancellation + # @execute_async - if true cancel the children asyncronously + def initialize( + pipeline:, + current_user:, + cascade_to_children: true, + auto_canceled_by_pipeline_id: nil, + execute_async: true) + @pipeline = pipeline + @current_user = current_user + @cascade_to_children = cascade_to_children + @auto_canceled_by_pipeline_id = auto_canceled_by_pipeline_id + @execute_async = execute_async + end + + def execute + unless can?(current_user, :update_pipeline, pipeline) + return ServiceResponse.error( + message: 'Insufficient permissions to cancel the pipeline', + reason: :insufficient_permissions) + end + + force_execute + end + + # This method should be used only when we want to always cancel the pipeline without + # checking whether the current_user has permissions to do so, or when we don't have + # a current_user available in the context. + def force_execute + return ServiceResponse.error(message: 'No pipeline provided', reason: :no_pipeline) unless pipeline + + unless pipeline.cancelable? + return ServiceResponse.error(message: 'Pipeline is not cancelable', reason: :pipeline_not_cancelable) + end + + log_pipeline_being_canceled + + pipeline.update_column(:auto_canceled_by_id, @auto_canceled_by_pipeline_id) if @auto_canceled_by_pipeline_id + cancel_jobs(pipeline.cancelable_statuses) + + return ServiceResponse.success unless cascade_to_children? + + # cancel any bridges that could spin up new child pipelines + cancel_jobs(pipeline.bridges_in_self_and_project_descendants.cancelable) + cancel_children + + ServiceResponse.success + end + + private + + attr_reader :pipeline, :current_user + + def log_pipeline_being_canceled + Gitlab::AppJsonLogger.info( + event: 'pipeline_cancel_running', + pipeline_id: pipeline.id, + auto_canceled_by_pipeline_id: @auto_canceled_by_pipeline_id, + cascade_to_children: cascade_to_children?, + execute_async: execute_async?, + **Gitlab::ApplicationContext.current + ) + end + + def cascade_to_children? + @cascade_to_children + end + + def execute_async? + @execute_async + end + + def cancel_jobs(jobs) + retries = 3 + retry_lock(jobs, retries, name: 'ci_pipeline_cancel_running') do |jobs_to_cancel| + preloaded_relations = [:project, :pipeline, :deployment, :taggings] + + jobs_to_cancel.find_in_batches do |batch| + relation = CommitStatus.id_in(batch) + Preloaders::CommitStatusPreloader.new(relation).execute(preloaded_relations) + + relation.each do |job| + job.auto_canceled_by_id = @auto_canceled_by_pipeline_id if @auto_canceled_by_pipeline_id + job.cancel + end + end + end + end + + # For parent child-pipelines only (not multi-project) + def cancel_children + pipeline.all_child_pipelines.each do |child_pipeline| + if execute_async? + ::Ci::CancelPipelineWorker.perform_async( + child_pipeline.id, + @auto_canceled_by_pipeline_id + ) + else + # cascade_to_children is false because we iterate through children + # we also cancel bridges prior to prevent more children + self.class.new( + pipeline: child_pipeline.reset, + current_user: nil, + cascade_to_children: false, + execute_async: execute_async?, + auto_canceled_by_pipeline_id: @auto_canceled_by_pipeline_id + ).force_execute + end + end + end + end +end diff --git a/app/services/ci/delete_unit_tests_service.rb b/app/services/ci/delete_unit_tests_service.rb index 230661a107d..a2fb44ff3fc 100644 --- a/app/services/ci/delete_unit_tests_service.rb +++ b/app/services/ci/delete_unit_tests_service.rb @@ -25,9 +25,7 @@ module Ci klass.transaction do ids = klass.deletable.lock('FOR UPDATE SKIP LOCKED').limit(BATCH_SIZE).pluck(:id) - break if ids.empty? - - deleted = klass.where(id: ids).delete_all + deleted = klass.where(id: ids).delete_all if ids.any? end deleted > 0 diff --git a/app/services/ci/destroy_pipeline_service.rb b/app/services/ci/destroy_pipeline_service.rb index 1c563396162..bdec13f98a7 100644 --- a/app/services/ci/destroy_pipeline_service.rb +++ b/app/services/ci/destroy_pipeline_service.rb @@ -7,7 +7,13 @@ module Ci Ci::ExpirePipelineCacheService.new.execute(pipeline, delete: true) - pipeline.cancel_running(cascade_to_children: true, execute_async: false) if pipeline.cancelable? + # ensure cancellation happens sync so we accumulate compute credits successfully + # before deleting the pipeline. + ::Ci::CancelPipelineService.new( + pipeline: pipeline, + current_user: current_user, + cascade_to_children: true, + execute_async: false).force_execute # The pipeline, the builds, job and pipeline artifacts all get destroyed here. # Ci::Pipeline#destroy triggers fast destroy on job_artifacts and diff --git a/app/services/ci/job_artifacts/create_service.rb b/app/services/ci/job_artifacts/create_service.rb index f7e04c59463..3ac0e83232f 100644 --- a/app/services/ci/job_artifacts/create_service.rb +++ b/app/services/ci/job_artifacts/create_service.rb @@ -26,7 +26,8 @@ module Ci headers = JobArtifactUploader.workhorse_authorize( has_length: false, maximum_size: max_size(artifact_type), - use_final_store_path: Feature.enabled?(:ci_artifacts_upload_to_final_location, project) + use_final_store_path: true, + final_store_path_root_id: project.id ) if lsif?(artifact_type) diff --git a/app/services/ci/job_token_scope/remove_project_service.rb b/app/services/ci/job_token_scope/remove_project_service.rb index d6a2defd5b9..eddd4d79484 100644 --- a/app/services/ci/job_token_scope/remove_project_service.rb +++ b/app/services/ci/job_token_scope/remove_project_service.rb @@ -26,7 +26,7 @@ module Ci ServiceResponse.error(message: link.errors.full_messages.to_sentence, payload: { project_link: link }) end rescue EditScopeValidations::ValidationError => e - ServiceResponse.error(message: e.message) + ServiceResponse.error(message: e.message, reason: :insufficient_permissions) end end end diff --git a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb index 48c3e6490ae..e197821a0c0 100644 --- a/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb +++ b/app/services/ci/pipeline_creation/cancel_redundant_pipelines_service.rb @@ -6,6 +6,7 @@ module Ci include Gitlab::Utils::StrongMemoize BATCH_SIZE = 25 + PAGE_SIZE = 500 def initialize(pipeline) @pipeline = pipeline @@ -14,13 +15,24 @@ module Ci # rubocop: disable CodeReuse/ActiveRecord def execute + return if service_disabled? return if pipeline.parent_pipeline? # skip if child pipeline return unless project.auto_cancel_pending_pipelines? - Gitlab::OptimisticLocking - .retry_lock(parent_and_child_pipelines, name: 'cancel_pending_pipelines') do |cancelables| - cancelables.select(:id).each_batch(of: BATCH_SIZE) do |cancelables_batch| - auto_cancel_interruptible_pipelines(cancelables_batch.ids) + if Feature.enabled?(:use_offset_pagination_for_canceling_redundant_pipelines, project) + paginator.each do |ids| + pipelines = parent_and_child_pipelines(ids) + + Gitlab::OptimisticLocking.retry_lock(pipelines, name: 'cancel_pending_pipelines') do |cancelables| + auto_cancel_interruptible_pipelines(cancelables.ids) + end + end + else + Gitlab::OptimisticLocking + .retry_lock(parent_and_child_pipelines, name: 'cancel_pending_pipelines') do |cancelables| + cancelables.select(:id).each_batch(of: BATCH_SIZE) do |cancelables_batch| + auto_cancel_interruptible_pipelines(cancelables_batch.ids) + end end end end @@ -29,17 +41,40 @@ module Ci attr_reader :pipeline, :project - def parent_auto_cancelable_pipelines - project.all_pipelines + def paginator + page = 1 + Enumerator.new do |yielder| + loop do + # leverage the index_ci_pipelines_on_project_id_and_status_and_created_at index + records = project.all_pipelines + .created_after(1.week.ago) + .order(:status, :created_at) + .page(page) # use offset pagination because there is no other way to loop over the data + .per(PAGE_SIZE) + .pluck(:id) + + raise StopIteration if records.empty? + + yielder << records + page += 1 + end + end + end + + def parent_auto_cancelable_pipelines(ids = nil) + scope = project.all_pipelines .created_after(1.week.ago) .for_ref(pipeline.ref) .where_not_sha(project.commit(pipeline.ref).try(:id)) .where("created_at < ?", pipeline.created_at) .ci_sources + + scope = scope.id_in(ids) if ids.present? + scope end - def parent_and_child_pipelines - Ci::Pipeline.object_hierarchy(parent_auto_cancelable_pipelines, project_condition: :same) + def parent_and_child_pipelines(ids = nil) + Ci::Pipeline.object_hierarchy(parent_auto_cancelable_pipelines(ids), project_condition: :same) .base_and_descendants .alive_or_scheduled end @@ -59,12 +94,23 @@ module Ci ) # cascade_to_children not needed because we iterate through descendants here - cancelable_pipeline.cancel_running( + ::Ci::CancelPipelineService.new( + pipeline: cancelable_pipeline, + current_user: nil, auto_canceled_by_pipeline_id: pipeline.id, cascade_to_children: false - ) + ).force_execute end end + + # Finding the pipelines to cancel is an expensive task that is not well + # covered by indexes for all project use-cases and sometimes it might + # harm other services. See https://gitlab.com/gitlab-com/gl-infra/production/-/issues/14758 + # This feature flag is in place to disable this feature for rogue projects. + # + def service_disabled? + Feature.enabled?(:disable_cancel_redundant_pipelines_service, project, type: :ops) + end end end end diff --git a/app/services/ci/pipeline_processing/atomic_processing_service.rb b/app/services/ci/pipeline_processing/atomic_processing_service.rb index 1094a131e68..c0ffbb401f6 100644 --- a/app/services/ci/pipeline_processing/atomic_processing_service.rb +++ b/app/services/ci/pipeline_processing/atomic_processing_service.rb @@ -22,9 +22,19 @@ module Ci # Run the process only if we can obtain an exclusive lease; returns nil if lease is unavailable success = try_obtain_lease { process! } - # Re-schedule if we need further processing - if success && pipeline.needs_processing? - PipelineProcessWorker.perform_async(pipeline.id) + if success + if ::Feature.enabled?(:ci_reset_skipped_jobs_in_atomic_processing, project) + # If any jobs changed from stopped to alive status during pipeline processing, we must + # re-reset their dependent jobs; see https://gitlab.com/gitlab-org/gitlab/-/issues/388539. + new_alive_jobs.group_by(&:user).each do |user, jobs| + log_running_reset_skipped_jobs_service(jobs) + + ResetSkippedJobsService.new(project, user).execute(jobs) + end + end + + # Re-schedule if we need further processing + PipelineProcessWorker.perform_async(pipeline.id) if pipeline.needs_processing? end success @@ -105,6 +115,25 @@ module Ci end end + # Gets the jobs that changed from stopped to alive status since the initial status collection + # was evaluated. We determine this by checking if their current status is no longer stopped. + def new_alive_jobs + initial_stopped_job_names = @collection.stopped_job_names + + return [] if initial_stopped_job_names.empty? + + new_collection = AtomicProcessingService::StatusCollection.new(pipeline) + new_alive_job_names = initial_stopped_job_names - new_collection.stopped_job_names + + return [] if new_alive_job_names.empty? + + pipeline + .current_jobs + .by_name(new_alive_job_names) + .preload(:user) # rubocop: disable CodeReuse/ActiveRecord + .to_a + end + def project pipeline.project end @@ -116,6 +145,17 @@ module Ci def lease_timeout DEFAULT_LEASE_TIMEOUT end + + def log_running_reset_skipped_jobs_service(jobs) + Gitlab::AppJsonLogger.info( + class: self.class.name.to_s, + message: 'Running ResetSkippedJobsService on new alive jobs', + project_id: project.id, + pipeline_id: pipeline.id, + user_id: jobs.first.user.id, + jobs_count: jobs.count + ) + end end end end diff --git a/app/services/ci/pipeline_processing/atomic_processing_service/status_collection.rb b/app/services/ci/pipeline_processing/atomic_processing_service/status_collection.rb index 85646b79254..9a53c6d8fc1 100644 --- a/app/services/ci/pipeline_processing/atomic_processing_service/status_collection.rb +++ b/app/services/ci/pipeline_processing/atomic_processing_service/status_collection.rb @@ -67,6 +67,11 @@ module Ci all_jobs.lazy.reject { |job| job[:processed] } end + # This method returns the names of jobs that have a stopped status + def stopped_job_names + all_jobs.select { |job| job[:status].in?(Ci::HasStatus::STOPPED_STATUSES) }.pluck(:name) # rubocop: disable CodeReuse/ActiveRecord + end + private # We use these columns to perform an efficient calculation of a status diff --git a/app/services/ci/pipelines/add_job_service.rb b/app/services/ci/pipelines/add_job_service.rb index 1a5c8d0dccf..dfbb37cf0dc 100644 --- a/app/services/ci/pipelines/add_job_service.rb +++ b/app/services/ci/pipelines/add_job_service.rb @@ -18,12 +18,6 @@ module Ci in_lock("ci:pipelines:#{pipeline.id}:add-job", ttl: LOCK_TIMEOUT, sleep_sec: LOCK_SLEEP, retries: LOCK_RETRIES) do Ci::Pipeline.transaction do - # This is used to reduce the deadlocks when partitioning `ci_builds` - # since inserting into this table requires locks on all foreign keys - # and we need to lock all the tables in a specific order for the - # migration to succeed. - Ci::Pipeline.connection.execute('LOCK "ci_pipelines", "ci_stages" IN ROW SHARE MODE;') - yield(job) job.update_older_statuses_retried! diff --git a/app/services/ci/reset_skipped_jobs_service.rb b/app/services/ci/reset_skipped_jobs_service.rb index cb793eb3e06..9e5c887b31b 100644 --- a/app/services/ci/reset_skipped_jobs_service.rb +++ b/app/services/ci/reset_skipped_jobs_service.rb @@ -7,7 +7,6 @@ module Ci def execute(processables) @processables = Array.wrap(processables) @pipeline = @processables.first.pipeline - @processable = @processables.first # Remove with FF `ci_support_reset_skipped_jobs_for_multiple_jobs` process_subsequent_jobs reset_source_bridge @@ -43,27 +42,17 @@ module Ci end def stage_dependent_jobs - if ::Feature.enabled?(:ci_support_reset_skipped_jobs_for_multiple_jobs, project) - # Get all jobs after the earliest stage of the inputted jobs - min_stage_idx = @processables.map(&:stage_idx).min - @pipeline.processables.after_stage(min_stage_idx) - else - @pipeline.processables.after_stage(@processable.stage_idx) - end + # Get all jobs after the earliest stage of the inputted jobs + min_stage_idx = @processables.map(&:stage_idx).min + @pipeline.processables.after_stage(min_stage_idx) end def needs_dependent_jobs - if ::Feature.enabled?(:ci_support_reset_skipped_jobs_for_multiple_jobs, project) - # We must include the hierarchy base here because @processables may include both a parent job - # and its dependents, and we do not want to exclude those dependents from being processed. - ::Gitlab::Ci::ProcessableObjectHierarchy.new( - ::Ci::Processable.where(id: @processables.map(&:id)) - ).base_and_descendants - else - ::Gitlab::Ci::ProcessableObjectHierarchy.new( - ::Ci::Processable.where(id: @processable.id) - ).descendants - end + # We must include the hierarchy base here because @processables may include both a parent job + # and its dependents, and we do not want to exclude those dependents from being processed. + ::Gitlab::Ci::ProcessableObjectHierarchy.new( + ::Ci::Processable.where(id: @processables.map(&:id)) + ).base_and_descendants end def ordered_by_dag(jobs) diff --git a/app/services/ci/runners/assign_runner_service.rb b/app/services/ci/runners/assign_runner_service.rb index 290f945cc72..4e7b08bdd7a 100644 --- a/app/services/ci/runners/assign_runner_service.rb +++ b/app/services/ci/runners/assign_runner_service.rb @@ -17,6 +17,10 @@ module Ci return ServiceResponse.error(message: 'user not allowed to assign runner', http_status: :forbidden) end + unless @user.can?(:register_project_runners, @project) + return ServiceResponse.error(message: 'user not allowed to add runners to project', http_status: :forbidden) + end + if @runner.assign_to(@project, @user) ServiceResponse.success else diff --git a/app/services/ci/runners/stale_managers_cleanup_service.rb b/app/services/ci/runners/stale_managers_cleanup_service.rb index b39f7315bc6..e216d8ea3d6 100644 --- a/app/services/ci/runners/stale_managers_cleanup_service.rb +++ b/app/services/ci/runners/stale_managers_cleanup_service.rb @@ -4,29 +4,32 @@ module Ci module Runners class StaleManagersCleanupService MAX_DELETIONS = 1000 + SUB_BATCH_LIMIT = 100 def execute - ServiceResponse.success(payload: { - # the `stale` relationship can return duplicates, so we don't try to return a precise count here - deleted_managers: delete_stale_runner_managers > 0 - }) + ServiceResponse.success(payload: delete_stale_runner_managers) end private def delete_stale_runner_managers + batch_counts = [] total_deleted_count = 0 loop do - sub_batch_limit = [100, MAX_DELETIONS].min + sub_batch_limit = [SUB_BATCH_LIMIT, MAX_DELETIONS].min - # delete_all discards part of the `stale` scope query, so we expliclitly wrap it with a SELECT as a workaround + # delete_all discards part of the `stale` scope query, so we explicitly wrap it with a SELECT as a workaround deleted_count = Ci::RunnerManager.id_in(Ci::RunnerManager.stale.limit(sub_batch_limit)).delete_all + batch_counts << deleted_count total_deleted_count += deleted_count break if deleted_count == 0 || total_deleted_count >= MAX_DELETIONS end - total_deleted_count + { + total_deleted: total_deleted_count, + batch_counts: batch_counts + } end end end diff --git a/app/services/clusters/agent_tokens/create_service.rb b/app/services/clusters/agent_tokens/create_service.rb index 66a3cb04d98..efa9716d2c8 100644 --- a/app/services/clusters/agent_tokens/create_service.rb +++ b/app/services/clusters/agent_tokens/create_service.rb @@ -4,6 +4,7 @@ module Clusters module AgentTokens class CreateService ALLOWED_PARAMS = %i[agent_id description name].freeze + ACTIVE_TOKENS_LIMIT = 2 attr_reader :agent, :current_user, :params @@ -15,6 +16,7 @@ module Clusters def execute return error_no_permissions unless current_user.can?(:create_cluster, agent.project) + return error_active_tokens_limit_reached if active_tokens_limit_reached? token = ::Clusters::AgentToken.new(filtered_params.merge(agent_id: agent.id, created_by_user: current_user)) @@ -33,6 +35,16 @@ module Clusters ServiceResponse.error(message: s_('ClusterAgent|User has insufficient permissions to create a token for this project')) end + def error_active_tokens_limit_reached + ServiceResponse.error(message: s_('ClusterAgent|An agent can have only two active tokens at a time')) + end + + def active_tokens_limit_reached? + return false unless Feature.enabled?(:cluster_agents_limit_tokens_created) + + ::Clusters::AgentTokensFinder.new(agent, current_user, status: :active).execute.count >= ACTIVE_TOKENS_LIMIT + end + def filtered_params params.slice(*ALLOWED_PARAMS) end diff --git a/app/services/commits/cherry_pick_service.rb b/app/services/commits/cherry_pick_service.rb index 7e982bf7686..2a634c5ec71 100644 --- a/app/services/commits/cherry_pick_service.rb +++ b/app/services/commits/cherry_pick_service.rb @@ -2,9 +2,18 @@ module Commits class CherryPickService < ChangeService + def initialize(*args) + super + + @start_project = params[:target_project] || @project + @source_project = params[:source_project] || @project + end + def create_commit! - commit_change(:cherry_pick).tap do |sha| - track_mr_picking(sha) + Gitlab::Git::CrossRepo.new(@project.repository, @source_project.repository).execute(@commit.id) do + commit_change(:cherry_pick).tap do |sha| + track_mr_picking(sha) + end end end diff --git a/app/services/concerns/search/filter.rb b/app/services/concerns/search/filter.rb new file mode 100644 index 00000000000..e234edcfce4 --- /dev/null +++ b/app/services/concerns/search/filter.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Search + module Filter + private + + def filters + { state: params[:state], confidential: params[:confidential], include_archived: params[:include_archived] } + end + end +end + +Search::Filter.prepend_mod diff --git a/app/services/concerns/update_repository_storage_methods.rb b/app/services/concerns/update_repository_storage_methods.rb index a0b4040cff7..bb43cab79bb 100644 --- a/app/services/concerns/update_repository_storage_methods.rb +++ b/app/services/concerns/update_repository_storage_methods.rb @@ -14,12 +14,16 @@ module UpdateRepositoryStorageMethods end def execute - repository_storage_move.with_lock do - return ServiceResponse.success unless repository_storage_move.scheduled? # rubocop:disable Cop/AvoidReturnFromBlocks + response = repository_storage_move.with_lock do + next ServiceResponse.success unless repository_storage_move.scheduled? repository_storage_move.start! + + nil end + return response if response + mirror_repositories unless same_filesystem? repository_storage_move.transaction do diff --git a/app/services/database/mark_migration_service.rb b/app/services/database/mark_migration_service.rb new file mode 100644 index 00000000000..aff10fa5f76 --- /dev/null +++ b/app/services/database/mark_migration_service.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module Database + class MarkMigrationService + def initialize(connection:, version:) + @connection = connection + @version = version + end + + def execute + return error(reason: :not_found) unless migration.present? + return error(reason: :invalid) if all_versions.include?(migration.version) + + if create_version(version) + ServiceResponse.success + else + error(reason: :invalid) + end + end + + private + + attr_reader :connection, :version + + def migration + @migration ||= connection + .migration_context + .migrations + .find { |migration| migration.version == version } + end + + def all_versions + all_executed_migrations.map(&:to_i) + end + + def all_executed_migrations + sm = Arel::SelectManager.new(arel_table) + sm.project(arel_table[:version]) + sm.order(arel_table[:version].asc) # rubocop: disable CodeReuse/ActiveRecord + connection.select_values(sm, "#{self.class} Load") + end + + def create_version(version) + im = Arel::InsertManager.new + im.into(arel_table) + im.insert(arel_table[:version] => version) + connection.insert(im, "#{self.class} Create", :version, version) + end + + def arel_table + @arel_table ||= Arel::Table.new(:schema_migrations) + end + + def error(reason:) + ServiceResponse.error(message: 'error', reason: reason) + end + end +end diff --git a/app/services/environments/create_service.rb b/app/services/environments/create_service.rb new file mode 100644 index 00000000000..760c8a6e306 --- /dev/null +++ b/app/services/environments/create_service.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module Environments + class CreateService < BaseService + ALLOWED_ATTRIBUTES = %i[name external_url tier cluster_agent].freeze + + def execute + unless can?(current_user, :create_environment, project) + return ServiceResponse.error( + message: _('Unauthorized to create an environment'), + payload: { environment: nil } + ) + end + + if unauthorized_cluster_agent? + return ServiceResponse.error( + message: _('Unauthorized to access the cluster agent in this project'), + payload: { environment: nil }) + end + + environment = project.environments.create(**params.slice(*ALLOWED_ATTRIBUTES)) + + if environment.persisted? + ServiceResponse.success(payload: { environment: environment }) + else + ServiceResponse.error( + message: environment.errors.full_messages, + payload: { environment: nil } + ) + end + end + + private + + def unauthorized_cluster_agent? + return false unless params[:cluster_agent] + + ::Clusters::Agents::Authorizations::UserAccess::Finder + .new(current_user, agent: params[:cluster_agent], project: project) + .execute + .empty? + end + end +end diff --git a/app/services/environments/destroy_service.rb b/app/services/environments/destroy_service.rb new file mode 100644 index 00000000000..f1530489a40 --- /dev/null +++ b/app/services/environments/destroy_service.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Environments + class DestroyService < BaseService + def execute(environment) + unless can?(current_user, :destroy_environment, environment) + return ServiceResponse.error( + message: 'Unauthorized to delete the environment' + ) + end + + environment.destroy + + unless environment.destroyed? + return ServiceResponse.error( + message: 'Attemped to destroy the environment but failed' + ) + end + + ServiceResponse.success + end + end +end diff --git a/app/services/environments/update_service.rb b/app/services/environments/update_service.rb new file mode 100644 index 00000000000..5eb4880ec4b --- /dev/null +++ b/app/services/environments/update_service.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module Environments + class UpdateService < BaseService + ALLOWED_ATTRIBUTES = %i[external_url tier cluster_agent].freeze + + def execute(environment) + unless can?(current_user, :update_environment, environment) + return ServiceResponse.error( + message: _('Unauthorized to update the environment'), + payload: { environment: environment } + ) + end + + if unauthorized_cluster_agent? + return ServiceResponse.error( + message: _('Unauthorized to access the cluster agent in this project'), + payload: { environment: environment }) + end + + if environment.update(**params.slice(*ALLOWED_ATTRIBUTES)) + ServiceResponse.success(payload: { environment: environment }) + else + ServiceResponse.error( + message: environment.errors.full_messages, + payload: { environment: environment } + ) + end + end + + private + + def unauthorized_cluster_agent? + return false unless params[:cluster_agent] + + ::Clusters::Agents::Authorizations::UserAccess::Finder + .new(current_user, agent: params[:cluster_agent], project: project) + .execute + .empty? + end + end +end diff --git a/app/services/error_tracking/collect_error_service.rb b/app/services/error_tracking/collect_error_service.rb deleted file mode 100644 index 8cb3793ba97..00000000000 --- a/app/services/error_tracking/collect_error_service.rb +++ /dev/null @@ -1,82 +0,0 @@ -# frozen_string_literal: true - -module ErrorTracking - class CollectErrorService < ::BaseService - include Gitlab::Utils::StrongMemoize - - def execute - error_repository.report_error( - name: exception['type'], - description: exception['value'], - actor: actor, - platform: event['platform'], - occurred_at: timestamp, - environment: event['environment'], - level: event['level'], - payload: event - ) - end - - private - - def error_repository - Gitlab::ErrorTracking::ErrorRepository.build(project) - end - - def event - @event ||= format_event(params[:event]) - end - - def format_event(event) - # Some SDK send exception payload as Array. For exmple Go lang SDK. - # We need to convert it to hash format we expect. - if event['exception'].is_a?(Array) - exception = event['exception'] - event['exception'] = { 'values' => exception } - end - - event - end - - def exception - strong_memoize(:exception) do - # Find the first exception that has a stacktrace since the first - # exception may not provide adequate context (e.g. in the Go SDK). - entries = event['exception']['values'] - entries.find { |x| x.key?('stacktrace') } || entries.first - end - end - - def stacktrace_frames - strong_memoize(:stacktrace_frames) do - exception.dig('stacktrace', 'frames') - end - end - - def actor - return event['transaction'] if event['transaction'].present? - - # Some SDKs do not have a transaction attribute. - # So we build it by combining function name and module name from - # the last item in stacktrace. - return unless stacktrace_frames.present? - - last_line = stacktrace_frames.last - - "#{last_line['function']}(#{last_line['module']})" - end - - def timestamp - return @timestamp if @timestamp - - @timestamp = (event['timestamp'] || Time.zone.now) - - # Some SDK send timestamp in numeric format like '1630945472.13'. - if @timestamp.to_s =~ /\A\d+(\.\d+)?\z/ - @timestamp = Time.zone.at(@timestamp.to_f) - end - - @timestamp - end - end -end diff --git a/app/services/feature_flags/base_service.rb b/app/services/feature_flags/base_service.rb index 028906a0b43..834409bf3c4 100644 --- a/app/services/feature_flags/base_service.rb +++ b/app/services/feature_flags/base_service.rb @@ -39,9 +39,9 @@ module FeatureFlags def created_strategy_message(strategy) scopes = strategy.scopes - .map { |scope| %Q("#{scope.environment_scope}") } + .map { |scope| %("#{scope.environment_scope}") } .join(', ') - %Q(Created strategy "#{strategy.name}" with scopes #{scopes}.) + %(Created strategy "#{strategy.name}" with scopes #{scopes}.) end def feature_flag_by_name diff --git a/app/services/git/branch_hooks_service.rb b/app/services/git/branch_hooks_service.rb index 2ead2e2a113..31da099d078 100644 --- a/app/services/git/branch_hooks_service.rb +++ b/app/services/git/branch_hooks_service.rb @@ -4,6 +4,9 @@ module Git class BranchHooksService < ::Git::BaseHooksService extend ::Gitlab::Utils::Override + JIRA_SYNC_BATCH_SIZE = 20 + JIRA_SYNC_BATCH_DELAY = 10.seconds + def execute execute_branch_hooks @@ -99,7 +102,6 @@ module Git def branch_change_hooks enqueue_process_commit_messages enqueue_jira_connect_sync_messages - enqueue_metrics_dashboard_sync track_ci_config_change_event end @@ -107,13 +109,6 @@ module Git project.repository.after_remove_branch(expire_cache: false) end - def enqueue_metrics_dashboard_sync - return unless default_branch? - return unless modified_file_types.include?(:metrics_dashboard) - - ::Metrics::Dashboard::SyncDashboardsWorker.perform_async(project.id) - end - def track_ci_config_change_event return unless ::ServicePing::ServicePingSettings.enabled? return unless default_branch? @@ -157,13 +152,34 @@ module Git return unless project.jira_subscription_exists? branch_to_sync = branch_name if Atlassian::JiraIssueKeyExtractors::Branch.has_keys?(project, branch_name) - commits_to_sync = limited_commits.select { |commit| Atlassian::JiraIssueKeyExtractor.has_keys?(commit.safe_message) }.map(&:sha) - - if branch_to_sync || commits_to_sync.any? - JiraConnect::SyncBranchWorker.perform_async(project.id, branch_to_sync, commits_to_sync, Atlassian::JiraConnect::Client.generate_update_sequence_id) + commits_to_sync = filtered_commit_shas + + return if branch_to_sync.nil? && commits_to_sync.empty? + + if commits_to_sync.any? && Feature.enabled?(:batch_delay_jira_branch_sync_worker, project) + commits_to_sync.each_slice(JIRA_SYNC_BATCH_SIZE).with_index do |commits, i| + JiraConnect::SyncBranchWorker.perform_in( + JIRA_SYNC_BATCH_DELAY * i, + project.id, + branch_to_sync, + commits, + Atlassian::JiraConnect::Client.generate_update_sequence_id + ) + end + else + JiraConnect::SyncBranchWorker.perform_async( + project.id, + branch_to_sync, + commits_to_sync, + Atlassian::JiraConnect::Client.generate_update_sequence_id + ) end end + def filtered_commit_shas + limited_commits.select { |commit| Atlassian::JiraIssueKeyExtractor.has_keys?(commit.safe_message) }.map(&:sha) + end + def signature_types [ ::CommitSignatures::GpgSignature, diff --git a/app/services/google_cloud/enable_vision_ai_service.rb b/app/services/google_cloud/enable_vision_ai_service.rb new file mode 100644 index 00000000000..f7adea706ed --- /dev/null +++ b/app/services/google_cloud/enable_vision_ai_service.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module GoogleCloud + class EnableVisionAiService < ::GoogleCloud::BaseService + def execute + gcp_project_ids = unique_gcp_project_ids + + if gcp_project_ids.empty? + error("No GCP projects found. Configure a service account or GCP_PROJECT_ID ci variable.") + else + gcp_project_ids.each do |gcp_project_id| + google_api_client.enable_vision_api(gcp_project_id) + end + + success({ gcp_project_ids: gcp_project_ids }) + end + end + end +end diff --git a/app/services/google_cloud/generate_pipeline_service.rb b/app/services/google_cloud/generate_pipeline_service.rb index 791be69f4d4..95de1fa21b7 100644 --- a/app/services/google_cloud/generate_pipeline_service.rb +++ b/app/services/google_cloud/generate_pipeline_service.rb @@ -4,6 +4,7 @@ module GoogleCloud class GeneratePipelineService < ::GoogleCloud::BaseService ACTION_DEPLOY_TO_CLOUD_RUN = 'DEPLOY_TO_CLOUD_RUN' ACTION_DEPLOY_TO_CLOUD_STORAGE = 'DEPLOY_TO_CLOUD_STORAGE' + ACTION_VISION_AI_PIPELINE = 'VISION_AI_PIPELINE' def execute commit_attributes = generate_commit_attributes @@ -53,6 +54,15 @@ module GoogleCloud branch_name: branch_name, start_branch: branch_name } + when ACTION_VISION_AI_PIPELINE + branch_name = "vision-ai-pipeline-#{SecureRandom.hex(8)}" + { + commit_message: 'Enable Vision AI Pipeline', + file_path: '.gitlab-ci.yml', + file_content: pipeline_content('gcp/vision-ai.gitlab-ci.yml'), + branch_name: branch_name, + start_branch: branch_name + } end end @@ -67,7 +77,11 @@ module GoogleCloud def append_remote_include(gitlab_ci_yml, include_url) stages = gitlab_ci_yml['stages'] || [] - gitlab_ci_yml['stages'] = (stages + %w[build test deploy]).uniq + gitlab_ci_yml['stages'] = if action == ACTION_VISION_AI_PIPELINE + (stages + %w[validate detect render]).uniq + else + (stages + %w[build test deploy]).uniq + end includes = gitlab_ci_yml['include'] || [] includes = Array.wrap(includes) diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index 1c8df157716..16454360ee2 100644 --- a/app/services/groups/transfer_service.rb +++ b/app/services/groups/transfer_service.rb @@ -69,7 +69,7 @@ module Groups return false if group.root_ancestor == @new_parent_group.root_ancestor return true if group.contacts.exists? && !current_user.can?(:admin_crm_contact, @new_parent_group.root_ancestor) - return true if group.organizations.exists? && !current_user.can?(:admin_crm_organization, @new_parent_group.root_ancestor) + return true if group.crm_organizations.exists? && !current_user.can?(:admin_crm_organization, @new_parent_group.root_ancestor) false end diff --git a/app/services/import_csv/base_service.rb b/app/services/import_csv/base_service.rb index 70834b8a85a..cfaf3e831eb 100644 --- a/app/services/import_csv/base_service.rb +++ b/app/services/import_csv/base_service.rb @@ -103,16 +103,12 @@ module ImportCsv strong_memoize_attr :detect_col_sep def create_object(attributes) - # NOTE: CSV imports are performed by workers, so we do not have a request context in order - # to create a SpamParams object to pass to the issuable create service. - spam_params = nil - # default_params can be extracted into a method if we need # to support creation of objects that belongs to groups. default_params = { container: project, current_user: user, params: attributes, - spam_params: spam_params } + perform_spam_check: false } create_service = create_object_class.new(**default_params.merge(extra_create_service_params)) diff --git a/app/services/incident_management/incidents/create_service.rb b/app/services/incident_management/incidents/create_service.rb index a75c5d2e75c..fe0b41a3a31 100644 --- a/app/services/incident_management/incidents/create_service.rb +++ b/app/services/incident_management/incidents/create_service.rb @@ -25,7 +25,7 @@ module IncidentManagement severity: severity, alert_management_alerts: [alert].compact }, - spam_params: nil + perform_spam_check: false ).execute if alert diff --git a/app/services/integrations/slack_interactions/incident_management/incident_modal_submit_service.rb b/app/services/integrations/slack_interactions/incident_management/incident_modal_submit_service.rb index 34af03640d3..e9d86e9228d 100644 --- a/app/services/integrations/slack_interactions/incident_management/incident_modal_submit_service.rb +++ b/app/services/integrations/slack_interactions/incident_management/incident_modal_submit_service.rb @@ -22,7 +22,7 @@ module Integrations container: project, current_user: find_user.user, params: incident_params, - spam_params: nil + perform_spam_check: false ).execute raise IssueCreateError, create_response.errors.to_sentence if create_response.error? diff --git a/app/services/issuable/callbacks/base.rb b/app/services/issuable/callbacks/base.rb index 3fabce2c949..368dd76c16c 100644 --- a/app/services/issuable/callbacks/base.rb +++ b/app/services/issuable/callbacks/base.rb @@ -12,6 +12,7 @@ module Issuable end def after_initialize; end + def before_update; end def after_update_commit; end def after_save_commit; end diff --git a/app/services/issuable/destroy_service.rb b/app/services/issuable/destroy_service.rb index 261afb767bb..47770d101f9 100644 --- a/app/services/issuable/destroy_service.rb +++ b/app/services/issuable/destroy_service.rb @@ -8,11 +8,15 @@ module Issuable end def execute(issuable) + before_destroy(issuable) after_destroy(issuable) if issuable.destroy end private + # overriden in EE + def before_destroy(issuable); end + def after_destroy(issuable) delete_associated_records(issuable) issuable.update_project_counter_caches diff --git a/app/services/issuable/discussions_list_service.rb b/app/services/issuable/discussions_list_service.rb index cb9271de11d..83efbf65b92 100644 --- a/app/services/issuable/discussions_list_service.rb +++ b/app/services/issuable/discussions_list_service.rb @@ -4,7 +4,6 @@ # System notes also have a discussion ID assigned including Synthetic system notes. module Issuable class DiscussionsListService - include RendersNotes include Gitlab::Utils::StrongMemoize attr_reader :current_user, :issuable, :params @@ -37,7 +36,9 @@ module Issuable ).execute(notes) end - notes = prepare_notes_for_rendering(notes) + # Here we assume all notes belong to the same project as the work item + project = notes.first&.project + notes = ::Preloaders::Projects::NotesPreloader.new(project, current_user).call(notes) # we need to check the permission on every note, because some system notes for instance can have references to # resources that some user do not have read access, so those notes are filtered out from the list of notes. diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index e9312bd6b31..3b007d4dba7 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -314,16 +314,19 @@ class IssuableBaseService < ::BaseContainerService before_update(issuable) - # Do not touch when saving the issuable if only changes position within a list. We should call - # this method at this point to capture all possible changes. - should_touch = update_timestamp?(issuable) - - issuable.updated_by = current_user if should_touch # We have to perform this check before saving the issuable as Rails resets # the changed fields upon calling #save. update_project_counters = issuable.project && update_project_counter_caches?(issuable) issuable_saved = issuable.with_transaction_returning_status do + @callbacks.each(&:before_update) + + # Do not touch when saving the issuable if only changes position within a list. We should call + # this method at this point to capture all possible changes. + should_touch = update_timestamp?(issuable) + + issuable.updated_by = current_user if should_touch + transaction_update(issuable, { save_with_touch: should_touch }) end diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index efe42fb29d5..f982d66eb08 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -124,6 +124,10 @@ module Issues def update_project_counter_caches?(issue) super || issue.confidential_changed? end + + def log_audit_event(issue, user, event_type, message) + # defined in EE + end end end diff --git a/app/services/issues/clone_service.rb b/app/services/issues/clone_service.rb index c2a724254a7..8af44fb1e3c 100644 --- a/app/services/issues/clone_service.rb +++ b/app/services/issues/clone_service.rb @@ -68,9 +68,7 @@ module Issues new_params.delete(:created_at) new_params.delete(:updated_at) - # spam checking is not necessary, as no new content is being created. Passing nil for - # spam_params will cause SpamActionService to skip checking and return a success response. - spam_params = nil + # spam checking is not necessary, as no new content is being created. # Skip creation of system notes for existing attributes of the issue when cloning with notes. # The system notes of the old issue are copied over so we don't want to end up with duplicate notes. @@ -79,7 +77,7 @@ module Issues container: target_project, current_user: current_user, params: new_params, - spam_params: spam_params + perform_spam_check: false ).execute(skip_system_notes: with_notes) raise CloneError, create_result.errors.join(', ') if create_result.error? && create_result[:issue].blank? diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index e45033f2b91..f848a8db12a 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -28,6 +28,11 @@ module Issues event_service.close_issue(issue, current_user) create_note(issue, closed_via) if system_note + if current_user.project_bot? + log_audit_event(issue, current_user, "#{issue.issue_type}_closed_by_project_bot", + "Closed #{issue.issue_type.humanize(capitalize: false)} #{issue.title}") + end + closed_via = _("commit %{commit_id}") % { commit_id: closed_via.id } if closed_via.is_a?(Commit) notification_service.async.close_issue(issue, current_user, { closed_via: closed_via }) if notifications diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index ba8f00d03d4..17b6866773e 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -9,14 +9,10 @@ module Issues rate_limit key: :issues_create, opts: { scope: [:project, :current_user, :external_author] } - # NOTE: For Issues::CreateService, we require the spam_params and do not default it to nil, because - # spam_checking is likely to be necessary. However, if there is not a request available in scope - # in the caller (for example, an issue created via email) and the required arguments to the - # SpamParams constructor are not otherwise available, spam_params: must be explicitly passed as nil. - def initialize(container:, spam_params:, current_user: nil, params: {}, build_service: nil) + def initialize(container:, current_user: nil, params: {}, build_service: nil, perform_spam_check: true) @extra_params = params.delete(:extra_params) || {} super(container: container, current_user: current_user, params: params) - @spam_params = spam_params + @perform_spam_check = perform_spam_check @build_service = build_service || BuildService.new(container: project, current_user: current_user, params: params) end @@ -51,12 +47,7 @@ module Issues end def before_create(issue) - Spam::SpamActionService.new( - spammable: issue, - spam_params: spam_params, - user: current_user, - action: :create - ).execute + issue.check_for_spam(user: current_user, action: :create) if perform_spam_check # current_user (defined in BaseService) is not available within run_after_commit block user = current_user @@ -109,7 +100,7 @@ module Issues :create_issue end - attr_reader :spam_params, :extra_params + attr_reader :perform_spam_check, :extra_params def create_timeline_event(issue) return unless issue.work_item_type&.incident? @@ -118,7 +109,7 @@ module Issues end def user_agent_detail_service - UserAgentDetailService.new(spammable: @issue, spam_params: spam_params) + UserAgentDetailService.new(spammable: @issue, perform_spam_check: perform_spam_check) end def handle_add_related_issue(issue) diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index a2180dabdea..c1599ceef6e 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -90,9 +90,7 @@ module Issues new_params = original_entity.serializable_hash.symbolize_keys.merge(new_params) new_params = new_params.merge(rewritten_old_entity_attributes) - # spam checking is not necessary, as no new content is being created. Passing nil for - # spam_params will cause SpamActionService to skip checking and return a success response. - spam_params = nil + # spam checking is not necessary, as no new content is being created. # Skip creation of system notes for existing attributes of the issue. The system notes of the old # issue are copied over so we don't want to end up with duplicate notes. @@ -100,7 +98,7 @@ module Issues container: @target_project, current_user: @current_user, params: new_params, - spam_params: spam_params + perform_spam_check: false ).execute(skip_system_notes: true) raise MoveError, create_result.errors.join(', ') if create_result.error? && create_result[:issue].blank? diff --git a/app/services/issues/reopen_service.rb b/app/services/issues/reopen_service.rb index f4d229ecec7..d71ba4e3414 100644 --- a/app/services/issues/reopen_service.rb +++ b/app/services/issues/reopen_service.rb @@ -7,6 +7,12 @@ module Issues if issue.reopen event_service.reopen_issue(issue, current_user) + + if current_user.project_bot? + log_audit_event(issue, current_user, "#{issue.issue_type}_reopened_by_project_bot", + "Reopened #{issue.issue_type.humanize(capitalize: false)} #{issue.title}") + end + create_note(issue, 'reopened') notification_service.async.reopen_issue(issue, current_user) perform_incident_management_actions(issue) diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 201bf19b535..7ad56d5a755 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -2,12 +2,12 @@ module Issues class UpdateService < Issues::BaseService - # NOTE: For Issues::UpdateService, we default the spam_params to nil, because spam_checking is not - # necessary in many cases, and we don't want to require every caller to explicitly pass it as nil + # NOTE: For Issues::UpdateService, we default perform_spam_check to false, because spam_checking is not + # necessary in many cases, and we don't want to require every caller to explicitly pass it # to disable spam checking. - def initialize(container:, current_user: nil, params: {}, spam_params: nil) + def initialize(container:, current_user: nil, params: {}, perform_spam_check: false) super(container: container, current_user: current_user, params: params) - @spam_params = spam_params + @perform_spam_check = perform_spam_check end def execute(issue) @@ -26,14 +26,9 @@ module Issues def before_update(issue, skip_spam_check: false) change_work_item_type(issue) - return if skip_spam_check + return if skip_spam_check || !perform_spam_check - Spam::SpamActionService.new( - spammable: issue, - spam_params: spam_params, - user: current_user, - action: :update - ).execute + issue.check_for_spam(user: current_user, action: :update) end def change_work_item_type(issue) @@ -115,7 +110,14 @@ module Issues private - attr_reader :spam_params + attr_reader :perform_spam_check + + override :after_update + def after_update(issue, _old_associations) + super + + GraphqlTriggers.work_item_updated(issue) + end def handle_date_changes(issue) return unless issue.previous_changes.slice('due_date', 'start_date').any? diff --git a/app/services/jira_connect_installations/update_service.rb b/app/services/jira_connect_installations/update_service.rb index ff5b9671e2b..d0cf614a068 100644 --- a/app/services/jira_connect_installations/update_service.rb +++ b/app/services/jira_connect_installations/update_service.rb @@ -51,7 +51,7 @@ module JiraConnectInstallations 'Could not be installed on the instance. Network error' end - ServiceResponse.error(message: { instance_url: [message] }) + ServiceResponse.error(message: message) end def update_error diff --git a/app/services/merge_requests/after_create_service.rb b/app/services/merge_requests/after_create_service.rb index f174778e12e..5c1ec5add73 100644 --- a/app/services/merge_requests/after_create_service.rb +++ b/app/services/merge_requests/after_create_service.rb @@ -7,7 +7,9 @@ module MergeRequests def execute(merge_request) merge_request.ensure_merge_request_diff + logger.info(**log_payload(merge_request, 'Executing hooks')) execute_hooks(merge_request) + logger.info(**log_payload(merge_request, 'Executed hooks')) prepare_for_mergeability(merge_request) prepare_merge_request(merge_request) @@ -17,7 +19,9 @@ module MergeRequests private def prepare_for_mergeability(merge_request) + logger.info(**log_payload(merge_request, 'Creating pipeline')) create_pipeline_for(merge_request, current_user) + logger.info(**log_payload(merge_request, 'Pipeline created')) merge_request.update_head_pipeline check_mergeability(merge_request) end @@ -58,6 +62,17 @@ module MergeRequests def mark_merge_request_as_prepared(merge_request) merge_request.update!(prepared_at: Time.current) end + + def logger + @logger ||= Gitlab::AppLogger + end + + def log_payload(merge_request, message) + Gitlab::ApplicationContext.current.merge( + merge_request_id: merge_request.id, + message: message + ) + end end end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 3a7b577d59a..b8853e8bcbc 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -334,7 +334,7 @@ module MergeRequests strong_memoize(:issue_iid) do @params_issue_iid || begin id = if target_project.external_issue_tracker - source_branch.match(target_project.external_issue_reference_pattern).try(:[], 0) + target_project.external_issue_reference_pattern.match(source_branch).try(:[], 0) end id || source_branch.match(/\A(\d+)-/).try(:[], 1) diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb index da3a9652d69..62928e05a89 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -12,6 +12,7 @@ module MergeRequests merge_request.allow_broken = true if merge_request.close + expire_unapproved_key(merge_request) create_event(merge_request) merge_request_activity_counter.track_close_mr_action(user: current_user) create_note(merge_request) @@ -40,8 +41,14 @@ module MergeRequests end end + def expire_unapproved_key(merge_request) + nil + end + def trigger_merge_request_merge_status_updated(merge_request) GraphqlTriggers.merge_request_merge_status_updated(merge_request) end end end + +MergeRequests::CloseService.prepend_mod diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 39e1594d215..9135a80c883 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -41,6 +41,7 @@ module MergeRequests # timeout, we do this before we attempt to save the merge request. merge_request.skip_ensure_merge_request_diff = true + merge_request.check_for_spam(user: current_user, action: :create) end def set_projects! diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 10301774f96..5e41375e7a0 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -160,7 +160,7 @@ module MergeRequests end def handle_merge_error(log_message:, save_message_on_model: false) - log_error("MergeService ERROR: #{merge_request_info} - #{log_message}") + log_error("MergeService ERROR: #{merge_request_info}:#{merge_status} - #{log_message}") @merge_request.update(merge_error: log_message) if save_message_on_model end @@ -186,6 +186,10 @@ module MergeRequests @merge_request_info ||= merge_request.to_reference(full: true) end + def merge_status + @merge_status ||= @merge_request.merge_status + end + def source_matches? # params-keys are symbols coming from the controller, but when they get # loaded from the database they're strings diff --git a/app/services/merge_requests/mergeability/logger.rb b/app/services/merge_requests/mergeability/logger.rb index 88ef6d81eaa..612c79f0aae 100644 --- a/app/services/merge_requests/mergeability/logger.rb +++ b/app/services/merge_requests/mergeability/logger.rb @@ -22,8 +22,8 @@ module MergeRequests result = yield + observe_result(mergeability_name, result) observe("mergeability.#{mergeability_name}.duration_s", current_monotonic_time - op_started_at) - observe_sql_counters(mergeability_name, op_start_db_counters, current_db_counter_payload) result @@ -31,7 +31,13 @@ module MergeRequests private - attr_reader :destination, :merge_request + attr_reader :destination, :merge_request, :stored_result + + def observe_result(name, result) + return unless result.respond_to?(:success?) + + observe("mergeability.#{name}.successful", result.success?) + end def observe(name, value) observations[name.to_s].push(value) diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb index d2247a6d4c1..b2e15cc7c7e 100644 --- a/app/services/merge_requests/reopen_service.rb +++ b/app/services/merge_requests/reopen_service.rb @@ -37,3 +37,5 @@ module MergeRequests end end end + +MergeRequests::ReopenService.prepend_mod diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index aaed01403cb..598dbaf93a9 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -209,6 +209,11 @@ module MergeRequests old_branch, new_branch) end + override :before_update + def before_update(merge_request, skip_spam_check: false) + merge_request.check_for_spam(user: current_user, action: :update) unless skip_spam_check + end + override :handle_quick_actions def handle_quick_actions(merge_request) super diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 7dd6cd9a87c..fdab2a07990 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -21,6 +21,8 @@ module Notes # only, there is no need be create a note! execute_quick_actions(note) do |only_commands| + note.check_for_spam(action: :create, user: current_user) unless only_commands + note.run_after_commit do # Finish the harder work in the background NewNoteWorker.perform_async(note.id) @@ -105,16 +107,10 @@ module Notes def do_commands(note, update_params, message, command_names, only_commands) return if quick_actions_service.commands_executed_count.to_i == 0 - if update_params.present? - invalid_message = validate_commands(note, update_params) - - if invalid_message - note.errors.add(:validation, invalid_message) - message = invalid_message - else - quick_actions_service.apply_updates(update_params, note) - note.commands_changes = update_params - end + update_error = quick_actions_update_errors(note, update_params) + if update_error + note.errors.add(:validation, update_error) + message = update_error end # We must add the error after we call #save because errors are reset @@ -127,6 +123,19 @@ module Notes end end + def quick_actions_update_errors(note, params) + return unless params.present? + + invalid_message = validate_commands(note, params) + return invalid_message if invalid_message + + service_response = quick_actions_service.apply_updates(params, note) + note.commands_changes = params + return if service_response.success? + + service_response.message.join(', ') + end + def quick_action_options { merge_request_diff_head_sha: params[:merge_request_diff_head_sha], diff --git a/app/services/notes/quick_actions_service.rb b/app/services/notes/quick_actions_service.rb index 38f7a23ce29..cba7398ebc0 100644 --- a/app/services/notes/quick_actions_service.rb +++ b/app/services/notes/quick_actions_service.rb @@ -50,7 +50,21 @@ module Notes update_params[:spend_time][:note_id] = note.id end - noteable_update_service(note, update_params).execute(note.noteable) + execute_update_service(note, update_params) + end + + private + + def execute_update_service(note, params) + service_response = noteable_update_service(note, params).execute(note.noteable) + + service_errors = if service_response.respond_to?(:errors) + service_response.errors.full_messages + elsif service_response.respond_to?(:[]) && service_response[:status] == :error + service_response[:message] + end + + service_errors.blank? ? ServiceResponse.success : ServiceResponse.error(message: service_errors) end def noteable_update_service(note, update_params) diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb index e04891da7f8..52940281018 100644 --- a/app/services/notes/update_service.rb +++ b/app/services/notes/update_service.rb @@ -9,6 +9,8 @@ module Notes note.assign_attributes(params) + return note unless note.valid? + track_note_edit_usage_for_issues(note) if note.for_issue? track_note_edit_usage_for_merge_requests(note) if note.for_merge_request? @@ -23,10 +25,7 @@ module Notes note.note = content end - if note.note_changed? - note.assign_attributes(last_edited_at: Time.current, updated_by: current_user) - end - + update_note(note, only_commands) note.save unless only_commands || note.for_personal_snippet? @@ -45,7 +44,6 @@ module Notes if only_commands delete_note(note, message) - note = nil else note.save end @@ -56,6 +54,13 @@ module Notes private + def update_note(note, only_commands) + return unless note.note_changed? + + note.assign_attributes(last_edited_at: Time.current, updated_by: current_user) + note.check_for_spam(action: :update, user: current_user) unless only_commands + end + def delete_note(note, message) # We must add the error after we call #save because errors are reset # when #save is called diff --git a/app/services/object_storage/delete_stale_direct_uploads_service.rb b/app/services/object_storage/delete_stale_direct_uploads_service.rb new file mode 100644 index 00000000000..e9560753fc4 --- /dev/null +++ b/app/services/object_storage/delete_stale_direct_uploads_service.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module ObjectStorage + class DeleteStaleDirectUploadsService < BaseService + MAX_EXEC_DURATION = 250.seconds.freeze + + def initialize; end + + def execute + total_pending_entries = ObjectStorage::PendingDirectUpload.count + total_deleted_stale_entries = 0 + + timeout = false + start = Time.current + + ObjectStorage::PendingDirectUpload.each do |pending_upload| + if pending_upload.stale? + pending_upload.delete + total_deleted_stale_entries += 1 + end + + if (Time.current - start) > MAX_EXEC_DURATION + timeout = true + break + end + end + + success( + total_pending_entries: total_pending_entries, + total_deleted_stale_entries: total_deleted_stale_entries, + execution_timeout: timeout + ) + end + end +end diff --git a/app/services/packages/cleanup/execute_policy_service.rb b/app/services/packages/cleanup/execute_policy_service.rb index b432f6d0acb..891866bce5f 100644 --- a/app/services/packages/cleanup/execute_policy_service.rb +++ b/app/services/packages/cleanup/execute_policy_service.rb @@ -79,10 +79,9 @@ module Packages end def batch_deadline - strong_memoize(:batch_deadline) do - MAX_EXECUTION_TIME.from_now - end + MAX_EXECUTION_TIME.from_now end + strong_memoize_attr :batch_deadline def response_success(timeout:) ServiceResponse.success( diff --git a/app/services/packages/cleanup/update_policy_service.rb b/app/services/packages/cleanup/update_policy_service.rb index 6744accc007..911a060a18f 100644 --- a/app/services/packages/cleanup/update_policy_service.rb +++ b/app/services/packages/cleanup/update_policy_service.rb @@ -18,10 +18,9 @@ module Packages private def policy - strong_memoize(:policy) do - project.packages_cleanup_policy - end + project.packages_cleanup_policy end + strong_memoize_attr :policy def allowed? can?(current_user, :admin_package, project) diff --git a/app/services/packages/composer/create_package_service.rb b/app/services/packages/composer/create_package_service.rb index 0f5429f667e..ae5933fad7c 100644 --- a/app/services/packages/composer/create_package_service.rb +++ b/app/services/packages/composer/create_package_service.rb @@ -27,10 +27,9 @@ module Packages end def composer_json - strong_memoize(:composer_json) do - ::Packages::Composer::ComposerJsonService.new(project, target).execute - end + ::Packages::Composer::ComposerJsonService.new(project, target).execute end + strong_memoize_attr :composer_json def package_name composer_json['name'] diff --git a/app/services/packages/debian/create_package_file_service.rb b/app/services/packages/debian/create_package_file_service.rb index 24e40b5c986..2e9299a847c 100644 --- a/app/services/packages/debian/create_package_file_service.rb +++ b/app/services/packages/debian/create_package_file_service.rb @@ -14,7 +14,7 @@ module Packages raise ArgumentError, "Invalid user" unless current_user.present? # Debian package file are first uploaded to incoming with empty metadata, - # and are moved later by Packages::Debian::ProcessChangesService + # and are moved later by Packages::Debian::ProcessPackageFileService package_file = package.package_files.create!( file: params[:file], size: params[:file]&.size, @@ -29,14 +29,12 @@ module Packages } ) - if params[:distribution].present? && params[:component].present? + if end_of_new_upload? ::Packages::Debian::ProcessPackageFileWorker.perform_async( package_file.id, params[:distribution], params[:component] ) - elsif params[:file_name].end_with? '.changes' - ::Packages::Debian::ProcessChangesWorker.perform_async(package_file.id, current_user.id) end package_file @@ -45,6 +43,10 @@ module Packages private attr_reader :package, :current_user, :params + + def end_of_new_upload? + params[:distribution].present? || params[:file_name].end_with?('.changes') + end end end end diff --git a/app/services/packages/debian/extract_changes_metadata_service.rb b/app/services/packages/debian/extract_changes_metadata_service.rb index 43a4db5bdfc..5f06f46de58 100644 --- a/app/services/packages/debian/extract_changes_metadata_service.rb +++ b/app/services/packages/debian/extract_changes_metadata_service.rb @@ -26,10 +26,9 @@ module Packages private def metadata - strong_memoize(:metadata) do - ::Packages::Debian::ExtractMetadataService.new(@package_file).execute - end + ::Packages::Debian::ExtractMetadataService.new(@package_file).execute end + strong_memoize_attr :metadata def file_type metadata[:file_type] @@ -40,20 +39,19 @@ module Packages end def files - strong_memoize(:files) do - raise ExtractionError, "is not a changes file" unless file_type == :changes - raise ExtractionError, "Files field is missing" if fields['Files'].blank? - raise ExtractionError, "Checksums-Sha1 field is missing" if fields['Checksums-Sha1'].blank? - raise ExtractionError, "Checksums-Sha256 field is missing" if fields['Checksums-Sha256'].blank? - - init_entries_from_files - entries_from_checksums_sha1 - entries_from_checksums_sha256 - entries_from_package_files - - @entries - end + raise ExtractionError, "is not a changes file" unless file_type == :changes + raise ExtractionError, "Files field is missing" if fields['Files'].blank? + raise ExtractionError, "Checksums-Sha1 field is missing" if fields['Checksums-Sha1'].blank? + raise ExtractionError, "Checksums-Sha256 field is missing" if fields['Checksums-Sha256'].blank? + + init_entries_from_files + entries_from_checksums_sha1 + entries_from_checksums_sha256 + entries_from_package_files + + @entries end + strong_memoize_attr :files def init_entries_from_files each_lines_for('Files') do |line| @@ -101,12 +99,17 @@ module Packages def entries_from_package_files @entries.each do |filename, entry| - entry.package_file = ::Packages::PackageFileFinder.new(@package_file.package, filename).execute! + entry.package_file = ::Packages::PackageFileFinder.new(incoming, filename).execute! entry.validate! rescue ActiveRecord::RecordNotFound raise ExtractionError, "#{filename} is listed in Files but was not uploaded" end end + + def incoming + @package_file.package.project.packages.debian_incoming_package! + end + strong_memoize_attr(:incoming) end end end diff --git a/app/services/packages/debian/generate_distribution_key_service.rb b/app/services/packages/debian/generate_distribution_key_service.rb index 917965da58e..25c84955a52 100644 --- a/app/services/packages/debian/generate_distribution_key_service.rb +++ b/app/services/packages/debian/generate_distribution_key_service.rb @@ -43,10 +43,9 @@ module Packages attr_reader :params def passphrase - strong_memoize(:passphrase) do - params[:passphrase] || ::User.random_password - end + params[:passphrase] || ::User.random_password end + strong_memoize_attr :passphrase def pinentry_script_content escaped_passphrase = Shellwords.escape(passphrase) @@ -90,7 +89,7 @@ module Packages 'Name-Email': params[:name_email] || Gitlab.config.gitlab.email_reply_to, 'Name-Comment': params[:name_comment] || 'GitLab Debian repository automatic signing key', 'Expire-Date': params[:expire_date] || 0, - 'Passphrase': passphrase + Passphrase: passphrase }.map { |k, v| "#{k}: #{v}\n" }.join + '</GnupgKeyParms>' end diff --git a/app/services/packages/debian/generate_distribution_service.rb b/app/services/packages/debian/generate_distribution_service.rb index d69f6eb1511..9feb860ae87 100644 --- a/app/services/packages/debian/generate_distribution_service.rb +++ b/app/services/packages/debian/generate_distribution_service.rb @@ -213,10 +213,9 @@ module Packages end def release_content - strong_memoize(:release_content) do - release_header + release_sums - end + release_header + release_sums end + strong_memoize_attr :release_content def release_header [ @@ -235,10 +234,9 @@ module Packages end def release_date - strong_memoize(:release_date) do - Time.now.utc - end + Time.now.utc end + strong_memoize_attr :release_date def release_sums # NB: MD5Sum was removed for FIPS compliance diff --git a/app/services/packages/debian/process_changes_service.rb b/app/services/packages/debian/process_changes_service.rb index 129f2e5c9bc..eb88e7c9b59 100644 --- a/app/services/packages/debian/process_changes_service.rb +++ b/app/services/packages/debian/process_changes_service.rb @@ -76,10 +76,9 @@ module Packages end def metadata - strong_memoize(:metadata) do - ::Packages::Debian::ExtractChangesMetadataService.new(package_file).execute - end + ::Packages::Debian::ExtractChangesMetadataService.new(package_file).execute end + strong_memoize_attr :metadata def files metadata[:files] @@ -90,16 +89,15 @@ module Packages end def package - strong_memoize(:package) do - params = { - 'name': metadata[:fields]['Source'], - 'version': metadata[:fields]['Version'], - 'distribution_name': metadata[:fields]['Distribution'] - } - response = Packages::Debian::FindOrCreatePackageService.new(project, creator, params).execute - response.payload[:package] - end + params = { + name: metadata[:fields]['Source'], + version: metadata[:fields]['Version'], + distribution_name: metadata[:fields]['Distribution'] + } + response = Packages::Debian::FindOrCreatePackageService.new(project, creator, params).execute + response.payload[:package] end + strong_memoize_attr :package # used by ExclusiveLeaseGuard def lease_key diff --git a/app/services/packages/debian/process_package_file_service.rb b/app/services/packages/debian/process_package_file_service.rb index f4fcd3a563c..684192f6006 100644 --- a/app/services/packages/debian/process_package_file_service.rb +++ b/app/services/packages/debian/process_package_file_service.rb @@ -10,6 +10,8 @@ module Packages # used by ExclusiveLeaseGuard DEFAULT_LEASE_TIMEOUT = 1.hour.to_i.freeze + SIMPLE_DEB_FILE_TYPES = %i[deb udeb ddeb].freeze + def initialize(package_file, distribution_name, component_name) @package_file = package_file @distribution_name = distribution_name @@ -22,9 +24,10 @@ module Packages validate! try_obtain_lease do - package.transaction do + distribution.transaction do rename_package_and_set_version update_package + update_files_metadata if changes_file? update_file_metadata cleanup_temp_package end @@ -36,28 +39,61 @@ module Packages private def validate! - raise ArgumentError, 'missing distribution name' unless @distribution_name.present? - raise ArgumentError, 'missing component name' unless @component_name.present? raise ArgumentError, 'package file without Debian metadata' unless @package_file.debian_file_metadatum raise ArgumentError, 'already processed package file' unless @package_file.debian_file_metadatum.unknown? - if file_metadata[:file_type] == :deb || file_metadata[:file_type] == :udeb || file_metadata[:file_type] == :ddeb - return - end + changes_file? ? validate_changes_file! : validate_package_file! + end + + def changes_file? + @package_file.file_name.end_with?('.changes') + end + + def validate_changes_file! + raise ArgumentError, 'unwanted distribution name' unless @distribution_name.nil? + raise ArgumentError, 'unwanted component name' unless @component_name.nil? + raise ArgumentError, 'missing Source field' unless file_metadata.dig(:fields, 'Source').present? + raise ArgumentError, 'missing Version field' unless file_metadata.dig(:fields, 'Version').present? + raise ArgumentError, 'missing Distribution field' unless file_metadata.dig(:fields, 'Distribution').present? + end + + def validate_package_file! + raise ArgumentError, 'missing distribution name' unless @distribution_name.present? + raise ArgumentError, 'missing component name' unless @component_name.present? + + return if SIMPLE_DEB_FILE_TYPES.include?(file_metadata[:file_type]) raise ArgumentError, "invalid package file type: #{file_metadata[:file_type]}" end def file_metadata - ::Packages::Debian::ExtractMetadataService.new(@package_file).execute + metadata_service_class.new(@package_file).execute end strong_memoize_attr :file_metadata + def metadata_service_class + changes_file? ? ::Packages::Debian::ExtractChangesMetadataService : ::Packages::Debian::ExtractMetadataService + end + + def distribution + Packages::Debian::DistributionsFinder.new( + @package_file.package.project, + codename_or_suite: package_distribution + ).execute.last! + end + strong_memoize_attr :distribution + + def package_distribution + return file_metadata[:fields]['Distribution'] if changes_file? + + @distribution_name + end + def package packages = temp_package.project .packages .existing_debian_packages_with(name: package_name, version: package_version) - package = packages.with_debian_codename_or_suite(@distribution_name) + package = packages.with_debian_codename_or_suite(package_distribution) .first unless package @@ -79,10 +115,14 @@ module Packages strong_memoize_attr :temp_package def package_name + return file_metadata[:fields]['Source'] if changes_file? + package_name_and_version[0] end def package_version + return file_metadata[:fields]['Version'] if changes_file? + package_name_and_version[1] end @@ -121,13 +161,24 @@ module Packages package.id == temp_package.id end - def distribution - Packages::Debian::DistributionsFinder.new( - @package_file.package.project, - codename_or_suite: @distribution_name - ).execute.last! + def update_files_metadata + file_metadata[:files].each do |_, entry| + file_metadata = ::Packages::Debian::ExtractMetadataService.new(entry.package_file).execute + + ::Packages::UpdatePackageFileService.new(entry.package_file, package_id: package.id) + .execute + + # Force reload from database, as package has changed + entry.package_file.reload_package + + entry.package_file.debian_file_metadatum.update!( + file_type: file_metadata[:file_type], + component: entry.component, + architecture: file_metadata[:architecture], + fields: file_metadata[:fields] + ) + end end - strong_memoize_attr :distribution def update_file_metadata ::Packages::UpdatePackageFileService.new(@package_file, package_id: package.id) @@ -150,7 +201,7 @@ module Packages # used by ExclusiveLeaseGuard def lease_key - "packages:debian:process_package_file_service:package_file:#{@package_file.id}" + "packages:debian:process_package_file_service:#{temp_package.project_id}_#{package_name}_#{package_version}" end # used by ExclusiveLeaseGuard diff --git a/app/services/packages/helm/process_file_service.rb b/app/services/packages/helm/process_file_service.rb index f53c63d2b01..219f3d8c781 100644 --- a/app/services/packages/helm/process_file_service.rb +++ b/app/services/packages/helm/process_file_service.rb @@ -57,28 +57,25 @@ module Packages end def temp_package - strong_memoize(:temp_package) do - package_file.package - end + package_file.package end + strong_memoize_attr :temp_package def package - strong_memoize(:package) do - project_packages = package_file.package.project.packages - package = project_packages.with_package_type(:helm) - .with_name(metadata['name']) - .with_version(metadata['version']) - .not_pending_destruction - .last - package || temp_package - end + project_packages = package_file.package.project.packages + package = project_packages.with_package_type(:helm) + .with_name(metadata['name']) + .with_version(metadata['version']) + .not_pending_destruction + .last + package || temp_package end + strong_memoize_attr :package def metadata - strong_memoize(:metadata) do - ::Packages::Helm::ExtractFileMetadataService.new(package_file).execute - end + ::Packages::Helm::ExtractFileMetadataService.new(package_file).execute end + strong_memoize_attr :metadata def file_name "#{metadata['name']}-#{metadata['version']}.tgz" diff --git a/app/services/packages/maven/metadata/base_create_xml_service.rb b/app/services/packages/maven/metadata/base_create_xml_service.rb index 3b0d93e1dfb..d67d5a21a91 100644 --- a/app/services/packages/maven/metadata/base_create_xml_service.rb +++ b/app/services/packages/maven/metadata/base_create_xml_service.rb @@ -19,12 +19,11 @@ module Packages attr_reader :logger def xml_doc - strong_memoize(:xml_doc) do - Nokogiri::XML(@metadata_content) do |config| - config.default_xml.noblanks - end + Nokogiri::XML(@metadata_content) do |config| + config.default_xml.noblanks end end + strong_memoize_attr :xml_doc def xml_node(name, content) xml_doc.create_element(name).tap { |e| e.content = content } diff --git a/app/services/packages/maven/metadata/create_plugins_xml_service.rb b/app/services/packages/maven/metadata/create_plugins_xml_service.rb index 707a8c577ba..e99a72bc0ab 100644 --- a/app/services/packages/maven/metadata/create_plugins_xml_service.rb +++ b/app/services/packages/maven/metadata/create_plugins_xml_service.rb @@ -40,37 +40,34 @@ module Packages end def plugins_xml_node - strong_memoize(:plugins_xml_node) do - xml_doc.xpath(XPATH_PLUGINS) + xml_doc.xpath(XPATH_PLUGINS) .first - end end + strong_memoize_attr :plugins_xml_node def plugin_artifact_ids_from_xml - strong_memoize(:plugin_artifact_ids_from_xml) do - plugins_xml_node.xpath(XPATH_PLUGIN_ARTIFACT_ID) + plugins_xml_node.xpath(XPATH_PLUGIN_ARTIFACT_ID) .map(&:content) - end end + strong_memoize_attr :plugin_artifact_ids_from_xml def plugin_artifact_ids_from_database - strong_memoize(:plugin_artifact_ids_from_database) do - package_names = plugin_artifact_ids_from_xml.map do |artifact_id| - "#{@package.name}/#{artifact_id}" - end - - packages = @package.project.packages - .maven - .displayable - .with_name(package_names) - .has_version - - ::Packages::Maven::Metadatum.for_package_ids(packages.select(:id)) - .order_created - .pluck_app_name - .uniq + package_names = plugin_artifact_ids_from_xml.map do |artifact_id| + "#{@package.name}/#{artifact_id}" end + + packages = @package.project.packages + .maven + .displayable + .with_name(package_names) + .has_version + + ::Packages::Maven::Metadatum.for_package_ids(packages.select(:id)) + .order_created + .pluck_app_name + .uniq end + strong_memoize_attr :plugin_artifact_ids_from_database def plugin_node_for(artifact_id) xml_doc.create_element('plugin').tap do |plugin_node| diff --git a/app/services/packages/maven/metadata/create_versions_xml_service.rb b/app/services/packages/maven/metadata/create_versions_xml_service.rb index c2ac7fea703..966540bcba2 100644 --- a/app/services/packages/maven/metadata/create_versions_xml_service.rb +++ b/app/services/packages/maven/metadata/create_versions_xml_service.rb @@ -91,49 +91,43 @@ module Packages end def versioning_xml_node - strong_memoize(:versioning_xml_node) do - xml_doc.xpath(XPATH_VERSIONING).first - end + xml_doc.xpath(XPATH_VERSIONING).first end + strong_memoize_attr :versioning_xml_node def versions_xml_node - strong_memoize(:versions_xml_node) do - versioning_xml_node&.xpath(XPATH_VERSIONS) + versioning_xml_node&.xpath(XPATH_VERSIONS) &.first - end end + strong_memoize_attr :versions_xml_node def version_xml_nodes versions_xml_node&.xpath(XPATH_VERSION) end def latest_xml_node - strong_memoize(:latest_xml_node) do - versioning_xml_node&.xpath(XPATH_LATEST) + versioning_xml_node&.xpath(XPATH_LATEST) &.first - end end + strong_memoize_attr :latest_xml_node def release_xml_node - strong_memoize(:release_xml_node) do - versioning_xml_node&.xpath(XPATH_RELEASE) + versioning_xml_node&.xpath(XPATH_RELEASE) &.first - end end + strong_memoize_attr :release_xml_node def last_updated_xml_node - strong_memoize(:last_updated_xml_mode) do - versioning_xml_node.xpath(XPATH_LAST_UPDATED) + versioning_xml_node.xpath(XPATH_LAST_UPDATED) .first - end end + strong_memoize_attr :last_updated_xml_node def versions_from_xml - strong_memoize(:versions_from_xml) do - versions_xml_node.xpath(XPATH_VERSION) + versions_xml_node.xpath(XPATH_VERSION) .map(&:text) - end end + strong_memoize_attr :versions_from_xml def latest_from_xml latest_xml_node&.text @@ -144,27 +138,25 @@ module Packages end def versions_from_database - strong_memoize(:versions_from_database) do - @package.project.packages + @package.project.packages .maven .displayable .with_name(@package.name) .has_version .order_created .pluck_versions - end end + strong_memoize_attr :versions_from_database def latest_from_database versions_from_database.last end def release_from_database - strong_memoize(:release_from_database) do - non_snapshot_versions_from_database = versions_from_database.reject { |v| v.ends_with?('SNAPSHOT') } - non_snapshot_versions_from_database.last - end + non_snapshot_versions_from_database = versions_from_database.reject { |v| v.ends_with?('SNAPSHOT') } + non_snapshot_versions_from_database.last end + strong_memoize_attr :release_from_database def log_malformed_content(reason) logger.warn( diff --git a/app/services/packages/maven/metadata/sync_service.rb b/app/services/packages/maven/metadata/sync_service.rb index dacf6750412..14196f090dd 100644 --- a/app/services/packages/maven/metadata/sync_service.rb +++ b/app/services/packages/maven/metadata/sync_service.rb @@ -70,25 +70,22 @@ module Packages end def metadata_package_file_for_versions - strong_memoize(:metadata_file_for_versions) do - metadata_package_file_for(versionless_package_for_versions) - end + metadata_package_file_for(versionless_package_for_versions) end + strong_memoize_attr :metadata_package_file_for_versions def versionless_package_for_versions - strong_memoize(:versionless_package_for_versions) do - versionless_package_named(package_name) - end + versionless_package_named(package_name) end + strong_memoize_attr :versionless_package_for_versions def metadata_package_file_for_plugins - strong_memoize(:metadata_package_file_for_plugins) do - pkg_name = package_name_for_plugins - next unless pkg_name + pkg_name = package_name_for_plugins + return unless pkg_name - metadata_package_file_for(versionless_package_named(package_name_for_plugins)) - end + metadata_package_file_for(versionless_package_named(package_name_for_plugins)) end + strong_memoize_attr :metadata_package_file_for_plugins def metadata_package_file_for(package) return unless package diff --git a/app/services/packages/ml_model/create_package_file_service.rb b/app/services/packages/ml_model/create_package_file_service.rb new file mode 100644 index 00000000000..574f70940fc --- /dev/null +++ b/app/services/packages/ml_model/create_package_file_service.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module Packages + module MlModel + class CreatePackageFileService < BaseService + def execute + ::Packages::Package.transaction do + create_package_file(find_or_create_package) + end + end + + private + + def find_or_create_package + package_params = { + name: params[:package_name], + version: params[:package_version], + build: params[:build], + status: params[:status] + } + + package = ::Packages::MlModel::FindOrCreatePackageService + .new(project, current_user, package_params) + .execute + + package.update_column(:status, params[:status]) if params[:status] && params[:status] != package.status + + package.create_build_infos!(params[:build]) + + package + end + + def create_package_file(package) + file_params = { + file: params[:file], + size: params[:file].size, + file_sha256: params[:file].sha256, + file_name: params[:file_name], + build: params[:build] + } + + ::Packages::CreatePackageFileService.new(package, file_params).execute + end + end + end +end diff --git a/app/services/packages/ml_model/find_or_create_package_service.rb b/app/services/packages/ml_model/find_or_create_package_service.rb new file mode 100644 index 00000000000..cab99e1b008 --- /dev/null +++ b/app/services/packages/ml_model/find_or_create_package_service.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Packages + module MlModel + class FindOrCreatePackageService < ::Packages::CreatePackageService + def execute + find_or_create_package!(::Packages::Package.package_types['ml_model']) + end + end + end +end diff --git a/app/services/packages/npm/create_metadata_cache_service.rb b/app/services/packages/npm/create_metadata_cache_service.rb index 1cc5f7f34e7..75cff5c5453 100644 --- a/app/services/packages/npm/create_metadata_cache_service.rb +++ b/app/services/packages/npm/create_metadata_cache_service.rb @@ -9,10 +9,9 @@ module Packages # used by ExclusiveLeaseGuard DEFAULT_LEASE_TIMEOUT = 1.hour.to_i.freeze - def initialize(project, package_name, packages) + def initialize(project, package_name) @project = project @package_name = package_name - @packages = packages end def execute @@ -28,13 +27,19 @@ module Packages private - attr_reader :package_name, :packages, :project + attr_reader :package_name, :project def metadata_content metadata.payload.to_json end strong_memoize_attr :metadata_content + def packages + ::Packages::Npm::PackageFinder + .new(package_name, project: project) + .execute + end + def metadata Packages::Npm::GenerateMetadataService.new(package_name, packages).execute end diff --git a/app/services/packages/npm/create_package_service.rb b/app/services/packages/npm/create_package_service.rb index c71ae060dd9..2c578760cc5 100644 --- a/app/services/packages/npm/create_package_service.rb +++ b/app/services/packages/npm/create_package_service.rb @@ -61,10 +61,9 @@ module Packages end def version - strong_memoize(:version) do - params[:versions].each_key.first - end + params[:versions].each_key.first end + strong_memoize_attr :version def version_data params[:versions][version] @@ -79,30 +78,27 @@ module Packages end def package_file_name - strong_memoize(:package_file_name) do - "#{name}-#{version}.tgz" - end + "#{name}-#{version}.tgz" end + strong_memoize_attr :package_file_name def attachment - strong_memoize(:attachment) do - params['_attachments'][package_file_name] - end + params['_attachments'][package_file_name] end + strong_memoize_attr :attachment # TODO (technical debt): Extract the package size calculation to its own component and unit test it separately. def calculated_package_file_size - strong_memoize(:calculated_package_file_size) do - # This calculation is based on: - # 1. 4 chars in a Base64 encoded string are 3 bytes in the original string. Meaning 1 char is 0.75 bytes. - # 2. The encoded string may have 1 or 2 extra '=' chars used for padding. Each padding char means 1 byte less in the original string. - # Reference: - # - https://blog.aaronlenoir.com/2017/11/10/get-original-length-from-base-64-string/ - # - https://en.wikipedia.org/wiki/Base64#Decoding_Base64_with_padding - encoded_data = attachment['data'] - ((encoded_data.length * 0.75) - encoded_data[-2..].count('=')).to_i - end + # This calculation is based on: + # 1. 4 chars in a Base64 encoded string are 3 bytes in the original string. Meaning 1 char is 0.75 bytes. + # 2. The encoded string may have 1 or 2 extra '=' chars used for padding. Each padding char means 1 byte less in the original string. + # Reference: + # - https://blog.aaronlenoir.com/2017/11/10/get-original-length-from-base-64-string/ + # - https://en.wikipedia.org/wiki/Base64#Decoding_Base64_with_padding + encoded_data = attachment['data'] + ((encoded_data.length * 0.75) - encoded_data[-2..].count('=')).to_i end + strong_memoize_attr :calculated_package_file_size def file_params { @@ -134,29 +130,26 @@ module Packages end def field_sizes - strong_memoize(:field_sizes) do - package_json.transform_values do |value| - value.to_s.size - end + package_json.transform_values do |value| + value.to_s.size end end + strong_memoize_attr :field_sizes def filtered_field_sizes - strong_memoize(:filtered_field_sizes) do - field_sizes.select do |_, size| - size >= ::Packages::Npm::Metadatum::MIN_PACKAGE_JSON_FIELD_SIZE_FOR_ERROR_TRACKING - end + field_sizes.select do |_, size| + size >= ::Packages::Npm::Metadatum::MIN_PACKAGE_JSON_FIELD_SIZE_FOR_ERROR_TRACKING end end + strong_memoize_attr :filtered_field_sizes def largest_fields - strong_memoize(:largest_fields) do - field_sizes + field_sizes .sort_by { |a| a[1] } .reverse[0..::Packages::Npm::Metadatum::NUM_FIELDS_FOR_ERROR_TRACKING - 1] .to_h - end end + strong_memoize_attr :largest_fields def field_sizes_for_error_tracking filtered_field_sizes.empty? ? largest_fields : filtered_field_sizes diff --git a/app/services/packages/npm/create_tag_service.rb b/app/services/packages/npm/create_tag_service.rb index 82974d0ca4b..e212b37c9ba 100644 --- a/app/services/packages/npm/create_tag_service.rb +++ b/app/services/packages/npm/create_tag_service.rb @@ -23,12 +23,11 @@ module Packages private def existing_tag - strong_memoize(:existing_tag) do - Packages::TagsFinder + Packages::TagsFinder .new(package.project, package.name, package_type: package.package_type) .find_by_name(tag_name) - end end + strong_memoize_attr :existing_tag end end end diff --git a/app/services/packages/nuget/metadata_extraction_service.rb b/app/services/packages/nuget/metadata_extraction_service.rb index 02086b2a282..5c60a2912ae 100644 --- a/app/services/packages/nuget/metadata_extraction_service.rb +++ b/app/services/packages/nuget/metadata_extraction_service.rb @@ -7,18 +7,22 @@ module Packages ExtractionError = Class.new(StandardError) + ROOT_XPATH = '//xmlns:package/xmlns:metadata/xmlns' + XPATHS = { - package_name: '//xmlns:package/xmlns:metadata/xmlns:id', - package_version: '//xmlns:package/xmlns:metadata/xmlns:version', - license_url: '//xmlns:package/xmlns:metadata/xmlns:licenseUrl', - project_url: '//xmlns:package/xmlns:metadata/xmlns:projectUrl', - icon_url: '//xmlns:package/xmlns:metadata/xmlns:iconUrl' + package_name: "#{ROOT_XPATH}:id", + package_version: "#{ROOT_XPATH}:version", + authors: "#{ROOT_XPATH}:authors", + description: "#{ROOT_XPATH}:description", + license_url: "#{ROOT_XPATH}:licenseUrl", + project_url: "#{ROOT_XPATH}:projectUrl", + icon_url: "#{ROOT_XPATH}:iconUrl" }.freeze - XPATH_DEPENDENCIES = '//xmlns:package/xmlns:metadata/xmlns:dependencies/xmlns:dependency' - XPATH_DEPENDENCY_GROUPS = '//xmlns:package/xmlns:metadata/xmlns:dependencies/xmlns:group' - XPATH_TAGS = '//xmlns:package/xmlns:metadata/xmlns:tags' - XPATH_PACKAGE_TYPES = '//xmlns:package/xmlns:metadata/xmlns:packageTypes/xmlns:packageType' + XPATH_DEPENDENCIES = "#{ROOT_XPATH}:dependencies/xmlns:dependency".freeze + XPATH_DEPENDENCY_GROUPS = "#{ROOT_XPATH}:dependencies/xmlns:group".freeze + XPATH_TAGS = "#{ROOT_XPATH}:tags".freeze + XPATH_PACKAGE_TYPES = "#{ROOT_XPATH}:packageTypes/xmlns:packageType".freeze MAX_FILE_SIZE = 4.megabytes.freeze @@ -35,14 +39,9 @@ module Packages private def package_file - strong_memoize(:package_file) do - ::Packages::PackageFile.find_by_id(@package_file_id) - end - end - - def project - package_file.package.project + ::Packages::PackageFile.find_by_id(@package_file_id) end + strong_memoize_attr :package_file def valid_package_file? package_file && diff --git a/app/services/packages/nuget/search_service.rb b/app/services/packages/nuget/search_service.rb index fea424b3aa8..7d1585f8903 100644 --- a/app/services/packages/nuget/search_service.rb +++ b/app/services/packages/nuget/search_service.rb @@ -89,17 +89,16 @@ module Packages end def base_matching_package_names - strong_memoize(:base_matching_package_names) do - # rubocop: disable CodeReuse/ActiveRecord - pkgs = nuget_packages.order_name + # rubocop: disable CodeReuse/ActiveRecord + pkgs = nuget_packages.order_name .select_distinct_name .where(project_id: project_ids) - pkgs = pkgs.without_version_like(PRE_RELEASE_VERSION_MATCHING_TERM) unless include_prerelease_versions? - pkgs = pkgs.search_by_name(@search_term) if @search_term.present? - pkgs - # rubocop: enable CodeReuse/ActiveRecord - end + pkgs = pkgs.without_version_like(PRE_RELEASE_VERSION_MATCHING_TERM) unless include_prerelease_versions? + pkgs = pkgs.search_by_name(@search_term) if @search_term.present? + pkgs + # rubocop: enable CodeReuse/ActiveRecord end + strong_memoize_attr :base_matching_package_names def nuget_packages Packages::Package.nuget @@ -111,11 +110,10 @@ module Packages def project_ids_cte return unless use_project_ids_cte? - strong_memoize(:project_ids_cte) do - query = projects_visible_to_user(@current_user, within_group: @project_or_group) - Gitlab::SQL::CTE.new(:project_ids, query.select(:id)) - end + query = projects_visible_to_user(@current_user, within_group: @project_or_group) + Gitlab::SQL::CTE.new(:project_ids, query.select(:id)) end + strong_memoize_attr :project_ids_cte def project_ids return @project_or_group.id if project? diff --git a/app/services/packages/nuget/sync_metadatum_service.rb b/app/services/packages/nuget/sync_metadatum_service.rb index ca9cc4d5b78..189b972c156 100644 --- a/app/services/packages/nuget/sync_metadatum_service.rb +++ b/app/services/packages/nuget/sync_metadatum_service.rb @@ -15,6 +15,8 @@ module Packages metadatum.destroy! if metadatum.persisted? else metadatum.update!( + authors: authors, + description: description, license_url: license_url, project_url: project_url, icon_url: icon_url @@ -24,26 +26,57 @@ module Packages private + attr_reader :package, :metadata + def metadatum - strong_memoize(:metadatum) do - @package.nuget_metadatum || @package.build_nuget_metadatum - end + package.nuget_metadatum || package.build_nuget_metadatum end + strong_memoize_attr :metadatum def blank_metadata? - project_url.blank? && license_url.blank? && icon_url.blank? + [authors, description, project_url, license_url, icon_url].all?(&:blank?) + end + + def authors + truncate_value(:authors, ::Packages::Nuget::Metadatum::MAX_AUTHORS_LENGTH) end + strong_memoize_attr :authors + + def description + truncate_value(:description, ::Packages::Nuget::Metadatum::MAX_DESCRIPTION_LENGTH) + end + strong_memoize_attr :description def project_url - @metadata[:project_url] + metadata[:project_url] end def license_url - @metadata[:license_url] + metadata[:license_url] end def icon_url - @metadata[:icon_url] + metadata[:icon_url] + end + + def truncate_value(field, max_length) + return unless metadata[field] + + if metadata[field].size > max_length + log_info("#{field.capitalize} is too long (maximum is #{max_length} characters)", field) + end + + metadata[field].truncate(max_length) + end + + def log_info(message, field) + Gitlab::AppLogger.info( + class: self.class.name, + message: message, + package_id: package.id, + project_id: package.project_id, + field => metadata[field] + ) end end end diff --git a/app/services/packages/nuget/update_package_from_metadata_service.rb b/app/services/packages/nuget/update_package_from_metadata_service.rb index 5456ad4cad7..8e2679db31b 100644 --- a/app/services/packages/nuget/update_package_from_metadata_service.rb +++ b/app/services/packages/nuget/update_package_from_metadata_service.rb @@ -9,6 +9,9 @@ module Packages # used by ExclusiveLeaseGuard DEFAULT_LEASE_TIMEOUT = 1.hour.to_i.freeze SYMBOL_PACKAGE_IDENTIFIER = 'SymbolsPackage' + INVALID_METADATA_ERROR_MESSAGE = 'package name, version, authors and/or description not found in metadata' + INVALID_METADATA_ERROR_SYMBOL_MESSAGE = 'package name, version and/or description not found in metadata' + MISSING_MATCHING_PACKAGE_ERROR_MESSAGE = 'symbol package is invalid, matching package does not exist' InvalidMetadataError = Class.new(StandardError) @@ -17,7 +20,10 @@ module Packages end def execute - raise InvalidMetadataError, 'package name and/or package version not found in metadata' unless valid_metadata? + unless valid_metadata? + error_message = symbol_package? ? INVALID_METADATA_ERROR_SYMBOL_MESSAGE : INVALID_METADATA_ERROR_MESSAGE + raise InvalidMetadataError, error_message + end try_obtain_lease do @package_file.transaction do @@ -39,7 +45,7 @@ module Packages target_package = existing_package else if symbol_package? - raise InvalidMetadataError, 'symbol package is invalid, matching package does not exist' + raise InvalidMetadataError, MISSING_MATCHING_PACKAGE_ERROR_MESSAGE end update_linked_package @@ -55,17 +61,21 @@ module Packages return if symbol_package? ::Packages::Nuget::SyncMetadatumService - .new(package, metadata.slice(:project_url, :license_url, :icon_url)) + .new(package, metadata.slice(:authors, :description, :project_url, :license_url, :icon_url)) .execute + ::Packages::UpdateTagsService .new(package, package_tags) .execute + rescue StandardError => e raise InvalidMetadataError, e.message end def valid_metadata? - package_name.present? && package_version.present? + fields = [package_name, package_version, package_description] + fields << package_authors unless symbol_package? + fields.all?(&:present?) end def link_to_existing_package @@ -93,15 +103,14 @@ module Packages end def existing_package - strong_memoize(:existing_package) do - @package_file.project.packages - .nuget - .with_name(package_name) - .with_version(package_version) - .not_pending_destruction - .first - end + @package_file.project.packages + .nuget + .with_name(package_name) + .with_version(package_version) + .not_pending_destruction + .first end + strong_memoize_attr :existing_package def package_name metadata[:package_name] @@ -123,15 +132,22 @@ module Packages metadata.fetch(:package_types, []) end + def package_authors + metadata[:authors] + end + + def package_description + metadata[:description] + end + def symbol_package? package_types.include?(SYMBOL_PACKAGE_IDENTIFIER) end def metadata - strong_memoize(:metadata) do - ::Packages::Nuget::MetadataExtractionService.new(@package_file.id).execute - end + ::Packages::Nuget::MetadataExtractionService.new(@package_file.id).execute end + strong_memoize_attr :metadata def package_filename "#{package_name.downcase}.#{package_version.downcase}.#{symbol_package? ? 'snupkg' : 'nupkg'}" diff --git a/app/services/packages/pypi/create_package_service.rb b/app/services/packages/pypi/create_package_service.rb index b464ce4504a..087a8e42a66 100644 --- a/app/services/packages/pypi/create_package_service.rb +++ b/app/services/packages/pypi/create_package_service.rb @@ -29,10 +29,9 @@ module Packages private def created_package - strong_memoize(:created_package) do - find_or_create_package!(:pypi) - end + find_or_create_package!(:pypi) end + strong_memoize_attr :created_package def file_params { diff --git a/app/services/packages/rpm/parse_package_service.rb b/app/services/packages/rpm/parse_package_service.rb index d2751c77c5b..3995eedef53 100644 --- a/app/services/packages/rpm/parse_package_service.rb +++ b/app/services/packages/rpm/parse_package_service.rb @@ -43,10 +43,9 @@ module Packages end def package_tags - strong_memoize(:package_tags) do - rpm.tags - end + rpm.tags end + strong_memoize_attr :package_tags def extract_static_attributes STATIC_ATTRIBUTES.index_with do |attribute| diff --git a/app/services/packages/rubygems/dependency_resolver_service.rb b/app/services/packages/rubygems/dependency_resolver_service.rb index 839a7683632..214a4adc47f 100644 --- a/app/services/packages/rubygems/dependency_resolver_service.rb +++ b/app/services/packages/rubygems/dependency_resolver_service.rb @@ -33,10 +33,9 @@ module Packages private def packages - strong_memoize(:packages) do - project.packages.with_name(gem_name) - end + project.packages.with_name(gem_name) end + strong_memoize_attr :packages def gem_name params[:gem_name] diff --git a/app/services/packages/rubygems/process_gem_service.rb b/app/services/packages/rubygems/process_gem_service.rb index c771af28f73..ca4aaa8fdde 100644 --- a/app/services/packages/rubygems/process_gem_service.rb +++ b/app/services/packages/rubygems/process_gem_service.rb @@ -64,10 +64,9 @@ module Packages end def gemspec - strong_memoize(:gemspec) do - gem.spec - end + gem.spec end + strong_memoize_attr :gemspec def success ServiceResponse.success(payload: { package: package }) @@ -78,24 +77,21 @@ module Packages end def temp_package - strong_memoize(:temp_package) do - package_file.package - end + package_file.package end + strong_memoize_attr :temp_package def package - strong_memoize(:package) do - # if package with name/version already exists, use that package - package = temp_package.project + package = temp_package.project .packages .rubygems .with_name(gemspec.name) .with_version(gemspec.version.to_s) .not_pending_destruction .last - package || temp_package - end + package || temp_package end + strong_memoize_attr :package def gem # use_file will set an exclusive lease on the file for as long as diff --git a/app/services/packages/terraform_module/create_package_service.rb b/app/services/packages/terraform_module/create_package_service.rb index 3afecc6c1ca..9df722db529 100644 --- a/app/services/packages/terraform_module/create_package_service.rb +++ b/app/services/packages/terraform_module/create_package_service.rb @@ -43,16 +43,14 @@ module Packages end def name - strong_memoize(:name) do - "#{params[:module_name]}/#{params[:module_system]}" - end + "#{params[:module_name]}/#{params[:module_system]}" end + strong_memoize_attr :name def file_name - strong_memoize(:file_name) do - "#{params[:module_name]}-#{params[:module_system]}-#{params[:module_version]}.tgz" - end + "#{params[:module_name]}-#{params[:module_system]}-#{params[:module_version]}.tgz" end + strong_memoize_attr :file_name def file_params { diff --git a/app/services/packages/update_tags_service.rb b/app/services/packages/update_tags_service.rb index f29c54dacb9..cf1acc6ee19 100644 --- a/app/services/packages/update_tags_service.rb +++ b/app/services/packages/update_tags_service.rb @@ -21,10 +21,9 @@ module Packages private def existing_tags - strong_memoize(:existing_tags) do - @package.tag_names - end + @package.tag_names end + strong_memoize_attr :existing_tags def rows(tags) now = Time.zone.now diff --git a/app/services/personal_access_tokens/create_service.rb b/app/services/personal_access_tokens/create_service.rb index adb7924f35e..31ba88af46c 100644 --- a/app/services/personal_access_tokens/create_service.rb +++ b/app/services/personal_access_tokens/create_service.rb @@ -13,7 +13,7 @@ module PersonalAccessTokens def execute return ServiceResponse.error(message: 'Not permitted to create') unless creation_permitted? - token = target_user.personal_access_tokens.create(params.slice(*allowed_params)) + token = target_user.personal_access_tokens.create(personal_access_token_params) if token.persisted? log_event(token) @@ -31,13 +31,17 @@ module PersonalAccessTokens attr_reader :target_user, :ip_address - def allowed_params - [ - :name, - :impersonation, - :scopes, - :expires_at - ] + def personal_access_token_params + { + name: params[:name], + impersonation: params[:impersonation] || false, + scopes: params[:scopes], + expires_at: pat_expiration + } + end + + def pat_expiration + params[:expires_at].presence || PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now end def creation_permitted? diff --git a/app/services/personal_access_tokens/last_used_service.rb b/app/services/personal_access_tokens/last_used_service.rb index 9066fd1acdf..6fc3110a70b 100644 --- a/app/services/personal_access_tokens/last_used_service.rb +++ b/app/services/personal_access_tokens/last_used_service.rb @@ -22,7 +22,14 @@ module PersonalAccessTokens last_used = @personal_access_token.last_used_at - last_used.nil? || (last_used <= 1.day.ago) + return true if last_used.nil? + + if Feature.enabled?(:update_personal_access_token_usage_information_every_10_minutes) && + last_used <= 10.minutes.ago + return true + end + + last_used <= 1.day.ago end end end diff --git a/app/services/post_receive_service.rb b/app/services/post_receive_service.rb index c376b4036f8..5ab5732ecf5 100644 --- a/app/services/post_receive_service.rb +++ b/app/services/post_receive_service.rb @@ -86,14 +86,15 @@ class PostReceiveService banner = nil if project - scoped_messages = BroadcastMessage.current_banner_messages(current_path: project.full_path).select do |message| - message.target_path.present? && message.matches_current_path(project.full_path) - end + scoped_messages = + BroadcastMessage.current_banner_messages(current_path: project.full_path).select do |message| + message.target_path.present? && message.matches_current_path(project.full_path) && message.show_in_cli? + end banner = scoped_messages.last end - banner ||= BroadcastMessage.current_banner_messages.last + banner ||= BroadcastMessage.current_show_in_cli_banner_messages.last banner&.message end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 8ad2b0ac761..e37b6516d21 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -24,7 +24,7 @@ module Projects def execute params[:wiki_enabled] = params[:wiki_access_level] if params[:wiki_access_level] params[:builds_enabled] = params[:builds_access_level] if params[:builds_access_level] - params[:snippets_enabled] = params[:builds_access_level] if params[:snippets_access_level] + params[:snippets_enabled] = params[:snippets_access_level] if params[:snippets_access_level] params[:merge_requests_enabled] = params[:merge_requests_access_level] if params[:merge_requests_access_level] params[:issues_enabled] = params[:issues_access_level] if params[:issues_access_level] @@ -231,7 +231,7 @@ module Projects @project.create_labels unless @project.gitlab_project_import? - break if @project.import? + next if @project.import? unless @project.create_repository(default_branch: default_branch) raise 'Failed to create repository' diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index ceab7098b32..e22b728cea3 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -20,6 +20,8 @@ module Projects add_repository_to_project + validate_repository_size! + download_lfs_objects import_data @@ -58,6 +60,10 @@ module Projects attr_reader :resolved_address + def validate_repository_size! + # Defined in EE::Projects::ImportService + end + def after_execute_hook # Defined in EE::Projects::ImportService end diff --git a/app/services/projects/lfs_pointers/lfs_import_service.rb b/app/services/projects/lfs_pointers/lfs_import_service.rb index c9791041088..95ddff45dff 100644 --- a/app/services/projects/lfs_pointers/lfs_import_service.rb +++ b/app/services/projects/lfs_pointers/lfs_import_service.rb @@ -14,7 +14,7 @@ module Projects end success - rescue StandardError => e + rescue StandardError, GRPC::Core::CallError => e error(e.message) end end diff --git a/app/services/projects/operations/update_service.rb b/app/services/projects/operations/update_service.rb index d0bef9da329..e7a8d5305ea 100644 --- a/app/services/projects/operations/update_service.rb +++ b/app/services/projects/operations/update_service.rb @@ -14,8 +14,6 @@ module Projects def project_update_params error_tracking_params .merge(alerting_setting_params) - .merge(metrics_setting_params) - .merge(grafana_integration_params) .merge(prometheus_integration_params) .merge(incident_management_setting_params) end @@ -37,15 +35,6 @@ module Projects { alerting_setting_attributes: attr } end - def metrics_setting_params - attribs = params[:metrics_setting_attributes] - return {} unless attribs - - attribs[:external_dashboard_url] = attribs[:external_dashboard_url].presence - - { metrics_setting_attributes: attribs } - end - def error_tracking_params settings = params[:error_tracking_setting_attributes] return {} if settings.blank? @@ -99,14 +88,6 @@ module Projects params end - def grafana_integration_params - return {} unless attrs = params[:grafana_integration_attributes] - - destroy = attrs[:grafana_url].blank? && attrs[:token].blank? - - { grafana_integration_attributes: attrs.merge(_destroy: destroy) } - end - def prometheus_integration_params return {} unless attrs = params[:prometheus_integration_attributes] diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb index c29770d0c5f..8c807e0016b 100644 --- a/app/services/projects/participants_service.rb +++ b/app/services/projects/participants_service.rb @@ -45,7 +45,7 @@ module Projects def visible_groups visible_groups = project.invited_groups - unless project.team.owner?(current_user) + unless project.team.member?(current_user) visible_groups = visible_groups.public_or_visible_to_user(current_user) end diff --git a/app/services/projects/prometheus/alerts/notify_service.rb b/app/services/projects/prometheus/alerts/notify_service.rb index 1e084c0e5eb..f1c093c89b7 100644 --- a/app/services/projects/prometheus/alerts/notify_service.rb +++ b/app/services/projects/prometheus/alerts/notify_service.rb @@ -36,7 +36,7 @@ module Projects truncate_alerts! if max_alerts_exceeded? - process_prometheus_alerts + process_prometheus_alerts(integration) created end @@ -79,12 +79,18 @@ module Projects end def valid_alert_manager_token?(token, integration) - valid_for_manual?(token) || - valid_for_alerts_endpoint?(token, integration) || + valid_for_alerts_endpoint?(token, integration) || + valid_for_manual?(token) || valid_for_cluster?(token) end def valid_for_manual?(token) + # If migration from Integrations::Prometheus to + # AlertManagement::HttpIntegrations is complete, + # we should use use the HttpIntegration as SSOT. + # Remove with https://gitlab.com/gitlab-org/gitlab/-/issues/409734 + return false if project.alert_management_http_integrations.legacy.prometheus.any? + prometheus = project.find_or_initialize_integration('prometheus') return false unless prometheus.manual_configuration? @@ -145,10 +151,10 @@ module Projects ActiveSupport::SecurityUtils.secure_compare(expected, actual) end - def process_prometheus_alerts + def process_prometheus_alerts(integration) alerts.map do |alert| AlertManagement::ProcessPrometheusAlertService - .new(project, alert) + .new(project, alert, integration: integration) .execute end end diff --git a/app/services/projects/readme_renderer_service.rb b/app/services/projects/readme_renderer_service.rb index 6871976aded..8fd33a717c5 100644 --- a/app/services/projects/readme_renderer_service.rb +++ b/app/services/projects/readme_renderer_service.rb @@ -17,9 +17,9 @@ module Projects end def sanitized_filename(template_name) - path = Gitlab::Utils.check_path_traversal!("#{template_name}.md.tt") + path = Gitlab::PathTraversal.check_path_traversal!("#{template_name}.md.tt") path = TEMPLATE_PATH.join(path).to_s - Gitlab::Utils.check_allowed_absolute_path!(path, [TEMPLATE_PATH.to_s]) + Gitlab::PathTraversal.check_allowed_absolute_path!(path, [TEMPLATE_PATH.to_s]) path end diff --git a/app/services/projects/slack_application_install_service.rb b/app/services/projects/slack_application_install_service.rb new file mode 100644 index 00000000000..812b8b0a082 --- /dev/null +++ b/app/services/projects/slack_application_install_service.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +module Projects + class SlackApplicationInstallService < BaseService + include Gitlab::Routing + + # Endpoint to initiate the OAuth flow, redirects to Slack's authorization screen + # https://api.slack.com/authentication/oauth-v2#asking + SLACK_AUTHORIZE_URL = 'https://slack.com/oauth/v2/authorize' + + # Endpoint to exchange the temporary authorization code for an access token + # https://api.slack.com/authentication/oauth-v2#exchanging + SLACK_EXCHANGE_TOKEN_URL = 'https://slack.com/api/oauth.v2.access' + + def execute + slack_data = exchange_slack_token + + return error("Slack: #{slack_data['error']}") unless slack_data['ok'] + + integration = project.gitlab_slack_application_integration \ + || project.create_gitlab_slack_application_integration! + + installation = integration.slack_integration || integration.build_slack_integration + + installation.update!( + bot_user_id: slack_data['bot_user_id'], + bot_access_token: slack_data['access_token'], + team_id: slack_data.dig('team', 'id'), + team_name: slack_data.dig('team', 'name'), + alias: project.full_path, + user_id: slack_data.dig('authed_user', 'id'), + authorized_scope_names: slack_data['scope'] + ) + + update_legacy_installations!(installation) + + success + end + + private + + def exchange_slack_token + query = { + client_id: Gitlab::CurrentSettings.slack_app_id, + client_secret: Gitlab::CurrentSettings.slack_app_secret, + code: params[:code], + # NOTE: Needs to match the `redirect_uri` passed to the authorization endpoint, + # otherwise we get a `bad_redirect_uri` error. + redirect_uri: slack_auth_project_settings_slack_url(project) + } + + Gitlab::HTTP.get(SLACK_EXCHANGE_TOKEN_URL, query: query).to_hash + end + + # Update any legacy SlackIntegration records for the Slack Workspace. Legacy SlackIntegration records + # are any created before our Slack App was upgraded to use Granular Bot Permissions and issue a + # bot_access_token. Any SlackIntegration records for the Slack Workspace will already have the same + # bot_access_token. + def update_legacy_installations!(installation) + updatable_attributes = installation.attributes.slice( + 'user_id', + 'bot_user_id', + 'encrypted_bot_access_token', + 'encrypted_bot_access_token_iv', + 'updated_at' + ) + + SlackIntegration.by_team(installation.team_id).id_not_in(installation.id).each_batch do |batch| + batch_ids = batch.pluck(:id) # rubocop: disable CodeReuse/ActiveRecord + batch.update_all(updatable_attributes) + + ::Integrations::SlackWorkspace::IntegrationApiScope.update_scopes(batch_ids, installation.slack_api_scopes) + end + end + end +end diff --git a/app/services/releases/create_service.rb b/app/services/releases/create_service.rb index e5883ca06f4..f0243d844d9 100644 --- a/app/services/releases/create_service.rb +++ b/app/services/releases/create_service.rb @@ -81,11 +81,17 @@ module Releases tag: tag.name, sha: tag.dereferenced_target.sha, released_at: released_at, - links_attributes: params.dig(:assets, 'links') || [], + links_attributes: links_attributes, milestones: milestones ) end + def links_attributes + (params.dig(:assets, 'links') || []).map do |link_params| + Releases::Links::Params.new(link_params).allowed_params + end + end + def create_evidence!(release, pipeline) return if release.historical_release? || release.upcoming_release? diff --git a/app/services/releases/links/base_service.rb b/app/services/releases/links/base_service.rb index 8bab258f80a..4c260e3183f 100644 --- a/app/services/releases/links/base_service.rb +++ b/app/services/releases/links/base_service.rb @@ -18,17 +18,7 @@ module Releases private def allowed_params - @allowed_params ||= params.slice(:name, :url, :link_type).tap do |hash| - hash[:filepath] = filepath if provided_filepath? - end - end - - def provided_filepath? - params.key?(:direct_asset_path) || params.key?(:filepath) - end - - def filepath - params[:direct_asset_path] || params[:filepath] + Params.new(params).allowed_params end end end diff --git a/app/services/releases/links/params.rb b/app/services/releases/links/params.rb new file mode 100644 index 00000000000..124ab333bbc --- /dev/null +++ b/app/services/releases/links/params.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Releases + module Links + class Params + def initialize(params) + @params = params.with_indifferent_access + end + + def allowed_params + @allowed_params ||= params.slice(:name, :url, :link_type).tap do |hash| + hash[:filepath] = filepath if provided_filepath? + end + end + + private + + attr_reader :params + + def provided_filepath? + params.key?(:direct_asset_path) || params.key?(:filepath) + end + + def filepath + params[:direct_asset_path] || params[:filepath] + end + end + end +end diff --git a/app/services/repositories/base_service.rb b/app/services/repositories/base_service.rb index 4d7e4ffe267..b262b4a1f7b 100644 --- a/app/services/repositories/base_service.rb +++ b/app/services/repositories/base_service.rb @@ -29,7 +29,7 @@ class Repositories::BaseService < BaseService end def move_error(path) - error = %Q{Repository "#{path}" could not be moved} + error = %{Repository "#{path}" could not be moved} log_error(error) error(error) diff --git a/app/services/resource_access_tokens/create_service.rb b/app/services/resource_access_tokens/create_service.rb index 553315f08f9..1fea894a599 100644 --- a/app/services/resource_access_tokens/create_service.rb +++ b/app/services/resource_access_tokens/create_service.rb @@ -17,6 +17,8 @@ module ResourceAccessTokens access_level = params[:access_level] || Gitlab::Access::MAINTAINER return error("Could not provision owner access to project access token") if do_not_allow_owner_access_level_for_project_bot?(access_level) + return error(s_('AccessTokens|Access token limit reached')) if reached_access_token_limit? + user = create_user return error(user.errors.full_messages.to_sentence) unless user.persisted? @@ -45,6 +47,10 @@ module ResourceAccessTokens attr_reader :resource_type, :resource + def reached_access_token_limit? + false + end + def username_and_email_generator Gitlab::Utils::UsernameAndEmailGenerator.new( username_prefix: "#{resource_type}_#{resource.id}_bot", @@ -91,7 +97,7 @@ module ResourceAccessTokens name: params[:name] || "#{resource_type}_bot", impersonation: false, scopes: params[:scopes] || default_scopes, - expires_at: params[:expires_at] || nil + expires_at: pat_expiration } end @@ -100,15 +106,11 @@ module ResourceAccessTokens end def create_membership(resource, user, access_level) - resource.add_member(user, access_level, expires_at: default_pat_expiration) + resource.add_member(user, access_level, expires_at: pat_expiration) end - def default_pat_expiration - if Feature.enabled?(:default_pat_expiration) - params[:expires_at].presence || PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now - else - params[:expires_at] - end + def pat_expiration + params[:expires_at].presence || PersonalAccessToken::MAX_PERSONAL_ACCESS_TOKEN_LIFETIME_IN_DAYS.days.from_now end def log_event(token) diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb index cee59360b4b..f4c0a743ef0 100644 --- a/app/services/search/global_service.rb +++ b/app/services/search/global_service.rb @@ -2,9 +2,11 @@ module Search class GlobalService + include Search::Filter include Gitlab::Utils::StrongMemoize - ALLOWED_SCOPES = %w(issues merge_requests milestones users).freeze + DEFAULT_SCOPE = 'projects' + ALLOWED_SCOPES = %w(projects issues merge_requests milestones users).freeze attr_accessor :current_user, :params @@ -19,12 +21,12 @@ module Search projects, order_by: params[:order_by], sort: params[:sort], - filters: { state: params[:state], confidential: params[:confidential] }) + filters: filters) end # rubocop: disable CodeReuse/ActiveRecord def projects - @projects ||= ProjectsFinder.new(params: { non_archived: true }, current_user: current_user).execute.preload(:topics, :project_topics) + @projects ||= ProjectsFinder.new(current_user: current_user).execute.preload(:topics, :project_topics) end def allowed_scopes @@ -33,7 +35,7 @@ module Search def scope strong_memoize(:scope) do - allowed_scopes.include?(params[:scope]) ? params[:scope] : 'projects' + allowed_scopes.include?(params[:scope]) ? params[:scope] : DEFAULT_SCOPE end end end diff --git a/app/services/search/group_service.rb b/app/services/search/group_service.rb index daed0df83f3..fa80a6ecf58 100644 --- a/app/services/search/group_service.rb +++ b/app/services/search/group_service.rb @@ -18,7 +18,7 @@ module Search group: group, order_by: params[:order_by], sort: params[:sort], - filters: { state: params[:state], confidential: params[:confidential] } + filters: filters ) end diff --git a/app/services/search/project_service.rb b/app/services/search/project_service.rb index 6acc32ea0a8..71314f85984 100644 --- a/app/services/search/project_service.rb +++ b/app/services/search/project_service.rb @@ -2,9 +2,11 @@ module Search class ProjectService + include Search::Filter include Gitlab::Utils::StrongMemoize + include ProjectsHelper - ALLOWED_SCOPES = %w(notes issues merge_requests milestones wiki_blobs commits users).freeze + ALLOWED_SCOPES = %w(blobs issues merge_requests wiki_blobs commits notes milestones users).freeze attr_accessor :project, :current_user, :params @@ -21,7 +23,7 @@ module Search repository_ref: params[:repository_ref], order_by: params[:order_by], sort: params[:sort], - filters: { confidential: params[:confidential], state: params[:state] } + filters: filters ) end @@ -31,7 +33,11 @@ module Search def scope strong_memoize(:scope) do - allowed_scopes.include?(params[:scope]) ? params[:scope] : 'blobs' + next params[:scope] if allowed_scopes.include?(params[:scope]) && project_search_tabs?(params[:scope].to_sym) + + allowed_scopes.find do |scope| + project_search_tabs?(scope.to_sym) + end end end end diff --git a/app/services/search_service.rb b/app/services/search_service.rb index 7fca6ed7a20..5705e4c7cef 100644 --- a/app/services/search_service.rb +++ b/app/services/search_service.rb @@ -113,6 +113,8 @@ class SearchService end def global_search_enabled_for_scope? + return false if show_snippets? && Feature.disabled?(:global_search_snippet_titles_tab, current_user, type: :ops) + case params[:scope] when 'blobs' Feature.enabled?(:global_search_code_tab, current_user, type: :ops) @@ -122,6 +124,8 @@ class SearchService Feature.enabled?(:global_search_issues_tab, current_user, type: :ops) when 'merge_requests' Feature.enabled?(:global_search_merge_requests_tab, current_user, type: :ops) + when 'snippet_titles' + Feature.enabled?(:global_search_snippet_titles_tab, current_user, type: :ops) when 'wiki_blobs' Feature.enabled?(:global_search_wiki_tab, current_user, type: :ops) when 'users' diff --git a/app/services/service_desk/custom_email_verifications/base_service.rb b/app/services/service_desk/custom_email_verifications/base_service.rb new file mode 100644 index 00000000000..fe456e4d3f3 --- /dev/null +++ b/app/services/service_desk/custom_email_verifications/base_service.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module ServiceDesk + module CustomEmailVerifications + class BaseService < ::BaseProjectService + attr_reader :settings + + def initialize(project:, current_user: nil, params: {}) + super(project: project, current_user: current_user, params: params) + + @settings = project.service_desk_setting + end + + private + + def notify_project_owners_and_user_with_email(email_method_name: nil, user: nil) + owner_emails = project.owners.map(&:email) + + owner_emails << user.email if user.present? + + owner_emails.uniq(&:downcase).each do |email_address| + Notify.try(email_method_name, settings, email_address).deliver_later + end + end + + def notify_project_owners_and_user_about_result(user: nil) + notify_project_owners_and_user_with_email( + email_method_name: :service_desk_verification_result_email, + user: user + ) + end + + def error_feature_flag_disabled + error_response('Feature flag service_desk_custom_email is not enabled') + end + + def error_response(message) + ServiceResponse.error(message: message) + end + + def error_not_verified(error_identifier) + ServiceResponse.error( + message: _('ServiceDesk|Custom email address could not be verified.'), + reason: error_identifier.to_s + ) + end + end + end +end diff --git a/app/services/service_desk/custom_email_verifications/create_service.rb b/app/services/service_desk/custom_email_verifications/create_service.rb new file mode 100644 index 00000000000..db518bfdf24 --- /dev/null +++ b/app/services/service_desk/custom_email_verifications/create_service.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +module ServiceDesk + module CustomEmailVerifications + class CreateService < BaseService + attr_reader :ramp_up_error + + def execute + return error_feature_flag_disabled unless Feature.enabled?(:service_desk_custom_email, project) + return error_settings_missing unless settings.present? + return error_user_not_authorized unless can?(current_user, :admin_project, project) + + update_settings + notify_project_owners_and_user_about_verification_start + send_verification_email_and_catch_delivery_errors + + if ramp_up_error + handle_error_case + else + ServiceResponse.success + end + end + + private + + def verification + @verification ||= settings.custom_email_verification || + ServiceDesk::CustomEmailVerification.new(project_id: settings.project_id) + end + + def update_settings + settings.update!(custom_email_enabled: false) if settings.custom_email_enabled? + + verification.mark_as_started!(current_user) + # We use verification association from project, to use it in email, we need to reset it here. + project.reset + end + + def notify_project_owners_and_user_about_verification_start + notify_project_owners_and_user_with_email( + email_method_name: :service_desk_verification_triggered_email, + user: current_user + ) + end + + def send_verification_email_and_catch_delivery_errors + # Send this synchronously as we need to get direct feedback on delivery errors. + Notify.service_desk_custom_email_verification_email(settings).deliver + rescue SocketError, OpenSSL::SSL::SSLError + # e.g. host not found or host certificate issues + @ramp_up_error = :smtp_host_issue + rescue Net::SMTPAuthenticationError + # incorrect username or password + @ramp_up_error = :invalid_credentials + end + + def handle_error_case + notify_project_owners_and_user_about_result(user: current_user) + + verification.mark_as_failed!(ramp_up_error) + + error_not_verified(ramp_up_error) + end + + def error_settings_missing + error_response(_('ServiceDesk|Service Desk setting missing')) + end + + def error_user_not_authorized + error_response(_('ServiceDesk|User cannot manage project.')) + end + end + end +end diff --git a/app/services/service_desk/custom_email_verifications/update_service.rb b/app/services/service_desk/custom_email_verifications/update_service.rb new file mode 100644 index 00000000000..813624cde23 --- /dev/null +++ b/app/services/service_desk/custom_email_verifications/update_service.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +module ServiceDesk + module CustomEmailVerifications + class UpdateService < BaseService + EMAIL_TOKEN_REGEXP = /Verification token: ([A-Za-z0-9_-]{12})/ + + def execute + return error_feature_flag_disabled unless Feature.enabled?(:service_desk_custom_email, project) + return error_parameter_missing if settings.blank? || verification.blank? + return error_already_finished if already_finished_and_no_mail? + return error_already_failed if already_failed_and_no_mail? + + verification_error = verify + + settings.update!(custom_email_enabled: false) if settings.custom_email_enabled? + + notify_project_owners_and_user_about_result(user: verification.triggerer) + + if verification_error.present? + verification.mark_as_failed!(verification_error) + + error_not_verified(verification_error) + else + verification.mark_as_finished! + + ServiceResponse.success + end + end + + private + + def mail + params[:mail] + end + + def verification + @verification ||= settings.custom_email_verification + end + + def already_finished_and_no_mail? + verification.finished? && mail.blank? + end + + def already_failed_and_no_mail? + verification.failed? && mail.blank? + end + + def verify + return :mail_not_received_within_timeframe if mail_not_received_within_timeframe? + return :incorrect_from if incorrect_from? + return :incorrect_token if incorrect_token? + + nil + end + + def mail_not_received_within_timeframe? + # (For completeness) also raise if no email provided + mail.blank? || !verification.in_timeframe? + end + + def incorrect_from? + # Does the email forwarder preserve the FROM header? + mail.from.first != settings.custom_email + end + + def incorrect_token? + message, _stripped_text = Gitlab::Email::ReplyParser.new(mail).execute + + scan_result = message.scan(EMAIL_TOKEN_REGEXP) + + return true if scan_result.empty? + + scan_result.first.first != verification.token + end + + def error_parameter_missing + error_response(_('ServiceDesk|Service Desk setting or verification object missing')) + end + + def error_already_finished + error_response(_('ServiceDesk|Custom email address has already been verified.')) + end + + def error_already_failed + error_response(_('ServiceDesk|Custom email address verification has already been processed and failed.')) + end + end + end +end diff --git a/app/services/service_ping/submit_service.rb b/app/services/service_ping/submit_service.rb index 6d39174b6c7..72d0c022609 100644 --- a/app/services/service_ping/submit_service.rb +++ b/app/services/service_ping/submit_service.rb @@ -3,7 +3,7 @@ module ServicePing class SubmitService PRODUCTION_BASE_URL = 'https://version.gitlab.com' - STAGING_BASE_URL = 'https://gitlab-services-version-gitlab-com-staging.gs-staging.gitlab.org' + STAGING_BASE_URL = 'https://gitlab-org-gitlab-services-version-gitlab-com-staging.version-staging.gitlab.org' USAGE_DATA_PATH = 'usage_data' ERROR_PATH = 'usage_ping_errors' METADATA_PATH = 'usage_ping_metadata' diff --git a/app/services/snippets/create_service.rb b/app/services/snippets/create_service.rb index a62d5290271..569b8b76518 100644 --- a/app/services/snippets/create_service.rb +++ b/app/services/snippets/create_service.rb @@ -2,11 +2,9 @@ module Snippets class CreateService < Snippets::BaseService - # NOTE: For Issues::CreateService, we require the spam_params and do not default it to nil, because - # spam_checking is likely to be necessary. - def initialize(project:, spam_params:, current_user: nil, params: {}) + def initialize(project:, current_user: nil, params: {}, perform_spam_check: true) super(project: project, current_user: current_user, params: params) - @spam_params = spam_params + @perform_spam_check = perform_spam_check end def execute @@ -20,16 +18,12 @@ module Snippets @snippet.author = current_user - Spam::SpamActionService.new( - spammable: @snippet, - spam_params: spam_params, - user: current_user, - action: :create, - extra_features: { files: file_paths_to_commit } - ).execute + if perform_spam_check + @snippet.check_for_spam(user: current_user, action: :create, extra_features: { files: file_paths_to_commit }) + end if save_and_commit - UserAgentDetailService.new(spammable: @snippet, spam_params: spam_params).create + UserAgentDetailService.new(spammable: @snippet, perform_spam_check: perform_spam_check).create Gitlab::UsageDataCounters::SnippetCounter.count(:create) move_temporary_files @@ -42,7 +36,7 @@ module Snippets private - attr_reader :snippet, :spam_params + attr_reader :snippet, :perform_spam_check def build_from_params if project diff --git a/app/services/snippets/update_service.rb b/app/services/snippets/update_service.rb index 067680f2abc..662e31a93aa 100644 --- a/app/services/snippets/update_service.rb +++ b/app/services/snippets/update_service.rb @@ -6,12 +6,9 @@ module Snippets UpdateError = Class.new(StandardError) - # NOTE: For Snippets::UpdateService, we default the spam_params to nil, because spam_checking is not - # necessary in many cases, and we don't want every caller to have to explicitly pass it as nil - # to disable spam checking. - def initialize(project:, current_user: nil, params: {}, spam_params: nil) + def initialize(project:, current_user: nil, params: {}, perform_spam_check: false) super(project: project, current_user: current_user, params: params) - @spam_params = spam_params + @perform_spam_check = perform_spam_check end def execute(snippet) @@ -25,13 +22,9 @@ module Snippets files = snippet.all_files.map { |f| { path: f } } + file_paths_to_commit - Spam::SpamActionService.new( - spammable: snippet, - spam_params: spam_params, - user: current_user, - action: :update, - extra_features: { files: files } - ).execute + if perform_spam_check + snippet.check_for_spam(user: current_user, action: :update, extra_features: { files: files }) + end if save_and_commit(snippet) Gitlab::UsageDataCounters::SnippetCounter.count(:update) @@ -44,7 +37,7 @@ module Snippets private - attr_reader :spam_params + attr_reader :perform_spam_check def visibility_changed?(snippet) visibility_level && visibility_level.to_i != snippet.visibility_level diff --git a/app/services/spam/spam_action_service.rb b/app/services/spam/spam_action_service.rb index 7c96f003e46..0527412e9bc 100644 --- a/app/services/spam/spam_action_service.rb +++ b/app/services/spam/spam_action_service.rb @@ -4,22 +4,35 @@ module Spam class SpamActionService include SpamConstants - def initialize(spammable:, spam_params:, user:, action:, extra_features: {}) + def initialize(spammable:, user:, action:, extra_features: {}) @target = spammable - @spam_params = spam_params @user = user @action = action @extra_features = extra_features end - # rubocop:disable Metrics/AbcSize def execute - # If spam_params is passed as `nil`, no check will be performed. This is the easiest way to allow - # composed services which may not need to do spam checking to "opt out". For example, when - # MoveService is calling CreateService, spam checking is not necessary, as no new content is - # being created. return ServiceResponse.success(message: 'Skipped spam check because spam_params was not present') unless spam_params + return ServiceResponse.success(message: 'Skipped spam check because user was not present') unless user + if target.supports_recaptcha? + execute_with_captcha_support + else + execute_spam_check + end + end + + delegate :check_for_spam?, to: :target + + private + + attr_reader :user, :action, :target, :spam_log, :extra_features + + def spam_params + Gitlab::RequestContext.instance.spam_params + end + + def execute_with_captcha_support recaptcha_verified = Captcha::CaptchaVerificationService.new(spam_params: spam_params).execute if recaptcha_verified @@ -28,20 +41,17 @@ module Spam SpamLog.verify_recaptcha!(user_id: user.id, id: spam_params.spam_log_id) ServiceResponse.success(message: "CAPTCHA successfully verified") else - return ServiceResponse.success(message: 'Skipped spam check because user was allowlisted') if allowlisted?(user) - return ServiceResponse.success(message: 'Skipped spam check because it was not required') unless check_for_spam?(user: user) - - perform_spam_service_check - ServiceResponse.success(message: "Spam check performed. Check #{target.class.name} spammable model for any errors or CAPTCHA requirement") + execute_spam_check end end - # rubocop:enable Metrics/AbcSize - delegate :check_for_spam?, to: :target - - private + def execute_spam_check + return ServiceResponse.success(message: 'Skipped spam check because user was allowlisted') if allowlisted?(user) + return ServiceResponse.success(message: 'Skipped spam check because it was not required') unless check_for_spam?(user: user) - attr_reader :user, :action, :target, :spam_params, :spam_log, :extra_features + perform_spam_service_check + ServiceResponse.success(message: "Spam check performed. Check #{target.class.name} spammable model for any errors or CAPTCHA requirement") + end ## # In order to be proceed to the spam check process, the target must be diff --git a/app/services/spam/spam_verdict_service.rb b/app/services/spam/spam_verdict_service.rb index 1279adf327b..2ecd431fd91 100644 --- a/app/services/spam/spam_verdict_service.rb +++ b/app/services/spam/spam_verdict_service.rb @@ -68,7 +68,7 @@ module Spam begin result = spamcheck_client.spam?(spammable: target, user: user, context: context, extra_features: extra_features) - if result.evaluated? && Feature.enabled?(:user_spam_scores) + if result.evaluated? Abuse::TrustScore.create!(user: user, score: result.score, source: :spamcheck) end diff --git a/app/services/tasks_to_be_done/base_service.rb b/app/services/tasks_to_be_done/base_service.rb index 1c74e803e0b..1d50e5081ff 100644 --- a/app/services/tasks_to_be_done/base_service.rb +++ b/app/services/tasks_to_be_done/base_service.rb @@ -19,7 +19,7 @@ module TasksToBeDone update_service = Issues::UpdateService.new(container: project, current_user: current_user, params: { add_assignee_ids: params[:assignee_ids] }) update_service.execute(issue) else - create_service = Issues::CreateService.new(container: project, current_user: current_user, params: params, spam_params: nil) + create_service = Issues::CreateService.new(container: project, current_user: current_user, params: params, perform_spam_check: false) create_service.execute end end diff --git a/app/services/user_agent_detail_service.rb b/app/services/user_agent_detail_service.rb index 01a98a15869..ccb5cec2df8 100644 --- a/app/services/user_agent_detail_service.rb +++ b/app/services/user_agent_detail_service.rb @@ -1,15 +1,16 @@ # frozen_string_literal: true class UserAgentDetailService - def initialize(spammable:, spam_params:) + def initialize(spammable:, perform_spam_check:) @spammable = spammable - @spam_params = spam_params + @perform_spam_check = perform_spam_check end def create - unless spam_params&.user_agent && spam_params&.ip_address - messasge = 'Skipped UserAgentDetail creation because necessary spam_params were not provided' - return ServiceResponse.success(message: messasge) + spam_params = Gitlab::RequestContext.instance.spam_params + if !perform_spam_check || spam_params&.user_agent.blank? || spam_params&.ip_address.blank? + message = 'Skipped UserAgentDetail creation because necessary spam_params were not provided' + return ServiceResponse.success(message: message) end spammable.create_user_agent_detail(user_agent: spam_params.user_agent, ip_address: spam_params.ip_address) @@ -17,5 +18,5 @@ class UserAgentDetailService private - attr_reader :spammable, :spam_params + attr_reader :spammable, :perform_spam_check end diff --git a/app/services/users/activate_service.rb b/app/services/users/activate_service.rb new file mode 100644 index 00000000000..dfc2996bcce --- /dev/null +++ b/app/services/users/activate_service.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module Users + class ActivateService < BaseService + def initialize(current_user) + @current_user = current_user + end + + def execute(user) + return error(_('You are not authorized to perform this action'), :forbidden) unless allowed? + + return error(_('Error occurred. A blocked user must be unblocked to be activated'), :forbidden) if user.blocked? + + return success(_('Successfully activated')) if user.active? + + if user.activate + after_activate_hook(user) + log_event(user) + success(_('Successfully activated')) + else + error(user.errors.full_messages.to_sentence, :unprocessable_entity) + end + end + + private + + attr_reader :current_user + + def allowed? + can?(current_user, :admin_all_resources) + end + + def after_activate_hook(user) + # overridden by EE module + end + + def log_event(user) + Gitlab::AppLogger.info(message: 'User activated', user: user.username.to_s, email: user.email.to_s, + activated_by: current_user.username.to_s, ip_address: current_user.current_sign_in_ip.to_s) + end + + def success(message) + ::ServiceResponse.success(message: message) + end + + def error(message, reason) + ::ServiceResponse.error(message: message, reason: reason) + end + end +end + +Users::ActivateService.prepend_mod_with('Users::ActivateService') # rubocop: disable Cop/InjectEnterpriseEditionModule diff --git a/app/services/users/set_namespace_commit_email_service.rb b/app/services/users/set_namespace_commit_email_service.rb new file mode 100644 index 00000000000..30ee597120d --- /dev/null +++ b/app/services/users/set_namespace_commit_email_service.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +module Users + class SetNamespaceCommitEmailService + include Gitlab::Allowable + + attr_reader :current_user, :target_user, :namespace, :email_id + + def initialize(current_user, namespace, email_id, params) + @current_user = current_user + @target_user = params.delete(:user) || current_user + @namespace = namespace + @email_id = email_id + end + + def execute + return error(_('Namespace must be provided.')) if namespace.nil? + + unless can?(current_user, :admin_user_email_address, target_user) + return error(_("User doesn't exist or you don't have permission to change namespace commit emails.")) + end + + unless can?(target_user, :read_namespace, namespace) + return error(_("Namespace doesn't exist or you don't have permission.")) + end + + email = target_user.emails.find_by(id: email_id) unless email_id.nil? # rubocop: disable CodeReuse/ActiveRecord + existing_namespace_commit_email = target_user.namespace_commit_email_for_namespace(namespace) + if existing_namespace_commit_email.nil? + return error(_('Email must be provided.')) if email.nil? + + create_namespace_commit_email(email) + elsif email_id.nil? + remove_namespace_commit_email(existing_namespace_commit_email) + else + update_namespace_commit_email(existing_namespace_commit_email, email) + end + end + + private + + def remove_namespace_commit_email(namespace_commit_email) + namespace_commit_email.destroy + success(nil) + end + + def create_namespace_commit_email(email) + namespace_commit_email = ::Users::NamespaceCommitEmail.new( + user: target_user, + namespace: namespace, + email: email + ) + + save_namespace_commit_email(namespace_commit_email) + end + + def update_namespace_commit_email(namespace_commit_email, email) + namespace_commit_email.email = email + + save_namespace_commit_email(namespace_commit_email) + end + + def save_namespace_commit_email(namespace_commit_email) + if !namespace_commit_email.save + error_in_save(namespace_commit_email) + else + success(namespace_commit_email) + end + end + + def success(namespace_commit_email) + ServiceResponse.success(payload: { + namespace_commit_email: namespace_commit_email + }) + end + + def error(message) + ServiceResponse.error(message: message) + end + + def error_in_save(namespace_commit_email) + return error(_('Failed to save namespace commit email.')) if namespace_commit_email.errors.empty? + + error(namespace_commit_email.errors.full_messages.to_sentence) + end + end +end diff --git a/app/services/webauthn/destroy_service.rb b/app/services/webauthn/destroy_service.rb new file mode 100644 index 00000000000..afad2680d42 --- /dev/null +++ b/app/services/webauthn/destroy_service.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Webauthn + class DestroyService < BaseService + attr_reader :webauthn_registration, :user, :current_user + + def initialize(current_user, user, webauthn_registrations_id) + @current_user = current_user + @user = user + @webauthn_registration = user.webauthn_registrations.find(webauthn_registrations_id) + end + + def execute + return error(_('You are not authorized to perform this action')) unless authorized? + + webauthn_registration.destroy + user.reset_backup_codes! if last_two_factor_registration? + end + + private + + def last_two_factor_registration? + user.webauthn_registrations.empty? && !user.otp_required_for_login? + end + + def authorized? + current_user.can?(:disable_two_factor, user) + end + end +end diff --git a/app/services/work_items/callbacks/award_emoji.rb b/app/services/work_items/callbacks/award_emoji.rb new file mode 100644 index 00000000000..6344813d4b9 --- /dev/null +++ b/app/services/work_items/callbacks/award_emoji.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module WorkItems + module Callbacks + class AwardEmoji < Base + def before_update + return unless params.present? && params.key?(:name) && params.key?(:action) + return unless has_permission?(:award_emoji) + + execute_emoji_service(params[:action], params[:name]) + end + + private + + def execute_emoji_service(action, name) + class_name = { + add: ::AwardEmojis::AddService, + remove: ::AwardEmojis::DestroyService + } + + raise_error(invalid_action_error(action)) unless class_name.key?(action) + + result = class_name[action].new(work_item, name, current_user).execute + + raise_error(result[:message]) if result[:status] == :error + end + + def invalid_action_error(key) + format(_("%{key} is not a valid action."), key: key) + end + end + end +end diff --git a/app/services/work_items/callbacks/base.rb b/app/services/work_items/callbacks/base.rb new file mode 100644 index 00000000000..c91e2b37d10 --- /dev/null +++ b/app/services/work_items/callbacks/base.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module WorkItems + module Callbacks + class Base < Issuable::Callbacks::Base + alias_method :work_item, :issuable + + def raise_error(message) + raise ::WorkItems::Widgets::BaseService::WidgetError, message + end + end + end +end diff --git a/app/services/work_items/create_and_link_service.rb b/app/services/work_items/create_and_link_service.rb index ae09e44b952..ba22b170a28 100644 --- a/app/services/work_items/create_and_link_service.rb +++ b/app/services/work_items/create_and_link_service.rb @@ -6,12 +6,12 @@ module WorkItems # This class should always be run inside a transaction as we could end up with # new work items that were never associated with other work items as expected. class CreateAndLinkService - def initialize(project:, spam_params:, current_user: nil, params: {}, link_params: {}) + def initialize(project:, perform_spam_check: true, current_user: nil, params: {}, link_params: {}) @project = project @current_user = current_user @params = params @link_params = link_params - @spam_params = spam_params + @perform_spam_check = perform_spam_check end def execute @@ -19,7 +19,7 @@ module WorkItems container: @project, current_user: @current_user, params: @params.merge(title: @params[:title].strip).reverse_merge(confidential: confidential_parent), - spam_params: @spam_params + perform_spam_check: @perform_spam_check ).execute return create_result if create_result.error? diff --git a/app/services/work_items/create_from_task_service.rb b/app/services/work_items/create_from_task_service.rb index ced5b17a21c..25ec3169fe7 100644 --- a/app/services/work_items/create_from_task_service.rb +++ b/app/services/work_items/create_from_task_service.rb @@ -2,11 +2,11 @@ module WorkItems class CreateFromTaskService - def initialize(work_item:, spam_params:, current_user: nil, work_item_params: {}) + def initialize(work_item:, perform_spam_check: true, current_user: nil, work_item_params: {}) @work_item = work_item @current_user = current_user @work_item_params = work_item_params - @spam_params = spam_params + @perform_spam_check = perform_spam_check @errors = [] end @@ -16,7 +16,7 @@ module WorkItems project: @work_item.project, current_user: @current_user, params: @work_item_params.slice(:title, :work_item_type_id), - spam_params: @spam_params, + perform_spam_check: @perform_spam_check, link_params: { parent_work_item: @work_item } ).execute diff --git a/app/services/work_items/create_service.rb b/app/services/work_items/create_service.rb index ae355dc6d96..903736cf662 100644 --- a/app/services/work_items/create_service.rb +++ b/app/services/work_items/create_service.rb @@ -5,12 +5,12 @@ module WorkItems extend ::Gitlab::Utils::Override include WidgetableService - def initialize(container:, spam_params:, current_user: nil, params: {}, widget_params: {}) + def initialize(container:, perform_spam_check: true, current_user: nil, params: {}, widget_params: {}) super( container: container, current_user: current_user, params: params, - spam_params: spam_params, + perform_spam_check: perform_spam_check, build_service: ::WorkItems::BuildService.new(container: container, current_user: current_user, params: params) ) @widget_params = widget_params diff --git a/app/services/work_items/delete_task_service.rb b/app/services/work_items/delete_task_service.rb index 3d66716543a..4c0ee2f827d 100644 --- a/app/services/work_items/delete_task_service.rb +++ b/app/services/work_items/delete_task_service.rb @@ -22,7 +22,7 @@ module WorkItems current_user: @current_user ).execute - break ::ServiceResponse.error(message: replacement_result.errors, http_status: 422) if replacement_result.error? + next ::ServiceResponse.error(message: replacement_result.errors, http_status: 422) if replacement_result.error? delete_result = ::WorkItems::DeleteService.new( container: @task.project, diff --git a/app/services/work_items/update_service.rb b/app/services/work_items/update_service.rb index defdeebfed8..27b318d280f 100644 --- a/app/services/work_items/update_service.rb +++ b/app/services/work_items/update_service.rb @@ -5,10 +5,10 @@ module WorkItems extend Gitlab::Utils::Override include WidgetableService - def initialize(container:, current_user: nil, params: {}, spam_params: nil, widget_params: {}) + def initialize(container:, current_user: nil, params: {}, perform_spam_check: false, widget_params: {}) params[:widget_params] = true if widget_params.present? - super(container: container, current_user: current_user, params: params, spam_params: spam_params) + super(container: container, current_user: current_user, params: params, perform_spam_check: perform_spam_check) @widget_params = widget_params end @@ -59,6 +59,7 @@ module WorkItems super end + override :after_update def after_update(work_item, old_associations) super diff --git a/app/services/work_items/widgets/award_emoji_service/update_service.rb b/app/services/work_items/widgets/award_emoji_service/update_service.rb deleted file mode 100644 index 7c58c0c9af9..00000000000 --- a/app/services/work_items/widgets/award_emoji_service/update_service.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -module WorkItems - module Widgets - module AwardEmojiService - class UpdateService < WorkItems::Widgets::BaseService - def before_update_in_transaction(params:) - return unless params.present? && params.key?(:name) && params.key?(:action) - return unless has_permission?(:award_emoji) - - service_response!(service_result(params[:action], params[:name])) - end - - private - - def service_result(action, name) - class_name = { - add: ::AwardEmojis::AddService, - remove: ::AwardEmojis::DestroyService - } - - return invalid_action_error(action) unless class_name.key?(action) - - class_name[action].new(work_item, name, current_user).execute - end - - def invalid_action_error(key) - error(format(_("%{key} is not a valid action."), key: key)) - end - end - end - end -end |