New upstream version 8.13.11+dfsg

CHANGELOG.md

@@ -1,8 +1,41 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
+## 8.13.11 (2017-01-10)
+
+- Update the gitlab-markup gem to the version 1.5.1. !8509
+- Updated Turbolinks to mitigate potential XSS attacks.
+
+## 8.13.10 (2016-12-14)
+
+- API: Memoize the current_user so that sudo can work properly. !8017
+- Filter `authentication_token`, `incoming_email_token` and `runners_token` parameters.
+- Issue#visible_to_user moved to IssuesFinder to prevent accidental use.
+- Fix missing Note access checks by moving Note#search to updated NoteFinder.
+
+## 8.13.9 (2016-12-08)
+
+- Reenables /user API request to return private-token if user is admin and request is made with sudo. !7615
+- Replace MR access checks with use of MergeRequestsFinder.
+
+## 8.13.8 (2016-12-02)
+
+- Pass tag SHA to post-receive hook when tag is created via UI. !7700
+- Validate state param when filtering issuables.
+
+## 8.13.7 (2016-11-28)
+
+- fixes 500 error on project show when user is not logged in and project is still empty. !7376
+- Update grape entity to 0.6.0. !7491
+- Fix information disclosure in `Projects::BlobController#update`.
+- Fix missing access checks on issue lookup using IssuableFinder.
+- Replace issue access checks with use of IssuableFinder.
+- Non members cannot create labels through the API.
+
 ## 8.13.6 (2016-11-17)
 
 - Omniauth auto link LDAP user falls back to find by DN when user cannot be found by UID. !7002
+- Fix Milestone dropdown not stay selected for `Upcoming` and `No Milestone` option. !7117
+- Fix relative links in Markdown wiki when displayed in "Project" tab. !7218
 - Fix no "Register" tab if ldap auth is enabled (#24038). !7274 (Luc Didry)
 - Fix cache for commit status in commits list to respect branches. !7372
 - Fix issue causing Labels not to appear in sidebar on MR page. !7416 (Alex Sanford)

Gemfile

@@ -68,7 +68,7 @@ gem 'github-linguist', '~> 4.7.0', require: 'linguist'
 
 # API
 gem 'grape',        '~> 0.15.0'
-gem 'grape-entity', '~> 0.4.2'
+gem 'grape-entity', '~> 0.6.0'
 gem 'rack-cors',    '~> 0.4.0', require: 'rack/cors'
 
 # Pagination
@@ -101,7 +101,7 @@ gem 'seed-fu', '~> 2.3.5'
 # Markdown and HTML processing
 gem 'html-pipeline',      '~> 1.11.0'
 gem 'deckar01-task_list', '1.0.5', require: 'task_list/railtie'
-gem 'gitlab-markup',      '~> 1.5.0'
+gem 'gitlab-markup',      '~> 1.5.1'
 gem 'redcarpet',          '~> 3.3.3'
 gem 'RedCloth',           '~> 4.3.2'
 gem 'rdoc',               '~>3.6'
@@ -214,8 +214,7 @@ gem 'chronic_duration', '~> 0.10.6'
 gem 'sass-rails', '~> 5.0.6'
 gem 'coffee-rails', '~> 4.1.0'
 gem 'uglifier', '~> 2.7.2'
-gem 'turbolinks', '~> 2.5.0'
+gem 'gitlab-turbolinks-classic', '~> 2.5', '>= 2.5.6'
-gem 'jquery-turbolinks', '~> 2.1.0'
 
 gem 'addressable',        '~> 2.3.8'
 gem 'bootstrap-sass',     '~> 3.3.0'

Gemfile.lock

@@ -282,7 +282,9 @@ GEM
       diff-lcs (~> 1.1)
       mime-types (>= 1.16, < 3)
       posix-spawn (~> 0.3)
-    gitlab-markup (1.5.0)
+    gitlab-markup (1.5.1)
+    gitlab-turbolinks-classic (2.5.6)
+      coffee-rails
     gitlab_git (10.7.0)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.7.3)
@@ -322,7 +324,7 @@ GEM
       rack-accept
       rack-mount
       virtus (>= 1.0.0)
-    grape-entity (0.4.8)
+    grape-entity (0.6.0)
       activesupport
       multi_json (>= 1.3.2)
     haml (4.0.7)
@@ -361,9 +363,6 @@ GEM
       rails-dom-testing (>= 1, < 3)
       railties (>= 4.2.0)
       thor (>= 0.14, < 2.0)
-    jquery-turbolinks (2.1.0)
-      railties (>= 3.1.0)
-      turbolinks
     jquery-ui-rails (5.0.5)
       railties (>= 3.2.16)
     json (1.8.3)
@@ -751,8 +750,6 @@ GEM
     truncato (0.7.8)
       htmlentities (~> 4.3.1)
       nokogiri (~> 1.6.1)
-    turbolinks (2.5.3)
-      coffee-rails
     tzinfo (1.2.2)
       thread_safe (~> 0.1)
     u2f (0.2.1)
@@ -866,14 +863,15 @@ DEPENDENCIES
   gemojione (~> 3.0)
   github-linguist (~> 4.7.0)
   gitlab-flowdock-git-hook (~> 1.0.1)
-  gitlab-markup (~> 1.5.0)
+  gitlab-markup (~> 1.5.1)
+  gitlab-turbolinks-classic (~> 2.5, >= 2.5.6)
   gitlab_git (~> 10.7.0)
   gitlab_omniauth-ldap (~> 1.2.1)
   gollum-lib (~> 4.2)
   gollum-rugged_adapter (~> 0.4.2)
   gon (~> 6.1.0)
   grape (~> 0.15.0)
-  grape-entity (~> 0.4.2)
+  grape-entity (~> 0.6.0)
   haml_lint (~> 0.18.2)
   hamlit (~> 2.6.1)
   health_check (~> 2.2.0)
@@ -883,7 +881,6 @@ DEPENDENCIES
   influxdb (~> 0.2)
   jquery-atwho-rails (~> 1.3.2)
   jquery-rails (~> 4.1.0)
-  jquery-turbolinks (~> 2.1.0)
   jquery-ui-rails (~> 5.0.0)
   json-schema (~> 2.6.2)
   jwt
@@ -979,7 +976,6 @@ DEPENDENCIES
   thin (~> 1.7.0)
   timecop (~> 0.8.0)
   truncato (~> 0.7.8)
-  turbolinks (~> 2.5.0)
   u2f (~> 0.2.1)
   uglifier (~> 2.7.2)
   underscore-rails (~> 1.8.0)
@@ -994,4 +990,4 @@ DEPENDENCIES
   wikicloth (= 0.8.1)
 
 BUNDLED WITH
-   1.13.5
+   1.13.6

VERSION

@@ -1 +1 @@
-8.13.6
+8.13.11

app/controllers/concerns/creates_commit.rb

@@ -81,10 +81,8 @@ module CreatesCommit
   def merge_request_exists?
     return @merge_request if defined?(@merge_request)
 
-    @merge_request = @mr_target_project.merge_requests.opened.find_by(
+    @merge_request = MergeRequestsFinder.new(current_user, project_id: @mr_target_project.id).execute.opened.
-      source_branch: @mr_source_branch,
+      find_by(source_branch: @mr_source_branch, target_branch: @mr_target_branch)
-      target_branch: @mr_target_branch
-    )
   end
 
   def different_project?

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) +
-          "#file-path-#{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
+  def after_edit_path
-    # If blob edit was initiated from merge request page
+    from_merge_request = MergeRequestsFinder.new(current_user, project_id: @project.id).execute.find_by(iid: params[:from_merge_request_iid])
-    @from_merge_request ||= MergeRequest.find_by(id: params[:from_merge_request_id])
+    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) +
+        "#file-path-#{hexdigest(@path)}"
+    else
+      namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @path))
+    end
   end
 
   def editor_variables

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
 

app/controllers/projects/commit_controller.rb

@@ -65,7 +65,7 @@ class Projects::CommitController < Projects::ApplicationController
 
     return render_404 if @target_branch.blank?
 
-    create_commit(Commits::RevertService, success_notice: "The #{@commit.change_type_title} has been successfully reverted.",
+    create_commit(Commits::RevertService, success_notice: "The #{@commit.change_type_title(current_user)} has been successfully reverted.",
                                           success_path: successful_change_path, failure_path: failed_change_path)
   end
 
@@ -74,26 +74,24 @@ class Projects::CommitController < Projects::ApplicationController
 
     return render_404 if @target_branch.blank?
 
-    create_commit(Commits::CherryPickService, success_notice: "The #{@commit.change_type_title} has been successfully cherry-picked.",
+    create_commit(Commits::CherryPickService, success_notice: "The #{@commit.change_type_title(current_user)} has been successfully cherry-picked.",
                                               success_path: successful_change_path, failure_path: failed_change_path)
   end
 
   private
 
   def successful_change_path
-    return referenced_merge_request_url if @commit.merged_merge_request
+    referenced_merge_request_url || namespace_project_commits_url(@project.namespace, @project, @target_branch)
-
-    namespace_project_commits_url(@project.namespace, @project, @target_branch)
   end
 
   def failed_change_path
-    return referenced_merge_request_url if @commit.merged_merge_request
+    referenced_merge_request_url || namespace_project_commit_url(@project.namespace, @project, params[:id])
-
-    namespace_project_commit_url(@project.namespace, @project, params[:id])
   end
 
   def referenced_merge_request_url
-    namespace_project_merge_request_url(@project.namespace, @project, @commit.merged_merge_request)
+    if merge_request = @commit.merged_merge_request(current_user)
+      namespace_project_merge_request_url(@project.namespace, @project, merge_request)
+    end
   end
 
   def commit

app/controllers/projects/commits_controller.rb

@@ -21,7 +21,7 @@ class Projects::CommitsController < Projects::ApplicationController
     @note_counts = project.notes.where(commit_id: @commits.map(&:id)).
       group(:commit_id).count
 
-    @merge_request = @project.merge_requests.opened.
+    @merge_request = MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened.
       find_by(source_project: @project, source_branch: @ref, target_branch: @repository.root_ref)
 
     respond_to do |format|

app/controllers/projects/compare_controller.rb

@@ -53,7 +53,7 @@ class Projects::CompareController < Projects::ApplicationController
   end
 
   def merge_request
-    @merge_request ||= @project.merge_requests.opened.
+    @merge_request ||= MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened.
       find_by(source_project: @project, source_branch: @head_ref, target_branch: @start_ref)
   end
 end

app/controllers/projects/cycle_analytics_controller.rb

@@ -5,7 +5,7 @@ class Projects::CycleAnalyticsController < Projects::ApplicationController
   before_action :authorize_read_cycle_analytics!
 
   def show
-    @cycle_analytics = CycleAnalytics.new(@project, from: parse_start_date)
+    @cycle_analytics = CycleAnalytics.new(@project, current_user, from: parse_start_date)
 
     respond_to do |format|
       format.html

app/controllers/projects/discussions_controller.rb

@@ -26,7 +26,7 @@ class Projects::DiscussionsController < Projects::ApplicationController
   private
 
   def merge_request
-    @merge_request ||= @project.merge_requests.find_by!(iid: params[:merge_request_id])
+    @merge_request ||= MergeRequestsFinder.new(current_user, project_id: @project.id).find_by!(iid: params[:merge_request_id])
   end
 
   def discussion

app/controllers/projects/notes_controller.rb

@@ -215,6 +215,6 @@ class Projects::NotesController < Projects::ApplicationController
   end
 
   def find_current_user_notes
-    @notes = NotesFinder.new.execute(project, current_user, params)
+    @notes = NotesFinder.new(project, current_user, params).execute.inc_author
   end
 end

app/controllers/projects/todos_controller.rb

@@ -16,15 +16,9 @@ class Projects::TodosController < Projects::ApplicationController
     @issuable ||= begin
       case params[:issuable_type]
       when "issue"
-        issue = @project.issues.find(params[:issuable_id])
+        IssuesFinder.new(current_user, project_id: @project.id).find(params[:issuable_id])
-
-        if can?(current_user, :read_issue, issue)
-          issue
-        else
-          render_404
-        end
       when "merge_request"
-        @project.merge_requests.find(params[:issuable_id])
+        MergeRequestsFinder.new(current_user, project_id: @project.id).find(params[:issuable_id])
       end
     end
   end

app/finders/issuable_finder.rb

@@ -7,7 +7,7 @@
 #   current_user - which user use
 #   params:
 #     scope: 'created-by-me' or 'assigned-to-me' or 'all'
-#     state: 'open' or 'closed' or 'all'
+#     state: 'opened' or 'closed' or 'all'
 #     group_id: integer
 #     project_id: integer
 #     milestone_title: string
@@ -23,7 +23,7 @@ class IssuableFinder
 
   attr_accessor :current_user, :params
 
-  def initialize(current_user, params)
+  def initialize(current_user, params = {})
     @current_user = current_user
     @params = params
   end
@@ -43,6 +43,18 @@ class IssuableFinder
     sort(items)
   end
 
+  def find(*params)
+    execute.find(*params)
+  end
+
+  def find_by(*params)
+    execute.find_by(*params)
+  end
+
+  def find_by!(*params)
+    execute.find_by!(*params)
+  end
+
   def group
     return @group if defined?(@group)
 
@@ -175,10 +187,13 @@ class IssuableFinder
   end
 
   def by_state(items)
-    params[:state] ||= 'all'
+    case params[:state].to_s
-
+    when 'closed'
-    if items.respond_to?(params[:state])
+      items.closed
-      items.public_send(params[:state])
+    when 'merged'
+      items.respond_to?(:merged) ? items.merged : items.closed
+    when 'opened'
+      items.opened
     else
       items
     end

app/finders/issues_finder.rb

@@ -23,10 +23,26 @@ class IssuesFinder < IssuableFinder
   private
 
   def init_collection
-    Issue.visible_to_user(current_user)
+    IssuesFinder.not_restricted_by_confidentiality(current_user)
   end
 
   def iid_pattern
     @iid_pattern ||= %r{\A#{Regexp.escape(Issue.reference_prefix)}(?<iid>\d+)\z}
   end
+
+  def self.not_restricted_by_confidentiality(user)
+    return Issue.where('issues.confidential IS NULL OR issues.confidential IS FALSE') if user.blank?
+
+    return Issue.all if user.admin?
+
+    Issue.where('
+      issues.confidential IS NULL
+      OR issues.confidential IS FALSE
+      OR (issues.confidential = TRUE
+        AND (issues.author_id = :user_id
+          OR issues.assignee_id = :user_id
+          OR issues.project_id IN(:project_ids)))',
+      user_id: user.id,
+      project_ids: user.authorized_projects(Gitlab::Access::REPORTER).select(:id))
+  end
 end

app/finders/notes_finder.rb

@@ -1,27 +1,102 @@
 class NotesFinder
   FETCH_OVERLAP = 5.seconds
 
-  def execute(project, current_user, params)
+  # Used to filter Notes
-    target_type = params[:target_type]
+  # When used with target_type and target_id this returns notes specifically for the controller
-    target_id   = params[:target_id]
+  #
-    # Default to 0 to remain compatible with old clients
+  # Arguments:
-    last_fetched_at = Time.at(params.fetch(:last_fetched_at, 0).to_i)
+  #   current_user - which user check authorizations with
+  #   project - which project to look for notes on
+  #   params:
+  #     target_type: string
+  #     target_id: integer
+  #     last_fetched_at: time
+  #     search: string
+  #
+  def initialize(project, current_user, params = {})
+    @project = project
+    @current_user = current_user
+    @params = params
+    init_collection
+  end
 
-    notes =
+  def execute
-      case target_type
+    @notes = since_fetch_at(@params[:last_fetched_at]) if @params[:last_fetched_at]
-      when "commit"
+    @notes
-        project.notes.for_commit_id(target_id).non_diff_notes
+  end
+
+  private
+
+  def init_collection
+    if @params[:target_id]
+      @notes = on_target(@params[:target_type], @params[:target_id])
+    else
+      @notes = notes_of_any_type
+    end
+  end
+
+  def notes_of_any_type
+    types = %w(commit issue merge_request snippet)
+    note_relations = types.map { |t| notes_for_type(t) }
+    note_relations.map!{ |notes| search(@params[:search], notes) } if @params[:search]
+    UnionFinder.new.find_union(note_relations, Note)
+  end
+
+  def noteables_for_type(noteable_type)
+    case noteable_type
     when "issue"
-        project.issues.visible_to_user(current_user).find(target_id).notes.inc_author
+      IssuesFinder.new(@current_user, project_id: @project.id).execute
     when "merge_request"
-        project.merge_requests.find(target_id).mr_and_commit_notes.inc_author
+      MergeRequestsFinder.new(@current_user, project_id: @project.id).execute
     when "snippet", "project_snippet"
-        project.snippets.find(target_id).notes
+      SnippetsFinder.new.execute(@current_user, filter: :by_project, project: @project)
     else
       raise 'invalid target_type'
     end
+  end
 
-    # Use overlapping intervals to avoid worrying about race conditions
+  def notes_for_type(noteable_type)
-    notes.where('updated_at > ?', last_fetched_at - FETCH_OVERLAP).fresh
+    if noteable_type == "commit"
+      if Ability.allowed?(@current_user, :download_code, @project)
+        @project.notes.where(noteable_type: 'Commit')
+      else
+        Note.none
+      end
+    else
+      finder = noteables_for_type(noteable_type)
+      @project.notes.where(noteable_type: finder.base_class.name, noteable_id: finder.reorder(nil))
+    end
+  end
+
+  def on_target(target_type, target_id)
+    if target_type == "commit"
+      notes_for_type('commit').for_commit_id(target_id)
+    else
+      target = noteables_for_type(target_type).find(target_id)
+
+      if target.respond_to?(:related_notes)
+        target.related_notes
+      else
+        target.notes
+      end
+    end
+  end
+
+  # Searches for notes matching the given query.
+  #
+  # This method uses ILIKE on PostgreSQL and LIKE on MySQL.
+  #
+  def search(query, notes_relation = @notes)
+    pattern = "%#{query}%"
+    notes_relation.where(Note.arel_table[:note].matches(pattern))
+  end
+
+  # Notes changed since last fetch
+  # Uses overlapping intervals to avoid worrying about race conditions
+  def since_fetch_at(fetch_time)
+    # Default to 0 to remain compatible with old clients
+    last_fetched_at = Time.at(@params.fetch(:last_fetched_at, 0).to_i)
+
+    @notes.where('updated_at > ?', last_fetched_at - FETCH_OVERLAP).fresh
   end
 end

app/helpers/commits_helper.rb

@@ -130,7 +130,7 @@ module CommitsHelper
   def revert_commit_link(commit, continue_to_path, btn_class: nil, has_tooltip: true)
     return unless current_user
 
-    tooltip = "Revert this #{commit.change_type_title} in a new merge request" if has_tooltip
+    tooltip = "Revert this #{commit.change_type_title(current_user)} in a new merge request" if has_tooltip
 
     if can_collaborate_with_project?
       btn_class = "btn btn-warning btn-#{btn_class}" unless btn_class.nil?
@@ -154,7 +154,7 @@ module CommitsHelper
   def cherry_pick_commit_link(commit, continue_to_path, btn_class: nil, has_tooltip: true)
     return unless current_user
 
-    tooltip = "Cherry-pick this #{commit.change_type_title} in a new merge request"
+    tooltip = "Cherry-pick this #{commit.change_type_title(current_user)} in a new merge request"
 
     if can_collaborate_with_project?
       btn_class = "btn btn-default btn-#{btn_class}" unless btn_class.nil?

app/helpers/preferences_helper.rb

@@ -50,7 +50,7 @@ module PreferencesHelper
   end
 
   def default_project_view
-    return 'readme' unless current_user
+    return anonymous_project_view unless current_user
 
     user_view = current_user.project_view
 
@@ -66,4 +66,8 @@ module PreferencesHelper
       "customize_workflow"
     end
   end
+
+  def anonymous_project_view
+    @project.empty_repo? || !can?(current_user, :download_code, @project) ? 'activity' : 'readme'
+  end
 end

app/models/commit.rb

@@ -249,44 +249,47 @@ class Commit
     project.repository.next_branch("cherry-pick-#{short_id}", mild: true)
   end
 
-  def revert_description
+  def revert_description(user)
-    if merged_merge_request
+    if merged_merge_request?(user)
-      "This reverts merge request #{merged_merge_request.to_reference}"
+      "This reverts merge request #{merged_merge_request(user).to_reference}"
     else
       "This reverts commit #{sha}"
     end
   end
 
-  def revert_message
+  def revert_message(user)
-    %Q{Revert "#{title.strip}"\n\n#{revert_description}}
+    %Q{Revert "#{title.strip}"\n\n#{revert_description(user)}}
   end
 
-  def reverts_commit?(commit)
+  def reverts_commit?(commit, user)
-    description? && description.include?(commit.revert_description)
+    description? && description.include?(commit.revert_description(user))
   end
 
   def merge_commit?
     parents.size > 1
   end
 
-  def merged_merge_request
+  def merged_merge_request(current_user)
-    return @merged_merge_request if defined?(@merged_merge_request)
+    # Memoize with per-user access check
-
+    @merged_merge_request_hash ||= Hash.new do |hash, user|
-    @merged_merge_request = project.merge_requests.find_by(merge_commit_sha: id) if merge_commit?
+      hash[user] = merged_merge_request_no_cache(user)
     end
     
-  def has_been_reverted?(current_user = nil, noteable = self)
+    @merged_merge_request_hash[current_user]
+  end
+
+  def has_been_reverted?(current_user, noteable = self)
     ext = all_references(current_user)
 
     noteable.notes_with_associations.system.each do |note|
       note.all_references(current_user, extractor: ext)
     end
 
-    ext.commits.any? { |commit_ref| commit_ref.reverts_commit?(self) }
+    ext.commits.any? { |commit_ref| commit_ref.reverts_commit?(self, current_user) }
   end
 
-  def change_type_title
+  def change_type_title(user)
-    merged_merge_request ? 'merge request' : 'commit'
+    merged_merge_request?(user) ? 'merge request' : 'commit'
   end
 
   # Get the URI type of the given path
@@ -344,4 +347,12 @@ class Commit
 
     changes
   end
+
+  def merged_merge_request?(user)
+    !!merged_merge_request(user)
+  end
+
+  def merged_merge_request_no_cache(user)
+    MergeRequestsFinder.new(user, project_id: project.id).find_by(merge_commit_sha: id) if merge_commit?
+  end
 end

app/models/concerns/milestoneish.rb

@@ -1,17 +1,17 @@
 module Milestoneish
-  def closed_items_count(user = nil)
+  def closed_items_count(user)
     issues_visible_to_user(user).closed.size + merge_requests.closed_and_merged.size
   end
 
-  def total_items_count(user = nil)
+  def total_items_count(user)
     issues_visible_to_user(user).size + merge_requests.size
   end
 
-  def complete?(user = nil)
+  def complete?(user)
     total_items_count(user) > 0 && total_items_count(user) == closed_items_count(user)
   end
 
-  def percent_complete(user = nil)
+  def percent_complete(user)
     ((closed_items_count(user) * 100) / total_items_count(user)).abs
   rescue ZeroDivisionError
     0
@@ -23,7 +23,7 @@ module Milestoneish
     (due_date - Date.today).to_i
   end
 
-  def issues_visible_to_user(user = nil)
+  def issues_visible_to_user(user)
-    issues.visible_to_user(user)
+    IssuesFinder.new(user).execute.where(id: issues)
   end
 end

app/models/cycle_analytics.rb

@@ -4,13 +4,14 @@ class CycleAnalytics
 
   DEPLOYMENT_METRIC_STAGES = %i[production staging]
 
-  def initialize(project, from:)
+  def initialize(project, current_user, from:)
     @project = project
+    @current_user = current_user
     @from = from
   end
 
   def summary
-    @summary ||= Summary.new(@project, from: @from)
+    @summary ||= Summary.new(@project, @current_user, from: @from)
   end
 
   def issue

app/models/cycle_analytics/summary.rb

@@ -1,12 +1,13 @@
 class CycleAnalytics
   class Summary
-    def initialize(project, from:)
+    def initialize(project, current_user, from:)
       @project = project
+      @current_user = current_user
       @from = from
     end
 
     def new_issues
-      @project.issues.created_after(@from).count
+      IssuesFinder.new(@current_user, project_id: @project.id).execute.created_after(@from).count
     end
 
     def commits

app/models/issue.rb

@@ -61,61 +61,6 @@ class Issue < ActiveRecord::Base
     attributes
   end
 
-  class << self
-    private
-
-    # Returns the project that the current scope belongs to if any, nil otherwise.
-    #
-    # Examples:
-    # - my_project.issues.without_due_date.owner_project => my_project
-    # - Issue.all.owner_project => nil
-    def owner_project
-      # No owner if we're not being called from an association
-      return unless all.respond_to?(:proxy_association)
-
-      owner = all.proxy_association.owner
-
-      # Check if the association is or belongs to a project
-      if owner.is_a?(Project)
-        owner
-      else
-        begin
-          owner.association(:project).target
-        rescue ActiveRecord::AssociationNotFoundError
-          nil
-        end
-      end
-    end
-  end
-
-  def self.visible_to_user(user)
-    return where('issues.confidential IS NULL OR issues.confidential IS FALSE') if user.blank?
-    return all if user.admin?
-
-    # Check if we are scoped to a specific project's issues
-    if owner_project
-      if owner_project.authorized_for_user?(user, Gitlab::Access::REPORTER)
-        # If the project is authorized for the user, they can see all issues in the project
-        return all
-      else
-        # else only non confidential and authored/assigned to them
-        return where('issues.confidential IS NULL OR issues.confidential IS FALSE
-          OR issues.author_id = :user_id OR issues.assignee_id = :user_id',
-          user_id: user.id)
-      end
-    end
-
-    where('
-      issues.confidential IS NULL
-      OR issues.confidential IS FALSE
-      OR (issues.confidential = TRUE
-        AND (issues.author_id = :user_id
-          OR issues.assignee_id = :user_id
-          OR issues.project_id IN(:project_ids)))',
-      user_id: user.id,
-      project_ids: user.authorized_projects(Gitlab::Access::REPORTER).select(:id))
-  end
-
   def self.reference_prefix
     '#'
   end

app/models/merge_request.rb

@@ -453,7 +453,7 @@ class MergeRequest < ActiveRecord::Base
     should_remove_source_branch? || force_remove_source_branch?
   end
 
-  def mr_and_commit_notes
+  def related_notes
     # Fetch comments only from last 100 commits
     commits_for_notes_limit = 100
     commit_ids = commits.last(commits_for_notes_limit).map(&:id)
@@ -469,7 +469,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def discussions
-    @discussions ||= self.mr_and_commit_notes.
+    @discussions ||= self.related_notes.
       inc_relations_for_view.
       fresh.
       discussions
@@ -803,7 +803,7 @@ class MergeRequest < ActiveRecord::Base
     @merge_commit ||= project.commit(merge_commit_sha) if merge_commit_sha
   end
 
-  def can_be_reverted?(current_user = nil)
+  def can_be_reverted?(current_user)
     merge_commit && !merge_commit.has_been_reverted?(current_user, self)
   end
 

app/models/note.rb

@@ -103,23 +103,6 @@ class Note < ActiveRecord::Base
       Discussion.for_diff_notes(active_notes).
         map { |d| [d.line_code, d] }.to_h
     end
-
-    # Searches for notes matching the given query.
-    #
-    # This method uses ILIKE on PostgreSQL and LIKE on MySQL.
-    #
-    # query   - The search query as a String.
-    # as_user - Limit results to those viewable by a specific user
-    #
-    # Returns an ActiveRecord::Relation.
-    def search(query, as_user: nil)
-      table   = arel_table
-      pattern = "%#{query}%"
-
-      Note.joins('LEFT JOIN issues ON issues.id = noteable_id').
-        where(table[:note].matches(pattern)).
-        merge(Issue.visible_to_user(as_user))
-    end
   end
 
   def cross_reference?

app/models/project.rb

@@ -679,9 +679,9 @@ class Project < ActiveRecord::Base
     self.id
   end
 
-  def get_issue(issue_id)
+  def get_issue(issue_id, current_user)
     if default_issues_tracker?
-      issues.find_by(iid: issue_id)
+      IssuesFinder.new(current_user, project_id: id).find_by(iid: issue_id)
     else
       ExternalIssue.new(issue_id, self)
     end

app/models/repository.rb

@@ -167,8 +167,9 @@ class Repository
 
     options = { message: message, tagger: user_to_committer(user) } if message
 
-    GitHooksService.new.execute(user, path_to_repo, oldrev, target, ref) do
+    GitHooksService.new.execute(user, path_to_repo, oldrev, target, ref) do |service|
-      rugged.tags.create(tag_name, target, options)
+      raw_tag = rugged.tags.create(tag_name, target, options)
+      service.newrev = raw_tag.target_id
     end
 
     find_tag(tag_name)
@@ -953,7 +954,7 @@ class Repository
     update_branch_with_hooks(user, base_branch) do
       committer = user_to_committer(user)
       source_sha = Rugged::Commit.create(rugged,
-        message: commit.revert_message,
+        message: commit.revert_message(user),
         author: committer,
         committer: committer,
         tree: revert_tree_id,

app/models/user.rb

@@ -275,10 +275,6 @@ class User < ActiveRecord::Base
       personal_access_token.user if personal_access_token
     end
 
-    def by_username_or_id(name_or_id)
-      find_by('users.username = ? OR users.id = ?', name_or_id.to_s, name_or_id.to_i)
-    end
-
     # Returns a user for the given SSH key.
     def find_by_ssh_key_id(key_id)
       find_by(id: Key.unscoped.select(:user_id).where(id: key_id))

app/services/commits/change_service.rb

@@ -34,7 +34,7 @@ module Commits
         repository.public_send(action, current_user, @commit, into, tree_id)
         success
       else
-        error_msg = "Sorry, we cannot #{action.to_s.dasherize} this #{@commit.change_type_title} automatically.
+        error_msg = "Sorry, we cannot #{action.to_s.dasherize} this #{@commit.change_type_title(current_user)} automatically.
                      It may have already been #{action.to_s.dasherize}, or a more recent commit may have updated some of its content."
         raise ChangeError, error_msg
       end

app/services/git_hooks_service.rb

@@ -1,6 +1,8 @@
 class GitHooksService
   PreReceiveError = Class.new(StandardError)
 
+  attr_accessor :oldrev, :newrev, :ref
+
   def execute(user, repo_path, oldrev, newrev, ref)
     @repo_path  = repo_path
     @user       = Gitlab::GlId.gl_id(user)
@@ -16,7 +18,7 @@ class GitHooksService
       end
     end
 
-    yield
+    yield self
 
     run_hook('post-receive')
   end
@@ -25,6 +27,6 @@ class GitHooksService
 
   def run_hook(name)
     hook = Gitlab::Git::Hook.new(name, @repo_path)
-    hook.trigger(@user, @oldrev, @newrev, @ref)
+    hook.trigger(@user, oldrev, newrev, ref)
   end
 end

app/services/issuable_base_service.rb

@@ -85,14 +85,15 @@ class IssuableBaseService < BaseService
 
   def find_or_create_label_ids
     labels = params.delete(:labels)
+
     return unless labels
 
-    params[:label_ids] = labels.split(',').map do |label_name|
+    params[:label_ids] = labels.split(",").map do |label_name|
       service = Labels::FindOrCreateService.new(current_user, project, title: label_name.strip)
       label   = service.execute
 
-      label.id
+      label.try(:id)
-    end
+    end.compact
   end
 
   def process_label_ids(attributes, existing_label_ids: nil)
@@ -140,6 +141,7 @@ class IssuableBaseService < BaseService
 
     params.delete(:state_event)
     params[:author] ||= current_user
+
     label_ids = process_label_ids(params)
 
     issuable.assign_attributes(params)

app/services/labels/find_or_create_service.rb

@@ -22,9 +22,14 @@ module Labels
       ).execute(skip_authorization: skip_authorization)
     end
 
+    # Only creates the label if current_user can do so, if the label does not exist
+    # and the user can not create the label, nil is returned
     def find_or_create_label
       new_label = available_labels.find_by(title: title)
-      new_label ||= project.labels.create(params)
+
+      if new_label.nil? && (skip_authorization || Ability.allowed?(current_user, :admin_label, project))
+        new_label = project.labels.create(params)
+      end
 
       new_label
     end

app/services/merge_requests/build_service.rb

@@ -65,7 +65,7 @@ module MergeRequests
         commit = commits.first
         merge_request.title = commit.title
         merge_request.description ||= commit.description.try(:strip)
-      elsif iid && (issue = merge_request.target_project.get_issue(iid)) && !issue.try(:confidential?)
+      elsif iid && issue = merge_request.target_project.get_issue(iid, current_user)
         case issue
         when Issue
           merge_request.title = "Resolve \"#{issue.title}\""

app/views/layouts/nav/_project.html.haml

@@ -70,14 +70,14 @@
           %span
             Issues
             - if @project.default_issues_tracker?
-              %span.badge.count.issue_counter= number_with_delimiter(@project.issues.visible_to_user(current_user).opened.count)
+              %span.badge.count.issue_counter= number_with_delimiter(IssuesFinder.new(current_user, project_id: @project.id).execute.opened.count)
 
     - if project_nav_tab? :merge_requests
       = nav_link(controller: :merge_requests) do
         = link_to namespace_project_merge_requests_path(@project.namespace, @project), title: 'Merge Requests', class: 'shortcuts-merge_requests' do
           %span
             Merge Requests
-            %span.badge.count.merge_counter= number_with_delimiter(@project.merge_requests.opened.count)
+            %span.badge.count.merge_counter= number_with_delimiter(MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened.count)
 
     - if project_nav_tab? :wiki
       = nav_link(controller: :wikis) do

app/views/projects/blob/edit.html.haml

@@ -27,5 +27,5 @@
       = render 'shared/new_commit_form', placeholder: "Update #{@blob.name}"
       = hidden_field_tag 'last_commit_sha', @last_commit_sha
       = hidden_field_tag 'content', '', id: "file-content"
-      = hidden_field_tag 'from_merge_request_id', params[:from_merge_request_id]
+      = hidden_field_tag 'from_merge_request_iid', params[:from_merge_request_iid]
       = render 'projects/commit_button', ref: @ref, cancel_path: namespace_project_blob_path(@project.namespace, @project, @id)

app/views/projects/commit/_change.html.haml

@@ -11,7 +11,7 @@
     .modal-content
       .modal-header
         %a.close{href: "#", "data-dismiss" => "modal"} ×
-        %h3.page-title== #{label} this #{commit.change_type_title}
+        %h3.page-title== #{label} this #{commit.change_type_title(current_user)}
       .modal-body
         = form_tag send("#{type.underscore}_namespace_project_commit_path", @project.namespace, @project, commit.id), method: :post, remote: false, class: 'form-horizontal js-#{type}-form js-requires-input' do
           .form-group.branch

app/views/projects/diffs/_file.html.haml

@@ -10,7 +10,7 @@
           \
           = clipboard_button(clipboard_text: diff_file.new_path, class: 'btn-file-option')
           - if editable_diff?(diff_file)
-            - link_opts = @merge_request.id ? { from_merge_request_id: @merge_request.id } : {}
+            - link_opts = @merge_request.persisted? ? { from_merge_request_iid: @merge_request.iid } : {}
             = edit_blob_link(@merge_request.source_project, @merge_request.source_branch, diff_file.new_path,
                              blob: blob, link_opts: link_opts)
 

app/views/projects/merge_requests/_merge_request.html.haml

@@ -39,7 +39,7 @@
           = icon('thumbs-down')
           = downvotes
 
-      - note_count = merge_request.mr_and_commit_notes.user.count
+      - note_count = merge_request.related_notes.user.count
       %li
         = link_to merge_request_path(merge_request, anchor: 'notes'), class: ('no-comments' if note_count.zero?) do
           = icon('comments')

app/views/projects/merge_requests/_show.html.haml

@@ -53,7 +53,7 @@
             %li.notes-tab
               = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do
                 Discussion
-                %span.badge= @merge_request.mr_and_commit_notes.user.count
+                %span.badge= @merge_request.related_notes.user.count
             - if @merge_request.source_project
               %li.commits-tab
                 = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do

config/application.rb

@@ -45,7 +45,7 @@ module Gitlab
     #
     # Parameters filtered:
     # - Password (:password, :password_confirmation)
-    # - Private tokens (:private_token)
+    # - Private tokens
     # - Two-factor tokens (:otp_attempt)
     # - Repo/Project Import URLs (:import_url)
     # - Build variables (:variables)
@@ -55,15 +55,18 @@ module Gitlab
     # - Sentry DSN (:sentry_dsn)
     # - Deploy keys (:key)
     config.filter_parameters += %i(
+      authentication_token
       certificate
       encrypted_key
       hook
       import_url
+      incoming_email_token
       key
       otp_attempt
       password
       password_confirmation
       private_token
+      runners_token
       secret_token
       sentry_dsn
       variables

db/migrate/20130319214458_create_forked_project_links.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateForkedProjectLinks < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :forked_project_links do |t|
       t.integer :forked_to_project_id, null: false

db/migrate/20130506090604_create_deploy_keys_projects.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateDeployKeysProjects < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :deploy_keys_projects do |t|
       t.integer :deploy_key_id, null: false

db/migrate/20130617095603_create_users_groups.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateUsersGroups < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :users_groups do |t|
       t.integer :group_access, null: false

db/migrate/20130711063759_create_project_group_links.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateProjectGroupLinks < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :project_group_links do |t|
       t.integer :project_id, null: false

db/migrate/20131112114325_create_broadcast_messages.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateBroadcastMessages < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :broadcast_messages do |t|
       t.text :message, null: false

db/migrate/20140122112253_create_merge_request_diffs.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateMergeRequestDiffs < ActiveRecord::Migration
+  DOWNTIME = false
+
   def up
     create_table :merge_request_diffs do |t|
       t.string :state, null: false, default: 'collected'

db/migrate/20140209025651_create_emails.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateEmails < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :emails do |t|
       t.integer  :user_id, null: false

db/migrate/20140625115202_create_users_star_projects.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateUsersStarProjects < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :users_star_projects do |t|
       t.integer :project_id, null: false

db/migrate/20140729134820_create_labels.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateLabels < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :labels do |t|
       t.string :title

db/migrate/20140729140420_create_label_links.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateLabelLinks < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :label_links do |t|
       t.integer :label_id

db/migrate/20140914113604_add_members_table.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class AddMembersTable < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :members do |t|
       t.integer :access_level, null: false

db/migrate/20140914173417_remove_old_member_tables.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class RemoveOldMemberTables < ActiveRecord::Migration
+  DOWNTIME = false
+
   def up
     drop_table :users_groups
     drop_table :users_projects

db/migrate/20141118150935_add_audit_event.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class AddAuditEvent < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :audit_events do |t|
       t.integer :author_id, null: false

db/migrate/20141216155758_create_doorkeeper_tables.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateDoorkeeperTables < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :oauth_applications do |t|
       t.string  :name,         null: false

db/migrate/20150108073740_create_application_settings.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateApplicationSettings < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :application_settings do |t|
       t.integer :default_projects_limit

db/migrate/20150313012111_create_subscriptions_table.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateSubscriptionsTable < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :subscriptions do |t|
       t.integer :user_id

db/migrate/20150806104937_create_abuse_reports.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateAbuseReports < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :abuse_reports do |t|
       t.integer :reporter_id

db/migrate/20151103134857_create_lfs_objects.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateLfsObjects < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :lfs_objects do |t|
       t.string :oid, null: false, unique: true

db/migrate/20151103134958_create_lfs_objects_projects.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateLfsObjectsProjects < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :lfs_objects_projects do |t|
       t.integer :lfs_object_id, null: false

db/migrate/20151105094515_create_releases.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateReleases < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :releases do |t|
       t.string :tag

db/migrate/20160212123307_create_tasks.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class CreateTasks < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :tasks do |t|
       t.references :user, null: false, index: true

db/migrate/20160416180807_add_award_emoji.rb

@@ -1,5 +1,7 @@
 # rubocop:disable all
 class AddAwardEmoji < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :award_emoji do |t|
       t.string :name

doc/api/services.md

@@ -451,34 +451,7 @@ GET /projects/:id/services/irker
 
 ## JIRA
 
-Jira issue tracker
+JIRA issue tracker.
-
-### Create/Edit JIRA service
-
-Set JIRA service for a project.
-
-> Setting `project_url`, `issues_url` and `new_issue_url` will allow a user to easily navigate to the Jira issue tracker. See the [integration doc](http://docs.gitlab.com/ce/integration/external-issue-tracker.html) for details.  Support for referencing commits and automatic closing of Jira issues directly from GitLab is [available in GitLab EE.](http://docs.gitlab.com/ee/integration/jira.html)
-
-```
-PUT /projects/:id/services/jira
-```
-
-Parameters:
-
-- `new_issue_url` (**required**) - New Issue url
-- `project_url` (**required**) - Project url
-- `issues_url` (**required**) - Issue url
-- `description` (optional) - Jira issue tracker
-- `username` (optional) - Jira username
-- `password` (optional) - Jira password
-
-### Delete JIRA service
-
-Delete JIRA service for a project.
-
-```
-DELETE /projects/:id/services/jira
-```
 
 ### Get JIRA service settings
 
@@ -488,6 +461,39 @@ Get JIRA service settings for a project.
 GET /projects/:id/services/jira
 ```
 
+### Create/Edit JIRA service
+
+Set JIRA service for a project.
+
+>**Note:**
+Setting `project_url`, `issues_url` and `new_issue_url` will allow a user to
+easily navigate to the JIRA issue tracker. See the [integration doc][jira-doc]
+for details.
+
+```
+PUT /projects/:id/services/jira
+```
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `active`        | boolean| no  | Enable/disable the JIRA service. |
+| `project_url`   | string | yes | The URL to the JIRA project which is being linked to this GitLab project. It is of the form: `https://<jira_host_url>/issues/?jql=project=<jira_project>`. |
+| `issues_url`    | string | yes | The URL to the JIRA project issues overview for the project that is linked to this GitLab project. It is of the form: `https://<jira_host_url>/browse/:id`. Leave `:id` as-is, it gets replaced by GitLab at runtime.|
+| `new_issue_url` | string | yes | This is the URL to create a new issue in JIRA for the project linked to this GitLab project, and it is of the form: `https://<jira_host_url>/secure/CreateIssue.jspa` |
+| `api_url`       | string | yes | The base URL of the JIRA API. It may be omitted, in which case GitLab will automatically use API version `2` based on the `project url`. It is of the form: `https://<jira_host_url>/rest/api/2`. |
+| `description`   | string | no  | A name for the issue tracker. |
+| `username`      | string | no  | The username of the user created to be used with GitLab/JIRA. |
+| `password`      | string | no  | The password of the user created to be used with GitLab/JIRA. |
+| `jira_issue_transition_id` | string | no | The ID of a transition that moves issues to a closed state. You can find this number under the JIRA workflow administration (**Administration > Issues > Workflows**) by selecting **View** under **Operations** of the desired workflow of your project. The ID of each state can be found inside the parenthesis of each transition name under the **Transitions (id)** column ([see screenshot][trans]). By default, this ID is set to `2`. |
+
+### Delete JIRA service
+
+Remove all previously JIRA settings from a project.
+
+```
+DELETE /projects/:id/services/jira
+```
+
 ## PivotalTracker
 
 Project Management Software (Source Commits Endpoint)
@@ -662,3 +668,5 @@ Get JetBrains TeamCity CI service settings for a project.
 ```
 GET /projects/:id/services/teamcity
 ```
+
+[jira-doc]: ../project_services/jira.md

doc/api/users.md

@@ -277,7 +277,9 @@ Parameters:
 
 - `id` (required) - The ID of the user
 
-## Current user
+## User
+
+### For normal users
 
 Gets currently authenticated user.
 
@@ -321,6 +323,53 @@ GET /user
 }
 ```
 
+### For admins
+
+Parameters:
+
+- `sudo` (required) - the ID of a user
+
+```
+GET /user
+```
+
+```json
+{
+  "id": 1,
+  "username": "john_smith",
+  "email": "john@example.com",
+  "name": "John Smith",
+  "state": "active",
+  "avatar_url": "http://localhost:3000/uploads/user/avatar/1/index.jpg",
+  "web_url": "http://localhost:3000/john_smith",
+  "created_at": "2012-05-23T08:00:58Z",
+  "is_admin": false,
+  "bio": null,
+  "location": null,
+  "skype": "",
+  "linkedin": "",
+  "twitter": "",
+  "website_url": "",
+  "organization": "",
+  "last_sign_in_at": "2012-06-01T11:41:01Z",
+  "confirmed_at": "2012-05-23T09:05:22Z",
+  "theme_id": 1,
+  "color_scheme_id": 2,
+  "projects_limit": 100,
+  "current_sign_in_at": "2012-06-02T06:36:55Z",
+  "identities": [
+    {"provider": "github", "extern_uid": "2435223452345"},
+    {"provider": "bitbucket", "extern_uid": "john_smith"},
+    {"provider": "google_oauth2", "extern_uid": "8776128412476123468721346"}
+  ],
+  "can_create_group": true,
+  "can_create_project": true,
+  "two_factor_enabled": true,
+  "external": false,
+  "private_token": "dd34asd13as"
+}
+```
+
 ## List SSH keys
 
 Get a list of currently authenticated user's SSH keys.

features/steps/shared/authentication.rb

@@ -33,6 +33,6 @@ module SharedAuthentication
   end
 
   def current_user
-    @user || User.first
+    @user || User.reorder(nil).first
   end
 end

lib/api/entities.rb

@@ -22,7 +22,7 @@ module API
       expose :provider, :extern_uid
     end
 
-    class UserFull < User
+    class UserPublic < User
       expose :last_sign_in_at
       expose :confirmed_at
       expose :email
@@ -34,7 +34,7 @@ module API
       expose :external
     end
 
-    class UserLogin < UserFull
+    class UserWithPrivateToken < UserPublic
       expose :private_token
     end
 
@@ -283,7 +283,7 @@ module API
     end
 
     class SSHKeyWithUser < SSHKey
-      expose :user, using: Entities::UserFull
+      expose :user, using: Entities::UserPublic
     end
 
     class Note < Grape::Entity

lib/api/helpers.rb

@@ -7,59 +7,23 @@ module API
     SUDO_HEADER = "HTTP_SUDO"
     SUDO_PARAM = :sudo
 
-    def private_token
+    def declared_params(options = {})
-      params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]
+      options = { include_parent_namespaces: false }.merge(options)
-    end
+      declared(params, options).to_h.symbolize_keys
-
-    def warden
-      env['warden']
-    end
-
-    # Check the Rails session for valid authentication details
-    #
-    # Until CSRF protection is added to the API, disallow this method for
-    # state-changing endpoints
-    def find_user_from_warden
-      warden.try(:authenticate) if %w[GET HEAD].include?(env['REQUEST_METHOD'])
-    end
-
-    def find_user_by_private_token
-      token = private_token
-      return nil unless token.present?
-
-      User.find_by_authentication_token(token) || User.find_by_personal_access_token(token)
     end
 
     def current_user
-      @current_user ||= find_user_by_private_token
+      return @current_user if defined?(@current_user)
-      @current_user ||= doorkeeper_guard
-      @current_user ||= find_user_from_warden
 
-      unless @current_user && Gitlab::UserAccess.new(@current_user).allowed?
+      @current_user = initial_current_user
-        return nil
-      end
 
-      identifier = sudo_identifier()
+      sudo!
-
-      # If the sudo is the current user do nothing
-      if identifier && !(@current_user.id == identifier || @current_user.username == identifier)
-        forbidden!('Must be admin to use sudo') unless @current_user.is_admin?
-        @current_user = User.by_username_or_id(identifier)
-        not_found!("No user id or username for: #{identifier}") if @current_user.nil?
-      end
 
       @current_user
     end
 
-    def sudo_identifier
+    def sudo?
-      identifier ||= params[SUDO_PARAM] || env[SUDO_HEADER]
+      initial_current_user != current_user
-
-      # Regex for integers
-      if !!(identifier =~ /\A[0-9]+\z/)
-        identifier.to_i
-      else
-        identifier
-      end
     end
 
     def user_project
@@ -70,6 +34,14 @@ module API
       @available_labels ||= LabelsFinder.new(current_user, project_id: user_project.id).execute
     end
 
+    def find_user(id)
+      if id =~ /^\d+$/
+        User.find_by(id: id)
+      else
+        User.find_by(username: id)
+      end
+    end
+
     def find_project(id)
       project = Project.find_with_namespace(id) || Project.find_by(id: id)
 
@@ -122,9 +94,7 @@ module API
     end
 
     def find_project_issue(id)
-      issue = user_project.issues.find(id)
+      IssuesFinder.new(current_user, project_id: user_project.id).find(id)
-      not_found! unless can?(current_user, :read_issue, issue)
-      issue
     end
 
     def paginate(relation)
@@ -192,20 +162,6 @@ module API
       ActionController::Parameters.new(attrs).permit!
     end
 
-    # Helper method for validating all labels against its names
-    def validate_label_params(params)
-      errors = {}
-
-      params[:labels].to_s.split(',').each do |label_name|
-        label = available_labels.find_or_initialize_by(title: label_name.strip)
-        next if label.valid?
-
-        errors[label.title] = label.errors
-      end
-
-      errors
-    end
-
     # Checks the occurrences of datetime attributes, each attribute if present in the params hash must be in ISO 8601
     # format (YYYY-MM-DDTHH:MM:SSZ) or a Bad Request error is invoked.
     #
@@ -394,6 +350,69 @@ module API
 
     private
 
+    def private_token
+      params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]
+    end
+
+    def warden
+      env['warden']
+    end
+
+    # Check the Rails session for valid authentication details
+    #
+    # Until CSRF protection is added to the API, disallow this method for
+    # state-changing endpoints
+    def find_user_from_warden
+      warden.try(:authenticate) if %w[GET HEAD].include?(env['REQUEST_METHOD'])
+    end
+
+    def find_user_by_private_token
+      token = private_token
+      return nil unless token.present?
+
+      User.find_by_authentication_token(token) || User.find_by_personal_access_token(token)
+    end
+
+    def initial_current_user
+      return @initial_current_user if defined?(@initial_current_user)
+
+      @initial_current_user ||= find_user_by_private_token
+      @initial_current_user ||= doorkeeper_guard
+      @initial_current_user ||= find_user_from_warden
+
+      unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed?
+        @initial_current_user = nil
+      end
+
+      @initial_current_user
+    end
+
+    def sudo!
+      return unless sudo_identifier
+      return unless initial_current_user
+
+      unless initial_current_user.is_admin?
+        forbidden!('Must be admin to use sudo')
+      end
+
+      # Only private tokens should be used for the SUDO feature
+      unless private_token == initial_current_user.private_token
+        forbidden!('Private token must be specified in order to use sudo')
+      end
+
+      sudoed_user = find_user(sudo_identifier)
+
+      if sudoed_user
+        @current_user = sudoed_user
+      else
+        not_found!("No user id or username for: #{sudo_identifier}")
+      end
+    end
+
+    def sudo_identifier
+      @sudo_identifier ||= params[SUDO_PARAM] || env[SUDO_HEADER]
+    end
+
     def add_pagination_headers(paginated_data)
       header 'X-Total',       paginated_data.total_count.to_s
       header 'X-Total-Pages', paginated_data.total_pages.to_s

lib/api/issues.rb

@@ -19,6 +19,15 @@ module API
       def filter_issues_milestone(issues, milestone)
         issues.includes(:milestone).where('milestones.title' => milestone)
       end
+
+      def issue_params
+        new_params = declared(params, include_parent_namespace: false, include_missing: false).to_h
+        new_params = new_params.with_indifferent_access
+        new_params.delete(:id)
+        new_params.delete(:issue_id)
+
+        new_params
+      end
     end
 
     resource :issues do
@@ -86,6 +95,10 @@ module API
       end
     end
 
+    params do
+      requires :id, type: String, desc: 'The ID of a project'
+    end
+
     resource :projects do
       # Get a list of project issues
       #
@@ -109,7 +122,7 @@ module API
       #   GET /projects/:id/issues?milestone=1.0.0&state=closed
       #   GET /issues?iid=42
       get ":id/issues" do
-        issues = user_project.issues.inc_notes_with_associations.visible_to_user(current_user)
+        issues = IssuesFinder.new(current_user, project_id: user_project.id).execute.inc_notes_with_associations
         issues = filter_issues_state(issues, params[:state]) unless params[:state].nil?
         issues = filter_issues_labels(issues, params[:labels]) unless params[:labels].nil?
         issues = filter_by_iid(issues, params[:iid]) unless params[:iid].nil?
@@ -152,17 +165,10 @@ module API
       post ':id/issues' do
         required_attributes! [:title]
 
-        keys = [:title, :description, :assignee_id, :milestone_id, :due_date, :confidential]
+        keys = [:title, :description, :assignee_id, :milestone_id, :due_date, :confidential, :labels]
         keys << :created_at if current_user.admin? || user_project.owner == current_user
         attrs = attributes_for_keys(keys)
 
-        # Validate label names in advance
-        if (errors = validate_label_params(params)).any?
-          render_api_error!({ labels: errors }, 400)
-        end
-
-        attrs[:labels] = params[:labels] if params[:labels]
-
         # Convert and filter out invalid confidential flags
         attrs['confidential'] = to_boolean(attrs['confidential'])
         attrs.delete('confidential') if attrs['confidential'].nil?
@@ -180,41 +186,35 @@ module API
         end
       end
 
-      # Update an existing issue
+      desc 'Update an existing issue' do
-      #
+        success Entities::Issue
-      # Parameters:
+      end
-      #   id (required) - The ID of a project
+      params do
-      #   issue_id (required) - The ID of a project issue
+        requires :id, type: String, desc: 'The ID of a project'
-      #   title (optional) - The title of an issue
+        requires :issue_id, type: Integer, desc: "The ID of a project issue"
-      #   description (optional) - The description of an issue
+        optional :title, type: String, desc: 'The new title of the issue'
-      #   assignee_id (optional) - The ID of a user to assign issue
+        optional :description, type: String, desc: 'The description of an issue'
-      #   milestone_id (optional) - The ID of a milestone to assign issue
+        optional :assignee_id, type: Integer, desc: 'The ID of a user to assign issue'
-      #   labels (optional) - The labels of an issue
+        optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign issue'
-      #   state_event (optional) - The state event of an issue (close|reopen)
+        optional :labels, type: String, desc: 'The labels of an issue'
-      #   updated_at (optional) - Date time string, ISO 8601 formatted
+        optional :state_event, type: String, values: ['close', 'reopen'], desc: 'The state event of an issue'
-      #   due_date (optional)     - Date time string in the format YEAR-MONTH-DAY
+        # TODO 9.0, use the Grape DateTime type here
-      #   confidential (optional) - Boolean parameter if the issue should be confidential
+        optional :updated_at, type: String, desc: 'Date time string, ISO 8601 formatted'
-      # Example Request:
+        optional :due_date, type: String, desc: 'Date time string in the format YEAR-MONTH-DAY'
-      #   PUT /projects/:id/issues/:issue_id
+        # TODO 9.0, use the Grape boolean type here
+        optional :confidential, type: String, desc: 'Boolean parameter if the issue should be confidential'
+      end
       put ':id/issues/:issue_id' do
         issue = user_project.issues.find(params[:issue_id])
         authorize! :update_issue, issue
-        keys = [:title, :description, :assignee_id, :milestone_id, :state_event, :due_date, :confidential]
-        keys << :updated_at if current_user.admin? || user_project.owner == current_user
-        attrs = attributes_for_keys(keys)
-
-        # Validate label names in advance
-        if (errors = validate_label_params(params)).any?
-          render_api_error!({ labels: errors }, 400)
-        end
-
-        attrs[:labels] = params[:labels] if params[:labels]
 
         # Convert and filter out invalid confidential flags
-        attrs['confidential'] = to_boolean(attrs['confidential'])
+        params[:confidential] = to_boolean(params[:confidential])
-        attrs.delete('confidential') if attrs['confidential'].nil?
+        params.delete(:confidential) if params[:confidential].nil?
 
-        issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue)
+        params.delete(:updated_at) unless current_user.admin? || user_project.owner == current_user
+
+        issue = ::Issues::UpdateService.new(user_project, current_user, issue_params).execute(issue)
 
         if issue.valid?
           present issue, with: Entities::Issue, current_user: current_user

lib/api/merge_requests.rb

@@ -79,12 +79,7 @@ module API
       post ":id/merge_requests" do
         authorize! :create_merge_request, user_project
         required_attributes! [:source_branch, :target_branch, :title]
-        attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id, :description, :milestone_id]
+        attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id, :description, :milestone_id, :labels]
-
-        # Validate label names in advance
-        if (errors = validate_label_params(params)).any?
-          render_api_error!({ labels: errors }, 400)
-        end
 
         attrs[:labels] = params[:labels] if params[:labels]
 
@@ -112,6 +107,9 @@ module API
       # Routing "merge_request/:merge_request_id/..." is DEPRECATED and WILL BE REMOVED in version 9.0
       # Use "merge_requests/:merge_request_id/..." instead.
       #
+      params do
+        requires :id, type: String, desc: 'The ID of a project'
+      end
       [":id/merge_request/:merge_request_id", ":id/merge_requests/:merge_request_id"].each do |path|
         # Show MR
         #
@@ -162,23 +160,20 @@ module API
           present merge_request, with: Entities::MergeRequestChanges, current_user: current_user
         end
 
-        # Update MR
+        desc 'Update a merge request' do
-        #
+          success Entities::MergeRequest
-        # Parameters:
+        end
-        #   id (required)               - The ID of a project
+        params do
-        #   merge_request_id (required) - ID of MR
+          requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
-        #   target_branch               - The target branch
+          optional :target_branch, type: String, desc: 'The new target branch'
-        #   assignee_id                 - Assignee user ID
+          optional :assignee_id, type: Integer, desc: 'The assignees user ID'
-        #   title                       - Title of MR
+          optional :title, type: String, desc: 'The new title for the merge request'
-        #   state_event                 - Status of MR. (close|reopen|merge)
+          optional :state_event, type: String, values: ['close', 'reopen', 'merge'], desc: 'The state of the merge request'
-        #   description                 - Description of MR
+          optional :description, type: String, desc: 'The description, with markdown support'
-        #   labels (optional)           - Labels for a MR as a comma-separated list
+          optional :labels, type: String, desc: 'Labels for a MR as a comma-separated list'
-        #   milestone_id (optional)     - Milestone ID
+          optional :milestone_id, type: Integer, desc: 'The ID of the new milestone'
-        # Example:
+        end
-        #   PUT /projects/:id/merge_requests/:merge_request_id
-        #
         put path do
-          attrs = attributes_for_keys [:target_branch, :assignee_id, :title, :state_event, :description, :milestone_id]
           merge_request = user_project.merge_requests.find(params[:merge_request_id])
           authorize! :update_merge_request, merge_request
 
@@ -187,14 +182,10 @@ module API
             render_api_error!('Source branch cannot be changed', 400)
           end
 
-          # Validate label names in advance
+          mr_params = declared(params, include_missing: false, include_parent_namespace: false).with_indifferent_access
-          if (errors = validate_label_params(params)).any?
+          mr_params.delete(:merge_request_id)
-            render_api_error!({ labels: errors }, 400)
-          end
 
-          attrs[:labels] = params[:labels] if params[:labels]
+          merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, mr_params).execute(merge_request)
-
-          merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, attrs).execute(merge_request)
 
           if merge_request.valid?
             present merge_request, with: Entities::MergeRequest, current_user: current_user

lib/api/session.rb

@@ -15,7 +15,7 @@ module API
 
       return unauthorized! unless user
       return render_api_error!('401 Unauthorized. You have 2FA enabled. Please use a personal access token to access the API', 401) if user.two_factor_enabled?
-      present user, with: Entities::UserLogin
+      present user, with: Entities::UserWithPrivateToken
     end
   end
 end

lib/api/users.rb

@@ -25,7 +25,7 @@ module API
         end
 
         if current_user.is_admin?
-          present @users, with: Entities::UserFull
+          present @users, with: Entities::UserPublic
         else
           present @users, with: Entities::UserBasic
         end
@@ -41,7 +41,7 @@ module API
         @user = User.find(params[:id])
 
         if current_user && current_user.is_admin?
-          present @user, with: Entities::UserFull
+          present @user, with: Entities::UserPublic
         elsif can?(current_user, :read_user, @user)
           present @user, with: Entities::User
         else
@@ -88,7 +88,7 @@ module API
         end
 
         if user.save
-          present user, with: Entities::UserFull
+          present user, with: Entities::UserPublic
         else
           conflict!('Email has already been taken') if User.
               where(email: user.email).
@@ -151,7 +151,7 @@ module API
         end
 
         if user.update_attributes(attrs)
-          present user, with: Entities::UserFull
+          present user, with: Entities::UserPublic
         else
           render_validation_error!(user)
         end
@@ -349,7 +349,7 @@ module API
       # Example Request:
       #   GET /user
       get do
-        present @current_user, with: Entities::UserFull
+        present current_user, with: sudo? ? Entities::UserWithPrivateToken : Entities::UserPublic
       end
 
       # Get currently authenticated user's keys

lib/gitlab/project_search_results.rb

@@ -69,7 +69,7 @@ module Gitlab
     end
 
     def notes
-      project.notes.user.search(query, as_user: @current_user).order('updated_at DESC')
+      @notes ||= NotesFinder.new(project, @current_user, search: query).execute.user.order('updated_at DESC')
     end
 
     def commits

lib/gitlab/search_results.rb

@@ -50,7 +50,7 @@ module Gitlab
     end
 
     def issues
-      issues = Issue.visible_to_user(current_user).where(project_id: project_ids_relation)
+      issues = IssuesFinder.new(current_user).execute.where(project_id: project_ids_relation)
 
       if query =~ /#(\d+)\z/
         issues = issues.where(iid: $1)
@@ -68,7 +68,7 @@ module Gitlab
     end
 
     def merge_requests
-      merge_requests = MergeRequest.in_projects(project_ids_relation)
+      merge_requests = MergeRequestsFinder.new(current_user).execute.in_projects(project_ids_relation)
       if query =~ /[#!](\d+)\z/
         merge_requests = merge_requests.where(iid: $1)
       else

lib/gitlab/sql/union.rb

@@ -22,9 +22,7 @@ module Gitlab
         # By using "unprepared_statements" we remove the usage of placeholders
         # (thus fixing this problem), at a slight performance cost.
         fragments = ActiveRecord::Base.connection.unprepared_statement do
-          @relations.map do |rel|
+          @relations.map { |rel| rel.reorder(nil).to_sql }.reject(&:blank?)
-            rel.reorder(nil).to_sql
-          end
         end
 
         fragments.join("\nUNION\n")

spec/controllers/projects/blob_controller_spec.rb

@@ -36,4 +36,53 @@ describe Projects::BlobController do
       end
     end
   end
+
+  describe 'PUT update' do
+    let(:default_params) do
+      {
+        namespace_id: project.namespace.to_param,
+        project_id: project.to_param,
+        id: 'master/CHANGELOG',
+        target_branch: 'master',
+        content: 'Added changes',
+        commit_message: 'Update CHANGELOG'
+      }
+    end
+
+    def blob_after_edit_path
+      namespace_project_blob_path(project.namespace, project, 'master/CHANGELOG')
+    end
+
+    it 'redirects to blob' do
+      put :update, default_params
+
+      expect(response).to redirect_to(blob_after_edit_path)
+    end
+
+    context '?from_merge_request_iid' do
+      let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+      let(:mr_params) { default_params.merge(from_merge_request_iid: merge_request.iid) }
+
+      it 'redirects to MR diff' do
+        put :update, mr_params
+
+        after_edit_path = diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
+        file_anchor = "#file-path-#{Digest::SHA1.hexdigest('CHANGELOG')}"
+        expect(response).to redirect_to(after_edit_path + file_anchor)
+      end
+
+      context "when user doesn't have access" do
+        before do
+          other_project = create(:empty_project)
+          merge_request.update!(source_project: other_project, target_project: other_project)
+        end
+
+        it "it redirect to blob" do
+          put :update, mr_params
+
+          expect(response).to redirect_to(blob_after_edit_path)
+        end
+      end
+    end
+  end
 end

spec/controllers/projects/branches_controller_spec.rb

@@ -88,6 +88,24 @@ describe Projects::BranchesController do
           branch_name: branch,
           issue_iid: issue.iid
       end
+
+      context 'without issue feature access' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+          project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE)
+          project.team.truncate
+        end
+
+        it "doesn't post a system note" do
+          expect(SystemNoteService).not_to receive(:new_issue_branch)
+
+          post :create,
+            namespace_id: project.namespace.to_param,
+            project_id: project.to_param,
+            branch_name: branch,
+            issue_iid: issue.iid
+        end
+      end
     end
   end
 

spec/controllers/projects/todo_controller_spec.rb

@@ -4,7 +4,7 @@ describe Projects::TodosController do
   include ApiHelpers
 
   let(:user)          { create(:user) }
-  let(:project)       { create(:project) }
+  let(:project)       { create(:empty_project) }
   let(:issue)         { create(:issue, project: project) }
   let(:merge_request) { create(:merge_request, source_project: project) }
 
@@ -42,7 +42,7 @@ describe Projects::TodosController do
         end
       end
 
-      context 'when not authorized' do
+      context 'when not authorized for project' do
         it 'does not create todo for issue that user has no access to' do
           sign_in(user)
           expect do
@@ -60,6 +60,19 @@ describe Projects::TodosController do
           expect(response).to have_http_status(302)
         end
       end
+
+      context 'when not authorized for issue' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+          project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE)
+          sign_in(user)
+        end
+
+        it "doesn't create todo" do
+          expect{ go }.not_to change { user.todos.count }
+          expect(response).to have_http_status(404)
+        end
+      end
     end
   end
 
@@ -97,7 +110,7 @@ describe Projects::TodosController do
         end
       end
 
-      context 'when not authorized' do
+      context 'when not authorized for project' do
         it 'does not create todo for merge request user has no access to' do
           sign_in(user)
           expect do
@@ -115,6 +128,19 @@ describe Projects::TodosController do
           expect(response).to have_http_status(302)
         end
       end
+
+      context 'when not authorized for merge_request' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+          project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
+          sign_in(user)
+        end
+
+        it "doesn't create todo" do
+          expect{ go }.not_to change { user.todos.count }
+          expect(response).to have_http_status(404)
+        end
+      end
     end
   end
 end

spec/controllers/search_controller_spec.rb

@@ -0,0 +1,61 @@
+require 'spec_helper'
+
+describe SearchController do
+  let(:user)    { create(:user) }
+  let(:project) { create(:empty_project, :public) }
+
+  before do
+    sign_in(user)
+  end
+
+  it 'finds issue comments' do
+    project = create(:empty_project, :public)
+    note = create(:note_on_issue, project: project)
+
+    get :show, project_id: project.id, scope: 'notes', search: note.note
+
+    expect(assigns[:search_objects].first).to eq note
+  end
+
+  context 'on restricted projects' do
+    context 'when signed out' do
+      before { sign_out(user) }
+
+      it "doesn't expose comments on issues" do
+        project = create(:empty_project, :public, issues_access_level: ProjectFeature::PRIVATE)
+        note = create(:note_on_issue, project: project)
+
+        get :show, project_id: project.id, scope: 'notes', search: note.note
+
+        expect(assigns[:search_objects].count).to eq(0)
+      end
+    end
+
+    it "doesn't expose comments on issues" do
+      project = create(:empty_project, :public, issues_access_level: ProjectFeature::PRIVATE)
+      note = create(:note_on_issue, project: project)
+
+      get :show, project_id: project.id, scope: 'notes', search: note.note
+
+      expect(assigns[:search_objects].count).to eq(0)
+    end
+
+    it "doesn't expose comments on merge_requests" do
+      project = create(:empty_project, :public, merge_requests_access_level: ProjectFeature::PRIVATE)
+      note = create(:note_on_merge_request, project: project)
+
+      get :show, project_id: project.id, scope: 'notes', search: note.note
+
+      expect(assigns[:search_objects].count).to eq(0)
+    end
+
+    it "doesn't expose comments on snippets" do
+      project = create(:empty_project, :public, snippets_access_level: ProjectFeature::PRIVATE)
+      note = create(:note_on_project_snippet, project: project)
+
+      get :show, project_id: project.id, scope: 'notes', search: note.note
+
+      expect(assigns[:search_objects].count).to eq(0)
+    end
+  end
+end

spec/factories/notes.rb

@@ -67,7 +67,7 @@ FactoryGirl.define do
     end
 
     trait :on_project_snippet do
-      noteable { create(:snippet, project: project) }
+      noteable { create(:project_snippet, project: project) }
     end
 
     trait :system do

spec/features/projects/blobs/edit_spec.rb

@@ -0,0 +1,45 @@
+require 'spec_helper'
+
+feature 'Editing file blob', feature: true, js: true do
+  include WaitForAjax
+
+  given(:user) { create(:user) }
+  given(:role) { :developer }
+  given(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') }
+  given(:project) { merge_request.target_project }
+
+  background do
+    login_as(user)
+    project.team << [user, role]
+  end
+
+  def edit_and_commit
+    wait_for_ajax
+    first('.file-actions').click_link 'Edit'
+    execute_script('ace.edit("editor").setValue("class NextFeature\nend\n")')
+    click_button 'Commit Changes'
+  end
+
+  context 'from MR diff' do
+    before do
+      visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
+      edit_and_commit
+    end
+
+    scenario 'returns me to the mr' do
+      expect(page).to have_content(merge_request.title)
+    end
+  end
+
+  context 'from blob file path' do
+    before do
+      visit namespace_project_blob_path(project.namespace, project, '/feature/files/ruby/feature.rb')
+      edit_and_commit
+    end
+
+    scenario 'updates content' do
+      expect(page).to have_content 'successfully committed'
+      expect(page).to have_content 'NextFeature'
+    end
+  end
+end

spec/finders/issues_finder_spec.rb

@@ -10,7 +10,13 @@ describe IssuesFinder do
   let(:issue1) { create(:issue, author: user, assignee: user, project: project1, milestone: milestone, title: 'gitlab') }
   let(:issue2) { create(:issue, author: user, assignee: user, project: project2, description: 'gitlab') }
   let(:issue3) { create(:issue, author: user2, assignee: user2, project: project2) }
+
+  describe '#execute' do
+    let(:closed_issue) { create(:issue, author: user2, assignee: user2, project: project2, state: 'closed') }
     let!(:label_link) { create(:label_link, label: label, target: issue2) }
+    let(:search_user) { user }
+    let(:params) { {} }
+    let(:issues) { IssuesFinder.new(search_user, params.reverse_merge(scope: scope, state: 'opened')).execute }
 
     before do
       project1.team << [user, :master]
@@ -22,11 +28,6 @@ describe IssuesFinder do
       issue3
     end
 
-  describe '#execute' do
-    let(:search_user) { user }
-    let(:params) { {} }
-    let(:issues) { IssuesFinder.new(search_user, params.merge(scope: scope, state: 'opened')).execute }
-
     context 'scope: all' do
       let(:scope) { 'all' }
 
@@ -143,6 +144,40 @@ describe IssuesFinder do
         end
       end
 
+      context 'filtering by state' do
+        context 'with opened' do
+          let(:params) { { state: 'opened' } }
+
+          it 'returns only opened issues' do
+            expect(issues).to contain_exactly(issue1, issue2, issue3)
+          end
+        end
+
+        context 'with closed' do
+          let(:params) { { state: 'closed' } }
+
+          it 'returns only closed issues' do
+            expect(issues).to contain_exactly(closed_issue)
+          end
+        end
+
+        context 'with all' do
+          let(:params) { { state: 'all' } }
+
+          it 'returns all issues' do
+            expect(issues).to contain_exactly(issue1, issue2, issue3, closed_issue)
+          end
+        end
+
+        context 'with invalid state' do
+          let(:params) { { state: 'invalid_state' } }
+
+          it 'returns all issues' do
+            expect(issues).to contain_exactly(issue1, issue2, issue3, closed_issue)
+          end
+        end
+      end
+
       context 'when the user is unauthorized' do
         let(:search_user) { nil }
 
@@ -158,6 +193,15 @@ describe IssuesFinder do
           expect(issues).to contain_exactly(issue2, issue3)
         end
       end
+
+      it 'finds issues user can access due to group' do
+        group = create(:group)
+        project = create(:empty_project, group: group)
+        issue = create(:issue, project: project)
+        group.add_user(user, :owner)
+
+        expect(issues).to include(issue)
+      end
     end
 
     context 'personal scope' do
@@ -175,5 +219,43 @@ describe IssuesFinder do
         end
       end
     end
+
+    context 'when project restricts issues' do
+      let(:scope) { nil }
+
+      it "doesn't return team-only issues to non team members" do
+        project = create(:empty_project, :public, issues_access_level: ProjectFeature::PRIVATE)
+        issue = create(:issue, project: project)
+
+        expect(issues).not_to include(issue)
+      end
+
+      it "doesn't return issues if feature disabled" do
+        [project1, project2].each do |project|
+          project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED)
+        end
+
+        expect(issues.count).to eq 0
+      end
+    end
+  end
+
+  describe '.not_restricted_by_confidentiality' do
+    let(:authorized_user) { create(:user) }
+    let(:project) { create(:empty_project, namespace: authorized_user.namespace) }
+    let!(:public_issue) { create(:issue, project: project) }
+    let!(:confidential_issue) { create(:issue, project: project, confidential: true) }
+
+    it 'returns non confidential issues for nil user' do
+      expect(IssuesFinder.send(:not_restricted_by_confidentiality, nil)).to include(public_issue)
+    end
+
+    it 'returns non confidential issues for user not authorized for the issues projects' do
+      expect(IssuesFinder.send(:not_restricted_by_confidentiality, user)).to include(public_issue)
+    end
+
+    it 'returns all issues for user authorized for the issues projects' do
+      expect(IssuesFinder.send(:not_restricted_by_confidentiality, authorized_user)).to include(public_issue, confidential_issue)
+    end
   end
 end

spec/finders/notes_finder_spec.rb

@@ -2,16 +2,84 @@ require 'spec_helper'
 
 describe NotesFinder do
   let(:user) { create :user }
-  let(:project) { create :project }
+  let(:project) { create(:empty_project) }
-  let(:note1) { create :note_on_commit, project: project }
-  let(:note2) { create :note_on_commit, project: project }
-  let(:commit) { note1.noteable }
 
   before do
     project.team << [user, :master]
   end
 
   describe '#execute' do
+    it 'finds notes on snippets when project is public and user isnt a member'
+
+    it 'finds notes on merge requests' do
+      create(:note_on_merge_request, project: project)
+
+      notes = described_class.new(project, user).execute
+
+      expect(notes.count).to eq(1)
+    end
+
+    it 'finds notes on snippets' do
+      create(:note_on_project_snippet, project: project)
+
+      notes = described_class.new(project, user).execute
+
+      expect(notes.count).to eq(1)
+    end
+
+    it "excludes notes on commits the author can't download" do
+      project = create(:project, :private)
+      note = create(:note_on_commit, project: project)
+      params = { target_type: 'commit', target_id: note.noteable.id }
+
+      notes = described_class.new(project, create(:user), params).execute
+
+      expect(notes.count).to eq(0)
+    end
+
+    it 'succeeds when no notes found' do
+      notes = described_class.new(project, create(:user)).execute
+
+      expect(notes.count).to eq(0)
+    end
+
+    context 'on restricted projects' do
+      let(:project) do
+        create(:empty_project, :public, issues_access_level: ProjectFeature::PRIVATE,
+                                        snippets_access_level: ProjectFeature::PRIVATE,
+                                        merge_requests_access_level: ProjectFeature::PRIVATE)
+      end
+
+      it 'publicly excludes notes on merge requests' do
+        create(:note_on_merge_request, project: project)
+
+        notes = described_class.new(project, create(:user)).execute
+
+        expect(notes.count).to eq(0)
+      end
+
+      it 'publicly excludes notes on issues' do
+        create(:note_on_issue, project: project)
+
+        notes = described_class.new(project, create(:user)).execute
+
+        expect(notes.count).to eq(0)
+      end
+
+      it 'publicly excludes notes on snippets' do
+        create(:note_on_project_snippet, project: project)
+
+        notes = described_class.new(project, create(:user)).execute
+
+        expect(notes.count).to eq(0)
+      end
+    end
+
+    context 'for target' do
+      let(:project) { create(:project) }
+      let(:note1) { create :note_on_commit, project: project }
+      let(:note2) { create :note_on_commit, project: project }
+      let(:commit) { note1.noteable }
       let(:params)  { { target_id: commit.id, target_type: 'commit', last_fetched_at: 1.hour.ago.to_i } }
 
       before do
@@ -20,18 +88,36 @@ describe NotesFinder do
       end
 
       it 'finds all notes' do
-      notes = NotesFinder.new.execute(project, user, params)
+        notes = described_class.new(project, user, params).execute
         expect(notes.size).to eq(2)
       end
 
+      it 'finds notes on merge requests' do
+        note = create(:note_on_merge_request, project: project)
+        params = { target_type: 'merge_request', target_id: note.noteable.id }
+
+        notes = described_class.new(project, user, params).execute
+
+        expect(notes).to include(note)
+      end
+
+      it 'finds notes on snippets' do
+        note = create(:note_on_project_snippet, project: project)
+        params = { target_type: 'snippet', target_id: note.noteable.id }
+
+        notes = described_class.new(project, user, params).execute
+
+        expect(notes.count).to eq(1)
+      end
+
       it 'raises an exception for an invalid target_type' do
         params.merge!(target_type: 'invalid')
-      expect { NotesFinder.new.execute(project, user, params) }.to raise_error('invalid target_type')
+        expect { described_class.new(project, user, params).execute }.to raise_error('invalid target_type')
       end
 
       it 'filters out old notes' do
         note2.update_attribute(:updated_at, 2.hours.ago)
-      notes = NotesFinder.new.execute(project, user, params)
+        notes = described_class.new(project, user, params).execute
         expect(notes).to eq([note1])
       end
 
@@ -42,19 +128,77 @@ describe NotesFinder do
         let(:params) { { target_id: confidential_issue.id, target_type: 'issue', last_fetched_at: 1.hour.ago.to_i } }
 
         it 'returns notes if user can see the issue' do
-        expect(NotesFinder.new.execute(project, user, params)).to eq([confidential_note])
+          expect(described_class.new(project, user, params).execute).to eq([confidential_note])
         end
 
         it 'raises an error if user can not see the issue' do
           user = create(:user)
-        expect { NotesFinder.new.execute(project, user, params) }.to raise_error(ActiveRecord::RecordNotFound)
+          expect { described_class.new(project, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound)
         end
 
         it 'raises an error for project members with guest role' do
           user = create(:user)
           project.team << [user, :guest]
 
-        expect { NotesFinder.new.execute(project, user, params) }.to raise_error(ActiveRecord::RecordNotFound)
+          expect { described_class.new(project, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound)
+        end
+      end
+    end
+  end
+
+  describe '.search' do
+    let(:project) { create(:empty_project, :public) }
+    let(:note) { create(:note_on_issue, note: 'WoW', project: project) }
+
+    it 'returns notes with matching content' do
+      expect(described_class.new(note.project, nil, search: note.note).execute).to eq([note])
+    end
+
+    it 'returns notes with matching content regardless of the casing' do
+      expect(described_class.new(note.project, nil, search: 'WOW').execute).to eq([note])
+    end
+
+    it 'returns commit notes user can access' do
+      note = create(:note_on_commit, project: project)
+
+      expect(described_class.new(note.project, create(:user), search: note.note).execute).to eq([note])
+    end
+
+    context "confidential issues" do
+      let(:user) { create(:user) }
+      let(:confidential_issue) { create(:issue, :confidential, project: project, author: user) }
+      let(:confidential_note) { create(:note, note: "Random", noteable: confidential_issue, project: confidential_issue.project) }
+
+      it "returns notes with matching content if user can see the issue" do
+        expect(described_class.new(confidential_note.project, user, search: confidential_note.note).execute).to eq([confidential_note])
+      end
+
+      it "does not return notes with matching content if user can not see the issue" do
+        user = create(:user)
+        expect(described_class.new(confidential_note.project, user, search: confidential_note.note).execute).to be_empty
+      end
+
+      it "does not return notes with matching content for project members with guest role" do
+        user = create(:user)
+        project.team << [user, :guest]
+        expect(described_class.new(confidential_note.project, user, search: confidential_note.note).execute).to be_empty
+      end
+
+      it "does not return notes with matching content for unauthenticated users" do
+        expect(described_class.new(confidential_note.project, nil, search: confidential_note.note).execute).to be_empty
+      end
+    end
+
+    context 'inlines SQL filters on subqueries for performance' do
+      let(:sql) { described_class.new(note.project, nil, search: note.note).execute.to_sql }
+      let(:number_of_noteable_types) { 4 }
+
+      specify 'project_id check' do
+        expect(sql.scan(/project_id/).count).to be >= (number_of_noteable_types + 2)
+      end
+
+      specify 'search filter' do
+        expect(sql.scan(/LIKE/).count).to be >= number_of_noteable_types
       end
     end
   end

spec/fixtures/api/schemas/user/login.json

@@ -0,0 +1,37 @@
+{
+  "type": "object",
+  "required": [
+    "id",
+    "username",
+    "email",
+    "name",
+    "state",
+    "avatar_url",
+    "web_url",
+    "created_at",
+    "is_admin",
+    "bio",
+    "location",
+    "skype",
+    "linkedin",
+    "twitter",
+    "website_url",
+    "organization",
+    "last_sign_in_at",
+    "confirmed_at",
+    "theme_id",
+    "color_scheme_id",
+    "projects_limit",
+    "current_sign_in_at",
+    "identities",
+    "can_create_group",
+    "can_create_project",
+    "two_factor_enabled",
+    "external",
+    "private_token"
+  ],
+  "properties": {
+    "$ref": "full.json",
+    "private_token": { "type": "string" }
+  }
+}

spec/fixtures/api/schemas/user/public.json

@@ -0,0 +1,79 @@
+{
+  "type": "object",
+  "required": [
+    "id",
+    "username",
+    "email",
+    "name",
+    "state",
+    "avatar_url",
+    "web_url",
+    "created_at",
+    "is_admin",
+    "bio",
+    "location",
+    "skype",
+    "linkedin",
+    "twitter",
+    "website_url",
+    "organization",
+    "last_sign_in_at",
+    "confirmed_at",
+    "theme_id",
+    "color_scheme_id",
+    "projects_limit",
+    "current_sign_in_at",
+    "identities",
+    "can_create_group",
+    "can_create_project",
+    "two_factor_enabled",
+    "external"
+  ],
+  "properties": {
+    "id": { "type": "integer" },
+    "username": { "type": "string" },
+    "email": { 
+      "type": "string",
+      "pattern": "^[^@]+@[^@]+$"
+    },
+    "name": { "type": "string" },
+    "state": { 
+      "type": "string",
+      "enum": ["active", "blocked"] 
+    },
+    "avatar_url": { "type": "string" },
+    "web_url": { "type": "string" },
+    "created_at": { "type": "date" },
+    "is_admin": { "type": "boolean" },
+    "bio": { "type": ["string", "null"] },
+    "location": { "type": ["string", "null"] },
+    "skype": { "type": "string" },
+    "linkedin": { "type": "string" },
+    "twitter": { "type": "string "},
+    "website_url": { "type": "string" },
+    "organization": { "type": ["string", "null"] },
+    "last_sign_in_at": { "type": "date" },
+    "confirmed_at": { "type": ["date", "null"] },
+    "theme_id": { "type": "integer" },
+    "color_scheme_id": { "type": "integer" },
+    "projects_limit": { "type": "integer" },
+    "current_sign_in_at": { "type": "date" },
+    "identities": { 
+      "type": "array",
+      "items": {
+        "type": "object",
+        "properties": {
+          "provider": { 
+            "type": "string",
+            "enum": ["github", "bitbucket", "google_oauth2"] 
+          },
+          "extern_uid": { "type": ["number", "string"] }
+        }
+      }
+    },
+    "can_create_group": { "type": "boolean" },
+    "can_create_project": { "type": "boolean" },
+    "two_factor_enabled": { "type": "boolean" },
+    "external": { "type": "boolean" } 
+  }
+}

spec/helpers/preferences_helper_spec.rb

@@ -85,4 +85,45 @@ describe PreferencesHelper do
         and_return(double('user', messages))
     end
   end
+
+  describe '#default_project_view' do
+    context 'user not signed in' do
+      before do
+        helper.instance_variable_set(:@project, project)
+        stub_user
+      end
+
+      context 'when repository is empty' do
+        let(:project) { create(:project_empty_repo, :public) }
+
+        it 'returns activity if user has repository access' do
+          allow(helper).to receive(:can?).with(nil, :download_code, project).and_return(true)
+
+          expect(helper.default_project_view).to eq('activity')
+        end
+
+        it 'returns activity if user does not have repository access' do
+          allow(helper).to receive(:can?).with(nil, :download_code, project).and_return(false)
+
+          expect(helper.default_project_view).to eq('activity')
+        end
+      end
+
+      context 'when repository is not empty' do
+        let(:project) { create(:project, :public) }
+
+        it 'returns readme if user has repository access' do
+          allow(helper).to receive(:can?).with(nil, :download_code, project).and_return(true)
+
+          expect(helper.default_project_view).to eq('readme')
+        end
+
+        it 'returns activity if user does not have repository access' do
+          allow(helper).to receive(:can?).with(nil, :download_code, project).and_return(false)
+
+          expect(helper.default_project_view).to eq('activity')
+        end
+      end
+    end
+  end
 end

spec/lib/gitlab/project_search_results_spec.rb

@@ -22,6 +22,14 @@ describe Gitlab::ProjectSearchResults, lib: true do
     it { expect(results.query).to eq('hello world') }
   end
 
+  it 'does not list issues on private projects' do
+    issue = create(:issue, project: project)
+
+    results = described_class.new(user, project, issue.title)
+
+    expect(results.objects('issues')).not_to include issue
+  end
+
   describe 'confidential issues' do
     let(:query) { 'issue' }
     let(:author) { create(:user) }
@@ -29,6 +37,7 @@ describe Gitlab::ProjectSearchResults, lib: true do
     let(:non_member) { create(:user) }
     let(:member) { create(:user) }
     let(:admin) { create(:admin) }
+    let(:project) { create(:empty_project, :internal) }
     let!(:issue) { create(:issue, project: project, title: 'Issue 1') }
     let!(:security_issue_1) { create(:issue, :confidential, project: project, title: 'Security issue 1', author: author) }
     let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project, assignee: assignee) }
@@ -97,4 +106,33 @@ describe Gitlab::ProjectSearchResults, lib: true do
       expect(results.issues_count).to eq 3
     end
   end
+
+  describe 'notes search' do
+    it 'lists notes' do
+      project = create(:empty_project, :public)
+      note = create(:note, project: project)
+
+      results = described_class.new(user, project, note.note)
+
+      expect(results.objects('notes')).to include note
+    end
+
+    it "doesn't list issue notes when access is restricted" do
+      project = create(:empty_project, :public, issues_access_level: ProjectFeature::PRIVATE)
+      note = create(:note_on_issue, project: project)
+
+      results = described_class.new(user, project, note.note)
+
+      expect(results.objects('notes')).not_to include note
+    end
+
+    it "doesn't list merge_request notes when access is restricted" do
+      project = create(:empty_project, :public, merge_requests_access_level: ProjectFeature::PRIVATE)
+      note = create(:note_on_merge_request, project: project)
+
+      results = described_class.new(user, project, note.note)
+
+      expect(results.objects('notes')).not_to include note
+    end
+  end
 end

spec/lib/gitlab/search_results_spec.rb

@@ -12,6 +12,11 @@ describe Gitlab::SearchResults do
   let!(:milestone) { create(:milestone, project: project, title: 'foo') }
   let(:results) { described_class.new(user, Project.all, 'foo') }
 
+  context 'as a user with access' do
+    before do
+      project.team << [user, :developer]
+    end
+
     describe '#projects_count' do
       it 'returns the total amount of projects' do
         expect(results.projects_count).to eq(1)
@@ -36,11 +41,28 @@ describe Gitlab::SearchResults do
       end
     end
 
+    it 'includes merge requests from source and target projects' do
+      forked_project = create(:empty_project, forked_from_project: project)
+      merge_request_2 = create(:merge_request, target_project: project, source_project: forked_project, title: 'foo')
+
+      results = described_class.new(user, Project.where(id: forked_project.id), 'foo')
+
+      expect(results.objects('merge_requests')).to include merge_request_2
+    end
+  end
+
+  it 'does not list issues on private projects' do
+    private_project = create(:empty_project, :private)
+    issue = create(:issue, project: private_project, title: 'foo')
+
+    expect(results.objects('issues')).not_to include issue
+  end
+
   describe 'confidential issues' do
-    let(:project_1) { create(:empty_project) }
+    let(:project_1) { create(:empty_project, :internal) }
-    let(:project_2) { create(:empty_project) }
+    let(:project_2) { create(:empty_project, :internal) }
-    let(:project_3) { create(:empty_project) }
+    let(:project_3) { create(:empty_project, :internal) }
-    let(:project_4) { create(:empty_project) }
+    let(:project_4) { create(:empty_project, :internal) }
     let(:query) { 'issue' }
     let(:limit_projects) { Project.where(id: [project_1.id, project_2.id, project_3.id]) }
     let(:author) { create(:user) }
@@ -139,4 +161,11 @@ describe Gitlab::SearchResults do
       expect(results.issues_count).to eq 5
     end
   end
+
+  it 'does not list merge requests on projects with limited access' do
+    project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+    project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
+
+    expect(results.objects('merge_requests')).not_to include merge_request
+  end
 end

spec/lib/gitlab/sql/union_spec.rb

@@ -1,16 +1,26 @@
 require 'spec_helper'
 
 describe Gitlab::SQL::Union, lib: true do
+  let(:relation_1) { User.where(email: 'alice@example.com').select(:id) }
+  let(:relation_2) { User.where(email: 'bob@example.com').select(:id) }
+
+  def to_sql(relation)
+    relation.reorder(nil).to_sql
+  end
+
   describe '#to_sql' do
     it 'returns a String joining relations together using a UNION' do
-      rel1  = User.where(email: 'alice@example.com')
+      union = described_class.new([relation_1, relation_2])
-      rel2  = User.where(email: 'bob@example.com')
-      union = described_class.new([rel1, rel2])
 
-      sql1 = rel1.reorder(nil).to_sql
+      expect(union.to_sql).to eq("#{to_sql(relation_1)}\nUNION\n#{to_sql(relation_2)}")
-      sql2 = rel2.reorder(nil).to_sql
+    end
 
-      expect(union.to_sql).to eq("#{sql1}\nUNION\n#{sql2}")
+    it 'skips Model.none segements' do
+      empty_relation = User.none
+      union = described_class.new([empty_relation, relation_1, relation_2])
+
+      expect{User.where("users.id IN (#{union.to_sql})").to_a}.not_to raise_error
+      expect(union.to_sql).to eq("#{to_sql(relation_1)}\nUNION\n#{to_sql(relation_2)}")
     end
   end
 end

spec/models/commit_range_spec.rb

@@ -137,26 +137,25 @@ describe CommitRange, models: true do
   end
 
   describe '#has_been_reverted?' do
-    it 'returns true if the commit has been reverted' do
+    let(:issue) { create(:issue) }
-      issue = create(:issue)
+    let(:user) { issue.author }
 
+    it 'returns true if the commit has been reverted' do
       create(:note_on_issue,
              noteable: issue,
              system: true,
-             note: commit1.revert_description,
+             note: commit1.revert_description(user),
              project: issue.project)
 
       expect_any_instance_of(Commit).to receive(:reverts_commit?).
-        with(commit1).
+        with(commit1, user).
         and_return(true)
 
-      expect(commit1.has_been_reverted?(nil, issue)).to eq(true)
+      expect(commit1.has_been_reverted?(user, issue)).to eq(true)
     end
 
     it 'returns false a commit has not been reverted' do
-      issue = create(:issue)
+      expect(commit1.has_been_reverted?(user, issue)).to eq(false)
-
-      expect(commit1.has_been_reverted?(nil, issue)).to eq(false)
     end
   end
 end

spec/models/commit_spec.rb

@@ -173,25 +173,26 @@ eos
 
   describe '#reverts_commit?' do
     let(:another_commit) { double(:commit, revert_description: "This reverts commit #{commit.sha}") }
+    let(:user) { commit.author }
 
-    it { expect(commit.reverts_commit?(another_commit)).to be_falsy }
+    it { expect(commit.reverts_commit?(another_commit, user)).to be_falsy }
 
     context 'commit has no description' do
       before { allow(commit).to receive(:description?).and_return(false) }
 
-      it { expect(commit.reverts_commit?(another_commit)).to be_falsy }
+      it { expect(commit.reverts_commit?(another_commit, user)).to be_falsy }
     end
 
     context "another_commit's description does not revert commit" do
       before { allow(commit).to receive(:description).and_return("Foo Bar") }
 
-      it { expect(commit.reverts_commit?(another_commit)).to be_falsy }
+      it { expect(commit.reverts_commit?(another_commit, user)).to be_falsy }
     end
 
     context "another_commit's description reverts commit" do
       before { allow(commit).to receive(:description).and_return("Foo #{another_commit.revert_description} Bar") }
 
-      it { expect(commit.reverts_commit?(another_commit)).to be_truthy }
+      it { expect(commit.reverts_commit?(another_commit, user)).to be_truthy }
     end
 
     context "another_commit's description reverts merged merge request" do
@@ -201,7 +202,7 @@ eos
         allow(commit).to receive(:description).and_return("Foo #{another_commit.revert_description} Bar")
       end
 
-      it { expect(commit.reverts_commit?(another_commit)).to be_truthy }
+      it { expect(commit.reverts_commit?(another_commit, user)).to be_truthy }
     end
   end
 

spec/models/cycle_analytics/code_spec.rb

@@ -6,7 +6,7 @@ describe 'CycleAnalytics#code', feature: true do
   let(:project) { create(:project) }
   let(:from_date) { 10.days.ago }
   let(:user) { create(:user, :admin) }
-  subject { CycleAnalytics.new(project, from: from_date) }
+  subject { CycleAnalytics.new(project, user, from: from_date) }
 
   context 'with deployment' do
     generate_cycle_analytics_spec(

spec/models/cycle_analytics/issue_spec.rb

@@ -6,7 +6,7 @@ describe 'CycleAnalytics#issue', models: true do
   let(:project) { create(:project) }
   let(:from_date) { 10.days.ago }
   let(:user) { create(:user, :admin) }
-  subject { CycleAnalytics.new(project, from: from_date) }
+  subject { CycleAnalytics.new(project, user, from: from_date) }
 
   generate_cycle_analytics_spec(
     phase: :issue,

spec/models/cycle_analytics/plan_spec.rb

@@ -6,7 +6,7 @@ describe 'CycleAnalytics#plan', feature: true do
   let(:project) { create(:project) }
   let(:from_date) { 10.days.ago }
   let(:user) { create(:user, :admin) }
-  subject { CycleAnalytics.new(project, from: from_date) }
+  subject { CycleAnalytics.new(project, user, from: from_date) }
 
   generate_cycle_analytics_spec(
     phase: :plan,

spec/models/cycle_analytics/production_spec.rb

@@ -6,7 +6,7 @@ describe 'CycleAnalytics#production', feature: true do
   let(:project) { create(:project) }
   let(:from_date) { 10.days.ago }
   let(:user) { create(:user, :admin) }
-  subject { CycleAnalytics.new(project, from: from_date) }
+  subject { CycleAnalytics.new(project, user, from: from_date) }
 
   generate_cycle_analytics_spec(
     phase: :production,

spec/models/cycle_analytics/review_spec.rb

@@ -6,7 +6,7 @@ describe 'CycleAnalytics#review', feature: true do
   let(:project) { create(:project) }
   let(:from_date) { 10.days.ago }
   let(:user) { create(:user, :admin) }
-  subject { CycleAnalytics.new(project, from: from_date) }
+  subject { CycleAnalytics.new(project, user, from: from_date) }
 
   generate_cycle_analytics_spec(
     phase: :review,

spec/models/cycle_analytics/staging_spec.rb

@@ -6,7 +6,7 @@ describe 'CycleAnalytics#staging', feature: true do
   let(:project) { create(:project) }
   let(:from_date) { 10.days.ago }
   let(:user) { create(:user, :admin) }
-  subject { CycleAnalytics.new(project, from: from_date) }
+  subject { CycleAnalytics.new(project, user, from: from_date) }
 
   generate_cycle_analytics_spec(
     phase: :staging,

spec/models/cycle_analytics/summary_spec.rb

@@ -4,7 +4,7 @@ describe CycleAnalytics::Summary, models: true do
   let(:project) { create(:project) }
   let(:from) { Time.now }
   let(:user) { create(:user, :admin) }
-  subject { described_class.new(project, from: from) }
+  subject { described_class.new(project, user, from: from) }
 
   describe "#new_issues" do
     it "finds the number of issues created after the 'from date'" do

spec/models/cycle_analytics/test_spec.rb

@@ -6,7 +6,7 @@ describe 'CycleAnalytics#test', feature: true do
   let(:project) { create(:project) }
   let(:from_date) { 10.days.ago }
   let(:user) { create(:user, :admin) }
-  subject { CycleAnalytics.new(project, from: from_date) }
+  subject { CycleAnalytics.new(project, user, from: from_date) }
 
   generate_cycle_analytics_spec(
     phase: :test,

spec/models/issue_spec.rb

@@ -22,26 +22,6 @@ describe Issue, models: true do
     it { is_expected.to have_db_index(:deleted_at) }
   end
 
-  describe '.visible_to_user' do
-    let(:user) { create(:user) }
-    let(:authorized_user) { create(:user) }
-    let(:project) { create(:project, namespace: authorized_user.namespace) }
-    let!(:public_issue) { create(:issue, project: project) }
-    let!(:confidential_issue) { create(:issue, project: project, confidential: true) }
-
-    it 'returns non confidential issues for nil user' do
-      expect(Issue.visible_to_user(nil).count).to be(1)
-    end
-
-    it 'returns non confidential issues for user not authorized for the issues projects' do
-      expect(Issue.visible_to_user(user).count).to be(1)
-    end
-
-    it 'returns all issues for user authorized for the issues projects' do
-      expect(Issue.visible_to_user(authorized_user).count).to be(2)
-    end
-  end
-
   describe '#to_reference' do
     it 'returns a String reference to the object' do
       expect(subject.to_reference).to eq "##{subject.iid}"