From c1cc4777caad078e3eff9ba6170beb7ee7254917 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= <ruben@gitlab.com>
Date: Tue, 12 Jun 2018 10:02:06 -0500
Subject: [PATCH] Hide events from internal projects in public feed for
 anonymous users

This change fixes a bug where an anonymous user was able to see the
activity related to internal projects when visiting the public profile
of a user of the GitLab instance.
---
 app/finders/user_recent_events_finder.rb      |  2 +-
 ...-not-show-internal-info-in-public-feed.yml |  5 +++
 .../finders/user_recent_events_finder_spec.rb | 45 +++++++++++++------
 3 files changed, 38 insertions(+), 14 deletions(-)
 create mode 100644 changelogs/unreleased/security-rd-do-not-show-internal-info-in-public-feed.yml

diff --git a/app/finders/user_recent_events_finder.rb b/app/finders/user_recent_events_finder.rb
index 65d6e019746..74776b2ed1f 100644
--- a/app/finders/user_recent_events_finder.rb
+++ b/app/finders/user_recent_events_finder.rb
@@ -56,7 +56,7 @@ class UserRecentEventsFinder
 
     visible = target_user
       .project_interactions
-      .where(visibility_level: [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC])
+      .where(visibility_level: Gitlab::VisibilityLevel.levels_for_user(current_user))
       .select(:id)
 
     Gitlab::SQL::Union.new([authorized, visible]).to_sql
diff --git a/changelogs/unreleased/security-rd-do-not-show-internal-info-in-public-feed.yml b/changelogs/unreleased/security-rd-do-not-show-internal-info-in-public-feed.yml
new file mode 100644
index 00000000000..ff78c162dff
--- /dev/null
+++ b/changelogs/unreleased/security-rd-do-not-show-internal-info-in-public-feed.yml
@@ -0,0 +1,5 @@
+---
+title: Don't show events from internal projects for anonymous users in public feed
+merge_request:
+author:
+type: security
diff --git a/spec/finders/user_recent_events_finder_spec.rb b/spec/finders/user_recent_events_finder_spec.rb
index 3ca0f7c3c89..da043f94021 100644
--- a/spec/finders/user_recent_events_finder_spec.rb
+++ b/spec/finders/user_recent_events_finder_spec.rb
@@ -1,31 +1,50 @@
 require 'spec_helper'
 
 describe UserRecentEventsFinder do
-  let(:user) { create(:user) }
-  let(:project) { create(:project) }
-  let(:project_owner) { project.creator }
-  let!(:event) { create(:event, project: project, author: project_owner) }
+  let(:current_user)     { create(:user) }
+  let(:project_owner)    { create(:user) }
+  let(:private_project)  { create(:project, :private, creator: project_owner) }
+  let(:internal_project) { create(:project, :internal, creator: project_owner) }
+  let(:public_project)   { create(:project, :public, creator: project_owner) }
+  let!(:private_event)   { create(:event, project: private_project, author: project_owner) }
+  let!(:internal_event)  { create(:event, project: internal_project, author: project_owner) }
+  let!(:public_event)    { create(:event, project: public_project, author: project_owner) }
 
-  subject(:finder) { described_class.new(user, project_owner) }
+  subject(:finder) { described_class.new(current_user, project_owner) }
 
   describe '#execute' do
-    it 'does not include the event when a user does not have access to the project' do
-      expect(finder.execute).to be_empty
+    context 'current user does not have access to projects' do
+      it 'returns public and internal events' do
+        records = finder.execute
+
+        expect(records).to include(public_event, internal_event)
+        expect(records).not_to include(private_event)
+      end
     end
 
-    context 'when the user has access to a project' do
+    context 'when current user has access to the projects' do
       before do
-        project.add_developer(user)
+        private_project.add_developer(current_user)
+        internal_project.add_developer(current_user)
+        public_project.add_developer(current_user)
       end
 
-      it 'includes the event' do
-        expect(finder.execute).to include(event)
+      it 'returns all the events' do
+        expect(finder.execute).to include(private_event, internal_event, public_event)
       end
 
-      it 'does not include the event if the user cannot read cross project' do
-        expect(Ability).to receive(:allowed?).with(user, :read_cross_project) { false }
+      it 'does not include the events if the user cannot read cross project' do
+        expect(Ability).to receive(:allowed?).with(current_user, :read_cross_project) { false }
         expect(finder.execute).to be_empty
       end
     end
+
+    context 'when current user is anonymous' do
+      let(:current_user) { nil }
+
+      it 'returns public events only' do
+        expect(finder.execute).to eq([public_event])
+      end
+    end
   end
 end