Merge remote-tracking branch 'dev/master'

CHANGELOG.md

@@ -2,6 +2,20 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 12.5.1
+
+### Security (8 changes)
+
+- Check permissions before showing a forked project's source.
+- Encrypt application setting tokens.
+- Update Workhorse and Gitaly to fix a security issue.
+- Hide commit counts from guest users in Cycle Analytics.
+- Limit potential for DNS rebind SSRF in chat notifications.
+- Ensure are cleaned by ImportExport::AttributeCleaner.
+- Remove notes regarding Related Branches from Issue activity feeds for guest users.
+- Escape namespace in label references to prevent XSS.
+
+
 ## 12.5.0
 
 ### Security (15 changes)
@@ -353,6 +367,21 @@ entry.
 - Change selects from default browser style to custom style.
 
 
+## 12.4.4
+
+### Security (9 changes)
+
+- Check permissions before showing a forked project's source.
+- Encrypt application setting tokens.
+- Update Workhorse and Gitaly to fix a security issue.
+- Hide commit counts from guest users in Cycle Analytics.
+- Limit potential for DNS rebind SSRF in chat notifications.
+- Fix 500 error caused by invalid byte sequences in links.
+- Ensure are cleaned by ImportExport::AttributeCleaner.
+- Remove notes regarding Related Branches from Issue activity feeds for guest users.
+- Escape namespace in label references to prevent XSS.
+
+
 ## 12.4.3
 
 ### Fixed (2 changes)
@@ -721,6 +750,21 @@ entry.
 - Remove Postgresql specific setup tasks and move to schema.rb.
 
 
+## 12.3.7
+
+### Security (9 changes)
+
+- Check permissions before showing a forked project's source.
+- Encrypt application setting tokens.
+- Update Workhorse and Gitaly to fix a security issue.
+- Hide commit counts from guest users in Cycle Analytics.
+- Limit potential for DNS rebind SSRF in chat notifications.
+- Fix 500 error caused by invalid byte sequences in links.
+- Ensure are cleaned by ImportExport::AttributeCleaner.
+- Remove notes regarding Related Branches from Issue activity feeds for guest users.
+- Escape namespace in label references to prevent XSS.
+
+
 ## 12.3.4
 
 ### Fixed (2 changes)

GITALY_SERVER_VERSION

-1.74.0
+1.72.1

GITLAB_WORKHORSE_VERSION

-8.14.0
+8.14.1

app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js

@@ -58,10 +58,16 @@ export default () => {
         service: this.createCycleAnalyticsService(cycleAnalyticsEl.dataset.requestPath),
       };
     },
+    defaultNumberOfSummaryItems: 3,
     computed: {
       currentStage() {
         return this.store.currentActiveStage();
       },
+      summaryTableColumnClass() {
+        return this.state.summary.length === this.$options.defaultNumberOfSummaryItems
+          ? 'col-sm-3'
+          : 'col-sm-4';
+      },
     },
     created() {
       // Conditional check placed here to prevent this method from being called on the

app/controllers/admin/application_settings_controller.rb

@@ -86,6 +86,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
 
     params[:application_setting][:import_sources]&.delete("")
     params[:application_setting][:restricted_visibility_levels]&.delete("")
+    params[:application_setting].delete(:elasticsearch_aws_secret_access_key) if params[:application_setting][:elasticsearch_aws_secret_access_key].blank?
     # TODO Remove domain_blacklist_raw in APIv5 (See https://gitlab.com/gitlab-org/gitlab-foss/issues/67204)
     params.delete(:domain_blacklist_raw) if params[:domain_blacklist_file]
     params.delete(:domain_blacklist_raw) if params[:domain_blacklist]

app/helpers/projects_helper.rb

@@ -110,17 +110,24 @@ module ProjectsHelper
       { project_full_name: project.full_name }
   end
 
-  def remove_fork_project_message(project)
-    _("You are going to remove the fork relationship to source project %{forked_from_project}. Are you ABSOLUTELY sure?") %
-      { forked_from_project: fork_source_name(project) }
-  end
+  def remove_fork_project_description_message(project)
+    source = visible_fork_source(project)
 
-  def fork_source_name(project)
-    if @project.fork_source
-      @project.fork_source.full_name
+    if source
+      _('This will remove the fork relationship between this project and %{fork_source}.') %
+        { fork_source: link_to(source.full_name, project_path(source)) }
     else
-      @project.fork_network&.deleted_root_project_name
+      _('This will remove the fork relationship between this project and other projects in the fork network.')
+    end
+  end
+
+  def remove_fork_project_warning_message(project)
+    _("You are going to remove the fork relationship from %{project_full_name}. Are you ABSOLUTELY sure?") %
+      { project_full_name: project.full_name }
   end
+
+  def visible_fork_source(project)
+    project.fork_source if project.fork_source && can?(current_user, :read_project, project.fork_source)
   end
 
   def project_nav_tabs

app/models/application_setting.rb

@@ -313,29 +313,25 @@ class ApplicationSetting < ApplicationRecord
                  algorithm: 'aes-256-cbc',
                  insecure_mode: true
 
-  attr_encrypted :external_auth_client_key,
-                 mode: :per_attribute_iv,
-                 key: Settings.attr_encrypted_db_key_base_truncated,
-                 algorithm: 'aes-256-gcm',
-                 encode: true
-
-  attr_encrypted :external_auth_client_key_pass,
-                 mode: :per_attribute_iv,
-                 key: Settings.attr_encrypted_db_key_base_truncated,
-                 algorithm: 'aes-256-gcm',
-                 encode: true
-
-  attr_encrypted :lets_encrypt_private_key,
+  private_class_method def self.encryption_options_base_truncated_aes_256_gcm
+    {
       mode: :per_attribute_iv,
       key: Settings.attr_encrypted_db_key_base_truncated,
       algorithm: 'aes-256-gcm',
       encode: true
+    }
+  end
 
-  attr_encrypted :eks_secret_access_key,
-                 mode: :per_attribute_iv,
-                 key: Settings.attr_encrypted_db_key_base_truncated,
-                 algorithm: 'aes-256-gcm',
-                 encode: true
+  attr_encrypted :external_auth_client_key, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :external_auth_client_key_pass, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :lets_encrypt_private_key, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :eks_secret_access_key, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :akismet_api_key, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :elasticsearch_aws_secret_access_key, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :recaptcha_private_key, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :recaptcha_site_key, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :slack_app_secret, encryption_options_base_truncated_aes_256_gcm
+  attr_encrypted :slack_app_verification_token, encryption_options_base_truncated_aes_256_gcm
 
   before_validation :ensure_uuid!
 

app/models/note.rb

@@ -37,6 +37,10 @@ class Note < ApplicationRecord
 
   redact_field :note
 
+  TYPES_RESTRICTED_BY_ABILITY = {
+    branch: :download_code
+  }.freeze
+
   # Aliases to make application_helper#edited_time_ago_with_tooltip helper work properly with notes.
   # See https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/10392/diffs#note_28719102
   alias_attribute :last_edited_at, :updated_at
@@ -341,7 +345,7 @@ class Note < ApplicationRecord
   end
 
   def visible_for?(user)
-    !cross_reference_not_visible_for?(user)
+    !cross_reference_not_visible_for?(user) && system_note_viewable_by?(user)
   end
 
   def award_emoji?
@@ -497,6 +501,15 @@ class Note < ApplicationRecord
 
   private
 
+  def system_note_viewable_by?(user)
+    return true unless system_note_metadata
+
+    restriction = TYPES_RESTRICTED_BY_ABILITY[system_note_metadata.action.to_sym]
+    return Ability.allowed?(user, restriction, project) if restriction
+
+    true
+  end
+
   def keep_around_commit
     project.repository.keep_around(self.commit_id)
   end

app/models/project.rb

@@ -512,7 +512,11 @@ class Project < ApplicationRecord
 
   # This scope returns projects where user has access to both the project and the feature.
   def self.filter_by_feature_visibility(feature, user)
-    with_feature_available_for_user(feature, user).public_or_visible_to_user(user)
+    with_feature_available_for_user(feature, user)
+      .public_or_visible_to_user(
+        user,
+        ProjectFeature.required_minimum_access_level_for_private_project(feature)
+      )
   end
 
   scope :active, -> { joins(:issues, :notes, :merge_requests).order('issues.created_at, notes.created_at, merge_requests.created_at DESC') }

app/models/project_feature.rb

@@ -24,6 +24,7 @@ class ProjectFeature < ApplicationRecord
 
   FEATURES = %i(issues merge_requests wiki snippets builds repository pages).freeze
   PRIVATE_FEATURES_MIN_ACCESS_LEVEL = { merge_requests: Gitlab::Access::REPORTER }.freeze
+  PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT = { repository: Gitlab::Access::REPORTER }.freeze
   STRING_OPTIONS = HashWithIndifferentAccess.new({
     'disabled' => DISABLED,
     'private'  => PRIVATE,
@@ -51,6 +52,15 @@ class ProjectFeature < ApplicationRecord
       PRIVATE_FEATURES_MIN_ACCESS_LEVEL.fetch(feature, Gitlab::Access::GUEST)
     end
 
+    # Guest users can perform certain features on public and internal projects, but not private projects.
+    def required_minimum_access_level_for_private_project(feature)
+      feature = ensure_feature!(feature)
+
+      PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT.fetch(feature) do
+        required_minimum_access_level(feature)
+      end
+    end
+
     def access_level_from_str(level)
       STRING_OPTIONS.fetch(level)
     end

app/models/project_services/chat_notification_service.rb

@@ -113,12 +113,9 @@ class ChatNotificationService < Service
 
   private
 
+  # every notifier must implement this independently
   def notify(message, opts)
-    Slack::Notifier.new(webhook, opts).ping(
-      message.pretext,
-      attachments: message.attachments,
-      fallback: message.fallback
-    )
+    raise NotImplementedError
   end
 
   def custom_data(data)

app/models/project_services/mattermost_service.rb

 # frozen_string_literal: true
 
 class MattermostService < ChatNotificationService
+  include ::SlackService::Notifier
+
   def title
     'Mattermost notifications'
   end

app/models/project_services/slack_service.rb

@@ -30,4 +30,28 @@ class SlackService < ChatNotificationService
   def webhook_placeholder
     'https://hooks.slack.com/services/…'
   end
+
+  module Notifier
+    private
+
+    def notify(message, opts)
+      # See https://github.com/stevenosloan/slack-notifier#custom-http-client
+      notifier = Slack::Notifier.new(webhook, opts.merge(http_client: HTTPClient))
+
+      notifier.ping(
+        message.pretext,
+        attachments: message.attachments,
+        fallback: message.fallback
+      )
+    end
+
+    class HTTPClient
+      def self.post(uri, params = {})
+        params.delete(:http_options) # these are internal to the client and we do not want them
+        Gitlab::HTTP.post(uri, body: params)
+      end
+    end
+  end
+
+  include Notifier
 end

app/views/projects/_home_panel.html.haml

@@ -74,13 +74,12 @@
 
     - if @project.forked?
       %p
-        - if @project.fork_source
+        - source = visible_fork_source(@project)
+        - if source
           #{ s_('ForkedFromProjectPath|Forked from') }
-          = link_to project_path(@project.fork_source) do
-            = fork_source_name(@project)
+          = link_to source.full_name, project_path(source)
         - else
-          - deleted_message = s_('ForkedFromProjectPath|Forked from %{project_name} (deleted)')
-          = deleted_message % { project_name: fork_source_name(@project) }
+          = s_('ForkedFromProjectPath|Forked from an inaccessible project')
 
     = render_if_exists "projects/home_mirror"
 

app/views/projects/cycle_analytics/show.html.haml

@@ -13,10 +13,10 @@
       .content-block
         .container-fluid
           .row
-            .col-sm-3.col-12.column{ "v-for" => "item in state.summary" }
+            .col-12.column{ "v-for" => "item in state.summary", ":class" => "summaryTableColumnClass" }
               %h3.header {{ item.value }}
               %p.text {{ item.title }}
-            .col-sm-3.col-12.column
+            .col-12.column{ ":class" => "summaryTableColumnClass" }
               .dropdown.inline.js-ca-dropdown
                 %button.dropdown-menu-toggle{ "data-toggle" => "dropdown", :type => "button" }
                   %span.dropdown-label {{ n__('Last %d day', 'Last %d days', 30) }}

app/views/projects/edit.html.haml

@@ -126,17 +126,12 @@
     - if @project.forked? && can?(current_user, :remove_fork_project, @project)
       .sub-section
         %h4.danger-title= _('Remove fork relationship')
-        %p
-          = _('This will remove the fork relationship to source project')
-          = succeed "." do
-            - if @project.fork_source
-              = link_to(fork_source_name(@project), project_path(@project.fork_source))
-            - else
-              = fork_source_name(@project)
+        %p= remove_fork_project_description_message(@project)
+
         = form_for([@project.namespace.becomes(Namespace), @project], url: remove_fork_project_path(@project), method: :delete, remote: true, html: { class: 'transfer-project' }) do |f|
           %p
             %strong= _('Once removed, the fork relationship cannot be restored and you will no longer be able to send merge requests to the source.')
-          = button_to _('Remove fork relationship'), '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_fork_project_message(@project) }
+          = button_to _('Remove fork relationship'), '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_fork_project_warning_message(@project) }
 
     - if can?(current_user, :remove_project, @project)
       .sub-section

changelogs/unreleased/security-29660-update-dependencies.yml

+---
+title: Update Workhorse and Gitaly to fix a security issue
+merge_request:
+author:
+type: security

config/initializers/hangouts_chat_http_override.rb

+# frozen_string_literal: true
+
+module HangoutsChat
+  class Sender
+    class HTTP
+      module GitlabHTTPOverride
+        extend ::Gitlab::Utils::Override
+
+        attr_reader :uri
+
+        # see https://github.com/enzinia/hangouts-chat/blob/6a509f61a56e757f8f417578b393b94423831ff7/lib/hangouts_chat/http.rb
+        override :post
+        def post(payload)
+          httparty_response = Gitlab::HTTP.post(
+            uri,
+            body: payload.to_json,
+            headers: { 'Content-Type' => 'application/json' },
+            parse: nil # disables automatic response parsing
+          )
+          net_http_response = httparty_response.response
+          # The rest of the integration expects a Net::HTTP response
+          net_http_response
+        end
+      end
+
+      prepend GitlabHTTPOverride
+    end
+  end
+end

db/migrate/20191120084627_add_encrypted_fields_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddEncryptedFieldsToApplicationSettings < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  PLAINTEXT_ATTRIBUTES = %w[
+    akismet_api_key
+    elasticsearch_aws_secret_access_key
+    recaptcha_private_key
+    recaptcha_site_key
+    slack_app_secret
+    slack_app_verification_token
+  ].freeze
+
+  def up
+    PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+      add_column :application_settings, "encrypted_#{plaintext_attribute}", :text
+      add_column :application_settings, "encrypted_#{plaintext_attribute}_iv", :string, limit: 255
+    end
+  end
+
+  def down
+    PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+      remove_column :application_settings, "encrypted_#{plaintext_attribute}"
+      remove_column :application_settings, "encrypted_#{plaintext_attribute}_iv"
+    end
+  end
+end

db/migrate/20191120115530_encrypt_plaintext_attributes_on_application_settings.rb

+# frozen_string_literal: true
+
+class EncryptPlaintextAttributesOnApplicationSettings < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  PLAINTEXT_ATTRIBUTES = %w[
+    akismet_api_key
+    elasticsearch_aws_secret_access_key
+    recaptcha_private_key
+    recaptcha_site_key
+    slack_app_secret
+    slack_app_verification_token
+  ].freeze
+
+  class ApplicationSetting < ActiveRecord::Base
+    self.table_name = 'application_settings'
+
+    def self.encryption_options_base_truncated_aes_256_gcm
+      {
+        mode: :per_attribute_iv,
+        key: Gitlab::Application.secrets.db_key_base[0..31],
+        algorithm: 'aes-256-gcm',
+        encode: true
+      }
+    end
+
+    attr_encrypted :akismet_api_key, encryption_options_base_truncated_aes_256_gcm
+    attr_encrypted :elasticsearch_aws_secret_access_key, encryption_options_base_truncated_aes_256_gcm
+    attr_encrypted :recaptcha_private_key, encryption_options_base_truncated_aes_256_gcm
+    attr_encrypted :recaptcha_site_key, encryption_options_base_truncated_aes_256_gcm
+    attr_encrypted :slack_app_secret, encryption_options_base_truncated_aes_256_gcm
+    attr_encrypted :slack_app_verification_token, encryption_options_base_truncated_aes_256_gcm
+
+    def akismet_api_key
+      decrypt(:akismet_api_key, self[:encrypted_akismet_api_key]) || self[:akismet_api_key]
+    end
+
+    def elasticsearch_aws_secret_access_key
+      decrypt(:elasticsearch_aws_secret_access_key, self[:encrypted_elasticsearch_aws_secret_access_key]) || self[:elasticsearch_aws_secret_access_key]
+    end
+
+    def recaptcha_private_key
+      decrypt(:recaptcha_private_key, self[:encrypted_recaptcha_private_key]) || self[:recaptcha_private_key]
+    end
+
+    def recaptcha_site_key
+      decrypt(:recaptcha_site_key, self[:encrypted_recaptcha_site_key]) || self[:recaptcha_site_key]
+    end
+
+    def slack_app_secret
+      decrypt(:slack_app_secret, self[:encrypted_slack_app_secret]) || self[:slack_app_secret]
+    end
+
+    def slack_app_verification_token
+      decrypt(:slack_app_verification_token, self[:encrypted_slack_app_verification_token]) || self[:slack_app_verification_token]
+    end
+  end
+
+  def up
+    ApplicationSetting.find_each do |application_setting|
+      # We are using the setter from attr_encrypted gem to encrypt the data.
+      # The gem updates the two columns needed to decrypt the value:
+      # - "encrypted_#{plaintext_attribute}"
+      # - "encrypted_#{plaintext_attribute}_iv"
+      application_setting.assign_attributes(
+        PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes|
+          attributes[plaintext_attribute] = application_setting.send(plaintext_attribute)
+        end
+      )
+      application_setting.save(validate: false)
+    end
+  end
+
+  def down
+    ApplicationSetting.find_each do |application_setting|
+      application_setting.update_columns(
+        PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes|
+          attributes[plaintext_attribute] = application_setting.send(plaintext_attribute)
+          attributes["encrypted_#{plaintext_attribute}"] = nil
+          attributes["encrypted_#{plaintext_attribute}_iv"] = nil
+        end
+      )
+    end
+  end
+end

db/post_migrate/20191122135327_remove_plaintext_columns_from_application_settings.rb

+# frozen_string_literal: true
+
+class RemovePlaintextColumnsFromApplicationSettings < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  PLAINTEXT_ATTRIBUTES = %w[
+    akismet_api_key
+    elasticsearch_aws_secret_access_key
+    recaptcha_private_key
+    recaptcha_site_key
+    slack_app_secret
+    slack_app_verification_token
+  ].freeze
+
+  def up
+    PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+      remove_column :application_settings, plaintext_attribute
+    end
+  end
+
+  def down
+    PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+      add_column :application_settings, plaintext_attribute, :text
+    end
+  end
+end

db/schema.rb

@@ -180,11 +180,8 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do
     t.integer "metrics_timeout", default: 10
     t.integer "metrics_method_call_threshold", default: 10
     t.boolean "recaptcha_enabled", default: false
-    t.string "recaptcha_site_key"
-    t.string "recaptcha_private_key"
     t.integer "metrics_port", default: 8089
     t.boolean "akismet_enabled", default: false
-    t.string "akismet_api_key"
     t.integer "metrics_sample_interval", default: 15
     t.boolean "email_author_in_body", default: false
     t.integer "default_group_visibility"
@@ -231,7 +228,6 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do
     t.boolean "elasticsearch_aws", default: false, null: false
     t.string "elasticsearch_aws_region", default: "us-east-1"
     t.string "elasticsearch_aws_access_key"
-    t.string "elasticsearch_aws_secret_access_key"
     t.integer "geo_status_timeout", default: 10
     t.string "uuid"
     t.decimal "polling_interval_multiplier", default: "1.0", null: false
@@ -247,8 +243,6 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do
     t.string "help_page_support_url"
     t.boolean "slack_app_enabled", default: false
     t.string "slack_app_id"
-    t.string "slack_app_secret"
-    t.string "slack_app_verification_token"
     t.integer "performance_bar_allowed_group_id"
     t.boolean "allow_group_owners_to_manage_ldap", default: true, null: false
     t.boolean "hashed_storage_enabled", default: true, null: false
@@ -355,6 +349,18 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do
     t.boolean "sourcegraph_enabled", default: false, null: false
     t.string "sourcegraph_url", limit: 255
     t.boolean "sourcegraph_public_only", default: true, null: false
+    t.text "encrypted_akismet_api_key"
+    t.string "encrypted_akismet_api_key_iv", limit: 255
+    t.text "encrypted_elasticsearch_aws_secret_access_key"
+    t.string "encrypted_elasticsearch_aws_secret_access_key_iv", limit: 255
+    t.text "encrypted_recaptcha_private_key"
+    t.string "encrypted_recaptcha_private_key_iv", limit: 255
+    t.text "encrypted_recaptcha_site_key"
+    t.string "encrypted_recaptcha_site_key_iv", limit: 255
+    t.text "encrypted_slack_app_secret"
+    t.string "encrypted_slack_app_secret_iv", limit: 255
+    t.text "encrypted_slack_app_verification_token"
+    t.string "encrypted_slack_app_verification_token_iv", limit: 255
     t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
     t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
     t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id"

lib/api/entities.rb

@@ -283,7 +283,9 @@ module API
       expose :shared_runners_enabled
       expose :lfs_enabled?, as: :lfs_enabled
       expose :creator_id
-      expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? }
+      expose :forked_from_project, using: Entities::BasicProjectDetails, if: ->(project, options) do
+        project.forked? && Ability.allowed?(options[:current_user], :read_project, project.forked_from_project)
+      end
       expose :import_status
 
       expose :import_error, if: lambda { |_project, options| options[:user_can_admin_project] } do |project|

lib/banzai/filter/label_reference_filter.rb

@@ -89,7 +89,7 @@ module Banzai
           parent_from_ref = from_ref_cached(project_path)
           reference       = parent_from_ref.to_human_reference(parent)
 
-          label_suffix = " <i>in #{reference}</i>" if reference.present?
+          label_suffix = " <i>in #{ERB::Util.html_escape(reference)}</i>" if reference.present?
         end
 
         presenter = object.present(issuable_subject: parent)

lib/banzai/filter/relative_link_filter.rb

@@ -172,7 +172,7 @@ module Banzai
       end
 
       def cleaned_file_path(uri)
-        Addressable::URI.unescape(uri.path).delete("\0").chomp("/")
+        Addressable::URI.unescape(uri.path).scrub.delete("\0").chomp("/")
       end
 
       def relative_file_path(uri)

lib/gitlab/cycle_analytics/stage_summary.rb

@@ -11,13 +11,29 @@ module Gitlab
       end
 
       def data
-        [serialize(Summary::Issue.new(project: @project, from: @from, to: @to, current_user: @current_user)),
-         serialize(Summary::Commit.new(project: @project, from: @from, to: @to)),
-         serialize(Summary::Deploy.new(project: @project, from: @from, to: @to))]
+        summary = [issue_stats]
+        summary << commit_stats if user_has_sufficient_access?
+        summary << deploy_stats
       end
 
       private
 
+      def issue_stats
+        serialize(Summary::Issue.new(project: @project, from: @from, to: @to, current_user: @current_user))
+      end
+
+      def commit_stats
+        serialize(Summary::Commit.new(project: @project, from: @from, to: @to))
+      end
+
+      def deploy_stats
+        serialize(Summary::Deploy.new(project: @project, from: @from, to: @to))
+      end
+
+      def user_has_sufficient_access?
+        @project.team.member?(@current_user, Gitlab::Access::REPORTER)
+      end
+
       def serialize(summary_object)
         AnalyticsSummarySerializer.new.represent(summary_object)
       end

lib/gitlab/import_export/attribute_cleaner.rb

@@ -4,7 +4,7 @@ module Gitlab
   module ImportExport
     class AttributeCleaner
       ALLOWED_REFERENCES = RelationFactory::PROJECT_REFERENCES + RelationFactory::USER_REFERENCES + %w[group_id commit_id]
-      PROHIBITED_REFERENCES = Regexp.union(/\Acached_markdown_version\Z/, /_id\Z/, /_html\Z/).freeze
+      PROHIBITED_REFERENCES = Regexp.union(/\Acached_markdown_version\Z/, /_id\Z/, /_ids\Z/, /_html\Z/).freeze
 
       def self.clean(*args)
         new(*args).clean

lib/microsoft_teams/notifier.rb

@@ -14,7 +14,6 @@ module MicrosoftTeams
         response = Gitlab::HTTP.post(
           @webhook.to_str,
           headers: @header,
-          allow_local_requests: true,
           body: body(options)
         )
 

locale/gitlab.pot

@@ -7705,7 +7705,7 @@ msgstr ""
 msgid "ForkedFromProjectPath|Forked from"
 msgstr ""
 
-msgid "ForkedFromProjectPath|Forked from %{project_name} (deleted)"
+msgid "ForkedFromProjectPath|Forked from an inaccessible project"
 msgstr ""
 
 msgid "Forking in progress"
@@ -17933,7 +17933,10 @@ msgstr ""
 msgid "This will redirect you to an external sign in page."
 msgstr ""
 
-msgid "This will remove the fork relationship to source project"
+msgid "This will remove the fork relationship between this project and %{fork_source}."
+msgstr ""
+
+msgid "This will remove the fork relationship between this project and other projects in the fork network."
 msgstr ""
 
 msgid "Those emails automatically become issues (with the comments becoming the email conversation) listed here."
@@ -19852,7 +19855,7 @@ msgstr ""
 msgid "You are going to remove %{project_full_name}. Removed project CANNOT be restored! Are you ABSOLUTELY sure?"
 msgstr ""
 
-msgid "You are going to remove the fork relationship to source project %{forked_from_project}. Are you ABSOLUTELY sure?"
+msgid "You are going to remove the fork relationship from %{project_full_name}. Are you ABSOLUTELY sure?"
 msgstr ""
 
 msgid "You are going to transfer %{project_full_name} to another owner. Are you ABSOLUTELY sure?"

spec/controllers/projects/issues_controller_spec.rb

@@ -1435,6 +1435,43 @@ describe Projects::IssuesController do
           expect { get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } }.not_to exceed_query_limit(control_count)
         end
       end
+
+      context 'private project' do
+        let!(:branch_note) { create(:discussion_note_on_issue, :system, noteable: issue, project: project) }
+        let!(:commit_note) { create(:discussion_note_on_issue, :system, noteable: issue, project: project) }
+        let!(:branch_note_meta) { create(:system_note_metadata, note: branch_note, action: "branch") }
+        let!(:commit_note_meta) { create(:system_note_metadata, note: commit_note, action: "commit") }
+
+        context 'user is allowed access' do
+          before do
+            project.add_user(user, :maintainer)
+          end
+
+          it 'displays all available notes' do
+            get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }
+
+            expect(json_response.length).to eq(3)
+          end
+        end
+
+        context 'user is a guest' do
+          let(:json_response_note_ids) do
+            json_response.collect { |discussion| discussion["notes"] }.flatten
+              .collect { |note| note["id"].to_i }
+          end
+
+          before do
+            project.add_guest(user)
+          end
+
+          it 'does not display notes w/type listed in TYPES_RESTRICTED_BY_ACCESS_LEVEL' do
+            get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }
+
+            expect(json_response.length).to eq(2)
+            expect(json_response_note_ids).not_to include(branch_note.id)
+          end
+        end
+      end
     end
   end
 

spec/features/cycle_analytics_spec.rb

@@ -112,6 +112,10 @@ describe 'Cycle Analytics', :js do
       wait_for_requests
     end
 
+    it 'does not show the commit stats' do
+      expect(page).to have_no_selector(:xpath, commits_counter_selector)
+    end
+
     it 'needs permissions to see restricted stages' do
       expect(find('.stage-events')).to have_content(issue.title)
 
@@ -127,8 +131,12 @@ describe 'Cycle Analytics', :js do
     find(:xpath, "//p[contains(text(),'New Issue')]/preceding-sibling::h3")
   end
 
+  def commits_counter_selector
+    "//p[contains(text(),'Commits')]/preceding-sibling::h3"
+  end
+
   def commits_counter
-    find(:xpath, "//p[contains(text(),'Commits')]/preceding-sibling::h3")
+    find(:xpath, commits_counter_selector)
   end
 
   def deploys_counter

spec/features/projects_spec.rb

@@ -202,13 +202,13 @@ describe 'Project' do
       expect(page).not_to have_content('Forked from')
     end
 
-    it 'shows the name of the deleted project when the source was deleted', :sidekiq_might_not_need_inline do
+    it 'does not show the name of the deleted project when the source was deleted', :sidekiq_might_not_need_inline do
       forked_project
       Projects::DestroyService.new(base_project, base_project.owner).execute
 
       visit project_path(forked_project)
 
-      expect(page).to have_content("Forked from #{base_project.full_name} (deleted)")
+      expect(page).to have_content('Forked from an inaccessible project')
     end
 
     context 'a fork of a fork' do

spec/initializers/hangouts_chat_http_override_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'HangoutsChat::Sender Gitlab::HTTP override' do
+  describe 'HangoutsChat::Sender::HTTP#post' do
+    it 'calls Gitlab::HTTP.post with default protection settings' do
+      webhook_url = 'https://example.gitlab.com'
+      payload = { key: 'value' }
+      http = HangoutsChat::Sender::HTTP.new(webhook_url)
+      mock_response = double(response: 'the response')
+
+      expect(Gitlab::HTTP).to receive(:post)
+        .with(
+          URI.parse(webhook_url),
+          body: payload.to_json,
+          headers: { 'Content-Type' => 'application/json' },
+          parse: nil
+        )
+        .and_return(mock_response)
+
+      expect(http.post(payload)).to eq(mock_response.response)
+    end
+
+    it_behaves_like 'a request using Gitlab::UrlBlocker' do
+      let(:http_method) { :post }
+      let(:url_blocked_error_class) { Gitlab::HTTP::BlockedUrlError }
+
+      def make_request(uri)
+        HangoutsChat::Sender::HTTP.new(uri).post({})
+      end
+    end
+  end
+end

spec/initializers/rest-client-hostname_override_spec.rb

@@ -3,147 +3,12 @@
 require 'spec_helper'
 
 describe 'rest-client dns rebinding protection' do
-  include StubRequests
+  it_behaves_like 'a request using Gitlab::UrlBlocker' do
+    let(:http_method) { :get }
+    let(:url_blocked_error_class) { ArgumentError }
 
-  context 'when local requests are not allowed' do
-    it 'allows an external request with http' do
-      request_stub = stub_full_request('http://example.com', ip_address: '93.184.216.34')
-
-      RestClient.get('http://example.com/')
-
-      expect(request_stub).to have_been_requested
-    end
-
-    it 'allows an external request with https' do
-      request_stub = stub_full_request('https://example.com', ip_address: '93.184.216.34')
-
-      RestClient.get('https://example.com/')
-
-      expect(request_stub).to have_been_requested
-    end
-
-    it 'raises error when it is a request that resolves to a local address' do
-      stub_full_request('https://example.com', ip_address: '172.16.0.0')
-
-      expect { RestClient.get('https://example.com') }
-        .to raise_error(ArgumentError,
-                        "URL 'https://example.com' is blocked: Requests to the local network are not allowed")
-    end
-
-    it 'raises error when it is a request that resolves to a localhost address' do
-      stub_full_request('https://example.com', ip_address: '127.0.0.1')
-
-      expect { RestClient.get('https://example.com') }
-        .to raise_error(ArgumentError,
-                        "URL 'https://example.com' is blocked: Requests to localhost are not allowed")
-    end
-
-    it 'raises error when it is a request to local address' do
-      expect { RestClient.get('http://172.16.0.0') }
-        .to raise_error(ArgumentError,
-                        "URL 'http://172.16.0.0' is blocked: Requests to the local network are not allowed")
-    end
-
-    it 'raises error when it is a request to localhost address' do
-      expect { RestClient.get('http://127.0.0.1') }
-        .to raise_error(ArgumentError,
-                        "URL 'http://127.0.0.1' is blocked: Requests to localhost are not allowed")
-    end
-  end
-
-  context 'when port different from URL scheme is used' do
-    it 'allows the request' do
-      request_stub = stub_full_request('https://example.com:8080', ip_address: '93.184.216.34')
-
-      RestClient.get('https://example.com:8080/')
-
-      expect(request_stub).to have_been_requested
-    end
-
-    it 'raises error when it is a request to local address' do
-      expect { RestClient.get('https://172.16.0.0:8080') }
-        .to raise_error(ArgumentError,
-                        "URL 'https://172.16.0.0:8080' is blocked: Requests to the local network are not allowed")
-    end
-
-    it 'raises error when it is a request to localhost address' do
-      expect { RestClient.get('https://127.0.0.1:8080') }
-        .to raise_error(ArgumentError,
-                        "URL 'https://127.0.0.1:8080' is blocked: Requests to localhost are not allowed")
-    end
-  end
-
-  context 'when DNS rebinding protection is disabled' do
-    before do
-      stub_application_setting(dns_rebinding_protection_enabled: false)
-    end
-
-    it 'allows the request' do
-      request_stub = stub_request(:get, 'https://example.com')
-
-      RestClient.get('https://example.com/')
-
-      expect(request_stub).to have_been_requested
-    end
-  end
-
-  context 'when http(s) proxy environment variable is set' do
-    before do
-      stub_env('https_proxy' => 'https://my.proxy')
-    end
-
-    it 'allows the request' do
-      request_stub = stub_request(:get, 'https://example.com')
-
-      RestClient.get('https://example.com/')
-
-      expect(request_stub).to have_been_requested
-    end
-  end
-
-  context 'when local requests are allowed' do
-    before do
-      stub_application_setting(allow_local_requests_from_web_hooks_and_services: true)
-    end
-
-    it 'allows an external request' do
-      request_stub = stub_full_request('https://example.com', ip_address: '93.184.216.34')
-
-      RestClient.get('https://example.com/')
-
-      expect(request_stub).to have_been_requested
-    end
-
-    it 'allows an external request that resolves to a local address' do
-      request_stub = stub_full_request('https://example.com', ip_address: '172.16.0.0')
-
-      RestClient.get('https://example.com/')
-
-      expect(request_stub).to have_been_requested
-    end
-
-    it 'allows an external request that resolves to a localhost address' do
-      request_stub = stub_full_request('https://example.com', ip_address: '127.0.0.1')
-
-      RestClient.get('https://example.com/')
-
-      expect(request_stub).to have_been_requested
-    end
-
-    it 'allows a local address request' do
-      request_stub = stub_request(:get, 'http://172.16.0.0')
-
-      RestClient.get('http://172.16.0.0')
-
-      expect(request_stub).to have_been_requested
-    end
-
-    it 'allows a localhost address request' do
-      request_stub = stub_request(:get, 'http://127.0.0.1')
-
-      RestClient.get('http://127.0.0.1')
-
-      expect(request_stub).to have_been_requested
+    def make_request(uri)
+      RestClient.get(uri)
     end
   end
 end

spec/lib/banzai/filter/label_reference_filter_spec.rb

@@ -521,6 +521,15 @@ describe Banzai::Filter::LabelReferenceFilter do
 
       expect(reference_filter(act).to_html).to eq exp
     end
+
+    context 'when group name has HTML entities' do
+      let(:another_group) { create(:group, name: '<img src=x onerror=alert(1)>', path: 'another_group') }
+
+      it 'escapes the HTML entities' do
+        expect(result.text)
+          .to eq "See #{group_label.name} in #{another_project.full_name}"
+      end
+    end
   end
 
   describe 'cross-project / same-group_label complete reference' do

spec/lib/banzai/filter/relative_link_filter_spec.rb

@@ -119,6 +119,11 @@ describe Banzai::Filter::RelativeLinkFilter do
     expect { filter(act) }.not_to raise_error
   end
 
+  it 'does not raise an exception on URIs containing invalid utf-8 byte sequences' do
+    act = link("%FF")
+    expect { filter(act) }.not_to raise_error
+  end
+
   it 'does not raise an exception with a garbled path' do
     act = link("open(/var/tmp/):%20/location%0Afrom:%20/test")
     expect { filter(act) }.not_to raise_error

spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb

@@ -6,6 +6,11 @@ describe Gitlab::CycleAnalytics::StageSummary do
   let(:project) { create(:project, :repository) }
   let(:options) { { from: 1.day.ago, current_user: user } }
   let(:user) { create(:user, :admin) }
+
+  before do
+    project.add_maintainer(user)
+  end
+
   let(:stage_summary) { described_class.new(project, options).data }
 
   describe "#new_issues" do
@@ -86,6 +91,24 @@ describe Gitlab::CycleAnalytics::StageSummary do
         expect(subject).to eq(2)
       end
     end
+
+    context 'when a guest user is signed in' do
+      let(:guest_user) { create(:user) }
+
+      before do
+        project.add_guest(guest_user)
+        options.merge!({ current_user: guest_user })
+      end
+
+      it 'does not include commit stats' do
+        data = described_class.new(project, options).data
+        expect(includes_commits?(data)).to be_falsy
+      end
+
+      def includes_commits?(data)
+        data.any? { |h| h["title"] == 'Commits' }
+      end
+    end
   end
 
   describe "#deploys" do

spec/lib/gitlab/import_export/attribute_cleaner_spec.rb

@@ -26,7 +26,10 @@ describe Gitlab::ImportExport::AttributeCleaner do
       '_html' => '<p>perfectly ordinary html</p>',
       'cached_markdown_version' => 12345,
       'group_id' => 99,
-      'commit_id' => 99
+      'commit_id' => 99,
+      'issue_ids' => [1, 2, 3],
+      'merge_request_ids' => [1, 2, 3],
+      'note_ids' => [1, 2, 3]
     }
   end
 

spec/migrations/encrypt_plaintext_attributes_on_application_settings_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20191120115530_encrypt_plaintext_attributes_on_application_settings.rb')
+
+describe EncryptPlaintextAttributesOnApplicationSettings, :migration do
+  let(:migration) { described_class.new }
+  let(:application_settings) { table(:application_settings) }
+  let(:plaintext) { 'secret-token' }
+
+  PLAINTEXT_ATTRIBUTES = %w[
+    akismet_api_key
+    elasticsearch_aws_secret_access_key
+    recaptcha_private_key
+    recaptcha_site_key
+    slack_app_secret
+    slack_app_verification_token
+  ].freeze
+
+  describe '#up' do
+    it 'encrypts token and saves it' do
+      application_setting = application_settings.create
+      application_setting.update_columns(
+        PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes|
+          attributes[plaintext_attribute] = plaintext
+        end
+      )
+
+      migration.up
+
+      application_setting.reload
+      PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+        expect(application_setting[plaintext_attribute]).not_to be_nil
+        expect(application_setting["encrypted_#{plaintext_attribute}"]).not_to be_nil
+        expect(application_setting["encrypted_#{plaintext_attribute}_iv"]).not_to be_nil
+      end
+    end
+  end
+
+  describe '#down' do
+    it 'decrypts encrypted token and saves it' do
+      application_setting = application_settings.create(
+        PLAINTEXT_ATTRIBUTES.each_with_object({}) do |plaintext_attribute, attributes|
+          attributes[plaintext_attribute] = plaintext
+        end
+      )
+
+      migration.down
+
+      application_setting.reload
+      PLAINTEXT_ATTRIBUTES.each do |plaintext_attribute|
+        expect(application_setting[plaintext_attribute]).to eq(plaintext)
+        expect(application_setting["encrypted_#{plaintext_attribute}"]).to be_nil
+        expect(application_setting["encrypted_#{plaintext_attribute}_iv"]).to be_nil
+      end
+    end
+  end
+end

spec/models/note_spec.rb

@@ -285,6 +285,70 @@ describe Note do
     end
   end
 
+  describe "#visible_for?" do
+    using RSpec::Parameterized::TableSyntax
+
+    let_it_be(:note) { create(:note) }
+    let_it_be(:user) { create(:user) }
+
+    where(:cross_reference_visible, :system_note_viewable, :result) do
+      true  | true  | false
+      false | true  | true
+      false | false | false
+    end
+
+    with_them do
+      it "returns expected result" do
+        expect(note).to receive(:cross_reference_not_visible_for?).and_return(cross_reference_visible)
+
+        unless cross_reference_visible
+          expect(note).to receive(:system_note_viewable_by?)
+            .with(user).and_return(system_note_viewable)
+        end
+
+        expect(note.visible_for?(user)).to eq result
+      end
+    end
+  end
+
+  describe "#system_note_viewable_by?(user)" do
+    let_it_be(:note) { create(:note) }
+    let_it_be(:user) { create(:user) }
+    let!(:metadata) { create(:system_note_metadata, note: note, action: "branch") }
+
+    context "when system_note_metadata is not present" do
+      it "returns true" do
+        expect(note).to receive(:system_note_metadata).and_return(nil)
+
+        expect(note.send(:system_note_viewable_by?, user)).to be_truthy
+      end
+    end
+
+    context "system_note_metadata isn't of type 'branch'" do
+      before do
+        metadata.action = "not_a_branch"
+      end
+
+      it "returns true" do
+        expect(note.send(:system_note_viewable_by?, user)).to be_truthy
+      end
+    end
+
+    context "user doesn't have :download_code ability" do
+      it "returns false" do
+        expect(note.send(:system_note_viewable_by?, user)).to be_falsey
+      end
+    end
+
+    context "user has the :download_code ability" do
+      it "returns true" do
+        expect(Ability).to receive(:allowed?).with(user, :download_code, note.project).and_return(true)
+
+        expect(note.send(:system_note_viewable_by?, user)).to be_truthy
+      end
+    end
+  end
+
   describe "cross_reference_not_visible_for?" do
     let(:private_user)    { create(:user) }
     let(:private_project) { create(:project, namespace: private_user.namespace) { |p| p.add_maintainer(private_user) } }

spec/models/project_feature_spec.rb

@@ -6,6 +6,16 @@ describe ProjectFeature do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
 
+  describe 'PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT' do
+    it 'has higher level than that of PRIVATE_FEATURES_MIN_ACCESS_LEVEL' do
+      described_class::PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT.each do |feature, level|
+        if generic_level = described_class::PRIVATE_FEATURES_MIN_ACCESS_LEVEL[feature]
+          expect(level).to be >= generic_level
+        end
+      end
+    end
+  end
+
   describe '.quoted_access_level_column' do
     it 'returns the table name and quoted column name for a feature' do
       expected = '"project_features"."issues_access_level"'
@@ -246,10 +256,24 @@ describe ProjectFeature do
       expect(described_class.required_minimum_access_level('merge_requests')).to eq(Gitlab::Access::REPORTER)
     end
 
+    it 'handles repository' do
+      expect(described_class.required_minimum_access_level(:repository)).to eq(Gitlab::Access::GUEST)
+    end
+
     it 'raises error if feature is invalid' do
       expect do
         described_class.required_minimum_access_level(:foos)
       end.to raise_error
     end
   end
+
+  describe '.required_minimum_access_level_for_private_project' do
+    it 'returns higher permission for repository' do
+      expect(described_class.required_minimum_access_level_for_private_project(:repository)).to eq(Gitlab::Access::REPORTER)
+    end
+
+    it 'returns normal permission for issues' do
+      expect(described_class.required_minimum_access_level_for_private_project(:issues)).to eq(Gitlab::Access::GUEST)
+    end
+  end
 end

spec/models/project_services/chat_notification_service_spec.rb

@@ -30,7 +30,8 @@ describe ChatNotificationService do
   end
 
   describe '#execute' do
-    let(:chat_service) { described_class.new }
+    subject(:chat_service) { described_class.new }
+
     let(:user) { create(:user) }
     let(:project) { create(:project, :repository) }
     let(:webhook_url) { 'https://example.gitlab.com/' }
@@ -53,12 +54,7 @@ describe ChatNotificationService do
         subject.project = project
         data = Gitlab::DataBuilder::Push.build_sample(project, user)
 
-        expect(Slack::Notifier).to receive(:new)
-         .with(webhook_url, {})
-         .and_return(
-           double(:slack_service).as_null_object
-         )
-
+        expect(chat_service).to receive(:notify).and_return(true)
         expect(chat_service.execute(data)).to be true
       end
     end
@@ -68,12 +64,7 @@ describe ChatNotificationService do
         subject.project = create(:project, :empty_repo)
         data = Gitlab::DataBuilder::Push.build_sample(subject.project, user)
 
-        expect(Slack::Notifier).to receive(:new)
-         .with(webhook_url, {})
-         .and_return(
-           double(:slack_service).as_null_object
-         )
-
+        expect(chat_service).to receive(:notify).and_return(true)
         expect(chat_service.execute(data)).to be true
       end
     end

spec/models/project_services/slack_service_spec.rb

@@ -3,5 +3,5 @@
 require 'spec_helper'
 
 describe SlackService do
-  it_behaves_like "slack or mattermost notifications", "Slack"
+  it_behaves_like "slack or mattermost notifications", 'Slack'
 end

spec/models/project_spec.rb

@@ -2924,7 +2924,7 @@ describe Project do
   end
 
   describe '#any_lfs_file_locks?', :request_store do
-    set(:project) { create(:project) }
+    let_it_be(:project) { create(:project) }
 
     it 'returns false when there are no LFS file locks' do
       expect(project.any_lfs_file_locks?).to be_falsey
@@ -3411,6 +3411,20 @@ describe Project do
         expect(projects).to eq([public_project])
       end
     end
+
+    context 'min_access_level' do
+      let!(:private_project) { create(:project, :private) }
+
+      before do
+        private_project.add_guest(user)
+      end
+
+      it 'excludes projects when user does not have required minimum access level' do
+        projects = described_class.all.public_or_visible_to_user(user, Gitlab::Access::REPORTER)
+
+        expect(projects).to contain_exactly(public_project)
+      end
+    end
   end
 
   describe '.ids_with_issuables_available_for' do
@@ -3539,7 +3553,7 @@ describe Project do
     include ProjectHelpers
     using RSpec::Parameterized::TableSyntax
 
-    set(:group) { create(:group) }
+    let_it_be(:group) { create(:group) }
     let!(:project) { create(:project, project_level, namespace: group ) }
     let(:user) { create_user_from_membership(project, membership) }
 
@@ -3562,6 +3576,66 @@ describe Project do
         end
       end
     end
+
+    context 'issues' do
+      let(:feature) { Issue }
+
+      where(:project_level, :feature_access_level, :membership, :expected_count) do
+        permission_table_for_guest_feature_access
+      end
+
+      with_them do
+        it "respects visibility" do
+          update_feature_access_level(project, feature_access_level)
+
+          expected_objects = expected_count == 1 ? [project] : []
+
+          expect(
+            described_class.filter_by_feature_visibility(feature, user)
+          ).to eq(expected_objects)
+        end
+      end
+    end
+
+    context 'wiki' do
+      let(:feature) { :wiki }
+
+      where(:project_level, :feature_access_level, :membership, :expected_count) do
+        permission_table_for_guest_feature_access
+      end
+
+      with_them do
+        it "respects visibility" do
+          update_feature_access_level(project, feature_access_level)
+
+          expected_objects = expected_count == 1 ? [project] : []
+
+          expect(
+            described_class.filter_by_feature_visibility(feature, user)
+          ).to eq(expected_objects)
+        end
+      end
+    end
+
+    context 'code' do
+      let(:feature) { :repository }
+
+      where(:project_level, :feature_access_level, :membership, :expected_count) do
+        permission_table_for_guest_feature_access_and_non_private_project_only
+      end
+
+      with_them do
+        it "respects visibility" do
+          update_feature_access_level(project, feature_access_level)
+
+          expected_objects = expected_count == 1 ? [project] : []
+
+          expect(
+            described_class.filter_by_feature_visibility(feature, user)
+          ).to eq(expected_objects)
+        end
+      end
+    end
   end
 
   describe '#pages_available?' do

spec/requests/api/projects_spec.rb

@@ -49,6 +49,8 @@ shared_examples 'languages and percentages JSON response' do
 end
 
 describe API::Projects do
+  include ProjectForksHelper
+
   let(:user) { create(:user) }
   let(:user2) { create(:user) }
   let(:user3) { create(:user) }
@@ -1163,6 +1165,18 @@ describe API::Projects do
         expect(json_response.keys).not_to include('permissions')
       end
 
+      context 'the project is a public fork' do
+        it 'hides details of a public fork parent' do
+          public_project = create(:project, :repository, :public)
+          fork = fork_project(public_project)
+
+          get api("/projects/#{fork.id}")
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response['forked_from_project']).to be_nil
+        end
+      end
+
       context 'and the project has a private repository' do
         let(:project) { create(:project, :repository, :public, :repository_private) }
         let(:protected_attributes) { %w(default_branch ci_config_path) }
@@ -1479,6 +1493,28 @@ describe API::Projects do
         end
       end
 
+      context 'the project is a fork' do
+        it 'shows details of a visible fork parent' do
+          fork = fork_project(project, user)
+
+          get api("/projects/#{fork.id}", user)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response['forked_from_project']).to include('id' => project.id)
+        end
+
+        it 'hides details of a hidden fork parent' do
+          fork = fork_project(project, user)
+          fork_user = create(:user)
+          fork.team.add_developer(fork_user)
+
+          get api("/projects/#{fork.id}", fork_user)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response['forked_from_project']).to be_nil
+        end
+      end
+
       describe 'permissions' do
         context 'all projects' do
           before do

spec/support/helpers/project_helpers.rb

@@ -10,16 +10,18 @@ module ProjectHelpers
       nil
     when :non_member
       create(:user, name: membership)
+    when :admin
+      create(:user, :admin, name: 'admin')
     else
       create(:user, name: membership).tap { |u| target.add_user(u, membership) }
     end
   end
 
   def update_feature_access_level(project, access_level)
-    project.update!(
-      repository_access_level: access_level,
-      merge_requests_access_level: access_level,
-      builds_access_level: access_level
-    )
+    features = ProjectFeature::FEATURES.dup
+    features.delete(:pages)
+    params = features.each_with_object({}) { |feature, h| h["#{feature}_access_level"] = access_level }
+
+    project.update!(params)
   end
 end

spec/support/helpers/search_helpers.rb

@@ -20,6 +20,12 @@ module SearchHelpers
     end
   end
 
+  def has_search_scope?(scope)
+    page.within '.search-filter' do
+      has_link?(scope)
+    end
+  end
+
   def max_limited_count
     Gitlab::SearchResults::COUNT_LIMIT_MESSAGE
   end

spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb

 # frozen_string_literal: true
 
 RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
+  include StubRequests
+
   let(:chat_service) { described_class.new }
-  let(:webhook_url) { 'https://example.gitlab.com/' }
+  let(:webhook_url) { 'https://example.gitlab.com' }
 
   def execute_with_options(options)
-    receive(:new).with(webhook_url, options)
+    receive(:new).with(webhook_url, options.merge(http_client: SlackService::Notifier::HTTPClient))
      .and_return(double(:slack_service).as_null_object)
   end
 
@@ -38,9 +40,13 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
       chat_service.branches_to_be_notified = branches_to_be_notified if branches_to_be_notified
     end
 
+    let!(:stubbed_resolved_hostname) do
+      stub_full_request(webhook_url, method: :post).request_pattern.uri_pattern.to_s
+    end
+
     it "notifies about #{event_type} events" do
       chat_service.execute(data)
-      expect(WebMock).to have_requested(:post, webhook_url)
+      expect(WebMock).to have_requested(:post, stubbed_resolved_hostname)
     end
   end
 
@@ -49,9 +55,13 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
       chat_service.branches_to_be_notified = branches_to_be_notified if branches_to_be_notified
     end
 
+    let!(:stubbed_resolved_hostname) do
+      stub_full_request(webhook_url, method: :post).request_pattern.uri_pattern.to_s
+    end
+
     it "notifies about #{event_type} events" do
       chat_service.execute(data)
-      expect(WebMock).not_to have_requested(:post, webhook_url)
+      expect(WebMock).not_to have_requested(:post, stubbed_resolved_hostname)
     end
   end
 
@@ -66,6 +76,10 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
       Gitlab::DataBuilder::Push.build_sample(project, user)
     end
 
+    let!(:stubbed_resolved_hostname) do
+      stub_full_request(webhook_url, method: :post).request_pattern.uri_pattern.to_s
+    end
+
     before do
       allow(chat_service).to receive_messages(
         project: project,
@@ -74,8 +88,6 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
         webhook: webhook_url
       )
 
-      WebMock.stub_request(:post, webhook_url)
-
       issue_service = Issues::CreateService.new(project, user, issue_service_options)
       @issue = issue_service.execute
       @issues_sample_data = issue_service.hook_data(@issue, 'open')
@@ -107,25 +119,25 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
     it "calls #{service_name} API for push events" do
       chat_service.execute(data)
 
-      expect(WebMock).to have_requested(:post, webhook_url).once
+      expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once
     end
 
     it "calls #{service_name} API for issue events" do
       chat_service.execute(@issues_sample_data)
 
-      expect(WebMock).to have_requested(:post, webhook_url).once
+      expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once
     end
 
     it "calls #{service_name} API for merge requests events" do
       chat_service.execute(@merge_sample_data)
 
-      expect(WebMock).to have_requested(:post, webhook_url).once
+      expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once
     end
 
     it "calls #{service_name} API for wiki page events" do
       chat_service.execute(@wiki_page_sample_data)
 
-      expect(WebMock).to have_requested(:post, webhook_url).once
+      expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once
     end
 
     it "calls #{service_name} API for deployment events" do
@@ -133,14 +145,14 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
 
       chat_service.execute(deployment_event_data)
 
-      expect(WebMock).to have_requested(:post, webhook_url).once
+      expect(WebMock).to have_requested(:post, stubbed_resolved_hostname).once
     end
 
     it 'uses the username as an option for slack when configured' do
       allow(chat_service).to receive(:username).and_return(username)
 
       expect(Slack::Notifier).to receive(:new)
-       .with(webhook_url, username: username)
+       .with(webhook_url, username: username, http_client: SlackService::Notifier::HTTPClient)
        .and_return(
          double(:slack_service).as_null_object
        )
@@ -151,7 +163,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
     it 'uses the channel as an option when it is configured' do
       allow(chat_service).to receive(:channel).and_return(channel)
       expect(Slack::Notifier).to receive(:new)
-        .with(webhook_url, channel: channel)
+        .with(webhook_url, channel: channel, http_client: SlackService::Notifier::HTTPClient)
         .and_return(
           double(:slack_service).as_null_object
         )
@@ -163,7 +175,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
         chat_service.update(push_channel: "random")
 
         expect(Slack::Notifier).to receive(:new)
-         .with(webhook_url, channel: "random")
+         .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
          .and_return(
            double(:slack_service).as_null_object
          )
@@ -175,7 +187,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
         chat_service.update(merge_request_channel: "random")
 
         expect(Slack::Notifier).to receive(:new)
-         .with(webhook_url, channel: "random")
+         .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
          .and_return(
            double(:slack_service).as_null_object
          )
@@ -187,7 +199,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
         chat_service.update(issue_channel: "random")
 
         expect(Slack::Notifier).to receive(:new)
-         .with(webhook_url, channel: "random")
+         .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
          .and_return(
            double(:slack_service).as_null_object
          )
@@ -219,7 +231,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
         chat_service.update(wiki_page_channel: "random")
 
         expect(Slack::Notifier).to receive(:new)
-         .with(webhook_url, channel: "random")
+         .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
          .and_return(
            double(:slack_service).as_null_object
          )
@@ -238,7 +250,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
           note_data = Gitlab::DataBuilder::Note.build(issue_note, user)
 
           expect(Slack::Notifier).to receive(:new)
-           .with(webhook_url, channel: "random")
+           .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
            .and_return(
              double(:slack_service).as_null_object
            )
@@ -286,7 +298,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
         webhook: webhook_url
       )
 
-      WebMock.stub_request(:post, webhook_url)
+      stub_full_request(webhook_url, method: :post)
     end
 
     context 'on default branch' do
@@ -461,7 +473,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
         webhook: webhook_url
       )
 
-      WebMock.stub_request(:post, webhook_url)
+      stub_full_request(webhook_url, method: :post)
     end
 
     context 'when commit comment event executed' do
@@ -535,7 +547,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
         webhook: webhook_url
       )
 
-      WebMock.stub_request(:post, webhook_url)
+      stub_full_request(webhook_url, method: :post)
     end
 
     context 'with succeeded pipeline' do

spec/support/shared_examples/uses_gitlab_url_blocker_examples.rb

+# frozen_string_literal: true
+
+shared_examples 'a request using Gitlab::UrlBlocker' do
+  # Written to test internal patches against 3rd party libraries
+  #
+  # Expects the following to be available in the example contexts:
+  #
+  #   make_request(uri): Wraps the request we want to test goes through Gitlab::HTTP
+  #   http_method: :get, :post etc
+  #   url_blocked_error_class: Probably just Gitlab::HTTP::BlockedUrlError
+
+  include StubRequests
+
+  context 'when local requests are not allowed' do
+    it 'allows an external request with http' do
+      request_stub = stub_full_request('http://example.com', method: http_method, ip_address: '93.184.216.34')
+
+      make_request('http://example.com/')
+
+      expect(request_stub).to have_been_requested
+    end
+
+    it 'allows an external request with https' do
+      request_stub = stub_full_request('https://example.com', method: http_method, ip_address: '93.184.216.34')
+
+      make_request('https://example.com/')
+
+      expect(request_stub).to have_been_requested
+    end
+
+    it 'raises error when it is a request that resolves to a local address' do
+      stub_full_request('https://example.com', method: http_method, ip_address: '172.16.0.0')
+
+      expect { make_request('https://example.com') }
+        .to raise_error(url_blocked_error_class,
+                        "URL 'https://example.com' is blocked: Requests to the local network are not allowed")
+    end
+
+    it 'raises error when it is a request that resolves to a localhost address' do
+      stub_full_request('https://example.com', method: http_method, ip_address: '127.0.0.1')
+
+      expect { make_request('https://example.com') }
+        .to raise_error(url_blocked_error_class,
+                        "URL 'https://example.com' is blocked: Requests to localhost are not allowed")
+    end
+
+    it 'raises error when it is a request to local address' do
+      expect { make_request('http://172.16.0.0') }
+        .to raise_error(url_blocked_error_class,
+                        "URL 'http://172.16.0.0' is blocked: Requests to the local network are not allowed")
+    end
+
+    it 'raises error when it is a request to localhost address' do
+      expect { make_request('http://127.0.0.1') }
+        .to raise_error(url_blocked_error_class,
+                        "URL 'http://127.0.0.1' is blocked: Requests to localhost are not allowed")
+    end
+  end
+
+  context 'when port different from URL scheme is used' do
+    it 'allows the request' do
+      request_stub = stub_full_request('https://example.com:8080', method: http_method, ip_address: '93.184.216.34')
+
+      make_request('https://example.com:8080/')
+
+      expect(request_stub).to have_been_requested
+    end
+
+    it 'raises error when it is a request to local address' do
+      expect { make_request('https://172.16.0.0:8080') }
+        .to raise_error(url_blocked_error_class,
+                        "URL 'https://172.16.0.0:8080' is blocked: Requests to the local network are not allowed")
+    end
+
+    it 'raises error when it is a request to localhost address' do
+      expect { make_request('https://127.0.0.1:8080') }
+        .to raise_error(url_blocked_error_class,
+                        "URL 'https://127.0.0.1:8080' is blocked: Requests to localhost are not allowed")
+    end
+  end
+
+  context 'when DNS rebinding protection is disabled' do
+    before do
+      stub_application_setting(dns_rebinding_protection_enabled: false)
+    end
+
+    it 'allows the request' do
+      request_stub = stub_request(http_method, 'https://example.com')
+
+      make_request('https://example.com/')
+
+      expect(request_stub).to have_been_requested
+    end
+  end
+
+  context 'when http(s) proxy environment variable is set' do
+    before do
+      stub_env('https_proxy' => 'https://my.proxy')
+    end
+
+    it 'allows the request' do
+      request_stub = stub_request(http_method, 'https://example.com')
+
+      make_request('https://example.com/')
+
+      expect(request_stub).to have_been_requested
+    end
+  end
+
+  context 'when local requests are allowed' do
+    before do
+      stub_application_setting(allow_local_requests_from_web_hooks_and_services: true)
+    end
+
+    it 'allows an external request' do
+      request_stub = stub_full_request('https://example.com', method: http_method, ip_address: '93.184.216.34')
+
+      make_request('https://example.com/')
+
+      expect(request_stub).to have_been_requested
+    end
+
+    it 'allows an external request that resolves to a local address' do
+      request_stub = stub_full_request('https://example.com', method: http_method, ip_address: '172.16.0.0')
+
+      make_request('https://example.com/')
+
+      expect(request_stub).to have_been_requested
+    end
+
+    it 'allows an external request that resolves to a localhost address' do
+      request_stub = stub_full_request('https://example.com', method: http_method, ip_address: '127.0.0.1')
+
+      make_request('https://example.com/')
+
+      expect(request_stub).to have_been_requested
+    end
+
+    it 'allows a local address request' do
+      request_stub = stub_request(http_method, 'http://172.16.0.0')
+
+      make_request('http://172.16.0.0')
+
+      expect(request_stub).to have_been_requested
+    end
+
+    it 'allows a localhost address request' do
+      request_stub = stub_request(http_method, 'http://127.0.0.1')
+
+      make_request('http://127.0.0.1')
+
+      expect(request_stub).to have_been_requested
+    end
+  end
+end

spec/views/projects/_home_panel.html.haml_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe 'projects/_home_panel' do
+  include ProjectForksHelper
+
   context 'notifications' do
     let(:project) { create(:project) }
 
@@ -144,4 +146,36 @@ describe 'projects/_home_panel' do
       end
     end
   end
+
+  context 'forks' do
+    let(:source_project) { create(:project, :repository) }
+    let(:project) { fork_project(source_project) }
+    let(:user) { create(:user) }
+
+    before do
+      assign(:project, project)
+
+      allow(view).to receive(:current_user).and_return(user)
+    end
+
+    context 'user can read fork source' do
+      it 'shows the forked-from project' do
+        allow(view).to receive(:can?).with(user, :read_project, source_project).and_return(true)
+
+        render
+
+        expect(rendered).to have_content("Forked from #{source_project.full_name}")
+      end
+    end
+
+    context 'user cannot read fork source' do
+      it 'does not show the forked-from project' do
+        allow(view).to receive(:can?).with(user, :read_project, source_project).and_return(false)
+
+        render
+
+        expect(rendered).to have_content("Forked from an inaccessible project")
+      end
+    end
+  end
 end

spec/views/projects/edit.html.haml_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 describe 'projects/edit' do
   include Devise::Test::ControllerHelpers
+  include ProjectForksHelper
 
   let(:project) { create(:project) }
   let(:user) { create(:admin) }
@@ -26,4 +27,59 @@ describe 'projects/edit' do
       expect(rendered).not_to have_content('Export project')
     end
   end
+
+  context 'forking' do
+    before do
+      assign(:project, project)
+
+      allow(view).to receive(:current_user).and_return(user)
+    end
+
+    context 'project is not a fork' do
+      it 'hides the remove fork relationship settings' do
+        render
+
+        expect(rendered).not_to have_content('Remove fork relationship')
+      end
+    end
+
+    context 'project is a fork' do
+      let(:source_project) { create(:project) }
+      let(:project) { fork_project(source_project) }
+
+      it 'shows the remove fork relationship settings to an authorized user' do
+        allow(view).to receive(:can?).with(user, :remove_fork_project, project).and_return(true)
+
+        render
+
+        expect(rendered).to have_content('Remove fork relationship')
+      end
+
+      it 'hides the fork relationship settings from an unauthorized user' do
+        allow(view).to receive(:can?).with(user, :remove_fork_project, project).and_return(false)
+
+        render
+
+        expect(rendered).not_to have_content('Remove fork relationship')
+      end
+
+      it 'hides the fork source from an unauthorized user' do
+        allow(view).to receive(:can?).with(user, :read_project, source_project).and_return(false)
+
+        render
+
+        expect(rendered).to have_content('Remove fork relationship')
+        expect(rendered).not_to have_content(source_project.full_name)
+      end
+
+      it 'shows the fork source to an authorized user' do
+        allow(view).to receive(:can?).with(user, :read_project, source_project).and_return(true)
+
+        render
+
+        expect(rendered).to have_content('Remove fork relationship')
+        expect(rendered).to have_content(source_project.full_name)
+      end
+    end
+  end
 end