diff --git a/CHANGELOG.md b/CHANGELOG.md index 968b872c52..2edfb07981 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,48 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. -## 11.8.10 (2019-04-30) +## 11.10.5 (2019-05-30) + +### Security (12 changes, 1 of them is from the community) + +- Protect Gitlab::HTTP against DNS rebinding attack. +- Fix project visibility level validation. (Peter Marko) +- Update Knative version. +- Add DNS rebinding protection settings. +- Prevent XSS injection in note imports. +- Prevent invalid branch for merge request. +- Filter relative links in wiki for XSS. +- Fix confidential issue label disclosure on milestone view. +- Fix url redaction for issue links. +- Resolve: Milestones leaked via search API. +- Prevent bypass of restriction disabling web password sign in. +- Hide confidential issue title on unsubscribe for anonymous users. + + +## 11.10.4 (2019-05-01) + +### Fixed (12 changes) + +- Fix MR popover on ToDos page. !27382 +- Fix 500 in general pipeline settings when passing an invalid build timeout. !27416 +- Fix bug where system note MR has no popover. !27589 +- Fix bug when project export to remote url fails. !27614 +- `on_stop` is not automatically triggered with pipelines for merge requests. !27618 +- Update Workhorse to v8.5.2. !27631 +- Show proper wiki links in search results. !27634 +- Make `CI_COMMIT_REF_NAME` and `SLUG` variable idempotent. !27663 +- Fix Kubernetes service template deployment jobs broken as of 11.10.0. !27687 +- Prevent text selection when dragging in issue boards. !27724 +- Fix pipelines for merge requests does not show pipeline page when source branch is removed. !27803 +- Fix Metrics Environments dropdown. + +### Performance (2 changes) + +- Prevent concurrent execution of PipelineScheduleWorker. !27781 +- Fix slow performance with compiling HAML templates. !27782 + + +## 11.10.3 (2019-04-30) ### Security (1 change) diff --git a/VERSION b/VERSION index e218d1c6e0..5413747c68 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -11.8.10 +11.10.5 diff --git a/app/controllers/concerns/milestone_actions.rb b/app/controllers/concerns/milestone_actions.rb index c0c0160a82..c6a59c450a 100644 --- a/app/controllers/concerns/milestone_actions.rb +++ b/app/controllers/concerns/milestone_actions.rb @@ -31,7 +31,7 @@ module MilestoneActions format.html { redirect_to milestone_redirect_path } format.json do render json: tabs_json("shared/milestones/_labels_tab", { - labels: @milestone.labels # rubocop:disable Gitlab/ModuleWithInstanceVariables + labels: @milestone.issue_labels_visible_by_user(current_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables }) end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 4bd7d71e26..bbf734b69f 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -15,6 +15,8 @@ class SessionsController < Devise::SessionsController prepend_before_action :check_captcha, only: [:create] prepend_before_action :store_redirect_uri, only: [:new] prepend_before_action :ldap_servers, only: [:new, :create] + prepend_before_action :ensure_password_authentication_enabled!, if: :password_based_login?, only: [:create] + before_action :auto_sign_in_with_provider, only: [:new] before_action :load_recaptcha @@ -126,6 +128,14 @@ class SessionsController < Devise::SessionsController end # rubocop: enable CodeReuse/ActiveRecord + def ensure_password_authentication_enabled! + render_403 unless Gitlab::CurrentSettings.password_authentication_enabled_for_web? + end + + def password_based_login? + user_params[:login].present? || user_params[:password].present? + end + def user_params params.require(:user).permit(:login, :password, :remember_me, :otp_attempt, :device_response) end diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index e635f60823..08d8bba6a9 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -127,6 +127,7 @@ module ApplicationSettingsHelper :akismet_api_key, :akismet_enabled, :allow_local_requests_from_hooks_and_services, + :dns_rebinding_protection_enabled, :archive_builds_in_human_readable, :authorized_keys_enabled, :auto_devops_enabled, diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 5318ab4dde..c08b8d41ba 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -93,4 +93,8 @@ module NotificationsHelper s_(event.to_s.humanize) end end + + def show_unsubscribe_title?(noteable) + can?(current_user, "read_#{noteable.to_ability_name}".to_sym, noteable) + end end diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb new file mode 100644 index 0000000000..ca99b1694d --- /dev/null +++ b/app/models/application_setting_implementation.rb @@ -0,0 +1,283 @@ +# frozen_string_literal: true + +module ApplicationSettingImplementation + extend ActiveSupport::Concern + + DOMAIN_LIST_SEPARATOR = %r{\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace + | # or + \s # any whitespace character + | # or + [\r\n] # any number of newline characters + }x + + # Setting a key restriction to `-1` means that all keys of this type are + # forbidden. + FORBIDDEN_KEY_VALUE = KeyRestrictionValidator::FORBIDDEN + SUPPORTED_KEY_TYPES = %i[rsa dsa ecdsa ed25519].freeze + + class_methods do + def defaults + { + after_sign_up_text: nil, + akismet_enabled: false, + allow_local_requests_from_hooks_and_services: false, + dns_rebinding_protection_enabled: true, + authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand + container_registry_token_expire_delay: 5, + default_artifacts_expire_in: '30 days', + default_branch_protection: Settings.gitlab['default_branch_protection'], + default_group_visibility: Settings.gitlab.default_projects_features['visibility_level'], + default_project_creation: Settings.gitlab['default_project_creation'], + default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], + default_projects_limit: Settings.gitlab['default_projects_limit'], + default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], + disabled_oauth_sign_in_sources: [], + domain_whitelist: Settings.gitlab['domain_whitelist'], + dsa_key_restriction: 0, + ecdsa_key_restriction: 0, + ed25519_key_restriction: 0, + first_day_of_week: 0, + gitaly_timeout_default: 55, + gitaly_timeout_fast: 10, + gitaly_timeout_medium: 30, + gravatar_enabled: Settings.gravatar['enabled'], + help_page_hide_commercial_content: false, + help_page_text: nil, + hide_third_party_offers: false, + housekeeping_bitmaps_enabled: true, + housekeeping_enabled: true, + housekeeping_full_repack_period: 50, + housekeeping_gc_period: 200, + housekeeping_incremental_repack_period: 10, + import_sources: Settings.gitlab['import_sources'], + max_artifacts_size: Settings.artifacts['max_size'], + max_attachment_size: Settings.gitlab['max_attachment_size'], + mirror_available: true, + password_authentication_enabled_for_git: true, + password_authentication_enabled_for_web: Settings.gitlab['signin_enabled'], + performance_bar_allowed_group_id: nil, + rsa_key_restriction: 0, + plantuml_enabled: false, + plantuml_url: nil, + polling_interval_multiplier: 1, + project_export_enabled: true, + recaptcha_enabled: false, + repository_checks_enabled: true, + repository_storages: ['default'], + require_two_factor_authentication: false, + restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'], + session_expire_delay: Settings.gitlab['session_expire_delay'], + send_user_confirmation_email: false, + shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'], + shared_runners_text: nil, + sign_in_text: nil, + signup_enabled: Settings.gitlab['signup_enabled'], + terminal_max_session_time: 0, + throttle_authenticated_api_enabled: false, + throttle_authenticated_api_period_in_seconds: 3600, + throttle_authenticated_api_requests_per_period: 7200, + throttle_authenticated_web_enabled: false, + throttle_authenticated_web_period_in_seconds: 3600, + throttle_authenticated_web_requests_per_period: 7200, + throttle_unauthenticated_enabled: false, + throttle_unauthenticated_period_in_seconds: 3600, + throttle_unauthenticated_requests_per_period: 3600, + two_factor_grace_period: 48, + unique_ips_limit_enabled: false, + unique_ips_limit_per_user: 10, + unique_ips_limit_time_window: 3600, + usage_ping_enabled: Settings.gitlab['usage_ping_enabled'], + instance_statistics_visibility_private: false, + user_default_external: false, + user_default_internal_regex: nil, + user_show_add_ssh_key_message: true, + usage_stats_set_by_user_id: nil, + diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES, + commit_email_hostname: default_commit_email_hostname, + protected_ci_variables: false, + local_markdown_version: 0 + } + end + + def default_commit_email_hostname + "users.noreply.#{Gitlab.config.gitlab.host}" + end + + def create_from_defaults + build_from_defaults.tap(&:save) + end + + def human_attribute_name(attr, _options = {}) + if attr == :default_artifacts_expire_in + 'Default artifacts expiration' + else + super + end + end + end + + def home_page_url_column_exists? + ::Gitlab::Database.cached_column_exists?(:application_settings, :home_page_url) + end + + def help_page_support_url_column_exists? + ::Gitlab::Database.cached_column_exists?(:application_settings, :help_page_support_url) + end + + def disabled_oauth_sign_in_sources=(sources) + sources = (sources || []).map(&:to_s) & Devise.omniauth_providers.map(&:to_s) + super(sources) + end + + def domain_whitelist_raw + self.domain_whitelist&.join("\n") + end + + def domain_blacklist_raw + self.domain_blacklist&.join("\n") + end + + def domain_whitelist_raw=(values) + self.domain_whitelist = [] + self.domain_whitelist = values.split(DOMAIN_LIST_SEPARATOR) + self.domain_whitelist.reject! { |d| d.empty? } + self.domain_whitelist + end + + def domain_blacklist_raw=(values) + self.domain_blacklist = [] + self.domain_blacklist = values.split(DOMAIN_LIST_SEPARATOR) + self.domain_blacklist.reject! { |d| d.empty? } + self.domain_blacklist + end + + def domain_blacklist_file=(file) + self.domain_blacklist_raw = file.read + end + + def repository_storages + Array(read_attribute(:repository_storages)) + end + + def commit_email_hostname + super.presence || self.class.default_commit_email_hostname + end + + def default_project_visibility=(level) + super(Gitlab::VisibilityLevel.level_value(level)) + end + + def default_snippet_visibility=(level) + super(Gitlab::VisibilityLevel.level_value(level)) + end + + def default_group_visibility=(level) + super(Gitlab::VisibilityLevel.level_value(level)) + end + + def restricted_visibility_levels=(levels) + super(levels&.map { |level| Gitlab::VisibilityLevel.level_value(level) }) + end + + def strip_sentry_values + sentry_dsn.strip! if sentry_dsn.present? + clientside_sentry_dsn.strip! if clientside_sentry_dsn.present? + end + + def performance_bar_allowed_group + Group.find_by_id(performance_bar_allowed_group_id) + end + + # Return true if the Performance Bar is enabled for a given group + def performance_bar_enabled + performance_bar_allowed_group_id.present? + end + + # Choose one of the available repository storage options. Currently all have + # equal weighting. + def pick_repository_storage + repository_storages.sample + end + + def runners_registration_token + ensure_runners_registration_token! + end + + def health_check_access_token + ensure_health_check_access_token! + end + + def usage_ping_can_be_configured? + Settings.gitlab.usage_ping_enabled + end + + def usage_ping_enabled + usage_ping_can_be_configured? && super + end + + def allowed_key_types + SUPPORTED_KEY_TYPES.select do |type| + key_restriction_for(type) != FORBIDDEN_KEY_VALUE + end + end + + def key_restriction_for(type) + attr_name = "#{type}_key_restriction" + + has_attribute?(attr_name) ? public_send(attr_name) : FORBIDDEN_KEY_VALUE # rubocop:disable GitlabSecurity/PublicSend + end + + def allow_signup? + signup_enabled? && password_authentication_enabled_for_web? + end + + def password_authentication_enabled? + password_authentication_enabled_for_web? || password_authentication_enabled_for_git? + end + + def user_default_internal_regex_enabled? + user_default_external? && user_default_internal_regex.present? + end + + def user_default_internal_regex_instance + Regexp.new(user_default_internal_regex, Regexp::IGNORECASE) + end + + delegate :terms, to: :latest_terms, allow_nil: true + def latest_terms + @latest_terms ||= ApplicationSetting::Term.latest + end + + def reset_memoized_terms + @latest_terms = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables + latest_terms + end + + def archive_builds_older_than + archive_builds_in_seconds.seconds.ago if archive_builds_in_seconds + end + + private + + def ensure_uuid! + return if uuid? + + self.uuid = SecureRandom.uuid + end + + def check_repository_storages + invalid = repository_storages - Gitlab.config.repositories.storages.keys + errors.add(:repository_storages, "can't include: #{invalid.join(", ")}") unless + invalid.empty? + end + + def terms_exist + return unless enforce_terms? + + errors.add(:terms, "You need to set terms to be enforced") unless terms.present? + end + + def expire_performance_bar_allowed_user_ids_cache + Gitlab::PerformanceBar.expire_allowed_user_ids_cache + end +end diff --git a/app/models/clusters/applications/knative.rb b/app/models/clusters/applications/knative.rb index 8d79b041b6..cd8af07b50 100644 --- a/app/models/clusters/applications/knative.rb +++ b/app/models/clusters/applications/knative.rb @@ -2,8 +2,8 @@ module Clusters module Applications - class Knative < ActiveRecord::Base - VERSION = '0.2.2'.freeze + class Knative < ApplicationRecord + VERSION = '0.5.0'.freeze REPOSITORY = 'https://storage.googleapis.com/triggermesh-charts'.freeze METRICS_CONFIG = 'https://storage.googleapis.com/triggermesh-charts/istio-metrics.yaml'.freeze FETCH_IP_ADDRESS_DELAY = 30.seconds diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 0e49fd3c62..f426b7b9e6 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -590,6 +590,8 @@ class MergeRequest < ActiveRecord::Base return end + [:source_branch, :target_branch].each { |attr| validate_branch_name(attr) } + if opened? similar_mrs = target_project .merge_requests @@ -610,6 +612,16 @@ class MergeRequest < ActiveRecord::Base end end + def validate_branch_name(attr) + return unless changes_include?(attr) + + branch = read_attribute(attr) + + return unless branch + + errors.add(attr) unless Gitlab::GitRefValidator.validate_merge_request_branch(branch) + end + def validate_target_project return true if target_project.merge_requests_enabled? diff --git a/app/models/project.rb b/app/models/project.rb index daa018d6f8..e6429ae966 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -333,8 +333,8 @@ class Project < ActiveRecord::Base validates :star_count, numericality: { greater_than_or_equal_to: 0 } validate :check_personal_projects_limit, on: :create validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? } - validate :visibility_level_allowed_by_group, if: -> { changes.has_key?(:visibility_level) } - validate :visibility_level_allowed_as_fork, if: -> { changes.has_key?(:visibility_level) } + validate :visibility_level_allowed_by_group, if: :should_validate_visibility_level? + validate :visibility_level_allowed_as_fork, if: :should_validate_visibility_level? validate :check_wiki_path_conflict validate :validate_pages_https_only, if: -> { changes.has_key?(:pages_https_only) } validates :repository_storage, @@ -402,6 +402,7 @@ class Project < ActiveRecord::Base scope :with_builds_enabled, -> { with_feature_enabled(:builds) } scope :with_issues_enabled, -> { with_feature_enabled(:issues) } scope :with_issues_available_for_user, ->(current_user) { with_feature_available_for_user(:issues, current_user) } + scope :with_merge_requests_available_for_user, ->(current_user) { with_feature_available_for_user(:merge_requests, current_user) } scope :with_merge_requests_enabled, -> { with_feature_enabled(:merge_requests) } scope :with_remote_mirrors, -> { joins(:remote_mirrors).where(remote_mirrors: { enabled: true }).distinct } @@ -586,6 +587,17 @@ class Project < ActiveRecord::Base def group_ids joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id) end + + # Returns ids of projects with milestones available for given user + # + # Used on queries to find milestones which user can see + # For example: Milestone.where(project_id: ids_with_milestone_available_for(user)) + def ids_with_milestone_available_for(user) + with_issues_enabled = with_issues_available_for_user(user).select(:id) + with_merge_requests_enabled = with_merge_requests_available_for_user(user).select(:id) + + from_union([with_issues_enabled, with_merge_requests_enabled]).select(:id) + end end def all_pipelines @@ -856,6 +868,10 @@ class Project < ActiveRecord::Base self.errors.add(:limit_reached, error % { limit: limit }) end + def should_validate_visibility_level? + new_record? || changes.has_key?(:visibility_level) + end + def visibility_level_allowed_by_group return if visibility_level_allowed_by_group? diff --git a/app/views/admin/application_settings/_outbound.html.haml b/app/views/admin/application_settings/_outbound.html.haml index f4bfb5af38..dd56bb99a0 100644 --- a/app/views/admin/application_settings/_outbound.html.haml +++ b/app/views/admin/application_settings/_outbound.html.haml @@ -8,4 +8,12 @@ = f.label :allow_local_requests_from_hooks_and_services, class: 'form-check-label' do Allow requests to the local network from hooks and services + .form-group + .form-check + = f.check_box :dns_rebinding_protection_enabled, class: 'form-check-input' + = f.label :dns_rebinding_protection_enabled, class: 'form-check-label' do + = _('Enforce DNS rebinding attack protection') + %span.form-text.text-muted + = _('Resolves IP addresses once and uses them to submit requests') + = f.submit 'Save changes', class: "btn btn-success" diff --git a/app/views/sent_notifications/unsubscribe.html.haml b/app/views/sent_notifications/unsubscribe.html.haml index ca392e1adf..22fcfcda29 100644 --- a/app/views/sent_notifications/unsubscribe.html.haml +++ b/app/views/sent_notifications/unsubscribe.html.haml @@ -1,6 +1,6 @@ - noteable = @sent_notification.noteable - noteable_type = @sent_notification.noteable_type.titleize.downcase -- noteable_text = %(#{noteable.title} (#{noteable.to_reference})) +- noteable_text = show_unsubscribe_title?(noteable) ? %(#{noteable.title} (#{noteable.to_reference})) : %(#{noteable.to_reference}) - page_title _("Unsubscribe"), noteable_text, noteable_type.pluralize, @sent_notification.project.full_name %h3.page-title diff --git a/config/initializers/hipchat_client_patch.rb b/config/initializers/hipchat_client_patch.rb index aec265312b..f83e8171d6 100644 --- a/config/initializers/hipchat_client_patch.rb +++ b/config/initializers/hipchat_client_patch.rb @@ -1,14 +1,14 @@ # This monkey patches the HTTParty used in https://github.com/hipchat/hipchat-rb. module HipChat class Client - connection_adapter ::Gitlab::ProxyHTTPConnectionAdapter + connection_adapter ::Gitlab::HTTPConnectionAdapter end class Room - connection_adapter ::Gitlab::ProxyHTTPConnectionAdapter + connection_adapter ::Gitlab::HTTPConnectionAdapter end class User - connection_adapter ::Gitlab::ProxyHTTPConnectionAdapter + connection_adapter ::Gitlab::HTTPConnectionAdapter end end diff --git a/config/initializers/http_hostname_override.rb b/config/initializers/http_hostname_override.rb new file mode 100644 index 0000000000..58dd380326 --- /dev/null +++ b/config/initializers/http_hostname_override.rb @@ -0,0 +1,49 @@ +# This override allows passing `@hostname_override` to the SNI protocol, +# which is used to lookup the correct SSL certificate in the +# request handshake process. +# +# Given we've forced the HTTP request to be sent to the resolved +# IP address in a few scenarios (e.g.: `Gitlab::HTTP` through +# `Gitlab::UrlBlocker.validate!`), we need to provide the _original_ +# hostname via SNI in order to have a clean connection setup. +# +# This is ultimately needed in order to avoid DNS rebinding attacks +# through HTTP requests. +# +class OpenSSL::SSL::SSLContext + attr_accessor :hostname_override +end + +class OpenSSL::SSL::SSLSocket + module HostnameOverride + # rubocop: disable Gitlab/ModuleWithInstanceVariables + def hostname=(hostname) + super(@context.hostname_override || hostname) + end + + def post_connection_check(hostname) + super(@context.hostname_override || hostname) + end + # rubocop: enable Gitlab/ModuleWithInstanceVariables + end + + prepend HostnameOverride +end + +class Net::HTTP + attr_accessor :hostname_override + SSL_IVNAMES << :@hostname_override + SSL_ATTRIBUTES << :hostname_override + + module HostnameOverride + def addr_port + return super unless hostname_override + + addr = hostname_override + default_port = use_ssl? ? Net::HTTP.https_default_port : Net::HTTP.http_default_port + default_port == port ? addr : "#{addr}:#{port}" + end + end + + prepend HostnameOverride +end diff --git a/config/prometheus/common_metrics.yml b/config/prometheus/common_metrics.yml index 9bdaf1575e..356f573c5e 100644 --- a/config/prometheus/common_metrics.yml +++ b/config/prometheus/common_metrics.yml @@ -259,3 +259,13 @@ label: Pod average unit: "cores" track: canary + - title: "Knative function invocations" + y_label: "Invocations" + required_metrics: + - istio_revision_request_count + weight: 1 + queries: + - id: system_metrics_knative_function_invocation_count + query_range: 'floor(sum(rate(istio_revision_request_count{destination_configuration="%{function_name}", destination_namespace="%{kube_namespace}"}[1m])/3))' + label: invocations / minute + unit: requests diff --git a/db/migrate/20190408163745_prometheus_knative05_fix.rb b/db/migrate/20190408163745_prometheus_knative05_fix.rb new file mode 100644 index 0000000000..c11f6f0e29 --- /dev/null +++ b/db/migrate/20190408163745_prometheus_knative05_fix.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class PrometheusKnative05Fix < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + require Rails.root.join('db/importers/common_metrics_importer.rb') + + DOWNTIME = false + + def up + Importers::CommonMetricsImporter.new.execute + end + + def down + # no-op + end +end diff --git a/db/migrate/20190529142545_add_dns_rebinding_protection_enabled_to_application_settings.rb b/db/migrate/20190529142545_add_dns_rebinding_protection_enabled_to_application_settings.rb new file mode 100644 index 0000000000..eeb4b69b60 --- /dev/null +++ b/db/migrate/20190529142545_add_dns_rebinding_protection_enabled_to_application_settings.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddDnsRebindingProtectionEnabledToApplicationSettings < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:application_settings, :dns_rebinding_protection_enabled, + :boolean, + default: true, + allow_null: false) + end + + def down + remove_column(:application_settings, :dns_rebinding_protection_enabled) + end +end diff --git a/db/schema.rb b/db/schema.rb index 023eee5f33..20574b8306 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20190204115450) do +ActiveRecord::Schema.define(version: 20190529142545) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -170,6 +170,17 @@ ActiveRecord::Schema.define(version: 20190204115450) do t.string "runners_registration_token_encrypted" t.integer "local_markdown_version", default: 0, null: false t.integer "first_day_of_week", default: 0, null: false + t.integer "default_project_creation", default: 2, null: false + t.boolean "external_authorization_service_enabled", default: false, null: false + t.string "external_authorization_service_url" + t.string "external_authorization_service_default_label" + t.float "external_authorization_service_timeout", default: 0.5 + t.text "external_auth_client_cert" + t.text "encrypted_external_auth_client_key" + t.string "encrypted_external_auth_client_key_iv" + t.string "encrypted_external_auth_client_key_pass" + t.string "encrypted_external_auth_client_key_pass_iv" + t.boolean "dns_rebinding_protection_enabled", default: true, null: false t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree end diff --git a/lib/banzai/filter/wiki_link_filter/rewriter.rb b/lib/banzai/filter/wiki_link_filter/rewriter.rb index f4cc8beeb5..77b5053f38 100644 --- a/lib/banzai/filter/wiki_link_filter/rewriter.rb +++ b/lib/banzai/filter/wiki_link_filter/rewriter.rb @@ -4,6 +4,8 @@ module Banzai module Filter class WikiLinkFilter < HTML::Pipeline::Filter class Rewriter + UNSAFE_SLUG_REGEXES = [/\Ajavascript:/i].freeze + def initialize(link_string, wiki:, slug:) @uri = Addressable::URI.parse(link_string) @wiki_base_path = wiki && wiki.wiki_base_path @@ -35,6 +37,8 @@ module Banzai # Of the form `./link`, `../link`, or similar def apply_hierarchical_link_rules! + return if slug_considered_unsafe? + @uri = Addressable::URI.join(@slug, @uri) if @uri.to_s[0] == '.' end @@ -54,6 +58,10 @@ module Banzai def repository_upload? @uri.relative? && @uri.path.starts_with?(Wikis::CreateAttachmentService::ATTACHMENT_PATH) end + + def slug_considered_unsafe? + UNSAFE_SLUG_REGEXES.any? { |r| r.match?(@slug) } + end end end end diff --git a/lib/banzai/redactor.rb b/lib/banzai/redactor.rb index 7db5f5e1f7..c2da7fec7c 100644 --- a/lib/banzai/redactor.rb +++ b/lib/banzai/redactor.rb @@ -70,8 +70,11 @@ module Banzai # Build the raw tag just with a link as href and content if # it's originally a link pattern. We shouldn't return a plain text href. original_link = - if link_reference == 'true' && href = original_content - %(#{href}) + if link_reference == 'true' + href = node.attr('href') + content = original_content + + %(#{content}) end # The reference should be replaced by the original link's content, diff --git a/lib/gitlab.rb b/lib/gitlab.rb index e073450283..7a1e4c4dca 100644 --- a/lib/gitlab.rb +++ b/lib/gitlab.rb @@ -40,6 +40,7 @@ module Gitlab SUBDOMAIN_REGEX = %r{\Ahttps://[a-z0-9]+\.gitlab\.com\z} VERSION = File.read(root.join("VERSION")).strip.freeze INSTALLATION_TYPE = File.read(root.join("INSTALLATION_TYPE")).strip.freeze + HTTP_PROXY_ENV_VARS = %w(http_proxy https_proxy HTTP_PROXY HTTPS_PROXY).freeze def self.com? # Check `gl_subdomain?` as well to keep parity with gitlab.com @@ -58,6 +59,14 @@ module Gitlab Rails.env.development? || org? || com? end + def self.ee? + Object.const_defined?(:License) + end + + def self.http_proxy_env? + HTTP_PROXY_ENV_VARS.any? { |name| ENV[name] } + end + def self.process_name return 'sidekiq' if Sidekiq.server? return 'console' if defined?(Rails::Console) diff --git a/lib/gitlab/git_ref_validator.rb b/lib/gitlab/git_ref_validator.rb index 3f13ebeb9d..dfff682368 100644 --- a/lib/gitlab/git_ref_validator.rb +++ b/lib/gitlab/git_ref_validator.rb @@ -5,12 +5,15 @@ module Gitlab module GitRefValidator extend self + + EXPANDED_PREFIXES = %w[refs/heads/ refs/remotes/].freeze + DISALLOWED_PREFIXES = %w[-].freeze + # Validates a given name against the git reference specification # # Returns true for a valid reference name, false otherwise def validate(ref_name) - not_allowed_prefixes = %w(refs/heads/ refs/remotes/ -) - return false if ref_name.start_with?(*not_allowed_prefixes) + return false if ref_name.start_with?(*(EXPANDED_PREFIXES + DISALLOWED_PREFIXES)) return false if ref_name == 'HEAD' begin @@ -19,5 +22,21 @@ module Gitlab return false end end + + def validate_merge_request_branch(ref_name) + return false if ref_name.start_with?(*DISALLOWED_PREFIXES) + + expanded_name = if ref_name.start_with?(*EXPANDED_PREFIXES) + ref_name + else + "refs/heads/#{ref_name}" + end + + begin + Rugged::Reference.valid_name?(expanded_name) + rescue ArgumentError + return false + end + end end end diff --git a/lib/gitlab/http.rb b/lib/gitlab/http.rb index bcd9e2be35..55ed7e7e83 100644 --- a/lib/gitlab/http.rb +++ b/lib/gitlab/http.rb @@ -11,7 +11,7 @@ module Gitlab include HTTParty # rubocop:disable Gitlab/HTTParty - connection_adapter ProxyHTTPConnectionAdapter + connection_adapter HTTPConnectionAdapter def self.perform_request(http_method, path, options, &block) super diff --git a/lib/gitlab/proxy_http_connection_adapter.rb b/lib/gitlab/http_connection_adapter.rb similarity index 52% rename from lib/gitlab/proxy_http_connection_adapter.rb rename to lib/gitlab/http_connection_adapter.rb index a64cb47e77..41eab3658b 100644 --- a/lib/gitlab/proxy_http_connection_adapter.rb +++ b/lib/gitlab/http_connection_adapter.rb @@ -10,17 +10,19 @@ # # This option will take precedence over the global setting. module Gitlab - class ProxyHTTPConnectionAdapter < HTTParty::ConnectionAdapter + class HTTPConnectionAdapter < HTTParty::ConnectionAdapter def connection - unless allow_local_requests? - begin - Gitlab::UrlBlocker.validate!(uri, allow_local_network: false) - rescue Gitlab::UrlBlocker::BlockedUrlError => e - raise Gitlab::HTTP::BlockedUrlError, "URL '#{uri}' is blocked: #{e.message}" - end + begin + @uri, hostname = Gitlab::UrlBlocker.validate!(uri, allow_local_network: allow_local_requests?, + allow_localhost: allow_local_requests?, + dns_rebind_protection: dns_rebind_protection?) + rescue Gitlab::UrlBlocker::BlockedUrlError => e + raise Gitlab::HTTP::BlockedUrlError, "URL '#{uri}' is blocked: #{e.message}" end - super + super.tap do |http| + http.hostname_override = hostname if hostname + end end private @@ -29,6 +31,12 @@ module Gitlab options.fetch(:allow_local_requests, allow_settings_local_requests?) end + def dns_rebind_protection? + return false if Gitlab.http_proxy_env? + + Gitlab::CurrentSettings.dns_rebinding_protection_enabled? + end + def allow_settings_local_requests? Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services? end diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb index 93b37b7bc5..c28a167401 100644 --- a/lib/gitlab/import_export/attribute_cleaner.rb +++ b/lib/gitlab/import_export/attribute_cleaner.rb @@ -4,6 +4,7 @@ module Gitlab module ImportExport class AttributeCleaner ALLOWED_REFERENCES = RelationFactory::PROJECT_REFERENCES + RelationFactory::USER_REFERENCES + ['group_id'] + PROHIBITED_REFERENCES = Regexp.union(/\Acached_markdown_version\Z/, /_id\Z/, /_html\Z/).freeze def self.clean(*args) new(*args).clean @@ -24,7 +25,11 @@ module Gitlab private def prohibited_key?(key) - key.end_with?('_id') && !ALLOWED_REFERENCES.include?(key) + key =~ PROHIBITED_REFERENCES && !permitted_key?(key) + end + + def permitted_key?(key) + ALLOWED_REFERENCES.include?(key) end def excluded_key?(key) diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index a68f8801c2..18f339973b 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -132,6 +132,12 @@ module Gitlab project end + def filter_milestones_by_project(milestones) + return Milestone.none unless Ability.allowed?(@current_user, :read_milestone, @project) + + milestones.where(project_id: project.id) # rubocop: disable CodeReuse/ActiveRecord + end + def repository_project_ref @repository_project_ref ||= repository_ref || project.default_branch end diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb index 491148ec1a..882279b6b4 100644 --- a/lib/gitlab/search_results.rb +++ b/lib/gitlab/search_results.rb @@ -105,8 +105,10 @@ module Gitlab # rubocop: disable CodeReuse/ActiveRecord def milestones - milestones = Milestone.where(project_id: project_ids_relation) - milestones = milestones.search(query) + milestones = Milestone.search(query) + + milestones = filter_milestones_by_project(milestones) + milestones.reorder('updated_at DESC') end # rubocop: enable CodeReuse/ActiveRecord @@ -133,6 +135,26 @@ module Gitlab 'projects' end + # Filter milestones by authorized projects. + # For performance reasons project_id is being plucked + # to be used on a smaller query. + # + # rubocop: disable CodeReuse/ActiveRecord + def filter_milestones_by_project(milestones) + project_ids = + milestones.where(project_id: project_ids_relation) + .select(:project_id).distinct + .pluck(:project_id) + + return Milestone.none if project_ids.nil? + + authorized_project_ids_relation = + Project.where(id: project_ids).ids_with_milestone_available_for(current_user) + + milestones.where(project_id: authorized_project_ids_relation) + end + # rubocop: enable CodeReuse/ActiveRecord + # rubocop: disable CodeReuse/ActiveRecord def project_ids_relation limit_projects.select(:id).reorder(nil) diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb index 9b7b0db952..68cfd9fae8 100644 --- a/lib/gitlab/url_blocker.rb +++ b/lib/gitlab/url_blocker.rb @@ -8,38 +8,68 @@ module Gitlab BlockedUrlError = Class.new(StandardError) class << self - def validate!(url, ports: [], protocols: [], allow_localhost: false, allow_local_network: true, ascii_only: false, enforce_user: false, enforce_sanitization: false) - return true if url.nil? + # Validates the given url according to the constraints specified by arguments. + # + # ports - Raises error if the given URL port does is not between given ports. + # allow_localhost - Raises error if URL resolves to a localhost IP address and argument is true. + # allow_local_network - Raises error if URL resolves to a link-local address and argument is true. + # ascii_only - Raises error if URL has unicode characters and argument is true. + # enforce_user - Raises error if URL user doesn't start with alphanumeric characters and argument is true. + # enforce_sanitization - Raises error if URL includes any HTML/CSS/JS tags and argument is true. + # + # Returns an array with [, ]. + # rubocop:disable Metrics/CyclomaticComplexity + # rubocop:disable Metrics/ParameterLists + def validate!( + url, + ports: [], + protocols: [], + allow_localhost: false, + allow_local_network: true, + ascii_only: false, + enforce_user: false, + enforce_sanitization: false, + dns_rebind_protection: true) + # rubocop:enable Metrics/CyclomaticComplexity + # rubocop:enable Metrics/ParameterLists + + return [nil, nil] if url.nil? # Param url can be a string, URI or Addressable::URI uri = parse_url(url) validate_html_tags!(uri) if enforce_sanitization - # Allow imports from the GitLab instance itself but only from the configured ports - return true if internal?(uri) - + hostname = uri.hostname port = get_port(uri) - validate_protocol!(uri.scheme, protocols) - validate_port!(port, ports) if ports.any? - validate_user!(uri.user) if enforce_user - validate_hostname!(uri.hostname) - validate_unicode_restriction!(uri) if ascii_only + + unless internal?(uri) + validate_protocol!(uri.scheme, protocols) + validate_port!(port, ports) if ports.any? + validate_user!(uri.user) if enforce_user + validate_hostname!(hostname) + validate_unicode_restriction!(uri) if ascii_only + end begin - addrs_info = Addrinfo.getaddrinfo(uri.hostname, port, nil, :STREAM).map do |addr| + addrs_info = Addrinfo.getaddrinfo(hostname, port, nil, :STREAM).map do |addr| addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr end rescue SocketError - return true + return [uri, nil] end + protected_uri_with_hostname = enforce_uri_hostname(addrs_info, uri, hostname, dns_rebind_protection) + + # Allow url from the GitLab instance itself but only for the configured hostname and ports + return protected_uri_with_hostname if internal?(uri) + validate_localhost!(addrs_info) unless allow_localhost validate_loopback!(addrs_info) unless allow_localhost validate_local_network!(addrs_info) unless allow_local_network validate_link_local!(addrs_info) unless allow_local_network - true + protected_uri_with_hostname end def blocked_url?(*args) @@ -52,6 +82,25 @@ module Gitlab private + # Returns the given URI with IP address as hostname and the original hostname respectively + # in an Array. + # + # It checks whether the resolved IP address matches with the hostname. If not, it changes + # the hostname to the resolved IP address. + # + # The original hostname is used to validate the SSL, given in that scenario + # we'll be making the request to the IP address, instead of using the hostname. + def enforce_uri_hostname(addrs_info, uri, hostname, dns_rebind_protection) + address = addrs_info.first + ip_address = address&.ip_address + + return [uri, nil] unless dns_rebind_protection && ip_address && ip_address != hostname + + uri = uri.dup + uri.hostname = ip_address + [uri, hostname] + end + def get_port(uri) uri.port || uri.default_port end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 3a50ed4f78..f3f8685d6b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2959,6 +2959,12 @@ msgstr "" msgid "Ends at (UTC)" msgstr "" +msgid "Enforce DNS rebinding attack protection" +msgstr "" + +msgid "Enter at least three characters to search" +msgstr "" + msgid "Enter in your Bitbucket Server URL and personal access token below" msgstr "" @@ -6216,6 +6222,15 @@ msgstr "" msgid "Resolved" msgstr "" +msgid "Resolved 1 discussion." +msgstr "" + +msgid "Resolved all discussions." +msgstr "" + +msgid "Resolves IP addresses once and uses them to submit requests" +msgstr "" + msgid "Response metrics (AWS ELB)" msgstr "" diff --git a/spec/controllers/projects/ci/lints_controller_spec.rb b/spec/controllers/projects/ci/lints_controller_spec.rb index cfa010c2d1..57708740d0 100644 --- a/spec/controllers/projects/ci/lints_controller_spec.rb +++ b/spec/controllers/projects/ci/lints_controller_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Projects::Ci::LintsController do + include StubRequests + let(:project) { create(:project, :repository) } let(:user) { create(:user) } @@ -68,7 +70,7 @@ describe Projects::Ci::LintsController do context 'with a valid gitlab-ci.yml' do before do - WebMock.stub_request(:get, remote_file_path).to_return(body: remote_file_content) + stub_full_request(remote_file_path).to_return(body: remote_file_content) project.add_developer(user) post :create, params: { namespace_id: project.namespace, project_id: project, content: content } diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index ac54b3c395..77120b0fc0 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -173,6 +173,40 @@ describe Projects::MilestonesController do end end + describe '#labels' do + render_views + + context 'as json' do + let!(:guest) { create(:user, username: 'guest1') } + let!(:group) { create(:group, :public) } + let!(:project) { create(:project, :public, group: group) } + let!(:label) { create(:label, title: 'test_label_on_private_issue', project: project) } + let!(:confidential_issue) { create(:labeled_issue, confidential: true, project: project, milestone: milestone, labels: [label]) } + + it 'does not render labels of private issues if user has no access' do + sign_in(guest) + + get :labels, params: { namespace_id: group.id, project_id: project.id, id: milestone.iid }, format: :json + + expect(response).to have_gitlab_http_status(200) + expect(response.content_type).to eq 'application/json' + + expect(json_response['html']).not_to include(label.title) + end + + it 'does render labels of private issues if user has access' do + sign_in(user) + + get :labels, params: { namespace_id: group.id, project_id: project.id, id: milestone.iid }, format: :json + + expect(response).to have_gitlab_http_status(200) + expect(response.content_type).to eq 'application/json' + + expect(json_response['html']).to include(label.title) + end + end + end + context 'promotion succeeds' do before do group.add_developer(user) diff --git a/spec/controllers/sent_notifications_controller_spec.rb b/spec/controllers/sent_notifications_controller_spec.rb index 75c91dd860..89857a9d21 100644 --- a/spec/controllers/sent_notifications_controller_spec.rb +++ b/spec/controllers/sent_notifications_controller_spec.rb @@ -1,16 +1,34 @@ +# frozen_string_literal: true + require 'rails_helper' describe SentNotificationsController do let(:user) { create(:user) } - let(:project) { create(:project) } - let(:sent_notification) { create(:sent_notification, project: project, noteable: issue, recipient: user) } + let(:project) { create(:project, :public) } + let(:private_project) { create(:project, :private) } + let(:sent_notification) { create(:sent_notification, project: target_project, noteable: noteable, recipient: user) } let(:issue) do - create(:issue, project: project, author: user) do |issue| - issue.subscriptions.create(user: user, project: project, subscribed: true) + create(:issue, project: target_project) do |issue| + issue.subscriptions.create(user: user, project: target_project, subscribed: true) end end + let(:confidential_issue) do + create(:issue, project: target_project, confidential: true) do |issue| + issue.subscriptions.create(user: user, project: target_project, subscribed: true) + end + end + + let(:merge_request) do + create(:merge_request, source_project: target_project, target_project: target_project) do |mr| + mr.subscriptions.create(user: user, project: target_project, subscribed: true) + end + end + + let(:noteable) { issue } + let(:target_project) { project } + describe 'GET unsubscribe' do context 'when the user is not logged in' do context 'when the force param is passed' do @@ -32,20 +50,93 @@ describe SentNotificationsController do end context 'when the force param is not passed' do + render_views + before do get(:unsubscribe, params: { id: sent_notification.reply_key }) end - it 'does not unsubscribe the user' do - expect(issue.subscribed?(user, project)).to be_truthy + shared_examples 'unsubscribing as anonymous' do + it 'does not unsubscribe the user' do + expect(noteable.subscribed?(user, target_project)).to be_truthy + end + + it 'does not set the flash message' do + expect(controller).not_to set_flash[:notice] + end + + it 'renders unsubscribe page' do + expect(response.status).to eq(200) + expect(response).to render_template :unsubscribe + end end - it 'does not set the flash message' do - expect(controller).not_to set_flash[:notice] + context 'when project is public' do + context 'when unsubscribing from issue' do + let(:noteable) { issue } + + it 'shows issue title' do + expect(response.body).to include(issue.title) + end + + it_behaves_like 'unsubscribing as anonymous' + end + + context 'when unsubscribing from confidential issue' do + let(:noteable) { confidential_issue } + + it 'does not show issue title' do + expect(response.body).not_to include(confidential_issue.title) + expect(response.body).to include(confidential_issue.to_reference) + end + + it_behaves_like 'unsubscribing as anonymous' + end + + context 'when unsubscribing from merge request' do + let(:noteable) { merge_request } + + it 'shows merge request title' do + expect(response.body).to include(merge_request.title) + end + + it_behaves_like 'unsubscribing as anonymous' + end end - it 'redirects to the login page' do - expect(response).to render_template :unsubscribe + context 'when project is not public' do + let(:target_project) { private_project } + + context 'when unsubscribing from issue' do + let(:noteable) { issue } + + it 'shows issue title' do + expect(response.body).not_to include(issue.title) + end + + it_behaves_like 'unsubscribing as anonymous' + end + + context 'when unsubscribing from confidential issue' do + let(:noteable) { confidential_issue } + + it 'does not show issue title' do + expect(response.body).not_to include(confidential_issue.title) + expect(response.body).to include(confidential_issue.to_reference) + end + + it_behaves_like 'unsubscribing as anonymous' + end + + context 'when unsubscribing from merge request' do + let(:noteable) { merge_request } + + it 'shows merge request title' do + expect(response.body).not_to include(merge_request.title) + end + + it_behaves_like 'unsubscribing as anonymous' + end end end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index ea7242c1aa..661c6d7959 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -56,7 +56,26 @@ describe SessionsController do it 'authenticates user correctly' do post(:create, params: { user: user_params }) - expect(subject.current_user). to eq user + expect(subject.current_user).to eq user + end + + context 'with password authentication disabled' do + before do + stub_application_setting(password_authentication_enabled_for_web: false) + end + + it 'does not sign in the user' do + post(:create, params: { user: user_params }) + + expect(@request.env['warden']).not_to be_authenticated + expect(subject.current_user).to be_nil + end + + it 'returns status 403' do + post(:create, params: { user: user_params }) + + expect(response.status).to eq 403 + end end it 'creates an audit log record' do @@ -151,6 +170,19 @@ describe SessionsController do end end + context 'with password authentication disabled' do + before do + stub_application_setting(password_authentication_enabled_for_web: false) + end + + it 'allows 2FA stage of non-password login' do + authenticate_2fa(otp_attempt: user.current_otp) + + expect(@request.env['warden']).to be_authenticated + expect(subject.current_user).to eq user + end + end + ## # See #14900 issue # diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index 04f39b807d..52e586bb5f 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -325,16 +325,19 @@ describe 'Admin updates settings' do end context 'Network page' do - it 'Enable outbound requests' do + it 'Changes Outbound requests settings' do visit network_admin_application_settings_path page.within('.as-outbound') do check 'Allow requests to the local network from hooks and services' + # Enabled by default + uncheck 'Enforce DNS rebinding attack protection' click_button 'Save changes' end expect(page).to have_content "Application settings saved successfully" expect(Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services).to be true + expect(Gitlab::CurrentSettings.dns_rebinding_protection_enabled).to be false end end diff --git a/spec/features/issuables/issuable_list_spec.rb b/spec/features/issuables/issuable_list_spec.rb index 2f45ef856a..7c447af6d6 100644 --- a/spec/features/issuables/issuable_list_spec.rb +++ b/spec/features/issuables/issuable_list_spec.rb @@ -52,7 +52,7 @@ describe 'issuable list' do create(:issue, project: project, author: user) else create(:merge_request, source_project: project, source_branch: generate(:branch)) - source_branch = FFaker::Name.name + source_branch = FFaker::Lorem.characters(8) pipeline = create(:ci_empty_pipeline, project: project, ref: source_branch, status: %w(running failed success).sample, sha: 'any') create(:merge_request, title: FFaker::Lorem.sentence, source_project: project, source_branch: source_branch, head_pipeline: pipeline) end diff --git a/spec/features/projects/import_export/export_file_spec.rb b/spec/features/projects/import_export/export_file_spec.rb index f76f9ba757..9d74a96ab3 100644 --- a/spec/features/projects/import_export/export_file_spec.rb +++ b/spec/features/projects/import_export/export_file_spec.rb @@ -12,7 +12,7 @@ describe 'Import/Export - project export integration test', :js do let(:export_path) { "#{Dir.tmpdir}/import_file_spec" } let(:config_hash) { YAML.load_file(Gitlab::ImportExport.config_file).deep_stringify_keys } - let(:sensitive_words) { %w[pass secret token key encrypted] } + let(:sensitive_words) { %w[pass secret token key encrypted html] } let(:safe_list) do { token: [ProjectHook, Ci::Trigger, CommitStatus], diff --git a/spec/javascripts/jobs/components/job_app_spec.js b/spec/javascripts/jobs/components/job_app_spec.js index ba5d672f18..89a583bc07 100644 --- a/spec/javascripts/jobs/components/job_app_spec.js +++ b/spec/javascripts/jobs/components/job_app_spec.js @@ -89,9 +89,12 @@ describe('Job App ', () => { describe('triggered job', () => { beforeEach(() => { + const aYearAgo = new Date(); + aYearAgo.setFullYear(aYearAgo.getFullYear() - 1); + mock .onGet(props.endpoint) - .replyOnce(200, Object.assign({}, job, { started: '2017-05-24T10:59:52.000+01:00' })); + .replyOnce(200, Object.assign({}, job, { started: aYearAgo.toISOString() })); vm = mountComponentWithStore(Component, { props, store }); }); diff --git a/spec/lib/banzai/filter/wiki_link_filter_spec.rb b/spec/lib/banzai/filter/wiki_link_filter_spec.rb index b9059b85fd..cce1cd0b28 100644 --- a/spec/lib/banzai/filter/wiki_link_filter_spec.rb +++ b/spec/lib/banzai/filter/wiki_link_filter_spec.rb @@ -70,5 +70,47 @@ describe Banzai::Filter::WikiLinkFilter do expect(filtered_link.attribute('href').value).to eq(invalid_link) end end + + context "when the slug is deemed unsafe or invalid" do + let(:link) { "alert(1);" } + + invalid_slugs = [ + "javascript:", + "JaVaScRiPt:", + "\u0001java\u0003script:", + "javascript :", + "javascript: ", + "javascript : ", + ":javascript:", + "javascript:", + "javascript:", + "javascript:", + "javascript:", + "java\0script:", + "  javascript:" + ] + + invalid_slugs.each do |slug| + context "with the slug #{slug}" do + it "doesn't rewrite a (.) relative link" do + filtered_link = filter( + "Link", + project_wiki: wiki, + page_slug: slug).children[0] + + expect(filtered_link.attribute('href').value).not_to include(slug) + end + + it "doesn't rewrite a (..) relative link" do + filtered_link = filter( + "Link", + project_wiki: wiki, + page_slug: slug).children[0] + + expect(filtered_link.attribute('href').value).not_to include(slug) + end + end + end + end end end diff --git a/spec/lib/banzai/redactor_spec.rb b/spec/lib/banzai/redactor_spec.rb index aaeec953e4..718649e0e1 100644 --- a/spec/lib/banzai/redactor_spec.rb +++ b/spec/lib/banzai/redactor_spec.rb @@ -13,10 +13,10 @@ describe Banzai::Redactor do it 'redacts an array of documents' do doc1 = Nokogiri::HTML - .fragment('foo') + .fragment('foo') doc2 = Nokogiri::HTML - .fragment('bar') + .fragment('bar') redacted_data = redactor.redact([doc1, doc2]) @@ -27,7 +27,7 @@ describe Banzai::Redactor do end it 'replaces redacted reference with inner HTML' do - doc = Nokogiri::HTML.fragment("foo") + doc = Nokogiri::HTML.fragment("foo") redactor.redact([doc]) expect(doc.to_html).to eq('foo') end @@ -35,20 +35,24 @@ describe Banzai::Redactor do context 'when data-original attribute provided' do let(:original_content) { 'foo' } it 'replaces redacted reference with original content' do - doc = Nokogiri::HTML.fragment("bar") + doc = Nokogiri::HTML.fragment("bar") redactor.redact([doc]) expect(doc.to_html).to eq(original_content) end - end - it 'returns tag with original href if it is originally a link reference' do - href = 'http://localhost:3000' - doc = Nokogiri::HTML - .fragment("#{href}") + it 'does not replace redacted reference with original content if href is given' do + html = "Marge" + doc = Nokogiri::HTML.fragment(html) + redactor.redact([doc]) + expect(doc.to_html).to eq('Marge') + end - redactor.redact([doc]) - - expect(doc.to_html).to eq('http://localhost:3000') + it 'uses the original content as the link content if given' do + html = "Marge" + doc = Nokogiri::HTML.fragment(html) + redactor.redact([doc]) + expect(doc.to_html).to eq('Homer') + end end end @@ -61,7 +65,7 @@ describe Banzai::Redactor do end it 'redacts an issue attached' do - doc = Nokogiri::HTML.fragment("foo") + doc = Nokogiri::HTML.fragment("foo") redactor.redact([doc]) @@ -69,7 +73,7 @@ describe Banzai::Redactor do end it 'redacts an external issue' do - doc = Nokogiri::HTML.fragment("foo") + doc = Nokogiri::HTML.fragment("foo") redactor.redact([doc]) diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index e1a2bae5fe..fbe87aaefa 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -5,6 +5,7 @@ describe Gitlab::BitbucketImport::Importer do before do stub_omniauth_provider('bitbucket') + stub_feature_flags(stricter_mr_branch_name: false) end let(:statuses) do diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb index 3e0fda9c30..121463a28e 100644 --- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb @@ -3,7 +3,9 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::File::Remote do - let(:context) { described_class::Context.new(nil, '12345', nil) } + include StubRequests + + let(:context) { described_class::Context.new(nil, '12345', nil, Set.new) } let(:params) { { remote: location } } let(:remote_file) { described_class.new(params, context) } let(:location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } @@ -46,7 +48,7 @@ describe Gitlab::Ci::Config::External::File::Remote do describe "#valid?" do context 'when is a valid remote url' do before do - WebMock.stub_request(:get, location).to_return(body: remote_file_content) + stub_full_request(location).to_return(body: remote_file_content) end it 'should return true' do @@ -92,7 +94,7 @@ describe Gitlab::Ci::Config::External::File::Remote do describe "#content" do context 'with a valid remote file' do before do - WebMock.stub_request(:get, location).to_return(body: remote_file_content) + stub_full_request(location).to_return(body: remote_file_content) end it 'should return the content of the file' do @@ -114,7 +116,7 @@ describe Gitlab::Ci::Config::External::File::Remote do let(:location) { 'https://asdasdasdaj48ggerexample.com' } before do - WebMock.stub_request(:get, location).to_raise(SocketError.new('Some HTTP error')) + stub_full_request(location).to_raise(SocketError.new('Some HTTP error')) end it 'should be nil' do @@ -144,7 +146,7 @@ describe Gitlab::Ci::Config::External::File::Remote do context 'when timeout error has been raised' do before do - WebMock.stub_request(:get, location).to_timeout + stub_full_request(location).to_timeout end it 'should returns error message about a timeout' do @@ -154,7 +156,7 @@ describe Gitlab::Ci::Config::External::File::Remote do context 'when HTTP error has been raised' do before do - WebMock.stub_request(:get, location).to_raise(Gitlab::HTTP::Error) + stub_full_request(location).to_raise(Gitlab::HTTP::Error) end it 'should returns error message about a HTTP error' do @@ -164,7 +166,7 @@ describe Gitlab::Ci::Config::External::File::Remote do context 'when response has 404 status' do before do - WebMock.stub_request(:get, location).to_return(body: remote_file_content, status: 404) + stub_full_request(location).to_return(body: remote_file_content, status: 404) end it 'should returns error message about a timeout' do diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb index 4cab4961b0..72a05327a7 100644 --- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::Mapper do + include StubRequests + set(:project) { create(:project, :repository) } set(:user) { create(:user) } @@ -17,7 +19,7 @@ describe Gitlab::Ci::Config::External::Mapper do end before do - WebMock.stub_request(:get, remote_url).to_return(body: file_content) + stub_full_request(remote_url).to_return(body: file_content) end describe '#process' do diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb index 1ac58139b2..b2c7a235d8 100644 --- a/spec/lib/gitlab/ci/config/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Gitlab::Ci::Config::External::Processor do + include StubRequests + set(:project) { create(:project, :repository) } set(:user) { create(:user) } @@ -37,7 +39,7 @@ describe Gitlab::Ci::Config::External::Processor do let(:values) { { include: remote_file, image: 'ruby:2.2' } } before do - WebMock.stub_request(:get, remote_file).to_raise(SocketError.new('Some HTTP error')) + stub_full_request(remote_file).and_raise(SocketError.new('Some HTTP error')) end it 'should raise an error' do @@ -70,7 +72,7 @@ describe Gitlab::Ci::Config::External::Processor do end before do - WebMock.stub_request(:get, remote_file).to_return(body: external_file_content) + stub_full_request(remote_file).to_return(body: external_file_content) end it 'should append the file to the values' do @@ -140,7 +142,7 @@ describe Gitlab::Ci::Config::External::Processor do allow_any_instance_of(Gitlab::Ci::Config::External::File::Local) .to receive(:fetch_local_content).and_return(local_file_content) - WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content) + stub_full_request(remote_file).to_return(body: remote_file_content) end it 'should append the files to the values' do @@ -185,10 +187,87 @@ describe Gitlab::Ci::Config::External::Processor do HEREDOC end - it 'should take precedence' do - WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content) + it 'takes precedence' do + stub_full_request(remote_file).to_return(body: remote_file_content) + expect(processor.perform[:image]).to eq('ruby:2.2') end end + + context "when a nested includes are defined" do + let(:values) do + { + include: [ + { local: '/local/file.yml' } + ], + image: 'ruby:2.2' + } + end + + before do + allow(project.repository).to receive(:blob_data_at).with('12345', '/local/file.yml') do + <<~HEREDOC + include: + - template: Ruby.gitlab-ci.yml + - remote: http://my.domain.com/config.yml + - project: #{another_project.full_path} + file: /templates/my-workflow.yml + HEREDOC + end + + allow_any_instance_of(Repository).to receive(:blob_data_at).with(another_project.commit.id, '/templates/my-workflow.yml') do + <<~HEREDOC + include: + - local: /templates/my-build.yml + HEREDOC + end + + allow_any_instance_of(Repository).to receive(:blob_data_at).with(another_project.commit.id, '/templates/my-build.yml') do + <<~HEREDOC + my_build: + script: echo Hello World + HEREDOC + end + + stub_full_request('http://my.domain.com/config.yml') + .to_return(body: 'remote_build: { script: echo Hello World }') + end + + context 'when project is public' do + before do + another_project.update!(visibility: 'public') + end + + it 'properly expands all includes' do + is_expected.to include(:my_build, :remote_build, :rspec) + end + end + + context 'when user is reporter of another project' do + before do + another_project.add_reporter(user) + end + + it 'properly expands all includes' do + is_expected.to include(:my_build, :remote_build, :rspec) + end + end + + context 'when user is not allowed' do + it 'raises an error' do + expect { subject }.to raise_error(Gitlab::Ci::Config::External::Processor::IncludeError, /not found or access denied/) + end + end + + context 'when too many includes is included' do + before do + stub_const('Gitlab::Ci::Config::External::Mapper::MAX_INCLUDES', 1) + end + + it 'raises an error' do + expect { subject }.to raise_error(Gitlab::Ci::Config::External::Processor::IncludeError, /Maximum of 1 nested/) + end + end + end end end diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 18f255c1ab..6853969fcf 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::Ci::Config do + include StubRequests + set(:user) { create(:user) } let(:config) do @@ -160,8 +162,7 @@ describe Gitlab::Ci::Config do end before do - WebMock.stub_request(:get, remote_location) - .to_return(body: remote_file_content) + stub_full_request(remote_location).to_return(body: remote_file_content) allow(project.repository) .to receive(:blob_data_at).and_return(local_file_content) diff --git a/spec/lib/gitlab/git_ref_validator_spec.rb b/spec/lib/gitlab/git_ref_validator_spec.rb index 3ab04a1c46..b63389af29 100644 --- a/spec/lib/gitlab/git_ref_validator_spec.rb +++ b/spec/lib/gitlab/git_ref_validator_spec.rb @@ -1,31 +1,69 @@ require 'spec_helper' describe Gitlab::GitRefValidator do - it { expect(described_class.validate('feature/new')).to be_truthy } - it { expect(described_class.validate('implement_@all')).to be_truthy } - it { expect(described_class.validate('my_new_feature')).to be_truthy } - it { expect(described_class.validate('my-branch')).to be_truthy } - it { expect(described_class.validate('#1')).to be_truthy } - it { expect(described_class.validate('feature/refs/heads/foo')).to be_truthy } - it { expect(described_class.validate('feature/~new/')).to be_falsey } - it { expect(described_class.validate('feature/^new/')).to be_falsey } - it { expect(described_class.validate('feature/:new/')).to be_falsey } - it { expect(described_class.validate('feature/?new/')).to be_falsey } - it { expect(described_class.validate('feature/*new/')).to be_falsey } - it { expect(described_class.validate('feature/[new/')).to be_falsey } - it { expect(described_class.validate('feature/new/')).to be_falsey } - it { expect(described_class.validate('feature/new.')).to be_falsey } - it { expect(described_class.validate('feature\@{')).to be_falsey } - it { expect(described_class.validate('feature\new')).to be_falsey } - it { expect(described_class.validate('feature//new')).to be_falsey } - it { expect(described_class.validate('feature new')).to be_falsey } - it { expect(described_class.validate('refs/heads/')).to be_falsey } - it { expect(described_class.validate('refs/remotes/')).to be_falsey } - it { expect(described_class.validate('refs/heads/feature')).to be_falsey } - it { expect(described_class.validate('refs/remotes/origin')).to be_falsey } - it { expect(described_class.validate('-')).to be_falsey } - it { expect(described_class.validate('-branch')).to be_falsey } - it { expect(described_class.validate('.tag')).to be_falsey } - it { expect(described_class.validate('my branch')).to be_falsey } - it { expect(described_class.validate("\xA0\u0000\xB0")).to be_falsey } + using RSpec::Parameterized::TableSyntax + + context '.validate' do + it { expect(described_class.validate('feature/new')).to be true } + it { expect(described_class.validate('implement_@all')).to be true } + it { expect(described_class.validate('my_new_feature')).to be true } + it { expect(described_class.validate('my-branch')).to be true } + it { expect(described_class.validate('#1')).to be true } + it { expect(described_class.validate('feature/refs/heads/foo')).to be true } + it { expect(described_class.validate('feature/~new/')).to be false } + it { expect(described_class.validate('feature/^new/')).to be false } + it { expect(described_class.validate('feature/:new/')).to be false } + it { expect(described_class.validate('feature/?new/')).to be false } + it { expect(described_class.validate('feature/*new/')).to be false } + it { expect(described_class.validate('feature/[new/')).to be false } + it { expect(described_class.validate('feature/new/')).to be false } + it { expect(described_class.validate('feature/new.')).to be false } + it { expect(described_class.validate('feature\@{')).to be false } + it { expect(described_class.validate('feature\new')).to be false } + it { expect(described_class.validate('feature//new')).to be false } + it { expect(described_class.validate('feature new')).to be false } + it { expect(described_class.validate('refs/heads/')).to be false } + it { expect(described_class.validate('refs/remotes/')).to be false } + it { expect(described_class.validate('refs/heads/feature')).to be false } + it { expect(described_class.validate('refs/remotes/origin')).to be false } + it { expect(described_class.validate('-')).to be false } + it { expect(described_class.validate('-branch')).to be false } + it { expect(described_class.validate('+foo:bar')).to be false } + it { expect(described_class.validate('foo:bar')).to be false } + it { expect(described_class.validate('.tag')).to be false } + it { expect(described_class.validate('my branch')).to be false } + it { expect(described_class.validate("\xA0\u0000\xB0")).to be false } + end + + context '.validate_merge_request_branch' do + it { expect(described_class.validate_merge_request_branch('HEAD')).to be true } + it { expect(described_class.validate_merge_request_branch('feature/new')).to be true } + it { expect(described_class.validate_merge_request_branch('implement_@all')).to be true } + it { expect(described_class.validate_merge_request_branch('my_new_feature')).to be true } + it { expect(described_class.validate_merge_request_branch('my-branch')).to be true } + it { expect(described_class.validate_merge_request_branch('#1')).to be true } + it { expect(described_class.validate_merge_request_branch('feature/refs/heads/foo')).to be true } + it { expect(described_class.validate_merge_request_branch('feature/~new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/^new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/:new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/?new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/*new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/[new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/new.')).to be false } + it { expect(described_class.validate_merge_request_branch('feature\@{')).to be false } + it { expect(described_class.validate_merge_request_branch('feature\new')).to be false } + it { expect(described_class.validate_merge_request_branch('feature//new')).to be false } + it { expect(described_class.validate_merge_request_branch('feature new')).to be false } + it { expect(described_class.validate_merge_request_branch('refs/heads/master')).to be true } + it { expect(described_class.validate_merge_request_branch('refs/heads/')).to be false } + it { expect(described_class.validate_merge_request_branch('refs/remotes/')).to be false } + it { expect(described_class.validate_merge_request_branch('-')).to be false } + it { expect(described_class.validate_merge_request_branch('-branch')).to be false } + it { expect(described_class.validate_merge_request_branch('+foo:bar')).to be false } + it { expect(described_class.validate_merge_request_branch('foo:bar')).to be false } + it { expect(described_class.validate_merge_request_branch('.tag')).to be false } + it { expect(described_class.validate_merge_request_branch('my branch')).to be false } + it { expect(described_class.validate_merge_request_branch("\xA0\u0000\xB0")).to be false } + end end diff --git a/spec/lib/gitlab/http_connection_adapter_spec.rb b/spec/lib/gitlab/http_connection_adapter_spec.rb new file mode 100644 index 0000000000..930d1f6227 --- /dev/null +++ b/spec/lib/gitlab/http_connection_adapter_spec.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::HTTPConnectionAdapter do + describe '#connection' do + context 'when local requests are not allowed' do + it 'sets up the connection' do + uri = URI('https://example.org') + + connection = described_class.new(uri).connection + + expect(connection).to be_a(Net::HTTP) + expect(connection.address).to eq('93.184.216.34') + expect(connection.hostname_override).to eq('example.org') + expect(connection.addr_port).to eq('example.org') + expect(connection.port).to eq(443) + end + + it 'raises error when it is a request to local address' do + uri = URI('http://172.16.0.0/12') + + expect { described_class.new(uri).connection } + .to raise_error(Gitlab::HTTP::BlockedUrlError, + "URL 'http://172.16.0.0/12' is blocked: Requests to the local network are not allowed") + end + + it 'raises error when it is a request to localhost address' do + uri = URI('http://127.0.0.1') + + expect { described_class.new(uri).connection } + .to raise_error(Gitlab::HTTP::BlockedUrlError, + "URL 'http://127.0.0.1' is blocked: Requests to localhost are not allowed") + end + + context 'when port different from URL scheme is used' do + it 'sets up the addr_port accordingly' do + uri = URI('https://example.org:8080') + + connection = described_class.new(uri).connection + + expect(connection.address).to eq('93.184.216.34') + expect(connection.hostname_override).to eq('example.org') + expect(connection.addr_port).to eq('example.org:8080') + expect(connection.port).to eq(8080) + end + end + end + + context 'when DNS rebinding protection is disabled' do + it 'sets up the connection' do + stub_application_setting(dns_rebinding_protection_enabled: false) + + uri = URI('https://example.org') + + connection = described_class.new(uri).connection + + expect(connection).to be_a(Net::HTTP) + expect(connection.address).to eq('example.org') + expect(connection.hostname_override).to eq(nil) + expect(connection.addr_port).to eq('example.org') + expect(connection.port).to eq(443) + end + end + + context 'when http(s) environment variable is set' do + it 'sets up the connection' do + stub_env('https_proxy' => 'https://my.proxy') + + uri = URI('https://example.org') + + connection = described_class.new(uri).connection + + expect(connection).to be_a(Net::HTTP) + expect(connection.address).to eq('example.org') + expect(connection.hostname_override).to eq(nil) + expect(connection.addr_port).to eq('example.org') + expect(connection.port).to eq(443) + end + end + + context 'when local requests are allowed' do + it 'sets up the connection' do + uri = URI('https://example.org') + + connection = described_class.new(uri, allow_local_requests: true).connection + + expect(connection).to be_a(Net::HTTP) + expect(connection.address).to eq('93.184.216.34') + expect(connection.hostname_override).to eq('example.org') + expect(connection.addr_port).to eq('example.org') + expect(connection.port).to eq(443) + end + + it 'sets up the connection when it is a local network' do + uri = URI('http://172.16.0.0/12') + + connection = described_class.new(uri, allow_local_requests: true).connection + + expect(connection).to be_a(Net::HTTP) + expect(connection.address).to eq('172.16.0.0') + expect(connection.hostname_override).to be(nil) + expect(connection.addr_port).to eq('172.16.0.0') + expect(connection.port).to eq(80) + end + + it 'sets up the connection when it is localhost' do + uri = URI('http://127.0.0.1') + + connection = described_class.new(uri, allow_local_requests: true).connection + + expect(connection).to be_a(Net::HTTP) + expect(connection.address).to eq('127.0.0.1') + expect(connection.hostname_override).to be(nil) + expect(connection.addr_port).to eq('127.0.0.1') + expect(connection.port).to eq(80) + end + end + end +end diff --git a/spec/lib/gitlab/http_spec.rb b/spec/lib/gitlab/http_spec.rb index 6c37c157f5..158f77cab2 100644 --- a/spec/lib/gitlab/http_spec.rb +++ b/spec/lib/gitlab/http_spec.rb @@ -1,6 +1,28 @@ require 'spec_helper' describe Gitlab::HTTP do + include StubRequests + + context 'when allow_local_requests' do + it 'sends the request to the correct URI' do + stub_full_request('https://example.org:8080', ip_address: '8.8.8.8').to_return(status: 200) + + described_class.get('https://example.org:8080', allow_local_requests: false) + + expect(WebMock).to have_requested(:get, 'https://8.8.8.8:8080').once + end + end + + context 'when not allow_local_requests' do + it 'sends the request to the correct URI' do + stub_full_request('https://example.org:8080') + + described_class.get('https://example.org:8080', allow_local_requests: true) + + expect(WebMock).to have_requested(:get, 'https://8.8.8.9:8080').once + end + end + describe 'allow_local_requests_from_hooks_and_services is' do before do WebMock.stub_request(:get, /.*/).to_return(status: 200, body: 'Success') @@ -21,6 +43,8 @@ describe Gitlab::HTTP do context 'if allow_local_requests set to true' do it 'override the global value and allow requests to localhost or private network' do + stub_full_request('http://localhost:3003') + expect { described_class.get('http://localhost:3003', allow_local_requests: true) }.not_to raise_error end end @@ -32,6 +56,8 @@ describe Gitlab::HTTP do end it 'allow requests to localhost' do + stub_full_request('http://localhost:3003') + expect { described_class.get('http://localhost:3003') }.not_to raise_error end @@ -49,7 +75,7 @@ describe Gitlab::HTTP do describe 'handle redirect loops' do before do - WebMock.stub_request(:any, "http://example.org").to_raise(HTTParty::RedirectionTooDeep.new("Redirection Too Deep")) + stub_full_request("http://example.org", method: :any).to_raise(HTTParty::RedirectionTooDeep.new("Redirection Too Deep")) end it 'handles GET requests' do diff --git a/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb b/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb index ec17ad8541..21a227335c 100644 --- a/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb +++ b/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::ImportExport::AfterExportStrategies::WebUploadStrategy do + include StubRequests + let(:example_url) { 'http://www.example.com' } let(:strategy) { subject.new(url: example_url, http_method: 'post') } let!(:project) { create(:project, :with_export) } @@ -32,5 +34,17 @@ describe Gitlab::ImportExport::AfterExportStrategies::WebUploadStrategy do strategy.execute(user, project) end + + context 'when upload fails' do + it 'stores the export error' do + stub_full_request(example_url, method: :post).to_return(status: [404, 'Page not found']) + + strategy.execute(user, project) + + errors = project.import_export_shared.errors + expect(errors).not_to be_empty + expect(errors.first).to eq "Error uploading the project. Code 404: Page not found" + 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 536cc359d3..99669285d5 100644 --- a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb +++ b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb @@ -18,7 +18,11 @@ describe Gitlab::ImportExport::AttributeCleaner do 'notid' => 99, 'import_source' => 'whatever', 'import_type' => 'whatever', - 'non_existent_attr' => 'whatever' + 'non_existent_attr' => 'whatever', + 'some_html' => '

dodgy html

', + 'legit_html' => '

legit html

', + '_html' => '

perfectly ordinary html

', + 'cached_markdown_version' => 12345 } end diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 58949f76bd..0b6c6cd3fe 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -158,6 +158,8 @@ { "id": 351, "note": "Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi.", + "note_html": "

something else entirely

", + "cached_markdown_version": 917504, "noteable_type": "Issue", "author_id": 26, "created_at": "2016-06-14T15:02:47.770Z", @@ -2363,6 +2365,8 @@ { "id": 671, "note": "Sit voluptatibus eveniet architecto quidem.", + "note_html": "

something else entirely

", + "cached_markdown_version": 917504, "noteable_type": "MergeRequest", "author_id": 26, "created_at": "2016-06-14T15:02:56.632Z", diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 6084dc9641..9d2b69ea79 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -58,6 +58,26 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect(Milestone.find_by_description('test milestone').issues.count).to eq(2) end + context 'when importing a project with cached_markdown_version and note_html' do + context 'for an Issue' do + it 'does not import note_html' do + note_content = 'Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi' + issue_note = Issue.find_by(description: 'Aliquam enim illo et possimus.').notes.select { |n| n.note.match(/#{note_content}/)}.first + + expect(issue_note.note_html).to match(/#{note_content}/) + end + end + + context 'for a Merge Request' do + it 'does not import note_html' do + note_content = 'Sit voluptatibus eveniet architecto quidem' + merge_request_note = MergeRequest.find_by(title: 'MR1').notes.select { |n| n.note.match(/#{note_content}/)}.first + + expect(merge_request_note.note_html).to match(/#{note_content}/) + end + end + end + it 'creates a valid pipeline note' do expect(Ci::Pipeline.find_by_sha('sha-notes').notes).not_to be_empty end diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb index 87288baedb..f8a93b78a8 100644 --- a/spec/lib/gitlab/search_results_spec.rb +++ b/spec/lib/gitlab/search_results_spec.rb @@ -240,4 +240,28 @@ describe Gitlab::SearchResults do expect(results.objects('merge_requests')).not_to include merge_request end + + context 'milestones' do + it 'returns correct set of milestones' do + private_project_1 = create(:project, :private) + private_project_2 = create(:project, :private) + internal_project = create(:project, :internal) + public_project_1 = create(:project, :public) + public_project_2 = create(:project, :public, :issues_disabled, :merge_requests_disabled) + private_project_1.add_developer(user) + # milestones that should not be visible + create(:milestone, project: private_project_2, title: 'Private project without access milestone') + create(:milestone, project: public_project_2, title: 'Public project with milestones disabled milestone') + # milestones that should be visible + milestone_1 = create(:milestone, project: private_project_1, title: 'Private project with access milestone', state: 'closed') + milestone_2 = create(:milestone, project: internal_project, title: 'Internal project milestone') + milestone_3 = create(:milestone, project: public_project_1, title: 'Public project with milestones enabled milestone') + # Global search scope takes user authorized projects, internal projects and public projects. + limit_projects = ProjectsFinder.new(current_user: user).execute + + milestones = described_class.new(user, limit_projects, 'milestone').objects('milestones') + + expect(milestones).to match_array([milestone_1, milestone_2, milestone_3]) + end + end end diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index 62970bd8cb..7cb1898d5c 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -2,6 +2,87 @@ require 'spec_helper' describe Gitlab::UrlBlocker do + describe '#validate!' do + context 'when URI is nil' do + let(:import_url) { nil } + + it 'returns no URI and hostname' do + uri, hostname = described_class.validate!(import_url) + + expect(uri).to be(nil) + expect(hostname).to be(nil) + end + end + + context 'when URI is internal' do + let(:import_url) { 'http://localhost' } + + it 'returns URI and no hostname' do + uri, hostname = described_class.validate!(import_url) + + expect(uri).to eq(Addressable::URI.parse('http://[::1]')) + expect(hostname).to eq('localhost') + end + end + + context 'when the URL hostname is a domain' do + let(:import_url) { 'https://example.org' } + + it 'returns URI and hostname' do + uri, hostname = described_class.validate!(import_url) + + expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34')) + expect(hostname).to eq('example.org') + end + end + + context 'when the URL hostname is an IP address' do + let(:import_url) { 'https://93.184.216.34' } + + it 'returns URI and no hostname' do + uri, hostname = described_class.validate!(import_url) + + expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34')) + expect(hostname).to be(nil) + end + end + + context 'disabled DNS rebinding protection' do + context 'when URI is internal' do + let(:import_url) { 'http://localhost' } + + it 'returns URI and no hostname' do + uri, hostname = described_class.validate!(import_url, dns_rebind_protection: false) + + expect(uri).to eq(Addressable::URI.parse('http://localhost')) + expect(hostname).to be(nil) + end + end + + context 'when the URL hostname is a domain' do + let(:import_url) { 'https://example.org' } + + it 'returns URI and no hostname' do + uri, hostname = described_class.validate!(import_url, dns_rebind_protection: false) + + expect(uri).to eq(Addressable::URI.parse('https://example.org')) + expect(hostname).to eq(nil) + end + end + + context 'when the URL hostname is an IP address' do + let(:import_url) { 'https://93.184.216.34' } + + it 'returns URI and no hostname' do + uri, hostname = described_class.validate!(import_url, dns_rebind_protection: false) + + expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34')) + expect(hostname).to be(nil) + end + end + end + end + describe '#blocked_url?' do let(:ports) { Project::VALID_IMPORT_PORTS } @@ -208,7 +289,7 @@ describe Gitlab::UrlBlocker do end def stub_domain_resolv(domain, ip) - address = double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false) + address = double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false, ipv4?: false) allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([address]) allow(address).to receive(:ipv6_v4mapped?).and_return(false) end diff --git a/spec/lib/gitlab_spec.rb b/spec/lib/gitlab_spec.rb index 5f7a0cca35..fd63e3d38a 100644 --- a/spec/lib/gitlab_spec.rb +++ b/spec/lib/gitlab_spec.rb @@ -95,4 +95,48 @@ describe Gitlab do expect(described_class.com?).to eq false end end + + describe '.ee?' do + it 'returns true when using Enterprise Edition' do + stub_const('License', Class.new) + + expect(described_class.ee?).to eq(true) + end + + it 'returns false when using Community Edition' do + hide_const('License') + + expect(described_class.ee?).to eq(false) + end + end + + describe '.http_proxy_env?' do + it 'returns true when lower case https' do + stub_env('https_proxy', 'https://my.proxy') + + expect(described_class.http_proxy_env?).to eq(true) + end + + it 'returns true when upper case https' do + stub_env('HTTPS_PROXY', 'https://my.proxy') + + expect(described_class.http_proxy_env?).to eq(true) + end + + it 'returns true when lower case http' do + stub_env('http_proxy', 'http://my.proxy') + + expect(described_class.http_proxy_env?).to eq(true) + end + + it 'returns true when upper case http' do + stub_env('HTTP_PROXY', 'http://my.proxy') + + expect(described_class.http_proxy_env?).to eq(true) + end + + it 'returns false when not set' do + expect(described_class.http_proxy_env?).to eq(false) + end + end end diff --git a/spec/lib/mattermost/session_spec.rb b/spec/lib/mattermost/session_spec.rb index 77fea5b2d2..346455067a 100644 --- a/spec/lib/mattermost/session_spec.rb +++ b/spec/lib/mattermost/session_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Mattermost::Session, type: :request do include ExclusiveLeaseHelpers + include StubRequests let(:user) { create(:user) } @@ -24,7 +25,7 @@ describe Mattermost::Session, type: :request do let(:location) { 'http://location.tld' } let(:cookie_header) {'MMOAUTH=taskik8az7rq8k6rkpuas7htia; Path=/;'} let!(:stub) do - WebMock.stub_request(:get, "#{mattermost_url}/oauth/gitlab/login") + stub_full_request("#{mattermost_url}/oauth/gitlab/login") .to_return(headers: { 'location' => location, 'Set-Cookie' => cookie_header }, status: 302) end @@ -63,7 +64,7 @@ describe Mattermost::Session, type: :request do end before do - WebMock.stub_request(:get, "#{mattermost_url}/signup/gitlab/complete") + stub_full_request("#{mattermost_url}/signup/gitlab/complete") .with(query: hash_including({ 'state' => state })) .to_return do |request| post "/oauth/token", @@ -80,7 +81,7 @@ describe Mattermost::Session, type: :request do end end - WebMock.stub_request(:post, "#{mattermost_url}/api/v4/users/logout") + stub_full_request("#{mattermost_url}/api/v4/users/logout", method: :post) .to_return(headers: { Authorization: 'token thisworksnow' }, status: 200) end diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index cd29e0d4f5..c2c6cfb1a3 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -122,6 +122,27 @@ describe Clusters::Applications::Knative do end end + describe '#install_command' do + subject { knative.install_command } + + it 'is initialized with latest version' do + expect(subject.version).to eq('0.5.0') + end + + it_behaves_like 'a command' + end + + describe '#update_command' do + let!(:current_installed_version) { knative.version = '0.1.0' } + subject { knative.update_command } + + it 'is initialized with current version' do + expect(subject.version).to eq(current_installed_version) + end + + it_behaves_like 'a command' + end + describe '#files' do let(:application) { knative } let(:values) { subject[:'values.yaml'] } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 39ffef5f05..16dbeab050 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -153,6 +153,42 @@ describe MergeRequest do end end + context 'for branch' do + before do + stub_feature_flags(stricter_mr_branch_name: false) + end + + using RSpec::Parameterized::TableSyntax + + where(:branch_name, :valid) do + 'foo' | true + 'foo:bar' | false + '+foo:bar' | false + 'foo bar' | false + '-foo' | false + 'HEAD' | true + 'refs/heads/master' | true + end + + with_them do + it "validates source_branch" do + subject = build(:merge_request, source_branch: branch_name, target_branch: 'master') + + subject.valid? + + expect(subject.errors.added?(:source_branch)).to eq(!valid) + end + + it "validates target_branch" do + subject = build(:merge_request, source_branch: 'master', target_branch: branch_name) + + subject.valid? + + expect(subject.errors.added?(:target_branch)).to eq(!valid) + end + end + end + context 'for forks' do let(:project) { create(:project) } let(:fork1) { fork_project(project) } diff --git a/spec/models/project_services/assembla_service_spec.rb b/spec/models/project_services/assembla_service_spec.rb index 5cb6d63659..9d6d2ccb0e 100644 --- a/spec/models/project_services/assembla_service_spec.rb +++ b/spec/models/project_services/assembla_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe AssemblaService do + include StubRequests + describe "Associations" do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } @@ -21,12 +23,12 @@ describe AssemblaService do ) @sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) @api_url = 'https://atlas.assembla.com/spaces/project_name/github_tool?secret_key=verySecret' - WebMock.stub_request(:post, @api_url) + stub_full_request(@api_url, method: :post) end it "calls Assembla API" do @assembla_service.execute(@sample_data) - expect(WebMock).to have_requested(:post, @api_url).with( + expect(WebMock).to have_requested(:post, stubbed_hostname(@api_url)).with( body: /#{@sample_data[:before]}.*#{@sample_data[:after]}.*#{project.path}/ ).once end diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb index b880d90d28..b67a5666b4 100644 --- a/spec/models/project_services/bamboo_service_spec.rb +++ b/spec/models/project_services/bamboo_service_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe BambooService, :use_clean_rails_memory_store_caching do include ReactiveCachingHelpers + include StubRequests let(:bamboo_url) { 'http://gitlab.com/bamboo' } @@ -255,7 +256,7 @@ describe BambooService, :use_clean_rails_memory_store_caching do end def stub_bamboo_request(url, status, body) - WebMock.stub_request(:get, url).to_return( + stub_full_request(url).to_return( status: status, headers: { 'Content-Type' => 'application/json' }, body: body diff --git a/spec/models/project_services/buildkite_service_spec.rb b/spec/models/project_services/buildkite_service_spec.rb index 1615a93a4c..8e0b774d8b 100644 --- a/spec/models/project_services/buildkite_service_spec.rb +++ b/spec/models/project_services/buildkite_service_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe BuildkiteService, :use_clean_rails_memory_store_caching do include ReactiveCachingHelpers + include StubRequests let(:project) { create(:project) } @@ -108,10 +109,9 @@ describe BuildkiteService, :use_clean_rails_memory_store_caching do body ||= %q({"status":"success"}) buildkite_full_url = 'https://gitlab.buildkite.com/status/secret-sauce-status-token.json?commit=123' - WebMock.stub_request(:get, buildkite_full_url).to_return( - status: status, - headers: { 'Content-Type' => 'application/json' }, - body: body - ) + stub_full_request(buildkite_full_url) + .to_return(status: status, + headers: { 'Content-Type' => 'application/json' }, + body: body) end end diff --git a/spec/models/project_services/campfire_service_spec.rb b/spec/models/project_services/campfire_service_spec.rb index ed8347edff..799d1d3cd3 100644 --- a/spec/models/project_services/campfire_service_spec.rb +++ b/spec/models/project_services/campfire_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe CampfireService do + include StubRequests + describe 'Associations' do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } @@ -47,39 +49,37 @@ describe CampfireService do it "calls Campfire API to get a list of rooms and speak in a room" do # make sure a valid list of rooms is returned body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms.json') - WebMock.stub_request(:get, @rooms_url).with(basic_auth: @auth).to_return( + + stub_full_request(@rooms_url).with(basic_auth: @auth).to_return( body: body, status: 200, headers: @headers ) + # stub the speak request with the room id found in the previous request's response speak_url = 'https://project-name.campfirenow.com/room/123/speak.json' - WebMock.stub_request(:post, speak_url).with(basic_auth: @auth) + stub_full_request(speak_url, method: :post).with(basic_auth: @auth) @campfire_service.execute(@sample_data) - expect(WebMock).to have_requested(:get, @rooms_url).once - expect(WebMock).to have_requested(:post, speak_url).with( - body: /#{project.path}.*#{@sample_data[:before]}.*#{@sample_data[:after]}/ - ).once + expect(WebMock).to have_requested(:get, stubbed_hostname(@rooms_url)).once + expect(WebMock).to have_requested(:post, stubbed_hostname(speak_url)) + .with(body: /#{project.path}.*#{@sample_data[:before]}.*#{@sample_data[:after]}/).once end it "calls Campfire API to get a list of rooms but shouldn't speak in a room" do # return a list of rooms that do not contain a room named 'test-room' body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms2.json') - WebMock.stub_request(:get, @rooms_url).with(basic_auth: @auth).to_return( + stub_full_request(@rooms_url).with(basic_auth: @auth).to_return( body: body, status: 200, headers: @headers ) - # we want to make sure no request is sent to the /speak endpoint, here is a basic - # regexp that matches this endpoint - speak_url = 'https://verySecret:X@project-name.campfirenow.com/room/.*/speak.json' @campfire_service.execute(@sample_data) - expect(WebMock).to have_requested(:get, @rooms_url).once - expect(WebMock).not_to have_requested(:post, /#{speak_url}/) + expect(WebMock).to have_requested(:get, 'https://8.8.8.9/rooms.json').once + expect(WebMock).not_to have_requested(:post, '*/room/.*/speak.json') end end end diff --git a/spec/models/project_services/pivotaltracker_service_spec.rb b/spec/models/project_services/pivotaltracker_service_spec.rb index f7d2372eca..4610738f4a 100644 --- a/spec/models/project_services/pivotaltracker_service_spec.rb +++ b/spec/models/project_services/pivotaltracker_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe PivotaltrackerService do + include StubRequests + describe 'Associations' do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } @@ -51,12 +53,12 @@ describe PivotaltrackerService do end before do - WebMock.stub_request(:post, url) + stub_full_request(url, method: :post) end it 'should post correct message' do service.execute(push_data) - expect(WebMock).to have_requested(:post, url).with( + expect(WebMock).to have_requested(:post, stubbed_hostname(url)).with( body: { 'source_commit' => { 'commit_id' => '21c12ea', @@ -83,14 +85,14 @@ describe PivotaltrackerService do service.execute(push_data(branch: 'master')) service.execute(push_data(branch: 'v10')) - expect(WebMock).to have_requested(:post, url).twice + expect(WebMock).to have_requested(:post, stubbed_hostname(url)).twice end it 'should not post message if branch is not in the list' do service.execute(push_data(branch: 'mas')) service.execute(push_data(branch: 'v11')) - expect(WebMock).not_to have_requested(:post, url) + expect(WebMock).not_to have_requested(:post, stubbed_hostname(url)) end end end diff --git a/spec/models/project_services/pushover_service_spec.rb b/spec/models/project_services/pushover_service_spec.rb index 54b8c658ff..a8580d349e 100644 --- a/spec/models/project_services/pushover_service_spec.rb +++ b/spec/models/project_services/pushover_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe PushoverService do + include StubRequests + describe 'Associations' do it { is_expected.to belong_to :project } it { is_expected.to have_one :service_hook } @@ -55,13 +57,13 @@ describe PushoverService do sound: sound ) - WebMock.stub_request(:post, api_url) + stub_full_request(api_url, method: :post, ip_address: '8.8.8.8') end it 'calls Pushover API' do pushover.execute(sample_data) - expect(WebMock).to have_requested(:post, api_url).once + expect(WebMock).to have_requested(:post, 'https://8.8.8.8/1/messages.json').once end end end diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb index 64b4efca43..605261a930 100644 --- a/spec/models/project_services/teamcity_service_spec.rb +++ b/spec/models/project_services/teamcity_service_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe TeamcityService, :use_clean_rails_memory_store_caching do include ReactiveCachingHelpers + include StubRequests let(:teamcity_url) { 'http://gitlab.com/teamcity' } @@ -210,7 +211,7 @@ describe TeamcityService, :use_clean_rails_memory_store_caching do body ||= %Q({"build":{"status":"#{build_status}","id":"666"}}) - WebMock.stub_request(:get, teamcity_full_url).with(basic_auth: auth).to_return( + stub_full_request(teamcity_full_url).with(basic_auth: auth).to_return( status: status, headers: { 'Content-Type' => 'application/json' }, body: body diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 851b796c73..c826c1451e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -219,6 +219,13 @@ describe Project do expect(project2).not_to be_valid end + it 'validates the visibility' do + expect_any_instance_of(described_class).to receive(:visibility_level_allowed_as_fork).and_call_original + expect_any_instance_of(described_class).to receive(:visibility_level_allowed_by_group).and_call_original + + create(:project) + end + describe 'wiki path conflict' do context "when the new path has been used by the wiki of other Project" do it 'has an error on the name attribute' do @@ -3108,6 +3115,23 @@ describe Project do end end + describe '.ids_with_milestone_available_for' do + let!(:user) { create(:user) } + + it 'returns project ids with milestones available for user' do + project_1 = create(:project, :public, :merge_requests_disabled, :issues_disabled) + project_2 = create(:project, :public, :merge_requests_disabled) + project_3 = create(:project, :public, :issues_disabled) + project_4 = create(:project, :public) + project_4.project_feature.update(issues_access_level: ProjectFeature::PRIVATE, merge_requests_access_level: ProjectFeature::PRIVATE ) + + project_ids = described_class.ids_with_milestone_available_for(user).pluck(:id) + + expect(project_ids).to include(project_2.id, project_3.id) + expect(project_ids).not_to include(project_1.id, project_4.id) + end + end + describe '.with_feature_available_for_user' do let!(:user) { create(:user) } let!(:feature) { MergeRequest } diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index 831f47debe..1f7a44102c 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -70,11 +70,30 @@ describe API::Search do context 'for milestones scope' do before do create(:milestone, project: project, title: 'awesome milestone') - - get api('/search', user), params: { scope: 'milestones', search: 'awesome' } end - it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' + context 'when user can read project milestones' do + before do + get api('/search', user), params: { scope: 'milestones', search: 'awesome' } + end + + it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' + end + + context 'when user cannot read project milestones' do + before do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'returns empty array' do + get api('/search', user), params: { scope: 'milestones', search: 'awesome' } + + milestones = JSON.parse(response.body) + + expect(milestones).to be_empty + end + end end context 'for snippet_titles scope' do @@ -262,11 +281,30 @@ describe API::Search do context 'for milestones scope' do before do create(:milestone, project: project, title: 'awesome milestone') - - get api("/projects/#{project.id}/search", user), params: { scope: 'milestones', search: 'awesome' } end - it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' + context 'when user can read milestones' do + before do + get api("/projects/#{project.id}/search", user), params: { scope: 'milestones', search: 'awesome' } + end + + it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' + end + + context 'when user cannot read project milestones' do + before do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'returns empty array' do + get api("/projects/#{project.id}/search", user), params: { scope: 'milestones', search: 'awesome' } + + milestones = JSON.parse(response.body) + + expect(milestones).to be_empty + end + end end context 'for notes scope' do diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb index b6e8d74c2e..0e2f3face7 100644 --- a/spec/requests/api/system_hooks_spec.rb +++ b/spec/requests/api/system_hooks_spec.rb @@ -1,12 +1,14 @@ require 'spec_helper' describe API::SystemHooks do + include StubRequests + let(:user) { create(:user) } let(:admin) { create(:admin) } let!(:hook) { create(:system_hook, url: "http://example.com") } before do - stub_request(:post, hook.url) + stub_full_request(hook.url, method: :post) end describe "GET /hooks" do @@ -68,6 +70,8 @@ describe API::SystemHooks do end it 'sets default values for events' do + stub_full_request('http://mep.mep', method: :post) + post api('/hooks', admin), params: { url: 'http://mep.mep' } expect(response).to have_gitlab_http_status(201) @@ -78,6 +82,8 @@ describe API::SystemHooks do end it 'sets explicit values for events' do + stub_full_request('http://mep.mep', method: :post) + post api('/hooks', admin), params: { url: 'http://mep.mep', diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 8497e90bd8..7a9dd77dc5 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -845,7 +845,7 @@ describe Ci::CreatePipelineService do let(:merge_request) do create(:merge_request, source_project: project, - source_branch: ref_name, + source_branch: Gitlab::Git.ref_name(ref_name), target_project: project, target_branch: 'master') end @@ -876,7 +876,7 @@ describe Ci::CreatePipelineService do let(:merge_request) do create(:merge_request, source_project: project, - source_branch: ref_name, + source_branch: Gitlab::Git.ref_name(ref_name), target_project: project, target_branch: 'master') end @@ -905,7 +905,7 @@ describe Ci::CreatePipelineService do let(:merge_request) do create(:merge_request, source_project: project, - source_branch: ref_name, + source_branch: Gitlab::Git.ref_name(ref_name), target_project: project, target_branch: 'master') end diff --git a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb index 876beb3980..bcfdbe5147 100644 --- a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb +++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Projects::LfsPointers::LfsDownloadService do + include StubRequests + let(:project) { create(:project) } let(:lfs_content) { SecureRandom.random_bytes(10) } let(:oid) { Digest::SHA256.hexdigest(lfs_content) } @@ -61,7 +63,7 @@ describe Projects::LfsPointers::LfsDownloadService do describe '#execute' do context 'when file download succeeds' do before do - WebMock.stub_request(:get, download_link).to_return(body: lfs_content) + stub_full_request(download_link).to_return(body: lfs_content) end it_behaves_like 'lfs object is created' @@ -103,7 +105,7 @@ describe Projects::LfsPointers::LfsDownloadService do let(:size) { 1 } before do - WebMock.stub_request(:get, download_link).to_return(body: lfs_content) + stub_full_request(download_link).to_return(body: lfs_content) end it_behaves_like 'no lfs object is created' @@ -117,7 +119,7 @@ describe Projects::LfsPointers::LfsDownloadService do context 'when downloaded lfs file has a different oid' do before do - WebMock.stub_request(:get, download_link).to_return(body: lfs_content) + stub_full_request(download_link).to_return(body: lfs_content) allow_any_instance_of(Digest::SHA256).to receive(:hexdigest).and_return('foobar') end @@ -135,7 +137,7 @@ describe Projects::LfsPointers::LfsDownloadService do let(:lfs_object) { LfsDownloadObject.new(oid: oid, size: size, link: download_link_with_credentials) } before do - WebMock.stub_request(:get, download_link).with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNzd29yZA==' }).to_return(body: lfs_content) + stub_full_request(download_link).with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNzd29yZA==' }).to_return(body: lfs_content) end it 'the request adds authorization headers' do @@ -148,7 +150,7 @@ describe Projects::LfsPointers::LfsDownloadService do let(:local_request_setting) { true } before do - WebMock.stub_request(:get, download_link).to_return(body: lfs_content) + stub_full_request(download_link, ip_address: '192.168.2.120').to_return(body: lfs_content) end it_behaves_like 'lfs object is created' @@ -172,7 +174,8 @@ describe Projects::LfsPointers::LfsDownloadService do with_them do before do - WebMock.stub_request(:get, download_link).to_return(status: 301, headers: { 'Location' => redirect_link }) + stub_full_request(download_link, ip_address: '192.168.2.120') + .to_return(status: 301, headers: { 'Location' => redirect_link }) end it_behaves_like 'no lfs object is created' @@ -183,8 +186,8 @@ describe Projects::LfsPointers::LfsDownloadService do let(:redirect_link) { "http://example.com/"} before do - WebMock.stub_request(:get, download_link).to_return(status: 301, headers: { 'Location' => redirect_link }) - WebMock.stub_request(:get, redirect_link).to_return(body: lfs_content) + stub_full_request(download_link).to_return(status: 301, headers: { 'Location' => redirect_link }) + stub_full_request(redirect_link).to_return(body: lfs_content) end it_behaves_like 'lfs object is created' diff --git a/spec/services/submit_usage_ping_service_spec.rb b/spec/services/submit_usage_ping_service_spec.rb index c8a6fc1a99..5dd9774059 100644 --- a/spec/services/submit_usage_ping_service_spec.rb +++ b/spec/services/submit_usage_ping_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe SubmitUsagePingService do + include StubRequests + context 'when usage ping is disabled' do before do stub_application_setting(usage_ping_enabled: false) @@ -97,7 +99,7 @@ describe SubmitUsagePingService do end def stub_response(body) - stub_request(:post, 'https://version.gitlab.com/usage_data') + stub_full_request('https://version.gitlab.com/usage_data', method: :post) .to_return( headers: { 'Content-Type' => 'application/json' }, body: body.to_json diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 5945a7dc0a..a77f31c577 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe WebHookService do + include StubRequests + let(:project) { create(:project) } let(:project_hook) { create(:project_hook) } let(:headers) do @@ -65,11 +67,11 @@ describe WebHookService do let(:project_hook) { create(:project_hook, url: 'https://demo:demo@example.org/') } it 'uses the credentials' do - WebMock.stub_request(:post, url) + stub_full_request(url, method: :post) service_instance.execute - expect(WebMock).to have_requested(:post, url).with( + expect(WebMock).to have_requested(:post, stubbed_hostname(url)).with( headers: headers.merge('Authorization' => 'Basic ZGVtbzpkZW1v') ).once end @@ -80,11 +82,11 @@ describe WebHookService do let(:project_hook) { create(:project_hook, url: 'https://demo@example.org/') } it 'uses the credentials anyways' do - WebMock.stub_request(:post, url) + stub_full_request(url, method: :post) service_instance.execute - expect(WebMock).to have_requested(:post, url).with( + expect(WebMock).to have_requested(:post, stubbed_hostname(url)).with( headers: headers.merge('Authorization' => 'Basic ZGVtbzo=') ).once end diff --git a/spec/support/helpers/stub_requests.rb b/spec/support/helpers/stub_requests.rb new file mode 100644 index 0000000000..5cad35282c --- /dev/null +++ b/spec/support/helpers/stub_requests.rb @@ -0,0 +1,40 @@ +module StubRequests + IP_ADDRESS_STUB = '8.8.8.9'.freeze + + # Fully stubs a request using WebMock class. This class also + # stubs the IP address the URL is translated to (DNS lookup). + # + # It expects the final request to go to the `ip_address` instead the given url. + # That's primarily a DNS rebind attack prevention of Gitlab::HTTP + # (see: Gitlab::UrlBlocker). + # + def stub_full_request(url, ip_address: IP_ADDRESS_STUB, port: 80, method: :get) + stub_dns(url, ip_address: ip_address, port: port) + + url = stubbed_hostname(url, hostname: ip_address) + WebMock.stub_request(method, url) + end + + def stub_dns(url, ip_address:, port: 80) + url = parse_url(url) + socket = Socket.sockaddr_in(port, ip_address) + addr = Addrinfo.new(socket) + + # See Gitlab::UrlBlocker + allow(Addrinfo).to receive(:getaddrinfo) + .with(url.hostname, url.port, nil, :STREAM) + .and_return([addr]) + end + + def stubbed_hostname(url, hostname: IP_ADDRESS_STUB) + url = parse_url(url) + url.hostname = hostname + url.to_s + end + + private + + def parse_url(url) + url.is_a?(URI) ? url : URI(url) + end +end