From 478ed161ffd67449c77e22d1ad3723ed97f85ff5 Mon Sep 17 00:00:00 2001 From: Pirate Praveen Date: Sat, 28 Mar 2020 13:19:24 +0530 Subject: [PATCH] New upstream version 12.8.8 --- CHANGELOG-EE.md | 11 + CHANGELOG.md | 30 ++ GITALY_SERVER_VERSION | 2 +- GITLAB_WORKHORSE_VERSION | 2 +- Gemfile.lock | 2 +- VERSION | 2 +- .../javascripts/frequent_items/utils.js | 21 +- .../concerns/hotlink_interceptor.rb | 15 + app/controllers/import/fogbugz_controller.rb | 18 + .../projects/mirrors_controller.rb | 2 +- .../projects/repositories_controller.rb | 2 + app/helpers/x509_helper.rb | 19 + app/models/group.rb | 3 + app/models/issue.rb | 6 +- .../merge_request_poll_widget_entity.rb | 2 +- app/uploaders/object_storage.rb | 2 +- .../x509/_certificate_details.html.haml | 10 +- .../projects/mirrors/_mirror_repos.html.haml | 50 ++- derailed-benchmarks/CHANGELOG.md | 5 + .../derailed_benchmarks.gemspec | 2 + .../core_ext/kernel_require.rb | 32 +- .../lib/derailed_benchmarks/load_tasks.rb | 13 +- .../lib/derailed_benchmarks/stats_from_dir.rb | 27 +- .../lib/derailed_benchmarks/tasks.rb | 6 +- .../lib/derailed_benchmarks/version.rb | 2 +- .../stats_from_dir_test.rb | 16 + .../test/integration/tasks_test.rb | 2 +- .../test/rails_app/config/application.rb | 2 + lib/api/helpers.rb | 4 + lib/api/repositories.rb | 2 + lib/api/snippets.rb | 2 + lib/api/triggers.rb | 2 + lib/gitlab/auth.rb | 6 +- lib/gitlab/gfm/uploads_rewriter.rb | 2 + lib/gitlab/hotlinking_detector.rb | 52 +++ lib/gitlab/import_export/attribute_cleaner.rb | 9 +- lib/gitlab/middleware/multipart.rb | 18 +- lib/gitlab/regex.rb | 8 + lib/gitlab/x509/commit.rb | 17 +- lib/uploaded_file.rb | 2 +- locale/gitlab.pot | 6 + spec/controllers/groups_controller_spec.rb | 22 + .../import/fogbugz_controller_spec.rb | 29 ++ .../projects/mirrors_controller_spec.rb | 66 +++ .../projects/repositories_controller_spec.rb | 6 + spec/factories/pages_domains.rb | 32 +- spec/factories/serverless/domain_cluster.rb | 32 +- spec/features/projects/pages_spec.rb | 36 +- .../settings/repository_settings_spec.rb | 43 +- spec/fixtures/ssl_certificate.pem | 12 + spec/fixtures/ssl_key.pem | 16 + spec/helpers/x509_helper_spec.rb | 60 +++ spec/javascripts/frequent_items/utils_spec.js | 18 + .../filter/label_reference_filter_spec.rb | 7 +- .../filter/reference_redactor_filter_spec.rb | 64 ++- spec/lib/gitlab/auth_spec.rb | 41 ++ spec/lib/gitlab/gfm/uploads_rewriter_spec.rb | 10 + spec/lib/gitlab/hotlinking_detector_spec.rb | 75 ++++ .../import_export/attribute_cleaner_spec.rb | 3 + spec/lib/gitlab/middleware/multipart_spec.rb | 42 +- spec/lib/gitlab/regex_spec.rb | 32 +- spec/lib/gitlab/x509/commit_spec.rb | 33 ++ spec/lib/uploaded_file_spec.rb | 10 + spec/models/group_spec.rb | 3 + spec/models/issue_spec.rb | 401 +++++++++--------- spec/models/pages_domain_spec.rb | 4 +- spec/policies/issue_policy_spec.rb | 12 + spec/requests/api/groups_spec.rb | 28 ++ spec/requests/api/project_snippets_spec.rb | 24 ++ spec/requests/api/repositories_spec.rb | 12 + spec/requests/api/snippets_spec.rb | 10 + spec/requests/api/triggers_spec.rb | 44 +- spec/requests/jwt_controller_spec.rb | 15 + .../merge_request_poll_widget_entity_spec.rb | 11 +- spec/support/helpers/workhorse_helpers.rb | 2 +- .../middleware/multipart_shared_contexts.rb | 42 ++ .../hotlink_interceptor_shared_examples.rb | 87 ++++ spec/uploaders/object_storage_spec.rb | 13 + 78 files changed, 1382 insertions(+), 451 deletions(-) create mode 100644 app/controllers/concerns/hotlink_interceptor.rb create mode 100644 app/helpers/x509_helper.rb create mode 100644 lib/gitlab/hotlinking_detector.rb create mode 100644 spec/fixtures/ssl_certificate.pem create mode 100644 spec/fixtures/ssl_key.pem create mode 100644 spec/helpers/x509_helper_spec.rb create mode 100644 spec/lib/gitlab/hotlinking_detector_spec.rb create mode 100644 spec/support/shared_contexts/lib/gitlab/middleware/multipart_shared_contexts.rb create mode 100644 spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb diff --git a/CHANGELOG-EE.md b/CHANGELOG-EE.md index e72168bfc2..a2f417a16c 100644 --- a/CHANGELOG-EE.md +++ b/CHANGELOG-EE.md @@ -1,5 +1,16 @@ Please view this file on the master branch, on stable branches it's out of date. +## 12.8.7 (2020-03-16) + +### Fixed (1 change) + +- Allow multipart uploads for packages. !26387 + + +## 12.8.6 (2020-03-11) + +- No changes. + ## 12.8.5 - No changes. diff --git a/CHANGELOG.md b/CHANGELOG.md index e95c28605b..3f9b53846b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,36 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 12.8.8 (2020-03-26) + +### Security (17 changes) + +- Redact notes in moved confidential issues. +- Ignore empty remote_id params from Workhorse accelerated uploads. +- External user can not create personal snippet through API. +- Prevent malicious entry for group name. +- Restrict mirroring changes to admins only when mirroring is disabled. +- Reject all container registry requests from blocked users. +- Deny localhost requests on fogbugz importer. +- Change GitHub service integration token input to password. +- Add permission check for pipeline status of MR. +- Fix UploadRewriter Path Traversal vulnerability. +- Block hotlinking to repository archives. +- Restrict access to project pipeline metrics reports. +- vulnerability_feedback records should be restricted to a dev role and above. +- Exclude Carrierwave remote URL methods from import. +- Update Nokogiri to fix CVE-2020-7595. +- Prevent updating trigger by other maintainers. +- Fix XSS vulnerability in `admin/email` "Recipient Group" dropdown. + + +## 12.8.7 (2020-03-16) + +### Fixed (1 change, 1 of them is from the community) + +- Fix crl_url parsing and certificate visualization. !25876 (Roger Meier) + + ## 12.8.6 (2020-03-11) ### Security (1 change) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 51fbd82b8c..aef81b964a 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -12.8.6 +12.8.8 diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index 72963fb08c..a4ea2549bb 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -8.21.0 +8.21.1 diff --git a/Gemfile.lock b/Gemfile.lock index aa33bd4cd6..9735b9be93 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -652,7 +652,7 @@ GEM netrc (0.11.0) nio4r (2.5.2) no_proxy_fix (0.1.2) - nokogiri (1.10.7) + nokogiri (1.10.8) mini_portile2 (~> 2.4.0) nokogumbo (1.5.0) nokogiri diff --git a/VERSION b/VERSION index 51fbd82b8c..aef81b964a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -12.8.6 +12.8.8 diff --git a/app/assets/javascripts/frequent_items/utils.js b/app/assets/javascripts/frequent_items/utils.js index 5188d6118a..76741a8b23 100644 --- a/app/assets/javascripts/frequent_items/utils.js +++ b/app/assets/javascripts/frequent_items/utils.js @@ -45,8 +45,19 @@ export const updateExistingFrequentItem = (frequentItem, item) => { }; }; -export const sanitizeItem = item => ({ - ...item, - name: sanitize(item.name.toString(), { allowedTags: [] }), - namespace: sanitize(item.namespace.toString(), { allowedTags: [] }), -}); +export const sanitizeItem = item => { + // Only sanitize if the key exists on the item + const maybeSanitize = key => { + if (!Object.prototype.hasOwnProperty.call(item, key)) { + return {}; + } + + return { [key]: sanitize(item[key].toString(), { allowedTags: [] }) }; + }; + + return { + ...item, + ...maybeSanitize('name'), + ...maybeSanitize('namespace'), + }; +}; diff --git a/app/controllers/concerns/hotlink_interceptor.rb b/app/controllers/concerns/hotlink_interceptor.rb new file mode 100644 index 0000000000..712a10eab9 --- /dev/null +++ b/app/controllers/concerns/hotlink_interceptor.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module HotlinkInterceptor + extend ActiveSupport::Concern + + def intercept_hotlinking! + return render_406 if Gitlab::HotlinkingDetector.intercept_hotlinking?(request) + end + + private + + def render_406 + head :not_acceptable + end +end diff --git a/app/controllers/import/fogbugz_controller.rb b/app/controllers/import/fogbugz_controller.rb index 28ead8d44d..4fb6efde7f 100644 --- a/app/controllers/import/fogbugz_controller.rb +++ b/app/controllers/import/fogbugz_controller.rb @@ -3,6 +3,7 @@ class Import::FogbugzController < Import::BaseController before_action :verify_fogbugz_import_enabled before_action :user_map, only: [:new_user_map, :create_user_map] + before_action :verify_blocked_uri, only: :callback rescue_from Fogbugz::AuthenticationException, with: :fogbugz_unauthorized @@ -106,4 +107,21 @@ class Import::FogbugzController < Import::BaseController def verify_fogbugz_import_enabled render_404 unless fogbugz_import_enabled? end + + def verify_blocked_uri + Gitlab::UrlBlocker.validate!( + params[:uri], + { + allow_localhost: allow_local_requests?, + allow_local_network: allow_local_requests?, + schemes: %w(http https) + } + ) + rescue Gitlab::UrlBlocker::BlockedUrlError => e + redirect_to new_import_fogbugz_url, alert: _('Specified URL cannot be used: "%{reason}"') % { reason: e.message } + end + + def allow_local_requests? + Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services? + end end diff --git a/app/controllers/projects/mirrors_controller.rb b/app/controllers/projects/mirrors_controller.rb index dd1ea151de..936f89e58e 100644 --- a/app/controllers/projects/mirrors_controller.rb +++ b/app/controllers/projects/mirrors_controller.rb @@ -67,7 +67,7 @@ class Projects::MirrorsController < Projects::ApplicationController end def check_mirror_available! - Gitlab::CurrentSettings.current_application_settings.mirror_available || current_user&.admin? + render_404 unless can?(current_user, :admin_remote_mirror, project) end def mirror_params_attributes diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb index d0fb814948..a97f123d8d 100644 --- a/app/controllers/projects/repositories_controller.rb +++ b/app/controllers/projects/repositories_controller.rb @@ -3,11 +3,13 @@ class Projects::RepositoriesController < Projects::ApplicationController include ExtractsPath include StaticObjectExternalStorage + include HotlinkInterceptor prepend_before_action(only: [:archive]) { authenticate_sessionless_user!(:archive) } # Authorize before_action :require_non_empty_project, except: :create + before_action :intercept_hotlinking!, only: :archive before_action :assign_archive_vars, only: :archive before_action :assign_append_sha, only: :archive before_action :authorize_download_code! diff --git a/app/helpers/x509_helper.rb b/app/helpers/x509_helper.rb new file mode 100644 index 0000000000..c330b599d7 --- /dev/null +++ b/app/helpers/x509_helper.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'net/ldap/dn' + +module X509Helper + def x509_subject(subject, search_keys) + subjects = {} + + Net::LDAP::DN.new(subject).each_pair do |key, value| + if key.upcase.start_with?(*search_keys.map(&:upcase)) + subjects[key.upcase] = value + end + end + + subjects + rescue + {} + end +end diff --git a/app/models/group.rb b/app/models/group.rb index bf771bd040..83b2d17cb9 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -67,6 +67,9 @@ class Group < Namespace validates :variables, variable_duplicates: true validates :two_factor_grace_period, presence: true, numericality: { greater_than_or_equal_to: 0 } + validates :name, + format: { with: Gitlab::Regex.group_name_regex, + message: Gitlab::Regex.group_name_regex_message } add_authentication_token_field :runners_token, encrypted: -> { Feature.enabled?(:groups_tokens_optional_encryption, default_enabled: true) ? :optional : :required } diff --git a/app/models/issue.rb b/app/models/issue.rb index be702134ce..4c5bd3da67 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -319,10 +319,8 @@ class Issue < ApplicationRecord true elsif project.owner == user true - elsif confidential? - author == user || - assignees.include?(user) || - project.team.member?(user, Gitlab::Access::REPORTER) + elsif confidential? && !assignee_or_author?(user) + project.team.member?(user, Gitlab::Access::REPORTER) else project.public? || project.internal? && !user.external? || diff --git a/app/serializers/merge_request_poll_widget_entity.rb b/app/serializers/merge_request_poll_widget_entity.rb index a45026ea01..18e8ec0e7d 100644 --- a/app/serializers/merge_request_poll_widget_entity.rb +++ b/app/serializers/merge_request_poll_widget_entity.rb @@ -53,7 +53,7 @@ class MergeRequestPollWidgetEntity < Grape::Entity # CI related expose :has_ci?, as: :has_ci - expose :ci_status do |merge_request| + expose :ci_status, if: -> (mr, _) { presenter(mr).can_read_pipeline? } do |merge_request| presenter(merge_request).ci_status end diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index 450ebb00b4..f4a4adc734 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -318,7 +318,7 @@ module ObjectStorage def cache!(new_file = sanitized_file) # We intercept ::UploadedFile which might be stored on remote storage # We use that for "accelerated" uploads, where we store result on remote storage - if new_file.is_a?(::UploadedFile) && new_file.remote_id + if new_file.is_a?(::UploadedFile) && new_file.remote_id.present? return cache_remote_file!(new_file.remote_id, new_file.original_filename) end diff --git a/app/views/projects/commit/x509/_certificate_details.html.haml b/app/views/projects/commit/x509/_certificate_details.html.haml index 2357c6d803..51667010d6 100644 --- a/app/views/projects/commit/x509/_certificate_details.html.haml +++ b/app/views/projects/commit/x509/_certificate_details.html.haml @@ -1,17 +1,15 @@ .gpg-popover-certificate-details %strong= _('Certificate Subject') %ul - - signature.x509_certificate.subject.split(",").each do |i| - - if i.start_with?("CN", "O") - %li= i + - x509_subject(signature.x509_certificate.subject, ["CN", "O"]).map do |key, value| + %li= key + "=" + value %li= _('Subject Key Identifier:') %li.unstyled= signature.x509_certificate.subject_key_identifier.gsub(":", " ") .gpg-popover-certificate-details %strong= _('Certificate Issuer') %ul - - signature.x509_certificate.x509_issuer.subject.split(",").each do |i| - - if i.start_with?("CN", "OU", "O") - %li= i + - x509_subject(signature.x509_certificate.x509_issuer.subject, ["CN", "OU", "O"]).map do |key, value| + %li= key + "=" + value %li= _('Subject Key Identifier:') %li.unstyled= signature.x509_certificate.x509_issuer.subject_key_identifier.gsub(":", " ") diff --git a/app/views/projects/mirrors/_mirror_repos.html.haml b/app/views/projects/mirrors/_mirror_repos.html.haml index 80d2d2afad..4004c4f4b0 100644 --- a/app/views/projects/mirrors/_mirror_repos.html.haml +++ b/app/views/projects/mirrors/_mirror_repos.html.haml @@ -1,7 +1,9 @@ - expanded = expanded_by_default? - protocols = Gitlab::UrlSanitizer::ALLOWED_SCHEMES.join('|') +- mirror_settings_enabled = can?(current_user, :admin_remote_mirror, @project) +- mirror_settings_class = "#{'expanded' if expanded} #{'js-mirror-settings' if mirror_settings_enabled}".strip -%section.settings.project-mirror-settings.js-mirror-settings.no-animate#js-push-remote-settings{ class: ('expanded' if expanded), data: { qa_selector: 'mirroring_repositories_settings_section' } } +%section.settings.project-mirror-settings.no-animate#js-push-remote-settings{ class: mirror_settings_class, data: { qa_selector: 'mirroring_repositories_settings_section' } } .settings-header %h4= _('Mirroring repositories') %button.btn.js-settings-toggle @@ -11,26 +13,32 @@ = link_to _('Read more'), help_page_path('workflow/repository_mirroring'), target: '_blank' .settings-content - = form_for @project, url: project_mirror_path(@project), html: { class: 'gl-show-field-errors js-mirror-form', autocomplete: 'new-password', data: mirrors_form_data_attributes } do |f| - .panel.panel-default - .panel-body - %div= form_errors(@project) + - if mirror_settings_enabled + = form_for @project, url: project_mirror_path(@project), html: { class: 'gl-show-field-errors js-mirror-form', autocomplete: 'new-password', data: mirrors_form_data_attributes } do |f| + .panel.panel-default + .panel-body + %div= form_errors(@project) - .form-group.has-feedback - = label_tag :url, _('Git repository URL'), class: 'label-light' - = text_field_tag :url, nil, class: 'form-control js-mirror-url js-repo-url qa-mirror-repository-url-input', placeholder: _('Input your repository URL'), required: true, pattern: "(#{protocols}):\/\/.+", autocomplete: 'new-password' + .form-group.has-feedback + = label_tag :url, _('Git repository URL'), class: 'label-light' + = text_field_tag :url, nil, class: 'form-control js-mirror-url js-repo-url qa-mirror-repository-url-input', placeholder: _('Input your repository URL'), required: true, pattern: "(#{protocols}):\/\/.+", autocomplete: 'new-password' - = render 'projects/mirrors/instructions' + = render 'projects/mirrors/instructions' - = render 'projects/mirrors/mirror_repos_form', f: f + = render 'projects/mirrors/mirror_repos_form', f: f - .form-check.append-bottom-10 - = check_box_tag :only_protected_branches, '1', false, class: 'js-mirror-protected form-check-input' - = label_tag :only_protected_branches, _('Only mirror protected branches'), class: 'form-check-label' - = link_to icon('question-circle'), help_page_path('user/project/protected_branches'), target: '_blank' + .form-check.append-bottom-10 + = check_box_tag :only_protected_branches, '1', false, class: 'js-mirror-protected form-check-input' + = label_tag :only_protected_branches, _('Only mirror protected branches'), class: 'form-check-label' + = link_to icon('question-circle'), help_page_path('user/project/protected_branches'), target: '_blank' - .panel-footer - = f.submit _('Mirror repository'), class: 'btn btn-success js-mirror-submit qa-mirror-repository-button', name: :update_remote_mirror + .panel-footer + = f.submit _('Mirror repository'), class: 'btn btn-success js-mirror-submit qa-mirror-repository-button', name: :update_remote_mirror + - else + .gl-alert.gl-alert-info{ role: 'alert' } + = sprite_icon('information-o', size: 16, css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title') + .gl-alert-body + = _('Mirror settings are only available to GitLab administrators.') .panel.panel-default .table-responsive @@ -61,8 +69,10 @@ - if mirror.last_error.present? .badge.mirror-error-badge{ data: { toggle: 'tooltip', html: 'true', qa_selector: 'mirror_error_badge' }, title: html_escape(mirror.last_error.try(:strip)) }= _('Error') %td - .btn-group.mirror-actions-group.pull-right{ role: 'group' } - - if mirror.ssh_key_auth? - = clipboard_button(text: mirror.ssh_public_key, class: 'btn btn-default', title: _('Copy SSH public key'), qa_selector: 'copy_public_key_button') - = render 'shared/remote_mirror_update_button', remote_mirror: mirror + - if mirror_settings_enabled + .btn-group.mirror-actions-group.pull-right{ role: 'group' } + - if mirror.ssh_key_auth? + = clipboard_button(text: mirror.ssh_public_key, class: 'btn btn-default', title: _('Copy SSH public key'), qa_selector: 'copy_public_key_button') + = render 'shared/remote_mirror_update_button', remote_mirror: mirror %button.js-delete-mirror.qa-delete-mirror.rspec-delete-mirror.btn.btn-danger{ type: 'button', data: { mirror_id: mirror.id, toggle: 'tooltip', container: 'body' }, title: _('Remove') }= icon('trash-o') + diff --git a/derailed-benchmarks/CHANGELOG.md b/derailed-benchmarks/CHANGELOG.md index aa5093d6c3..268536e115 100644 --- a/derailed-benchmarks/CHANGELOG.md +++ b/derailed-benchmarks/CHANGELOG.md @@ -1,5 +1,10 @@ ## master (unreleased) +## 1.7.0 + +- Add histogram support to `perf:library` (https://github.com/schneems/derailed_benchmarks/pull/169) +- Fix bug with `Kernel#require` patch when Zeitwerk is enabled (https://github.com/schneems/derailed_benchmarks/pull/170) + ## 1.6.0 - Added the `perf:app` command to compare commits within the same application. (https://github.com/schneems/derailed_benchmarks/pull/157) diff --git a/derailed-benchmarks/derailed_benchmarks.gemspec b/derailed-benchmarks/derailed_benchmarks.gemspec index 1264b4d541..c0622ed760 100644 --- a/derailed-benchmarks/derailed_benchmarks.gemspec +++ b/derailed-benchmarks/derailed_benchmarks.gemspec @@ -30,6 +30,8 @@ Gem::Specification.new do |gem| gem.add_dependency "rake", "> 10", "< 14" gem.add_dependency "thor", ">= 0.19", "< 2" gem.add_dependency "ruby-statistics", ">= 2.1" + gem.add_dependency "unicode_plot", ">= 0.0.4", "< 1.0.0" + gem.add_dependency "mini_histogram", "~> 0" gem.add_development_dependency "capybara", "~> 2" gem.add_development_dependency "m" diff --git a/derailed-benchmarks/lib/derailed_benchmarks/core_ext/kernel_require.rb b/derailed-benchmarks/lib/derailed_benchmarks/core_ext/kernel_require.rb index cbb5f01328..4c4d553e81 100644 --- a/derailed-benchmarks/lib/derailed_benchmarks/core_ext/kernel_require.rb +++ b/derailed-benchmarks/lib/derailed_benchmarks/core_ext/kernel_require.rb @@ -11,14 +11,20 @@ ENV['CUT_OFF'] ||= "0.3" # Monkey patch kernel to ensure that all `require` calls call the same # method module Kernel - - private - - alias :original_require :require REQUIRE_STACK = [] + module_function + + alias_method :original_require, :require + alias_method :original_require_relative, :require_relative + def require(file) - Kernel.require(file) + measure_memory_impact(file) do |file| + # "source_annotation_extractor" is deprecated in Rails 6 + # # if we don't skip the library it leads to a crash + # next if file == "rails/source_annotation_extractor" && Rails.version >= '6.0' + original_require(file) + end end def require_relative(file) @@ -29,10 +35,7 @@ module Kernel end end - class << self - alias :original_require :require - alias :original_require_relative :require_relative - end + private # The core extension we use to measure require time of all requires # When a file is required we create a tree node with its file name. @@ -46,7 +49,7 @@ module Kernel # When a require returns we remove it from the require stack so we don't # accidentally push additional children nodes to it. We then store the # memory cost of the require in the tree node. - def self.measure_memory_impact(file, &block) + def measure_memory_impact(file, &block) mem = GetProcessMem.new node = DerailedBenchmarks::RequireTree.new(file) @@ -68,15 +71,6 @@ end TOP_REQUIRE = DerailedBenchmarks::RequireTree.new("TOP") REQUIRE_STACK.push(TOP_REQUIRE) -Kernel.define_singleton_method(:require) do |file| - measure_memory_impact(file) do |file| - # "source_annotation_extractor" is deprecated in Rails 6 - # # if we don't skip the library it leads to a crash - # next if file == "rails/source_annotation_extractor" && Rails.version >= '6.0' - original_require(file) - end -end - class Object private diff --git a/derailed-benchmarks/lib/derailed_benchmarks/load_tasks.rb b/derailed-benchmarks/lib/derailed_benchmarks/load_tasks.rb index 52220b9d92..1cce56d823 100644 --- a/derailed-benchmarks/lib/derailed_benchmarks/load_tasks.rb +++ b/derailed-benchmarks/lib/derailed_benchmarks/load_tasks.rb @@ -30,7 +30,7 @@ namespace :perf do DERAILED_APP.initialize! unless DERAILED_APP.instance_variable_get(:@initialized) end - if ENV["DERAILED_SKIP_ACTIVE_RECORD"] && defined? ActiveRecord + if !ENV["DERAILED_SKIP_ACTIVE_RECORD"] && defined? ActiveRecord if defined? ActiveRecord::Tasks::DatabaseTasks ActiveRecord::Tasks::DatabaseTasks.create_current else # Rails 3.2 @@ -39,7 +39,14 @@ namespace :perf do ActiveRecord::Migrator.migrations_paths = DERAILED_APP.paths['db/migrate'].to_a ActiveRecord::Migration.verbose = true - ActiveRecord::Migrator.migrate(ActiveRecord::Migrator.migrations_paths, nil) + + if Rails.version.start_with? '6' + ActiveRecord::MigrationContext.new(ActiveRecord::Migrator.migrations_paths, ActiveRecord::SchemaMigration).migrate + elsif Rails.version.start_with? '5.2' + ActiveRecord::MigrationContext.new(ActiveRecord::Migrator.migrations_paths).migrate + else + ActiveRecord::Migrator.migrate(ActiveRecord::Migrator.migrations_paths, nil) + end end DERAILED_APP.config.consider_all_requests_local = true @@ -135,4 +142,4 @@ namespace :perf do WARM_COUNT.times { call_app } end end -end \ No newline at end of file +end diff --git a/derailed-benchmarks/lib/derailed_benchmarks/stats_from_dir.rb b/derailed-benchmarks/lib/derailed_benchmarks/stats_from_dir.rb index f1ff7503dd..63b6a06c3e 100644 --- a/derailed-benchmarks/lib/derailed_benchmarks/stats_from_dir.rb +++ b/derailed-benchmarks/lib/derailed_benchmarks/stats_from_dir.rb @@ -2,6 +2,9 @@ require 'bigdecimal' require 'statistics' +require 'unicode_plot' +require 'stringio' +require 'mini_histogram' module DerailedBenchmarks # A class used to read several benchmark files @@ -100,7 +103,26 @@ module DerailedBenchmarks " " * (percent_faster.to_s.index(".") - x_faster.to_s.index(".")) end - def banner(io = Kernel) + def histogram(io = $stdout) + newest_histogram = MiniHistogram.new(newest.values) + oldest_histogram = MiniHistogram.new(oldest.values) + MiniHistogram.set_average_edges!(newest_histogram, oldest_histogram) + + {newest => newest_histogram, oldest => oldest_histogram}.each do |report, histogram| + plot = UnicodePlot.histogram( + histogram, + title: "\n#{' ' * 18 }Histogram - [#{report.name}] #{report.desc.inspect}", + ylabel: "Time (s)", + xlabel: "# of runs in range" + ) + plot.render(io) + io.puts + end + + io.puts + end + + def banner(io = $stdout) io.puts if significant? io.puts "❤️ ❤️ ❤️ (Statistically Significant) ❤️ ❤️ ❤️" @@ -122,6 +144,9 @@ module DerailedBenchmarks io.puts "Is significant? (max > critical): #{significant?}" io.puts "D critical: #{d_critical}" io.puts "D max: #{d_max}" + + histogram(io) + io.puts end end diff --git a/derailed-benchmarks/lib/derailed_benchmarks/tasks.rb b/derailed-benchmarks/lib/derailed_benchmarks/tasks.rb index 12c5d13801..9f9248e1bb 100644 --- a/derailed-benchmarks/lib/derailed_benchmarks/tasks.rb +++ b/derailed-benchmarks/lib/derailed_benchmarks/tasks.rb @@ -88,7 +88,8 @@ namespace :perf do if (i % 50).zero? puts "Intermediate result" - stats.call.banner + stats.call + stats.banner puts "Continuing execution" end end @@ -102,7 +103,8 @@ namespace :perf do end if stats - stats.call.banner + stats.call + stats.banner result_file = out_dir + "results.txt" File.open(result_file, "w") do |f| diff --git a/derailed-benchmarks/lib/derailed_benchmarks/version.rb b/derailed-benchmarks/lib/derailed_benchmarks/version.rb index b457df02f3..69c86963ad 100644 --- a/derailed-benchmarks/lib/derailed_benchmarks/version.rb +++ b/derailed-benchmarks/lib/derailed_benchmarks/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module DerailedBenchmarks - VERSION = "1.6.0" + VERSION = "1.7.0" end diff --git a/derailed-benchmarks/test/derailed_benchmarks/stats_from_dir_test.rb b/derailed-benchmarks/test/derailed_benchmarks/stats_from_dir_test.rb index 61e067a49a..17e434fe1e 100644 --- a/derailed-benchmarks/test/derailed_benchmarks/stats_from_dir_test.rb +++ b/derailed-benchmarks/test/derailed_benchmarks/stats_from_dir_test.rb @@ -29,6 +29,22 @@ class StatsFromDirTest < ActiveSupport::TestCase assert_equal "11.3844", format % newest.median end + test "histogram output" do + dir = fixtures_dir("stats/significant") + branch_info = {} + branch_info["loser"] = { desc: "Old commit", time: Time.now, file: dir.join("loser.bench.txt"), name: "loser" } + branch_info["winner"] = { desc: "I am the new commit", time: Time.now + 1, file: dir.join("winner.bench.txt"), name: "winner" } + stats = DerailedBenchmarks::StatsFromDir.new(branch_info).call + + io = StringIO.new + stats.call.banner(io) + puts io.string + + assert_match(/11\.2 , 11\.28/, io.string) + assert_match(/11\.8 , 11\.88/, io.string) + end + + test "alignment" do dir = fixtures_dir("stats/significant") branch_info = {} diff --git a/derailed-benchmarks/test/integration/tasks_test.rb b/derailed-benchmarks/test/integration/tasks_test.rb index a5e6fcc1f9..efd841ea1c 100644 --- a/derailed-benchmarks/test/integration/tasks_test.rb +++ b/derailed-benchmarks/test/integration/tasks_test.rb @@ -26,7 +26,7 @@ class TasksTest < ActiveSupport::TestCase env_string = env.map {|key, value| "#{key.shellescape}=#{value.to_s.shellescape}" }.join(" ") cmd = "env #{env_string} bundle exec rake -f perf.rake #{cmd} --trace" puts "Running: #{cmd}" - result = `cd '#{rails_app_path}' && #{cmd}` + result = Bundler.with_original_env { `cd '#{rails_app_path}' && #{cmd}` } if assert_success assert $?.success?, "Expected '#{cmd}' to return a success status.\nOutput: #{result}" end diff --git a/derailed-benchmarks/test/rails_app/config/application.rb b/derailed-benchmarks/test/rails_app/config/application.rb index 236dac56bb..b58f9e6116 100644 --- a/derailed-benchmarks/test/rails_app/config/application.rb +++ b/derailed-benchmarks/test/rails_app/config/application.rb @@ -13,6 +13,8 @@ require 'devise' module Dummy class Application < Rails::Application + config.load_defaults Rails.version.to_f + config.action_mailer.default_url_options = { host: 'localhost:3000' } # Settings in config/environments/* take precedence over those specified here. diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 001fb92ec5..4b51bbff80 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -359,6 +359,10 @@ module API render_api_error!('405 Method Not Allowed', 405) end + def not_acceptable! + render_api_error!('406 Not Acceptable', 406) + end + def conflict!(message = nil) render_api_error!(message || '409 Conflict', 409) end diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 00473db1ff..9953d3138f 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -89,6 +89,8 @@ module API optional :format, type: String, desc: 'The archive format' end get ':id/repository/archive', requirements: { format: Gitlab::PathRegex.archive_formats_regex } do + not_acceptable! if Gitlab::HotlinkingDetector.intercept_hotlinking?(request) + send_git_archive user_project.repository, ref: params[:sha], format: params[:format], append_sha: true rescue not_found!('File') diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb index b5df036c5c..0aaab9a812 100644 --- a/lib/api/snippets.rb +++ b/lib/api/snippets.rb @@ -74,6 +74,8 @@ module API desc: 'The visibility of the snippet' end post do + authorize! :create_snippet + attrs = declared_params(include_missing: false).merge(request: request, api: true) service_response = ::Snippets::CreateService.new(nil, current_user, attrs).execute snippet = service_response.payload[:snippet] diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb index 76af29b297..e182940394 100644 --- a/lib/api/triggers.rb +++ b/lib/api/triggers.rb @@ -109,6 +109,8 @@ module API trigger = user_project.triggers.find(params.delete(:trigger_id)) break not_found!('Trigger') unless trigger + authorize! :admin_trigger, trigger + if trigger.update(declared_params(include_missing: false)) present trigger, with: Entities::Trigger, current_user: current_user else diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 1329357d0b..a1f6de338d 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -171,6 +171,8 @@ module Gitlab if valid_oauth_token?(token) user = User.find_by(id: token.resource_owner_id) + return unless user.can?(:log_in) + Gitlab::Auth::Result.new(user, nil, :oauth, full_authentication_abilities) end end @@ -182,7 +184,7 @@ module Gitlab token = PersonalAccessTokensFinder.new(state: 'active').find_by_token(password) - if token && valid_scoped_token?(token, all_available_scopes) + if token && valid_scoped_token?(token, all_available_scopes) && token.user.can?(:log_in) Gitlab::Auth::Result.new(token.user, nil, :personal_access_token, abilities_for_scopes(token.scopes)) end end @@ -260,6 +262,8 @@ module Gitlab return unless build.project.builds_enabled? if build.user + return unless build.user.can?(:log_in) + # If user is assigned to build, use restricted credentials of user Gitlab::Auth::Result.new(build.user, build.project, :build, build_authentication_abilities) else diff --git a/lib/gitlab/gfm/uploads_rewriter.rb b/lib/gitlab/gfm/uploads_rewriter.rb index 6b52d6e88e..23af0a9bb1 100644 --- a/lib/gitlab/gfm/uploads_rewriter.rb +++ b/lib/gitlab/gfm/uploads_rewriter.rb @@ -22,6 +22,8 @@ module Gitlab return @text unless needs_rewrite? @text.gsub(@pattern) do |markdown| + Gitlab::Utils.check_path_traversal!($~[:file]) + file = find_file(@source_project, $~[:secret], $~[:file]) break markdown unless file.try(:exists?) diff --git a/lib/gitlab/hotlinking_detector.rb b/lib/gitlab/hotlinking_detector.rb new file mode 100644 index 0000000000..4490129787 --- /dev/null +++ b/lib/gitlab/hotlinking_detector.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module Gitlab + class HotlinkingDetector + IMAGE_FORMATS = %w(image/jpeg image/apng image/png image/webp image/svg+xml image/*).freeze + MEDIA_FORMATS = %w(video/webm video/ogg video/* application/ogg audio/webm audio/ogg audio/wav audio/*).freeze + CSS_FORMATS = %w(text/css).freeze + INVALID_FORMATS = (IMAGE_FORMATS + MEDIA_FORMATS + CSS_FORMATS).freeze + INVALID_FETCH_MODES = %w(cors no-cors websocket).freeze + + class << self + def intercept_hotlinking?(request) + request_accepts = parse_request_accepts(request) + + return false unless Feature.enabled?(:repository_archive_hotlinking_interception, default_enabled: true) + + # Block attempts to embed as JS + return true if sec_fetch_invalid?(request) + + # If no Accept header was set, skip the rest + return false if request_accepts.empty? + + # Workaround for IE8 weirdness + return false if IMAGE_FORMATS.include?(request_accepts.first) && request_accepts.include?("application/x-ms-application") + + # Block all other media requests if the first format is a media type + return true if INVALID_FORMATS.include?(request_accepts.first) + + false + end + + private + + def sec_fetch_invalid?(request) + fetch_mode = request.headers["Sec-Fetch-Mode"] + + return if fetch_mode.blank? + return true if INVALID_FETCH_MODES.include?(fetch_mode) + end + + def parse_request_accepts(request) + # Rails will already have parsed the Accept header + return request.accepts if request.respond_to?(:accepts) + + # Grape doesn't parse it, so we can use the Rails system for this + return Mime::Type.parse(request.headers["Accept"]) if request.respond_to?(:headers) && request.headers["Accept"].present? + + [] + end + end + end +end diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb index d1c20dff79..4c32d0c958 100644 --- a/lib/gitlab/import_export/attribute_cleaner.rb +++ b/lib/gitlab/import_export/attribute_cleaner.rb @@ -11,7 +11,14 @@ module Gitlab 'discussion_id', 'custom_attributes' ].freeze - PROHIBITED_REFERENCES = Regexp.union(/\Acached_markdown_version\Z/, /_id\Z/, /_ids\Z/, /_html\Z/, /attributes/).freeze + PROHIBITED_REFERENCES = Regexp.union( + /\Acached_markdown_version\Z/, + /_id\Z/, + /_ids\Z/, + /_html\Z/, + /attributes/, + /\Aremote_\w+_(url|urls|request_header)\Z/ # carrierwave automatically creates these attribute methods for uploads + ).freeze def self.clean(*args) new(*args).clean diff --git a/lib/gitlab/middleware/multipart.rb b/lib/gitlab/middleware/multipart.rb index 0ee9563c22..cb4213d23a 100644 --- a/lib/gitlab/middleware/multipart.rb +++ b/lib/gitlab/middleware/multipart.rb @@ -84,12 +84,6 @@ module Gitlab end def open_file(params, key) - allowed_paths = [ - ::FileUploader.root, - Gitlab.config.uploads.storage_path, - File.join(Rails.root, 'public/uploads/tmp') - ] - ::UploadedFile.from_params(params, key, allowed_paths) end @@ -106,6 +100,16 @@ module Gitlab # inside other env keys, here we ensure everything is updated correctly ActionDispatch::Request.new(@request.env).update_param(key, value) end + + private + + def allowed_paths + [ + ::FileUploader.root, + Gitlab.config.uploads.storage_path, + File.join(Rails.root, 'public/uploads/tmp') + ] + end end def initialize(app) @@ -125,3 +129,5 @@ module Gitlab end end end + +::Gitlab::Middleware::Multipart::Handler.prepend_if_ee('EE::Gitlab::Middleware::Multipart::Handler') diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index fd6e24a96d..b64603d0dd 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -16,6 +16,14 @@ module Gitlab "It must start with letter, digit, emoji or '_'." end + def group_name_regex + project_name_regex + end + + def group_name_regex_message + project_name_regex_message + end + ## # Docker Distribution Registry repository / tag name rules # diff --git a/lib/gitlab/x509/commit.rb b/lib/gitlab/x509/commit.rb index ce298b80a4..b1d1504798 100644 --- a/lib/gitlab/x509/commit.rb +++ b/lib/gitlab/x509/commit.rb @@ -105,13 +105,22 @@ module Gitlab def certificate_crl extension = get_certificate_extension('crlDistributionPoints') - extension.split('URI:').each do |item| - item.strip + crl_url = nil - if item.start_with?("http") - return item.strip + extension.each_line do |line| + break if crl_url + + line.split('URI:').each do |item| + item.strip + + if item.start_with?("http") + crl_url = item.strip + break + end end end + + crl_url end def get_certificate_extension(extension) diff --git a/lib/uploaded_file.rb b/lib/uploaded_file.rb index 424db653fb..f8d596b5d1 100644 --- a/lib/uploaded_file.rb +++ b/lib/uploaded_file.rb @@ -48,7 +48,7 @@ class UploadedFile return if path.blank? && remote_id.blank? file_path = nil - if path + if path.present? file_path = File.realpath(path) paths = Array(upload_paths) << Dir.tmpdir diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e1e04c3880..28cea78c8c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -12357,6 +12357,9 @@ msgstr "" msgid "Mirror repository" msgstr "" +msgid "Mirror settings are only available to GitLab administrators." +msgstr "" + msgid "Mirror user" msgstr "" @@ -18176,6 +18179,9 @@ msgstr "" msgid "Specific Runners" msgstr "" +msgid "Specified URL cannot be used: \"%{reason}\"" +msgstr "" + msgid "Specify an e-mail address regex pattern to identify default internal users." msgstr "" diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 1c58c2b5c9..35f5a8e872 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -258,6 +258,18 @@ describe GroupsController do end end end + + context "malicious group name" do + subject { post :create, params: { group: { name: "", path: "invalid_group_url" } } } + + before do + sign_in(user) + end + + it { expect { subject }.not_to change { Group.count } } + + it { expect(subject).to render_template(:new) } + end end describe 'GET #index' do @@ -829,6 +841,16 @@ describe GroupsController do put :update, params: { id: group.to_param, group: { name: 'world' } } end.to change { group.reload.name } end + + context "malicious group name" do + subject { put :update, params: { id: group.to_param, group: { name: "" } } } + + it { is_expected.to render_template(:edit) } + + it 'does not update name' do + expect { subject }.not_to change { group.reload.name } + end + end end describe 'DELETE #destroy' do diff --git a/spec/controllers/import/fogbugz_controller_spec.rb b/spec/controllers/import/fogbugz_controller_spec.rb index 9a647b8caa..c833fbfaea 100644 --- a/spec/controllers/import/fogbugz_controller_spec.rb +++ b/spec/controllers/import/fogbugz_controller_spec.rb @@ -25,6 +25,35 @@ describe Import::FogbugzController do expect(session[:fogbugz_uri]).to eq(uri) expect(response).to redirect_to(new_user_map_import_fogbugz_path) end + + context 'verify url' do + shared_examples 'denies local request' do |reason| + it 'does not allow requests' do + post :callback, params: { uri: uri, email: 'test@example.com', password: 'mypassword' } + + expect(response).to redirect_to(new_import_fogbugz_url) + expect(flash[:alert]).to eq("Specified URL cannot be used: \"#{reason}\"") + end + end + + context 'when host is localhost' do + let(:uri) { 'https://localhost:3000' } + + include_examples 'denies local request', 'Requests to localhost are not allowed' + end + + context 'when host is on local network' do + let(:uri) { 'http://192.168.0.1/' } + + include_examples 'denies local request', 'Requests to the local network are not allowed' + end + + context 'when host is ftp protocol' do + let(:uri) { 'ftp://testing' } + + include_examples 'denies local request', 'Only allowed schemes are http, https' + end + end end describe 'POST #create_user_map' do diff --git a/spec/controllers/projects/mirrors_controller_spec.rb b/spec/controllers/projects/mirrors_controller_spec.rb index 4362febda5..3579e4aa2c 100644 --- a/spec/controllers/projects/mirrors_controller_spec.rb +++ b/spec/controllers/projects/mirrors_controller_spec.rb @@ -5,6 +5,72 @@ require 'spec_helper' describe Projects::MirrorsController do include ReactiveCachingHelpers + shared_examples 'only admin is allowed when mirroring is disabled' do + let(:subject_action) { raise 'subject_action is required' } + let(:user) { project.owner } + let(:project_settings_path) { project_settings_repository_path(project, anchor: 'js-push-remote-settings') } + + context 'when project mirroring is enabled' do + it 'allows requests from a maintainer' do + sign_in(user) + + subject_action + expect(response).to redirect_to(project_settings_path) + end + + it 'allows requests from an admin user' do + user.update!(admin: true) + sign_in(user) + + subject_action + expect(response).to redirect_to(project_settings_path) + end + end + + context 'when project mirroring is disabled' do + before do + stub_application_setting(mirror_available: false) + end + + it 'disallows requests from a maintainer' do + sign_in(user) + + subject_action + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'allows requests from an admin user' do + user.update!(admin: true) + sign_in(user) + + subject_action + expect(response).to redirect_to(project_settings_path) + end + end + end + + describe 'Access control' do + let(:project) { create(:project, :repository) } + + describe '#update' do + include_examples 'only admin is allowed when mirroring is disabled' do + let(:subject_action) do + do_put(project, remote_mirrors_attributes: { '0' => { 'enabled' => 1, 'url' => 'http://foo.com' } }) + end + end + end + + describe '#update_now' do + include_examples 'only admin is allowed when mirroring is disabled' do + let(:options) { { namespace_id: project.namespace, project_id: project } } + + let(:subject_action) do + get :update_now, params: options.merge(sync_remote: true) + end + end + end + end + describe 'setting up a remote mirror' do let_it_be(:project) { create(:project, :repository) } diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb index d4a81f24d9..820ac594b2 100644 --- a/spec/controllers/projects/repositories_controller_spec.rb +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -24,6 +24,12 @@ describe Projects::RepositoriesController do sign_in(user) end + it_behaves_like "hotlink interceptor" do + let(:http_request) do + get :archive, params: { namespace_id: project.namespace, project_id: project, id: "master" }, format: "zip" + end + end + it "uses Gitlab::Workhorse" do get :archive, params: { namespace_id: project.namespace, project_id: project, id: "master" }, format: "zip" diff --git a/spec/factories/pages_domains.rb b/spec/factories/pages_domains.rb index f914128ed3..4efb5c7dbb 100644 --- a/spec/factories/pages_domains.rb +++ b/spec/factories/pages_domains.rb @@ -7,39 +7,11 @@ FactoryBot.define do enabled_until { 1.week.from_now } certificate do - '-----BEGIN CERTIFICATE----- -MIICGzCCAYSgAwIBAgIBATANBgkqhkiG9w0BAQUFADAbMRkwFwYDVQQDExB0ZXN0 -LWNlcnRpZmljYXRlMB4XDTE2MDIxMjE0MzIwMFoXDTIwMDQxMjE0MzIwMFowGzEZ -MBcGA1UEAxMQdGVzdC1jZXJ0aWZpY2F0ZTCBnzANBgkqhkiG9w0BAQEFAAOBjQAw -gYkCgYEApL4J9L0ZxFJ1hI1LPIflAlAGvm6ZEvoT4qKU5Xf2JgU7/2geNR1qlNFa -SvCc08Knupp5yTgmvyK/Xi09U0N82vvp4Zvr/diSc4A/RA6Mta6egLySNT438kdT -nY2tR5feoTLwQpX0t4IMlwGQGT5h6Of2fKmDxzuwuyffcIHqLdsCAwEAAaNvMG0w -DAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUxl9WSxBprB0z0ibJs3rXEk0+95AwCwYD -VR0PBAQDAgXgMBEGCWCGSAGG+EIBAQQEAwIGQDAeBglghkgBhvhCAQ0EERYPeGNh -IGNlcnRpZmljYXRlMA0GCSqGSIb3DQEBBQUAA4GBAGC4T8SlFHK0yPSa+idGLQFQ -joZp2JHYvNlTPkRJ/J4TcXxBTJmArcQgTIuNoBtC+0A/SwdK4MfTCUY4vNWNdese -5A4K65Nb7Oh1AdQieTBHNXXCdyFsva9/ScfQGEl7p55a52jOPs0StPd7g64uvjlg -YHi2yesCrOvVXt+lgPTd ------END CERTIFICATE-----' + File.read(Rails.root.join('spec/fixtures/', 'ssl_certificate.pem')) end key do - '-----BEGIN PRIVATE KEY----- -MIICdgIBADANBgkqhkiG9w0BAQEFAASCAmAwggJcAgEAAoGBAKS+CfS9GcRSdYSN -SzyH5QJQBr5umRL6E+KilOV39iYFO/9oHjUdapTRWkrwnNPCp7qaeck4Jr8iv14t -PVNDfNr76eGb6/3YknOAP0QOjLWunoC8kjU+N/JHU52NrUeX3qEy8EKV9LeCDJcB -kBk+Yejn9nypg8c7sLsn33CB6i3bAgMBAAECgYA2D26w80T7WZvazYr86BNMePpd -j2mIAqx32KZHzt/lhh40J/SRtX9+Kl0Y7nBoRR5Ja9u/HkAIxNxLiUjwg9r6cpg/ -uITEF5nMt7lAk391BuI+7VOZZGbJDsq2ulPd6lO+C8Kq/PI/e4kXcIjeH6KwQsuR -5vrXfBZ3sQfflaiN4QJBANBt8JY2LIGQF8o89qwUpRL5vbnKQ4IzZ5+TOl4RLR7O -AQpJ81tGuINghO7aunctb6rrcKJrxmEH1whzComybrMCQQDKV49nOBudRBAIgG4K -EnLzsRKISUHMZSJiYTYnablof8cKw1JaQduw7zgrUlLwnroSaAGX88+Jw1f5n2Lh -Vlg5AkBDdUGnrDLtYBCDEQYZHblrkc7ZAeCllDOWjxUV+uMqlCv8A4Ey6omvY57C -m6I8DkWVAQx8VPtozhvHjUw80rZHAkB55HWHAM3h13axKG0htCt7klhPsZHpx6MH -EPjGlXIT+aW2XiPmK3ZlCDcWIenE+lmtbOpI159Wpk8BGXs/s/xBAkEAlAY3ymgx -63BDJEwvOb2IaP8lDDxNsXx9XJNVvQbv5n15vNsLHbjslHfAhAbxnLQ1fLhUPqSi -nNp/xedE1YxutQ== ------END PRIVATE KEY-----' + File.read(Rails.root.join('spec/fixtures/', 'ssl_key.pem')) end trait :disabled do diff --git a/spec/factories/serverless/domain_cluster.rb b/spec/factories/serverless/domain_cluster.rb index bc32552d4c..40e0ecad5a 100644 --- a/spec/factories/serverless/domain_cluster.rb +++ b/spec/factories/serverless/domain_cluster.rb @@ -7,39 +7,11 @@ FactoryBot.define do creator { create(:user) } certificate do - '-----BEGIN CERTIFICATE----- -MIICGzCCAYSgAwIBAgIBATANBgkqhkiG9w0BAQUFADAbMRkwFwYDVQQDExB0ZXN0 -LWNlcnRpZmljYXRlMB4XDTE2MDIxMjE0MzIwMFoXDTIwMDQxMjE0MzIwMFowGzEZ -MBcGA1UEAxMQdGVzdC1jZXJ0aWZpY2F0ZTCBnzANBgkqhkiG9w0BAQEFAAOBjQAw -gYkCgYEApL4J9L0ZxFJ1hI1LPIflAlAGvm6ZEvoT4qKU5Xf2JgU7/2geNR1qlNFa -SvCc08Knupp5yTgmvyK/Xi09U0N82vvp4Zvr/diSc4A/RA6Mta6egLySNT438kdT -nY2tR5feoTLwQpX0t4IMlwGQGT5h6Of2fKmDxzuwuyffcIHqLdsCAwEAAaNvMG0w -DAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUxl9WSxBprB0z0ibJs3rXEk0+95AwCwYD -VR0PBAQDAgXgMBEGCWCGSAGG+EIBAQQEAwIGQDAeBglghkgBhvhCAQ0EERYPeGNh -IGNlcnRpZmljYXRlMA0GCSqGSIb3DQEBBQUAA4GBAGC4T8SlFHK0yPSa+idGLQFQ -joZp2JHYvNlTPkRJ/J4TcXxBTJmArcQgTIuNoBtC+0A/SwdK4MfTCUY4vNWNdese -5A4K65Nb7Oh1AdQieTBHNXXCdyFsva9/ScfQGEl7p55a52jOPs0StPd7g64uvjlg -YHi2yesCrOvVXt+lgPTd ------END CERTIFICATE-----' + File.read(Rails.root.join('spec/fixtures/', 'ssl_certificate.pem')) end key do - '-----BEGIN PRIVATE KEY----- -MIICdgIBADANBgkqhkiG9w0BAQEFAASCAmAwggJcAgEAAoGBAKS+CfS9GcRSdYSN -SzyH5QJQBr5umRL6E+KilOV39iYFO/9oHjUdapTRWkrwnNPCp7qaeck4Jr8iv14t -PVNDfNr76eGb6/3YknOAP0QOjLWunoC8kjU+N/JHU52NrUeX3qEy8EKV9LeCDJcB -kBk+Yejn9nypg8c7sLsn33CB6i3bAgMBAAECgYA2D26w80T7WZvazYr86BNMePpd -j2mIAqx32KZHzt/lhh40J/SRtX9+Kl0Y7nBoRR5Ja9u/HkAIxNxLiUjwg9r6cpg/ -uITEF5nMt7lAk391BuI+7VOZZGbJDsq2ulPd6lO+C8Kq/PI/e4kXcIjeH6KwQsuR -5vrXfBZ3sQfflaiN4QJBANBt8JY2LIGQF8o89qwUpRL5vbnKQ4IzZ5+TOl4RLR7O -AQpJ81tGuINghO7aunctb6rrcKJrxmEH1whzComybrMCQQDKV49nOBudRBAIgG4K -EnLzsRKISUHMZSJiYTYnablof8cKw1JaQduw7zgrUlLwnroSaAGX88+Jw1f5n2Lh -Vlg5AkBDdUGnrDLtYBCDEQYZHblrkc7ZAeCllDOWjxUV+uMqlCv8A4Ey6omvY57C -m6I8DkWVAQx8VPtozhvHjUw80rZHAkB55HWHAM3h13axKG0htCt7klhPsZHpx6MH -EPjGlXIT+aW2XiPmK3ZlCDcWIenE+lmtbOpI159Wpk8BGXs/s/xBAkEAlAY3ymgx -63BDJEwvOb2IaP8lDDxNsXx9XJNVvQbv5n15vNsLHbjslHfAhAbxnLQ1fLhUPqSi -nNp/xedE1YxutQ== ------END PRIVATE KEY-----' + File.read(Rails.root.join('spec/fixtures/', 'ssl_key.pem')) end end end diff --git a/spec/features/projects/pages_spec.rb b/spec/features/projects/pages_spec.rb index c8da87041f..f4f70e7efb 100644 --- a/spec/features/projects/pages_spec.rb +++ b/spec/features/projects/pages_spec.rb @@ -135,43 +135,11 @@ shared_examples 'pages settings editing' do context 'when pages are exposed on external HTTPS address', :https_pages_enabled, :js do let(:certificate_pem) do - <<~PEM - -----BEGIN CERTIFICATE----- - MIICGzCCAYSgAwIBAgIBATANBgkqhkiG9w0BAQUFADAbMRkwFwYDVQQDExB0ZXN0 - LWNlcnRpZmljYXRlMB4XDTE2MDIxMjE0MzIwMFoXDTIwMDQxMjE0MzIwMFowGzEZ - MBcGA1UEAxMQdGVzdC1jZXJ0aWZpY2F0ZTCBnzANBgkqhkiG9w0BAQEFAAOBjQAw - gYkCgYEApL4J9L0ZxFJ1hI1LPIflAlAGvm6ZEvoT4qKU5Xf2JgU7/2geNR1qlNFa - SvCc08Knupp5yTgmvyK/Xi09U0N82vvp4Zvr/diSc4A/RA6Mta6egLySNT438kdT - nY2tR5feoTLwQpX0t4IMlwGQGT5h6Of2fKmDxzuwuyffcIHqLdsCAwEAAaNvMG0w - DAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUxl9WSxBprB0z0ibJs3rXEk0+95AwCwYD - VR0PBAQDAgXgMBEGCWCGSAGG+EIBAQQEAwIGQDAeBglghkgBhvhCAQ0EERYPeGNh - IGNlcnRpZmljYXRlMA0GCSqGSIb3DQEBBQUAA4GBAGC4T8SlFHK0yPSa+idGLQFQ - joZp2JHYvNlTPkRJ/J4TcXxBTJmArcQgTIuNoBtC+0A/SwdK4MfTCUY4vNWNdese - 5A4K65Nb7Oh1AdQieTBHNXXCdyFsva9/ScfQGEl7p55a52jOPs0StPd7g64uvjlg - YHi2yesCrOvVXt+lgPTd - -----END CERTIFICATE----- - PEM + attributes_for(:pages_domain)[:certificate] end let(:certificate_key) do - <<~KEY - -----BEGIN PRIVATE KEY----- - MIICdgIBADANBgkqhkiG9w0BAQEFAASCAmAwggJcAgEAAoGBAKS+CfS9GcRSdYSN - SzyH5QJQBr5umRL6E+KilOV39iYFO/9oHjUdapTRWkrwnNPCp7qaeck4Jr8iv14t - PVNDfNr76eGb6/3YknOAP0QOjLWunoC8kjU+N/JHU52NrUeX3qEy8EKV9LeCDJcB - kBk+Yejn9nypg8c7sLsn33CB6i3bAgMBAAECgYA2D26w80T7WZvazYr86BNMePpd - j2mIAqx32KZHzt/lhh40J/SRtX9+Kl0Y7nBoRR5Ja9u/HkAIxNxLiUjwg9r6cpg/ - uITEF5nMt7lAk391BuI+7VOZZGbJDsq2ulPd6lO+C8Kq/PI/e4kXcIjeH6KwQsuR - 5vrXfBZ3sQfflaiN4QJBANBt8JY2LIGQF8o89qwUpRL5vbnKQ4IzZ5+TOl4RLR7O - AQpJ81tGuINghO7aunctb6rrcKJrxmEH1whzComybrMCQQDKV49nOBudRBAIgG4K - EnLzsRKISUHMZSJiYTYnablof8cKw1JaQduw7zgrUlLwnroSaAGX88+Jw1f5n2Lh - Vlg5AkBDdUGnrDLtYBCDEQYZHblrkc7ZAeCllDOWjxUV+uMqlCv8A4Ey6omvY57C - m6I8DkWVAQx8VPtozhvHjUw80rZHAkB55HWHAM3h13axKG0htCt7klhPsZHpx6MH - EPjGlXIT+aW2XiPmK3ZlCDcWIenE+lmtbOpI159Wpk8BGXs/s/xBAkEAlAY3ymgx - 63BDJEwvOb2IaP8lDDxNsXx9XJNVvQbv5n15vNsLHbjslHfAhAbxnLQ1fLhUPqSi - nNp/xedE1YxutQ== - -----END PRIVATE KEY----- - KEY + attributes_for(:pages_domain)[:key] end it 'adds new domain with certificate' do diff --git a/spec/features/projects/settings/repository_settings_spec.rb b/spec/features/projects/settings/repository_settings_spec.rb index 18031a40f1..a104feece5 100644 --- a/spec/features/projects/settings/repository_settings_spec.rb +++ b/spec/features/projects/settings/repository_settings_spec.rb @@ -142,11 +142,7 @@ describe 'Projects > Settings > Repository settings' do end context 'remote mirror settings' do - let(:user2) { create(:user) } - before do - project.add_maintainer(user2) - visit project_settings_repository_path(project) end @@ -206,6 +202,18 @@ describe 'Projects > Settings > Repository settings' do expect(page).to have_selector('[title="Copy SSH public key"]') end + context 'when project mirroring is disabled' do + before do + stub_application_setting(mirror_available: false) + visit project_settings_repository_path(project) + end + + it 'hides remote mirror settings' do + expect(page.find('.project-mirror-settings')).not_to have_selector('form') + expect(page).to have_content('Mirror settings are only available to GitLab administrators.') + end + end + def select_direction(direction = 'push') direction_select = find('#mirror_direction') @@ -270,4 +278,31 @@ describe 'Projects > Settings > Repository settings' do expect(mirror).not_to have_selector('.rspec-update-now-button') end end + + context 'for admin' do + shared_examples_for 'shows mirror settings' do + it 'shows mirror settings' do + expect(page.find('.project-mirror-settings')).to have_selector('form') + expect(page).not_to have_content('Changing mirroring setting is disabled for non-admin users.') + end + end + + before do + stub_application_setting(mirror_available: mirror_available) + user.update!(admin: true) + visit project_settings_repository_path(project) + end + + context 'when project mirroring is enabled' do + let(:mirror_available) { true } + + include_examples 'shows mirror settings' + end + + context 'when project mirroring is disabled' do + let(:mirror_available) { false } + + include_examples 'shows mirror settings' + end + end end diff --git a/spec/fixtures/ssl_certificate.pem b/spec/fixtures/ssl_certificate.pem new file mode 100644 index 0000000000..6c9a8dd42c --- /dev/null +++ b/spec/fixtures/ssl_certificate.pem @@ -0,0 +1,12 @@ +-----BEGIN CERTIFICATE----- +MIIBrzCCARgCCQDbfQx2zdkNYTANBgkqhkiG9w0BAQsFADAbMRkwFwYDVQQDDBB0 +ZXN0LWNlcnRpZmljYXRlMCAXDTIwMDMxNjE0MjAzNFoYDzIyMjAwMTI4MTQyMDM0 +WjAbMRkwFwYDVQQDDBB0ZXN0LWNlcnRpZmljYXRlMIGfMA0GCSqGSIb3DQEBAQUA +A4GNADCBiQKBgQCkvgn0vRnEUnWEjUs8h+UCUAa+bpkS+hPiopTld/YmBTv/aB41 +HWqU0VpK8JzTwqe6mnnJOCa/Ir9eLT1TQ3za++nhm+v92JJzgD9EDoy1rp6AvJI1 +PjfyR1Odja1Hl96hMvBClfS3ggyXAZAZPmHo5/Z8qYPHO7C7J99wgeot2wIDAQAB +MA0GCSqGSIb3DQEBCwUAA4GBACc+chrTAuvnMBTedc4/dy16pEesK6oGjywYUd/0 +/FBr8Vry7QUXMSgfraza9S0V+JvFvZFqkkOyJKW+m30kThWzyc/2e+BRxTh/QrxP +0j84QXtmnVtW4jsAwfBBfg78ST27eyp/WhruI6F/kZlXhfAed0RcPbRnbi3yvUPL +Lo4T +-----END CERTIFICATE----- diff --git a/spec/fixtures/ssl_key.pem b/spec/fixtures/ssl_key.pem new file mode 100644 index 0000000000..1b53126536 --- /dev/null +++ b/spec/fixtures/ssl_key.pem @@ -0,0 +1,16 @@ +-----BEGIN PRIVATE KEY----- +MIICdgIBADANBgkqhkiG9w0BAQEFAASCAmAwggJcAgEAAoGBAKS+CfS9GcRSdYSN +SzyH5QJQBr5umRL6E+KilOV39iYFO/9oHjUdapTRWkrwnNPCp7qaeck4Jr8iv14t +PVNDfNr76eGb6/3YknOAP0QOjLWunoC8kjU+N/JHU52NrUeX3qEy8EKV9LeCDJcB +kBk+Yejn9nypg8c7sLsn33CB6i3bAgMBAAECgYA2D26w80T7WZvazYr86BNMePpd +j2mIAqx32KZHzt/lhh40J/SRtX9+Kl0Y7nBoRR5Ja9u/HkAIxNxLiUjwg9r6cpg/ +uITEF5nMt7lAk391BuI+7VOZZGbJDsq2ulPd6lO+C8Kq/PI/e4kXcIjeH6KwQsuR +5vrXfBZ3sQfflaiN4QJBANBt8JY2LIGQF8o89qwUpRL5vbnKQ4IzZ5+TOl4RLR7O +AQpJ81tGuINghO7aunctb6rrcKJrxmEH1whzComybrMCQQDKV49nOBudRBAIgG4K +EnLzsRKISUHMZSJiYTYnablof8cKw1JaQduw7zgrUlLwnroSaAGX88+Jw1f5n2Lh +Vlg5AkBDdUGnrDLtYBCDEQYZHblrkc7ZAeCllDOWjxUV+uMqlCv8A4Ey6omvY57C +m6I8DkWVAQx8VPtozhvHjUw80rZHAkB55HWHAM3h13axKG0htCt7klhPsZHpx6MH +EPjGlXIT+aW2XiPmK3ZlCDcWIenE+lmtbOpI159Wpk8BGXs/s/xBAkEAlAY3ymgx +63BDJEwvOb2IaP8lDDxNsXx9XJNVvQbv5n15vNsLHbjslHfAhAbxnLQ1fLhUPqSi +nNp/xedE1YxutQ== +-----END PRIVATE KEY----- diff --git a/spec/helpers/x509_helper_spec.rb b/spec/helpers/x509_helper_spec.rb new file mode 100644 index 0000000000..dcdf57ce03 --- /dev/null +++ b/spec/helpers/x509_helper_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe X509Helper do + describe '#x509_subject' do + let(:search_uppercase) { %w[CN OU O] } + let(:search_lowercase) { %w[cn ou o] } + let(:certificate_attributes) do + { + 'CN' => 'CA Issuing', + 'OU' => 'Trust Center', + 'O' => 'Example' + } + end + + context 'with uppercase DN' do + let(:upper_dn) { 'CN=CA Issuing,OU=Trust Center,O=Example,L=World,C=Galaxy' } + + it 'returns the attributes on any case search' do + expect(x509_subject(upper_dn, search_lowercase)).to eq(certificate_attributes) + expect(x509_subject(upper_dn, search_uppercase)).to eq(certificate_attributes) + end + end + + context 'with lowercase DN' do + let(:lower_dn) { 'cn=CA Issuing,ou=Trust Center,o=Example,l=World,c=Galaxy' } + + it 'returns the attributes on any case search' do + expect(x509_subject(lower_dn, search_lowercase)).to eq(certificate_attributes) + expect(x509_subject(lower_dn, search_uppercase)).to eq(certificate_attributes) + end + end + + context 'with comma within DN' do + let(:comma_dn) { 'cn=CA\, Issuing,ou=Trust Center,o=Example,l=World,c=Galaxy' } + let(:certificate_attributes) do + { + 'CN' => 'CA, Issuing', + 'OU' => 'Trust Center', + 'O' => 'Example' + } + end + + it 'returns the attributes on any case search' do + expect(x509_subject(comma_dn, search_lowercase)).to eq(certificate_attributes) + expect(x509_subject(comma_dn, search_uppercase)).to eq(certificate_attributes) + end + end + + context 'with mal formed DN' do + let(:bad_dn) { 'cn=CA, Issuing,ou=Trust Center,o=Example,l=World,c=Galaxy' } + + it 'returns nil on any case search' do + expect(x509_subject(bad_dn, search_lowercase)).to eq({}) + expect(x509_subject(bad_dn, search_uppercase)).to eq({}) + end + end + end +end diff --git a/spec/javascripts/frequent_items/utils_spec.js b/spec/javascripts/frequent_items/utils_spec.js index fd5bd00242..2939b46bc3 100644 --- a/spec/javascripts/frequent_items/utils_spec.js +++ b/spec/javascripts/frequent_items/utils_spec.js @@ -108,5 +108,23 @@ describe('Frequent Items utils spec', () => { expect(sanitizeItem(input)).toEqual({ name: 'test', namespace: 'test', id: 1 }); }); + + it("skips `name` key if it doesn't exist on the item", () => { + const input = { + namespace: '
test', + id: 1, + }; + + expect(sanitizeItem(input)).toEqual({ namespace: 'test', id: 1 }); + }); + + it("skips `namespace` key if it doesn't exist on the item", () => { + const input = { + name: '
test', + id: 1, + }; + + expect(sanitizeItem(input)).toEqual({ name: 'test', id: 1 }); + }); }); }); diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb index 82df506489..f4d017df2d 100644 --- a/spec/lib/banzai/filter/label_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/label_reference_filter_spec.rb @@ -523,7 +523,12 @@ describe Banzai::Filter::LabelReferenceFilter do end context 'when group name has HTML entities' do - let(:another_group) { create(:group, name: '', path: 'another_group') } + let(:another_group) { create(:group, name: 'random', path: 'another_group') } + + before do + another_group.name = "" + another_group.save!(validate: false) + end it 'escapes the HTML entities' do expect(result.text) diff --git a/spec/lib/banzai/filter/reference_redactor_filter_spec.rb b/spec/lib/banzai/filter/reference_redactor_filter_spec.rb index 9739afd3d5..a68581b300 100644 --- a/spec/lib/banzai/filter/reference_redactor_filter_spec.rb +++ b/spec/lib/banzai/filter/reference_redactor_filter_spec.rb @@ -20,8 +20,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do it 'skips when the skip_redaction flag is set' do user = create(:user) project = create(:project) - link = reference_link(project: project.id, reference_type: 'test') + doc = filter(link, current_user: user, skip_redaction: true) expect(doc.css('a').length).to eq 1 @@ -51,8 +51,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do user = create(:user) project = create(:project) project.add_maintainer(user) - link = reference_link(project: project.id, reference_type: 'test') + doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 @@ -69,8 +69,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do it 'removes unpermitted references' do user = create(:user) project = create(:project) - link = reference_link(project: project.id, reference_type: 'test') + doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 0 @@ -90,8 +90,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do non_member = create(:user) project = create(:project, :public) issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') + doc = filter(link, current_user: non_member) expect(doc.css('a').length).to eq 0 @@ -124,8 +124,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do assignee = create(:user) project = create(:project, :public) issue = create(:issue, :confidential, project: project, assignees: [assignee]) - link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') + doc = filter(link, current_user: assignee) expect(doc.css('a').length).to eq 1 @@ -136,8 +136,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do project = create(:project, :public) project.add_developer(member) issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') + doc = filter(link, current_user: member) expect(doc.css('a').length).to eq 1 @@ -147,20 +147,62 @@ describe Banzai::Filter::ReferenceRedactorFilter do admin = create(:admin) project = create(:project, :public) issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') + doc = filter(link, current_user: admin) expect(doc.css('a').length).to eq 1 end + + context "when a confidential issue is moved from a public project to a private one" do + let(:public_project) { create(:project, :public) } + let(:private_project) { create(:project, :private) } + + it 'removes references for author' do + author = create(:user) + issue = create(:issue, :confidential, project: public_project, author: author) + issue.update!(project: private_project) # move issue to private project + link = reference_link(project: private_project.id, issue: issue.id, reference_type: 'issue') + + doc = filter(link, current_user: author) + + expect(doc.css('a').length).to eq 0 + end + + it 'removes references for assignee' do + assignee = create(:user) + issue = create(:issue, :confidential, project: public_project, assignees: [assignee]) + issue.update!(project: private_project) # move issue to private project + link = reference_link(project: private_project.id, issue: issue.id, reference_type: 'issue') + + doc = filter(link, current_user: assignee) + + expect(doc.css('a').length).to eq 0 + end + + it 'allows references for project members' do + member = create(:user) + project = create(:project, :public) + project_2 = create(:project, :private) + project.add_developer(member) + project_2.add_developer(member) + issue = create(:issue, :confidential, project: project) + issue.update!(project: project_2) # move issue to private project + link = reference_link(project: project_2.id, issue: issue.id, reference_type: 'issue') + + doc = filter(link, current_user: member) + + expect(doc.css('a').length).to eq 1 + end + end end it 'allows references for non confidential issues' do user = create(:user) project = create(:project, :public) issue = create(:issue, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') + doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 @@ -172,8 +214,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do it 'removes unpermitted Group references' do user = create(:user) group = create(:group, :private) - link = reference_link(group: group.id, reference_type: 'user') + doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 0 @@ -183,8 +225,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do user = create(:user) group = create(:group, :private) group.add_developer(user) - link = reference_link(group: group.id, reference_type: 'user') + doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 @@ -200,8 +242,8 @@ describe Banzai::Filter::ReferenceRedactorFilter do context 'with data-user' do it 'allows any User reference' do user = create(:user) - link = reference_link(user: user.id, reference_type: 'user') + doc = filter(link) expect(doc.css('a').length).to eq 1 diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index ed763f6375..76be2c0cd2 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -165,6 +165,12 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do expect(subject).to eq(Gitlab::Auth::Result.new(build.user, build.project, :build, described_class.build_authentication_abilities)) end + + it 'fails with blocked user token' do + build.update(user: create(:user, :blocked)) + + expect(subject).to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil)) + end end (HasStatus::AVAILABLE_STATUSES - ['running']).each do |build_status| @@ -260,6 +266,15 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip') end + + context 'blocked user' do + let(:user) { create(:user, :blocked) } + + it 'fails' do + expect(gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip')) + .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil)) + end + end end context 'while using personal access tokens as passwords' do @@ -308,9 +323,35 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do it 'fails if password is nil' do expect_results_with_abilities(nil, nil, false) end + + context 'when user is blocked' do + let(:user) { create(:user, :blocked) } + let(:personal_access_token) { create(:personal_access_token, scopes: ['read_registry'], user: user) } + + before do + stub_container_registry_config(enabled: true) + end + + it 'fails if user is blocked' do + expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')) + .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil)) + end + end end context 'while using regular user and password' do + it 'fails for a blocked user' do + user = create( + :user, + :blocked, + username: 'normal_user', + password: 'my-secret' + ) + + expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')) + .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil)) + end + it 'goes through lfs authentication' do user = create( :user, diff --git a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb index 5a930d44dc..ebd7c7af26 100644 --- a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb +++ b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb @@ -68,6 +68,16 @@ describe Gitlab::Gfm::UploadsRewriter do expect(moved_text.scan(/\A\[.*?\]/).count).to eq(1) end + context 'path traversal in file name' do + let(:text) do + "![a](/uploads/11111111111111111111111111111111/../../../../../../../../../../../../../../etc/passwd)" + end + + it 'throw an error' do + expect { rewriter.rewrite(new_project) }.to raise_error(an_instance_of(StandardError).and having_attributes(message: "Invalid path")) + end + end + context "file are stored locally" do include_examples "files are accessible" end diff --git a/spec/lib/gitlab/hotlinking_detector_spec.rb b/spec/lib/gitlab/hotlinking_detector_spec.rb new file mode 100644 index 0000000000..536d744c19 --- /dev/null +++ b/spec/lib/gitlab/hotlinking_detector_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Gitlab::HotlinkingDetector do + describe ".intercept_hotlinking?" do + using RSpec::Parameterized::TableSyntax + + subject { described_class.intercept_hotlinking?(request) } + + let(:request) { double("request", headers: headers) } + let(:headers) { {} } + + context "hotlinked as media" do + where(:return_value, :accept_header) do + # These are default formats in modern browsers, and IE + false | "*/*" + false | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" + false | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8" + false | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" + false | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8" + false | "image/jpeg, application/x-ms-application, image/gif, application/xaml+xml, image/pjpeg, application/x-ms-xbap, application/x-shockwave-flash, application/msword, */*" + false | "text/html, application/xhtml+xml, image/jxr, */*" + false | "text/html, application/xml;q=0.9, application/xhtml+xml, image/png, image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1" + + # These are image request formats + true | "image/webp,*/*" + true | "image/png,image/*;q=0.8,*/*;q=0.5" + true | "image/webp,image/apng,image/*,*/*;q=0.8" + true | "image/png,image/svg+xml,image/*;q=0.8, */*;q=0.5" + + # Video request formats + true | "video/webm,video/ogg,video/*;q=0.9,application/ogg;q=0.7,audio/*;q=0.6,*/*;q=0.5" + + # Audio request formats + true | "audio/webm,audio/ogg,audio/wav,audio/*;q=0.9,application/ogg;q=0.7,video/*;q=0.6,*/*;q=0.5" + + # CSS request formats + true | "text/css,*/*;q=0.1" + true | "text/css" + true | "text/css,*/*;q=0.1" + end + + with_them do + let(:headers) do + { "Accept" => accept_header } + end + + it { is_expected.to be(return_value) } + end + end + + context "hotlinked as a script" do + where(:return_value, :fetch_mode) do + # Standard navigation fetch modes + false | "navigate" + false | "nested-navigate" + false | "same-origin" + + # Fetch modes when linking as JS + true | "cors" + true | "no-cors" + true | "websocket" + end + + with_them do + let(:headers) do + { "Sec-Fetch-Mode" => fetch_mode } + end + + it { is_expected.to be(return_value) } + end + end + end +end diff --git a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb index 12857f97f7..65e99c0c3b 100644 --- a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb +++ b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb @@ -32,6 +32,9 @@ describe Gitlab::ImportExport::AttributeCleaner do 'issue_ids' => [1, 2, 3], 'merge_request_ids' => [1, 2, 3], 'note_ids' => [1, 2, 3], + 'remote_attachment_url' => 'http://something.dodgy', + 'remote_attachment_request_header' => 'bad value', + 'remote_attachment_urls' => %w(http://something.dodgy http://something.okay), 'attributes' => { 'issue_ids' => [1, 2, 3], 'merge_request_ids' => [1, 2, 3], diff --git a/spec/lib/gitlab/middleware/multipart_spec.rb b/spec/lib/gitlab/middleware/multipart_spec.rb index 3379781757..1d3ad3afb5 100644 --- a/spec/lib/gitlab/middleware/multipart_spec.rb +++ b/spec/lib/gitlab/middleware/multipart_spec.rb @@ -5,9 +5,7 @@ require 'spec_helper' require 'tempfile' describe Gitlab::Middleware::Multipart do - let(:app) { double(:app) } - let(:middleware) { described_class.new(app) } - let(:original_filename) { 'filename' } + include_context 'multipart middleware context' shared_examples_for 'multipart upload files' do it 'opens top-level files' do @@ -82,22 +80,12 @@ describe Gitlab::Middleware::Multipart do end it 'allows files in uploads/tmp directory' do - Dir.mktmpdir do |dir| - uploads_dir = File.join(dir, 'public/uploads/tmp') - FileUtils.mkdir_p(uploads_dir) - - allow(Rails).to receive(:root).and_return(dir) - allow(Dir).to receive(:tmpdir).and_return(File.join(Dir.tmpdir, 'tmpsubdir')) - - Tempfile.open('top-level', uploads_dir) do |tempfile| - env = post_env({ 'file' => tempfile.path }, { 'file.name' => original_filename, 'file.path' => tempfile.path }, Gitlab::Workhorse.secret, 'gitlab-workhorse') - - expect(app).to receive(:call) do |env| - expect(get_params(env)['file']).to be_a(::UploadedFile) - end - - middleware.call(env) + with_tmp_dir('public/uploads/tmp') do |dir, env| + expect(app).to receive(:call) do |env| + expect(get_params(env)['file']).to be_a(::UploadedFile) end + + middleware.call(env) end end @@ -127,22 +115,4 @@ describe Gitlab::Middleware::Multipart do middleware.call(env) end end - - # Rails 5 doesn't combine the GET/POST parameters in - # ActionDispatch::HTTP::Parameters if action_dispatch.request.parameters is set: - # https://github.com/rails/rails/blob/aea6423f013ca48f7704c70deadf2cd6ac7d70a1/actionpack/lib/action_dispatch/http/parameters.rb#L41 - def get_params(env) - req = ActionDispatch::Request.new(env) - req.GET.merge(req.POST) - end - - def post_env(rewritten_fields, params, secret, issuer) - token = JWT.encode({ 'iss' => issuer, 'rewritten_fields' => rewritten_fields }, secret, 'HS256') - Rack::MockRequest.env_for( - '/', - method: 'post', - params: params, - described_class::RACK_ENV_KEY => token - ) - end end diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index c580b46cf8..f1b5393a2d 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -3,9 +3,7 @@ require 'spec_helper' describe Gitlab::Regex do - describe '.project_name_regex' do - subject { described_class.project_name_regex } - + shared_examples_for 'project/group name regex' do it { is_expected.to match('gitlab-ce') } it { is_expected.to match('GitLab CE') } it { is_expected.to match('100 lines') } @@ -15,6 +13,34 @@ describe Gitlab::Regex do it { is_expected.not_to match('?gitlab') } end + shared_examples_for 'project/group name error message' do + it { is_expected.to eq("can contain only letters, digits, emojis, '_', '.', dash, space. It must start with letter, digit, emoji or '_'.") } + end + + describe '.project_name_regex' do + subject { described_class.project_name_regex } + + it_behaves_like 'project/group name regex' + end + + describe '.group_name_regex' do + subject { described_class.group_name_regex } + + it_behaves_like 'project/group name regex' + end + + describe '.project_name_regex_message' do + subject { described_class.project_name_regex_message } + + it_behaves_like 'project/group name error message' + end + + describe '.group_name_regex_message' do + subject { described_class.group_name_regex_message } + + it_behaves_like 'project/group name error message' + end + describe '.environment_name_regex' do subject { described_class.environment_name_regex } diff --git a/spec/lib/gitlab/x509/commit_spec.rb b/spec/lib/gitlab/x509/commit_spec.rb index 9cddf27ddc..c31e9e4b8e 100644 --- a/spec/lib/gitlab/x509/commit_spec.rb +++ b/spec/lib/gitlab/x509/commit_spec.rb @@ -204,5 +204,38 @@ describe Gitlab::X509::Commit do expect(described_class.new(commit).signature).to be_nil end end + + context 'certificate_crl' do + let!(:commit) { create :commit, project: project, sha: commit_sha, created_at: Time.utc(2019, 1, 1, 20, 15, 0), committer_email: X509Helpers::User1.emails.first } + let(:signed_commit) { described_class.new(commit) } + + describe 'valid crlDistributionPoints' do + before do + allow(signed_commit).to receive(:get_certificate_extension).and_call_original + + allow(signed_commit).to receive(:get_certificate_extension) + .with('crlDistributionPoints') + .and_return("\nFull Name:\n URI:http://ch.siemens.com/pki?ZZZZZZA2.crl\n URI:ldap://cl.siemens.net/CN=ZZZZZZA2,L=PKI?certificateRevocationList\n URI:ldap://cl.siemens.com/CN=ZZZZZZA2,o=Trustcenter?certificateRevocationList\n") + end + + it 'returns an unverified signature' do + expect(signed_commit.signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes) + end + end + + describe 'valid crlDistributionPoints providing multiple http URIs' do + before do + allow(signed_commit).to receive(:get_certificate_extension).and_call_original + + allow(signed_commit).to receive(:get_certificate_extension) + .with('crlDistributionPoints') + .and_return("\nFull Name:\n URI:http://cdp1.pca.dfn.de/dfn-ca-global-g2/pub/crl/cacrl.crl\n\nFull Name:\n URI:http://cdp2.pca.dfn.de/dfn-ca-global-g2/pub/crl/cacrl.crl\n") + end + + it 'extracts the first URI' do + expect(signed_commit.signature.x509_certificate.x509_issuer.crl_url).to eq("http://cdp1.pca.dfn.de/dfn-ca-global-g2/pub/crl/cacrl.crl") + end + end + end end end diff --git a/spec/lib/uploaded_file_spec.rb b/spec/lib/uploaded_file_spec.rb index 2bbbd67b13..25536c07dd 100644 --- a/spec/lib/uploaded_file_spec.rb +++ b/spec/lib/uploaded_file_spec.rb @@ -59,6 +59,16 @@ describe UploadedFile do expect(subject.sha256).to eq('sha256') expect(subject.remote_id).to eq('remote_id') end + + it 'handles a blank path' do + params['file.path'] = '' + + # Not a real file, so can't determine size itself + params['file.size'] = 1.byte + + expect { described_class.from_params(params, :file, upload_path) } + .not_to raise_error + end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index d42888e1d5..66df7792a1 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -48,6 +48,9 @@ describe Group do describe 'validations' do it { is_expected.to validate_presence_of :name } + it { is_expected.to allow_value('group test_4').for(:name) } + it { is_expected.not_to allow_value('test/../foo').for(:name) } + it { is_expected.not_to allow_value('').for(:name) } it { is_expected.to validate_presence_of :path } it { is_expected.not_to validate_presence_of :owner } it { is_expected.to validate_presence_of :two_factor_grace_period } diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index c0501fb16c..3f1bc8a2de 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -536,88 +536,146 @@ describe Issue do end describe '#visible_to_user?' do + let(:project) { build(:project) } + let(:issue) { build(:issue, project: project) } + let(:user) { create(:user) } + + subject { issue.visible_to_user?(user) } + + context 'with a project' do + it 'returns false when feature is disabled' do + project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED) + + is_expected.to eq(false) + end + + it 'returns false when restricted for members' do + project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE) + + is_expected.to eq(false) + end + end + context 'without a user' do - let(:issue) { build(:issue) } + let(:user) { nil } it 'returns true when the issue is publicly visible' do expect(issue).to receive(:publicly_visible?).and_return(true) - expect(issue.visible_to_user?).to eq(true) + is_expected.to eq(true) end it 'returns false when the issue is not publicly visible' do expect(issue).to receive(:publicly_visible?).and_return(false) - expect(issue.visible_to_user?).to eq(false) + is_expected.to eq(false) end end context 'with a user' do - let(:user) { create(:user) } - let(:issue) { build(:issue) } - - it 'returns true when the issue is readable' do - expect(issue).to receive(:readable_by?).with(user).and_return(true) - - expect(issue.visible_to_user?(user)).to eq(true) + shared_examples 'issue readable by user' do + it { is_expected.to eq(true) } end - it 'returns false when the issue is not readable' do - expect(issue).to receive(:readable_by?).with(user).and_return(false) - - expect(issue.visible_to_user?(user)).to eq(false) + shared_examples 'issue not readable by user' do + it { is_expected.to eq(false) } end - it 'returns false when feature is disabled' do - expect(issue).not_to receive(:readable_by?) + shared_examples 'confidential issue readable by user' do + specify do + issue.confidential = true - issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED) - - expect(issue.visible_to_user?(user)).to eq(false) - end - - it 'returns false when restricted for members' do - expect(issue).not_to receive(:readable_by?) - - issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE) - - expect(issue.visible_to_user?(user)).to eq(false) - end - end - - describe 'with a regular user that is not a team member' do - let(:user) { create(:user) } - - context 'using a public project' do - let(:project) { create(:project, :public) } - - it 'returns true for a regular issue' do - issue = build(:issue, project: project) - - expect(issue.visible_to_user?(user)).to eq(true) - end - - it 'returns false for a confidential issue' do - issue = build(:issue, project: project, confidential: true) - - expect(issue.visible_to_user?(user)).to eq(false) + is_expected.to eq(true) end end - context 'using an internal project' do - let(:project) { create(:project, :internal) } + shared_examples 'confidential issue not readable by user' do + specify do + issue.confidential = true - context 'using an internal user' do - it 'returns true for a regular issue' do - issue = build(:issue, project: project) + is_expected.to eq(false) + end + end - expect(issue.visible_to_user?(user)).to eq(true) + context 'with an admin user' do + let(:user) { build(:admin) } + + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue readable by user' + end + + context 'with an owner' do + before do + project.add_maintainer(user) + end + + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue readable by user' + end + + context 'with a reporter user' do + before do + project.add_reporter(user) + end + + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue readable by user' + end + + context 'with a guest user' do + before do + project.add_guest(user) + end + + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue not readable by user' + + context 'when user is an assignee' do + before do + issue.update!(assignees: [user]) end - it 'returns false for a confidential issue' do - issue = build(:issue, :confidential, project: project) + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue readable by user' + end - expect(issue.visible_to_user?(user)).to eq(false) + context 'when user is the author' do + before do + issue.update!(author: user) + end + + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue readable by user' + end + end + + context 'with a user that is not a member' do + context 'using a public project' do + let(:project) { build(:project, :public) } + + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue not readable by user' + end + + context 'using an internal project' do + let(:project) { build(:project, :internal) } + + context 'using an internal user' do + before do + allow(user).to receive(:external?).and_return(false) + end + + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue not readable by user' + end + + context 'using an external user' do + before do + allow(user).to receive(:external?).and_return(true) + end + + it_behaves_like 'issue not readable by user' + it_behaves_like 'confidential issue not readable by user' end end @@ -626,134 +684,112 @@ describe Issue do allow(user).to receive(:external?).and_return(true) end - it 'returns false for a regular issue' do - issue = build(:issue, project: project) - - expect(issue.visible_to_user?(user)).to eq(false) - end - - it 'returns false for a confidential issue' do - issue = build(:issue, :confidential, project: project) - - expect(issue.visible_to_user?(user)).to eq(false) - end + it_behaves_like 'issue not readable by user' + it_behaves_like 'confidential issue not readable by user' end end - context 'using a private project' do - let(:project) { create(:project, :private) } + context 'with an external authentication service' do + before do + enable_external_authorization_service_check + end - it 'returns false for a regular issue' do + it 'is `false` when an external authorization service is enabled' do + issue = build(:issue, project: build(:project, :public)) + + expect(issue).not_to be_visible_to_user + end + + it 'checks the external service to determine if an issue is readable by a user' do + project = build(:project, :public, + external_authorization_classification_label: 'a-label') issue = build(:issue, project: project) + user = build(:user) - expect(issue.visible_to_user?(user)).to eq(false) + expect(::Gitlab::ExternalAuthorization).to receive(:access_allowed?).with(user, 'a-label') { false } + expect(issue.visible_to_user?(user)).to be_falsy end - it 'returns false for a confidential issue' do - issue = build(:issue, :confidential, project: project) + it 'does not check the external service if a user does not have access to the project' do + project = build(:project, :private, + external_authorization_classification_label: 'a-label') + issue = build(:issue, project: project) + user = build(:user) - expect(issue.visible_to_user?(user)).to eq(false) + expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?) + expect(issue.visible_to_user?(user)).to be_falsy end - context 'when the user is the project owner' do + it 'does not check the external webservice for admins' do + issue = build(:issue) + user = build(:admin) + + expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?) + + issue.visible_to_user?(user) + end + end + + context 'when issue is moved to a private project' do + let(:private_project) { build(:project, :private)} + + before do + issue.update(project: private_project) # move issue to private project + end + + shared_examples 'issue visible if user has guest access' do + context 'when user is not a member' do + it_behaves_like 'issue not readable by user' + it_behaves_like 'confidential issue not readable by user' + end + + context 'when user is a guest' do + before do + private_project.add_guest(user) + end + + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue readable by user' + end + end + + context 'when user is the author of the original issue' do before do - project.add_maintainer(user) + issue.update!(author: user) end - it 'returns true for a regular issue' do - issue = build(:issue, project: project) + it_behaves_like 'issue visible if user has guest access' + end - expect(issue.visible_to_user?(user)).to eq(true) + context 'when user is an assignee in the original issue' do + before do + issue.update!(assignees: [user]) end - it 'returns true for a confidential issue' do - issue = build(:issue, :confidential, project: project) + it_behaves_like 'issue visible if user has guest access' + end - expect(issue.visible_to_user?(user)).to eq(true) + context 'when user is not the author or an assignee in original issue' do + context 'when user is a guest' do + before do + private_project.add_guest(user) + end + + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue not readable by user' + end + + context 'when user is a reporter' do + before do + private_project.add_reporter(user) + end + + it_behaves_like 'issue readable by user' + it_behaves_like 'confidential issue readable by user' end end end end - - context 'with a regular user that is a team member' do - let(:user) { create(:user) } - let(:project) { create(:project, :public) } - - context 'using a public project' do - before do - project.add_developer(user) - end - - it 'returns true for a regular issue' do - issue = build(:issue, project: project) - - expect(issue.visible_to_user?(user)).to eq(true) - end - - it 'returns true for a confidential issue' do - issue = build(:issue, :confidential, project: project) - - expect(issue.visible_to_user?(user)).to eq(true) - end - end - - context 'using an internal project' do - let(:project) { create(:project, :internal) } - - before do - project.add_developer(user) - end - - it 'returns true for a regular issue' do - issue = build(:issue, project: project) - - expect(issue.visible_to_user?(user)).to eq(true) - end - - it 'returns true for a confidential issue' do - issue = build(:issue, :confidential, project: project) - - expect(issue.visible_to_user?(user)).to eq(true) - end - end - - context 'using a private project' do - let(:project) { create(:project, :private) } - - before do - project.add_developer(user) - end - - it 'returns true for a regular issue' do - issue = build(:issue, project: project) - - expect(issue.visible_to_user?(user)).to eq(true) - end - - it 'returns true for a confidential issue' do - issue = build(:issue, :confidential, project: project) - - expect(issue.visible_to_user?(user)).to eq(true) - end - end - end - - context 'with an admin user' do - let(:project) { create(:project) } - let(:user) { create(:admin) } - - it 'returns true for a regular issue' do - issue = build(:issue, project: project) - - expect(issue.visible_to_user?(user)).to eq(true) - end - - it 'returns true for a confidential issue' do - issue = build(:issue, :confidential, project: project) - - expect(issue.visible_to_user?(user)).to eq(true) - end - end end describe '#publicly_visible?' do @@ -875,49 +911,6 @@ describe Issue do subject { create(:issue, updated_at: 1.hour.ago) } end - context 'when an external authentication service' do - before do - enable_external_authorization_service_check - end - - describe '#visible_to_user?' do - it 'is `false` when an external authorization service is enabled' do - issue = build(:issue, project: build(:project, :public)) - - expect(issue).not_to be_visible_to_user - end - - it 'checks the external service to determine if an issue is readable by a user' do - project = build(:project, :public, - external_authorization_classification_label: 'a-label') - issue = build(:issue, project: project) - user = build(:user) - - expect(::Gitlab::ExternalAuthorization).to receive(:access_allowed?).with(user, 'a-label') { false } - expect(issue.visible_to_user?(user)).to be_falsy - end - - it 'does not check the external service if a user does not have access to the project' do - project = build(:project, :private, - external_authorization_classification_label: 'a-label') - issue = build(:issue, project: project) - user = build(:user) - - expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?) - expect(issue.visible_to_user?(user)).to be_falsy - end - - it 'does not check the external webservice for admins' do - issue = build(:issue) - user = build(:admin) - - expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?) - - issue.visible_to_user?(user) - end - end - end - describe "#labels_hook_attrs" do let(:label) { create(:label) } let(:issue) { create(:labeled_issue, labels: [label]) } diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index d2a54c3eea..3b7caeae9f 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -96,8 +96,8 @@ describe PagesDomain do it 'saves validity time' do domain.save - expect(domain.certificate_valid_not_before).to be_like_time(Time.parse("2016-02-12 14:32:00 UTC")) - expect(domain.certificate_valid_not_after).to be_like_time(Time.parse("2020-04-12 14:32:00 UTC")) + expect(domain.certificate_valid_not_before).to be_like_time(Time.parse("2020-03-16 14:20:34 UTC")) + expect(domain.certificate_valid_not_after).to be_like_time(Time.parse("2220-01-28 14:20:34 UTC")) end end diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb index 89fcf3c10d..242a002bc2 100644 --- a/spec/policies/issue_policy_spec.rb +++ b/spec/policies/issue_policy_spec.rb @@ -103,12 +103,24 @@ describe IssuePolicy do expect(permissions(author, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end + it 'does not allow issue author to read or update confidential issue moved to an private project' do + confidential_issue.project = build(:project, :private) + + expect(permissions(author, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue) + end + it 'allows issue assignees to read and update their confidential issues' do expect(permissions(assignee, confidential_issue)).to be_allowed(:read_issue, :read_issue_iid, :update_issue) expect(permissions(assignee, confidential_issue)).to be_disallowed(:admin_issue) expect(permissions(assignee, confidential_issue_no_assignee)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue, :admin_issue) end + + it 'does not allow issue assignees to read or update confidential issue moved to an private project' do + confidential_issue.project = build(:project, :private) + + expect(permissions(assignee, confidential_issue)).to be_disallowed(:read_issue, :read_issue_iid, :update_issue) + end end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 35b77832c7..4421f0f118 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -568,6 +568,20 @@ describe API::Groups do expect(json_response['shared_projects'].length).to eq(0) end + context 'malicious group name' do + subject { put api("/groups/#{group1.id}", user1), params: { name: "" } } + + it 'returns bad request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'does not update group name' do + expect { subject }.not_to change { group1.reload.name } + end + end + it 'returns 404 for a non existing group' do put api('/groups/1328', user1), params: { name: new_group_name } @@ -999,6 +1013,20 @@ describe API::Groups do expect(json_response["parent_id"]).to eq(parent.id) end + context 'malicious group name' do + subject { post api("/groups", user3), params: group_params } + + let(:group_params) { attributes_for_group_api name: "", path: "unique-url" } + + it 'returns bad request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it { expect { subject }.not_to change { Group.count } } + end + it "does not create group, duplicate" do post api("/groups", user3), params: { name: 'Duplicate Test', path: group2.path } diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 2c6a13efc1..b618800857 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -98,6 +98,30 @@ describe API::ProjectSnippets do } end + context 'with an external user' do + let(:user) { create(:user, :external) } + + context 'that belongs to the project' do + before do + project.add_developer(user) + end + + it 'creates a new snippet' do + post api("/projects/#{project.id}/snippets/", user), params: params + + expect(response).to have_gitlab_http_status(201) + end + end + + context 'that does not belong to the project' do + it 'does not create a new snippet' do + post api("/projects/#{project.id}/snippets/", user), params: params + + expect(response).to have_gitlab_http_status(403) + end + end + end + context 'with a regular user' do let(:user) { create(:user) } diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 8bca458bec..cea08aa876 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -263,6 +263,18 @@ describe API::Repositories do let(:message) { '404 File Not Found' } end end + + context "when hotlinking detection is enabled" do + before do + Feature.enable(:repository_archive_hotlinking_interception) + end + + it_behaves_like "hotlink interceptor" do + let(:http_request) do + get api(route, current_user), headers: headers + end + end + end end context 'when unauthenticated', 'and project is public' do diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 21565265b9..c93f9a34e0 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -224,6 +224,16 @@ describe API::Snippets do it_behaves_like 'snippet creation' + context 'with an external user' do + let(:user) { create(:user, :external) } + + it 'does not create a new snippet' do + post api("/snippets/", user), params: params + + expect(response).to have_gitlab_http_status(403) + end + end + it 'returns 400 for missing parameters' do params.delete(:title) diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index 1042e4e970..e60318d1c3 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -238,24 +238,44 @@ describe API::Triggers do end describe 'PUT /projects/:id/triggers/:trigger_id' do - context 'authenticated user with valid permissions' do - let(:new_description) { 'new description' } + context 'user is maintainer of the project' do + context 'the trigger belongs to user' do + let(:new_description) { 'new description' } - it 'updates description' do - put api("/projects/#{project.id}/triggers/#{trigger.id}", user), - params: { description: new_description } + it 'updates description' do + put api("/projects/#{project.id}/triggers/#{trigger.id}", user), + params: { description: new_description } - expect(response).to have_gitlab_http_status(200) - expect(json_response).to include('description' => new_description) - expect(trigger.reload.description).to eq(new_description) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include('description' => new_description) + expect(trigger.reload.description).to eq(new_description) + end + end + + context 'the trigger does not belong to user' do + it 'does not update trigger' do + put api("/projects/#{project.id}/triggers/#{trigger2.id}", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end end end - context 'authenticated user with invalid permissions' do - it 'does not update trigger' do - put api("/projects/#{project.id}/triggers/#{trigger.id}", user2) + context 'user is developer of the project' do + context 'the trigger belongs to user' do + it 'does not update trigger' do + put api("/projects/#{project.id}/triggers/#{trigger2.id}", user2) - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'the trigger does not belong to user' do + it 'does not update trigger' do + put api("/projects/#{project.id}/triggers/#{trigger.id}", user2) + + expect(response).to have_gitlab_http_status(:forbidden) + end end end diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index 754ab3e6a4..73dc9d8c63 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -25,6 +25,17 @@ describe JwtController do end context 'when using authenticated request' do + shared_examples 'rejecting a blocked user' do + context 'with blocked user' do + let(:user) { create(:user, :blocked) } + + it 'rejects the request as unauthorized' do + expect(response).to have_gitlab_http_status(:unauthorized) + expect(response.body).to include('HTTP Basic: Access denied') + end + end + end + context 'using CI token' do let(:build) { create(:ci_build, :running) } let(:project) { build.project } @@ -61,6 +72,8 @@ describe JwtController do expect(response).to have_gitlab_http_status(:ok) expect(service_class).to have_received(:new).with(nil, user, ActionController::Parameters.new(parameters).permit!) end + + it_behaves_like 'rejecting a blocked user' end end @@ -72,6 +85,8 @@ describe JwtController do it { expect(service_class).to have_received(:new).with(nil, user, ActionController::Parameters.new(parameters).permit!) } + it_behaves_like 'rejecting a blocked user' + context 'when passing a flat array of scopes' do # We use this trick to make rails to generate a query_string: # scope=scope1&scope=scope2 diff --git a/spec/serializers/merge_request_poll_widget_entity_spec.rb b/spec/serializers/merge_request_poll_widget_entity_spec.rb index 0593dd527c..29d35fdc81 100644 --- a/spec/serializers/merge_request_poll_widget_entity_spec.rb +++ b/spec/serializers/merge_request_poll_widget_entity_spec.rb @@ -138,7 +138,7 @@ describe MergeRequestPollWidgetEntity do end describe 'pipeline' do - let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) } + let!(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) } before do allow_any_instance_of(MergeRequestPresenter).to receive(:can?).and_call_original @@ -158,6 +158,10 @@ describe MergeRequestPollWidgetEntity do expect(subject[:pipeline]).to eq(pipeline_payload) end + + it 'returns ci_status' do + expect(subject[:ci_status]).to eq('pending') + end end context 'when is not up to date' do @@ -171,10 +175,15 @@ describe MergeRequestPollWidgetEntity do context 'when user does not have access to pipelines' do let(:result) { false } + let(:req) { double('request', current_user: user, project: project) } it 'does not have pipeline' do expect(subject[:pipeline]).to eq(nil) end + + it 'does not return ci_status' do + expect(subject[:ci_status]).to eq(nil) + end end end end diff --git a/spec/support/helpers/workhorse_helpers.rb b/spec/support/helpers/workhorse_helpers.rb index e0fba191de..0de8b38240 100644 --- a/spec/support/helpers/workhorse_helpers.rb +++ b/spec/support/helpers/workhorse_helpers.rb @@ -76,7 +76,7 @@ module WorkhorseHelpers "#{key}.size" => file.size }.tap do |params| params["#{key}.path"] = file.path if file.path - params["#{key}.remote_id"] = file.remote_id if file.respond_to?(:remote_id) && file.remote_id + params["#{key}.remote_id"] = file.remote_id if file.respond_to?(:remote_id) && file.remote_id.present? end end diff --git a/spec/support/shared_contexts/lib/gitlab/middleware/multipart_shared_contexts.rb b/spec/support/shared_contexts/lib/gitlab/middleware/multipart_shared_contexts.rb new file mode 100644 index 0000000000..f1554ea8e9 --- /dev/null +++ b/spec/support/shared_contexts/lib/gitlab/middleware/multipart_shared_contexts.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +RSpec.shared_context 'multipart middleware context' do + let(:app) { double(:app) } + let(:middleware) { described_class.new(app) } + let(:original_filename) { 'filename' } + + # Rails 5 doesn't combine the GET/POST parameters in + # ActionDispatch::HTTP::Parameters if action_dispatch.request.parameters is set: + # https://github.com/rails/rails/blob/aea6423f013ca48f7704c70deadf2cd6ac7d70a1/actionpack/lib/action_dispatch/http/parameters.rb#L41 + def get_params(env) + req = ActionDispatch::Request.new(env) + req.GET.merge(req.POST) + end + + def post_env(rewritten_fields, params, secret, issuer) + token = JWT.encode({ 'iss' => issuer, 'rewritten_fields' => rewritten_fields }, secret, 'HS256') + Rack::MockRequest.env_for( + '/', + method: 'post', + params: params, + described_class::RACK_ENV_KEY => token + ) + end + + def with_tmp_dir(uploads_sub_dir, storage_path = '') + Dir.mktmpdir do |dir| + upload_dir = File.join(dir, storage_path, uploads_sub_dir) + FileUtils.mkdir_p(upload_dir) + + allow(Rails).to receive(:root).and_return(dir) + allow(Dir).to receive(:tmpdir).and_return(File.join(Dir.tmpdir, 'tmpsubdir')) + allow(GitlabUploader).to receive(:root).and_return(File.join(dir, storage_path)) + + Tempfile.open('top-level', upload_dir) do |tempfile| + env = post_env({ 'file' => tempfile.path }, { 'file.name' => original_filename, 'file.path' => tempfile.path }, Gitlab::Workhorse.secret, 'gitlab-workhorse') + + yield dir, env + end + end + end +end diff --git a/spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb b/spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb new file mode 100644 index 0000000000..93a394387a --- /dev/null +++ b/spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +RSpec.shared_examples "hotlink interceptor" do + let(:http_request) { nil } + let(:headers) { nil } + + describe "DDOS prevention" do + using RSpec::Parameterized::TableSyntax + + context "hotlinked as media" do + where(:response_status, :accept_header) do + # These are default formats in modern browsers, and IE + :ok | "*/*" + :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" + :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8" + :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" + :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8" + :ok | "image/jpeg, application/x-ms-application, image/gif, application/xaml+xml, image/pjpeg, application/x-ms-xbap, application/x-shockwave-flash, application/msword, */*" + :ok | "text/html, application/xhtml+xml, image/jxr, */*" + :ok | "text/html, application/xml;q=0.9, application/xhtml+xml, image/png, image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1" + + # These are image request formats + :not_acceptable | "image/webp,*/*" + :not_acceptable | "image/png,image/*;q=0.8,*/*;q=0.5" + :not_acceptable | "image/webp,image/apng,image/*,*/*;q=0.8" + :not_acceptable | "image/png,image/svg+xml,image/*;q=0.8, */*;q=0.5" + + # Video request formats + :not_acceptable | "video/webm,video/ogg,video/*;q=0.9,application/ogg;q=0.7,audio/*;q=0.6,*/*;q=0.5" + + # Audio request formats + :not_acceptable | "audio/webm,audio/ogg,audio/wav,audio/*;q=0.9,application/ogg;q=0.7,video/*;q=0.6,*/*;q=0.5" + + # CSS request formats + :not_acceptable | "text/css,*/*;q=0.1" + :not_acceptable | "text/css" + :not_acceptable | "text/css,*/*;q=0.1" + end + + with_them do + let(:headers) do + { "Accept" => accept_header } + end + + before do + request.headers.merge!(headers) if request.present? + end + + it "renders the response" do + http_request + + expect(response).to have_gitlab_http_status(response_status) + end + end + end + + context "hotlinked as a script" do + where(:response_status, :fetch_mode) do + # Standard navigation fetch modes + :ok | "navigate" + :ok | "nested-navigate" + :ok | "same-origin" + + # Fetch modes when linking as JS + :not_acceptable | "cors" + :not_acceptable | "no-cors" + :not_acceptable | "websocket" + end + + with_them do + let(:headers) do + { "Sec-Fetch-Mode" => fetch_mode } + end + + before do + request.headers.merge!(headers) if request.present? + end + + it "renders the response" do + http_request + + expect(response).to have_gitlab_http_status(response_status) + end + end + end + end +end diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index 2f2ed28891..2b9f55aa8f 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -714,6 +714,19 @@ describe ObjectStorage do end end + context 'when empty remote_id is specified' do + let(:uploaded_file) do + UploadedFile.new(temp_file.path, remote_id: '') + end + + it 'uses local storage' do + subject + + expect(uploader).to be_file_storage + expect(uploader.object_store).to eq(described_class::Store::LOCAL) + end + end + context 'when valid file is specified' do let(:uploaded_file) do UploadedFile.new(temp_file.path, filename: "my_file.txt", remote_id: "test/123123")