Allow acces to confidential issues to user set as the assignee

app/models/ability.rb

@@ -43,8 +43,6 @@ class Ability
         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 @@ class Ability
       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 @@ class Ability
         rules = [
           :read_project,
           :read_wiki,
-          :read_issue,
           :read_label,
           :read_milestone,
           :read_project_snippet,
@@ -85,6 +76,9 @@ class Ability
         # 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 @@ class Ability
         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 @@ class Ability
       ]
     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

app/models/issue.rb

@@ -76,7 +76,9 @@ class Issue < ActiveRecord::Base
       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)
+            )
           )
         )
       )

lib/elastic/issues_search.rb

@@ -19,6 +19,7 @@ module Elastic
 
         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 @@ module Elastic
           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

lib/gitlab/elastic/project_search_results.rb

@@ -107,25 +107,6 @@ module Gitlab
         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

lib/gitlab/elastic/search_results.rb

@@ -64,16 +64,10 @@ module Gitlab
 
       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
 

spec/controllers/projects/issues_controller_spec.rb

@@ -42,13 +42,14 @@ describe Projects::IssuesController do
 
   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 @@ describe Projects::IssuesController do
         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 @@ describe Projects::IssuesController do
         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]

spec/lib/banzai/filter/redactor_filter_spec.rb

@@ -68,6 +68,17 @@ describe Banzai::Filter::RedactorFilter, lib: true do
         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)

spec/lib/gitlab/elastic/project_search_results_spec.rb

@@ -68,12 +68,13 @@ describe Gitlab::Elastic::ProjectSearchResults, lib: true do
   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 @@ describe Gitlab::Elastic::ProjectSearchResults, lib: true do
       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]
 

spec/lib/gitlab/elastic/search_results_spec.rb

@@ -86,14 +86,15 @@ describe Gitlab::Elastic::SearchResults, lib: true do
     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 @@ describe Gitlab::Elastic::SearchResults, lib: true do
       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]

spec/lib/gitlab/project_search_results_spec.rb

@@ -25,12 +25,13 @@ describe Gitlab::ProjectSearchResults, lib: true do
   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 @@ describe Gitlab::ProjectSearchResults, lib: true do
       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]
 

spec/lib/gitlab/search_results_spec.rb

@@ -62,14 +62,15 @@ describe Gitlab::SearchResults do
     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 @@ describe Gitlab::SearchResults do
       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]

spec/requests/api/issues_spec.rb

@@ -5,6 +5,7 @@ describe API::API, api: true  do
   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 @@ describe API::API, api: true  do
     create :issue,
            :confidential,
            project: project,
-           author: author
+           author: author,
+           assignee: assignee
   end
   let!(:issue) do
     create :issue,
@@ -148,6 +150,14 @@ describe API::API, api: true  do
       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 @@ describe API::API, api: true  do
         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)