@Security Annotation Driven Weighted N-Level Bootstrap Menus

[jonny827]

Goals

I would like to have a menu that is responsive to both itself->(weights) but also responsive to its enviornment->(security) , have N-Levels, showActiveclass, N-Level(active class), Extended by Other Bundles through Events, and of course has top performance/caching.

Current Enviornment / Approach

Symfony: 3.0.5 and will be upgrading to 3.1.
SensioFrameWorkExtraBundle: 3.0.16
KNPMenuBundle: 2.1.1
MopaBootstrapBundle: 3.0.0 - master @dev

1. KNPMenuBundle(Gold Standard for Menus)
   A) Implementing KNPMenuBundle Menu Builder as a Service
   B) Implemented an Event Driven KNPMenuBundle Menu
   C) For performance I recommend disabling your unused KNPMenuBundle Menu Providers as it boosts performance.
2. MopaBootstrapBundle(for active class, and menu icons(3 libraries to chose from)
3. SmartMenusJS(for N-Level Menus JS/CSS that was removed in Bootrap 3.x)
4. Filter KNPMenuBundle By Weights*
5. Filter KNPMenuBundle By Current User Access* (Goal 3: Ideally by role, voter or expression just like the power of the @Security Annotation allows)
6. Allow filtering to be N-Level.

References for the Two Filter Options

• Symfony2 Advanced Menus Tutorial -outdated repository on GITHUB
  Note: This tutorial is Pre Symfony 2.6 due to the use of security.context service). I have updated my solution to take advantage of constructor injection of the service arguments '@security.token_storage' and '@security.authorization_checker' to the menu builder service(also an update to the tutorial) AND if you use event listeners or subscribers to extend your menu injecting the services in there will allow you further control. (Is the security injection best practices? -- discussed below)

Concerns with the Implementation

1. Is Filtering by Weight already possible in a merged commit into KNPMenu?

A. #176 - referred to (Mar 11, 2013) [KNPMenu Issue#97(https://github.com/KnpLabs/KnpMenu/issues/97) - Fixed Via (Jun 24, 2015) @folliked PR #209 Commit
B. KNPMenu PR#207 by @NiR- referencing Issue #97.
C. Sorting From Different Bundles requires using subscriber versus listener as noted here by @TomasVotruba
D. Gitter
E. #118

Filter by Weight Potentially Interested Developers(due to your participation on these Issues/PR's etc): @khal3d , @Nek- , @benjamindulau, @ Maxwell2022 , @dbu , @NiR- , @althaus, @Naitsirch

@stof

1. Is the follow approach I take cached by Symfony?
2. Is there a better approach using a KNPMenu Interator?
3. For those that develop sets of optional bundles ( example SonataFramework) would there be performance impact in product for 20+ Event Listeners all with injected Security Authorization/TokenStorage services loading on each request?

/**
    * Features:  Sorts Menu Items By Weight Assigned in Each Bundles Menu Listener 
    * Source: http://www.studiomashbo.com/blog/article/working-with-menus-in-symfony
    */
    public function reorderMenuItems($menu)
    {
        $menuOrderArray = array();  
        $addLast        = array();  
        $alreadyTaken   = array();  

        foreach($menu->getChildren() as $key => $menuItem) 
        {            
            if ($menuItem->hasChildren()) 
            {                
                $this->reorderMenuItems($menuItem);  
            }           

            $orderNumber = $menuItem->getExtra('weight');  

            if ($orderNumber != null) 
            {    
                if (!isset($menuOrderArray[$orderNumber])) 
                {       
                    $menuOrderArray[$orderNumber] = $menuItem->getName();  
                } 
                else 
                {                
                    $alreadyTaken[$orderNumber] = $menuItem->getName();  
                }            
            } 
            else 
            {                
                $addLast[] = $menuItem->getName();  
            }        
        }        

        // Sort them after first pass.        
        ksort($menuOrderArray);  

        // Handle position duplicates.        
        if(count($alreadyTaken))
        {            
            foreach($alreadyTaken as $key => $value) 
            {                
                // The ever shifting target                
                $keysArray = array_keys($menuOrderArray);  
                $position = array_search($key, $keysArray);  

                if($position === false)
                {                    
                    continue;  
                }

                $menuOrderArray = array_merge(array_slice($menuOrderArray, 0, $position), array($value), array_slice($menuOrderArray, $position));  
            }
        }        

        // Sort them after second pass.        
        ksort($menuOrderArray);  

        // Add items without ordernumber to the end.        
        if(count($addLast)) 
        {
            foreach($addLast as $key => $value)
            {                
                $menuOrderArray[] = $value;  
            }        
        }

        if(count($menuOrderArray))
        {            
            $menu->reorderChildren($menuOrderArray);  
        }    

    }

2. Filter by Current User Access

A) Symfony says @Security is the best practice to use when possible however he tutorial calls for [JMSSecurityExtraBundle}(https://github.com/schmittjoh/JMSSecurityExtraBundle) which requires the @Secure Annotations NOT the @Security annotations.
B. According to @dustin10 (Nov 1, 2011) in #83 it is best to inject into the builder and USE IF/ELSE
C. According to @stof (Jul 15, 2011) and @jared-fraser (Jan 5, 2012) in #51 they also say it is best to inject into the builder and use IF/ELSE.
D. Another Option that was not integrated due to the nature of the implementation - config file based role/expression filtering (https://github.com//pull/243).
E. There are other options on the Market however they also require JMSSecurityExtraBundle, and both do not seem to be cacheable, and limit the usage of the full security that JMSSecurityExtraBundle has to offer especially expressions, voters.
Other Available Solutions To Filter By User Roles

1. ZettaMenuBundle (rules defined in firewall?, and seems to demonstratre only ROLES filters of @secure) No Cache. Wrong? @zetta
2. ZerkalicaMillwrightMenuBundle (only role-based and acl-based) (no voters/expressions) No Cache. Wrong? @zerkalica , @HadesArchitect (a current contributor for added input).

Filter by User Roles Potentially Interested Developers(due to your participation on these Issues/PR's, or the Bundles you contribute to or developed the etc): @zerkalica

@stof

1. Is there functionality like JMSSecurityExtraBundle's Annotation Driver available in SensioFrameWorkExtraBundle to obtain class/method metadata or to accomplish similar functionality to the below code?
2. Can we make it cacheable?
3. The filterMenuByRouteAuthorization I presume could possibly benefit from the Iterator as well?
4. KNPMenuBundle - already checks for existing routes or the route collection I believe. If you declare a non existing route then it throws an error when building. We could hook into it at this point to ready the annotations as well ?

    /**
    * @param Bundle Controller $class
    * @return \JMS\SecurityExtraBundle\Metadata\ClassMetadata
    */
    public function getMetadata($controllerClass)
    {
        return $this->metadataReader->loadMetadataForClass(new \ReflectionClass($controllerClass));
    }

    /**
    * @param $routeName
    * @return boolean from AuthorizationCheckerInterface
    */
     public function hasRouteAccess($routeName)
    {
        $token = $this->tokenStorage->getToken();
    if ($token->isAuthenticated()) 
        {
            //Logout is assigned in services so it does not have a default(_controller) $fullyQualifiedClassNameAndMethod
            if($routeName != 'user_auth_logout')
            {
                $route = $this->router->getRouteCollection()->get($routeName);
                $fullyQualifiedClassNameAndMethod = $route->getDefault('_controller');
                list($routeClass, $routeMethod) = explode('::', $fullyQualifiedClassNameAndMethod, 2); 

                $metadata = $this->getMetadata($routeClass);

                if (!isset($metadata->methodMetadata[$routeMethod])) 
                {
                    return false;
                }

                foreach ($metadata->methodMetadata[$routeMethod]->roles as $role)
                {
                    if ($this->authorizationChecker->isGranted($role)) 
                    {
                        return true;
                    }
                }
                return false;
            }
            elseif(($routeName == 'user_auth_logout') && ((true === $this->authorizationChecker->isGranted('IS_AUTHENTICATED_FULLY'))  || (true === $this->authorizationChecker->isGranted('IS_AUTHENTICATED_REMEMBERED'))))
            {
                return true;
            }
            else
            {
                return false;
            }
        }
        return false;
    }


    /**
    * @param Knp\Menu\ItemInterface $menu
    * @return Knp\Menu\ItemInterface $menu
    */  
    public function filterMenuByRouteAuthorization(ItemInterface $menu)
    {
        /** @var \Knp\Menu\MenuItem $child */
        foreach($menu->getChildren() as $child) 
        { 
            $routes = $child->getExtra('routes');

            if ($routes !== null) 
            {
                $route = current(current($routes));

                if ($route && !$this->hasRouteAccess($route)) 
                {
                    $menu->removeChild($child);
                }
            }
            elseif($child->hasChildren())
            {
                $this->filterMenuByRouteAuthorization($child);
            }   
        }
        return $menu;
    }

Requirements

The solution must:

1. Utilize the full power of @Security(Roles, Voters, Expressions, ACL) annotations - A) Symfony Best Practice B) already use SensioFrameWorkExtraBundle for @route --- and plan to use for @Cache etc. C) why unnecessarily override part of the features(@Security) with another bundle that does the same thing(roles,voters,expressions,acl etc) by using JMSSecurityExtraBundle?
2. Be cacheable so that I can avoid the performance hit of filtering annotations for every bundle/controller/method on every request OR take advantage of an existing cache of the same information done by the SensioFrameworkExtraBundle or others(if necessary). One part of the caching issue - > According to @stof here -> using getRouteCollection() on the router directly is not advisable because it is not cached
3. Not require a user to be authenticated (based on firewall caching or access).
4. Allow @Security to be optional on classes/methods
5. Work with multiple bundles within the Symfony /src directory
6. Utilize the annotations as the source for permissions - Why should you duplicate permissions in the builder/listener/subscriber? Then you have a problem where your annotation declarations and your builder link permissions are DIFFERENT because you would have to essentially declare them again.

General Components For The Solution

1. Develop a FilterInterface to apply @Security authorization's AND Weights to a KNPMenu object so other filters in the future could be developed.
2. Develop something for SensioFrameWorkExtraBundle similar to the AnnotationDriver from [JMSSecurityExtraBundle}(https://github.com/schmittjoh/JMSSecurityExtraBundle) to build the UserAccessFilter from (possibly AnnotationDriver could be used to read Custom Annotations as well )
3. Caching? One part of the caching issue - > According to @stof here -> using getRouteCollection() on the router directly is not advisable because it is not cached.

Approach Thought Process

1. Remove the IF/ElSE linear development pattern for a more Symfony OOP approach.
2. Centralize authorization logic for both controllers but also front/end navigation.
3. Develop best practice approaches for what should be default or at least supported features for a menu implementation in Symfony.

Perceived Choices

1. Utilize linear IF/ELSE(Does this feel Symfony to you ?) and security injection into each builder/listener/subscriber and forgo the @secure annotations and RouteAccess filtering at the builder level as discussed above. Use current Weight filtering functions and maybe try iterators to increase performance. Caching? Maintenance of Duplicate security settings in Controller AND MenuConfig?
2. Use Doctrine Annotation Reader and process the annotations again by bootstrapping my own AnnotationDriver?
3. Hope that the community can band together around this large (my opinion) problem. KNP menu already develops a route instance, SensioFrameWorkExtraBundle already reads annotations at least for each Kernel.Request for the requested route. Symfony/SensioFrameWorkExtraBundle already have caching built in and/or caching functionality.

Can you please help @stof @fabpot @schmittjoh , @tvlooy, and the rest of everyone mentioned in this inquiry to come up with a solution with me if you agree with the needs and potential other uses of the potential changes (FilterInterface, SenseioAnnotationDriver/CustomAnnotationDriver) etc?

Disclaimers

Please excuse the format of my post. I am trying to make a well organized, detailed inquiry that is clear, searchable, and contains enough information and gets the right people involved.

[zetta]

Filter by Current User Access
...
Other Available Solutions To Filter By User Roles
ZetaMenuBundle (rules defined in firewall?, and seems to demonstratre only ROLES filters of @secure) No Cache. Wrong? @zetta

the bundle is intended to use your existing firewall's configuration , so you don't need to add extra configuration for the menu, or if you are using @Secure annotation (JMSSecurityExtraBundle) your menu will be filtered as well.

• Yes, the bundle has no Cache, but I think is a quick feature I can add to it.

[jonny827]

@zetta

1. Does your bundle supports all of these expressions?
2. Does it support recursive checks(N-Levels)?
3. I see you just use the normal KNP render option , does this mean it will work with MopaBootstrapBundle as you are not changing the rendering options?
4. Cache is important I believe instead of reading annotations from every controller<->method on every request for every user. Implementing this feature would be a big win!
5. How would your bundle integrate with the weight sorting above? I presume because the filtering is done inside the menu builder function your UserAccess filter would be applied after it?
6. Could you add integration for @Security(SensioFrameworkExtraBundle)?

[zetta]

1. Bundle should support all the expressions because is using the accessDecisionManager to check if the current user can access to the url.
2. Yes, it checks recursive in N-levels using RecursiveItemIterator
3. Never used MopaBoostrapBundle but I'm not overriding the render option or changing the way it works, i just declare a new knp provider.
4. I agree.
5. Sorry I didn't understand the issue with the weights.
6. I think is possible to add that integration, should be easily injected to the Security Service on the Menu Bundle.

[jonny827]

@zetta

1. Excellent, That looks like a logical approach to implement the functionality.
2. That is perfect. Implementations should not be limited number of nested levels (Bootstrap 3.X does not support more than root->level1->level2 hence my above use of SmartMenusJs as a sort of polyfill ) .
3. I believe MopaBootstrapBundle uses a KNPMenu extension to concert the menu to use attributes etc. I believe this should not affect this solution.
4. Great! Are you willing to implement it into your bundle?
5. Weight or sorting menu items by an assigned "Extra" option. At least this is one of the implementation I was discussing above in detail. KNPMenu currently sorts the menu alphabetically I believe, I would like to be able to override that just like the reorderMenuItems approach does above based on each items Extra(and if no extra->weight they go at the end). I believe however being able to add weights(sort by Weight feature) may be better implemented in your RecursiveItemIterator .
   Example
   Current KNPMenu would show this.
   Root

• Level 1
  • Bar
  • Foo

I want to be able to make Bar "heavier by weight" than Foo , so I assign Foo Weight=20, and Bar Weight=30

$menu->addChild('Foo', array('route' => 'user_home', 'icon' => 'list-alt'))->setExtra('weight', 20);


$menu->addChild('Bar', array('route' => 'user_home', 'icon' => 'list-alt'))->setExtra('weight', 30);

1. Excellent. I would like to avoid having to utilize @secure at all due to the added complexity, divergence from stated Symphony best practice, and because I already depend on its core for @route annotations at the very least. Would you be willing to also implement this into your bundle?

Can I help with documentation or developing examples or testing (not writing code based Tests) ?

[zetta]

As far as I can remember Knp Bundle was showing the menu as you declared, not modifying the order in any way (maybe that was time ago when I create this plugin).

It will take a while until I can address this changes, but I'll do.

[stof]

KNPMenu currently sorts the menu alphabetically I believe

That's wrong. KnpMenu does not sort the menu at all. It displays items in the order in which you added them (unless you reorder them after that of course, but the library won't reorder anything for you)

[jonny827]

@zetta and @stof I have fixed my comment. I thought I saw somewhere that it was filtering by name but I have been looking through several bundles, gists's , tutorials, and discussions so thank you for the clarification on my (I believe) statement. @stof do you think KNPMenu/KNPMenuBundle will ever integrate weights/user access?

@zetta , thank you for being open to input and for implementing these features! What is your time estimate to add these features? I only ask so I can work on other aspects of my project around your schedule.

Let me know if I can be of any help!