From 8cbfeddeb011f3731152b7b2512ec1beba62b919 Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sun, 8 May 2016 06:48:36 +0000
Subject: [PATCH] Child nodes should only be rendered if the user is actually
 authorized to see them (#15880).

Patch by Jan Schulz-Hofen.

git-svn-id: http://svn.redmine.org/redmine/trunk@15393 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 lib/redmine/menu_manager.rb                   |  5 +-
 .../redmine/menu_manager/menu_helper_test.rb  | 50 ++++++++++++++++++-
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/lib/redmine/menu_manager.rb b/lib/redmine/menu_manager.rb
index 619f8f30c..fa7777065 100644
--- a/lib/redmine/menu_manager.rb
+++ b/lib/redmine/menu_manager.rb
@@ -114,7 +114,7 @@ module Redmine
           # Standard children
           standard_children_list = "".html_safe.tap do |child_html|
             node.children.each do |child|
-              child_html << render_menu_node(child, project)
+              child_html << render_menu_node(child, project) if allowed_node?(child, User.current, project)
             end
           end
 
@@ -138,7 +138,7 @@ module Redmine
           # Tree nodes support #each so we need to do object detection
           if unattached_children.is_a? Array
             unattached_children.each do |child|
-              child_html << content_tag(:li, render_unattached_menu_item(child, project))
+              child_html << content_tag(:li, render_unattached_menu_item(child, project)) if allowed_node?(child, User.current, project)
             end
           else
             raise MenuError, ":child_menus must be an array of MenuItems"
@@ -192,6 +192,7 @@ module Redmine
 
       # See MenuItem#allowed?
       def allowed_node?(node, user, project)
+        raise MenuError, ":child_menus must be an array of MenuItems" unless node.is_a? MenuItem
         node.allowed?(user, project)
       end
     end
diff --git a/test/unit/lib/redmine/menu_manager/menu_helper_test.rb b/test/unit/lib/redmine/menu_manager/menu_helper_test.rb
index 975991eb5..404ec6406 100644
--- a/test/unit/lib/redmine/menu_manager/menu_helper_test.rb
+++ b/test/unit/lib/redmine/menu_manager/menu_helper_test.rb
@@ -119,7 +119,7 @@ class Redmine::MenuManager::MenuHelperTest < ActionView::TestCase
     User.current = User.find(2)
 
     parent_node = Redmine::MenuManager::MenuItem.new(:parent_node,
-                                                     '/test',
+                                                     {:controller => 'issues', :action => 'index'},
                                                      {
                                                        :children => Proc.new {|p|
                                                          children = []
@@ -131,7 +131,7 @@ class Redmine::MenuManager::MenuHelperTest < ActionView::TestCase
                                                      })
 
     parent_node << Redmine::MenuManager::MenuItem.new(:child_node,
-                                                     '/test',
+                                                     {:controller => 'issues', :action => 'index'},
                                                      {
                                                        :children => Proc.new {|p|
                                                          children = []
@@ -163,6 +163,52 @@ class Redmine::MenuManager::MenuHelperTest < ActionView::TestCase
     end
   end
 
+  def test_render_menu_node_with_allowed_and_unallowed_unattached_children
+    User.current = User.find(2)
+
+    parent_node = Redmine::MenuManager::MenuItem.new(:parent_node,
+                                                     {:controller => 'issues', :action => 'index'},
+                                                     {
+                                                       :children => Proc.new {|p|
+                                                         [
+                                                           Redmine::MenuManager::MenuItem.new("test_child_allowed", {:controller => 'issues', :action => 'index'}, {}),
+                                                           Redmine::MenuManager::MenuItem.new("test_child_unallowed", {:controller => 'issues', :action => 'unallowed'}, {}),
+                                                         ]
+                                                       }
+                                                     })
+
+    @output_buffer = render_menu_node(parent_node, Project.find(1))
+
+    assert_select("li") do
+      assert_select("a.parent-node", "Parent node")
+      assert_select("ul.menu-children.unattached") do
+        assert_select("li a.test-child-allowed", "Test child allowed")
+        assert_select("li a.test-child-unallowed", false)
+      end
+    end
+  end
+
+  def test_render_menu_node_with_allowed_and_unallowed_standard_children
+    User.current = User.find(6)
+
+    Redmine::MenuManager.map :some_menu do |menu|
+      menu.push(:parent_node, {:controller => 'issues', :action => 'index'}, { })
+      menu.push(:test_child_allowed, {:controller => 'issues', :action => 'index'}, {:parent => :parent_node})
+      menu.push(:test_child_unallowed, {:controller => 'issues', :action => 'new'}, {:parent => :parent_node})
+    end
+
+    @output_buffer = render_menu(:some_menu, Project.find(1))
+
+    assert_select("li") do
+      assert_select("a.parent-node", "Parent node")
+      assert_select("ul.menu-children.unattached", false)
+      assert_select("ul.menu-children") do
+        assert_select("li a.test-child-allowed", "Test child allowed")
+        assert_select("li a.test-child-unallowed", false)
+      end
+    end
+  end
+
   def test_render_menu_node_with_children_without_an_array
     parent_node = Redmine::MenuManager::MenuItem.new(:parent_node,
                                                      '/test',
-- 
2.39.5