Merge branch 'master' into fix/rename-mwbs-to-merge-when-pipeline-succeeds

* master: (110 commits)
  Rewrite an HTTP link to use HTTPS
  Edit /spec/features/profiles/preferences_spec.rb to match changes in 084d90ac
  Add blue back to sub nav active
  Remove JSX/React eslint plugins.
  Fix a transient spec failure
  Adds hoverstates for collapsed Issue/Merge Request sidebar
  Moved groups above projects
  Add StackProf to the Gemfile, along with a utility to get a profile for a spec
  Update Sidekiq-cron to fix compatibility issues with Sidekiq 4.2.1
  Add a CHANGELOG entry
  Alert user when logged in user email is not the same as the invitation
  Expose timestamp in build entity used by serializer
  Rename `MergeRequest#pipeline` to `head_pipeline`
  Remove unnecessary database indexes
  CE-specific changes gitlab-org/gitlab-ee#1137
  Fixing typo & Clarifying Key name
  fix started_at check
  fix blob controller spec failure - updated not to use file-path-
  fix blob controller spec failure
  Merge branch 'jej-use-issuable-finder-instead-of-access-check' into 'security'
  ...

Conflicts:
	app/controllers/projects/merge_requests_controller.rb
	lib/api/merge_requests.rb
	spec/requests/api/merge_requests_spec.rb

10 files changed, 49 insertions, 58 deletions

diff --git a/app/controllers/projects/avatars_controller.rb b/app/controllers/projects/avatars_controller.rb
index ada7db3c552..53788687076 100644
--- a/app/controllers/projects/avatars_controller.rb
+++ b/app/controllers/projects/avatars_controller.rb
@@ -20,7 +20,6 @@ class Projects::AvatarsController < Projects::ApplicationController
     @project.remove_avatar!
 
     @project.save
-    @project.reset_events_cache
 
     redirect_to edit_project_path(@project)
   end
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index 56ced786311..9940263ae24 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -13,7 +13,6 @@ class Projects::BlobController < Projects::ApplicationController
   before_action :assign_blob_vars
   before_action :commit, except: [:new, :create]
   before_action :blob, except: [:new, :create]
-  before_action :from_merge_request, only: [:edit, :update]
   before_action :require_branch_head, only: [:edit, :update]
   before_action :editor_variables, except: [:show, :preview, :diff]
   before_action :validate_diff_params, only: :diff
@@ -39,14 +38,6 @@ class Projects::BlobController < Projects::ApplicationController
 
   def update
     @path = params[:file_path] if params[:file_path].present?
-    after_edit_path =
-      if from_merge_request && @target_branch == @ref
-        diffs_namespace_project_merge_request_path(from_merge_request.target_project.namespace, from_merge_request.target_project, from_merge_request) +
-          "##{hexdigest(@path)}"
-      else
-        namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @path))
-      end
-
     create_commit(Files::UpdateService, success_path: after_edit_path,
                                         failure_view: :edit,
                                         failure_path: namespace_project_blob_path(@project.namespace, @project, @id))
@@ -124,9 +115,14 @@ class Projects::BlobController < Projects::ApplicationController
     render_404
   end
 
-  def from_merge_request
-    # If blob edit was initiated from merge request page
-    @from_merge_request ||= MergeRequest.find_by(id: params[:from_merge_request_id])
+  def after_edit_path
+    from_merge_request = MergeRequestsFinder.new(current_user, project_id: @project.id).execute.find_by(iid: params[:from_merge_request_iid])
+    if from_merge_request && @target_branch == @ref
+      diffs_namespace_project_merge_request_path(from_merge_request.target_project.namespace, from_merge_request.target_project, from_merge_request) +
+        "##{hexdigest(@path)}"
+    else
+      namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @path))
+    end
   end
 
   def editor_variables
diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb
index 6b9f37983c4..89d84809e3a 100644
--- a/app/controllers/projects/branches_controller.rb
+++ b/app/controllers/projects/branches_controller.rb
@@ -36,7 +36,7 @@ class Projects::BranchesController < Projects::ApplicationController
         execute(branch_name, ref)
 
     if params[:issue_iid]
-      issue = @project.issues.find_by(iid: params[:issue_iid])
+      issue = IssuesFinder.new(current_user, project_id: @project.id).find_by(iid: params[:issue_iid])
       SystemNoteService.new_issue_branch(issue, @project, current_user, branch_name) if issue
     end
 
diff --git a/app/controllers/projects/cycle_analytics_controller.rb b/app/controllers/projects/cycle_analytics_controller.rb
index fd263960b93..ac639ef015b 100644
--- a/app/controllers/projects/cycle_analytics_controller.rb
+++ b/app/controllers/projects/cycle_analytics_controller.rb
@@ -6,7 +6,7 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController
   before_action :authorize_read_cycle_analytics!
 
   def show
-    @cycle_analytics = ::CycleAnalytics.new(@project, from: start_date(cycle_analytics_params))
+    @cycle_analytics = ::CycleAnalytics.new(@project, current_user, from: start_date(cycle_analytics_params))
 
     stats_values, cycle_analytics_json = generate_cycle_analytics_data
 
diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index 3f41916e6d3..8714349e27f 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -18,6 +18,14 @@ class Projects::GitHttpClientController < Projects::ApplicationController
 
   private
 
+  def download_request?
+    raise NotImplementedError
+  end
+
+  def upload_request?
+    raise NotImplementedError
+  end
+
   def authenticate_user
     @authentication_result = Gitlab::Auth::Result.new
 
@@ -130,10 +138,6 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     authentication_result.ci?(project)
   end
 
-  def lfs_deploy_token?
-    authentication_result.lfs_deploy_token?(project)
-  end
-
   def authentication_has_download_access?
     has_authentication_ability?(:download_code) || has_authentication_ability?(:build_download_code)
   end
@@ -149,8 +153,4 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   def authentication_project
     authentication_result.project
   end
-
-  def verify_workhorse_api!
-    Gitlab::Workhorse.verify_api_request!(request.headers)
-  end
 end
diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb
index 13caeb42d40..9184dcccac5 100644
--- a/app/controllers/projects/git_http_controller.rb
+++ b/app/controllers/projects/git_http_controller.rb
@@ -1,7 +1,5 @@
-# This file should be identical in GitLab Community Edition and Enterprise Edition
-
 class Projects::GitHttpController < Projects::GitHttpClientController
-  before_action :verify_workhorse_api!
+  include WorkhorseRequest
 
   # GET /foo/bar.git/info/refs?service=git-upload-pack (git pull)
   # GET /foo/bar.git/info/refs?service=git-receive-pack (git push)
@@ -67,14 +65,18 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   end
 
   def render_denied
-    if user && user.can?(:read_project, project)
-      render plain: 'Access denied', status: :forbidden
+    if user && can?(user, :read_project, project)
+      render plain: access_denied_message, status: :forbidden
     else
       # Do not leak information about project existence
       render_not_found
     end
   end
 
+  def access_denied_message
+    'Access denied'
+  end
+
   def upload_pack_allowed?
     return false unless Gitlab.config.gitlab_shell.upload_pack
 
diff --git a/app/controllers/projects/lfs_api_controller.rb b/app/controllers/projects/lfs_api_controller.rb
index 2d493276941..440259b643c 100644
--- a/app/controllers/projects/lfs_api_controller.rb
+++ b/app/controllers/projects/lfs_api_controller.rb
@@ -1,8 +1,7 @@
 class Projects::LfsApiController < Projects::GitHttpClientController
-  include LfsHelper
+  include LfsRequest
 
-  before_action :require_lfs_enabled!
-  before_action :lfs_check_access!, except: [:deprecated]
+  skip_before_action :lfs_check_access!, only: [:deprecated]
 
   def batch
     unless objects.present?
@@ -31,6 +30,14 @@ class Projects::LfsApiController < Projects::GitHttpClientController
 
   private
 
+  def download_request?
+    params[:operation] == 'download'
+  end
+
+  def upload_request?
+    params[:operation] == 'upload'
+  end
+
   def existing_oids
     @existing_oids ||= begin
       storage_project.lfs_objects.where(oid: objects.map { |o| o['oid'].to_s }).pluck(:oid)
@@ -79,12 +86,4 @@ class Projects::LfsApiController < Projects::GitHttpClientController
       }
     }
   end
-
-  def download_request?
-    params[:operation] == 'download'
-  end
-
-  def upload_request?
-    params[:operation] == 'upload'
-  end
 end
diff --git a/app/controllers/projects/lfs_storage_controller.rb b/app/controllers/projects/lfs_storage_controller.rb
index 9005b104e90..32759672b6c 100644
--- a/app/controllers/projects/lfs_storage_controller.rb
+++ b/app/controllers/projects/lfs_storage_controller.rb
@@ -1,9 +1,8 @@
 class Projects::LfsStorageController < Projects::GitHttpClientController
-  include LfsHelper
+  include LfsRequest
+  include WorkhorseRequest
 
-  before_action :require_lfs_enabled!
-  before_action :lfs_check_access!
-  before_action :verify_workhorse_api!, only: [:upload_authorize]
+  skip_before_action :verify_workhorse_api!, only: [:download, :upload_finalize]
 
   def download
     lfs_object = LfsObject.find_by_oid(oid)
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 707cc018a12..c7cde9e6a14 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -329,17 +329,18 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @merge_request.update(merge_error: nil)
 
     if params[:merge_when_build_succeeds].present?
-      unless @merge_request.pipeline
+      unless @merge_request.head_pipeline
         @status = :failed
         return
       end
 
-      if @merge_request.pipeline.active?
+      if @merge_request.head_pipeline.active?
         MergeRequests::MergeWhenPipelineSucceedsService
           .new(@project, current_user, merge_params)
           .execute(@merge_request)
+
         @status = :merge_when_build_succeeds
-      elsif @merge_request.pipeline.success?
+      elsif @merge_request.head_pipeline.success?
         # This can be triggered when a user clicks the auto merge button while
         # the tests finish at about the same time
         MergeWorker.perform_async(@merge_request.id, current_user.id, params)
@@ -403,7 +404,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def ci_status
-    pipeline = @merge_request.pipeline
+    pipeline = @merge_request.head_pipeline
+
     if pipeline
       status = pipeline.status
       coverage = pipeline.try(:coverage)
@@ -539,7 +541,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def define_widget_vars
-    @pipeline = @merge_request.pipeline
+    @pipeline = @merge_request.head_pipeline
   end
 
   def define_commit_vars
@@ -568,8 +570,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
   def define_pipelines_vars
     @pipelines = @merge_request.all_pipelines
-    @pipeline = @merge_request.pipeline
-    @statuses = @pipeline.statuses.relevant if @pipeline.present?
+    @pipeline = @merge_request.head_pipeline
+    @statuses_count = @pipeline.present? ? @pipeline.statuses.relevant.count : 0
   end
 
   def define_new_vars
@@ -636,7 +638,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
   def merge_when_build_succeeds_active?
     params[:merge_when_build_succeeds].present? &&
-      @merge_request.pipeline && @merge_request.pipeline.active?
+      @merge_request.head_pipeline && @merge_request.head_pipeline.active?
   end
 
   def build_merge_request
diff --git a/app/controllers/projects/todos_controller.rb b/app/controllers/projects/todos_controller.rb
index 5685d0f4e7c..52517381c65 100644
--- a/app/controllers/projects/todos_controller.rb
+++ b/app/controllers/projects/todos_controller.rb
@@ -16,13 +16,7 @@ class Projects::TodosController < Projects::ApplicationController
     @issuable ||= begin
       case params[:issuable_type]
       when "issue"
-        issue = @project.issues.find(params[:issuable_id])
-
-        if can?(current_user, :read_issue, issue)
-          issue
-        else
-          render_404
-        end
+        IssuesFinder.new(current_user, project_id: @project.id).find(params[:issuable_id])
       when "merge_request"
         @project.merge_requests.find(params[:issuable_id])
       end