Restrict access to confidential issues on activity feed

5 files changed, 43 insertions, 5 deletions

diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb
index 37a888d9c60..a67a6b208e2 100644
--- a/app/helpers/events_helper.rb
+++ b/app/helpers/events_helper.rb
@@ -194,7 +194,7 @@ module EventsHelper
   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 9a0bbf50f8b..a5cfeaf388e 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -73,15 +73,17 @@ class Event < ActiveRecord::Base
     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 36fb2d51629..2d9d9dd6342 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 7a6ae15ffa5..e5b7db4c5e3 100644
--- a/features/steps/groups.rb
+++ b/features/steps/groups.rb
@@ -35,7 +35,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 ec2a923f91b..5fe44246738 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -65,6 +65,42 @@ describe Event, models: true do
     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) }