Project members with guest role can't access confidential issues

7 files changed, 39 insertions, 14 deletions

diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb
index 01cbf91c658..00ff1611039 100644
--- a/app/finders/snippets_finder.rb
+++ b/app/finders/snippets_finder.rb
@@ -51,7 +51,7 @@ class SnippetsFinder
     snippets = project.snippets.fresh
 
     if current_user
-      if project.team.member?(current_user.id) || current_user.admin?
+      if project.team.member?(current_user) || current_user.admin?
         snippets
       else
         snippets.public_and_internal
diff --git a/app/models/ability.rb b/app/models/ability.rb
index 44515550d9e..aea946f9224 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -533,7 +533,7 @@ class Ability
     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)
+      unless issue.author == user || issue.assignee == user || issue.project.team.member?(user, Gitlab::Access::REPORTER)
         rules.delete(:admin_issue)
         rules.delete(:read_issue)
         rules.delete(:update_issue)
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 235922710ad..6ecb3535359 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -54,7 +54,15 @@ class Issue < ActiveRecord::Base
     return where(confidential: false) if user.blank?
     return all if user.admin?
 
-    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))
+    where('
+      issues.confidential IS NULL
+      OR issues.confidential IS 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(Gitlab::Access::REPORTER).select(:id))
   end
 
   def self.reference_prefix
diff --git a/app/models/note.rb b/app/models/note.rb
index 585d8c4ad84..8ce2b6fa538 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -100,7 +100,7 @@ class Note < ActiveRecord::Base
             OR issues.assignee_id = :user_id
             OR issues.project_id IN(:project_ids)))',
           user_id: as_user.id,
-          project_ids: as_user.authorized_projects.select(:id))
+          project_ids: as_user.authorized_projects(Gitlab::Access::REPORTER).select(:id))
       else
         found_notes.where('issues.confidential IS NULL OR issues.confidential IS FALSE')
       end
diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index 70a8bbaba65..e29e854860a 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -131,8 +131,14 @@ class ProjectTeam
     max_member_access(user.id) == Gitlab::Access::MASTER
   end
 
-  def member?(user_id)
-    !!find_member(user_id)
+  def member?(user, min_member_access = nil)
+    member = !!find_member(user.id)
+
+    if min_member_access
+      member && max_member_access(user.id) >= min_member_access
+    else
+      member
+    end
   end
 
   def human_max_access(user_id)
diff --git a/app/models/user.rb b/app/models/user.rb
index 7afbfbf112a..69c1bf4bc3d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -405,8 +405,8 @@ class User < ActiveRecord::Base
   end
 
   # Returns projects user is authorized to access.
-  def authorized_projects
-    Project.where("projects.id IN (#{projects_union.to_sql})")
+  def authorized_projects(min_access_level = nil)
+    Project.where("projects.id IN (#{projects_union(min_access_level).to_sql})")
   end
 
   def viewable_starred_projects
@@ -824,11 +824,22 @@ class User < ActiveRecord::Base
 
   private
 
-  def projects_union
-    Gitlab::SQL::Union.new([personal_projects.select(:id),
-                            groups_projects.select(:id),
-                            projects.select(:id),
-                            groups.joins(:shared_projects).select(:project_id)])
+  def projects_union(min_access_level = nil)
+    relations = if min_access_level
+                  scope = { access_level: Gitlab::Access.values.select { |access| access >= min_access_level } }
+
+                  [personal_projects.select(:id),
+                   groups_projects.where(members: scope).select(:id),
+                   projects.where(members: scope).select(:id),
+                   groups.joins(:shared_projects).where(members: scope).select(:project_id)]
+                else
+                  [personal_projects.select(:id),
+                   groups_projects.select(:id),
+                   projects.select(:id),
+                   groups.joins(:shared_projects).select(:project_id)]
+                end
+
+    Gitlab::SQL::Union.new(relations)
   end
 
   def ci_projects_union
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index d503026f913..c30bdb0ae91 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -41,7 +41,7 @@
       .checkbox
         = f.label :confidential do
           = f.check_box :confidential
-          This issue is confidential and should only be visible to team members
+          This issue is confidential and should only be visible to team members with at least Reporter access.
 
 - if can?(current_user, :"admin_#{issuable.to_ability_name}", issuable.project)
   - has_due_date = issuable.has_attribute?(:due_date)