Merge branch 'fix-milestone-browse-issues-permission-check' into 'master'

Refactor permission checks with issues and merge requests project settings

Slight refinement for read/write permission checks for 04d44522 as discussed in !691. Currently it appears that there is [no role that only has read-only access to issues](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/permissions/permissions.md), but it could be possible.

/cc: @DouweM

See merge request !704

4 files changed, 21 insertions, 6 deletions

diff --git a/CHANGELOG b/CHANGELOG
index aebebd8bfd9..a168342fceb 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 7.12.0 (unreleased)
+  - Refactor permission checks with issues and merge requests project settings (Stan Hu)
   - Add web hook support for note events (Stan Hu)
   - Disable "New Issue" and "New Merge Request" buttons when features are disabled in project settings (Stan Hu)
   - Remove Rack Attack monkey patches and bump to version 4.3.0 (Stan Hu)
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 96d2606f1a1..f8df39d236a 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -148,7 +148,7 @@ module ProjectsHelper
       nav_tabs << [:files, :commits, :network, :graphs]
     end
 
-    if project.repo_exists? && project.merge_requests_enabled
+    if project.repo_exists? && can?(current_user, :read_merge_request, project)
       nav_tabs << :merge_requests
     end
 
@@ -156,11 +156,19 @@ module ProjectsHelper
       nav_tabs << :settings
     end
 
-    [:issues, :wiki, :snippets].each do |feature|
-      nav_tabs << feature if project.send :"#{feature}_enabled"
+    if can?(current_user, :read_issue, project)
+      nav_tabs << :issues
     end
 
-    if project.issues_enabled || project.merge_requests_enabled
+    if can?(current_user, :read_wiki, project)
+      nav_tabs << :wiki
+    end
+
+    if can?(current_user, :read_project_snippet, project)
+      nav_tabs << :snippets
+    end
+
+    if can?(current_user, :read_milestone, project)
       nav_tabs << [:milestones, :labels]
     end
 
diff --git a/app/models/ability.rb b/app/models/ability.rb
index 04d9dccf916..e166b4197fd 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -109,6 +109,11 @@ class Ability
           rules -= named_abilities('merge_request')
         end
 
+        unless project.issues_enabled or project.merge_requests_enabled
+          rules -= named_abilities('label')
+          rules -= named_abilities('milestone')
+        end
+
         unless project.snippets_enabled
           rules -= named_abilities('snippet')
         end
diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml
index 22172a31289..5845fd744f4 100644
--- a/app/views/projects/milestones/show.html.haml
+++ b/app/views/projects/milestones/show.html.haml
@@ -61,11 +61,12 @@
       Participants
       %span.badge= @users.count
 
-  - if can?(current_user, :write_issue, @project)
-    .pull-right
+  .pull-right
+    - if can?(current_user, :write_issue, @project)
       = link_to new_namespace_project_issue_path(@project.namespace, @project, issue: { milestone_id: @milestone.id }), class: "btn  btn-grouped", title: "New Issue" do
         %i.fa.fa-plus
         New Issue
+    - if can?(current_user, :read_issue, @project)
       = link_to 'Browse Issues', namespace_project_issues_path(@milestone.project.namespace, @milestone.project, milestone_id: @milestone.id), class: "btn  edit-milestone-link btn-grouped"
 
 .tab-content