sonata-project / SonataAdminBundle

The missing Symfony Admin Generator
https://docs.sonata-project.org/projects/SonataAdminBundle
MIT License
2.11k stars 1.26k forks source link

custom routes should not be visible if you don't have the configured ROLE #4112

Closed OskarStark closed 8 years ago

OskarStark commented 8 years ago

Environment

Sonata packages composer show sonata-project/* sonata-project/admin-bundle 3.5.0 The missing Symfony Admin Generator sonata-project/block-bundle 3.1.1 Symfony SonataBlockBundle sonata-project/cache 1.0.7 Cache library sonata-project/cache-bundle 2.2.5 This bundle provides caching services sonata-project/classification-bundle 3.1.0 Symfony SonataClassificationBundle sonata-project/core-bundle 3.1.1 Symfony SonataCoreBundle sonata-project/datagrid-bundle 2.2 Symfony SonataDatagridBundle sonata-project/doctrine-extensions 1.0.2 Doctrine2 behavioral extensions sonata-project/doctrine-orm-admin-bundle 3.0.5 Symfony Sonata / Integrate Doctrine ORM into the ... sonata-project/easy-extends-bundle 2.1.10 Symfony SonataEasyExtendsBundle sonata-project/exporter 1.7.0 Lightweight Exporter library sonata-project/formatter-bundle 3.0.1 Symfony SonataFormatterBundle sonata-project/google-authenticator 1.0.2 Library to integrate Google Authenticator into a ... sonata-project/intl-bundle 2.2.4 Symfony SonataIntlBundle sonata-project/media-bundle 3.2.0 Symfony SonataMediaBundle sonata-project/notification-bundle 3.0.0 Symfony SonataNotificationBundle sonata-project/page-bundle 3.1.0 This bundle provides a Site and Page management t... sonata-project/seo-bundle 2.0.2 Symfony SonataSeoBundle sonata-project/timeline-bundle 3.0.0 Integrates SpyTimelineBundle into Sonata sonata-project/user-bundle 3.0.1 Symfony SonataUserBundle
Symfony packages composer show symfony/* symfony/assetic-bundle v2.8.0 Integrates Assetic into Symfony2 symfony/monolog-bundle 2.11.1 Symfony MonologBundle symfony/phpunit-bridge v2.8.9 Symfony PHPUnit Bridge symfony/polyfill-apcu v1.2.0 Symfony polyfill backporting apcu_\* functions to lower PHP ver... symfony/polyfill-intl-icu v1.2.0 Symfony polyfill for intl's ICU-related data and classes symfony/polyfill-mbstring v1.2.0 Symfony polyfill for the Mbstring extension symfony/polyfill-php54 v1.2.0 Symfony polyfill backporting some PHP 5.4+ features to lower P... symfony/polyfill-php55 v1.2.0 Symfony polyfill backporting some PHP 5.5+ features to lower P... symfony/polyfill-php56 v1.2.0 Symfony polyfill backporting some PHP 5.6+ features to lower P... symfony/polyfill-php70 v1.2.0 Symfony polyfill backporting some PHP 7.0+ features to lower P... symfony/polyfill-util v1.2.0 Symfony utilities for portability of PHP codes symfony/security-acl v3.0.0 Symfony Security Component - ACL (Access Control List) symfony/swiftmailer-bundle v2.3.11 Symfony SwiftmailerBundle symfony/symfony v2.8.9 The Symfony PHP framework

PHP version

php -v
PHP 7.0.4-7+deb.sury.org~trusty+2 (cli) ( NTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2016 Zend Technologies
    with blackfire v1.10.4, https://blackfire.io, by Blackfireio Inc.

Subject

Looks, like custom routes are not checked against the configured roles.

Steps to reproduce

Configure a custom route like this (_admin_today):

            sonata.admin.group.video:
                label:           navigation.folder.video # VIDEO
                label_catalogue: messages
                icon:            '<i class="fa fa-video-camera"></i>'
                items:
                    - route: _admin_today # HEUTE
                      label: admin.navigation.today
                    - livestream.admin.livestream
                    - video.admin.episode
                roles: [ ROLE_ADMIN, ROLE_STAFF, ROLE_PROMO ]

Create a new role ROLE_FOO and access the navigation with a user with this role.

Expected results

The custom navigation node with a custom route should be hidden.

Actual results

The custom navigation node with a custom route is visible.

bildschirmfoto 2016-08-31 um 13 07 28
codebach commented 8 years ago

Yes, I can confirm this bug. I can also provide a solution for this:

Here is the check for group items when it has an admin: https://github.com/sonata-project/SonataAdminBundle/blob/3.x/Menu/Provider/GroupMenuProvider.php#L72

But since route does not have an admin it just generates the menu item for this group item. https://github.com/sonata-project/SonataAdminBundle/blob/3.x/Menu/Provider/GroupMenuProvider.php#L82

The solution could be that we will inject the Session into SonataAdminBundle/Menu/Provider/GroupMenuProvider.php and then we will compare if user role exist in group roles in else case.

If you want I can create a PR for this.

greg0ire commented 8 years ago

The solution could be that we will inject the Session

Wouldn't the security.authorization_checker service be enough for that?

OskarStark commented 8 years ago

Wouldn't the security.authorization_checker service be enough for that?

indeed 👍

@ahmetakbn can you please provide a PR, including tests?

codebach commented 8 years ago

@ahmetakbn can you please provide a PR, including tests?

@OskarStark of course