Merge remote-tracking branch 'dev/12-4-stable' into 12-4-stable

31 files changed, 175 insertions, 41 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 1443a71f6b1..27e88ae569e 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -16,7 +16,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
@@ -97,7 +97,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
 
diff --git a/app/controllers/concerns/internal_redirect.rb b/app/controllers/concerns/internal_redirect.rb
index 99bbfd56516..a35bc19aa37 100644
--- a/app/controllers/concerns/internal_redirect.rb
+++ b/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,
diff --git a/app/controllers/concerns/lfs_request.rb b/app/controllers/concerns/lfs_request.rb
index 733265f4099..417bb169f39 100644
--- a/app/controllers/concerns/lfs_request.rb
+++ b/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?
 
diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb
index e523942ea4c..027cdc4fc78 100644
--- a/app/finders/labels_finder.rb
+++ b/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
diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb
index 4c8612c8f2e..1899278ff3c 100644
--- a/app/graphql/gitlab_schema.rb
+++ b/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)
diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb
index d76a0f3a3b8..e2524938e10 100644
--- a/app/helpers/markup_helper.rb
+++ b/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
diff --git a/app/models/concerns/mentionable/reference_regexes.rb b/app/models/concerns/mentionable/reference_regexes.rb
index fec31cd262b..f44a674b3c9 100644
--- a/app/models/concerns/mentionable/reference_regexes.rb
+++ b/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
 
diff --git a/app/models/discussion.rb b/app/models/discussion.rb
index 0d066d0d99f..b8525f7b135 100644
--- a/app/models/discussion.rb
+++ b/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?,
diff --git a/app/models/member.rb b/app/models/member.rb
index e2d26773d45..2654453cf3f 100644
--- a/app/models/member.rb
+++ b/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
 
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 7cdaa3e3ca7..32741046f39 100644
--- a/app/models/merge_request.rb
+++ b/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
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 2fa0cfc9b93..a9f4cdec901 100644
--- a/app/models/milestone.rb
+++ b/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
diff --git a/app/models/note.rb b/app/models/note.rb
index 43f349c6fa2..ce60413b8a0 100644
--- a/app/models/note.rb
+++ b/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
 
diff --git a/app/models/project.rb b/app/models/project.rb
index 3525f37f8d5..74da042d5a5 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -609,11 +609,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)
 
@@ -1260,6 +1260,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
diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb
index 11cbeb60bba..5a44ee7211b 100644
--- a/app/models/system_note_metadata.rb
+++ b/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[
diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb
index 1fa29e5b933..68241d2bd95 100644
--- a/app/models/wiki_page.rb
+++ b/app/models/wiki_page.rb
@@ -134,6 +134,12 @@ class WikiPage
     @version ||= @page.version
   end
 
+  def path
+    return unless persisted?
+
+    @path ||= @page.path
+  end
+
   def versions(options = {})
     return [] unless persisted?
 
diff --git a/app/policies/commit_policy.rb b/app/policies/commit_policy.rb
index 4d4f0ba9267..4b358c45ec2 100644
--- a/app/policies/commit_policy.rb
+++ b/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
diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb
index 9e8ee3acf00..13e5b4ae41a 100644
--- a/app/policies/group_policy.rb
+++ b/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?
 
diff --git a/app/policies/namespace_policy.rb b/app/policies/namespace_policy.rb
index fd3bdddded6..350dd208499 100644
--- a/app/policies/namespace_policy.rb
+++ b/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')
diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb
index b2af6c874c7..dcde8cefa0d 100644
--- a/app/policies/note_policy.rb
+++ b/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) }
 
diff --git a/app/services/auto_merge/base_service.rb b/app/services/auto_merge/base_service.rb
index e06659a39cd..e08b4ac2260 100644
--- a/app/services/auto_merge/base_service.rb
+++ b/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
 
diff --git a/app/services/concerns/merge_requests/assigns_merge_params.rb b/app/services/concerns/merge_requests/assigns_merge_params.rb
new file mode 100644
index 00000000000..bd870d9a1e7
--- /dev/null
+++ b/app/services/concerns/merge_requests/assigns_merge_params.rb
@@ -0,0 +1,24 @@
+# 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
diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb
index 8d08f0cda94..92d4ef85ecf 100644
--- a/app/services/error_tracking/list_projects_service.rb
+++ b/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
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index 7d4227e4a41..aacc3d6831e 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/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
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index 214f145d09b..bf4da01723b 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -10,13 +10,14 @@ module MergeRequests
       # TODO: this should handle all quick actions that don't have side effects
       # https://gitlab.com/gitlab-org/gitlab-foss/issues/53658
       merge_quick_actions_into_params!(merge_request, only: [:target_branch])
-      merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch) if params.has_key?(:force_remove_source_branch)
 
       # Assign the projects first so we can use policies for `filter_params`
       merge_request.author = current_user
       merge_request.source_project = find_source_project
       merge_request.target_project = find_target_project
 
+      self.params = assign_allowed_merge_params(merge_request, params)
+
       filter_params(merge_request)
 
       # merge_request.assign_attributes(...) below is a Rails
diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb
index 1c730232abb..9a37a0330fc 100644
--- a/app/services/merge_requests/create_service.rb
+++ b/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
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index ae678d4c036..7c9abb12b6e 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/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
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index b56b2cf14e3..1709474a6c7 100644
--- a/app/services/notification_service.rb
+++ b/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|
diff --git a/app/services/projects/operations/update_service.rb b/app/services/projects/operations/update_service.rb
index 64519501ff4..0ca89664304 100644
--- a/app/services/projects/operations/update_service.rb
+++ b/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
diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb
index 7080f388e53..1cd81fe37c7 100644
--- a/app/services/projects/participants_service.rb
+++ b/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
diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb
index 4b3aca58dd7..525fc18b312 100644
--- a/app/services/projects/transfer_service.rb
+++ b/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)
diff --git a/app/views/projects/settings/operations/_error_tracking.html.haml b/app/views/projects/settings/operations/_error_tracking.html.haml
index 583fc08f375..589d3037eba 100644
--- a/app/views/projects/settings/operations/_error_tracking.html.haml
+++ b/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 } }