diff options
Diffstat (limited to 'app/services')
| -rw-r--r-- | app/services/groups/update_service.rb | 2 | ||||
| -rw-r--r-- | app/services/issues/update_service.rb | 2 | ||||
| -rw-r--r-- | app/services/members/base_service.rb | 6 | ||||
| -rw-r--r-- | app/services/members/update_service.rb | 9 | ||||
| -rw-r--r-- | app/services/merge_requests/build_service.rb | 24 | ||||
| -rw-r--r-- | app/services/projects/lfs_pointers/lfs_download_service.rb | 35 | ||||
| -rw-r--r-- | app/services/projects/update_service.rb | 4 |
7 files changed, 63 insertions, 19 deletions
diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index 0bf0e967dcc..83ffc3dc8cd 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -31,7 +31,7 @@ module Groups def after_update if group.previous_changes.include?(:visibility_level) && group.private? # don't enqueue immediately to prevent todos removal in case of a mistake - TodosDestroyer::GroupPrivateWorker.perform_in(1.hour, group.id) + TodosDestroyer::GroupPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, group.id) end end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index fba252b0bae..67b246a6bdb 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -38,7 +38,7 @@ module Issues if issue.previous_changes.include?('confidential') # don't enqueue immediately to prevent todos removal in case of a mistake - TodosDestroyer::ConfidentialIssueWorker.perform_in(1.hour, issue.id) if issue.confidential? + TodosDestroyer::ConfidentialIssueWorker.perform_in(Todo::WAIT_FOR_DELETE, issue.id) if issue.confidential? create_confidentiality_note(issue) end diff --git a/app/services/members/base_service.rb b/app/services/members/base_service.rb index d734571f835..e78affff797 100644 --- a/app/services/members/base_service.rb +++ b/app/services/members/base_service.rb @@ -47,5 +47,11 @@ module Members raise "Unknown action '#{action}' on #{member}!" end end + + def enqueue_delete_todos(member) + type = member.is_a?(GroupMember) ? 'Group' : 'Project' + # don't enqueue immediately to prevent todos removal in case of a mistake + TodosDestroyer::EntityLeaveWorker.perform_in(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, type) + end end end diff --git a/app/services/members/update_service.rb b/app/services/members/update_service.rb index 1f5618dae53..ff8d5c1d8c9 100644 --- a/app/services/members/update_service.rb +++ b/app/services/members/update_service.rb @@ -10,9 +10,18 @@ module Members if member.update(params) after_execute(action: permission, old_access_level: old_access_level, member: member) + + # Deletes only confidential issues todos for guests + enqueue_delete_todos(member) if downgrading_to_guest? end member end + + private + + def downgrading_to_guest? + params[:access_level] == Gitlab::Access::GUEST + end end end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 6c69452e2ab..fe29df22868 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -17,7 +17,7 @@ module MergeRequests merge_request.source_project = find_source_project merge_request.target_project = find_target_project merge_request.target_branch = find_target_branch - merge_request.can_be_created = branches_valid? + merge_request.can_be_created = projects_and_branches_valid? # compare branches only if branches are valid, otherwise # compare_branches may raise an error @@ -48,15 +48,19 @@ module MergeRequests to: :merge_request def find_source_project - return source_project if source_project.present? && can?(current_user, :read_project, source_project) + return source_project if source_project.present? && can?(current_user, :create_merge_request_from, source_project) project end def find_target_project - return target_project if target_project.present? && can?(current_user, :read_project, target_project) + return target_project if target_project.present? && can?(current_user, :create_merge_request_in, target_project) - project.default_merge_request_target + target_project = project.default_merge_request_target + + return target_project if target_project.present? && can?(current_user, :create_merge_request_in, target_project) + + project end def find_target_branch @@ -71,10 +75,11 @@ module MergeRequests params[:target_branch].present? end - def branches_valid? + def projects_and_branches_valid? + return false if source_project.nil? || target_project.nil? return false unless source_branch_specified? || target_branch_specified? - validate_branches + validate_projects_and_branches errors.blank? end @@ -93,7 +98,12 @@ module MergeRequests end end - def validate_branches + def validate_projects_and_branches + merge_request.validate_target_project + merge_request.validate_fork + + return if errors.any? + add_error('You must select source and target branch') unless branches_present? add_error('You must select different branches') if same_source_and_target? add_error("Source branch \"#{source_branch}\" does not exist") unless source_branch_exists? diff --git a/app/services/projects/lfs_pointers/lfs_download_service.rb b/app/services/projects/lfs_pointers/lfs_download_service.rb index f9b9781ad5f..b5128443435 100644 --- a/app/services/projects/lfs_pointers/lfs_download_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_service.rb @@ -12,28 +12,43 @@ module Projects return if LfsObject.exists?(oid: oid) - sanitized_uri = Gitlab::UrlSanitizer.new(url) - Gitlab::UrlBlocker.validate!(sanitized_uri.sanitized_url, protocols: VALID_PROTOCOLS) + sanitized_uri = sanitize_url!(url) with_tmp_file(oid) do |file| - size = download_and_save_file(file, sanitized_uri) - lfs_object = LfsObject.new(oid: oid, size: size, file: file) + download_and_save_file(file, sanitized_uri) + lfs_object = LfsObject.new(oid: oid, size: file.size, file: file) project.all_lfs_objects << lfs_object end + rescue Gitlab::UrlBlocker::BlockedUrlError => e + Rails.logger.error("LFS file with oid #{oid} couldn't be downloaded: #{e.message}") rescue StandardError => e - Rails.logger.error("LFS file with oid #{oid} could't be downloaded from #{sanitized_uri.sanitized_url}: #{e.message}") + Rails.logger.error("LFS file with oid #{oid} couldn't be downloaded from #{sanitized_uri.sanitized_url}: #{e.message}") end # rubocop: enable CodeReuse/ActiveRecord private + def sanitize_url!(url) + Gitlab::UrlSanitizer.new(url).tap do |sanitized_uri| + # Just validate that HTTP/HTTPS protocols are used. The + # subsequent Gitlab::HTTP.get call will do network checks + # based on the settings. + Gitlab::UrlBlocker.validate!(sanitized_uri.sanitized_url, + protocols: VALID_PROTOCOLS) + end + end + def download_and_save_file(file, sanitized_uri) - IO.copy_stream(open(sanitized_uri.sanitized_url, headers(sanitized_uri)), file) # rubocop:disable Security/Open + response = Gitlab::HTTP.get(sanitized_uri.sanitized_url, headers(sanitized_uri)) do |fragment| + file.write(fragment) + end + + raise StandardError, "Received error code #{response.code}" unless response.success? end def headers(sanitized_uri) - {}.tap do |headers| + query_options.tap do |headers| credentials = sanitized_uri.credentials if credentials[:user].present? || credentials[:password].present? @@ -43,10 +58,14 @@ module Projects end end + def query_options + { stream_body: true } + end + def with_tmp_file(oid) create_tmp_storage_dir - File.open(File.join(tmp_storage_dir, oid), 'w') { |file| yield file } + File.open(File.join(tmp_storage_dir, oid), 'wb') { |file| yield file } end def create_tmp_storage_dir diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 93e48fc0199..dd1b9680ece 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -61,9 +61,9 @@ module Projects if project.previous_changes.include?(:visibility_level) && project.private? # don't enqueue immediately to prevent todos removal in case of a mistake - TodosDestroyer::ProjectPrivateWorker.perform_in(1.hour, project.id) + TodosDestroyer::ProjectPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, project.id) elsif (project_changed_feature_keys & todos_features_changes).present? - TodosDestroyer::PrivateFeaturesWorker.perform_in(1.hour, project.id) + TodosDestroyer::PrivateFeaturesWorker.perform_in(Todo::WAIT_FOR_DELETE, project.id) end if project.previous_changes.include?('path') |