Merge dev master into GitLab.com master

29 files changed, 148 insertions, 57 deletions

diff --git a/app/assets/javascripts/behaviors/markdown/render_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_mermaid.js
index 35380ca49fb..798114b4b0b 100644
--- a/app/assets/javascripts/behaviors/markdown/render_mermaid.js
+++ b/app/assets/javascripts/behaviors/markdown/render_mermaid.js
@@ -1,4 +1,5 @@
 import flash from '~/flash';
+import { sprintf, __ } from '../../locale';
 
 // Renders diagrams and flowcharts from text using Mermaid in any element with the
 // `js-render-mermaid` class.
@@ -14,6 +15,9 @@ import flash from '~/flash';
 // </pre>
 //
 
+// This is an arbitary number; Can be iterated upon when suitable.
+const MAX_CHAR_LIMIT = 5000;
+
 export default function renderMermaid($els) {
   if (!$els.length) return;
 
@@ -34,6 +38,21 @@ export default function renderMermaid($els) {
       $els.each((i, el) => {
         const source = el.textContent;
 
+        /**
+         * Restrict the rendering to a certain amount of character to
+         * prevent mermaidjs from hanging up the entire thread and
+         * causing a DoS.
+         */
+        if (source && source.length > MAX_CHAR_LIMIT) {
+          el.textContent = sprintf(
+            __(
+              'Cannot render the image. Maximum character count (%{charLimit}) has been exceeded.',
+            ),
+            { charLimit: MAX_CHAR_LIMIT },
+          );
+          return;
+        }
+
         // Remove any extra spans added by the backend syntax highlighting.
         Object.assign(el, { textContent: source });
 
diff --git a/app/controllers/concerns/milestone_actions.rb b/app/controllers/concerns/milestone_actions.rb
index eccbe35577b..c0c0160a827 100644
--- a/app/controllers/concerns/milestone_actions.rb
+++ b/app/controllers/concerns/milestone_actions.rb
@@ -8,7 +8,7 @@ module MilestoneActions
       format.html { redirect_to milestone_redirect_path }
       format.json do
         render json: tabs_json("shared/milestones/_merge_requests_tab", {
-          merge_requests: @milestone.sorted_merge_requests, # rubocop:disable Gitlab/ModuleWithInstanceVariables
+          merge_requests: @milestone.sorted_merge_requests(current_user), # rubocop:disable Gitlab/ModuleWithInstanceVariables
           show_project_name: true
         })
       end
diff --git a/app/controllers/google_api/authorizations_controller.rb b/app/controllers/google_api/authorizations_controller.rb
index dd9f5af61b3..ed0995e7ffd 100644
--- a/app/controllers/google_api/authorizations_controller.rb
+++ b/app/controllers/google_api/authorizations_controller.rb
@@ -2,6 +2,10 @@
 
 module GoogleApi
   class AuthorizationsController < ApplicationController
+    include Gitlab::Utils::StrongMemoize
+
+    before_action :validate_session_key!
+
     def callback
       token, expires_at = GoogleApi::CloudPlatform::Client
         .new(nil, callback_google_api_auth_url)
@@ -11,21 +15,27 @@ module GoogleApi
       session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] =
         expires_at.to_s
 
-      state_redirect_uri = redirect_uri_from_session_key(params[:state])
-
-      if state_redirect_uri
-        redirect_to state_redirect_uri
-      else
-        redirect_to root_path
-      end
+      redirect_to redirect_uri_from_session
     end
 
     private
 
-    def redirect_uri_from_session_key(state)
-      key = GoogleApi::CloudPlatform::Client
-        .session_key_for_redirect_uri(params[:state])
-      session[key] if key
+    def validate_session_key!
+      access_denied! unless redirect_uri_from_session.present?
+    end
+
+    def redirect_uri_from_session
+      strong_memoize(:redirect_uri_from_session) do
+        if params[:state].present?
+          session[session_key_for_redirect_uri(params[:state])]
+        else
+          nil
+        end
+      end
+    end
+
+    def session_key_for_redirect_uri(state)
+      GoogleApi::CloudPlatform::Client.session_key_for_redirect_uri(state)
     end
   end
 end
diff --git a/app/controllers/profiles/active_sessions_controller.rb b/app/controllers/profiles/active_sessions_controller.rb
index efe7ede5efa..c473023cacb 100644
--- a/app/controllers/profiles/active_sessions_controller.rb
+++ b/app/controllers/profiles/active_sessions_controller.rb
@@ -2,15 +2,6 @@
 
 class Profiles::ActiveSessionsController < Profiles::ApplicationController
   def index
-    @sessions = ActiveSession.list(current_user)
-  end
-
-  def destroy
-    ActiveSession.destroy(current_user, params[:id])
-
-    respond_to do |format|
-      format.html { redirect_to profile_active_sessions_url, status: :found }
-      format.js { head :ok }
-    end
+    @sessions = ActiveSession.list(current_user).reject(&:is_impersonated)
   end
 end
diff --git a/app/controllers/projects/autocomplete_sources_controller.rb b/app/controllers/projects/autocomplete_sources_controller.rb
index 9c130af8394..0e3f13045ce 100644
--- a/app/controllers/projects/autocomplete_sources_controller.rb
+++ b/app/controllers/projects/autocomplete_sources_controller.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class Projects::AutocompleteSourcesController < Projects::ApplicationController
+  before_action :authorize_read_milestone!, only: :milestones
+
   def members
     render json: ::Projects::ParticipantsService.new(@project, current_user).execute(target)
   end
diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index b13c0ae3967..939a09d4fd2 100644
--- a/app/controllers/projects/commit_controller.rb
+++ b/app/controllers/projects/commit_controller.rb
@@ -65,7 +65,11 @@ class Projects::CommitController < Projects::ApplicationController
   # rubocop: enable CodeReuse/ActiveRecord
 
   def merge_requests
-    @merge_requests = @commit.merge_requests.map do |mr|
+    @merge_requests = MergeRequestsFinder.new(
+      current_user,
+      project_id: @project.id,
+      commit_sha: @commit.sha
+    ).execute.map do |mr|
       { iid: mr.iid, path: merge_request_path(mr), title: mr.title }
     end
 
diff --git a/app/controllers/projects/group_links_controller.rb b/app/controllers/projects/group_links_controller.rb
index 7c713c19762..bc942ba9288 100644
--- a/app/controllers/projects/group_links_controller.rb
+++ b/app/controllers/projects/group_links_controller.rb
@@ -13,9 +13,10 @@ class Projects::GroupLinksController < Projects::ApplicationController
     group = Group.find(params[:link_group_id]) if params[:link_group_id].present?
 
     if group
-      return render_404 unless can?(current_user, :read_group, group)
+      result = Projects::GroupLinks::CreateService.new(project, current_user, group_link_create_params).execute(group)
+      return render_404 if result[:http_status] == 404
 
-      Projects::GroupLinks::CreateService.new(project, current_user, group_link_create_params).execute(group)
+      flash[:alert] = result[:message] if result[:http_status] == 409
     else
       flash[:alert] = 'Please select a group.'
     end
diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb
index b645011a3c5..93bee3f1488 100644
--- a/app/finders/merge_requests_finder.rb
+++ b/app/finders/merge_requests_finder.rb
@@ -37,13 +37,20 @@ class MergeRequestsFinder < IssuableFinder
   end
 
   def filter_items(_items)
-    items = by_source_branch(super)
+    items = by_commit(super)
+    items = by_source_branch(items)
     items = by_wip(items)
     by_target_branch(items)
   end
 
   private
 
+  def by_commit(items)
+    return items unless params[:commit_sha].presence
+
+    items.by_commit_sha(params[:commit_sha])
+  end
+
   def source_branch
     @source_branch ||= params[:source_branch].presence
   end
diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb
index 3ef0cc5020c..b96c2f3afb2 100644
--- a/app/graphql/types/project_type.rb
+++ b/app/graphql/types/project_type.rb
@@ -16,7 +16,6 @@ module Types
 
     field :description, GraphQL::STRING_TYPE, null: true
 
-    field :default_branch, GraphQL::STRING_TYPE, null: true
     field :tag_list, GraphQL::STRING_TYPE, null: true
 
     field :ssh_url_to_repo, GraphQL::STRING_TYPE, null: true
@@ -59,7 +58,6 @@ module Types
     end
 
     field :import_status, GraphQL::STRING_TYPE, null: true
-    field :ci_config_path, GraphQL::STRING_TYPE, null: true
 
     field :only_allow_merge_if_pipeline_succeeds, GraphQL::BOOLEAN_TYPE, null: true
     field :request_access_enabled, GraphQL::BOOLEAN_TYPE, null: true
diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb
index 654ae211310..d2e334fb856 100644
--- a/app/mailers/emails/issues.rb
+++ b/app/mailers/emails/issues.rb
@@ -74,6 +74,7 @@ module Emails
 
       @new_issue = new_issue
       @new_project = new_issue.project
+      @can_access_project = recipient.can?(:read_project, @new_project)
       mail_answer_thread(issue, issue_thread_options(updated_by_user.id, recipient.id, reason))
     end
 
diff --git a/app/models/active_session.rb b/app/models/active_session.rb
index 0d9c6a4a1f0..1e01f1d17e6 100644
--- a/app/models/active_session.rb
+++ b/app/models/active_session.rb
@@ -5,7 +5,8 @@ class ActiveSession
 
   attr_accessor :created_at, :updated_at,
     :session_id, :ip_address,
-    :browser, :os, :device_name, :device_type
+    :browser, :os, :device_name, :device_type,
+    :is_impersonated
 
   def current?(session)
     return false if session_id.nil? || session.id.nil?
@@ -31,7 +32,8 @@ class ActiveSession
         device_type: client.device_type,
         created_at: user.current_sign_in_at || timestamp,
         updated_at: timestamp,
-        session_id: session_id
+        session_id: session_id,
+        is_impersonated: request.session[:impersonator_id].present?
       )
 
       redis.pipelined do
diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb
index 46d0898014e..814fc591408 100644
--- a/app/models/clusters/platforms/kubernetes.rb
+++ b/app/models/clusters/platforms/kubernetes.rb
@@ -41,7 +41,7 @@ module Clusters
       validate :no_namespace, unless: :allow_user_defined_namespace?
 
       # We expect to be `active?` only when enabled and cluster is created (the api_url is assigned)
-      validates :api_url, url: true, presence: true
+      validates :api_url, public_url: true, presence: true
       validates :token, presence: true
       validates :ca_cert, certificate: true, allow_blank: true, if: :ca_cert_changed?
 
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 670103bc3f3..c7ad182ab82 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -75,6 +75,7 @@ module Issuable
 
     validates :author, presence: true
     validates :title, presence: true, length: { maximum: 255 }
+    validate :milestone_is_valid
 
     scope :authored, ->(user) { where(author_id: user) }
     scope :recent, -> { reorder(id: :desc) }
@@ -118,6 +119,16 @@ module Issuable
     def has_multiple_assignees?
       assignees.count > 1
     end
+
+    def milestone_available?
+      project_id == milestone&.project_id || project.ancestors_upto.compact.include?(milestone&.group)
+    end
+
+    private
+
+    def milestone_is_valid
+      errors.add(:milestone_id, message: "is invalid") if milestone_id.present? && !milestone_available?
+    end
   end
 
   class_methods do
diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index 055ffe04646..39372c4f68b 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/app/models/concerns/milestoneish.rb
@@ -46,12 +46,19 @@ module Milestoneish
     end
   end
 
+  def merge_requests_visible_to_user(user)
+    memoize_per_user(user, :merge_requests_visible_to_user) do
+      MergeRequestsFinder.new(user, {})
+        .execute.where(milestone_id: milestoneish_id)
+    end
+  end
+
   def sorted_issues(user)
     issues_visible_to_user(user).preload_associations.sort_by_attribute('label_priority')
   end
 
-  def sorted_merge_requests
-    merge_requests.sort_by_attribute('label_priority')
+  def sorted_merge_requests(user)
+    merge_requests_visible_to_user(user).sort_by_attribute('label_priority')
   end
 
   def upcoming?
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 1468ae1c34a..87954e3a5ff 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -71,7 +71,7 @@ class MergeRequest < ActiveRecord::Base
 
   serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize
 
-  after_create :ensure_merge_request_diff, unless: :importing?
+  after_create :ensure_merge_request_diff
   after_update :clear_memoized_shas
   after_update :reload_diff_if_branch_changed
   after_save :ensure_metrics
@@ -189,6 +189,9 @@ class MergeRequest < ActiveRecord::Base
 
   after_save :keep_around_commit
 
+  alias_attribute :project, :target_project
+  alias_attribute :project_id, :target_project_id
+
   def self.reference_prefix
     '!'
   end
@@ -847,10 +850,6 @@ class MergeRequest < ActiveRecord::Base
     target_project != source_project
   end
 
-  def project
-    target_project
-  end
-
   # If the merge request closes any issues, save this information in the
   # `MergeRequestsClosingIssues` model. This is a performance optimization.
   # Calculating this information for a number of merge requests requires
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index e286a4e57f2..351a662ae83 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -22,6 +22,8 @@ class MergeRequestDiff < ActiveRecord::Base
 
   has_many :merge_request_diff_commits, -> { order(:merge_request_diff_id, :relative_order) }
 
+  validates :base_commit_sha, :head_commit_sha, :start_commit_sha, sha: true
+
   state_machine :state, initial: :empty do
     event :clean do
       transition any => :without_files
diff --git a/app/models/project_services/prometheus_service.rb b/app/models/project_services/prometheus_service.rb
index 60cb2d380d5..c68a9d923c8 100644
--- a/app/models/project_services/prometheus_service.rb
+++ b/app/models/project_services/prometheus_service.rb
@@ -71,7 +71,7 @@ class PrometheusService < MonitoringService
   end
 
   def prometheus_client
-    RestClient::Resource.new(api_url, max_redirects: 0) if api_url && manual_configuration? && active?
+    RestClient::Resource.new(api_url, max_redirects: 0) if should_return_client?
   end
 
   def prometheus_available?
@@ -83,6 +83,10 @@ class PrometheusService < MonitoringService
 
   private
 
+  def should_return_client?
+    api_url && manual_configuration? && active? && valid?
+  end
+
   def synchronize_service_state
     self.active = prometheus_available? || manual_configuration?
 
diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb
index 298769c0eb8..2b0f1d25534 100644
--- a/app/policies/group_policy.rb
+++ b/app/policies/group_policy.rb
@@ -53,8 +53,11 @@ class GroupPolicy < BasePolicy
   rule { admin }.enable :read_group
 
   rule { has_projects }.policy do
+<<<<<<< HEAD
     enable :read_group
     enable :read_list
+=======
+>>>>>>> dev/master
     enable :read_label
   end
 
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 533782104ac..031b72ad9c3 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -300,6 +300,8 @@ class ProjectPolicy < BasePolicy
 
   rule { issues_disabled }.policy do
     prevent(*create_read_update_admin_destroy(:issue))
+    prevent(*create_read_update_admin_destroy(:board))
+    prevent(*create_read_update_admin_destroy(:list))
   end
 
   rule { merge_requests_disabled | repository_disabled }.policy do
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index ef991eaf234..1e1f2fbd08e 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -387,4 +387,10 @@ class IssuableBaseService < BaseService
   def parent
     project
   end
+
+  # we need to check this because milestone from milestone_id param is displayed on "new" page
+  # where private project milestone could leak without this check
+  def ensure_milestone_available(issuable)
+    issuable.milestone_id = nil unless issuable.milestone_available?
+  end
 end
diff --git a/app/services/issues/build_service.rb b/app/services/issues/build_service.rb
index 3fb2c2b3007..61615ac2058 100644
--- a/app/services/issues/build_service.rb
+++ b/app/services/issues/build_service.rb
@@ -6,7 +6,9 @@ module Issues
 
     def execute
       filter_resolve_discussion_params
-      @issue = project.issues.new(issue_params)
+      @issue = project.issues.new(issue_params).tap do |issue|
+        ensure_milestone_available(issue)
+      end
     end
 
     def issue_params_with_info_from_discussions
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index 48419da98ad..109c964e577 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -19,6 +19,7 @@ module MergeRequests
       merge_request.target_project  = find_target_project
       merge_request.target_branch   = find_target_branch
       merge_request.can_be_created  = projects_and_branches_valid?
+      ensure_milestone_available(merge_request)
 
       # compare branches only if branches are valid, otherwise
       # compare_branches may raise an error
diff --git a/app/services/projects/group_links/create_service.rb b/app/services/projects/group_links/create_service.rb
index 1392775f805..e3d5bea0852 100644
--- a/app/services/projects/group_links/create_service.rb
+++ b/app/services/projects/group_links/create_service.rb
@@ -4,13 +4,19 @@ module Projects
   module GroupLinks
     class CreateService < BaseService
       def execute(group)
-        return false unless group
+        return error('Not Found', 404) unless group && can?(current_user, :read_namespace, group)
 
-        project.project_group_links.create(
+        link = project.project_group_links.new(
           group: group,
           group_access: params[:link_group_access],
           expires_at: params[:expires_at]
         )
+
+        if link.save
+          success(link: link)
+        else
+          error(link.errors.full_messages.to_sentence, 409)
+        end
       end
     end
   end
diff --git a/app/uploaders/file_mover.rb b/app/uploaders/file_mover.rb
index a7f8615e9ba..236b7ed2b3d 100644
--- a/app/uploaders/file_mover.rb
+++ b/app/uploaders/file_mover.rb
@@ -11,6 +11,8 @@ class FileMover
   end
 
   def execute
+    return unless valid?
+
     move
 
     if update_markdown
@@ -21,6 +23,12 @@ class FileMover
 
   private
 
+  def valid?
+    Pathname.new(temp_file_path).realpath.to_path.start_with?(
+      (Pathname(temp_file_uploader.root) + temp_file_uploader.base_dir).to_path
+    )
+  end
+
   def move
     FileUtils.mkdir_p(File.dirname(file_path))
     FileUtils.move(temp_file_path, file_path)
diff --git a/app/validators/sha_validator.rb b/app/validators/sha_validator.rb
new file mode 100644
index 00000000000..085fca4d65d
--- /dev/null
+++ b/app/validators/sha_validator.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class ShaValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    return if value.blank? || value.match(/\A\h{40}\z/)
+
+    record.errors.add(attribute, 'is not a valid SHA')
+  end
+end
diff --git a/app/views/notify/issue_moved_email.html.haml b/app/views/notify/issue_moved_email.html.haml
index 472c31e9a5e..b766cb1a523 100644
--- a/app/views/notify/issue_moved_email.html.haml
+++ b/app/views/notify/issue_moved_email.html.haml
@@ -1,6 +1,9 @@
 %p
   Issue was moved to another project.
-%p
-  New issue:
-  = link_to project_issue_url(@new_project, @new_issue) do
-    = @new_issue.title
+- if @can_access_project
+  %p
+    New issue:
+    = link_to project_issue_url(@new_project, @new_issue) do
+      = @new_issue.title
+- else
+  You don't have access to the project.
diff --git a/app/views/notify/issue_moved_email.text.erb b/app/views/notify/issue_moved_email.text.erb
index 66ede43635b..985e689aa9d 100644
--- a/app/views/notify/issue_moved_email.text.erb
+++ b/app/views/notify/issue_moved_email.text.erb
@@ -1,4 +1,8 @@
 Issue was moved to another project.
 
+<% if @can_access_project %>
 New issue location:
 <%= project_issue_url(@new_project, @new_issue) %>
+<% else %>
+You don't have access to the project.
+<% end %>
diff --git a/app/views/profiles/active_sessions/_active_session.html.haml b/app/views/profiles/active_sessions/_active_session.html.haml
index 23ef31a0c85..2bf514d72a5 100644
--- a/app/views/profiles/active_sessions/_active_session.html.haml
+++ b/app/views/profiles/active_sessions/_active_session.html.haml
@@ -23,9 +23,3 @@
       %strong Signed in
       on
       = l(active_session.created_at, format: :short)
-
-  - unless is_current_session
-    .float-right
-      = link_to profile_active_session_path(active_session.session_id), data: { confirm: 'Are you sure? The device will be signed out of GitLab.' }, method: :delete, class: "btn btn-danger prepend-left-10" do
-        %span.sr-only Revoke
-        Revoke
diff --git a/app/views/projects/blob/viewers/_dependency_manager.html.haml b/app/views/projects/blob/viewers/_dependency_manager.html.haml
index 87aa7c1dbf8..5970d41fdab 100644
--- a/app/views/projects/blob/viewers/_dependency_manager.html.haml
+++ b/app/views/projects/blob/viewers/_dependency_manager.html.haml
@@ -3,9 +3,4 @@
   This project manages its dependencies using
   %strong= viewer.manager_name
 
-  - if viewer.package_name
-    and defines a #{viewer.package_type} named
-    %strong<
-      = link_to_if viewer.package_url.present?, viewer.package_name, viewer.package_url, target: '_blank', rel: 'noopener noreferrer'
-
 = link_to 'Learn more', viewer.manager_url, target: '_blank', rel: 'noopener noreferrer'