From e5370ba226b185a53b179cd964740062fa23fca6 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Tue, 23 Feb 2016 19:05:15 -0300
Subject: [PATCH 01/30] Add a confidential flag to issues

---
 db/migrate/20160223192159_add_confidential_to_issues.rb | 6 ++++++
 db/schema.rb                                            | 2 ++
 2 files changed, 8 insertions(+)
 create mode 100644 db/migrate/20160223192159_add_confidential_to_issues.rb

diff --git a/db/migrate/20160223192159_add_confidential_to_issues.rb b/db/migrate/20160223192159_add_confidential_to_issues.rb
new file mode 100644
index 00000000000000..e9d47fd589aff4
--- /dev/null
+++ b/db/migrate/20160223192159_add_confidential_to_issues.rb
@@ -0,0 +1,6 @@
+class AddConfidentialToIssues < ActiveRecord::Migration
+  def change
+    add_column :issues, :confidential, :boolean, default: false
+    add_index :issues, :confidential
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 6baa49e538ce32..cfc31bf383bb80 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -486,10 +486,12 @@
     t.integer  "iid"
     t.integer  "updated_by_id"
     t.integer  "weight"
+    t.boolean  "confidential",              default: false
   end
 
   add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
   add_index "issues", ["author_id"], name: "index_issues_on_author_id", using: :btree
+  add_index "issues", ["confidential"], name: "index_issues_on_confidential", using: :btree
   add_index "issues", ["created_at", "id"], name: "index_issues_on_created_at_and_id", using: :btree
   add_index "issues", ["created_at"], name: "index_issues_on_created_at", using: :btree
   add_index "issues", ["description"], name: "index_issues_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
-- 
GitLab


From fe88ad87d4700179ca343ec17925b0ec254f93ad Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Tue, 23 Feb 2016 19:28:37 -0300
Subject: [PATCH 02/30] Add an option to user make an issue confidential

---
 app/controllers/projects/issues_controller.rb | 2 +-
 app/views/shared/issuable/_form.html.haml     | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index e884a286c4358a..cb0815a6e30d51 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -163,7 +163,7 @@ def redirect_old
 
   def issue_params
     params.require(:issue).permit(
-      :title, :assignee_id, :position, :description, :weight,
+      :title, :assignee_id, :position, :description, :confidential, :weight,
       :milestone_id, :state_event, :task_num, label_ids: []
     )
   end
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 476d0131747dd1..51f4ab5a188462 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -29,6 +29,13 @@
       = render 'projects/notes/hints'
       .clearfix
       .error-alert
+- if issuable.is_a?(Issue)
+  .form-group
+    .col-sm-offset-2.col-sm-10
+      .checkbox
+        = f.label :confidential do
+          = f.check_box :confidential
+          Make this issue confidential
 - if can?(current_user, :"admin_#{issuable.to_ability_name}", issuable.project)
   %hr
   .form-group
-- 
GitLab


From dc4c876960c3665aded3783eb54f0928b4bb395c Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 25 Feb 2016 18:05:44 -0300
Subject: [PATCH 03/30] Restrict access to confidential issues

---
 app/controllers/projects/issues_controller.rb |   6 +-
 app/finders/issuable_finder.rb                |  21 +++
 app/models/ability.rb                         |  22 +++
 .../projects/issues_controller_spec.rb        | 146 +++++++++++++++++-
 spec/factories/issues.rb                      |   4 +
 5 files changed, 190 insertions(+), 9 deletions(-)

diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index cb0815a6e30d51..bf621a1c89af24 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -5,7 +5,7 @@ class Projects::IssuesController < Projects::ApplicationController
   before_action :issue, only: [:edit, :update, :show]
 
   # Allow read any issue
-  before_action :authorize_read_issue!
+  before_action :authorize_read_issue!, only: [:show]
 
   # Allow write(create) issue
   before_action :authorize_create_issue!, only: [:new, :create]
@@ -133,6 +133,10 @@ def issue
   end
   alias_method :subscribable_resource, :issue
 
+  def authorize_read_issue!
+    return render_404 unless can?(current_user, :read_issue, @issue)
+  end
+
   def authorize_update_issue!
     return render_404 unless can?(current_user, :update_issue, @issue)
   end
diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index f1fb01769d21c9..36f909065b9ab2 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -40,6 +40,7 @@ def execute
     items = by_author(items)
     items = by_label(items)
     items = by_weight(items)
+    items = by_confidentiality(items)
     sort(items)
   end
 
@@ -308,6 +309,26 @@ def filter_by_any_weight?
     params[:weight] == Issue::WEIGHT_ANY
   end
 
+  def by_confidentiality(items)
+    return items unless klass == Issue
+
+    if current_user
+      if current_user.admin? || project.team.member?(current_user.id)
+        items
+      else
+        issuable_table = items.arel_table
+
+        items.where(
+          issuable_table[:confidential].eq(false).or(
+            issuable_table[:confidential].eq(true).and(issuable_table[:author_id].eq(current_user.id))
+          )
+        )
+      end
+    else
+      items.not_confidential
+    end
+  end
+
   def label_names
     params[:label_name].split(',')
   end
diff --git a/app/models/ability.rb b/app/models/ability.rb
index 1941229e973ca1..4a941d28a2cfba 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -43,6 +43,8 @@ def anonymous_abilities(user, subject)
         anonymous_personal_snippet_abilities(subject)
       when subject.is_a?(CommitStatus)
         anonymous_commit_status_abilities(subject)
+      when subject.is_a?(Issue)
+        anonymous_issue_abilities(subject)
       when subject.is_a?(Project) || subject.respond_to?(:project)
         anonymous_project_abilities(subject)
       when subject.is_a?(Group) || subject.respond_to?(:group)
@@ -52,6 +54,12 @@ def anonymous_abilities(user, subject)
       end
     end
 
+    def anonymous_issue_abilities(subject)
+      rules = anonymous_project_abilities(subject)
+      rules -= confidential_issue_rules if subject.confidential?
+      rules
+    end
+
     def anonymous_project_abilities(subject)
       project = if subject.is_a?(Project)
                   subject
@@ -343,6 +351,13 @@ def namespace_abilities(user, namespace)
         end
 
         rules += project_abilities(user, subject.project)
+
+        if subject.respond_to?(:confidential) && subject.confidential?
+          unless user.admin? || subject.author == user || subject.project.team.member?(user.id)
+            rules -= confidential_issue_rules
+          end
+        end
+
         rules
       end
     end
@@ -461,5 +476,12 @@ def named_abilities(name)
         :"admin_#{name}"
       ]
     end
+
+    def confidential_issue_rules
+      [
+        :read_issue,
+        :update_issue
+      ]
+    end
   end
 end
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 76d56bc989d975..cacd22609aa77f 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -1,16 +1,16 @@
 require('spec_helper')
 
 describe Projects::IssuesController do
-  let(:project) { create(:project) }
-  let(:user)    { create(:user) }
-  let(:issue) { create(:issue, project: project) }
+  describe "GET #index" do
+    let(:project) { create(:project) }
+    let(:user) { create(:user) }
+    let(:issue) { create(:issue, project: project) }
 
-  before do
-    sign_in(user)
-    project.team << [user, :developer]
-  end
+    before do
+      sign_in(user)
+      project.team << [user, :developer]
+    end
 
-  describe "GET #index" do
     it "returns index" do
       get :index, namespace_id: project.namespace.path, project_id: project.path
 
@@ -38,6 +38,136 @@
       get :index, namespace_id: project.namespace.path, project_id: project.path
       expect(response.status).to eq(404)
     end
+  end
+
+  describe 'Confidential Issues' do
+    let(:project) { create(:empty_project, :public) }
+    let(:author) { create(:user) }
+    let(:non_member) { create(:user) }
+    let(:member) { create(:user) }
+    let(:admin) { create(:admin) }
+    let!(:issue) { create(:issue, project: project) }
+    let!(:unescaped_parameter_value) { create(:issue, :confidential, project: project, author: author) }
+    let!(:request_forgery_timing_attack) { create(:issue, :confidential, project: project) }
+
+    describe 'GET #index' do
+      it 'should not list confidential issues for guests' do
+        sign_out(:user)
+        get_issues
+
+        expect(assigns(:issues)).to eq [issue]
+      end
+
+      it 'should not list confidential issues for non project members' do
+        sign_in(non_member)
+        get_issues
+
+        expect(assigns(:issues)).to eq [issue]
+      end
+
+      it 'should list confidential issues for author' do
+        sign_in(author)
+        get_issues
+
+        expect(assigns(:issues)).to include unescaped_parameter_value
+        expect(assigns(:issues)).not_to include request_forgery_timing_attack
+      end
+
+      it 'should list confidential issues for project members' do
+        sign_in(member)
+        project.team << [member, :developer]
+
+        get_issues
+
+        expect(assigns(:issues)).to include unescaped_parameter_value
+        expect(assigns(:issues)).to include request_forgery_timing_attack
+      end
+
+      it 'should list confidential issues for admin' do
+        sign_in(admin)
+        get_issues
+
+        expect(assigns(:issues)).to include unescaped_parameter_value
+        expect(assigns(:issues)).to include request_forgery_timing_attack
+      end
+
+      def get_issues
+        get :index,
+          namespace_id: project.namespace.to_param,
+          project_id: project.to_param
+      end
+    end
 
+    shared_examples_for 'restricted action' do |http_status|
+      it 'returns 404 for guests' do
+        sign_out :user
+        go(id: unescaped_parameter_value.to_param)
+
+        expect(response).to have_http_status :not_found
+      end
+
+      it 'returns 404 for non project members' do
+        sign_in(non_member)
+        go(id: unescaped_parameter_value.to_param)
+
+        expect(response).to have_http_status :not_found
+      end
+
+      it "returns #{http_status[:success]} for author" do
+        sign_in(author)
+        go(id: unescaped_parameter_value.to_param)
+
+        expect(response).to have_http_status http_status[:success]
+      end
+
+      it "returns #{http_status[:success]} for project members" do
+        sign_in(member)
+        project.team << [member, :developer]
+        go(id: unescaped_parameter_value.to_param)
+
+        expect(response).to have_http_status http_status[:success]
+      end
+
+      it "returns #{http_status[:success]} for admin" do
+        sign_in(admin)
+        go(id: unescaped_parameter_value.to_param)
+
+        expect(response).to have_http_status http_status[:success]
+      end
+    end
+
+    describe 'GET #show' do
+      it_behaves_like 'restricted action', success: 200
+
+      def go(id:)
+        get :show,
+          namespace_id: project.namespace.to_param,
+          project_id: project.to_param,
+          id: id
+      end
+    end
+
+    describe 'GET #edit' do
+      it_behaves_like 'restricted action', success: 200
+
+      def go(id:)
+        get :edit,
+          namespace_id: project.namespace.to_param,
+          project_id: project.to_param,
+          id: id
+      end
+    end
+
+    describe 'PUT #update' do
+      it_behaves_like 'restricted action', success: 302
+
+      def go(id:)
+        put :update,
+          namespace_id: project.namespace.to_param,
+          project_id: project.to_param,
+          id: id,
+          issue: { title: 'New title' }
+      end
+    end
   end
 end
diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb
index 722095de5905ef..e72aa9479b7579 100644
--- a/spec/factories/issues.rb
+++ b/spec/factories/issues.rb
@@ -4,6 +4,10 @@
     author
     project
 
+    trait :confidential do
+      confidential true
+    end
+
     trait :closed do
       state :closed
     end
-- 
GitLab


From 4ded8adf7c5b8ef545e56f33de1e6740cddefb01 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 25 Feb 2016 19:06:47 -0300
Subject: [PATCH 04/30] Remove references for confidential issues

---
 lib/banzai/filter/issue_reference_filter.rb   |  9 +++++++
 .../lib/banzai/filter/redactor_filter_spec.rb | 25 ++++++++++++++++++-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/lib/banzai/filter/issue_reference_filter.rb b/lib/banzai/filter/issue_reference_filter.rb
index 9f08aa36e8b5bf..13395b934f2c0b 100644
--- a/lib/banzai/filter/issue_reference_filter.rb
+++ b/lib/banzai/filter/issue_reference_filter.rb
@@ -9,6 +9,15 @@ def self.object_class
         Issue
       end
 
+      def self.user_can_see_reference?(user, node, context)
+        if node.has_attribute?('data-issue')
+          issue = Issue.find(node.attr('data-issue')) rescue nil
+          issue && !issue.confidential?
+        else
+          super
+        end
+      end
+
       def find_object(project, id)
         project.get_issue(id)
       end
diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb
index e9bb388e361b44..2898334e501496 100644
--- a/spec/lib/banzai/filter/redactor_filter_spec.rb
+++ b/spec/lib/banzai/filter/redactor_filter_spec.rb
@@ -44,8 +44,31 @@ def reference_link(data)
     end
   end
 
-  context "for user references" do
+  context 'with data-issue' do
+    it 'removes references for confidential issues' do
+      user = create(:user)
+      project = create(:empty_project)
+      issue = create(:issue, :confidential, project: project)
+
+      link = reference_link(issue: issue.id, reference_filter: 'IssueReferenceFilter')
+      doc = filter(link, current_user: user)
+
+      expect(doc.css('a').length).to eq 0
+    end
+
+    it 'allows references for non confidential issues' do
+      user = create(:user)
+      project = create(:empty_project)
+      issue = create(:issue, project: project)
 
+      link = reference_link(issue: issue.id, reference_filter: 'IssueReferenceFilter')
+      doc = filter(link, current_user: user)
+
+      expect(doc.css('a').length).to eq 1
+    end
+  end
+
+  context "for user references" do
     context 'with data-group' do
       it 'removes unpermitted Group references' do
         user = create(:user)
-- 
GitLab


From 3075eedeeb3e68e0b8f377c8fe0be2cbc7e15e60 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 25 Feb 2016 19:29:41 -0300
Subject: [PATCH 05/30] Remove references for confidential issues from
 autocomplete

---
 app/models/issue.rb                              |  1 +
 app/services/projects/autocomplete_service.rb    |  2 +-
 .../projects/autocomplete_service_spec.rb        | 16 ++++++++++++++++
 3 files changed, 18 insertions(+), 1 deletion(-)
 create mode 100644 spec/services/projects/autocomplete_service_spec.rb

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 89b4c827e44c64..6c60153ee024fb 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -45,6 +45,7 @@ class Issue < ActiveRecord::Base
   scope :cared, ->(user) { where(assignee_id: user) }
   scope :open_for, ->(user) { opened.assigned_to(user) }
   scope :in_projects, ->(project_ids) { where(project_id: project_ids) }
+  scope :not_confidential, -> { where(confidential: false) }
 
   state_machine :state, initial: :opened do
     event :close do
diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb
index 7408e09ed1e9df..74d26aaaaed37f 100644
--- a/app/services/projects/autocomplete_service.rb
+++ b/app/services/projects/autocomplete_service.rb
@@ -5,7 +5,7 @@ def initialize(project)
     end
 
     def issues
-      @project.issues.opened.select([:iid, :title])
+      @project.issues.not_confidential.opened.select([:iid, :title])
     end
 
     def merge_requests
diff --git a/spec/services/projects/autocomplete_service_spec.rb b/spec/services/projects/autocomplete_service_spec.rb
new file mode 100644
index 00000000000000..83e606e6d39417
--- /dev/null
+++ b/spec/services/projects/autocomplete_service_spec.rb
@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+describe Projects::AutocompleteService, services: true do
+  let(:project) { create(:empty_project, :public) }
+
+  subject(:autocomplete) { described_class.new(project) }
+
+  describe '#issues' do
+    it 'should not list confidential issues' do
+      issue = create(:issue, project: project)
+      create(:issue, :confidential, project: project)
+
+      expect(autocomplete.issues.map(&:iid)).to eq [issue.iid]
+    end
+  end
+end
-- 
GitLab


From eb9165e6b0d68e4f7297172a7deae01164fae868 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 25 Feb 2016 19:49:15 -0300
Subject: [PATCH 06/30] Add a warning icon to confidential issues on list page
 and issue details

---
 app/views/projects/issues/_issue.html.haml | 4 ++++
 app/views/projects/issues/show.html.haml   | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml
index 0daf492b652c08..c062f6d648d483 100644
--- a/app/views/projects/issues/_issue.html.haml
+++ b/app/views/projects/issues/_issue.html.haml
@@ -11,6 +11,10 @@
         %li
           CLOSED
 
+      - if issue.confidential?
+        %li
+          = icon('lock')
+
       - if issue.assignee
         %li
           = link_to_member(@project, issue.assignee, name: false, title: "Assigned to :name")
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index 0242276cd84176..375adfba15e56e 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -22,6 +22,8 @@
       = icon('angle-double-left')
 
     .issue-meta
+      - if @issue.confidential
+        = icon('lock')
       %strong.identifier
         Issue ##{@issue.iid}
       %span.creator
-- 
GitLab


From 19ec6e58c57c27412942f28faf3dcd0e113f576e Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 25 Feb 2016 21:46:56 -0300
Subject: [PATCH 07/30] Restrict access to confidential issues through API

---
 lib/api/issues.rb                | 16 ++++++
 spec/requests/api/issues_spec.rb | 95 +++++++++++++++++++++++++++++++-
 2 files changed, 109 insertions(+), 2 deletions(-)

diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index 252744515da880..f82b1c75d0719d 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -22,6 +22,20 @@ def filter_issues_milestone(issues, milestone)
         issues.includes(:milestone).where('milestones.title' => milestone)
       end
 
+      def filter_issues_confidentiality(issues)
+        if current_user.admin? || user_project.team.member?(current_user.id)
+          issues
+        else
+          issuable_table = issues.arel_table
+
+          issues.where(
+            issuable_table[:confidential].eq(false).or(
+              issuable_table[:author_id].eq(current_user.id).and(issuable_table[:confidential].eq(true))
+            )
+          )
+        end
+      end
+
       def create_spam_log(project, current_user, attrs)
         params = attrs.merge({
           source_ip: env['REMOTE_ADDR'],
@@ -86,6 +100,7 @@ def create_spam_log(project, current_user, attrs)
         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?
+        issues = filter_issues_confidentiality(issues)
 
         unless params[:milestone].nil?
           issues = filter_issues_milestone(issues, params[:milestone])
@@ -104,6 +119,7 @@ def create_spam_log(project, current_user, attrs)
       #   GET /projects/:id/issues/:issue_id
       get ":id/issues/:issue_id" do
         @issue = user_project.issues.find(params[:issue_id])
+        not_found! unless can?(current_user, :read_issue, @issue)
         present @issue, with: Entities::Issue
       end
 
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 571ea2dae4c752..8abf74d3ee3cd7 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -3,7 +3,10 @@
 describe API::API, api: true  do
   include ApiHelpers
   let(:user) { create(:user) }
-  let!(:project) { create(:project, namespace: user.namespace ) }
+  let(:non_member) { create(:user) }
+  let(:author) { create(:author) }
+  let(:admin) { create(:admin) }
+  let!(:project) { create(:project, :public, namespace: user.namespace ) }
   let!(:closed_issue) do
     create :closed_issue,
            author: user,
@@ -12,6 +15,12 @@
            state: :closed,
            milestone: milestone
   end
+  let!(:confidential_issue) do
+    create :issue,
+           :confidential,
+           project: project,
+           author: author
+  end
   let!(:issue) do
     create :issue,
            author: user,
@@ -123,10 +132,35 @@
     let(:base_url) { "/projects/#{project.id}" }
     let(:title) { milestone.title }
 
-    it "should return project issues" do
+    it 'should return project issues without confidential issues for non project members' do
+      get api("#{base_url}/issues", non_member)
+      expect(response.status).to eq(200)
+      expect(json_response).to be_an Array
+      expect(json_response.length).to eq(2)
+      expect(json_response.first['title']).to eq(issue.title)
+    end
+
+    it 'should return project confidential issues for author' do
+      get api("#{base_url}/issues", author)
+      expect(response.status).to eq(200)
+      expect(json_response).to be_an Array
+      expect(json_response.length).to eq(3)
+      expect(json_response.first['title']).to eq(issue.title)
+    end
+
+    it 'should return project issues with confidential issues for project members' do
       get api("#{base_url}/issues", user)
       expect(response.status).to eq(200)
       expect(json_response).to be_an Array
+      expect(json_response.length).to eq(3)
+      expect(json_response.first['title']).to eq(issue.title)
+    end
+
+    it 'should return project confidential issues for admin' do
+      get api("#{base_url}/issues", admin)
+      expect(response.status).to eq(200)
+      expect(json_response).to be_an Array
+      expect(json_response.length).to eq(3)
       expect(json_response.first['title']).to eq(issue.title)
     end
 
@@ -206,6 +240,34 @@
       get api("/projects/#{project.id}/issues/54321", user)
       expect(response.status).to eq(404)
     end
+
+    context 'confidential issues' do
+      it "should return 404 for non project members" do
+        get api("/projects/#{project.id}/issues/#{confidential_issue.id}", non_member)
+        expect(response.status).to eq(404)
+      end
+
+      it "should return confidential issue for project members" do
+        get api("/projects/#{project.id}/issues/#{confidential_issue.id}", user)
+        expect(response.status).to eq(200)
+        expect(json_response['title']).to eq(confidential_issue.title)
+        expect(json_response['iid']).to eq(confidential_issue.iid)
+      end
+
+      it "should return confidential issue for author" do
+        get api("/projects/#{project.id}/issues/#{confidential_issue.id}", author)
+        expect(response.status).to eq(200)
+        expect(json_response['title']).to eq(confidential_issue.title)
+        expect(json_response['iid']).to eq(confidential_issue.iid)
+      end
+
+      it "should return confidential issue for admin" do
+        get api("/projects/#{project.id}/issues/#{confidential_issue.id}", admin)
+        expect(response.status).to eq(200)
+        expect(json_response['title']).to eq(confidential_issue.title)
+        expect(json_response['iid']).to eq(confidential_issue.iid)
+      end
+    end
   end
 
   describe "POST /projects/:id/issues" do
@@ -294,6 +356,35 @@
       expect(response.status).to eq(400)
       expect(json_response['message']['labels']['?']['title']).to eq(['is invalid'])
     end
+
+    context 'confidential issues' do
+      it "should return 403 for non project members" do
+        put api("/projects/#{project.id}/issues/#{confidential_issue.id}", non_member),
+          title: 'updated title'
+        expect(response.status).to eq(403)
+      end
+
+      it "should update a confidential issue for project members" do
+        put api("/projects/#{project.id}/issues/#{confidential_issue.id}", user),
+          title: 'updated title'
+        expect(response.status).to eq(200)
+        expect(json_response['title']).to eq('updated title')
+      end
+
+      it "should update a confidential issue for author" do
+        put api("/projects/#{project.id}/issues/#{confidential_issue.id}", author),
+          title: 'updated title'
+        expect(response.status).to eq(200)
+        expect(json_response['title']).to eq('updated title')
+      end
+
+      it "should update a confidential issue for admin" do
+        put api("/projects/#{project.id}/issues/#{confidential_issue.id}", admin),
+          title: 'updated title'
+        expect(response.status).to eq(200)
+        expect(json_response['title']).to eq('updated title')
+      end
+    end
   end
 
   describe 'PUT /projects/:id/issues/:issue_id to update labels' do
-- 
GitLab


From 740a54a6399ed041b8812b0281ee0fc96c9d18b9 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 29 Feb 2016 23:04:45 -0300
Subject: [PATCH 08/30] Restrict access to confidential issues on search
 results

---
 app/services/search/global_service.rb         |  2 +-
 app/services/search/project_service.rb        |  3 +-
 lib/gitlab/project_search_results.rb          | 25 +++++-
 lib/gitlab/search_results.rb                  | 20 ++++-
 .../lib/gitlab/project_search_results_spec.rb | 58 +++++++++++++-
 spec/lib/gitlab/search_results_spec.rb        | 77 ++++++++++++++++++-
 6 files changed, 177 insertions(+), 8 deletions(-)

diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb
index a515317bb14688..4fae2f5c3a8cc3 100644
--- a/app/services/search/global_service.rb
+++ b/app/services/search/global_service.rb
@@ -14,7 +14,7 @@ def execute
       if Gitlab.config.elasticsearch.enabled
         Gitlab::Elastic::SearchResults.new(projects.pluck(:id), params[:search])
       else
-        Gitlab::SearchResults.new(projects, params[:search])
+        Gitlab::SearchResults.new(current_user, projects, params[:search])
       end
     end
   end
diff --git a/app/services/search/project_service.rb b/app/services/search/project_service.rb
index 1ab5339597a63c..4d8db54ba05e7e 100644
--- a/app/services/search/project_service.rb
+++ b/app/services/search/project_service.rb
@@ -12,7 +12,8 @@ def execute
                                          params[:search],
                                          params[:repository_ref])
       else
-        Gitlab::ProjectSearchResults.new(project,
+        Gitlab::ProjectSearchResults.new(current_user,
+                                         project,
                                          params[:search],
                                          params[:repository_ref])
       end
diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb
index 0607a8b9592794..f261030469cabc 100644
--- a/lib/gitlab/project_search_results.rb
+++ b/lib/gitlab/project_search_results.rb
@@ -2,7 +2,8 @@ module Gitlab
   class ProjectSearchResults < SearchResults
     attr_reader :project, :repository_ref
 
-    def initialize(project, query, repository_ref = nil)
+    def initialize(user, project, query, repository_ref = nil)
+      @user = user
       @project = project
       @repository_ref = if repository_ref.present?
                           repository_ref
@@ -84,6 +85,28 @@ def commits
       end
     end
 
+    def issues
+      issues = Issue.where(project_id: project_ids_relation)
+
+      unless user.admin? || project.team.member?(user.id)
+        issuable_table = issues.arel_table
+
+        issues = issues.where(
+          issuable_table[:confidential].eq(false).or(
+            issuable_table[:confidential].eq(true).and(issuable_table[:author_id].eq(user.id))
+          )
+        )
+      end
+
+      if query =~ /#(\d+)\z/
+        issues = issues.where(iid: $1)
+      else
+        issues = issues.full_search(query)
+      end
+
+      issues.order('updated_at DESC')
+    end
+
     def project_ids_relation
       project
     end
diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb
index f13528a2eea549..43a490fc1b0568 100644
--- a/lib/gitlab/search_results.rb
+++ b/lib/gitlab/search_results.rb
@@ -1,12 +1,13 @@
 module Gitlab
   class SearchResults
-    attr_reader :query
+    attr_reader :user, :query
 
     # Limit search results by passed projects
     # It allows us to search only for projects user has access to
     attr_reader :limit_projects
 
-    def initialize(limit_projects, query)
+    def initialize(user, limit_projects, query)
+      @user = user
       @limit_projects = limit_projects || Project.all
       @query = Shellwords.shellescape(query) if query.present?
     end
@@ -60,6 +61,21 @@ def projects
     def issues
       issues = Issue.where(project_id: project_ids_relation)
 
+      unless user.admin?
+        issues_table = issues.arel_table
+        authorized_projects_ids = user.authorized_projects.pluck(:id)
+
+        issues = issues.where(
+          issues_table[:confidential].eq(false).or(
+            issues_table[:confidential].eq(true).and(
+              issues_table[:author_id].eq(user.id).or(
+                issues_table[:project_id].in(authorized_projects_ids)
+              )
+            )
+          )
+        )
+      end
+
       if query =~ /#(\d+)\z/
         issues = issues.where(iid: $1)
       else
diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb
index 09adbc07dcbbdb..a21083d88b07f2 100644
--- a/spec/lib/gitlab/project_search_results_spec.rb
+++ b/spec/lib/gitlab/project_search_results_spec.rb
@@ -1,11 +1,12 @@
 require 'spec_helper'
 
 describe Gitlab::ProjectSearchResults, lib: true do
+  let(:user) { create(:user) }
   let(:project) { create(:project) }
   let(:query) { 'hello world' }
 
   describe 'initialize with empty ref' do
-    let(:results) { Gitlab::ProjectSearchResults.new(project, query, '') }
+    let(:results) { Gitlab::ProjectSearchResults.new(user, project, query, '') }
 
     it { expect(results.project).to eq(project) }
     it { expect(results.repository_ref).to be_nil }
@@ -14,10 +15,63 @@
 
   describe 'initialize with ref' do
     let(:ref) { 'refs/heads/test' }
-    let(:results) { Gitlab::ProjectSearchResults.new(project, query, ref) }
+    let(:results) { Gitlab::ProjectSearchResults.new(user, project, query, ref) }
 
     it { expect(results.project).to eq(project) }
     it { expect(results.repository_ref).to eq(ref) }
     it { expect(results.query).to eq('hello world') }
   end
+
+  describe 'confidential issues' do
+    let(:query) { 'issue' }
+    let(:author) { create(:user) }
+    let(:non_member) { create(:user) }
+    let(:member) { create(:user) }
+    let(:admin) { create(:admin) }
+    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) }
+
+    it 'should not list project confidential issues for non project members' do
+      results = described_class.new(non_member, project, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).not_to include security_issue_1
+      expect(issues).not_to include security_issue_2
+      expect(results.issues_count).to eq 1
+    end
+
+    it 'should list project confidential issues for author' do
+      results = described_class.new(author, project, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).not_to include security_issue_2
+      expect(results.issues_count).to eq 2
+    end
+
+    it 'should list project confidential issues for project members' do
+      project.team << [member, :developer]
+
+      results = described_class.new(member, project, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(results.issues_count).to eq 3
+    end
+
+    it 'should list all project issues for admin' do
+      results = described_class.new(admin, project, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(results.issues_count).to eq 3
+    end
+  end
 end
diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb
index bb18f41785824b..945292117e1151 100644
--- a/spec/lib/gitlab/search_results_spec.rb
+++ b/spec/lib/gitlab/search_results_spec.rb
@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::SearchResults do
+  let(:user) { create(:user) }
   let!(:project) { create(:project, name: 'foo') }
   let!(:issue) { create(:issue, project: project, title: 'foo') }
 
@@ -9,7 +10,7 @@
   end
 
   let!(:milestone) { create(:milestone, project: project, title: 'foo') }
-  let(:results) { described_class.new(Project.all, 'foo') }
+  let(:results) { described_class.new(user, Project.all, 'foo') }
 
   describe '#total_count' do
     it 'returns the total amount of search hits' do
@@ -52,4 +53,78 @@
       expect(results.empty?).to eq(false)
     end
   end
+
+  describe 'confidential issues' do
+    let(:project_1) { create(:empty_project) }
+    let(:project_2) { create(:empty_project) }
+    let(:project_3) { create(:empty_project) }
+    let(:project_4) { create(:empty_project) }
+    let(:query) { 'issue' }
+    let(:limit_projects) { Project.where(id: [project_1.id, project_2.id, project_3.id]) }
+    let(:author) { create(:user) }
+    let(:non_member) { create(:user) }
+    let(:member) { create(:user) }
+    let(:admin) { create(:admin) }
+    let!(:issue) { create(:issue, project: project_1, title: 'Issue 1') }
+    let!(:security_issue_1) { create(:issue, :confidential, project: project_1, title: 'Security issue 1', author: author) }
+    let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project_1) }
+    let!(:security_issue_3) { create(:issue, :confidential, project: project_2, title: 'Security issue 3', author: author) }
+    let!(:security_issue_4) { create(:issue, :confidential, project: project_3, title: 'Security issue 4') }
+    let!(:security_issue_5) { create(:issue, :confidential, project: project_4, title: 'Security issue 5') }
+
+    it 'should not list confidential issues for non project members' do
+      results = described_class.new(non_member, limit_projects, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).not_to include security_issue_1
+      expect(issues).not_to include security_issue_2
+      expect(issues).not_to include security_issue_3
+      expect(issues).not_to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 1
+    end
+
+    it 'should list confidential issues for author' do
+      results = described_class.new(author, limit_projects, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).not_to include security_issue_2
+      expect(issues).to include security_issue_3
+      expect(issues).not_to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 3
+    end
+
+    it 'should list confidential issues for project members' do
+      project_1.team << [member, :developer]
+      project_2.team << [member, :developer]
+
+      results = described_class.new(member, limit_projects, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(issues).to include security_issue_3
+      expect(issues).not_to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 4
+    end
+
+    it 'should list all issues for admin' do
+      results = described_class.new(admin, limit_projects, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(issues).to include security_issue_3
+      expect(issues).to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 5
+    end
+  end
 end
-- 
GitLab


From d461d50f27bf387fc13f6b5e60f8df85ace12115 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Tue, 1 Mar 2016 02:32:27 -0300
Subject: [PATCH 09/30] Add a lock icon to confidential issues on search
 results

---
 app/views/search/results/_issue.html.haml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/views/search/results/_issue.html.haml b/app/views/search/results/_issue.html.haml
index 45d700781f3e3d..849e1ef8940f64 100644
--- a/app/views/search/results/_issue.html.haml
+++ b/app/views/search/results/_issue.html.haml
@@ -1,5 +1,7 @@
 .search-result-row
   %h4
+    - if issue.confidential?
+      = icon('lock')
     = link_to [issue.project.namespace.becomes(Namespace), issue.project, issue] do
       %span.term.str-truncated= issue.title
     .pull-right ##{issue.iid}
-- 
GitLab


From 3d244b4c8b9ae7d885294023f3745f7446af2e76 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Tue, 1 Mar 2016 03:29:08 -0300
Subject: [PATCH 10/30] Restrict access to confidential issues on Elasticsearch
 results

---
 app/services/search/global_service.rb         |  2 +-
 app/services/search/project_service.rb        |  7 +-
 lib/elastic/issues_search.rb                  | 32 ++++++-
 lib/gitlab/elastic/project_search_results.rb  | 22 ++++-
 lib/gitlab/elastic/search_results.rb          | 12 ++-
 .../elastic/project_search_results_spec.rb    | 81 +++++++++++++---
 .../lib/gitlab/elastic/search_results_spec.rb | 93 +++++++++++++++++--
 7 files changed, 222 insertions(+), 27 deletions(-)

diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb
index 4fae2f5c3a8cc3..bc4478b5a44d91 100644
--- a/app/services/search/global_service.rb
+++ b/app/services/search/global_service.rb
@@ -12,7 +12,7 @@ def execute
       projects = projects.in_namespace(group.id) if group
 
       if Gitlab.config.elasticsearch.enabled
-        Gitlab::Elastic::SearchResults.new(projects.pluck(:id), params[:search])
+        Gitlab::Elastic::SearchResults.new(current_user, projects.pluck(:id), params[:search])
       else
         Gitlab::SearchResults.new(current_user, projects, params[:search])
       end
diff --git a/app/services/search/project_service.rb b/app/services/search/project_service.rb
index 4d8db54ba05e7e..0531c046ba0aca 100644
--- a/app/services/search/project_service.rb
+++ b/app/services/search/project_service.rb
@@ -8,9 +8,10 @@ def initialize(project, user, params)
 
     def execute
       if Gitlab.config.elasticsearch.enabled
-        Gitlab::Elastic::ProjectSearchResults.new(project.id,
-                                         params[:search],
-                                         params[:repository_ref])
+        Gitlab::Elastic::ProjectSearchResults.new(current_user,
+                                                  project.id,
+                                                  params[:search],
+                                                  params[:repository_ref])
       else
         Gitlab::ProjectSearchResults.new(current_user,
                                          project,
diff --git a/lib/elastic/issues_search.rb b/lib/elastic/issues_search.rb
index 90593d4dd91d4a..e5d5c3aa9945e8 100644
--- a/lib/elastic/issues_search.rb
+++ b/lib/elastic/issues_search.rb
@@ -23,6 +23,8 @@ module IssuesSearch
         indexes :project,     type: :nested
         indexes :author,      type: :nested
 
+        indexes :confidential, type: :boolean
+
         indexes :updated_at_sort, type: :date,   index: :not_analyzed
       end
 
@@ -48,10 +50,38 @@ def self.elastic_search(query, options: {})
           query_hash = basic_query_hash(%w(title^2 description), query)
         end
 
-        query_hash = project_ids_filter(query_hash, options[:project_ids])
+        query_hash = project_ids_filter(query_hash, options[:projects_ids])
+        query_hash = confidentiality_filter(query_hash, options[:user_id], options[:authorized_projects_ids])
 
         self.__elasticsearch__.search(query_hash)
       end
+
+      def self.confidentiality_filter(query_hash, user_id, projects_ids)
+        if user_id
+          query_hash[:query][:filtered][:filter] = {
+            bool: {
+              should: [
+                { term: { confidential: false } },
+                { bool: {
+                    must: [
+                      { term: { confidential: true } },
+                      { bool: {
+                          should: [
+                            { term: { author_id: user_id } },
+                            { terms: { project_id: projects_ids } }
+                          ]
+                        }
+                      }
+                    ]
+                  }
+                }
+              ]
+            }
+          }
+        end
+
+        query_hash
+      end
     end
   end
 end
diff --git a/lib/gitlab/elastic/project_search_results.rb b/lib/gitlab/elastic/project_search_results.rb
index 55803e04fe4187..be3e6f009b48e0 100644
--- a/lib/gitlab/elastic/project_search_results.rb
+++ b/lib/gitlab/elastic/project_search_results.rb
@@ -3,7 +3,8 @@ module Elastic
     class ProjectSearchResults < SearchResults
       attr_reader :project, :repository_ref
 
-      def initialize(project_id, query, repository_ref = nil)
+      def initialize(user, project_id, query, repository_ref = nil)
+        @user = user
         @project = Project.find(project_id)
 
         @repository_ref = if repository_ref.present?
@@ -106,6 +107,25 @@ def commits
         end
       end
 
+      def issues
+        opt = {
+          projects_ids: limit_project_ids
+        }
+
+        unless user.admin? || project.team.member?(user.id)
+          opt.merge!(
+            authorized_projects_ids: user.authorized_projects.pluck(:id),
+            user_id: user.id
+          )
+        end
+
+        if query =~ /#(\d+)\z/
+          Issue.in_projects(limit_project_ids).where(iid: $1)
+        else
+          Issue.elastic_search(query, options: opt).records
+        end
+      end
+
       def limit_project_ids
         [project.id]
       end
diff --git a/lib/gitlab/elastic/search_results.rb b/lib/gitlab/elastic/search_results.rb
index ef295d04e17edd..a208dc4aab1a6a 100644
--- a/lib/gitlab/elastic/search_results.rb
+++ b/lib/gitlab/elastic/search_results.rb
@@ -1,13 +1,14 @@
 module Gitlab
   module Elastic
     class SearchResults
-      attr_reader :query
+      attr_reader :user, :query
 
       # Limit search results by passed project ids
       # It allows us to search only for projects user has access to
       attr_reader :limit_project_ids
 
-      def initialize(limit_project_ids, query)
+      def initialize(user, limit_project_ids, query)
+        @user = user
         @limit_project_ids = limit_project_ids || Project.all
         @query = Shellwords.shellescape(query) if query.present?
       end
@@ -66,6 +67,13 @@ def issues
           project_ids: limit_project_ids
         }
 
+        unless user.admin?
+          opt.merge!(
+            authorized_projects_ids: user.authorized_projects.pluck(:id),
+            user_id: user.id
+          )
+        end
+
         Issue.elastic_search(query, options: opt)
       end
 
diff --git a/spec/lib/gitlab/elastic/project_search_results_spec.rb b/spec/lib/gitlab/elastic/project_search_results_spec.rb
index 32c8d38941c0a9..ca59d844bc674c 100644
--- a/spec/lib/gitlab/elastic/project_search_results_spec.rb
+++ b/spec/lib/gitlab/elastic/project_search_results_spec.rb
@@ -1,33 +1,35 @@
 require 'spec_helper'
 
 describe Gitlab::Elastic::ProjectSearchResults, lib: true do
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:query) { 'hello world' }
+
   before do
     allow(Gitlab.config.elasticsearch).to receive(:enabled).and_return(true)
     Project.__elasticsearch__.create_index!
-
-    @project = create(:project)
+    Issue.__elasticsearch__.create_index!
   end
 
   after do
     allow(Gitlab.config.elasticsearch).to receive(:enabled).and_return(false)
     Project.__elasticsearch__.delete_index!
+    Issue.__elasticsearch__.delete_index!
   end
 
-  let(:query) { 'hello world' }
-
   describe 'initialize with empty ref' do
-    let(:results) { Gitlab::Elastic::ProjectSearchResults.new(@project.id, query, '') }
+    subject(:results) { described_class.new(user, project.id, query, '') }
 
-    it { expect(results.project).to eq(@project) }
+    it { expect(results.project).to eq(project) }
     it { expect(results.repository_ref).to be_nil }
     it { expect(results.query).to eq('hello world') }
   end
 
   describe 'initialize with ref' do
     let(:ref) { 'refs/heads/test' }
-    let(:results) { Gitlab::Elastic::ProjectSearchResults.new(@project.id, query, ref) }
+    subject(:results) { described_class.new(user, project.id, query, ref) }
 
-    it { expect(results.project).to eq(@project) }
+    it { expect(results.project).to eq(project) }
     it { expect(results.repository_ref).to eq(ref) }
     it { expect(results.query).to eq('hello world') }
   end
@@ -39,7 +41,7 @@
 
       project.repository.index_blobs
       project.repository.index_commits
-      
+
       # Notes
       create :note, note: 'bla-bla term', project: project
       # The note in the project you have no access to
@@ -53,13 +55,70 @@
 
       Project.__elasticsearch__.refresh_index!
 
-      result = Gitlab::Elastic::ProjectSearchResults.new(project.id, "term")
+      result = Gitlab::Elastic::ProjectSearchResults.new(user, project.id, "term")
       expect(result.notes_count).to eq(1)
       expect(result.wiki_blobs_count).to eq(1)
       expect(result.blobs_count).to eq(1)
 
-      result1 = Gitlab::Elastic::ProjectSearchResults.new(project.id, "initial")
+      result1 = Gitlab::Elastic::ProjectSearchResults.new(user, project.id, "initial")
       expect(result1.commits_count).to eq(1)
     end
   end
+
+  describe 'confidential issues' do
+    let(:query) { 'issue' }
+    let(:author) { create(:user) }
+    let(:non_member) { create(:user) }
+    let(:member) { create(:user) }
+    let(:admin) { create(:admin) }
+    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) }
+
+    before do
+      Issue.__elasticsearch__.refresh_index!
+    end
+
+    it 'should not list project confidential issues for non project members' do
+      results = described_class.new(non_member, project.id, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).not_to include security_issue_1
+      expect(issues).not_to include security_issue_2
+      expect(results.issues_count).to eq 1
+    end
+
+    it 'should list project confidential issues for author' do
+      results = described_class.new(author, project.id, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).not_to include security_issue_2
+      expect(results.issues_count).to eq 2
+    end
+
+    it 'should list project confidential issues for project members' do
+      project.team << [member, :developer]
+
+      results = described_class.new(member, project.id, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(results.issues_count).to eq 3
+    end
+
+    it 'should list all project issues for admin' do
+      results = described_class.new(admin, project.id, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(results.issues_count).to eq 3
+    end
+  end
 end
diff --git a/spec/lib/gitlab/elastic/search_results_spec.rb b/spec/lib/gitlab/elastic/search_results_spec.rb
index 89b8af6206321b..dde527db7054c7 100644
--- a/spec/lib/gitlab/elastic/search_results_spec.rb
+++ b/spec/lib/gitlab/elastic/search_results_spec.rb
@@ -9,6 +9,7 @@
     allow(Gitlab.config.elasticsearch).to receive(:enabled).and_return(false)
   end
 
+  let(:user) { create(:user) }
   let(:project_1) { create(:project) }
   let(:project_2) { create(:project) }
   let(:limit_project_ids) { [project_1.id] }
@@ -45,7 +46,7 @@
     end
 
     it 'should list issues that title or description contain the query' do
-      results = described_class.new(limit_project_ids, 'hello world')
+      results = described_class.new(user, limit_project_ids, 'hello world')
       issues = results.objects('issues')
 
       expect(issues).to include @issue_1
@@ -55,14 +56,14 @@
     end
 
     it 'should return empty list when issues title or description does not contain the query' do
-      results = described_class.new(limit_project_ids, 'security')
+      results = described_class.new(user, limit_project_ids, 'security')
 
       expect(results.objects('issues')).to be_empty
       expect(results.issues_count).to eq 0
     end
 
     it 'should list issue when search by a valid iid' do
-      results = described_class.new(limit_project_ids, '#2')
+      results = described_class.new(user, limit_project_ids, '#2')
       issues = results.objects('issues')
 
       expect(issues).not_to include @issue_1
@@ -72,13 +73,89 @@
     end
 
     it 'should return empty list when search by invalid iid' do
-      results = described_class.new(limit_project_ids, '#222')
+      results = described_class.new(user, limit_project_ids, '#222')
 
       expect(results.objects('issues')).to be_empty
       expect(results.issues_count).to eq 0
     end
   end
 
+  describe 'confidential issues' do
+    let(:project_3) { create(:empty_project) }
+    let(:project_4) { create(:empty_project) }
+    let(:query) { 'issue' }
+    let(:limit_project_ids) { [project_1.id, project_2.id, project_3.id] }
+    let(:author) { create(:user) }
+    let(:non_member) { create(:user) }
+    let(:member) { create(:user) }
+    let(:admin) { create(:admin) }
+    let!(:issue) { create(:issue, project: project_1, title: 'Issue 1') }
+    let!(:security_issue_1) { create(:issue, :confidential, project: project_1, title: 'Security issue 1', author: author) }
+    let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project_1) }
+    let!(:security_issue_3) { create(:issue, :confidential, project: project_2, title: 'Security issue 3', author: author) }
+    let!(:security_issue_4) { create(:issue, :confidential, project: project_3, title: 'Security issue 4') }
+    let!(:security_issue_5) { create(:issue, :confidential, project: project_4, title: 'Security issue 5') }
+
+    before do
+      Issue.__elasticsearch__.refresh_index!
+    end
+
+    it 'should not list confidential issues for non project members' do
+      results = described_class.new(non_member, limit_project_ids, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).not_to include security_issue_1
+      expect(issues).not_to include security_issue_2
+      expect(issues).not_to include security_issue_3
+      expect(issues).not_to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 1
+    end
+
+    it 'should list confidential issues for author' do
+      results = described_class.new(author, limit_project_ids, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).not_to include security_issue_2
+      expect(issues).to include security_issue_3
+      expect(issues).not_to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 3
+    end
+
+    it 'should list confidential issues for project members' do
+      project_1.team << [member, :developer]
+      project_2.team << [member, :developer]
+
+      results = described_class.new(member, limit_project_ids, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(issues).to include security_issue_3
+      expect(issues).not_to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 4
+    end
+
+    it 'should list all issues for admin' do
+      results = described_class.new(admin, limit_project_ids, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(issues).to include security_issue_3
+      expect(issues).to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 5
+    end
+  end
+
   describe 'merge requests' do
     before do
       MergeRequest.__elasticsearch__.create_index!
@@ -115,7 +192,7 @@
     end
 
     it 'should list merge requests that title or description contain the query' do
-      results = described_class.new(limit_project_ids, 'hello world')
+      results = described_class.new(user, limit_project_ids, 'hello world')
       merge_requests = results.objects('merge_requests')
 
       expect(merge_requests).to include @merge_request_1
@@ -125,14 +202,14 @@
     end
 
     it 'should return empty list when merge requests title or description does not contain the query' do
-      results = described_class.new(limit_project_ids, 'security')
+      results = described_class.new(user, limit_project_ids, 'security')
 
       expect(results.objects('merge_requests')).to be_empty
       expect(results.merge_requests_count).to eq 0
     end
 
     it 'should list merge request when search by a valid iid' do
-      results = described_class.new(limit_project_ids, '#2')
+      results = described_class.new(user, limit_project_ids, '#2')
       merge_requests = results.objects('merge_requests')
 
       expect(merge_requests).not_to include @merge_request_1
@@ -142,7 +219,7 @@
     end
 
     it 'should return empty list when search by invalid iid' do
-      results = described_class.new(limit_project_ids, '#222')
+      results = described_class.new(user, limit_project_ids, '#222')
 
       expect(results.objects('merge_requests')).to be_empty
       expect(results.merge_requests_count).to eq 0
-- 
GitLab


From 5c4026ccbc83ae436e8d18b947ea96724324f62c Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Wed, 2 Mar 2016 17:35:31 -0300
Subject: [PATCH 11/30] Add class method to restrict access to issues and avoid
 code duplication

---
 app/finders/issuable_finder.rb       | 21 ---------------------
 app/finders/issues_finder.rb         |  6 ++++++
 app/models/issue.rb                  | 18 ++++++++++++++++++
 lib/api/issues.rb                    | 17 +----------------
 lib/gitlab/project_search_results.rb | 22 ----------------------
 lib/gitlab/search_results.rb         | 17 +----------------
 6 files changed, 26 insertions(+), 75 deletions(-)

diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 36f909065b9ab2..f1fb01769d21c9 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -40,7 +40,6 @@ def execute
     items = by_author(items)
     items = by_label(items)
     items = by_weight(items)
-    items = by_confidentiality(items)
     sort(items)
   end
 
@@ -309,26 +308,6 @@ def filter_by_any_weight?
     params[:weight] == Issue::WEIGHT_ANY
   end
 
-  def by_confidentiality(items)
-    return items unless klass == Issue
-
-    if current_user
-      if current_user.admin? || project.team.member?(current_user.id)
-        items
-      else
-        issuable_table = items.arel_table
-
-        items.where(
-          issuable_table[:confidential].eq(false).or(
-            issuable_table[:confidential].eq(true).and(issuable_table[:author_id].eq(current_user.id))
-          )
-        )
-      end
-    else
-      items.not_confidential
-    end
-  end
-
   def label_names
     params[:label_name].split(',')
   end
diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb
index 20a2b0ce8f0dbb..0251b35d12abce 100644
--- a/app/finders/issues_finder.rb
+++ b/app/finders/issues_finder.rb
@@ -19,4 +19,10 @@ class IssuesFinder < IssuableFinder
   def klass
     Issue
   end
+
+  private
+
+  def init_collection
+    Issue.available_for(current_user)
+  end
 end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 6c60153ee024fb..0a02b956b572c8 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -65,6 +65,24 @@ def hook_attrs
     attributes
   end
 
+  def self.available_for(user)
+    return not_confidential if user.blank?
+    return all if user.admin?
+
+    issues_table = self.arel_table
+    project_ids  = user.authorized_projects.pluck(:id)
+
+    where(
+      issues_table[:confidential].eq(false).or(
+        issues_table[:confidential].eq(true).and(
+          issues_table[:author_id].eq(user.id).or(
+            issues_table[:project_id].in(project_ids)
+          )
+        )
+      )
+    )
+  end
+
   def self.reference_prefix
     '#'
   end
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index f82b1c75d0719d..d1cbd86741263a 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -22,20 +22,6 @@ def filter_issues_milestone(issues, milestone)
         issues.includes(:milestone).where('milestones.title' => milestone)
       end
 
-      def filter_issues_confidentiality(issues)
-        if current_user.admin? || user_project.team.member?(current_user.id)
-          issues
-        else
-          issuable_table = issues.arel_table
-
-          issues.where(
-            issuable_table[:confidential].eq(false).or(
-              issuable_table[:author_id].eq(current_user.id).and(issuable_table[:confidential].eq(true))
-            )
-          )
-        end
-      end
-
       def create_spam_log(project, current_user, attrs)
         params = attrs.merge({
           source_ip: env['REMOTE_ADDR'],
@@ -96,11 +82,10 @@ def create_spam_log(project, current_user, attrs)
       #   GET /projects/:id/issues?milestone=1.0.0&state=closed
       #   GET /issues?iid=42
       get ":id/issues" do
-        issues = user_project.issues
+        issues = user_project.issues.available_for(current_user)
         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?
-        issues = filter_issues_confidentiality(issues)
 
         unless params[:milestone].nil?
           issues = filter_issues_milestone(issues, params[:milestone])
diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb
index f261030469cabc..7edd8f9ff56682 100644
--- a/lib/gitlab/project_search_results.rb
+++ b/lib/gitlab/project_search_results.rb
@@ -85,28 +85,6 @@ def commits
       end
     end
 
-    def issues
-      issues = Issue.where(project_id: project_ids_relation)
-
-      unless user.admin? || project.team.member?(user.id)
-        issuable_table = issues.arel_table
-
-        issues = issues.where(
-          issuable_table[:confidential].eq(false).or(
-            issuable_table[:confidential].eq(true).and(issuable_table[:author_id].eq(user.id))
-          )
-        )
-      end
-
-      if query =~ /#(\d+)\z/
-        issues = issues.where(iid: $1)
-      else
-        issues = issues.full_search(query)
-      end
-
-      issues.order('updated_at DESC')
-    end
-
     def project_ids_relation
       project
     end
diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb
index 43a490fc1b0568..32f76570895d6c 100644
--- a/lib/gitlab/search_results.rb
+++ b/lib/gitlab/search_results.rb
@@ -59,22 +59,7 @@ def projects
     end
 
     def issues
-      issues = Issue.where(project_id: project_ids_relation)
-
-      unless user.admin?
-        issues_table = issues.arel_table
-        authorized_projects_ids = user.authorized_projects.pluck(:id)
-
-        issues = issues.where(
-          issues_table[:confidential].eq(false).or(
-            issues_table[:confidential].eq(true).and(
-              issues_table[:author_id].eq(user.id).or(
-                issues_table[:project_id].in(authorized_projects_ids)
-              )
-            )
-          )
-        )
-      end
+      issues = Issue.available_for(user).where(project_id: project_ids_relation)
 
       if query =~ /#(\d+)\z/
         issues = issues.where(iid: $1)
-- 
GitLab


From 6ce5f97ecb5129969887703bb2dc72c2cc85568e Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Wed, 2 Mar 2016 18:03:57 -0300
Subject: [PATCH 12/30] Use eye-slash icon on confidential issues instead of
 lock icon

---
 app/views/projects/issues/_issue.html.haml | 7 +++----
 app/views/projects/issues/show.html.haml   | 3 +--
 app/views/search/results/_issue.html.haml  | 2 +-
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml
index c062f6d648d483..7f793e9e2e68d3 100644
--- a/app/views/projects/issues/_issue.html.haml
+++ b/app/views/projects/issues/_issue.html.haml
@@ -5,16 +5,15 @@
 
   .issue-title
     %span.issue-title-text
+      - if issue.confidential?
+        = icon('eye-slash')
+
       = link_to_gfm issue.title, issue_path(issue), class: "title"
     %ul.controls.light
       - if issue.closed?
         %li
           CLOSED
 
-      - if issue.confidential?
-        %li
-          = icon('lock')
-
       - if issue.assignee
         %li
           = link_to_member(@project, issue.assignee, name: false, title: "Assigned to :name")
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index 375adfba15e56e..f323cef1a7a43a 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -23,7 +23,7 @@
 
     .issue-meta
       - if @issue.confidential
-        = icon('lock')
+        = icon('eye-slash')
       %strong.identifier
         Issue ##{@issue.iid}
       %span.creator
@@ -52,7 +52,6 @@
           = icon('pencil-square-o')
           Edit
 
-
   .issue-details.issuable-details
     .detail-page-description.content-block
       %h2.title
diff --git a/app/views/search/results/_issue.html.haml b/app/views/search/results/_issue.html.haml
index 849e1ef8940f64..d84f28bf40e866 100644
--- a/app/views/search/results/_issue.html.haml
+++ b/app/views/search/results/_issue.html.haml
@@ -1,7 +1,7 @@
 .search-result-row
   %h4
     - if issue.confidential?
-      = icon('lock')
+      = icon('eye-slash')
     = link_to [issue.project.namespace.becomes(Namespace), issue.project, issue] do
       %span.term.str-truncated= issue.title
     .pull-right ##{issue.iid}
-- 
GitLab


From 52f183bae17e79e5bbc6d8160983db93e003f9d5 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Wed, 2 Mar 2016 18:21:20 -0300
Subject: [PATCH 13/30]  Change the label to make a issue confidential to one
 more clear

---
 app/views/shared/issuable/_form.html.haml | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 51f4ab5a188462..57f78461f06d6a 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -19,6 +19,15 @@
         - else
           Start the title with <code>[WIP]</code> or <code>WIP:</code> to prevent a
           <strong>Work In Progress</strong> merge request from being merged before it's ready.
+
+- if issuable.is_a?(Issue)
+  .form-group
+    .col-sm-offset-2.col-sm-10
+      .checkbox
+        = f.label :confidential do
+          = f.check_box :confidential
+          This issue is confidential and should only be visible to team members
+
 .form-group.detail-page-description
   = f.label :description, 'Description', class: 'control-label'
   .col-sm-10
@@ -29,13 +38,6 @@
       = render 'projects/notes/hints'
       .clearfix
       .error-alert
-- if issuable.is_a?(Issue)
-  .form-group
-    .col-sm-offset-2.col-sm-10
-      .checkbox
-        = f.label :confidential do
-          = f.check_box :confidential
-          Make this issue confidential
 - if can?(current_user, :"admin_#{issuable.to_ability_name}", issuable.project)
   %hr
   .form-group
-- 
GitLab


From 405f5f23630ed45cfed6e6e61a53529d8b788a40 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Wed, 2 Mar 2016 21:33:43 -0300
Subject: [PATCH 14/30] Restrict access to references for confidential issues

---
 lib/banzai/filter/issue_reference_filter.rb   | 20 +++++++--
 .../lib/banzai/filter/redactor_filter_spec.rb | 43 +++++++++++++++----
 2 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/lib/banzai/filter/issue_reference_filter.rb b/lib/banzai/filter/issue_reference_filter.rb
index 13395b934f2c0b..a655585e02ea76 100644
--- a/lib/banzai/filter/issue_reference_filter.rb
+++ b/lib/banzai/filter/issue_reference_filter.rb
@@ -10,14 +10,28 @@ def self.object_class
       end
 
       def self.user_can_see_reference?(user, node, context)
-        if node.has_attribute?('data-issue')
-          issue = Issue.find(node.attr('data-issue')) rescue nil
-          issue && !issue.confidential?
+        project = Project.find(node.attr('data-project')) rescue nil
+        return unless project
+
+        id = node.attr('data-issue')
+        issue = find_object(project, id)
+        return unless issue
+
+        if issue.is_a?(Issue) && issue.confidential?
+          Ability.abilities.allowed?(user, :read_issue, issue)
         else
           super
         end
       end
 
+      def self.find_object(project, id)
+        if project.default_issues_tracker?
+          project.issues.find_by(id: id)
+        else
+          ExternalIssue.new(id, project)
+        end
+      end
+
       def find_object(project, id)
         project.get_issue(id)
       end
diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb
index 2898334e501496..595a8f7b1878d8 100644
--- a/spec/lib/banzai/filter/redactor_filter_spec.rb
+++ b/spec/lib/banzai/filter/redactor_filter_spec.rb
@@ -45,23 +45,48 @@ def reference_link(data)
   end
 
   context 'with data-issue' do
-    it 'removes references for confidential issues' do
-      user = create(:user)
-      project = create(:empty_project)
-      issue = create(:issue, :confidential, project: project)
+    context 'for confidential issues' do
+      it 'removes references for non project members' do
+        non_member = create(:user)
+        project = create(:empty_project, :public)
+        issue = create(:issue, :confidential, project: project)
 
-      link = reference_link(issue: issue.id, reference_filter: 'IssueReferenceFilter')
-      doc = filter(link, current_user: user)
+        link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter')
+        doc = filter(link, current_user: non_member)
 
-      expect(doc.css('a').length).to eq 0
+        expect(doc.css('a').length).to eq 0
+      end
+
+      it 'allows references for author' do
+        author = create(:user)
+        project = create(:empty_project, :public)
+        issue = create(:issue, :confidential, project: project, author: author)
+
+        link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter')
+        doc = filter(link, current_user: author)
+
+        expect(doc.css('a').length).to eq 1
+      end
+
+      it 'allows references for project members' do
+        member = create(:user)
+        project = create(:empty_project, :public)
+        project.team << [member, :developer]
+        issue = create(:issue, :confidential, project: project)
+
+        link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter')
+        doc = filter(link, current_user: member)
+
+        expect(doc.css('a').length).to eq 1
+      end
     end
 
     it 'allows references for non confidential issues' do
       user = create(:user)
-      project = create(:empty_project)
+      project = create(:empty_project, :public)
       issue = create(:issue, project: project)
 
-      link = reference_link(issue: issue.id, reference_filter: 'IssueReferenceFilter')
+      link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter')
       doc = filter(link, current_user: user)
 
       expect(doc.css('a').length).to eq 1
-- 
GitLab


From cdc66a222477f099da2e65b0b56a8bcadce6e27f Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 3 Mar 2016 00:24:56 -0300
Subject: [PATCH 15/30] Allow acces to confidential issues to user set as the
 assignee

---
 app/models/ability.rb                         | 34 ++++++--------
 app/models/issue.rb                           |  4 +-
 lib/elastic/issues_search.rb                  | 46 ++++++++++---------
 lib/gitlab/elastic/project_search_results.rb  | 19 --------
 lib/gitlab/elastic/search_results.rb          | 10 +---
 .../projects/issues_controller_spec.rb        | 18 +++++++-
 .../lib/banzai/filter/redactor_filter_spec.rb | 11 +++++
 .../elastic/project_search_results_spec.rb    | 13 +++++-
 .../lib/gitlab/elastic/search_results_spec.rb | 18 +++++++-
 .../lib/gitlab/project_search_results_spec.rb | 13 +++++-
 spec/lib/gitlab/search_results_spec.rb        | 18 +++++++-
 spec/requests/api/issues_spec.rb              | 19 +++++++-
 12 files changed, 144 insertions(+), 79 deletions(-)

diff --git a/app/models/ability.rb b/app/models/ability.rb
index 4a941d28a2cfba..9e568c74614e5a 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -43,8 +43,6 @@ def anonymous_abilities(user, subject)
         anonymous_personal_snippet_abilities(subject)
       when subject.is_a?(CommitStatus)
         anonymous_commit_status_abilities(subject)
-      when subject.is_a?(Issue)
-        anonymous_issue_abilities(subject)
       when subject.is_a?(Project) || subject.respond_to?(:project)
         anonymous_project_abilities(subject)
       when subject.is_a?(Group) || subject.respond_to?(:group)
@@ -54,12 +52,6 @@ def anonymous_abilities(user, subject)
       end
     end
 
-    def anonymous_issue_abilities(subject)
-      rules = anonymous_project_abilities(subject)
-      rules -= confidential_issue_rules if subject.confidential?
-      rules
-    end
-
     def anonymous_project_abilities(subject)
       project = if subject.is_a?(Project)
                   subject
@@ -71,7 +63,6 @@ def anonymous_project_abilities(subject)
         rules = [
           :read_project,
           :read_wiki,
-          :read_issue,
           :read_label,
           :read_milestone,
           :read_project_snippet,
@@ -85,6 +76,9 @@ def anonymous_project_abilities(subject)
         # Allow to read builds by anonymous user if guests are allowed
         rules << :read_build if project.public_builds?
 
+        # Allow to read issues by anonymous user if issue is not confidential
+        rules << :read_issue unless subject.is_a?(Issue) && subject.confidential?
+
         rules - project_disabled_features_rules(project)
       else
         []
@@ -351,13 +345,7 @@ def namespace_abilities(user, namespace)
         end
 
         rules += project_abilities(user, subject.project)
-
-        if subject.respond_to?(:confidential) && subject.confidential?
-          unless user.admin? || subject.author == user || subject.project.team.member?(user.id)
-            rules -= confidential_issue_rules
-          end
-        end
-
+        rules = filter_confidential_issues_abilities(user, subject, rules) if subject.is_a?(Issue)
         rules
       end
     end
@@ -477,11 +465,15 @@ def named_abilities(name)
       ]
     end
 
-    def confidential_issue_rules
-      [
-        :read_issue,
-        :update_issue
-      ]
+    def filter_confidential_issues_abilities(user, issue, rules)
+      return rules if user.admin? || !issue.confidential?
+
+      unless issue.author == user || issue.assignee == user || issue.project.team.member?(user.id)
+        rules.delete(:read_issue)
+        rules.delete(:update_issue)
+      end
+
+      rules
     end
   end
 end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 0a02b956b572c8..ce9fb554f0fcf8 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -76,7 +76,9 @@ def self.available_for(user)
       issues_table[:confidential].eq(false).or(
         issues_table[:confidential].eq(true).and(
           issues_table[:author_id].eq(user.id).or(
-            issues_table[:project_id].in(project_ids)
+            issues_table[:assignee_id].eq(user.id).or(
+              issues_table[:project_id].in(project_ids)
+            )
           )
         )
       )
diff --git a/lib/elastic/issues_search.rb b/lib/elastic/issues_search.rb
index e5d5c3aa9945e8..a56ee9bdd12b9d 100644
--- a/lib/elastic/issues_search.rb
+++ b/lib/elastic/issues_search.rb
@@ -19,6 +19,7 @@ module IssuesSearch
 
         indexes :project_id,  type: :integer
         indexes :author_id,   type: :integer
+        indexes :assignee_id, type: :integer
 
         indexes :project,     type: :nested
         indexes :author,      type: :nested
@@ -50,35 +51,36 @@ def self.elastic_search(query, options: {})
           query_hash = basic_query_hash(%w(title^2 description), query)
         end
 
-        query_hash = project_ids_filter(query_hash, options[:projects_ids])
-        query_hash = confidentiality_filter(query_hash, options[:user_id], options[:authorized_projects_ids])
+        query_hash = project_ids_filter(query_hash, options[:project_ids])
+        query_hash = confidentiality_filter(query_hash, options[:user])
 
         self.__elasticsearch__.search(query_hash)
       end
 
-      def self.confidentiality_filter(query_hash, user_id, projects_ids)
-        if user_id
-          query_hash[:query][:filtered][:filter] = {
-            bool: {
-              should: [
-                { term: { confidential: false } },
-                { bool: {
-                    must: [
-                      { term: { confidential: true } },
-                      { bool: {
-                          should: [
-                            { term: { author_id: user_id } },
-                            { terms: { project_id: projects_ids } }
-                          ]
-                        }
+      def self.confidentiality_filter(query_hash, user)
+        return query_hash if user.blank? || user.admin?
+
+        query_hash[:query][:filtered][:filter] = {
+          bool: {
+            should: [
+              { term: { confidential: false } },
+              { bool: {
+                  must: [
+                    { term: { confidential: true } },
+                    { bool: {
+                        should: [
+                          { term: { author_id: user.id } },
+                          { term: { assignee_id: user.id } },
+                          { terms: { project_id: user.authorized_projects.pluck(:id) } }
+                        ]
                       }
-                    ]
-                  }
+                    }
+                  ]
                 }
-              ]
-            }
+              }
+            ]
           }
-        end
+        }
 
         query_hash
       end
diff --git a/lib/gitlab/elastic/project_search_results.rb b/lib/gitlab/elastic/project_search_results.rb
index be3e6f009b48e0..5432546842d264 100644
--- a/lib/gitlab/elastic/project_search_results.rb
+++ b/lib/gitlab/elastic/project_search_results.rb
@@ -107,25 +107,6 @@ def commits
         end
       end
 
-      def issues
-        opt = {
-          projects_ids: limit_project_ids
-        }
-
-        unless user.admin? || project.team.member?(user.id)
-          opt.merge!(
-            authorized_projects_ids: user.authorized_projects.pluck(:id),
-            user_id: user.id
-          )
-        end
-
-        if query =~ /#(\d+)\z/
-          Issue.in_projects(limit_project_ids).where(iid: $1)
-        else
-          Issue.elastic_search(query, options: opt).records
-        end
-      end
-
       def limit_project_ids
         [project.id]
       end
diff --git a/lib/gitlab/elastic/search_results.rb b/lib/gitlab/elastic/search_results.rb
index a208dc4aab1a6a..8cd6ac33c6ff2a 100644
--- a/lib/gitlab/elastic/search_results.rb
+++ b/lib/gitlab/elastic/search_results.rb
@@ -64,16 +64,10 @@ def projects
 
       def issues
         opt = {
-          project_ids: limit_project_ids
+          projects_ids: limit_project_ids,
+          user: user
         }
 
-        unless user.admin?
-          opt.merge!(
-            authorized_projects_ids: user.authorized_projects.pluck(:id),
-            user_id: user.id
-          )
-        end
-
         Issue.elastic_search(query, options: opt)
       end
 
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index cacd22609aa77f..2cd81231144c41 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -42,13 +42,14 @@
 
   describe 'Confidential Issues' do
     let(:project) { create(:empty_project, :public) }
+    let(:assignee) { create(:assignee) }
     let(:author) { create(:user) }
     let(:non_member) { create(:user) }
     let(:member) { create(:user) }
     let(:admin) { create(:admin) }
     let!(:issue) { create(:issue, project: project) }
     let!(:unescaped_parameter_value) { create(:issue, :confidential, project: project, author: author) }
-    let!(:request_forgery_timing_attack) { create(:issue, :confidential, project: project) }
+    let!(:request_forgery_timing_attack) { create(:issue, :confidential, project: project, assignee: assignee) }
 
     describe 'GET #index' do
       it 'should not list confidential issues for guests' do
@@ -73,6 +74,14 @@
         expect(assigns(:issues)).not_to include request_forgery_timing_attack
       end
 
+      it 'should list confidential issues for assignee' do
+        sign_in(assignee)
+        get_issues
+
+        expect(assigns(:issues)).not_to include unescaped_parameter_value
+        expect(assigns(:issues)).to include request_forgery_timing_attack
+      end
+
       it 'should list confidential issues for project members' do
         sign_in(member)
         project.team << [member, :developer]
@@ -120,6 +129,13 @@ def get_issues
         expect(response).to have_http_status http_status[:success]
       end
 
+      it "returns #{http_status[:success]} for assignee" do
+        sign_in(assignee)
+        go(id: request_forgery_timing_attack.to_param)
+
+        expect(response).to have_http_status http_status[:success]
+      end
+
       it "returns #{http_status[:success]} for project members" do
         sign_in(member)
         project.team << [member, :developer]
diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb
index 595a8f7b1878d8..7a9009e9eb80c2 100644
--- a/spec/lib/banzai/filter/redactor_filter_spec.rb
+++ b/spec/lib/banzai/filter/redactor_filter_spec.rb
@@ -68,6 +68,17 @@ def reference_link(data)
         expect(doc.css('a').length).to eq 1
       end
 
+      it 'allows references for assignee' do
+        assignee = create(:user)
+        project = create(:empty_project, :public)
+        issue = create(:issue, :confidential, project: project, assignee: assignee)
+
+        link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter')
+        doc = filter(link, current_user: assignee)
+
+        expect(doc.css('a').length).to eq 1
+      end
+
       it 'allows references for project members' do
         member = create(:user)
         project = create(:empty_project, :public)
diff --git a/spec/lib/gitlab/elastic/project_search_results_spec.rb b/spec/lib/gitlab/elastic/project_search_results_spec.rb
index ca59d844bc674c..cee317fe21377c 100644
--- a/spec/lib/gitlab/elastic/project_search_results_spec.rb
+++ b/spec/lib/gitlab/elastic/project_search_results_spec.rb
@@ -68,12 +68,13 @@
   describe 'confidential issues' do
     let(:query) { 'issue' }
     let(:author) { create(:user) }
+    let(:assignee) { create(:user) }
     let(:non_member) { create(:user) }
     let(:member) { create(:user) }
     let(:admin) { create(:admin) }
     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) }
+    let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project, assignee: assignee) }
 
     before do
       Issue.__elasticsearch__.refresh_index!
@@ -99,6 +100,16 @@
       expect(results.issues_count).to eq 2
     end
 
+    it 'should list project confidential issues for assignee' do
+      results = described_class.new(assignee, project.id, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).not_to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(results.issues_count).to eq 2
+    end
+
     it 'should list project confidential issues for project members' do
       project.team << [member, :developer]
 
diff --git a/spec/lib/gitlab/elastic/search_results_spec.rb b/spec/lib/gitlab/elastic/search_results_spec.rb
index dde527db7054c7..1cb833c3ffeb16 100644
--- a/spec/lib/gitlab/elastic/search_results_spec.rb
+++ b/spec/lib/gitlab/elastic/search_results_spec.rb
@@ -86,14 +86,15 @@
     let(:query) { 'issue' }
     let(:limit_project_ids) { [project_1.id, project_2.id, project_3.id] }
     let(:author) { create(:user) }
+    let(:assignee) { create(:user) }
     let(:non_member) { create(:user) }
     let(:member) { create(:user) }
     let(:admin) { create(:admin) }
     let!(:issue) { create(:issue, project: project_1, title: 'Issue 1') }
     let!(:security_issue_1) { create(:issue, :confidential, project: project_1, title: 'Security issue 1', author: author) }
-    let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project_1) }
+    let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project_1, assignee: assignee) }
     let!(:security_issue_3) { create(:issue, :confidential, project: project_2, title: 'Security issue 3', author: author) }
-    let!(:security_issue_4) { create(:issue, :confidential, project: project_3, title: 'Security issue 4') }
+    let!(:security_issue_4) { create(:issue, :confidential, project: project_3, title: 'Security issue 4', assignee: assignee) }
     let!(:security_issue_5) { create(:issue, :confidential, project: project_4, title: 'Security issue 5') }
 
     before do
@@ -126,6 +127,19 @@
       expect(results.issues_count).to eq 3
     end
 
+    it 'should list confidential issues for assignee' do
+      results = described_class.new(assignee, limit_project_ids, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).not_to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(issues).not_to include security_issue_3
+      expect(issues).to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 3
+    end
+
     it 'should list confidential issues for project members' do
       project_1.team << [member, :developer]
       project_2.team << [member, :developer]
diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb
index a21083d88b07f2..db0ff95b4f5c6c 100644
--- a/spec/lib/gitlab/project_search_results_spec.rb
+++ b/spec/lib/gitlab/project_search_results_spec.rb
@@ -25,12 +25,13 @@
   describe 'confidential issues' do
     let(:query) { 'issue' }
     let(:author) { create(:user) }
+    let(:assignee) { create(:user) }
     let(:non_member) { create(:user) }
     let(:member) { create(:user) }
     let(:admin) { create(:admin) }
     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) }
+    let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project, assignee: assignee) }
 
     it 'should not list project confidential issues for non project members' do
       results = described_class.new(non_member, project, query)
@@ -52,6 +53,16 @@
       expect(results.issues_count).to eq 2
     end
 
+    it 'should list project confidential issues for assignee' do
+      results = described_class.new(assignee, project.id, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).not_to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(results.issues_count).to eq 2
+    end
+
     it 'should list project confidential issues for project members' do
       project.team << [member, :developer]
 
diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb
index 945292117e1151..f4afe597e8d8c4 100644
--- a/spec/lib/gitlab/search_results_spec.rb
+++ b/spec/lib/gitlab/search_results_spec.rb
@@ -62,14 +62,15 @@
     let(:query) { 'issue' }
     let(:limit_projects) { Project.where(id: [project_1.id, project_2.id, project_3.id]) }
     let(:author) { create(:user) }
+    let(:assignee) { create(:user) }
     let(:non_member) { create(:user) }
     let(:member) { create(:user) }
     let(:admin) { create(:admin) }
     let!(:issue) { create(:issue, project: project_1, title: 'Issue 1') }
     let!(:security_issue_1) { create(:issue, :confidential, project: project_1, title: 'Security issue 1', author: author) }
-    let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project_1) }
+    let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project_1, assignee: assignee) }
     let!(:security_issue_3) { create(:issue, :confidential, project: project_2, title: 'Security issue 3', author: author) }
-    let!(:security_issue_4) { create(:issue, :confidential, project: project_3, title: 'Security issue 4') }
+    let!(:security_issue_4) { create(:issue, :confidential, project: project_3, title: 'Security issue 4', assignee: assignee) }
     let!(:security_issue_5) { create(:issue, :confidential, project: project_4, title: 'Security issue 5') }
 
     it 'should not list confidential issues for non project members' do
@@ -98,6 +99,19 @@
       expect(results.issues_count).to eq 3
     end
 
+    it 'should list confidential issues for assignee' do
+      results = described_class.new(assignee, limit_projects, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).not_to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(issues).not_to include security_issue_3
+      expect(issues).to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 3
+    end
+
     it 'should list confidential issues for project members' do
       project_1.team << [member, :developer]
       project_2.team << [member, :developer]
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 8abf74d3ee3cd7..bb2ab058003cfe 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -5,6 +5,7 @@
   let(:user) { create(:user) }
   let(:non_member) { create(:user) }
   let(:author) { create(:author) }
+  let(:assignee) { create(:assignee) }
   let(:admin) { create(:admin) }
   let!(:project) { create(:project, :public, namespace: user.namespace ) }
   let!(:closed_issue) do
@@ -19,7 +20,8 @@
     create :issue,
            :confidential,
            project: project,
-           author: author
+           author: author,
+           assignee: assignee
   end
   let!(:issue) do
     create :issue,
@@ -148,6 +150,14 @@
       expect(json_response.first['title']).to eq(issue.title)
     end
 
+    it 'should return project confidential issues for assignee' do
+      get api("#{base_url}/issues", assignee)
+      expect(response.status).to eq(200)
+      expect(json_response).to be_an Array
+      expect(json_response.length).to eq(3)
+      expect(json_response.first['title']).to eq(issue.title)
+    end
+
     it 'should return project issues with confidential issues for project members' do
       get api("#{base_url}/issues", user)
       expect(response.status).to eq(200)
@@ -261,6 +271,13 @@
         expect(json_response['iid']).to eq(confidential_issue.iid)
       end
 
+      it "should return confidential issue for assignee" do
+        get api("/projects/#{project.id}/issues/#{confidential_issue.id}", assignee)
+        expect(response.status).to eq(200)
+        expect(json_response['title']).to eq(confidential_issue.title)
+        expect(json_response['iid']).to eq(confidential_issue.iid)
+      end
+
       it "should return confidential issue for admin" do
         get api("/projects/#{project.id}/issues/#{confidential_issue.id}", admin)
         expect(response.status).to eq(200)
-- 
GitLab


From 5972a22035f71c5c241daadb5a77a4cb4bafdd8d Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 3 Mar 2016 00:39:41 -0300
Subject: [PATCH 16/30] Restrict access for confidential issues on autocomplete

---
 app/controllers/projects_controller.rb        |  2 +-
 app/models/issue.rb                           |  3 +-
 app/services/projects/autocomplete_service.rb |  6 +-
 .../projects/autocomplete_service_spec.rb     | 77 +++++++++++++++++--
 4 files changed, 73 insertions(+), 15 deletions(-)

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 89e8ea678786f5..fd8580d49e0605 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -135,7 +135,7 @@ def destroy
   def autocomplete_sources
     note_type = params['type']
     note_id = params['type_id']
-    autocomplete = ::Projects::AutocompleteService.new(@project)
+    autocomplete = ::Projects::AutocompleteService.new(@project, current_user)
     participants = ::Projects::ParticipantsService.new(@project, current_user).execute(note_type, note_id)
 
     @suggestions = {
diff --git a/app/models/issue.rb b/app/models/issue.rb
index ce9fb554f0fcf8..b3ce266f7115c7 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -45,7 +45,6 @@ class Issue < ActiveRecord::Base
   scope :cared, ->(user) { where(assignee_id: user) }
   scope :open_for, ->(user) { opened.assigned_to(user) }
   scope :in_projects, ->(project_ids) { where(project_id: project_ids) }
-  scope :not_confidential, -> { where(confidential: false) }
 
   state_machine :state, initial: :opened do
     event :close do
@@ -66,7 +65,7 @@ def hook_attrs
   end
 
   def self.available_for(user)
-    return not_confidential if user.blank?
+    return where(confidential: false) if user.blank?
     return all if user.admin?
 
     issues_table = self.arel_table
diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb
index 74d26aaaaed37f..a18debb80a8087 100644
--- a/app/services/projects/autocomplete_service.rb
+++ b/app/services/projects/autocomplete_service.rb
@@ -1,11 +1,7 @@
 module Projects
   class AutocompleteService < BaseService
-    def initialize(project)
-      @project = project
-    end
-
     def issues
-      @project.issues.not_confidential.opened.select([:iid, :title])
+      @project.issues.available_for(current_user).opened.select([:iid, :title])
     end
 
     def merge_requests
diff --git a/spec/services/projects/autocomplete_service_spec.rb b/spec/services/projects/autocomplete_service_spec.rb
index 83e606e6d39417..6108c26a78b862 100644
--- a/spec/services/projects/autocomplete_service_spec.rb
+++ b/spec/services/projects/autocomplete_service_spec.rb
@@ -1,16 +1,79 @@
 require 'spec_helper'
 
 describe Projects::AutocompleteService, services: true do
-  let(:project) { create(:empty_project, :public) }
+  describe '#issues' do
+    describe 'confidential issues' do
+      let(:author) { create(:user) }
+      let(:assignee) { create(:user) }
+      let(:non_member) { create(:user) }
+      let(:member) { create(:user) }
+      let(:admin) { create(:admin) }
+      let(:project) { create(:empty_project, :public) }
+      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) }
 
-  subject(:autocomplete) { described_class.new(project) }
+      it 'should not list project confidential issues for guests' do
+        autocomplete = described_class.new(project, nil)
+        issues = autocomplete.issues.map(&:iid)
 
-  describe '#issues' do
-    it 'should not list confidential issues' do
-      issue = create(:issue, project: project)
-      create(:issue, :confidential, project: project)
+        expect(issues).to include issue.iid
+        expect(issues).not_to include security_issue_1.iid
+        expect(issues).not_to include security_issue_2.iid
+        expect(issues.count).to eq 1
+      end
+
+      it 'should not list project confidential issues for non project members' do
+        autocomplete = described_class.new(project, non_member)
+        issues = autocomplete.issues.map(&:iid)
+
+        expect(issues).to include issue.iid
+        expect(issues).not_to include security_issue_1.iid
+        expect(issues).not_to include security_issue_2.iid
+        expect(issues.count).to eq 1
+      end
+
+      it 'should list project confidential issues for author' do
+        autocomplete = described_class.new(project, author)
+        issues = autocomplete.issues.map(&:iid)
+
+        expect(issues).to include issue.iid
+        expect(issues).to include security_issue_1.iid
+        expect(issues).not_to include security_issue_2.iid
+        expect(issues.count).to eq 2
+      end
+
+      it 'should list project confidential issues for assignee' do
+        autocomplete = described_class.new(project, assignee)
+        issues = autocomplete.issues.map(&:iid)
+
+        expect(issues).to include issue.iid
+        expect(issues).not_to include security_issue_1.iid
+        expect(issues).to include security_issue_2.iid
+        expect(issues.count).to eq 2
+      end
+
+      it 'should list project confidential issues for project members' do
+        project.team << [member, :developer]
+
+        autocomplete = described_class.new(project, member)
+        issues = autocomplete.issues.map(&:iid)
+
+        expect(issues).to include issue.iid
+        expect(issues).to include security_issue_1.iid
+        expect(issues).to include security_issue_2.iid
+        expect(issues.count).to eq 3
+      end
+
+      it 'should list all project issues for admin' do
+        autocomplete = described_class.new(project, admin)
+        issues = autocomplete.issues.map(&:iid)
 
-      expect(autocomplete.issues.map(&:iid)).to eq [issue.iid]
+        expect(issues).to include issue.iid
+        expect(issues).to include security_issue_1.iid
+        expect(issues).to include security_issue_2.iid
+        expect(issues.count).to eq 3
+      end
     end
   end
 end
-- 
GitLab


From f90533f2e0b13ffabbd1929fc50218f4ca4d7878 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 4 Mar 2016 14:21:17 -0300
Subject: [PATCH 17/30] Improve Elasticsearch search results spec

---
 .../lib/gitlab/elastic/search_results_spec.rb | 199 ++++++++++++------
 1 file changed, 137 insertions(+), 62 deletions(-)

diff --git a/spec/lib/gitlab/elastic/search_results_spec.rb b/spec/lib/gitlab/elastic/search_results_spec.rb
index 1cb833c3ffeb16..180bde15bb9053 100644
--- a/spec/lib/gitlab/elastic/search_results_spec.rb
+++ b/spec/lib/gitlab/elastic/search_results_spec.rb
@@ -83,90 +83,165 @@
   describe 'confidential issues' do
     let(:project_3) { create(:empty_project) }
     let(:project_4) { create(:empty_project) }
-    let(:query) { 'issue' }
     let(:limit_project_ids) { [project_1.id, project_2.id, project_3.id] }
     let(:author) { create(:user) }
     let(:assignee) { create(:user) }
     let(:non_member) { create(:user) }
     let(:member) { create(:user) }
     let(:admin) { create(:admin) }
-    let!(:issue) { create(:issue, project: project_1, title: 'Issue 1') }
-    let!(:security_issue_1) { create(:issue, :confidential, project: project_1, title: 'Security issue 1', author: author) }
-    let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project_1, assignee: assignee) }
-    let!(:security_issue_3) { create(:issue, :confidential, project: project_2, title: 'Security issue 3', author: author) }
-    let!(:security_issue_4) { create(:issue, :confidential, project: project_3, title: 'Security issue 4', assignee: assignee) }
-    let!(:security_issue_5) { create(:issue, :confidential, project: project_4, title: 'Security issue 5') }
+    let!(:issue) { create(:issue, project: project_1, title: 'Issue 1', iid: 1) }
+    let!(:security_issue_1) { create(:issue, :confidential, project: project_1, title: 'Security issue 1', author: author, iid: 2) }
+    let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project_1, assignee: assignee, iid: 3) }
+    let!(:security_issue_3) { create(:issue, :confidential, project: project_2, title: 'Security issue 3', author: author, iid: 1) }
+    let!(:security_issue_4) { create(:issue, :confidential, project: project_3, title: 'Security issue 4', assignee: assignee, iid: 1) }
+    let!(:security_issue_5) { create(:issue, :confidential, project: project_4, title: 'Security issue 5', iid: 1) }
 
     before do
       Issue.__elasticsearch__.refresh_index!
     end
 
-    it 'should not list confidential issues for non project members' do
-      results = described_class.new(non_member, limit_project_ids, query)
-      issues = results.objects('issues')
+    context 'search by term' do
+      let(:query) { 'issue' }
 
-      expect(issues).to include issue
-      expect(issues).not_to include security_issue_1
-      expect(issues).not_to include security_issue_2
-      expect(issues).not_to include security_issue_3
-      expect(issues).not_to include security_issue_4
-      expect(issues).not_to include security_issue_5
-      expect(results.issues_count).to eq 1
-    end
+      it 'should not list confidential issues for non project members' do
+        results = described_class.new(non_member, limit_project_ids, query)
+        issues = results.objects('issues')
 
-    it 'should list confidential issues for author' do
-      results = described_class.new(author, limit_project_ids, query)
-      issues = results.objects('issues')
+        expect(issues).to include issue
+        expect(issues).not_to include security_issue_1
+        expect(issues).not_to include security_issue_2
+        expect(issues).not_to include security_issue_3
+        expect(issues).not_to include security_issue_4
+        expect(issues).not_to include security_issue_5
+        expect(results.issues_count).to eq 1
+      end
 
-      expect(issues).to include issue
-      expect(issues).to include security_issue_1
-      expect(issues).not_to include security_issue_2
-      expect(issues).to include security_issue_3
-      expect(issues).not_to include security_issue_4
-      expect(issues).not_to include security_issue_5
-      expect(results.issues_count).to eq 3
-    end
+      it 'should list confidential issues for author' do
+        results = described_class.new(author, limit_project_ids, query)
+        issues = results.objects('issues')
+
+        expect(issues).to include issue
+        expect(issues).to include security_issue_1
+        expect(issues).not_to include security_issue_2
+        expect(issues).to include security_issue_3
+        expect(issues).not_to include security_issue_4
+        expect(issues).not_to include security_issue_5
+        expect(results.issues_count).to eq 3
+      end
 
-    it 'should list confidential issues for assignee' do
-      results = described_class.new(assignee, limit_project_ids, query)
-      issues = results.objects('issues')
+      it 'should list confidential issues for assignee' do
+        results = described_class.new(assignee, limit_project_ids, query)
+        issues = results.objects('issues')
+
+        expect(issues).to include issue
+        expect(issues).not_to include security_issue_1
+        expect(issues).to include security_issue_2
+        expect(issues).not_to include security_issue_3
+        expect(issues).to include security_issue_4
+        expect(issues).not_to include security_issue_5
+        expect(results.issues_count).to eq 3
+      end
 
-      expect(issues).to include issue
-      expect(issues).not_to include security_issue_1
-      expect(issues).to include security_issue_2
-      expect(issues).not_to include security_issue_3
-      expect(issues).to include security_issue_4
-      expect(issues).not_to include security_issue_5
-      expect(results.issues_count).to eq 3
-    end
+      it 'should list confidential issues for project members' do
+        project_1.team << [member, :developer]
+        project_2.team << [member, :developer]
 
-    it 'should list confidential issues for project members' do
-      project_1.team << [member, :developer]
-      project_2.team << [member, :developer]
+        results = described_class.new(member, limit_project_ids, query)
+        issues = results.objects('issues')
 
-      results = described_class.new(member, limit_project_ids, query)
-      issues = results.objects('issues')
+        expect(issues).to include issue
+        expect(issues).to include security_issue_1
+        expect(issues).to include security_issue_2
+        expect(issues).to include security_issue_3
+        expect(issues).not_to include security_issue_4
+        expect(issues).not_to include security_issue_5
+        expect(results.issues_count).to eq 4
+      end
 
-      expect(issues).to include issue
-      expect(issues).to include security_issue_1
-      expect(issues).to include security_issue_2
-      expect(issues).to include security_issue_3
-      expect(issues).not_to include security_issue_4
-      expect(issues).not_to include security_issue_5
-      expect(results.issues_count).to eq 4
+      it 'should list all issues for admin' do
+        results = described_class.new(admin, limit_project_ids, query)
+        issues = results.objects('issues')
+
+        expect(issues).to include issue
+        expect(issues).to include security_issue_1
+        expect(issues).to include security_issue_2
+        expect(issues).to include security_issue_3
+        expect(issues).to include security_issue_4
+        expect(issues).not_to include security_issue_5
+        expect(results.issues_count).to eq 5
+      end
     end
 
-    it 'should list all issues for admin' do
-      results = described_class.new(admin, limit_project_ids, query)
-      issues = results.objects('issues')
+    context 'search by iid' do
+      let(:query) { '#1' }
+
+      it 'should not list confidential issues for non project members' do
+        results = described_class.new(non_member, limit_project_ids, query)
+        issues = results.objects('issues')
+
+        expect(issues).to include issue
+        expect(issues).not_to include security_issue_1
+        expect(issues).not_to include security_issue_2
+        expect(issues).not_to include security_issue_3
+        expect(issues).not_to include security_issue_4
+        expect(issues).not_to include security_issue_5
+        expect(results.issues_count).to eq 1
+      end
 
-      expect(issues).to include issue
-      expect(issues).to include security_issue_1
-      expect(issues).to include security_issue_2
-      expect(issues).to include security_issue_3
-      expect(issues).to include security_issue_4
-      expect(issues).not_to include security_issue_5
-      expect(results.issues_count).to eq 5
+      it 'should list confidential issues for author' do
+        results = described_class.new(author, limit_project_ids, query)
+        issues = results.objects('issues')
+
+        expect(issues).to include issue
+        expect(issues).not_to include security_issue_1
+        expect(issues).not_to include security_issue_2
+        expect(issues).to include security_issue_3
+        expect(issues).not_to include security_issue_4
+        expect(issues).not_to include security_issue_5
+        expect(results.issues_count).to eq 2
+      end
+
+      it 'should list confidential issues for assignee' do
+        results = described_class.new(assignee, limit_project_ids, query)
+        issues = results.objects('issues')
+
+        expect(issues).to include issue
+        expect(issues).not_to include security_issue_1
+        expect(issues).not_to include security_issue_2
+        expect(issues).not_to include security_issue_3
+        expect(issues).to include security_issue_4
+        expect(issues).not_to include security_issue_5
+        expect(results.issues_count).to eq 2
+      end
+
+      it 'should list confidential issues for project members' do
+        project_2.team << [member, :developer]
+        project_3.team << [member, :developer]
+
+        results = described_class.new(member, limit_project_ids, query)
+        issues = results.objects('issues')
+
+        expect(issues).to include issue
+        expect(issues).not_to include security_issue_1
+        expect(issues).not_to include security_issue_2
+        expect(issues).to include security_issue_3
+        expect(issues).to include security_issue_4
+        expect(issues).not_to include security_issue_5
+        expect(results.issues_count).to eq 3
+      end
+
+      it 'should list all issues for admin' do
+        results = described_class.new(admin, limit_project_ids, query)
+        issues = results.objects('issues')
+
+        expect(issues).to include issue
+        expect(issues).not_to include security_issue_1
+        expect(issues).not_to include security_issue_2
+        expect(issues).to include security_issue_3
+        expect(issues).to include security_issue_4
+        expect(issues).not_to include security_issue_5
+        expect(results.issues_count).to eq 3
+      end
     end
   end
 
-- 
GitLab


From 8a95867b62c204972d392ed99955d64da2836661 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 4 Mar 2016 14:33:13 -0300
Subject: [PATCH 18/30] Update CHANGELOG

---
 CHANGELOG-EE | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG-EE b/CHANGELOG-EE
index a864e2df731a49..506ff45c69966e 100644
--- a/CHANGELOG-EE
+++ b/CHANGELOG-EE
@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.6.0 (unreleased)
   - Handle duplicate appearances table creation issue with upgrade from CE to EE
+  - Add confidential issues
   - Improve weight filter for issues
   - Clear "stuck" mirror updates before periodically updating all mirrors.
   - [Elastic] Add elastic checker to gitlab:check
-- 
GitLab


From ca19217c85c9d7d3d0bafae0859cf12996dd7911 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 7 Mar 2016 20:30:08 -0300
Subject: [PATCH 19/30] Fix Gitlab::Elastic::SearchResults

---
 lib/elastic/application_search.rb             |  4 +-
 lib/elastic/issues_search.rb                  | 59 ++++++++++---------
 lib/gitlab/elastic/project_search_results.rb  |  4 +-
 lib/gitlab/elastic/search_results.rb          | 10 ++--
 spec/lib/elastic/issue_spec.rb                |  3 +-
 .../lib/gitlab/elastic/search_results_spec.rb | 33 +++++++++--
 6 files changed, 73 insertions(+), 40 deletions(-)

diff --git a/lib/elastic/application_search.rb b/lib/elastic/application_search.rb
index fdebd58b4bfecd..8ef796f0215fab 100644
--- a/lib/elastic/application_search.rb
+++ b/lib/elastic/application_search.rb
@@ -113,7 +113,9 @@ def iid_query_hash(query_hash, iid)
       def project_ids_filter(query_hash, project_ids)
         if project_ids
           query_hash[:query][:filtered][:filter] = {
-            and: [ { terms: { project_id: project_ids } } ]
+            bool: {
+              must: [ { terms: { project_id: project_ids } } ]
+            }
           }
         end
 
diff --git a/lib/elastic/issues_search.rb b/lib/elastic/issues_search.rb
index a56ee9bdd12b9d..55a6ce1eb90f79 100644
--- a/lib/elastic/issues_search.rb
+++ b/lib/elastic/issues_search.rb
@@ -34,7 +34,7 @@ def as_indexed_json(options = {})
 
         # We don't use as_json(only: ...) because it calls all virtual and serialized attributtes
         # https://gitlab.com/gitlab-org/gitlab-ee/issues/349
-        [:id, :iid, :title, :description, :created_at, :updated_at, :state, :project_id, :author_id].each do |attr|
+        [:id, :iid, :title, :description, :created_at, :updated_at, :state, :project_id, :author_id, :assignee_id, :confidential].each do |attr|
           data[attr.to_s] = self.send(attr)
         end
 
@@ -52,36 +52,41 @@ def self.elastic_search(query, options: {})
         end
 
         query_hash = project_ids_filter(query_hash, options[:project_ids])
-        query_hash = confidentiality_filter(query_hash, options[:user])
+        query_hash = confidentiality_filter(query_hash, options[:current_user])
 
         self.__elasticsearch__.search(query_hash)
       end
 
-      def self.confidentiality_filter(query_hash, user)
-        return query_hash if user.blank? || user.admin?
-
-        query_hash[:query][:filtered][:filter] = {
-          bool: {
-            should: [
-              { term: { confidential: false } },
-              { bool: {
-                  must: [
-                    { term: { confidential: true } },
-                    { bool: {
-                        should: [
-                          { term: { author_id: user.id } },
-                          { term: { assignee_id: user.id } },
-                          { terms: { project_id: user.authorized_projects.pluck(:id) } }
-                        ]
-                      }
-                    }
-                  ]
-                }
-              }
-            ]
-          }
-        }
-
+      def self.confidentiality_filter(query_hash, current_user)
+        return query_hash if current_user.present? && current_user.admin?
+
+        filter = if current_user.present?
+                   {
+                     bool: {
+                       should: [
+                         { term: { confidential: false } },
+                         { bool: {
+                             must: [
+                               { term: { confidential: true } },
+                               { bool: {
+                                   should: [
+                                     { term: { author_id: current_user.id } },
+                                     { term: { assignee_id: current_user.id } },
+                                     { terms: { project_id: current_user.authorized_projects.pluck(:id) } }
+                                   ]
+                                 }
+                               }
+                             ]
+                           }
+                         }
+                       ]
+                     }
+                   }
+                 else
+                   { term: { confidential: false } }
+                 end
+
+        query_hash[:query][:filtered][:filter][:bool][:must] << filter
         query_hash
       end
     end
diff --git a/lib/gitlab/elastic/project_search_results.rb b/lib/gitlab/elastic/project_search_results.rb
index 5432546842d264..78e53af685392f 100644
--- a/lib/gitlab/elastic/project_search_results.rb
+++ b/lib/gitlab/elastic/project_search_results.rb
@@ -3,8 +3,8 @@ module Elastic
     class ProjectSearchResults < SearchResults
       attr_reader :project, :repository_ref
 
-      def initialize(user, project_id, query, repository_ref = nil)
-        @user = user
+      def initialize(current_user, project_id, query, repository_ref = nil)
+        @current_user = current_user
         @project = Project.find(project_id)
 
         @repository_ref = if repository_ref.present?
diff --git a/lib/gitlab/elastic/search_results.rb b/lib/gitlab/elastic/search_results.rb
index 8cd6ac33c6ff2a..37ddd1c13c7621 100644
--- a/lib/gitlab/elastic/search_results.rb
+++ b/lib/gitlab/elastic/search_results.rb
@@ -1,14 +1,14 @@
 module Gitlab
   module Elastic
     class SearchResults
-      attr_reader :user, :query
+      attr_reader :current_user, :query
 
       # Limit search results by passed project ids
       # It allows us to search only for projects user has access to
       attr_reader :limit_project_ids
 
-      def initialize(user, limit_project_ids, query)
-        @user = user
+      def initialize(current_user, limit_project_ids, query)
+        @current_user = current_user
         @limit_project_ids = limit_project_ids || Project.all
         @query = Shellwords.shellescape(query) if query.present?
       end
@@ -64,8 +64,8 @@ def projects
 
       def issues
         opt = {
-          projects_ids: limit_project_ids,
-          user: user
+          project_ids: limit_project_ids,
+          current_user: current_user
         }
 
         Issue.elastic_search(query, options: opt)
diff --git a/spec/lib/elastic/issue_spec.rb b/spec/lib/elastic/issue_spec.rb
index 73ad0b7d29125b..372bbd0dcb1e6f 100644
--- a/spec/lib/elastic/issue_spec.rb
+++ b/spec/lib/elastic/issue_spec.rb
@@ -33,7 +33,8 @@
     issue = create :issue, project: project
 
     expected_hash =  issue.attributes.extract!('id', 'iid', 'title', 'description', 'created_at',
-                                                'updated_at', 'state', 'project_id', 'author_id')
+                                                'updated_at', 'state', 'project_id', 'author_id',
+                                                'assignee_id', 'confidential')
 
     expected_hash['project'] = { "id" => project.id }
     expected_hash['author'] = { "id" => issue.author_id }
diff --git a/spec/lib/gitlab/elastic/search_results_spec.rb b/spec/lib/gitlab/elastic/search_results_spec.rb
index 180bde15bb9053..667a466dfdb0c7 100644
--- a/spec/lib/gitlab/elastic/search_results_spec.rb
+++ b/spec/lib/gitlab/elastic/search_results_spec.rb
@@ -103,6 +103,19 @@
     context 'search by term' do
       let(:query) { 'issue' }
 
+      it 'should not list confidential issues for guests' do
+        results = described_class.new(nil, limit_project_ids, query)
+        issues = results.objects('issues')
+
+        expect(issues).to include issue
+        expect(issues).not_to include security_issue_1
+        expect(issues).not_to include security_issue_2
+        expect(issues).not_to include security_issue_3
+        expect(issues).not_to include security_issue_4
+        expect(issues).not_to include security_issue_5
+        expect(results.issues_count).to eq 1
+      end
+
       it 'should not list confidential issues for non project members' do
         results = described_class.new(non_member, limit_project_ids, query)
         issues = results.objects('issues')
@@ -175,6 +188,19 @@
     context 'search by iid' do
       let(:query) { '#1' }
 
+      it 'should not list confidential issues for guests' do
+        results = described_class.new(nil, limit_project_ids, query)
+        issues = results.objects('issues')
+
+        expect(issues).to include issue
+        expect(issues).not_to include security_issue_1
+        expect(issues).not_to include security_issue_2
+        expect(issues).not_to include security_issue_3
+        expect(issues).not_to include security_issue_4
+        expect(issues).not_to include security_issue_5
+        expect(results.issues_count).to eq 1
+      end
+
       it 'should not list confidential issues for non project members' do
         results = described_class.new(non_member, limit_project_ids, query)
         issues = results.objects('issues')
@@ -257,8 +283,8 @@
         iid: 1
       )
       @merge_request_2 = create(
-        :merge_request, 
-        :conflict, 
+        :merge_request,
+        :conflict,
         source_project: project_1,
         target_project: project_1,
         title: 'Merge Request 2',
@@ -315,7 +341,6 @@
     end
   end
 
-
   describe 'project scoping' do
     before do
       [Project, MergeRequest, Issue, Milestone].each do |model|
@@ -356,7 +381,7 @@
         model.__elasticsearch__.refresh_index!
       end
 
-      result = Gitlab::Elastic::SearchResults.new([project.id], 'term')
+      result = Gitlab::Elastic::SearchResults.new(user, [project.id], 'term')
 
       expect(result.issues_count).to eq(2)
       expect(result.merge_requests_count).to eq(2)
-- 
GitLab


From 483808aec1b63889db73c42a632a79ce766e6fac Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 7 Mar 2016 21:25:41 -0300
Subject: [PATCH 20/30] Fix issues count on project view

---
 app/helpers/application_helper.rb        | 2 +-
 app/views/layouts/nav/_project.html.haml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 324fee6c8069d7..5206baa67417f3 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -288,7 +288,7 @@ def state_filters_text_for(entity, project)
       if project.nil?
         nil
       elsif current_controller?(:issues)
-        project.issues.send(entity).count
+        project.issues.available_for(current_user).send(entity).count
       elsif current_controller?(:merge_requests)
         project.merge_requests.send(entity).count
       end
diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml
index 0ae83ee01ebc6b..3aeafcabdc47bf 100644
--- a/app/views/layouts/nav/_project.html.haml
+++ b/app/views/layouts/nav/_project.html.haml
@@ -67,7 +67,7 @@
         %span
           Issues
           - if @project.default_issues_tracker?
-            %span.count.issue_counter= number_with_delimiter(@project.issues.opened.count)
+            %span.count.issue_counter= number_with_delimiter(@project.issues.available_for(current_user).opened.count)
 
   - if project_nav_tab? :merge_requests
     = nav_link(controller: :merge_requests) do
-- 
GitLab


From 254161c2fb49d051fadbc1ce4617992d22435cbf Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 7 Mar 2016 21:36:45 -0300
Subject: [PATCH 21/30] Move confidential issue checkbox below the description
 field

---
 app/views/shared/issuable/_form.html.haml | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 57f78461f06d6a..47c8a6425decfe 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -20,14 +20,6 @@
           Start the title with <code>[WIP]</code> or <code>WIP:</code> to prevent a
           <strong>Work In Progress</strong> merge request from being merged before it's ready.
 
-- if issuable.is_a?(Issue)
-  .form-group
-    .col-sm-offset-2.col-sm-10
-      .checkbox
-        = f.label :confidential do
-          = f.check_box :confidential
-          This issue is confidential and should only be visible to team members
-
 .form-group.detail-page-description
   = f.label :description, 'Description', class: 'control-label'
   .col-sm-10
@@ -38,6 +30,15 @@
       = render 'projects/notes/hints'
       .clearfix
       .error-alert
+
+- if issuable.is_a?(Issue)
+  .form-group
+    .col-sm-offset-2.col-sm-10
+      .checkbox
+        = f.label :confidential do
+          = f.check_box :confidential
+          This issue is confidential and should only be visible to team members
+
 - if can?(current_user, :"admin_#{issuable.to_ability_name}", issuable.project)
   %hr
   .form-group
-- 
GitLab


From 7d3c46e3aad55ff20108060bed3a3ef4c32f0e54 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 14 Mar 2016 16:45:43 -0300
Subject: [PATCH 22/30] Extract confidential icon to a helper

---
 app/helpers/issues_helper.rb               | 4 ++++
 app/views/projects/issues/_issue.html.haml | 4 +---
 app/views/projects/issues/show.html.haml   | 3 +--
 app/views/search/results/_issue.html.haml  | 3 +--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index ea6f1d5f6aefc4..1f0340549468f2 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -98,6 +98,10 @@ def merge_requests_sentence(merge_requests)
     end.sort.to_sentence(last_word_connector: ', or ')
   end
 
+  def confidential_icon(issue)
+    icon('eye-slash') if issue.confidential?
+  end
+
   def emoji_icon(name, unicode = nil, aliases = [])
     unicode ||= Emoji.emoji_filename(name) rescue ""
 
diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml
index 7f793e9e2e68d3..3a1e4d13b7c663 100644
--- a/app/views/projects/issues/_issue.html.haml
+++ b/app/views/projects/issues/_issue.html.haml
@@ -5,9 +5,7 @@
 
   .issue-title
     %span.issue-title-text
-      - if issue.confidential?
-        = icon('eye-slash')
-
+      = confidential_icon(issue)
       = link_to_gfm issue.title, issue_path(issue), class: "title"
     %ul.controls.light
       - if issue.closed?
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index f323cef1a7a43a..c6c6fe967b4dd5 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -22,8 +22,7 @@
       = icon('angle-double-left')
 
     .issue-meta
-      - if @issue.confidential
-        = icon('eye-slash')
+      = confidential_icon(@issue)
       %strong.identifier
         Issue ##{@issue.iid}
       %span.creator
diff --git a/app/views/search/results/_issue.html.haml b/app/views/search/results/_issue.html.haml
index d84f28bf40e866..710f5613c81723 100644
--- a/app/views/search/results/_issue.html.haml
+++ b/app/views/search/results/_issue.html.haml
@@ -1,7 +1,6 @@
 .search-result-row
   %h4
-    - if issue.confidential?
-      = icon('eye-slash')
+    = confidential_icon(issue)
     = link_to [issue.project.namespace.becomes(Namespace), issue.project, issue] do
       %span.term.str-truncated= issue.title
     .pull-right ##{issue.iid}
-- 
GitLab


From 7055c284dfffdf133662d90fd26e289e6b6d2fda Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 14 Mar 2016 16:50:47 -0300
Subject: [PATCH 23/30] Rename `Issue.available_for` to `Issue.visible_to_user`

---
 app/finders/issues_finder.rb                  | 2 +-
 app/helpers/application_helper.rb             | 2 +-
 app/models/issue.rb                           | 2 +-
 app/services/projects/autocomplete_service.rb | 2 +-
 app/views/layouts/nav/_group.html.haml        | 2 +-
 app/views/layouts/nav/_project.html.haml      | 2 +-
 lib/api/issues.rb                             | 2 +-
 lib/gitlab/search_results.rb                  | 2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb
index 0251b35d12abce..c2befa5a5b3a48 100644
--- a/app/finders/issues_finder.rb
+++ b/app/finders/issues_finder.rb
@@ -23,6 +23,6 @@ def klass
   private
 
   def init_collection
-    Issue.available_for(current_user)
+    Issue.visible_to_user(current_user)
   end
 end
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 5206baa67417f3..2c98cb69ad9454 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -288,7 +288,7 @@ def state_filters_text_for(entity, project)
       if project.nil?
         nil
       elsif current_controller?(:issues)
-        project.issues.available_for(current_user).send(entity).count
+        project.issues.visible_to_user(current_user).send(entity).count
       elsif current_controller?(:merge_requests)
         project.merge_requests.send(entity).count
       end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index b3ce266f7115c7..1142ca4ab79e0f 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -64,7 +64,7 @@ def hook_attrs
     attributes
   end
 
-  def self.available_for(user)
+  def self.visible_to_user(user)
     return where(confidential: false) if user.blank?
     return all if user.admin?
 
diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb
index a18debb80a8087..ba50305dbd5fca 100644
--- a/app/services/projects/autocomplete_service.rb
+++ b/app/services/projects/autocomplete_service.rb
@@ -1,7 +1,7 @@
 module Projects
   class AutocompleteService < BaseService
     def issues
-      @project.issues.available_for(current_user).opened.select([:iid, :title])
+      @project.issues.visible_to_user(current_user).opened.select([:iid, :title])
     end
 
     def merge_requests
diff --git a/app/views/layouts/nav/_group.html.haml b/app/views/layouts/nav/_group.html.haml
index a9152b2ab41513..7a90d4474cf009 100644
--- a/app/views/layouts/nav/_group.html.haml
+++ b/app/views/layouts/nav/_group.html.haml
@@ -30,7 +30,7 @@
         %span
           Issues
           - if current_user
-            %span.count= number_with_delimiter(Issue.opened.of_group(@group).count)
+            %span.count= number_with_delimiter(Issue.visible_to_user(current_user).opened.of_group(@group).count)
     = nav_link(path: 'groups#merge_requests') do
       = link_to merge_requests_group_path(@group), title: 'Merge Requests' do
         = icon('tasks fw')
diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml
index 3aeafcabdc47bf..86b46e8c75ec33 100644
--- a/app/views/layouts/nav/_project.html.haml
+++ b/app/views/layouts/nav/_project.html.haml
@@ -67,7 +67,7 @@
         %span
           Issues
           - if @project.default_issues_tracker?
-            %span.count.issue_counter= number_with_delimiter(@project.issues.available_for(current_user).opened.count)
+            %span.count.issue_counter= number_with_delimiter(@project.issues.visible_to_user(current_user).opened.count)
 
   - if project_nav_tab? :merge_requests
     = nav_link(controller: :merge_requests) do
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index d1cbd86741263a..fda6f8414384a7 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -82,7 +82,7 @@ def create_spam_log(project, current_user, attrs)
       #   GET /projects/:id/issues?milestone=1.0.0&state=closed
       #   GET /issues?iid=42
       get ":id/issues" do
-        issues = user_project.issues.available_for(current_user)
+        issues = user_project.issues.visible_to_user(current_user)
         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?
diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb
index 32f76570895d6c..bf7500e2dd2ea5 100644
--- a/lib/gitlab/search_results.rb
+++ b/lib/gitlab/search_results.rb
@@ -59,7 +59,7 @@ def projects
     end
 
     def issues
-      issues = Issue.available_for(user).where(project_id: project_ids_relation)
+      issues = Issue.visible_to_user(user).where(project_id: project_ids_relation)
 
       if query =~ /#(\d+)\z/
         issues = issues.where(iid: $1)
-- 
GitLab


From a888d1651fe5f965835d34ba86c2dc412cd6de5b Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 14 Mar 2016 18:56:10 -0300
Subject: [PATCH 24/30] Restrict access for confidential issues on milestone
 view

---
 app/helpers/milestones_helper.rb              |   2 +-
 app/models/concerns/milestoneish.rb           |  20 ++--
 app/models/milestone.rb                       |   4 +-
 app/views/projects/milestones/show.html.haml  |   2 +-
 .../shared/milestones/_issuable.html.haml     |   2 +
 .../shared/milestones/_milestone.html.haml    |   4 +-
 .../shared/milestones/_summary.html.haml      |   8 +-
 app/views/shared/milestones/_tabs.html.haml   |   4 +-
 app/views/shared/milestones/_top.html.haml    |   4 +-
 spec/models/concerns/milestoneish_spec.rb     | 104 ++++++++++++++++++
 spec/models/milestone_spec.rb                 |  20 ++--
 11 files changed, 142 insertions(+), 32 deletions(-)
 create mode 100644 spec/models/concerns/milestoneish_spec.rb

diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb
index e8ac8788d9dcc3..92ed0891e92bdf 100644
--- a/app/helpers/milestones_helper.rb
+++ b/app/helpers/milestones_helper.rb
@@ -38,7 +38,7 @@ def milestone_issues_by_label_count(milestone, label, state:)
   def milestone_progress_bar(milestone)
     options = {
       class: 'progress-bar progress-bar-success',
-      style: "width: #{milestone.percent_complete}%;"
+      style: "width: #{milestone.percent_complete(current_user)}%;"
     }
 
     content_tag :div, class: 'progress' do
diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index d67df7c1d9c193..5b8e3f654ea6be 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/app/models/concerns/milestoneish.rb
@@ -1,18 +1,18 @@
 module Milestoneish
-  def closed_items_count
-    issues.closed.size + merge_requests.closed_and_merged.size
+  def closed_items_count(user = nil)
+    issues_visible_to_user(user).closed.size + merge_requests.closed_and_merged.size
   end
 
-  def total_items_count
-    issues.size + merge_requests.size
+  def total_items_count(user = nil)
+    issues_visible_to_user(user).size + merge_requests.size
   end
 
-  def complete?
-    total_items_count == closed_items_count
+  def complete?(user = nil)
+    total_items_count(user) == closed_items_count(user)
   end
 
-  def percent_complete
-    ((closed_items_count * 100) / total_items_count).abs
+  def percent_complete(user = nil)
+    ((closed_items_count(user) * 100) / total_items_count(user)).abs
   rescue ZeroDivisionError
     0
   end
@@ -22,4 +22,8 @@ def remaining_days
 
     (due_date - Date.today).to_i
   end
+
+  def issues_visible_to_user(user = nil)
+    issues.visible_to_user(user)
+  end
 end
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index a15876bde2d62c..0ce1654d8f1568 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -122,8 +122,8 @@ def can_be_closed?
     active? && issues.opened.count.zero?
   end
 
-  def is_empty?
-    total_items_count.zero?
+  def is_empty?(user = nil)
+    total_items_count(user).zero?
   end
 
   def author_id
diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml
index b4597043a27af1..be63875ab34c0a 100644
--- a/app/views/projects/milestones/show.html.haml
+++ b/app/views/projects/milestones/show.html.haml
@@ -42,7 +42,7 @@
           = preserve do
             = markdown @milestone.description
 
-- if @milestone.complete? && @milestone.active?
+- if @milestone.complete?(current_user) && @milestone.active?
   .alert.alert-success.prepend-top-default
     %span All issues for this milestone are closed. You may close milestone now.
 
diff --git a/app/views/shared/milestones/_issuable.html.haml b/app/views/shared/milestones/_issuable.html.haml
index f7c6fc14adf196..85888096722f51 100644
--- a/app/views/shared/milestones/_issuable.html.haml
+++ b/app/views/shared/milestones/_issuable.html.haml
@@ -10,6 +10,8 @@
       %strong #{project.name} &middot;
     - elsif show_full_project_name
       %strong #{project.name_with_namespace} &middot;
+    - if issuable.is_a?(Issue)
+      = confidential_icon(issuable)
     = link_to_gfm issuable.title, [project.namespace.becomes(Namespace), project, issuable], title: issuable.title
   %div{class: 'issuable-detail'}
     = link_to [project.namespace.becomes(Namespace), project, issuable] do
diff --git a/app/views/shared/milestones/_milestone.html.haml b/app/views/shared/milestones/_milestone.html.haml
index f01138af3f09c2..6b25745c55420f 100644
--- a/app/views/shared/milestones/_milestone.html.haml
+++ b/app/views/shared/milestones/_milestone.html.haml
@@ -6,10 +6,10 @@
     .col-sm-6
       %strong= link_to_gfm truncate(milestone.title, length: 100), milestone_path
     .col-sm-6
-      .pull-right.light #{milestone.percent_complete}% complete
+      .pull-right.light #{milestone.percent_complete(current_user)}% complete
   .row
     .col-sm-6
-      = link_to pluralize(milestone.issues.size, 'Issue'), issues_path
+      = link_to pluralize(milestone.issues_visible_to_user(current_user).size, 'Issue'), issues_path
       &middot;
       = link_to pluralize(milestone.merge_requests.size, 'Merge Request'), merge_requests_path
     .col-sm-6= milestone_progress_bar(milestone)
diff --git a/app/views/shared/milestones/_summary.html.haml b/app/views/shared/milestones/_summary.html.haml
index 59d4ae29f791f3..385c6596606748 100644
--- a/app/views/shared/milestones/_summary.html.haml
+++ b/app/views/shared/milestones/_summary.html.haml
@@ -3,15 +3,15 @@
 .context.prepend-top-default
   .milestone-summary
     %h4 Progress
-    %strong= milestone.issues.size
+    %strong= milestone.issues_visible_to_user(current_user).size
     issues:
     %span.milestone-stat
-      %strong= milestone.issues.opened.size
+      %strong= milestone.issues_visible_to_user(current_user).opened.size
       open and
-      %strong= milestone.issues.closed.size
+      %strong= milestone.issues_visible_to_user(current_user).closed.size
       closed
     %span.milestone-stat
-      %strong== #{milestone.percent_complete}%
+      %strong== #{milestone.percent_complete(current_user)}%
       complete
 
     %span.milestone-stat
diff --git a/app/views/shared/milestones/_tabs.html.haml b/app/views/shared/milestones/_tabs.html.haml
index 57d7ee85a3b3ac..2b6ce2d7e7a38c 100644
--- a/app/views/shared/milestones/_tabs.html.haml
+++ b/app/views/shared/milestones/_tabs.html.haml
@@ -2,7 +2,7 @@
   %li.active
     = link_to '#tab-issues', 'data-toggle' => 'tab', 'data-show' => '.tab-issues-buttons' do
       Issues
-      %span.badge= milestone.issues.size
+      %span.badge= milestone.issues_visible_to_user(current_user).size
   %li
     = link_to '#tab-merge-requests', 'data-toggle' => 'tab', 'data-show' => '.tab-merge-requests-buttons' do
       Merge Requests
@@ -21,7 +21,7 @@
 
 .tab-content.milestone-content
   .tab-pane.active#tab-issues
-    = render 'shared/milestones/issues_tab', issues: milestone.issues, show_project_name: show_project_name, show_full_project_name: show_full_project_name
+    = render 'shared/milestones/issues_tab', issues: milestone.issues_visible_to_user(current_user), show_project_name: show_project_name, show_full_project_name: show_full_project_name
   .tab-pane#tab-merge-requests
     = render 'shared/milestones/merge_requests_tab', merge_requests: milestone.merge_requests, show_project_name: show_project_name, show_full_project_name: show_full_project_name
   .tab-pane#tab-participants
diff --git a/app/views/shared/milestones/_top.html.haml b/app/views/shared/milestones/_top.html.haml
index 4cf1d948b5b952..cab8743a0772dc 100644
--- a/app/views/shared/milestones/_top.html.haml
+++ b/app/views/shared/milestones/_top.html.haml
@@ -28,7 +28,7 @@
   %h2.title
     = markdown escape_once(milestone.title), pipeline: :single_line
 
-- if milestone.complete? && milestone.active?
+- if milestone.complete?(current_user) && milestone.active?
   .alert.alert-success.prepend-top-default
     - close_msg = group ? 'You may close the milestone now.' : 'Navigate to the project to close the milestone.'
     %span All issues for this milestone are closed. #{close_msg}
@@ -47,7 +47,7 @@
           - project_name = group ? ms.project.name : ms.project.name_with_namespace
           = link_to project_name, namespace_project_milestone_path(ms.project.namespace, ms.project, ms)
         %td
-          = ms.issues.opened.count
+          = ms.issues_visible_to_user(current_user).opened.count
         %td
           - if ms.closed?
             Closed
diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb
new file mode 100644
index 00000000000000..47c3be673c5681
--- /dev/null
+++ b/spec/models/concerns/milestoneish_spec.rb
@@ -0,0 +1,104 @@
+require 'spec_helper'
+
+describe Milestone, 'Milestoneish' do
+  let(:author) { create(:user) }
+  let(:assignee) { create(:user) }
+  let(:non_member) { create(:user) }
+  let(:member) { create(:user) }
+  let(:admin) { create(:admin) }
+  let(:project) { create(:project, :public) }
+  let(:milestone) { create(:milestone, project: project) }
+  let!(:issue) { create(:issue, project: project, milestone: milestone) }
+  let!(:security_issue_1) { create(:issue, :confidential, project: project, author: author, milestone: milestone) }
+  let!(:security_issue_2) { create(:issue, :confidential, project: project, assignee: assignee, milestone: milestone) }
+  let!(:closed_issue_1) { create(:issue, :closed, project: project, milestone: milestone) }
+  let!(:closed_issue_2) { create(:issue, :closed, project: project, milestone: milestone) }
+  let!(:closed_security_issue_1) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) }
+  let!(:closed_security_issue_2) { create(:issue, :confidential, :closed, project: project, assignee: assignee, milestone: milestone) }
+  let!(:closed_security_issue_3) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) }
+  let!(:closed_security_issue_4) { create(:issue, :confidential, :closed, project: project, assignee: assignee, milestone: milestone) }
+  let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) }
+
+  before do
+    project.team << [member, :developer]
+  end
+
+  describe '#closed_items_count' do
+    it 'should not count confidential issues for non project members' do
+      expect(milestone.closed_items_count(non_member)).to eq 2
+    end
+
+    it 'should count confidential issues for author' do
+      expect(milestone.closed_items_count(author)).to eq 4
+    end
+
+    it 'should count confidential issues for assignee' do
+      expect(milestone.closed_items_count(assignee)).to eq 4
+    end
+
+    it 'should count confidential issues for project members' do
+      expect(milestone.closed_items_count(member)).to eq 6
+    end
+
+    it 'should count all issues for admin' do
+      expect(milestone.closed_items_count(admin)).to eq 6
+    end
+  end
+
+  describe '#total_items_count' do
+    it 'should not count confidential issues for non project members' do
+      expect(milestone.total_items_count(non_member)).to eq 4
+    end
+
+    it 'should count confidential issues for author' do
+      expect(milestone.total_items_count(author)).to eq 7
+    end
+
+    it 'should count confidential issues for assignee' do
+      expect(milestone.total_items_count(assignee)).to eq 7
+    end
+
+    it 'should count confidential issues for project members' do
+      expect(milestone.total_items_count(member)).to eq 10
+    end
+
+    it 'should count all issues for admin' do
+      expect(milestone.total_items_count(admin)).to eq 10
+    end
+  end
+
+  describe '#complete?' do
+    it 'returns false when has items opened' do
+      expect(milestone.complete?(non_member)).to eq false
+    end
+
+    it 'returns true when all items are closed' do
+      issue.close
+      merge_request.close
+
+      expect(milestone.complete?(non_member)).to eq true
+    end
+  end
+
+  describe '#percent_complete' do
+    it 'should not count confidential issues for non project members' do
+      expect(milestone.percent_complete(non_member)).to eq 50
+    end
+
+    it 'should count confidential issues for author' do
+      expect(milestone.percent_complete(author)).to eq 57
+    end
+
+    it 'should count confidential issues for assignee' do
+      expect(milestone.percent_complete(assignee)).to eq 57
+    end
+
+    it 'should count confidential issues for project members' do
+      expect(milestone.percent_complete(member)).to eq 60
+    end
+
+    it 'should count confidential issues for admin' do
+      expect(milestone.percent_complete(admin)).to eq 60
+    end
+  end
+end
diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb
index de1757bf67a5f3..72a4ea702281ae 100644
--- a/spec/models/milestone_spec.rb
+++ b/spec/models/milestone_spec.rb
@@ -32,6 +32,7 @@
 
   let(:milestone) { create(:milestone) }
   let(:issue) { create(:issue) }
+  let(:user) { create(:user) }
 
   describe "unique milestone title per project" do
     it "shouldn't accept the same title in a project twice" do
@@ -50,18 +51,17 @@
   describe "#percent_complete" do
     it "should not count open issues" do
       milestone.issues << issue
-      expect(milestone.percent_complete).to eq(0)
+      expect(milestone.percent_complete(user)).to eq(0)
     end
 
     it "should count closed issues" do
       issue.close
       milestone.issues << issue
-      expect(milestone.percent_complete).to eq(100)
+      expect(milestone.percent_complete(user)).to eq(100)
     end
 
     it "should recover from dividing by zero" do
-      expect(milestone.issues).to receive(:size).and_return(0)
-      expect(milestone.percent_complete).to eq(0)
+      expect(milestone.percent_complete(user)).to eq(0)
     end
   end
 
@@ -103,7 +103,7 @@
       )
     end
 
-    it { expect(milestone.percent_complete).to eq(75) }
+    it { expect(milestone.percent_complete(user)).to eq(75) }
   end
 
   describe :items_count do
@@ -113,23 +113,23 @@
       milestone.merge_requests << create(:merge_request)
     end
 
-    it { expect(milestone.closed_items_count).to eq(1) }
-    it { expect(milestone.total_items_count).to eq(3) }
-    it { expect(milestone.is_empty?).to be_falsey }
+    it { expect(milestone.closed_items_count(user)).to eq(1) }
+    it { expect(milestone.total_items_count(user)).to eq(3) }
+    it { expect(milestone.is_empty?(user)).to be_falsey }
   end
 
   describe :can_be_closed? do
     it { expect(milestone.can_be_closed?).to be_truthy }
   end
 
-  describe :is_empty? do
+  describe :total_items_count do
     before do
       create :closed_issue, milestone: milestone
       create :merge_request, milestone: milestone
     end
 
     it 'Should return total count of issues and merge requests assigned to milestone' do
-      expect(milestone.total_items_count).to eq 2
+      expect(milestone.total_items_count(user)).to eq 2
     end
   end
 
-- 
GitLab


From 43c76d4a3fe00b0c0190fefa8ef8051c47afce66 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 14 Mar 2016 20:42:15 -0300
Subject: [PATCH 25/30] Use sub-query instead of pluck when retrive issues
 visible to user

---
 app/models/issue.rb | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 1142ca4ab79e0f..0cb16f97f2b365 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -68,20 +68,7 @@ def self.visible_to_user(user)
     return where(confidential: false) if user.blank?
     return all if user.admin?
 
-    issues_table = self.arel_table
-    project_ids  = user.authorized_projects.pluck(:id)
-
-    where(
-      issues_table[:confidential].eq(false).or(
-        issues_table[:confidential].eq(true).and(
-          issues_table[:author_id].eq(user.id).or(
-            issues_table[:assignee_id].eq(user.id).or(
-              issues_table[:project_id].in(project_ids)
-            )
-          )
-        )
-      )
-    )
+    where('issues.confidential = 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.select(:id))
   end
 
   def self.reference_prefix
-- 
GitLab


From fc47425808da035ce81bd2cc857cfc76e7932429 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Mon, 14 Mar 2016 21:40:22 -0300
Subject: [PATCH 26/30] Remove the `admin_issue` role when user can't access
 confidential issue

---
 app/models/ability.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/models/ability.rb b/app/models/ability.rb
index 9e568c74614e5a..357d48168da6c2 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -469,6 +469,7 @@ def filter_confidential_issues_abilities(user, issue, rules)
       return rules if user.admin? || !issue.confidential?
 
       unless issue.author == user || issue.assignee == user || issue.project.team.member?(user.id)
+        rules.delete(:admin_issue)
         rules.delete(:read_issue)
         rules.delete(:update_issue)
       end
-- 
GitLab


From 8b8cb00befd3986c9a6f63ba3b7bcd3982165a58 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Tue, 15 Mar 2016 16:36:28 -0300
Subject: [PATCH 27/30] Restrict access to confidential issues on activity feed

---
 app/helpers/events_helper.rb      |  2 +-
 app/models/event.rb               |  6 ++++--
 app/views/events/_event.html.haml |  2 +-
 features/steps/groups.rb          |  2 +-
 spec/models/event_spec.rb         | 36 +++++++++++++++++++++++++++++++
 5 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb
index 37a888d9c60bdd..a67a6b208e256b 100644
--- a/app/helpers/events_helper.rb
+++ b/app/helpers/events_helper.rb
@@ -194,7 +194,7 @@ def event_commit_title(message)
   end
 
   def event_to_atom(xml, event)
-    if event.proper?
+    if event.proper?(current_user)
       xml.entry do
         event_link = event_feed_url(event)
         event_title = event_feed_title(event)
diff --git a/app/models/event.rb b/app/models/event.rb
index 67b75ea251d9dc..59057983f216dc 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -79,15 +79,17 @@ def limit_recent(limit = 20, offset = nil)
     end
   end
 
-  def proper?
+  def proper?(user = nil)
     if push?
       true
     elsif membership_changed?
       true
     elsif created_project?
       true
+    elsif issue?
+      Ability.abilities.allowed?(user, :read_issue, issue)
     else
-      ((issue? || merge_request? || note?) && target) || milestone?
+      ((merge_request? || note?) && target) || milestone?
     end
   end
 
diff --git a/app/views/events/_event.html.haml b/app/views/events/_event.html.haml
index 36fb2d5162955f..2d9d9dd634243e 100644
--- a/app/views/events/_event.html.haml
+++ b/app/views/events/_event.html.haml
@@ -1,4 +1,4 @@
-- if event.proper?
+- if event.proper?(current_user)
   .event-item{class: "#{event.body? ? "event-block" : "event-inline" }"}
     .event-item-timestamp
       #{time_ago_with_tooltip(event.created_at)}
diff --git a/features/steps/groups.rb b/features/steps/groups.rb
index 5df6401cee2625..dd9e4dce23df59 100644
--- a/features/steps/groups.rb
+++ b/features/steps/groups.rb
@@ -39,7 +39,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
   end
 
   step 'I should see projects activity feed' do
-    expect(page).to have_content 'closed issue'
+    expect(page).to have_content 'joined project'
   end
 
   step 'I should see issues from group "Owned" assigned to me' do
diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb
index ec2a923f91bb4a..5fe442467385f9 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -65,6 +65,42 @@
     it { expect(@event.author).to eq(@user) }
   end
 
+  describe '#proper?' do
+    context 'issue event' do
+      let(:project) { create(:empty_project, :public) }
+      let(:non_member) { create(:user) }
+      let(:member)  { create(:user) }
+      let(:author) { create(:author) }
+      let(:assignee) { create(:user) }
+      let(:admin) { create(:admin) }
+      let(:event) { Event.new(project: project, action: Event::CREATED, target: issue, author_id: author.id) }
+
+      before do
+        project.team << [member, :developer]
+      end
+
+      context 'for non confidential issues' do
+        let(:issue) { create(:issue, project: project, author: author, assignee: assignee) }
+
+        it { expect(event.proper?(non_member)).to eq true }
+        it { expect(event.proper?(author)).to eq true }
+        it { expect(event.proper?(assignee)).to eq true }
+        it { expect(event.proper?(member)).to eq true }
+        it { expect(event.proper?(admin)).to eq true }
+      end
+
+      context 'for confidential issues' do
+        let(:issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) }
+
+        it { expect(event.proper?(non_member)).to eq false }
+        it { expect(event.proper?(author)).to eq true }
+        it { expect(event.proper?(assignee)).to eq true }
+        it { expect(event.proper?(member)).to eq true }
+        it { expect(event.proper?(admin)).to eq true }
+      end
+    end
+  end
+
   describe '.limit_recent' do
     let!(:event1) { create(:closed_issue_event) }
     let!(:event2) { create(:closed_issue_event) }
-- 
GitLab


From 978ae22288da4746566b8a14641a916f4cebca4c Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Tue, 15 Mar 2016 14:33:22 -0300
Subject: [PATCH 28/30] Use ability to check access to references for
 confidential issues

---
 lib/banzai/filter/issue_reference_filter.rb   | 22 ++-----------------
 .../lib/banzai/filter/redactor_filter_spec.rb | 11 ++++++++++
 .../gitlab/closing_issue_extractor_spec.rb    |  1 +
 spec/lib/gitlab/reference_extractor_spec.rb   |  2 ++
 spec/models/commit_spec.rb                    | 13 ++++++++++-
 spec/models/concerns/mentionable_spec.rb      |  5 +++--
 spec/models/merge_request_spec.rb             |  1 +
 spec/services/git_push_service_spec.rb        |  4 ++++
 spec/support/mentionable_shared_examples.rb   |  2 ++
 9 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/lib/banzai/filter/issue_reference_filter.rb b/lib/banzai/filter/issue_reference_filter.rb
index a655585e02ea76..2732e0b51455ca 100644
--- a/lib/banzai/filter/issue_reference_filter.rb
+++ b/lib/banzai/filter/issue_reference_filter.rb
@@ -10,26 +10,8 @@ def self.object_class
       end
 
       def self.user_can_see_reference?(user, node, context)
-        project = Project.find(node.attr('data-project')) rescue nil
-        return unless project
-
-        id = node.attr('data-issue')
-        issue = find_object(project, id)
-        return unless issue
-
-        if issue.is_a?(Issue) && issue.confidential?
-          Ability.abilities.allowed?(user, :read_issue, issue)
-        else
-          super
-        end
-      end
-
-      def self.find_object(project, id)
-        if project.default_issues_tracker?
-          project.issues.find_by(id: id)
-        else
-          ExternalIssue.new(id, project)
-        end
+        issue = Issue.find(node.attr('data-issue')) rescue nil
+        Ability.abilities.allowed?(user, :read_issue, issue)
       end
 
       def find_object(project, id)
diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb
index 7a9009e9eb80c2..9acf6304bcb19f 100644
--- a/spec/lib/banzai/filter/redactor_filter_spec.rb
+++ b/spec/lib/banzai/filter/redactor_filter_spec.rb
@@ -90,6 +90,17 @@ def reference_link(data)
 
         expect(doc.css('a').length).to eq 1
       end
+
+      it 'allows references for admin' do
+        admin = create(:admin)
+        project = create(:empty_project, :public)
+        issue = create(:issue, :confidential, project: project)
+
+        link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter')
+        doc = filter(link, current_user: admin)
+
+        expect(doc.css('a').length).to eq 1
+      end
     end
 
     it 'allows references for non confidential issues' do
diff --git a/spec/lib/gitlab/closing_issue_extractor_spec.rb b/spec/lib/gitlab/closing_issue_extractor_spec.rb
index 04cf11fc6f1752..844fd79c991af8 100644
--- a/spec/lib/gitlab/closing_issue_extractor_spec.rb
+++ b/spec/lib/gitlab/closing_issue_extractor_spec.rb
@@ -11,6 +11,7 @@
   subject { described_class.new(project, project.creator) }
 
   before do
+    project.team  << [project.creator, :developer]
     project2.team << [project.creator, :master]
   end
 
diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb
index 7d963795e17b48..65af37e24f14e6 100644
--- a/spec/lib/gitlab/reference_extractor_spec.rb
+++ b/spec/lib/gitlab/reference_extractor_spec.rb
@@ -2,6 +2,7 @@
 
 describe Gitlab::ReferenceExtractor, lib: true do
   let(:project) { create(:project) }
+
   subject { Gitlab::ReferenceExtractor.new(project, project.creator) }
 
   it 'accesses valid user objects' do
@@ -41,6 +42,7 @@
   end
 
   it 'accesses valid issue objects' do
+    project.team << [project.creator, :developer]
     @i0 = create(:issue, project: project)
     @i1 = create(:issue, project: project)
 
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 253902512c337b..0e9111c8029332 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -86,10 +86,21 @@
     let(:issue) { create :issue, project: project }
     let(:other_project) { create :project, :public }
     let(:other_issue) { create :issue, project: other_project }
+    let(:commiter) { create :user }
+
+    before do
+      project.team << [commiter, :developer]
+      other_project.team << [commiter, :developer]
+    end
 
     it 'detects issues that this commit is marked as closing' do
       ext_ref = "#{other_project.path_with_namespace}##{other_issue.iid}"
-      allow(commit).to receive(:safe_message).and_return("Fixes ##{issue.iid} and #{ext_ref}")
+
+      allow(commit).to receive_messages(
+        safe_message: "Fixes ##{issue.iid} and #{ext_ref}",
+        committer_email: commiter.email
+      )
+
       expect(commit.closes_issues).to include(issue)
       expect(commit.closes_issues).to include(other_issue)
     end
diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb
index 20f0c561e44b16..cb33edde820d22 100644
--- a/spec/models/concerns/mentionable_spec.rb
+++ b/spec/models/concerns/mentionable_spec.rb
@@ -48,7 +48,8 @@ def author
 
   describe '#create_new_cross_references!' do
     let(:project) { create(:project) }
-    let(:issues)  { create_list(:issue, 2, project: project) }
+    let(:author)  { create(:author) }
+    let(:issues)  { create_list(:issue, 2, project: project, author: author) }
 
     context 'before changes are persisted' do
       it 'ignores pre-existing references' do
@@ -91,7 +92,7 @@ def author
     end
 
     def create_issue(description:)
-      create(:issue, project: project, description: description)
+      create(:issue, project: project, description: description, author: author)
     end
   end
 end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 879b0d5e717ee1..001063d8f5ef4d 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -150,6 +150,7 @@
     let(:commit2) { double('commit2', safe_message: "Fixes #{issue1.to_reference}") }
 
     before do
+      subject.project.team << [subject.author, :developer]
       allow(subject).to receive(:commits).and_return([commit0, commit1, commit2])
     end
 
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index d8ad5e689383f2..5fc85600ff953e 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -228,12 +228,16 @@
     let(:commit) { project.commit }
 
     before do
+      project.team << [commit_author, :developer]
+      project.team << [user, :developer]
+
       allow(commit).to receive_messages(
         safe_message: "this commit \n mentions #{issue.to_reference}",
         references: [issue],
         author_name: commit_author.name,
         author_email: commit_author.email
       )
+
       allow(project.repository).to receive(:commits_between).and_return([commit])
     end
 
diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb
index fce91015fd4b9c..e876d44c166dc3 100644
--- a/spec/support/mentionable_shared_examples.rb
+++ b/spec/support/mentionable_shared_examples.rb
@@ -52,6 +52,8 @@
     end
 
     set_mentionable_text.call(ref_string)
+
+    project.team << [author, :developer]
   end
 end
 
-- 
GitLab


From 6e129d0205b9ee51b1c45c1f30394ecf91bcc631 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 17 Mar 2016 15:44:21 -0300
Subject: [PATCH 29/30] Rename user attribute to current_user on search results

---
 lib/gitlab/project_search_results.rb | 4 ++--
 lib/gitlab/search_results.rb         | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb
index 7edd8f9ff56682..71c5b6801fb66e 100644
--- a/lib/gitlab/project_search_results.rb
+++ b/lib/gitlab/project_search_results.rb
@@ -2,8 +2,8 @@ module Gitlab
   class ProjectSearchResults < SearchResults
     attr_reader :project, :repository_ref
 
-    def initialize(user, project, query, repository_ref = nil)
-      @user = user
+    def initialize(current_user, project, query, repository_ref = nil)
+      @current_user = current_user
       @project = project
       @repository_ref = if repository_ref.present?
                           repository_ref
diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb
index bf7500e2dd2ea5..f8ab2b1f09ec0a 100644
--- a/lib/gitlab/search_results.rb
+++ b/lib/gitlab/search_results.rb
@@ -1,13 +1,13 @@
 module Gitlab
   class SearchResults
-    attr_reader :user, :query
+    attr_reader :current_user, :query
 
     # Limit search results by passed projects
     # It allows us to search only for projects user has access to
     attr_reader :limit_projects
 
-    def initialize(user, limit_projects, query)
-      @user = user
+    def initialize(current_user, limit_projects, query)
+      @current_user = current_user
       @limit_projects = limit_projects || Project.all
       @query = Shellwords.shellescape(query) if query.present?
     end
@@ -59,7 +59,7 @@ def projects
     end
 
     def issues
-      issues = Issue.visible_to_user(user).where(project_id: project_ids_relation)
+      issues = Issue.visible_to_user(current_user).where(project_id: project_ids_relation)
 
       if query =~ /#(\d+)\z/
         issues = issues.where(iid: $1)
-- 
GitLab


From 79b61d09457d822bea3c2713f508bbeb4470bac9 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Thu, 17 Mar 2016 15:53:39 -0300
Subject: [PATCH 30/30] Hide the checkbox to make an issue confidential on
 private projects

---
 app/views/shared/issuable/_form.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 47c8a6425decfe..2955ad22a8dec9 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -31,7 +31,7 @@
       .clearfix
       .error-alert
 
-- if issuable.is_a?(Issue)
+- if issuable.is_a?(Issue) && !issuable.project.private?
   .form-group
     .col-sm-offset-2.col-sm-10
       .checkbox
-- 
GitLab