Merge dev.gitlab.org@master into GitLab.com@master

CHANGELOG-EE.md

 Please view this file on the master branch, on stable branches it's out of date.
 
+## 12.4.1
+
+### Security (6 changes)
+
+- Do not display project labels that are not visible for user accessing group labels.
+- Do not index system notes for issue update.
+- Redact search results based on Ability.allowed?.
+- Do not show private cross references in epic notes.
+- Filter out packages the user does'nt have permission to see at group level.
+- Fixes a Open Redirect issue in `InternalRedirect`.
+
+
 ## 12.4.0
 
 ### Security (2 changes)

CHANGELOG.md

@@ -2,6 +2,26 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 12.4.1
+
+### Security (14 changes)
+
+- Standardize error response when route is missing.
+- Do not display project labels that are not visible for user accessing group labels.
+- Show cross-referenced label and milestones in issues' activities only to authorized users.
+- Show cross-referenced label and milestones in issues' activities only to authorized users.
+- Analyze incoming GraphQL queries and check for recursion.
+- Disallow unprivileged users from commenting on private repository commits.
+- Don't allow maintainers of a target project to delete the source branch of a merge request from a fork.
+- Require Maintainer permission on group where project is transferred to.
+- Don't leak private members in project member autocomplete suggestions.
+- Return 404 on LFS request if project doesn't exist.
+- Mask sentry auth token in Error Tracking dashboard.
+- Fixes a Open Redirect issue in `InternalRedirect`.
+- Remove deploy access level when project/group link is deleted.
+- Sanitize all wiki markup formats with GitLab sanitization pipelines.
+
+
 ## 12.4.0
 
 ### Security (14 changes)

app/assets/javascripts/project_find_file.js

@@ -5,6 +5,7 @@ import fuzzaldrinPlus from 'fuzzaldrin-plus';
 import axios from '~/lib/utils/axios_utils';
 import flash from '~/flash';
 import { __ } from '~/locale';
+import sanitize from 'sanitize-html';
 
 // highlight text(awefwbwgtc -> <b>a</b>wefw<b>b</b>wgt<b>c</b> )
 const highlighter = function(element, text, matches) {
@@ -74,7 +75,7 @@ export default class ProjectFindFile {
 
   findFile() {
     var result, searchText;
-    searchText = this.inputElement.val();
+    searchText = sanitize(this.inputElement.val());
     result =
       searchText.length > 0 ? fuzzaldrinPlus.filter(this.filePaths, searchText) : this.filePaths;
     return this.renderList(result, searchText);

app/controllers/application_controller.rb

@@ -17,7 +17,7 @@ class ApplicationController < ActionController::Base
   include Gitlab::Tracking::ControllerConcern
   include Gitlab::Experimentation::ControllerConcern
 
-  before_action :authenticate_user!
+  before_action :authenticate_user!, except: [:route_not_found]
   before_action :enforce_terms!, if: :should_enforce_terms?
   before_action :validate_user_service_ticket!
   before_action :check_password_expiration
@@ -98,7 +98,9 @@ class ApplicationController < ActionController::Base
     if current_user
       not_found
     else
-      authenticate_user!
+      store_location_for(:user, request.fullpath) unless request.xhr?
+
+      redirect_to new_user_session_path, alert: I18n.t('devise.failure.unauthenticated')
     end
   end
 

app/controllers/concerns/internal_redirect.rb

@@ -6,7 +6,7 @@ module InternalRedirect
   def safe_redirect_path(path)
     return unless path
     # Verify that the string starts with a `/` and a known route character.
-    return unless path =~ %r{^/[-\w].*$}
+    return unless path =~ %r{\A/[-\w].*\z}
 
     uri = URI(path)
     # Ignore anything path of the redirect except for the path, querystring and,

app/controllers/concerns/lfs_request.rb

@@ -34,6 +34,7 @@ module LfsRequest
   end
 
   def lfs_check_access!
+    return render_lfs_not_found unless project
     return if download_request? && lfs_download_access?
     return if upload_request? && lfs_upload_access?
 

app/finders/labels_finder.rb

@@ -51,7 +51,7 @@ class LabelsFinder < UnionFinder
       end
 
       label_ids << Label.where(group_id: projects.group_ids)
-      label_ids << Label.where(project_id: projects.select(:id)) unless only_group_labels?
+      label_ids << Label.where(project_id: ids_user_can_read_labels(projects)) unless only_group_labels?
     end
 
     label_ids
@@ -188,4 +188,10 @@ class LabelsFinder < UnionFinder
       groups.select { |group| authorized_to_read_labels?(group) }
     end
   end
+
+  # rubocop: disable CodeReuse/ActiveRecord
+  def ids_user_can_read_labels(projects)
+    Project.where(id: projects.select(:id)).ids_with_issuables_available_for(current_user)
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
 end

app/graphql/gitlab_schema.rb

@@ -18,15 +18,15 @@ class GitlabSchema < GraphQL::Schema
   use Gitlab::Graphql::GenericTracing
 
   query_analyzer Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer.new
-
-  query(Types::QueryType)
-
-  default_max_page_size 100
+  query_analyzer Gitlab::Graphql::QueryAnalyzers::RecursionAnalyzer.new
 
   max_complexity DEFAULT_MAX_COMPLEXITY
   max_depth DEFAULT_MAX_DEPTH
 
-  mutation(Types::MutationType)
+  query Types::QueryType
+  mutation Types::MutationType
+
+  default_max_page_size 100
 
   class << self
     def multiplex(queries, **kwargs)

app/helpers/markup_helper.rb

@@ -133,15 +133,7 @@ module MarkupHelper
       issuable_state_filter_enabled: true
     )
 
-    html =
-      case wiki_page.format
-      when :markdown
-        markdown_unsafe(text, context)
-      when :asciidoc
-        asciidoc_unsafe(text)
-      else
-        wiki_page.formatted_content.html_safe
-      end
+    html = markup_unsafe(wiki_page.path, text, context)
 
     prepare_for_rendering(html, context)
   end

app/models/concerns/mentionable/reference_regexes.rb

@@ -13,7 +13,9 @@ module Mentionable
     def self.other_patterns
       [
         Commit.reference_pattern,
-        MergeRequest.reference_pattern
+        MergeRequest.reference_pattern,
+        Label.reference_pattern,
+        Milestone.reference_pattern
       ]
     end
 

app/models/discussion.rb

@@ -16,6 +16,7 @@ class Discussion
             :commit_id,
             :for_commit?,
             :for_merge_request?,
+            :noteable_ability_name,
             :to_ability_name,
             :editable?,
             :visible_for?,

app/models/member.rb

@@ -8,6 +8,7 @@ class Member < ApplicationRecord
   include Gitlab::Access
   include Presentable
   include Gitlab::Utils::StrongMemoize
+  include FromUnion
 
   attr_accessor :raw_invite_token
 

app/models/merge_request.rb

@@ -69,6 +69,14 @@ class MergeRequest < ApplicationRecord
   has_many :merge_request_assignees
   has_many :assignees, class_name: "User", through: :merge_request_assignees
 
+  KNOWN_MERGE_PARAMS = [
+    :auto_merge_strategy,
+    :should_remove_source_branch,
+    :force_remove_source_branch,
+    :commit_message,
+    :squash_commit_message,
+    :sha
+  ].freeze
   serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize
 
   after_create :ensure_merge_request_diff

app/models/milestone.rb

@@ -261,6 +261,10 @@ class Milestone < ApplicationRecord
     group || project
   end
 
+  def to_ability_name
+    model_name.singular
+  end
+
   def group_milestone?
     group_id.present?
   end

app/models/note.rb

@@ -361,6 +361,10 @@ class Note < ApplicationRecord
   end
 
   def to_ability_name
+    model_name.singular
+  end
+
+  def noteable_ability_name
     for_snippet? ? noteable.class.name.underscore : noteable_type.demodulize.underscore
   end
 

app/models/project.rb

@@ -614,11 +614,11 @@ class Project < ApplicationRecord
       joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id)
     end
 
-    # Returns ids of projects with milestones available for given user
+    # Returns ids of projects with issuables 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)
+    # Used on queries to find milestones or labels which user can see
+    # For example: Milestone.where(project_id: ids_with_issuables_available_for(user))
+    def ids_with_issuables_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)
 
@@ -1265,6 +1265,10 @@ class Project < ApplicationRecord
     end
   end
 
+  def to_ability_name
+    model_name.singular
+  end
+
   # rubocop: disable CodeReuse/ServiceClass
   def execute_hooks(data, hooks_scope = :push_hooks)
     run_after_commit_or_now do

app/models/system_note_metadata.rb

@@ -10,6 +10,7 @@ class SystemNoteMetadata < ApplicationRecord
     commit cross_reference
     close duplicate
     moved merge
+    label milestone
   ].freeze
 
   ICON_TYPES = %w[

app/models/wiki_page.rb

@@ -121,6 +121,12 @@ class WikiPage
     @version ||= @page.version
   end
 
+  def path
+    return unless persisted?
+
+    @path ||= @page.path
+  end
+
   def versions(options = {})
     return [] unless persisted?
 

app/policies/commit_policy.rb

@@ -4,4 +4,5 @@ class CommitPolicy < BasePolicy
   delegate { @subject.project }
 
   rule { can?(:download_code) }.enable :read_commit
+  rule { ~can?(:read_commit) }.prevent :create_note
 end

app/policies/group_policy.rb

@@ -131,6 +131,8 @@ class GroupPolicy < BasePolicy
 
   rule { owner | admin }.enable :read_statistics
 
+  rule { maintainer & can?(:create_projects) }.enable :transfer_projects
+
   def access_level
     return GroupMember::NO_ACCESS if @user.nil?
 

app/policies/namespace_policy.rb

@@ -15,6 +15,8 @@ class NamespacePolicy < BasePolicy
   end
 
   rule { personal_project & ~can_create_personal_project }.prevent :create_projects
+
+  rule { (owner | admin) & can?(:create_projects) }.enable :transfer_projects
 end
 
 NamespacePolicy.prepend_if_ee('EE::NamespacePolicy')

app/policies/note_policy.rb

@@ -9,7 +9,7 @@ class NotePolicy < BasePolicy
 
   condition(:editable, scope: :subject) { @subject.editable? }
 
-  condition(:can_read_noteable) { can?(:"read_#{@subject.to_ability_name}") }
+  condition(:can_read_noteable) { can?(:"read_#{@subject.noteable_ability_name}") }
 
   condition(:is_visible) { @subject.visible_for?(@user) }
 

app/services/auto_merge/base_service.rb

@@ -3,12 +3,13 @@
 module AutoMerge
   class BaseService < ::BaseService
     include Gitlab::Utils::StrongMemoize
+    include MergeRequests::AssignsMergeParams
 
     def execute(merge_request)
-      merge_request.merge_params.merge!(params)
+      assign_allowed_merge_params(merge_request, params.merge(auto_merge_strategy: strategy))
+
       merge_request.auto_merge_enabled = true
       merge_request.merge_user = current_user
-      merge_request.auto_merge_strategy = strategy
 
       return :failed unless merge_request.save
 
@@ -21,7 +22,7 @@ module AutoMerge
     end
 
     def update(merge_request)
-      merge_request.merge_params.merge!(params)
+      assign_allowed_merge_params(merge_request, params.merge(auto_merge_strategy: strategy))
 
       return :failed unless merge_request.save
 

app/services/concerns/merge_requests/assigns_merge_params.rb

+# frozen_string_literal: true
+
+module MergeRequests
+  module AssignsMergeParams
+    def self.included(klass)
+      raise "#{self} can not be included in #{klass} without implementing #current_user" unless klass.method_defined?(:current_user)
+    end
+
+    def assign_allowed_merge_params(merge_request, merge_params)
+      known_merge_params = merge_params.to_h.with_indifferent_access.slice(*MergeRequest::KNOWN_MERGE_PARAMS)
+
+      # Not checking `MergeRequest#can_remove_source_branch` as that includes
+      # other checks that aren't needed here.
+      known_merge_params.delete(:force_remove_source_branch) unless current_user.can?(:push_code, merge_request.source_project)
+
+      merge_request.merge_params.merge!(known_merge_params)
+
+      # Delete the known params now that they're assigned, so we don't try to
+      # assign them through an `#assign_attributes` later.
+      # They could be coming in as strings or symbols
+      merge_params.to_h.with_indifferent_access.except!(*MergeRequest::KNOWN_MERGE_PARAMS)
+    end
+  end
+end

app/services/error_tracking/list_projects_service.rb

@@ -32,7 +32,7 @@ module ErrorTracking
           project_slug: 'proj'
         )
 
-        setting.token = params[:token]
+        setting.token = token(setting)
         setting.enabled = true
       end
     end
@@ -40,5 +40,12 @@ module ErrorTracking
     def can_read?
       can?(current_user, :read_sentry_issue, project)
     end
+
+    def token(setting)
+      # Use param token if not masked, otherwise use database token
+      return params[:token] unless /\A\*+\z/.match?(params[:token])
+
+      setting.token
+    end
   end
 end

app/services/merge_requests/base_service.rb

@@ -2,6 +2,8 @@
 
 module MergeRequests
   class BaseService < ::IssuableBaseService
+    include MergeRequests::AssignsMergeParams
+
     def create_note(merge_request, state = merge_request.state)
       SystemNoteService.change_status(merge_request, merge_request.target_project, current_user, state, nil)
     end
@@ -29,6 +31,18 @@ module MergeRequests
 
     private
 
+    def create(merge_request)
+      self.params = assign_allowed_merge_params(merge_request, params)
+
+      super
+    end
+
+    def update(merge_request)
+      self.params = assign_allowed_merge_params(merge_request, params)
+
+      super
+    end
+
     def handle_wip_event(merge_request)
       if wip_event = params.delete(:wip_event)
         # We update the title that is provided in the params or we use the mr title

app/services/merge_requests/build_service.rb

@@ -24,6 +24,8 @@ module MergeRequests
           merge_request.source_project.remove_source_branch_after_merge?
         end
 
+      self.params = assign_allowed_merge_params(merge_request, params)
+
       filter_params(merge_request)
 
       # merge_request.assign_attributes(...) below is a Rails

app/services/merge_requests/create_service.rb

@@ -9,7 +9,6 @@ module MergeRequests
       merge_request.target_project = @project
       merge_request.source_project = @source_project
       merge_request.source_branch = params[:source_branch]
-      merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch)
 
       create(merge_request)
     end

app/services/merge_requests/update_service.rb

@@ -16,10 +16,6 @@ module MergeRequests
         params.delete(:force_remove_source_branch)
       end
 
-      if params.has_key?(:force_remove_source_branch)
-        merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch)
-      end
-
       handle_wip_event(merge_request)
       update_task_event(merge_request) || update(merge_request)
     end

app/services/notification_service.rb

@@ -281,7 +281,7 @@ class NotificationService
   end
 
   def send_new_note_notifications(note)
-    notify_method = "note_#{note.to_ability_name}_email".to_sym
+    notify_method = "note_#{note.noteable_ability_name}_email".to_sym
 
     recipients = NotificationRecipientService.build_new_note_recipients(note)
     recipients.each do |recipient|

app/services/projects/operations/update_service.rb

@@ -36,15 +36,17 @@ module Projects
           organization_slug: settings.dig(:project, :organization_slug)
         )
 
-        {
+        params = {
           error_tracking_setting_attributes: {
             api_url: api_url,
-            token: settings[:token],
             enabled: settings[:enabled],
             project_name: settings.dig(:project, :name),
             organization_name: settings.dig(:project, :organization_name)
           }
         }
+        params[:error_tracking_setting_attributes][:token] = settings[:token] unless /\A\*+\z/.match?(settings[:token]) # Don't update token if we receive masked value
+
+        params
       end
 
       def grafana_integration_params

app/services/projects/participants_service.rb

@@ -7,16 +7,69 @@ module Projects
     def execute(noteable)
       @noteable = noteable
 
-      participants = noteable_owner + participants_in_noteable + all_members + groups + project_members
+      participants =
+        noteable_owner +
+        participants_in_noteable +
+        all_members +
+        groups +
+        project_members
+
       participants.uniq
     end
 
     def project_members
-      @project_members ||= sorted(project.team.members)
+      @project_members ||= sorted(get_project_members)
+    end
+
+    def get_project_members
+      members = Member.from_union([project_members_through_ancestral_groups,
+                                   project_members_through_invited_groups,
+                                   individual_project_members])
+
+      User.id_in(members.select(:user_id))
     end
 
     def all_members
       [{ username: "all", name: "All Project and Group Members", count: project_members.count }]
     end
+
+    private
+
+    def project_members_through_invited_groups
+      groups_with_ancestors_ids = Gitlab::ObjectHierarchy
+        .new(visible_groups)
+        .base_and_ancestors
+        .pluck_primary_key
+
+      GroupMember
+        .active_without_invites_and_requests
+        .with_source_id(groups_with_ancestors_ids)
+    end
+
+    def visible_groups
+      visible_groups = project.invited_groups
+
+      unless project_owner?
+        visible_groups = visible_groups.public_or_visible_to_user(current_user)
+      end
+
+      visible_groups
+    end
+
+    def project_members_through_ancestral_groups
+      project.group.present? ? project.group.members_with_parents : Member.none
+    end
+
+    def individual_project_members
+      project.project_members
+    end
+
+    def project_owner?
+      if project.group.present?
+        project.group.owners.include?(current_user)
+      else
+        project.namespace.owner == current_user
+      end
+    end
   end
 end

app/services/projects/transfer_service.rb

@@ -98,7 +98,7 @@ module Projects
       @new_namespace &&
         can?(current_user, :change_namespace, project) &&
         @new_namespace.id != project.namespace_id &&
-        current_user.can?(:create_projects, @new_namespace)
+        current_user.can?(:transfer_projects, @new_namespace)
     end
 
     def update_namespace_and_visibility(to_namespace)

app/views/projects/settings/operations/_error_tracking.html.haml

@@ -17,4 +17,4 @@
         project: error_tracking_setting_project_json,
         api_host: setting.api_host,
         enabled: setting.enabled.to_json,
-        token: setting.token } }
+        token: setting.token.present? ? '*' * 12 : nil } }

changelogs/unreleased/29986-remove-leaky-401-responses.yml

+---
+title: Standardize error response when route is missing
+merge_request:
+author:
+type: security

changelogs/unreleased/security-2914-labels-visible-despite-no-access-to-issues-repositories.yml

+---
+title: Do not display project labels that are not visible for user accessing group labels
+merge_request:
+author:
+type: security

changelogs/unreleased/security-2920-fix-notes-with-label-cross-reference.yml

+---
+title: Show cross-referenced label and milestones in issues' activities only to authorized users
+merge_request:
+author:
+type: security

changelogs/unreleased/security-64519-nested-graphql-query-can-cause-denial-of-service.yml

+---
+title: Analyze incoming GraphQL queries and check for recursion
+merge_request:
+author:
+type: security

changelogs/unreleased/security-65756-ex-admin-attacker-can-comment-in-internal.yml

+---
+title: Disallow unprivileged users from commenting on private repository commits
+merge_request:
+author:
+type: security

changelogs/unreleased/security-bvl-validate-force-remove-branch-on-mrs.yml

+---
+title: Don't allow maintainers of a target project to delete the source branch of
+  a merge request from a fork
+merge_request:
+author:
+type: security

changelogs/unreleased/security-developer-transfer-project.yml

+---
+title: Require Maintainer permission on group where project is transferred to
+merge_request:
+author:
+type: security

changelogs/unreleased/security-hide-private-members-in-project-member-autocomplete.yml

+---
+title: "Don't leak private members in project member autocomplete suggestions"
+type: security

changelogs/unreleased/security-id-fix-disclosure-of-private-repo-names.yml

+---
+title: Return 404 on LFS request if project doesn't exist
+merge_request:
+author:
+type: security

changelogs/unreleased/security-mask-sentry-token-ce.yml

+---
+title: Mask sentry auth token in Error Tracking dashboard
+author:
+type: security

changelogs/unreleased/security-remove-deploy-access-levels-on-project-group-link-deletion.yml

+---
+title: Remove deploy access level when project/group link is deleted
+merge_request:
+author:
+type: security

changelogs/unreleased/security-stored-xss-using-find-file.yml

+---
+title: Sanitize search text to prevent XSS
+merge_request:
+author:
+type: security

changelogs/unreleased/security-wiki-rdoc-content.yml

+---
+title: Sanitize all wiki markup formats with GitLab sanitization pipelines
+merge_request:
+author:
+type: security

doc/user/project/members/share_project_with_groups.md

@@ -44,6 +44,10 @@ Admins are able to share projects with any group in the system.
 
 In the example above, the maximum access level of 'Developer' for members from 'Engineering' means that users with higher access levels in 'Engineering' ('Maintainer' or 'Owner') will only have 'Developer' access to 'Project Acme'.
 
+## Sharing public project with private group
+
+When sharing a public project with a private group, owners and maintainers of the project will see the name of the group in the `members` page. Owners will also have the possibility to see members of the private group they don't have access to when mentioning them in the issue or merge request.
+
 ## Share project with group lock
 
 It is possible to prevent projects in a group from [sharing

ee/app/models/ee/project_group_link.rb

@@ -5,14 +5,18 @@ module EE
     extend ActiveSupport::Concern
 
     prepended do
-      before_destroy :delete_branch_protection
+      before_destroy :delete_related_access_levels
     end
 
-    def delete_branch_protection
-      if group.present? && project.present?
-        project.protected_branches.merge_access_by_group(group).destroy_all # rubocop: disable DestroyAll
-        project.protected_branches.push_access_by_group(group).destroy_all # rubocop: disable DestroyAll
-      end
+    def delete_related_access_levels
+      return unless group.present? && project.present?
+
+      # For protected branches
+      project.protected_branches.merge_access_by_group(group).destroy_all # rubocop: disable DestroyAll
+      project.protected_branches.push_access_by_group(group).destroy_all # rubocop: disable DestroyAll
+
+      # For protected environments
+      project.protected_environments.deploy_access_levels_by_group(group).delete_all
     end
   end
 end

ee/app/models/ee/system_note_metadata.rb

@@ -14,7 +14,7 @@ module EE
     EE_TYPES_WITH_CROSS_REFERENCES = %w[
       relate unrelate
       epic_issue_added issue_added_to_epic epic_issue_removed issue_removed_from_epic
-      epic_issue_moved issue_changed_epic
+      epic_issue_moved issue_changed_epic relate_epic unrelate_epic
     ].freeze
 
     override :icon_types

ee/app/models/protected_environment.rb

@@ -19,6 +19,11 @@ class ProtectedEnvironment < ApplicationRecord
              ' AND protected_environments.project_id = environments.project_id')
   end
 
+  scope :deploy_access_levels_by_group, -> (group) do
+    ProtectedEnvironment::DeployAccessLevel
+      .joins(:protected_environment).where(group: group)
+  end
+
   def accessible_to?(user)
     deploy_access_levels
       .any? { |deploy_access_level| deploy_access_level.check_access(user) }

ee/app/services/elastic/index_record_service.rb

@@ -34,11 +34,13 @@ module Elastic
 
     private
 
+    # rubocop: disable CodeReuse/ActiveRecord
     def update_issue_notes(record, changed_fields)
       if changed_fields && (changed_fields & ISSUE_TRACKED_FIELDS).any?
-        import_association(Note, query: -> { where(noteable: record) })
+        import_association(Note, query: -> { searchable.where(noteable: record) })
       end
     end
+    # rubocop: enable CodeReuse/ActiveRecord
 
     def initial_index_project(project)
       # Enqueue the repository indexing jobs immediately so they run in parallel

ee/changelogs/unreleased/security-33660-comment-search-should-not-show-system-notes.yml

+---
+title: Do not index system notes for issue update
+merge_request:
+author:
+type: security

ee/changelogs/unreleased/security-33689-post-filter-search-results.yml

+---
+title: Redact search results based on Ability.allowed?
+merge_request:
+author:
+type: security

ee/changelogs/unreleased/security-377-private-subgroup-path-visible-in-epic-timeline-when-child.yml

+---
+title: Do not show private cross references in epic notes
+merge_request:
+author:
+type: security

ee/changelogs/unreleased/security-open-redirect-internalredirect.yml

+---
+title: Fixes a Open Redirect issue in `InternalRedirect`.
+merge_request:
+author:
+type: security

ee/lib/gitlab/elastic/search_results.rb

@@ -167,9 +167,26 @@ module Gitlab
       def eager_load(es_result, page, eager:)
         paginated_base = es_result.page(page).per(per_page)
         relation = paginated_base.records.includes(eager) # rubocop:disable CodeReuse/ActiveRecord
+        filtered_results = []
+        permitted_results = relation.select do |o|
+          ability = :"read_#{o.to_ability_name}"
+          if Ability.allowed?(current_user, ability, o)
+            true
+          else
+            # Redact any search result the user may not have access to. This
+            # could be due to incorrect data in the index or a bug in our query
+            # so we log this as an error.
+            filtered_results << { ability: ability, id: o.id, class_name: o.class.name }
+            false
+          end
+        end
+
+        if filtered_results.any?
+          logger.error(message: "redacted_search_results", filtered: filtered_results, current_user_id: current_user&.id, query: query)
+        end
 
         Kaminari.paginate_array(
-          relation,
+          permitted_results,
           total_count: paginated_base.total_count,
           limit: per_page,
           offset: per_page * (page - 1)
@@ -363,6 +380,10 @@ module Gitlab
       def per_page
         20
       end
+
+      def logger
+        @logger ||= Gitlab::ProjectServiceLogger.build
+      end
     end
   end
 end

ee/spec/controllers/groups/boards_controller_spec.rb

@@ -56,6 +56,16 @@ describe Groups::BoardsController do
     let(:parent) { group }
 
     it_behaves_like 'returns recently visited boards'
+
+    context 'unauthenticated' do
+      it 'returns a 401' do
+        sign_out(user)
+
+        list_boards(recent: true)
+
+        expect(response).to have_gitlab_http_status(401)
+      end
+    end
   end
 
   describe 'GET show' do

ee/spec/controllers/projects/boards_controller_spec.rb

@@ -31,6 +31,16 @@ describe Projects::BoardsController do
     let(:parent) { project }
 
     it_behaves_like 'returns recently visited boards'
+
+    context 'unauthenticated' do
+      it 'returns a 302' do
+        sign_out(user)
+
+        list_boards(recent: true)
+
+        expect(response).to have_gitlab_http_status(302)
+      end
+    end
   end
 
   describe 'GET show' do

ee/spec/controllers/projects/managed_licenses_controller_spec.rb

@@ -72,10 +72,10 @@ describe Projects::ManagedLicensesController do
     context 'with no logged in user' do
       let(:user) { unlogged_user }
 
-      it 'returns an unauthorized status' do
+      it 'returns a redirect' do
         subject
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+        expect(response).to have_gitlab_http_status(:redirect)
       end
     end
 
@@ -122,10 +122,10 @@ describe Projects::ManagedLicensesController do
     context 'with no logged in user' do
       let(:user) { unlogged_user }
 
-      it 'returns an unauthorized status' do
+      it 'returns a redirect' do
         subject
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+        expect(response).to have_gitlab_http_status(:redirect)
       end
     end
 
@@ -235,10 +235,10 @@ describe Projects::ManagedLicensesController do
         new_software_license_policy_attributes
       end
 
-      it 'returns an unauthorized status' do
+      it 'returns a redirect' do
         expect { subject }.not_to change { project.software_license_policies.count }
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+        expect(response).to have_gitlab_http_status(:redirect)
       end
     end
 
@@ -347,10 +347,10 @@ describe Projects::ManagedLicensesController do
         new_software_license_policy_attributes
       end
 
-      it 'returns an unauthorized status' do
+      it 'returns a redirect' do
         expect { subject }.not_to change { project.software_license_policies.count }
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+        expect(response).to have_gitlab_http_status(:redirect)
       end
     end
 
@@ -452,10 +452,10 @@ describe Projects::ManagedLicensesController do
         new_software_license_policy_attributes
       end
 
-      it 'returns an unauthorized status' do
+      it 'returns a redirect' do
         expect { subject }.not_to change { project.software_license_policies.count }
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+        expect(response).to have_gitlab_http_status(:redirect)
       end
     end
 

ee/spec/controllers/projects/settings/operations_controller_spec.rb

@@ -506,10 +506,10 @@ describe Projects::Settings::OperationsController do
         sign_out(user)
       end
 
-      it 'returns unauthorized status' do
+      it 'returns a redirect' do
         reset_alerting_token
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+        expect(response).to have_gitlab_http_status(:redirect)
       end
     end
 

ee/spec/features/search/elastic/global_search_spec.rb

@@ -43,8 +43,10 @@ describe 'Global elastic search', :elastic do
       let(:object) { :project }
       let(:creation_args) { { namespace: user.namespace } }
       let(:path) { search_path(search: 'project*', scope: 'projects') }
-      # Each Project requires 4 extra queries: one for each "count" (forks, open MRs, open Issues) and one for access level
-      let(:query_count_multiplier) { 4 }
+      # Each Project requires 5 extra queries: one for each "count" (forks,
+      # open MRs, open Issues) and twice for access level. This should be fixed
+      # per https://gitlab.com/gitlab-org/gitlab/issues/34457
+      let(:query_count_multiplier) { 5 }
 
       it_behaves_like 'an efficient database result'
     end

ee/spec/lib/gitlab/elastic/search_results_spec.rb

@@ -215,6 +215,45 @@ describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need_inlin
       expect(results.objects('notes')).to be_empty
       expect(results.notes_count).to eq 0
     end
+
+    it 'redacts issue comments on public projects where issue has lower access_level' do
+      project_1.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE)
+
+      results = described_class.new(user, 'foo', limit_project_ids)
+
+      expect(results.send(:logger))
+        .to receive(:error)
+        .with(hash_including(message: "redacted_search_results", filtered: array_including([
+          { class_name: "Note", id: @note_1.id, ability: :read_note },
+          { class_name: "Note", id: @note_2.id, ability: :read_note }
+      ])))
+
+      expect(results.notes_count).to eq(2) # 2 because redacting only happens when we instantiate the results
+      expect(results.objects('notes')).to be_empty
+    end
+
+    it 'redacts commit comments when user is a guest on a private project' do
+      project_1.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+      project_1.add_guest(user)
+      note_on_commit = create(
+        :note_on_commit,
+        project: project_1,
+        note: 'foo note on commit'
+      )
+
+      Gitlab::Elastic::Helper.refresh_index
+
+      results = described_class.new(user, 'foo', limit_project_ids)
+
+      expect(results.send(:logger))
+        .to receive(:error)
+        .with(hash_including(message: "redacted_search_results", filtered: array_including([
+          { class_name: "Note", id: note_on_commit.id, ability: :read_note }
+      ])))
+
+      expect(results.notes_count).to eq(3) # 3 because redacting only happens when we instantiate the results
+      expect(results.objects('notes')).to match_array([@note_1, @note_2])
+    end
   end
 
   describe 'confidential issues' do
@@ -869,15 +908,20 @@ describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need_inlin
 
       context 'when project_ids is not present' do
         context 'when project_ids is :any' do
-          it 'returns all milestones' do
+          it 'returns all milestones and redacts them when the user has no access' do
             results = described_class.new(user, 'project', :any)
+            expect(results.send(:logger))
+              .to receive(:error)
+              .with(hash_including(message: "redacted_search_results", filtered: [{ class_name: "Milestone", id: milestone_2.id, ability: :read_milestone }]))
+
             milestones = results.objects('milestones')
 
+            expect(results.milestones_count).to eq(4) # 4 because redacting only happens when we instantiate the results
+
             expect(milestones).to include(milestone_1)
-            expect(milestones).to include(milestone_2)
+            expect(milestones).not_to include(milestone_2)
             expect(milestones).to include(milestone_3)
             expect(milestones).to include(milestone_4)
-            expect(results.milestones_count).to eq(4)
           end
         end
 

ee/spec/models/ee/project_group_link_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ProjectGroupLink do
+  describe '#destroy' do
+    let(:project) { create(:project) }
+    let(:group) { create(:group) }
+    let(:user) { create(:user) }
+    let!(:project_group_link) { create(:project_group_link, project: project, group: group) }
+
+    it 'removes related protected environment deploy access levels' do
+      params = attributes_for(:protected_environment,
+                              deploy_access_levels_attributes: [{ group_id: group.id }, { user_id: user.id }])
+
+      protected_environment = ProtectedEnvironments::CreateService.new(project, user, params).execute
+
+      expect { project_group_link.destroy! }.to change(ProtectedEnvironment::DeployAccessLevel, :count).by(-1)
+
+      expect(protected_environment.deploy_access_levels.find_by_group_id(group)).to be_nil
+      expect(protected_environment.deploy_access_levels.find_by_user_id(user)).to be_persisted
+    end
+  end
+end

ee/spec/models/note_spec.rb

@@ -72,4 +72,17 @@ describe Note do
       end
     end
   end
+
+  describe '#cross_reference?' do
+    [:relate_epic, :unrelate_epic].each do |type|
+      it "delegates #{type} system note to the cross-reference regex" do
+        note = create(:note, :system)
+        create(:system_note_metadata, note: note, action: type)
+
+        expect(note).to receive(:matches_cross_reference_regex?).and_return(false)
+
+        note.cross_reference?
+      end
+    end
+  end
 end

ee/spec/models/protected_environment_spec.rb

@@ -116,6 +116,21 @@ describe ProtectedEnvironment do
     end
   end
 
+  describe '.deploy_access_levels_by_group' do
+    let(:group) { create(:group) }
+    let(:project) { create(:project) }
+    let(:environment) { create(:environment, project: project, name: 'production') }
+    let(:protected_environment) { create(:protected_environment, project: project, name: 'production') }
+
+    it 'returns matching deploy access levels for the given group' do
+      _deploy_access_level_for_different_group = create_deploy_access_level(group: create(:group))
+      _deploy_access_level_for_user = create_deploy_access_level(user: create(:user))
+      deploy_access_level = create_deploy_access_level(group: group)
+
+      expect(described_class.deploy_access_levels_by_group(group)).to contain_exactly(deploy_access_level)
+    end
+  end
+
   def create_deploy_access_level(**opts)
     protected_environment.deploy_access_levels.create(**opts)
   end

ee/spec/services/elastic/index_record_service_spec.rb

@@ -320,4 +320,50 @@ describe Elastic::IndexRecordService, :elastic do
 
     expect(Project.elastic_search('project_1').present?).to eq(false)
   end
+
+  context 'when updating an Issue' do
+    context 'when changing the confidential value' do
+      it 'updates issue notes excluding system notes' do
+        issue = nil
+        Sidekiq::Testing.disable! do
+          issue = create(:issue, confidential: false)
+          subject.execute(issue.project, true)
+          subject.execute(issue, false)
+          create(:note, note: 'the_normal_note', noteable: issue, project: issue.project)
+          create(:note, note: 'the_system_note', system: true, noteable: issue, project: issue.project)
+        end
+
+        options = { project_ids: [issue.project.id] }
+
+        Sidekiq::Testing.inline! do
+          expect(subject.execute(issue, false, 'changed_fields' => ['confidential'])).to eq(true)
+          Gitlab::Elastic::Helper.refresh_index
+        end
+
+        expect(Note.elastic_search('the_normal_note', options: options).present?).to eq(true)
+        expect(Note.elastic_search('the_system_note', options: options).present?).to eq(false)
+      end
+    end
+
+    context 'when changing the title' do
+      it 'does not update issue notes' do
+        issue = nil
+        Sidekiq::Testing.disable! do
+          issue = create(:issue, confidential: false)
+          subject.execute(issue.project, true)
+          subject.execute(issue, false)
+          create(:note, note: 'the_normal_note', noteable: issue, project: issue.project)
+        end
+
+        options = { project_ids: [issue.project.id] }
+
+        Sidekiq::Testing.inline! do
+          expect(subject.execute(issue, false, 'changed_fields' => ['title'])).to eq(true)
+          Gitlab::Elastic::Helper.refresh_index
+        end
+
+        expect(Note.elastic_search('the_normal_note', options: options).present?).to eq(false)
+      end
+    end
+  end
 end

ee/spec/support/shared_examples/controllers/recent_boards.rb

@@ -5,16 +5,6 @@ require 'spec_helper'
 shared_examples 'returns recently visited boards' do
   let(:boards) { create_list(:board, 8, resource_parent: parent) }
 
-  context 'unauthenticated' do
-    it 'returns a 401' do
-      sign_out(user)
-
-      list_boards(recent: true)
-
-      expect(response).to have_gitlab_http_status(401)
-    end
-  end
-
   it 'returns last 4 visited boards' do
     [0, 2, 5, 3, 7, 1].each_with_index do |board_index, i|
       visit_board(boards[board_index], Time.now + i.minutes)

lib/gitlab/graphql/query_analyzers/recursion_analyzer.rb

+# frozen_string_literal: true
+
+# Recursive queries, with relatively low effort, can quickly spiral out of control exponentially
+# and may not be picked up by depth and complexity alone.
+module Gitlab
+  module Graphql
+    module QueryAnalyzers
+      class RecursionAnalyzer
+        IGNORED_FIELDS = %w(node edges ofType).freeze
+        RECURSION_THRESHOLD = 2
+
+        def initial_value(query)
+          {
+              recurring_fields: {}
+          }
+        end
+
+        def call(memo, visit_type, irep_node)
+          return memo if skip_node?(irep_node)
+
+          node_name = irep_node.ast_node.name
+          times_encountered = memo[node_name] || 0
+
+          if visit_type == :enter
+            times_encountered += 1
+            memo[:recurring_fields][node_name] = times_encountered if recursion_too_deep?(node_name, times_encountered)
+          else
+            times_encountered -= 1
+          end
+
+          memo[node_name] = times_encountered
+          memo
+        end
+
+        def final_value(memo)
+          recurring_fields = memo[:recurring_fields]
+          recurring_fields = recurring_fields.select { |k, v| recursion_too_deep?(k, v) }
+          if recurring_fields.any?
+            GraphQL::AnalysisError.new("Recursive query - too many of fields '#{recurring_fields}' detected in single branch of the query")
+          end
+        end
+
+        private
+
+        def recursion_too_deep?(node_name, times_encountered)
+          return if IGNORED_FIELDS.include?(node_name)
+
+          times_encountered > recursion_threshold
+        end
+
+        def skip_node?(irep_node)
+          ast_node = irep_node.ast_node
+          !ast_node.is_a?(GraphQL::Language::Nodes::Field) || ast_node.selections.empty?
+        end
+
+        def recursion_threshold
+          RECURSION_THRESHOLD
+        end
+      end
+    end
+  end
+end

lib/gitlab/other_markup.rb

@@ -10,7 +10,7 @@ module Gitlab
     def self.render(file_name, input, context)
       html = GitHub::Markup.render(file_name, input)
         .force_encoding(input.encoding)
-      context[:pipeline] = :markup
+      context[:pipeline] ||= :markup
 
       html = Banzai.render(html, context)
 

lib/gitlab/search_results.rb

@@ -163,7 +163,7 @@ module Gitlab
       return Milestone.none if project_ids.nil?
 
       authorized_project_ids_relation =
-        Project.where(id: project_ids).ids_with_milestone_available_for(current_user)
+        Project.where(id: project_ids).ids_with_issuables_available_for(current_user)
 
       milestones.where(project_id: authorized_project_ids_relation)
     end

spec/controllers/application_controller_spec.rb

@@ -186,7 +186,7 @@ describe ApplicationController do
       expect(response).to have_gitlab_http_status(404)
     end
 
-    it 'redirects to login page via authenticate_user! if not authenticated' do
+    it 'redirects to login page if not authenticated' do
       get :index
 
       expect(response).to redirect_to new_user_session_path

spec/controllers/concerns/internal_redirect_spec.rb

@@ -19,7 +19,8 @@ describe InternalRedirect do
       [
         'Hello world',
         '//example.com/hello/world',
-        'https://example.com/hello/world'
+        'https://example.com/hello/world',
+        "not-starting-with-a-slash\n/starting/with/slash"
       ]
     end
 

spec/controllers/concerns/lfs_request_spec.rb

@@ -16,13 +16,17 @@ describe LfsRequest do
     end
 
     def project
-      @project ||= Project.find(params[:id])
+      @project ||= Project.find_by(id: params[:id])
     end
 
     def download_request?
       true
     end
 
+    def upload_request?
+      false
+    end
+
     def ci?
       false
     end
@@ -49,4 +53,41 @@ describe LfsRequest do
       expect(assigns(:storage_project)).to eq(project)
     end
   end
+
+  context 'user is authenticated without access to lfs' do
+    before do
+      allow(controller).to receive(:authenticate_user)
+      allow(controller).to receive(:authentication_result) do
+        Gitlab::Auth::Result.new
+      end
+    end
+
+    context 'with access to the project' do
+      it 'returns 403' do
+        get :show, params: { id: project.id }
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'without access to the project' do
+      context 'project does not exist' do
+        it 'returns 404' do
+          get :show, params: { id: 'does not exist' }
+
+          expect(response.status).to eq(404)
+        end
+      end
+
+      context 'project is private' do
+        let(:project) { create(:project, :private) }
+
+        it 'returns 404' do
+          get :show, params: { id: project.id }
+
+          expect(response.status).to eq(404)
+        end
+      end
+    end
+  end
 end

spec/controllers/projects/autocomplete_sources_controller_spec.rb

@@ -8,6 +8,10 @@ describe Projects::AutocompleteSourcesController do
   let_it_be(:issue) { create(:issue, project: project) }
   let_it_be(:user) { create(:user) }
 
+  def members_by_username(username)
+    json_response.find { |member| member['username'] == username }
+  end
+
   describe 'GET members' do
     before do
       group.add_owner(user)
@@ -17,22 +21,21 @@ describe Projects::AutocompleteSourcesController do
     it 'returns an array of member object' do
       get :members, format: :json, params: { namespace_id: group.path, project_id: project.path, type: issue.class.name, type_id: issue.id }
 
-      all = json_response.find {|member| member["username"] == 'all'}
-      the_group = json_response.find {|member| member["username"] == group.full_path}
-      the_user = json_response.find {|member| member["username"] == user.username}
-
-      expect(all.symbolize_keys).to include(username: 'all',
-                                            name: 'All Project and Group Members',
-                                            count: 1)
-
-      expect(the_group.symbolize_keys).to include(type: group.class.name,
-                                                  name: group.full_name,
-                                                  avatar_url: group.avatar_url,
-                                                  count: 1)
-
-      expect(the_user.symbolize_keys).to include(type: user.class.name,
-                                                 name: user.name,
-                                                 avatar_url: user.avatar_url)
+      expect(members_by_username('all').symbolize_keys).to include(
+        username: 'all',
+        name: 'All Project and Group Members',
+        count: 1)
+
+      expect(members_by_username(group.full_path).symbolize_keys).to include(
+        type: group.class.name,
+        name: group.full_name,
+        avatar_url: group.avatar_url,
+        count: 1)
+
+      expect(members_by_username(user.username).symbolize_keys).to include(
+        type: user.class.name,
+        name: user.name,
+        avatar_url: user.avatar_url)
     end
   end
 

spec/controllers/projects/commits_controller_spec.rb

@@ -142,7 +142,7 @@ describe Projects::CommitsController do
 
   context 'token authentication' do
     context 'public project' do
-      it_behaves_like 'authenticates sessionless user', :show, :atom, public: true do
+      it_behaves_like 'authenticates sessionless user', :show, :atom, { public: true, ignore_incrementing: true } do
         before do
           public_project = create(:project, :repository, :public)
 
@@ -152,7 +152,7 @@ describe Projects::CommitsController do
     end
 
     context 'private project' do
-      it_behaves_like 'authenticates sessionless user', :show, :atom, public: false do
+      it_behaves_like 'authenticates sessionless user', :show, :atom, { public: false, ignore_incrementing: true } do
         before do
           private_project = create(:project, :repository, :private)
           private_project.add_maintainer(user)

spec/controllers/projects/error_tracking_controller_spec.rb

@@ -146,7 +146,7 @@ describe Projects::ErrorTrackingController do
       it 'redirects to sign-in page' do
         post :list_projects, params: list_projects_params
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+        expect(response).to have_gitlab_http_status(:redirect)
       end
     end
 

spec/controllers/projects/issues_controller_spec.rb

@@ -1441,7 +1441,7 @@ describe Projects::IssuesController do
   context 'private project with token authentication' do
     let(:private_project) { create(:project, :private) }
 
-    it_behaves_like 'authenticates sessionless user', :index, :atom do
+    it_behaves_like 'authenticates sessionless user', :index, :atom, ignore_incrementing: true do
       before do
         default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
 
@@ -1449,7 +1449,7 @@ describe Projects::IssuesController do
       end
     end
 
-    it_behaves_like 'authenticates sessionless user', :calendar, :ics do
+    it_behaves_like 'authenticates sessionless user', :calendar, :ics, ignore_incrementing: true do
       before do
         default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
 

spec/controllers/projects/tags_controller_spec.rb

@@ -41,7 +41,7 @@ describe Projects::TagsController do
   context 'private project with token authentication' do
     let(:private_project) { create(:project, :repository, :private) }
 
-    it_behaves_like 'authenticates sessionless user', :index, :atom do
+    it_behaves_like 'authenticates sessionless user', :index, :atom, ignore_incrementing: true do
       before do
         default_params.merge!(project_id: private_project, namespace_id: private_project.namespace)
 

spec/controllers/projects_controller_spec.rb

@@ -1149,7 +1149,7 @@ describe ProjectsController do
   context 'private project with token authentication' do
     let(:private_project) { create(:project, :private) }
 
-    it_behaves_like 'authenticates sessionless user', :show, :atom do
+    it_behaves_like 'authenticates sessionless user', :show, :atom, ignore_incrementing: true do
       before do
         default_params.merge!(id: private_project, namespace_id: private_project.namespace)
 

spec/features/projects/pipelines/pipelines_spec.rb

@@ -819,7 +819,10 @@ describe 'Pipelines', :js do
     context 'when project is private' do
       let(:project) { create(:project, :private, :repository) }
 
-      it { expect(page).to have_content 'You need to sign in' }
+      it 'redirects the user to sign_in and displays the flash alert' do
+        expect(page).to have_content 'You need to sign in'
+        expect(page.current_path).to eq("/users/sign_in")
+      end
     end
   end
 

spec/features/projects/tags/user_views_tags_spec.rb

@@ -15,7 +15,7 @@ describe 'User views tags', :feature do
       it do
         visit project_tags_path(project, format: :atom)
 
-        expect(page).to have_gitlab_http_status(401)
+        expect(page.current_path).to eq("/users/sign_in")
       end
     end
 

spec/finders/labels_finder_spec.rb

@@ -128,6 +128,89 @@ describe LabelsFinder do
           expect(finder.execute).to eq [private_subgroup_label_1]
         end
       end
+
+      context 'when including labels from group projects with limited visibility' do
+        let(:finder)                     { described_class.new(user, group_id: group_4.id) }
+        let(:group_4)                    { create(:group) }
+        let(:limited_visibility_project) { create(:project, :public, group: group_4) }
+        let(:visible_project)            { create(:project, :public, group: group_4) }
+        let!(:group_label_1)             { create(:group_label, group: group_4) }
+        let!(:limited_visibility_label)  { create(:label, project: limited_visibility_project) }
+        let!(:visible_label)             { create(:label, project: visible_project) }
+
+        shared_examples 'with full visibility' do
+          it 'returns all projects labels' do
+            expect(finder.execute).to eq [group_label_1, limited_visibility_label, visible_label]
+          end
+        end
+
+        shared_examples 'with limited visibility' do
+          it 'returns only authorized projects labels' do
+            expect(finder.execute).to eq [group_label_1, visible_label]
+          end
+        end
+
+        context 'when merge requests and issues are not visible for non members' do
+          before do
+            limited_visibility_project.project_feature.update!(
+              merge_requests_access_level: ProjectFeature::PRIVATE,
+              issues_access_level: ProjectFeature::PRIVATE
+            )
+          end
+
+          context 'when user is not a group member' do
+            it_behaves_like 'with limited visibility'
+          end
+
+          context 'when user is a group member' do
+            before do
+              group_4.add_developer(user)
+            end
+
+            it_behaves_like 'with full visibility'
+          end
+        end
+
+        context 'when merge requests are not visible for non members' do
+          before do
+            limited_visibility_project.project_feature.update!(
+              merge_requests_access_level: ProjectFeature::PRIVATE
+            )
+          end
+
+          context 'when user is not a group member' do
+            it_behaves_like 'with full visibility'
+          end
+
+          context 'when user is a group member' do
+            before do
+              group_4.add_developer(user)
+            end
+
+            it_behaves_like 'with full visibility'
+          end
+        end
+
+        context 'when issues are not visible for non members' do
+          before do
+            limited_visibility_project.project_feature.update!(
+              issues_access_level: ProjectFeature::PRIVATE
+            )
+          end
+
+          context 'when user is not a group member' do
+            it_behaves_like 'with full visibility'
+          end
+
+          context 'when user is a group member' do
+            before do
+              group_4.add_developer(user)
+            end
+
+            it_behaves_like 'with full visibility'
+          end
+        end
+      end
     end
 
     context 'filtering by project_id' do

spec/fixtures/api/graphql/recursive-introspection.graphql

+query allSchemaTypes {
+  __schema {
+    types {
+      fields {
+        type{
+          fields {
+            type {
+              fields {
+                type {
+                  fields {
+                    type {
+                      fields {
+                        type {
+                          fields {
+                            type {
+                              fields {
+                                type {
+                                  fields {
+                                    type {
+                                      fields {
+                                        type {
+                                          fields {
+                                            type {
+                                              fields {
+                                                type {
+                                                  fields {
+                                                    type {
+                                                      fields {
+                                                        name
+                                                      }
+                                                    }
+                                                  }
+                                                }
+                                              }
+                                            }
+                                          }
+                                        }
+                                      }
+                                    }
+                                  }
+                                }
+                              }
+                            }
+                          }
+                        }
+                      }
+                    }
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+}
\ No newline at end of file

spec/fixtures/api/graphql/recursive-query.graphql

+{
+  project(fullPath: "gitlab-org/gitlab-ce") {
+    group {
+      projects {
+        edges {
+          node {
+            group {
+              projects {
+                edges {
+                  node {
+                    group {
+                      projects {
+                        edges {
+                          node {
+                            group {
+                              projects {
+                                edges {
+                                  node {
+                                    group {
+                                      projects {
+                                        edges {
+                                          node {
+                                            group {
+                                              description
+                                            }
+                                          }
+                                        }
+                                      }
+                                    }
+                                  }
+                                }
+                              }
+                            }
+                          }
+                        }
+                      }
+                    }
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+}

spec/fixtures/api/graphql/small-recursive-introspection.graphql

+query allSchemaTypes {
+  __schema {
+    types {
+      fields {
+        type {
+          fields {
+            type {
+              name
+            }
+          }
+        }
+      }
+    }
+  }
+}

spec/frontend/project_find_file_spec.js

@@ -3,6 +3,9 @@ import $ from 'jquery';
 import ProjectFindFile from '~/project_find_file';
 import axios from '~/lib/utils/axios_utils';
 import { TEST_HOST } from 'helpers/test_constants';
+import sanitize from 'sanitize-html';
+
+jest.mock('sanitize-html', () => jest.fn(val => val));
 
 const BLOB_URL_TEMPLATE = `${TEST_HOST}/namespace/project/blob/master`;
 const FILE_FIND_URL = `${TEST_HOST}/namespace/project/files/master?format=json`;
@@ -38,31 +41,31 @@ describe('ProjectFindFile', () => {
         href: el.querySelector('a').href,
       }));
 
+  const files = [
+    'fileA.txt',
+    'fileB.txt',
+    'fi#leC.txt',
+    'folderA/fileD.txt',
+    'folder#B/fileE.txt',
+    'folde?rC/fil#F.txt',
+  ];
+
   beforeEach(() => {
     // Create a mock adapter for stubbing axios API requests
     mock = new MockAdapter(axios);
 
     element = $(TEMPLATE);
+    mock.onGet(FILE_FIND_URL).replyOnce(200, files);
+    getProjectFindFileInstance(); // This triggers a load / axios call + subsequent render in the constructor
   });
 
   afterEach(() => {
     // Reset the mock adapter
     mock.restore();
+    sanitize.mockClear();
   });
 
   it('loads and renders elements from remote server', done => {
-    const files = [
-      'fileA.txt',
-      'fileB.txt',
-      'fi#leC.txt',
-      'folderA/fileD.txt',
-      'folder#B/fileE.txt',
-      'folde?rC/fil#F.txt',
-    ];
-    mock.onGet(FILE_FIND_URL).replyOnce(200, files);
-
-    getProjectFindFileInstance(); // This triggers a load / axios call + subsequent render in the constructor
-
     setImmediate(() => {
       expect(findFiles()).toEqual(
         files.map(text => ({
@@ -74,4 +77,14 @@ describe('ProjectFindFile', () => {
       done();
     });
   });
+
+  it('sanitizes search text', done => {
+    const searchText = element.find('.file-finder-input').val();
+
+    setImmediate(() => {
+      expect(sanitize).toHaveBeenCalledTimes(1);
+      expect(sanitize).toHaveBeenCalledWith(searchText);
+      done();
+    });
+  });
 });

spec/helpers/markup_helper_spec.rb

@@ -3,18 +3,18 @@
 require 'spec_helper'
 
 describe MarkupHelper do
-  let!(:project) { create(:project, :repository) }
-
-  let(:user)          { create(:user, username: 'gfm') }
-  let(:commit)        { project.commit }
-  let(:issue)         { create(:issue, project: project) }
-  let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
-  let(:snippet)       { create(:project_snippet, project: project) }
-
-  before do
-    # Ensure the generated reference links aren't redacted
+  set(:project) { create(:project, :repository) }
+  set(:user) do
+    user = create(:user, username: 'gfm')
     project.add_maintainer(user)
+    user
+  end
+  set(:issue) { create(:issue, project: project) }
+  set(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+  set(:snippet) { create(:project_snippet, project: project) }
+  let(:commit) { project.commit }
 
+  before do
     # Helper expects a @project instance variable
     helper.instance_variable_set(:@project, project)
 
@@ -44,8 +44,8 @@ describe MarkupHelper do
 
     describe "override default project" do
       let(:actual) { issue.to_reference }
-      let(:second_project) { create(:project, :public) }
-      let(:second_issue) { create(:issue, project: second_project) }
+      set(:second_project) { create(:project, :public) }
+      set(:second_issue) { create(:issue, project: second_project) }
 
       it 'links to the issue' do
         expected = urls.project_issue_path(second_project, second_issue)
@@ -55,7 +55,7 @@ describe MarkupHelper do
 
     describe 'uploads' do
       let(:text) { "![ImageTest](/uploads/test.png)" }
-      let(:group) { create(:group) }
+      set(:group) { create(:group) }
 
       subject { helper.markdown(text) }
 
@@ -77,7 +77,7 @@ describe MarkupHelper do
       end
 
       describe "with a group in the context" do
-        let(:project_in_group) { create(:project, group: group) }
+        set(:project_in_group) { create(:project, group: group) }
 
         before do
           helper.instance_variable_set(:@group, group)
@@ -235,38 +235,48 @@ describe MarkupHelper do
   end
 
   describe '#render_wiki_content' do
+    let(:wiki) { double('WikiPage', path: "file.#{extension}") }
+    let(:context) do
+      {
+        pipeline: :wiki, project: project, project_wiki: wiki,
+        page_slug: 'nested/page', issuable_state_filter_enabled: true
+      }
+    end
+
     before do
-      @wiki = double('WikiPage')
-      allow(@wiki).to receive(:content).and_return('wiki content')
-      allow(@wiki).to receive(:slug).and_return('nested/page')
-      helper.instance_variable_set(:@project_wiki, @wiki)
+      expect(wiki).to receive(:content).and_return('wiki content')
+      expect(wiki).to receive(:slug).and_return('nested/page')
+      helper.instance_variable_set(:@project_wiki, wiki)
     end
 
-    it "uses Wiki pipeline for markdown files" do
-      allow(@wiki).to receive(:format).and_return(:markdown)
+    context 'when file is Markdown' do
+      let(:extension) { 'md' }
 
-      expect(helper).to receive(:markdown_unsafe).with('wiki content',
-        pipeline: :wiki, project: project, project_wiki: @wiki, page_slug: "nested/page",
-        issuable_state_filter_enabled: true)
+      it 'renders using #markdown_unsafe helper method' do
+        expect(helper).to receive(:markdown_unsafe).with('wiki content', context)
 
-      helper.render_wiki_content(@wiki)
+        helper.render_wiki_content(wiki)
+      end
     end
 
-    it "uses Asciidoctor for asciidoc files" do
-      allow(@wiki).to receive(:format).and_return(:asciidoc)
+    context 'when file is Asciidoc' do
+      let(:extension) { 'adoc' }
 
-      expect(helper).to receive(:asciidoc_unsafe).with('wiki content')
+      it 'renders using Gitlab::Asciidoc' do
+        expect(Gitlab::Asciidoc).to receive(:render)
 
-      helper.render_wiki_content(@wiki)
+        helper.render_wiki_content(wiki)
+      end
     end
 
-    it "uses the Gollum renderer for all other file types" do
-      allow(@wiki).to receive(:format).and_return(:rdoc)
-      formatted_content_stub = double('formatted_content')
-      expect(formatted_content_stub).to receive(:html_safe)
-      allow(@wiki).to receive(:formatted_content).and_return(formatted_content_stub)
+    context 'any other format' do
+      let(:extension) { 'foo' }
 
-      helper.render_wiki_content(@wiki)
+      it 'renders all other formats using Gitlab::OtherMarkup' do
+        expect(Gitlab::OtherMarkup).to receive(:render)
+
+        helper.render_wiki_content(wiki)
+      end
     end
   end
 

spec/models/milestone_spec.rb

@@ -227,6 +227,14 @@ describe Milestone do
     end
   end
 
+  describe '#to_ability_name' do
+    it 'returns milestone' do
+      milestone = build(:milestone)
+
+      expect(milestone.to_ability_name).to eq('milestone')
+    end
+  end
+
   describe '.search' do
     let(:milestone) { create(:milestone, title: 'foo', description: 'bar') }
 

spec/models/note_spec.rb

@@ -379,6 +379,63 @@ describe Note do
         expect(label_note.cross_reference?).to be_falsy
       end
     end
+
+    context 'when system note metadata is not present' do
+      let(:note) { build(:note, :system) }
+
+      before do
+        allow(note).to receive(:system_note_metadata).and_return(nil)
+      end
+
+      it 'delegates to the system note service' do
+        expect(SystemNotes::IssuablesService).to receive(:cross_reference?).with(note.note)
+
+        note.cross_reference?
+      end
+    end
+
+    context 'with a system note' do
+      let(:issue)     { create(:issue, project: create(:project, :repository)) }
+      let(:note)      { create(:system_note, note: "test", noteable: issue, project: issue.project) }
+
+      shared_examples 'system_note_metadata includes note action' do
+        it 'delegates to the cross-reference regex' do
+          expect(note).to receive(:matches_cross_reference_regex?)
+
+          note.cross_reference?
+        end
+      end
+
+      context 'with :label action' do
+        let!(:metadata) {create(:system_note_metadata, note: note, action: :label)}
+
+        it_behaves_like 'system_note_metadata includes note action'
+
+        it { expect(note.cross_reference?).to be_falsy }
+
+        context 'with cross reference label note' do
+          let(:label) { create(:label, project: issue.project)}
+          let(:note) { create(:system_note, note: "added #{label.to_reference} label", noteable: issue, project: issue.project) }
+
+          it { expect(note.cross_reference?).to be_truthy }
+        end
+      end
+
+      context 'with :milestone action' do
+        let!(:metadata) {create(:system_note_metadata, note: note, action: :milestone)}
+
+        it_behaves_like 'system_note_metadata includes note action'
+
+        it { expect(note.cross_reference?).to be_falsy }
+
+        context 'with cross reference milestone note' do
+          let(:milestone) { create(:milestone, project: issue.project)}
+          let(:note) { create(:system_note, note: "added #{milestone.to_reference} milestone", noteable: issue, project: issue.project) }
+
+          it { expect(note.cross_reference?).to be_truthy }
+        end
+      end
+    end
   end
 
   describe 'clear_blank_line_code!' do
@@ -578,24 +635,30 @@ describe Note do
   end
 
   describe '#to_ability_name' do
-    it 'returns snippet for a project snippet note' do
-      expect(build(:note_on_project_snippet).to_ability_name).to eq('project_snippet')
+    it 'returns note' do
+      expect(build(:note).to_ability_name).to eq('note')
+    end
+  end
+
+  describe '#noteable_ability_name' do
+    it 'returns project_snippet for a project snippet note' do
+      expect(build(:note_on_project_snippet).noteable_ability_name).to eq('project_snippet')
     end
 
     it 'returns personal_snippet for a personal snippet note' do
-      expect(build(:note_on_personal_snippet).to_ability_name).to eq('personal_snippet')
+      expect(build(:note_on_personal_snippet).noteable_ability_name).to eq('personal_snippet')
     end
 
     it 'returns merge_request for an MR note' do
-      expect(build(:note_on_merge_request).to_ability_name).to eq('merge_request')
+      expect(build(:note_on_merge_request).noteable_ability_name).to eq('merge_request')
     end
 
     it 'returns issue for an issue note' do
-      expect(build(:note_on_issue).to_ability_name).to eq('issue')
+      expect(build(:note_on_issue).noteable_ability_name).to eq('issue')
     end
 
-    it 'returns issue for a commit note' do
-      expect(build(:note_on_commit).to_ability_name).to eq('commit')
+    it 'returns commit for a commit note' do
+      expect(build(:note_on_commit).noteable_ability_name).to eq('commit')
     end
   end
 

spec/models/project_spec.rb

@@ -3416,7 +3416,7 @@ describe Project do
     end
   end
 
-  describe '.ids_with_milestone_available_for' do
+  describe '.ids_with_issuables_available_for' do
     let!(:user) { create(:user) }
 
     it 'returns project ids with milestones available for user' do
@@ -3426,7 +3426,7 @@ describe Project do
       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)
+      project_ids = described_class.ids_with_issuables_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)
@@ -4444,6 +4444,14 @@ describe Project do
     end
   end
 
+  describe '#to_ability_name' do
+    it 'returns project' do
+      project = build(:project_empty_repo)
+
+      expect(project.to_ability_name).to eq('project')
+    end
+  end
+
   describe '#execute_hooks' do
     let(:data) { { ref: 'refs/heads/master', data: 'data' } }
     it 'executes active projects hooks with the specified scope' do

spec/models/wiki_page_spec.rb

@@ -358,6 +358,23 @@ describe WikiPage do
     end
   end
 
+  describe '#path' do
+    let(:path) { 'mypath.md' }
+    let(:wiki_page) { instance_double('Gitlab::Git::WikiPage', path: path).as_null_object }
+
+    it 'returns the path when persisted' do
+      page = described_class.new(wiki, wiki_page, true)
+
+      expect(page.path).to eq(path)
+    end
+
+    it 'returns nil when not persisted' do
+      page = described_class.new(wiki, wiki_page, false)
+
+      expect(page.path).to be_nil
+    end
+  end
+
   describe '#directory' do
     context 'when the page is at the root directory' do
       it 'returns an empty string' do

spec/policies/commit_policy_spec.rb

@@ -8,28 +8,42 @@ describe CommitPolicy do
     let(:commit) { project.repository.head_commit }
     let(:policy) { described_class.new(user, commit) }
 
+    shared_examples 'can read commit and create a note' do
+      it 'can read commit' do
+        expect(policy).to be_allowed(:read_commit)
+      end
+
+      it 'can create a note' do
+        expect(policy).to be_allowed(:create_note)
+      end
+    end
+
+    shared_examples 'cannot read commit nor create a note' do
+      it 'can not read commit' do
+        expect(policy).to be_disallowed(:read_commit)
+      end
+
+      it 'can not create a note' do
+        expect(policy).to be_disallowed(:create_note)
+      end
+    end
+
     context 'when project is public' do
       let(:project) { create(:project, :public, :repository) }
 
-      it 'can read commit and create a note' do
-        expect(policy).to be_allowed(:read_commit)
-      end
+      it_behaves_like 'can read commit and create a note'
 
       context 'when repository access level is private' do
         let(:project) { create(:project, :public, :repository, :repository_private) }
 
-        it 'can not read commit and create a note' do
-          expect(policy).to be_disallowed(:read_commit)
-        end
+        it_behaves_like 'cannot read commit nor create a note'
 
         context 'when the user is a project member' do
           before do
             project.add_developer(user)
           end
 
-          it 'can read commit and create a note' do
-            expect(policy).to be_allowed(:read_commit)
-          end
+          it_behaves_like 'can read commit and create a note'
         end
       end
     end
@@ -37,9 +51,7 @@ describe CommitPolicy do
     context 'when project is private' do
       let(:project) { create(:project, :private, :repository) }
 
-      it 'can not read commit and create a note' do
-        expect(policy).to be_disallowed(:read_commit)
-      end
+      it_behaves_like 'cannot read commit nor create a note'
 
       context 'when the user is a project member' do
         before do
@@ -50,6 +62,18 @@ describe CommitPolicy do
           expect(policy).to be_allowed(:read_commit)
         end
       end
+
+      context 'when the user is a guest' do
+        before do
+          project.add_guest(user)
+        end
+
+        it_behaves_like 'cannot read commit nor create a note'
+
+        it 'cannot download code' do
+          expect(policy).to be_disallowed(:download_code)
+        end
+      end
     end
   end
 end

spec/policies/group_policy_spec.rb

@@ -356,6 +356,88 @@ describe GroupPolicy do
     end
   end
 
+  context 'transfer_projects' do
+    shared_examples_for 'allowed to transfer projects' do
+      before do
+        group.update(project_creation_level: project_creation_level)
+      end
+
+      it { is_expected.to be_allowed(:transfer_projects) }
+    end
+
+    shared_examples_for 'not allowed to transfer projects' do
+      before do
+        group.update(project_creation_level: project_creation_level)
+      end
+
+      it { is_expected.to be_disallowed(:transfer_projects) }
+    end
+
+    context 'reporter' do
+      let(:current_user) { reporter }
+
+      it_behaves_like 'not allowed to transfer projects' do
+        let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS }
+      end
+
+      it_behaves_like 'not allowed to transfer projects' do
+        let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS }
+      end
+
+      it_behaves_like 'not allowed to transfer projects' do
+        let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS }
+      end
+    end
+
+    context 'developer' do
+      let(:current_user) { developer }
+
+      it_behaves_like 'not allowed to transfer projects' do
+        let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS }
+      end
+
+      it_behaves_like 'not allowed to transfer projects' do
+        let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS }
+      end
+
+      it_behaves_like 'not allowed to transfer projects' do
+        let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS }
+      end
+    end
+
+    context 'maintainer' do
+      let(:current_user) { maintainer }
+
+      it_behaves_like 'not allowed to transfer projects' do
+        let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS }
+      end
+
+      it_behaves_like 'allowed to transfer projects' do
+        let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS }
+      end
+
+      it_behaves_like 'allowed to transfer projects' do
+        let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS }
+      end
+    end
+
+    context 'owner' do
+      let(:current_user) { owner }
+
+      it_behaves_like 'not allowed to transfer projects' do
+        let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS }
+      end
+
+      it_behaves_like 'allowed to transfer projects' do
+        let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS }
+      end
+
+      it_behaves_like 'allowed to transfer projects' do
+        let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS }
+      end
+    end
+  end
+
   context "create_projects" do
     context 'when group has no project creation level set' do
       before_all do

spec/policies/namespace_policy_spec.rb

@@ -8,7 +8,7 @@ describe NamespacePolicy do
   let(:admin) { create(:admin) }
   let(:namespace) { create(:namespace, owner: owner) }
 
-  let(:owner_permissions) { [:create_projects, :admin_namespace, :read_namespace, :read_statistics] }
+  let(:owner_permissions) { [:create_projects, :admin_namespace, :read_namespace, :read_statistics, :transfer_projects] }
 
   subject { described_class.new(current_user, namespace) }
 
@@ -33,6 +33,7 @@ describe NamespacePolicy do
       let(:owner) { create(:user, projects_limit: 0) }
 
       it { is_expected.to be_disallowed(:create_projects) }
+      it { is_expected.to be_disallowed(:transfer_projects) }
     end
   end
 

spec/requests/api/graphql/gitlab_schema_spec.rb

@@ -15,7 +15,7 @@ describe 'GitlabSchema configurations' do
 
           subject
 
-          expect(graphql_errors.flatten.first['message']).to include('which exceeds max complexity of 1')
+          expect_graphql_errors_to_include /which exceeds max complexity of 1/
         end
       end
     end
@@ -23,12 +23,11 @@ describe 'GitlabSchema configurations' do
     describe '#max_depth' do
       context 'when query depth is too high' do
         it 'shows error' do
-          errors = { "message" => "Query has depth of 2, which exceeds max depth of 1" }
           allow(GitlabSchema).to receive(:max_query_depth).and_return 1
 
           subject
 
-          expect(graphql_errors.flatten).to include(errors)
+          expect_graphql_errors_to_include /exceeds max depth/
         end
       end
 
@@ -38,7 +37,42 @@ describe 'GitlabSchema configurations' do
 
           subject
 
-          expect(Array.wrap(graphql_errors).compact).to be_empty
+          expect_graphql_errors_to_be_empty
+        end
+      end
+    end
+  end
+
+  context 'depth, complexity and recursion checking' do
+    context 'unauthenticated recursive queries' do
+      context 'a not-quite-recursive-enough introspective query' do
+        it 'succeeds' do
+          query = File.read(Rails.root.join('spec/fixtures/api/graphql/small-recursive-introspection.graphql'))
+
+          post_graphql(query, current_user: nil)
+
+          expect_graphql_errors_to_be_empty
+        end
+      end
+
+      context 'a deep but simple recursive introspective query' do
+        it 'fails due to recursion' do
+          query = File.read(Rails.root.join('spec/fixtures/api/graphql/recursive-introspection.graphql'))
+
+          post_graphql(query, current_user: nil)
+
+          expect_graphql_errors_to_include [/Recursive query/]
+        end
+      end
+
+      context 'a deep recursive non-introspective query' do
+        it 'fails due to recursion, complexity and depth' do
+          allow(GitlabSchema).to receive(:max_query_complexity).and_return 1
+          query = File.read(Rails.root.join('spec/fixtures/api/graphql/recursive-query.graphql'))
+
+          post_graphql(query, current_user: nil)
+
+          expect_graphql_errors_to_include [/Recursive query/, /exceeds max complexity/, /exceeds max depth/]
         end
       end
     end
@@ -88,7 +122,7 @@ describe 'GitlabSchema configurations' do
         # Expect errors for each query
         expect(graphql_errors.size).to eq(3)
         graphql_errors.each do |single_query_errors|
-          expect(single_query_errors.first['message']).to include('which exceeds max complexity of 4')
+          expect_graphql_errors_to_include(/which exceeds max complexity of 4/)
         end
       end
     end
@@ -105,12 +139,12 @@ describe 'GitlabSchema configurations' do
   end
 
   context 'when IntrospectionQuery' do
-    it 'is not too complex' do
+    it 'is not too complex nor recursive' do
       query = File.read(Rails.root.join('spec/fixtures/api/graphql/introspection.graphql'))
 
       post_graphql(query, current_user: nil)
 
-      expect(graphql_errors).to be_nil
+      expect_graphql_errors_to_be_empty
     end
   end
 

spec/requests/api/merge_requests_spec.rb

@@ -1741,6 +1741,38 @@ describe API::MergeRequests do
         expect(json_response['state']).to eq('closed')
         expect(json_response['force_remove_source_branch']).to be_truthy
       end
+
+      context 'with a merge request across forks' do
+        let(:fork_owner) { create(:user) }
+        let(:source_project) { fork_project(project, fork_owner) }
+        let(:target_project) { project }
+
+        let(:merge_request) do
+          create(:merge_request,
+                 source_project: source_project,
+                 target_project: target_project,
+                 source_branch: 'fixes',
+                 merge_params: { 'force_remove_source_branch' => false })
+        end
+
+        it 'is true for an authorized user' do
+          put api("/projects/#{target_project.id}/merge_requests/#{merge_request.iid}", fork_owner), params: { state_event: 'close', remove_source_branch: true }
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response['state']).to eq('closed')
+          expect(json_response['force_remove_source_branch']).to be true
+        end
+
+        it 'is false for an unauthorized user' do
+          expect do
+            put api("/projects/#{target_project.id}/merge_requests/#{merge_request.iid}", target_project.owner), params: { state_event: 'close', remove_source_branch: true }
+          end.not_to change { merge_request.reload.merge_params }
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response['state']).to eq('closed')
+          expect(json_response['force_remove_source_branch']).to be false
+        end
+      end
     end
 
     context "to close a MR" do

spec/requests/api/projects_spec.rb

@@ -2684,6 +2684,22 @@ describe API::Projects do
         expect(response).to have_gitlab_http_status(400)
       end
     end
+
+    context 'when authenticated as developer' do
+      before do
+        group.add_developer(user)
+      end
+
+      context 'target namespace allows developers to create projects' do
+        let(:group) { create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) }
+
+        it 'fails transferring the project to the target namespace' do
+          put api("/projects/#{project.id}/transfer", user), params: { namespace: group.id }
+
+          expect(response).to have_gitlab_http_status(400)
+        end
+      end
+    end
   end
 
   it_behaves_like 'custom attributes endpoints', 'projects' do

spec/services/auto_merge/base_service_spec.rb

@@ -51,7 +51,7 @@ describe AutoMerge::BaseService do
         expect(merge_request.merge_params['commit_message']).to eq("Merge branch 'patch-12' into 'master'")
         expect(merge_request.merge_params['sha']).to eq('200fcc9c260f7219eaf0daba87d818f0922c5b18')
         expect(merge_request.merge_params['should_remove_source_branch']).to eq(false)
-        expect(merge_request.merge_params['squash']).to eq(false)
+        expect(merge_request.squash).to eq(false)
         expect(merge_request.merge_params['squash_commit_message']).to eq('Update README.md')
       end
     end
@@ -108,7 +108,6 @@ describe AutoMerge::BaseService do
           'commit_message' => "Merge branch 'patch-12' into 'master'",
           'sha' => "200fcc9c260f7219eaf0daba87d818f0922c5b18",
           'should_remove_source_branch' => false,
-          'squash' => false,
           'squash_commit_message' => "Update README.md"
         }
       end

spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb

@@ -59,8 +59,9 @@ describe AutoMerge::MergeWhenPipelineSucceedsService do
       it 'sets the params, merge_user, and flag' do
         expect(merge_request).to be_valid
         expect(merge_request.merge_when_pipeline_succeeds).to be_truthy
-        expect(merge_request.merge_params).to include commit_message: 'Awesome message'
+        expect(merge_request.merge_params).to include 'commit_message' => 'Awesome message'
         expect(merge_request.merge_user).to be user
+        expect(merge_request.auto_merge_strategy).to eq AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS
       end
 
       it 'creates a system note' do
@@ -73,7 +74,7 @@ describe AutoMerge::MergeWhenPipelineSucceedsService do
     end
 
     context 'already approved' do
-      let(:service) { described_class.new(project, user, new_key: true) }
+      let(:service) { described_class.new(project, user, should_remove_source_branch: true) }
       let(:build)   { create(:ci_build, ref: mr_merge_if_green_enabled.source_branch) }
 
       before do
@@ -90,7 +91,7 @@ describe AutoMerge::MergeWhenPipelineSucceedsService do
         expect(SystemNoteService).not_to receive(:merge_when_pipeline_succeeds)
 
         service.execute(mr_merge_if_green_enabled)
-        expect(mr_merge_if_green_enabled.merge_params).to have_key(:new_key)
+        expect(mr_merge_if_green_enabled.merge_params).to have_key('should_remove_source_branch')
       end
     end
   end

spec/services/concerns/merge_requests/assigns_merge_params_spec.rb

+require 'spec_helper'
+
+describe MergeRequests::AssignsMergeParams do
+  it 'raises an error when used from an instance that does not respond to #current_user' do
+    define_class = -> { Class.new { include MergeRequests::AssignsMergeParams }.new }
+
+    expect { define_class.call }.to raise_error %r{can not be included in (.*) without implementing #current_user}
+  end
+
+  describe '#assign_allowed_merge_params' do
+    let(:merge_request) { build(:merge_request) }
+
+    let(:params) do
+      { commit_message: 'Commit Message',
+        'should_remove_source_branch' => true,
+        unknown_symbol: 'Unknown symbol',
+        'unknown_string' => 'Unknown String' }
+    end
+
+    subject(:merge_request_service) do
+      Class.new do
+        attr_accessor :current_user
+
+        include MergeRequests::AssignsMergeParams
+
+        def initialize(user)
+          @current_user = user
+        end
+      end
+    end
+
+    it 'only assigns known parameters to the merge request' do
+      service = merge_request_service.new(merge_request.author)
+
+      service.assign_allowed_merge_params(merge_request, params)
+
+      expect(merge_request.merge_params).to eq('commit_message' => 'Commit Message', 'should_remove_source_branch' => true)
+    end
+
+    it 'returns a hash without the known merge params' do
+      service = merge_request_service.new(merge_request.author)
+
+      result = service.assign_allowed_merge_params(merge_request, params)
+
+      expect(result).to eq({ 'unknown_symbol' => 'Unknown symbol', 'unknown_string' => 'Unknown String' })
+    end
+
+    context 'the force_remove_source_branch param' do
+      let(:params) { { force_remove_source_branch: true } }
+
+      it 'assigns the param if the user is allowed to do that' do
+        service = merge_request_service.new(merge_request.author)
+
+        result = service.assign_allowed_merge_params(merge_request, params)
+
+        expect(merge_request.force_remove_source_branch?).to be true
+        expect(result).to be_empty
+      end
+
+      it 'only removes the param if the user is not allowed to do that' do
+        service = merge_request_service.new(build(:user))
+
+        result = service.assign_allowed_merge_params(merge_request, params)
+
+        expect(merge_request.force_remove_source_branch?).to be_falsy
+        expect(result).to be_empty
+      end
+    end
+  end
+end

spec/services/error_tracking/list_projects_service_spec.rb

@@ -50,6 +50,19 @@ describe ErrorTracking::ListProjectsService do
         end
       end
 
+      context 'masked param token' do
+        let(:params) { ActionController::Parameters.new(token: "*********", api_host: new_api_host) }
+
+        before do
+          expect(error_tracking_setting).to receive(:list_sentry_projects)
+            .and_return({ projects: [] })
+        end
+
+        it 'uses database token' do
+          expect { subject.execute }.not_to change { error_tracking_setting.token }
+        end
+      end
+
       context 'sentry client raises exception' do
         context 'Sentry::Client::Error' do
           before do

spec/services/merge_requests/build_service_spec.rb

@@ -83,8 +83,9 @@ describe MergeRequests::BuildService do
       expect(merge_request.force_remove_source_branch?).to be_truthy
     end
 
-    context 'with force_remove_source_branch parameter' do
+    context 'with force_remove_source_branch parameter when the user is authorized' do
       let(:mr_params) { params.merge(force_remove_source_branch: '1') }
+      let(:source_project) { fork_project(project, user) }
       let(:merge_request) { described_class.new(project, user, mr_params).execute }
 
       it 'assigns force_remove_source_branch' do

spec/services/merge_requests/update_service_spec.rb

@@ -646,5 +646,29 @@ describe MergeRequests::UpdateService, :mailer do
         expect(merge_request.allow_collaboration).to be_truthy
       end
     end
+
+    context 'updating `force_remove_source_branch`' do
+      let(:target_project) { create(:project, :repository, :public) }
+      let(:source_project) { fork_project(target_project, nil, repository: true) }
+      let(:user) { target_project.owner }
+      let(:merge_request) do
+        create(:merge_request,
+               source_project: source_project,
+               source_branch: 'fixes',
+               target_project: target_project)
+      end
+
+      it "cannot be done by members of the target project when they don't have access" do
+        expect { update_merge_request(force_remove_source_branch: true) }
+          .not_to change { merge_request.reload.force_remove_source_branch? }.from(nil)
+      end
+
+      it 'can be done by members of the target project if they can push to the source project' do
+        source_project.add_developer(user)
+
+        expect { update_merge_request(force_remove_source_branch: true) }
+          .to change { merge_request.reload.force_remove_source_branch? }.from(nil).to(true)
+      end
+    end
   end
 end

spec/services/projects/operations/update_service_spec.rb

@@ -145,6 +145,27 @@ describe Projects::Operations::UpdateService do
         end
       end
 
+      context 'with masked param token' do
+        let(:params) do
+          {
+            error_tracking_setting_attributes: {
+              enabled: false,
+              token: '*' * 8
+            }
+          }
+        end
+
+        before do
+          create(:project_error_tracking_setting, project: project, token: 'token')
+        end
+
+        it 'does not update token' do
+          expect(result[:status]).to eq(:success)
+
+          expect(project.error_tracking_setting.token).to eq('token')
+        end
+      end
+
       context 'with invalid parameters' do
         let(:params) { {} }
 

spec/services/projects/participants_service_spec.rb

@@ -57,4 +57,108 @@ describe Projects::ParticipantsService do
       end
     end
   end
+
+  describe '#project_members' do
+    subject(:usernames) { service.project_members.map { |member| member[:username] } }
+
+    context 'when there is a project in group namespace' do
+      set(:public_group) { create(:group, :public) }
+      set(:public_project) { create(:project, :public, namespace: public_group)}
+
+      set(:public_group_owner) { create(:user) }
+
+      let(:service) { described_class.new(public_project, create(:user)) }
+
+      before do
+        public_group.add_owner(public_group_owner)
+      end
+
+      it 'returns members of a group' do
+        expect(usernames).to include(public_group_owner.username)
+      end
+    end
+
+    context 'when there is a private group and a public project' do
+      set(:public_group) { create(:group, :public) }
+      set(:private_group) { create(:group, :private, :nested) }
+      set(:public_project) { create(:project, :public, namespace: public_group)}
+
+      set(:project_issue) { create(:issue, project: public_project)}
+
+      set(:public_group_owner) { create(:user) }
+      set(:private_group_member) { create(:user) }
+      set(:public_project_maintainer) { create(:user) }
+      set(:private_group_owner) { create(:user) }
+
+      set(:group_ancestor_owner) { create(:user) }
+
+      before(:context) do
+        public_group.add_owner public_group_owner
+        private_group.add_developer private_group_member
+        public_project.add_maintainer public_project_maintainer
+
+        private_group.add_owner private_group_owner
+        private_group.parent.add_owner group_ancestor_owner
+      end
+
+      context 'when the private group is invited to the public project' do
+        before(:context) do
+          create(:project_group_link, group: private_group, project: public_project)
+        end
+
+        context 'when a user who is outside the public project and the private group is signed in' do
+          let(:service) { described_class.new(public_project, create(:user)) }
+
+          it 'does not return the private group' do
+            expect(usernames).not_to include(private_group.name)
+          end
+
+          it 'does not return private group members' do
+            expect(usernames).not_to include(private_group_member.username)
+          end
+
+          it 'returns the project maintainer' do
+            expect(usernames).to include(public_project_maintainer.username)
+          end
+
+          it 'returns project members from an invited public group' do
+            invited_public_group = create(:group, :public)
+            invited_public_group.add_owner create(:user)
+
+            create(:project_group_link, group: invited_public_group, project: public_project)
+
+            expect(usernames).to include(invited_public_group.users.first.username)
+          end
+
+          it 'does not return ancestors of the private group' do
+            expect(usernames).not_to include(group_ancestor_owner.username)
+          end
+        end
+
+        context 'when private group owner is signed in' do
+          let(:service) { described_class.new(public_project, private_group_owner) }
+
+          it 'returns private group members' do
+            expect(usernames).to include(private_group_member.username)
+          end
+
+          it 'returns ancestors of the the private group' do
+            expect(usernames).to include(group_ancestor_owner.username)
+          end
+        end
+
+        context 'when the namespace owner of the public project is signed in' do
+          let(:service) { described_class.new(public_project, public_group_owner) }
+
+          it 'returns private group members' do
+            expect(usernames).to include(private_group_member.username)
+          end
+
+          it 'does not return members of the ancestral groups of the private group' do
+            expect(usernames).to include(group_ancestor_owner.username)
+          end
+        end
+      end
+    end
+  end
 end

spec/services/projects/transfer_service_spec.rb

@@ -222,6 +222,24 @@ describe Projects::TransferService do
     it { expect(project.errors[:new_namespace]).to include('Project with same name or path in target namespace already exists') }
   end
 
+  context 'target namespace allows developers to create projects' do
+    let(:group) { create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) }
+
+    context 'the user is a member of the target namespace with developer permissions' do
+      subject(:transfer_project_result) { transfer_project(project, user, group) }
+
+      before do
+        group.add_developer(user)
+      end
+
+      it 'does not allow project transfer to the target namespace' do
+        expect(transfer_project_result).to eq false
+        expect(project.namespace).to eq(user.namespace)
+        expect(project.errors[:new_namespace]).to include('Transfer failed, please contact an admin.')
+      end
+    end
+  end
+
   def transfer_project(project, user, new_namespace)
     service = Projects::TransferService.new(project, user)
 

spec/support/controllers/sessionless_auth_controller_shared_examples.rb

@@ -34,8 +34,15 @@ shared_examples 'authenticates sessionless user' do |path, format, params|
 
   context 'when the personal access token has no api scope', unless: params[:public] do
     it 'does not log the user in' do
-      expect(authentication_metrics)
-        .to increment(:user_unauthenticated_counter)
+      # Several instances of where these specs are shared route the request
+      #   through ApplicationController#route_not_found which does not involve
+      #   the usual auth code from Devise, so does not increment the
+      #   :user_unauthenticated_counter
+      #
+      unless params[:ignore_incrementing]
+        expect(authentication_metrics)
+          .to increment(:user_unauthenticated_counter)
+      end
 
       personal_access_token.update(scopes: [:read_user])
 
@@ -84,8 +91,15 @@ shared_examples 'authenticates sessionless user' do |path, format, params|
   end
 
   it "doesn't log the user in otherwise", unless: params[:public] do
-    expect(authentication_metrics)
-      .to increment(:user_unauthenticated_counter)
+    # Several instances of where these specs are shared route the request
+    #   through ApplicationController#route_not_found which does not involve
+    #   the usual auth code from Devise, so does not increment the
+    #   :user_unauthenticated_counter
+    #
+    unless params[:ignore_incrementing]
+      expect(authentication_metrics)
+        .to increment(:user_unauthenticated_counter)
+    end
 
     get path, params: default_params.merge(private_token: 'token')
 

spec/support/helpers/graphql_helpers.rb

@@ -129,6 +129,7 @@ module GraphqlHelpers
 
     allow_unlimited_graphql_complexity
     allow_unlimited_graphql_depth
+    allow_high_graphql_recursion
 
     type = GitlabSchema.types[class_name.to_s]
     return "" unless type
@@ -213,6 +214,23 @@ module GraphqlHelpers
     end
   end
 
+  def expect_graphql_errors_to_include(regexes_to_match)
+    raise "No errors. Was expecting to match #{regexes_to_match}" if graphql_errors.nil? || graphql_errors.empty?
+
+    error_messages = flattened_errors.collect { |error_hash| error_hash["message"] }
+    Array.wrap(regexes_to_match).flatten.each do |regex|
+      expect(error_messages).to include a_string_matching regex
+    end
+  end
+
+  def expect_graphql_errors_to_be_empty
+    expect(flattened_errors).to be_empty
+  end
+
+  def flattened_errors
+    Array.wrap(graphql_errors).flatten.compact
+  end
+
   # Raises an error if no response is found
   def graphql_mutation_response(mutation_name)
     graphql_data.fetch(GraphqlHelpers.fieldnamerize(mutation_name))
@@ -260,6 +278,10 @@ module GraphqlHelpers
     allow_any_instance_of(GitlabSchema).to receive(:max_depth).and_return nil
     allow(GitlabSchema).to receive(:max_query_depth).with(any_args).and_return nil
   end
+
+  def allow_high_graphql_recursion
+    allow_any_instance_of(Gitlab::Graphql::QueryAnalyzers::RecursionAnalyzer).to receive(:recursion_threshold).and_return 1000
+  end
 end
 
 # This warms our schema, doing this as part of loading the helpers to avoid

spec/support/shared_examples/controllers/todos_shared_examples.rb

@@ -39,7 +39,7 @@ shared_examples 'todos actions' do
         post_create
       end.to change { user.todos.count }.by(0)
 
-      expect(response).to have_gitlab_http_status(parent.is_a?(Group) ? 401 : 302)
+      expect(response).to have_gitlab_http_status(302)
     end
   end
 end