Restrict starred projects to viewable ones

`User#starred_projects` doesn't perform any visibility checks. This has
a couple of problems:

1. It assumes a user can always view all of their starred projects in
   perpetuity (project not changed to private, access revoked, etc.).
2. It assumes that we'll only ever allow a user to star a project they
   can view. This is currently the case, but bugs happen.

Add `User#viewable_starred_projects` to filter the starred projects by
those the user either has explicit access to, or are public or
internal. Then use that in all places where we list the user's starred
projects.

1 files changed, 24 insertions, 0 deletions

diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 26d4e139396..06d1ca3b7da 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -233,6 +233,8 @@ describe User, models: true do
       @project = create :project, namespace: @user.namespace
       @project_2 = create :project, group: create(:group) # Grant MASTER access to the user
       @project_3 = create :project, group: create(:group) # Grant DEVELOPER access to the user
+      @project_4 = create :project, group: create(:group)
+      @project_5 = create :project, group: create(:group)
 
       @project_2.team << [@user, :master]
       @project_3.team << [@user, :developer]
@@ -782,4 +784,26 @@ describe User, models: true do
 
     it { is_expected.to eq([private_project]) }
   end
+
+  describe '#viewable_starred_projects' do
+    let(:user) { create(:user) }
+    let(:public_project) { create(:project, :public) }
+    let(:private_project) { create(:project, :private) }
+    let(:private_viewable_project) { create(:project, :private) }
+    let(:viewable?) { -> (project) { user.can?(:read_project, project) } }
+    let(:projects) { [public_project, private_project, private_viewable_project] }
+
+    before do
+      private_viewable_project.team << [user, Gitlab::Access::MASTER]
+      projects.each { |project| user.toggle_star(project) }
+    end
+
+    it 'returns only starred projects the user can view' do
+      expect(user.viewable_starred_projects).to all(satisfy(&viewable?))
+    end
+
+    it 'rejects only starred projects the user can not view' do
+      expect(projects - user.viewable_starred_projects).not_to include(satisfy(&viewable?))
+    end
+  end
 end