Add latest changes from gitlab-org/security/gitlab@13-12-stable-ee

16 files changed, 71 insertions, 54 deletions

diff --git a/app/assets/javascripts/behaviors/markdown/render_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_mermaid.js
index f5b2d266c18..d18a1a72eea 100644
--- a/app/assets/javascripts/behaviors/markdown/render_mermaid.js
+++ b/app/assets/javascripts/behaviors/markdown/render_mermaid.js
@@ -48,6 +48,7 @@ export function initMermaid(mermaid) {
       useMaxWidth: true,
       htmlLabels: false,
     },
+    secure: ['secure', 'securityLevel', 'startOnLoad', 'maxTextSize', 'htmlLabels'],
     securityLevel: 'strict',
   });
 
diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb
index 782c8c293fd..5948d9b1181 100644
--- a/app/controllers/dashboard/todos_controller.rb
+++ b/app/controllers/dashboard/todos_controller.rb
@@ -17,6 +17,8 @@ class Dashboard::TodosController < Dashboard::ApplicationController
     @todos = @todos.with_entity_associations
 
     return if redirect_out_of_range(@todos, todos_page_count(@todos))
+
+    @allowed_todos = ::Todos::AllowedTargetFilterService.new(@todos, current_user).execute
   end
 
   def destroy
diff --git a/app/graphql/resolvers/todo_resolver.rb b/app/graphql/resolvers/todo_resolver.rb
index 8966285fccc..af48ceefd6f 100644
--- a/app/graphql/resolvers/todo_resolver.rb
+++ b/app/graphql/resolvers/todo_resolver.rb
@@ -34,7 +34,7 @@ module Resolvers
       return Todo.none unless current_user.present? && target.present?
       return Todo.none if target.is_a?(User) && target != current_user
 
-      TodosFinder.new(current_user, todo_finder_params(args)).execute
+      TodosFinder.new(current_user, todo_finder_params(args)).execute.with_entity_associations
     end
 
     private
diff --git a/app/graphql/types/alert_management/alert_type.rb b/app/graphql/types/alert_management/alert_type.rb
index 5a2a5c68c8d..7cad62b9bfe 100644
--- a/app/graphql/types/alert_management/alert_type.rb
+++ b/app/graphql/types/alert_management/alert_type.rb
@@ -115,11 +115,9 @@ module Types
             null: true,
             description: 'Runbook for the alert as defined in alert details.'
 
-      field :todos,
-            Types::TodoType.connection_type,
-            null: true,
-            description: 'To-do items of the current user for the alert.',
-            resolver: Resolvers::TodoResolver
+      field :todos, description: 'To-do items of the current user for the alert.', resolver: Resolvers::TodoResolver do
+        extension(::Gitlab::Graphql::TodosProjectPermissionPreloader::FieldExtension)
+      end
 
       field :details_url,
             GraphQL::STRING_TYPE,
diff --git a/app/graphql/types/user_interface.rb b/app/graphql/types/user_interface.rb
index e5abc033155..7d61b296eae 100644
--- a/app/graphql/types/user_interface.rb
+++ b/app/graphql/types/user_interface.rb
@@ -55,9 +55,6 @@ module Types
           type: GraphQL::STRING_TYPE,
           null: false,
           description: 'Web path of the user.'
-    field :todos,
-          resolver: Resolvers::TodoResolver,
-          description: 'To-do items of the user.'
     field :group_memberships,
           type: Types::GroupMemberType.connection_type,
           null: true,
@@ -81,6 +78,10 @@ module Types
           description: 'Projects starred by the user.',
           resolver: Resolvers::UserStarredProjectsResolver
 
+    field :todos, resolver: Resolvers::TodoResolver, description: 'To-do items of the user.' do
+      extension(::Gitlab::Graphql::TodosProjectPermissionPreloader::FieldExtension)
+    end
+
     # Merge request field: MRs can be authored, assigned, or assigned-for-review:
     field :authored_merge_requests,
           resolver: Resolvers::AuthoredMergeRequestsResolver,
diff --git a/app/models/alert_management/alert.rb b/app/models/alert_management/alert.rb
index 156111ffaf3..8ab9048b856 100644
--- a/app/models/alert_management/alert.rb
+++ b/app/models/alert_management/alert.rb
@@ -262,6 +262,10 @@ module AlertManagement
       end
     end
 
+    def to_ability_name
+      'alert_management_alert'
+    end
+
     private
 
     def hook_data
diff --git a/app/models/application_record.rb b/app/models/application_record.rb
index 5e5bc00458e..4c869e9c6ea 100644
--- a/app/models/application_record.rb
+++ b/app/models/application_record.rb
@@ -84,6 +84,14 @@ class ApplicationRecord < ActiveRecord::Base
     values = enum_mod.definition.transform_values { |v| v[:value] }
     enum(enum_mod.key => values)
   end
+
+  def readable_by?(user)
+    Ability.allowed?(user, "read_#{to_ability_name}".to_sym, self)
+  end
+
+  def to_ability_name
+    model_name.element
+  end
 end
 
 ApplicationRecord.prepend_mod_with('ApplicationRecordHelpers')
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 09e43bb8f20..c7edfa6c5c6 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -530,6 +530,10 @@ class Commit
     expire_note_etag_cache_for_related_mrs
   end
 
+  def readable_by?(user)
+    Ability.allowed?(user, :read_commit, self)
+  end
+
   private
 
   def expire_note_etag_cache_for_related_mrs
diff --git a/app/models/design_management/design.rb b/app/models/design_management/design.rb
index e2d10cc7e78..79f5a63bcb6 100644
--- a/app/models/design_management/design.rb
+++ b/app/models/design_management/design.rb
@@ -182,10 +182,6 @@ module DesignManagement
       File.join(DesignManagement.designs_directory, "issue-#{issue.iid}", design.filename)
     end
 
-    def to_ability_name
-      'design'
-    end
-
     def description
       ''
     end
diff --git a/app/models/group.rb b/app/models/group.rb
index da795651c63..8114d75f682 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -720,10 +720,6 @@ class Group < Namespace
     Gitlab::ServiceDesk.supported? && all_projects.service_desk_enabled.exists?
   end
 
-  def to_ability_name
-    model_name.singular
-  end
-
   def activity_path
     Gitlab::Routing.url_helpers.activity_group_path(self)
   end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 2077f9bfdbb..5338e2e6678 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -476,23 +476,6 @@ class Issue < ApplicationRecord
     issue_assignees.pluck(:user_id)
   end
 
-  private
-
-  # Ensure that the metrics association is safely created and respecting the unique constraint on issue_id
-  override :ensure_metrics
-  def ensure_metrics
-    if !association(:metrics).loaded? || metrics.blank?
-      metrics_record = Issue::Metrics.safe_find_or_create_by(issue: self)
-      self.metrics = metrics_record
-    end
-
-    metrics.record!
-  end
-
-  def record_create_action
-    Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_created_action(author: author)
-  end
-
   # Returns `true` if the given User can read the current Issue.
   #
   # This method duplicates the same check of issue_policy.rb
@@ -512,6 +495,23 @@ class Issue < ApplicationRecord
     end
   end
 
+  private
+
+  # Ensure that the metrics association is safely created and respecting the unique constraint on issue_id
+  override :ensure_metrics
+  def ensure_metrics
+    if !association(:metrics).loaded? || metrics.blank?
+      metrics_record = Issue::Metrics.safe_find_or_create_by(issue: self)
+      self.metrics = metrics_record
+    end
+
+    metrics.record!
+  end
+
+  def record_create_action
+    Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_created_action(author: author)
+  end
+
   # Returns `true` if this Issue is visible to everybody.
   def publicly_visible?
     project.public? && !confidential? && !::Gitlab::ExternalAuthorization.enabled?
diff --git a/app/models/note.rb b/app/models/note.rb
index ae4a8859d4d..96977f3c6c3 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -378,12 +378,6 @@ class Note < ApplicationRecord
     super
   end
 
-  # This method is to be used for checking read permissions on a note instead of `system_note_with_references_visible_for?`
-  def readable_by?(user)
-    # note_policy accounts for #system_note_with_references_visible_for?(user) check when granting read access
-    Ability.allowed?(user, :read_note, self)
-  end
-
   def award_emoji?
     can_be_award_emoji? && contains_emoji_only?
   end
@@ -400,10 +394,6 @@ class Note < ApplicationRecord
     note =~ /\A#{Banzai::Filter::EmojiFilter.emoji_pattern}\s?\Z/
   end
 
-  def to_ability_name
-    model_name.singular
-  end
-
   def noteable_ability_name
     if for_snippet?
       'snippet'
diff --git a/app/models/project.rb b/app/models/project.rb
index 9d572b7e2f8..fb6a6dbf246 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1496,10 +1496,6 @@ 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/policies/todo_policy.rb b/app/policies/todo_policy.rb
index d01a046c343..6237fbc50fa 100644
--- a/app/policies/todo_policy.rb
+++ b/app/policies/todo_policy.rb
@@ -5,7 +5,10 @@ class TodoPolicy < BasePolicy
   condition(:own_todo) do
     @user && @subject.user_id == @user.id
   end
+  condition(:can_read_target) do
+    @user && @subject.target&.readable_by?(@user)
+  end
 
-  rule { own_todo }.enable :read_todo
-  rule { own_todo }.enable :update_todo
+  rule { own_todo & can_read_target }.enable :read_todo
+  rule { own_todo & can_read_target }.enable :update_todo
 end
diff --git a/app/services/todos/allowed_target_filter_service.rb b/app/services/todos/allowed_target_filter_service.rb
new file mode 100644
index 00000000000..dfed616710b
--- /dev/null
+++ b/app/services/todos/allowed_target_filter_service.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module Todos
+  class AllowedTargetFilterService
+    include Gitlab::Allowable
+
+    def initialize(todos, current_user)
+      @todos = todos
+      @current_user = current_user
+    end
+
+    def execute
+      Preloaders::UserMaxAccessLevelInProjectsPreloader.new(@todos.map(&:project).compact, @current_user).execute
+
+      @todos.select { |todo| can?(@current_user, :read_todo, todo) }
+    end
+  end
+end
diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml
index 52e41946ed1..2a32fc96ca6 100644
--- a/app/views/dashboard/todos/index.html.haml
+++ b/app/views/dashboard/todos/index.html.haml
@@ -25,7 +25,7 @@
             = number_with_delimiter(todos_done_count)
 
     .nav-controls
-      - if @todos.any?(&:pending?)
+      - if @allowed_todos.any?(&:pending?)
         .gl-mr-3
           = link_to destroy_all_dashboard_todos_path(todos_filter_params), class: 'gl-button btn btn-default btn-loading align-items-center js-todos-mark-all', method: :delete, data: { href: destroy_all_dashboard_todos_path(todos_filter_params) } do
             Mark all as done
@@ -82,11 +82,11 @@
                   = sort_title_oldest_created
 
 .row.js-todos-all
-  - if @todos.any?
+  - if @allowed_todos.any?
     .col.js-todos-list-container{ data: { qa_selector: "todos_list_container" } }
-      .js-todos-options{ data: { per_page: @todos.limit_value, current_page: @todos.current_page, total_pages: @todos.total_pages } }
+      .js-todos-options{ data: { per_page: @allowed_todos.count, current_page: @todos.current_page, total_pages: @todos.total_pages } }
         %ul.content-list.todos-list
-          = render @todos
+          = render @allowed_todos
       = paginate @todos, theme: "gitlab"
     .js-nothing-here-container.empty-state.hidden
       .svg-content