search

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

Security informations are ignored in role security #4925

Closed core23 closed 2 years ago

core23 commented 6 years ago

Environment

Sonata packages

sonata-project/admin-bundle              3.31.0             3.31.0             The missing Symfony Admin Generator
sonata-project/block-bundle              3.11.0             3.11.0             Symfony SonataBlockBundle
sonata-project/cache                     1.1.1              2.0.1              Cache library
sonata-project/cache-bundle              2.4.0              2.4.0              This bundle provides caching services
sonata-project/classification-bundle     3.6.1              3.6.1              Symfony SonataClassificationBundle
sonata-project/core-bundle               3.9.0              3.9.0              Symfony SonataCoreBundle
sonata-project/dashboard-bundle          dev-master 4d1eb19 dev-master 4d1eb19 Provides a Dashboard management through container and block services
sonata-project/datagrid-bundle           2.3.1              2.3.1              Symfony SonataDatagridBundle
sonata-project/doctrine-extensions       1.0.2              1.0.2              Doctrine2 behavioral extensions
sonata-project/doctrine-orm-admin-bundle 3.4.1              3.4.1              Symfony Sonata / Integrate Doctrine ORM into the SonataAdminBundle
sonata-project/easy-extends-bundle       2.4.0              2.4.0              Symfony SonataEasyExtendsBundle
sonata-project/exporter                  1.8.0              1.8.0              Lightweight Exporter library
sonata-project/formatter-bundle          3.4.1              3.4.1              Symfony SonataFormatterBundle
sonata-project/intl-bundle               2.4.0              2.4.0              Symfony SonataIntlBundle
sonata-project/media-bundle              3.10.2             3.10.2             Symfony SonataMediaBundle
sonata-project/news-bundle               3.3.0              3.3.0              Symfony SonataNewsBundle
sonata-project/notification-bundle       3.3.1              3.3.1              Symfony SonataNotificationBundle
sonata-project/page-bundle               3.7.1              3.7.1              This bundle provides a Site and Page management through container and block services
sonata-project/seo-bundle                2.5.0              2.5.0              Symfony SonataSeoBundle
sonata-project/timeline-bundle           3.2.0              3.2.0              Integrates SpyTimelineBundle into Sonata
sonata-project/user-bundle               4.1.0              4.1.0              Symfony SonataUserBundle

Symfony packages

symfony/asset                v3.4.4  v4.0.4 Symfony Asset Component
symfony/assetic-bundle       v2.8.2  v2.8.2 Integrates Assetic into Symfony2
symfony/cache                v3.4.4  v4.0.4 Symfony Cache component with PSR-6, PSR-16, and tags
symfony/class-loader         v3.4.4  v3.4.4 Symfony ClassLoader Component
symfony/config               v3.4.4  v4.0.4 Symfony Config Component
symfony/console              v3.4.4  v4.0.4 Symfony Console Component
symfony/css-selector         v3.4.4  v4.0.4 Symfony CssSelector Component
symfony/debug                v3.4.4  v4.0.4 Symfony Debug Component
symfony/debug-bundle         v3.4.4  v4.0.4 Symfony DebugBundle
symfony/dependency-injection v3.4.4  v4.0.4 Symfony DependencyInjection Component
symfony/doctrine-bridge      v3.4.4  v4.0.4 Symfony Doctrine Bridge
symfony/dom-crawler          v3.4.4  v4.0.4 Symfony DomCrawler Component
symfony/dotenv               v3.4.4  v4.0.4 Registers environment variables from a .env file
symfony/event-dispatcher     v3.4.4  v4.0.4 Symfony EventDispatcher Component
symfony/expression-language  v3.4.4  v4.0.4 Symfony ExpressionLanguage Component
symfony/filesystem           v3.4.4  v4.0.4 Symfony Filesystem Component
symfony/finder               v3.4.4  v4.0.4 Symfony Finder Component
symfony/form                 v3.4.4  v4.0.4 Symfony Form Component
symfony/framework-bundle     v3.4.4  v4.0.4 Symfony FrameworkBundle
symfony/http-foundation      v3.4.4  v4.0.4 Symfony HttpFoundation Component
symfony/http-kernel          v3.4.4  v4.0.4 Symfony HttpKernel Component
symfony/inflector            v3.4.4  v4.0.4 Symfony Inflector Component
symfony/intl                 v3.4.4  v4.0.4 A PHP replacement layer for the C intl extension that includes additional data from the ICU library.
symfony/lts                  v3      v3     Enforces Long Term Supported versions of Symfony components
symfony/maker-bundle         v1.0.2  v1.0.2
symfony/monolog-bridge       v3.4.4  v4.0.4 Symfony Monolog Bridge
symfony/monolog-bundle       v3.1.2  v3.1.2 Symfony MonologBundle
symfony/options-resolver     v3.4.4  v4.0.4 Symfony OptionsResolver Component
symfony/phpunit-bridge       v4.0.4  v4.0.4 Symfony PHPUnit Bridge
symfony/polyfill-apcu        v1.7.0  v1.7.0 Symfony polyfill backporting apcu_* functions to lower PHP versions
symfony/polyfill-intl-icu    v1.7.0  v1.7.0 Symfony polyfill for intl's ICU-related data and classes
symfony/polyfill-mbstring    v1.7.0  v1.7.0 Symfony polyfill for the Mbstring extension
symfony/polyfill-php56       v1.7.0  v1.7.0 Symfony polyfill backporting some PHP 5.6+ features to lower PHP versions
symfony/polyfill-php70       v1.7.0  v1.7.0 Symfony polyfill backporting some PHP 7.0+ features to lower PHP versions
symfony/polyfill-php72       v1.7.0  v1.7.0 Symfony polyfill backporting some PHP 7.2+ features to lower PHP versions
symfony/polyfill-util        v1.7.0  v1.7.0 Symfony utilities for portability of PHP codes
symfony/process              v3.4.4  v4.0.4 Symfony Process Component
symfony/property-access      v3.4.4  v4.0.4 Symfony PropertyAccess Component
symfony/routing              v3.4.4  v4.0.4 Symfony Routing Component
symfony/security             v3.3.16 v4.0.4 Symfony Security Component
symfony/security-acl         v3.0.1  v3.0.1 Symfony Security Component - ACL (Access Control List)
symfony/security-bundle      v3.3.16 v4.0.4 Symfony SecurityBundle
symfony/security-csrf        v3.4.4  v4.0.4 Symfony Security Component - CSRF Library
symfony/stopwatch            v3.4.4  v4.0.4 Symfony Stopwatch Component
symfony/swiftmailer-bundle   v3.1.6  v3.1.6 Symfony SwiftmailerBundle
symfony/templating           v3.4.4  v4.0.4 Symfony Templating Component
symfony/translation          v3.4.4  v4.0.4 Symfony Translation Component
symfony/twig-bridge          v3.4.4  v4.0.4 Symfony Twig Bridge
symfony/twig-bundle          v3.4.4  v4.0.4 Symfony TwigBundle
symfony/validator            v3.4.4  v4.0.4 Symfony Validator Component
symfony/var-dumper           v3.4.4  v4.0.4 Symfony mechanism for exploring and dumping PHP variables
symfony/web-profiler-bundle  v3.4.4  v4.0.4 Symfony WebProfilerBundle
symfony/yaml                 v3.4.4  v4.0.4 Symfony Yaml Component

PHP version

Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2017 Zend Technologies
    with Zend OPcache v7.1.12, Copyright (c) 1999-2017, by Zend Technologies
    with Xdebug v2.5.5, Copyright (c) 2002-2017, by Derick Rethans

Subject

When using the role security model and defining an information mapping, the mapping is ignored.

Steps to reproduce

Define the following config:

sonata_admin:
    security:
        handler:    sonata.admin.security.handler.role

        information:
            VIEWER:    [VIEW, LIST, EXPORT]
            EDITOR:    [EDIT, LIST, CREATE]
            ADMIN:     [OPERATOR, MASTER]

Add a the ACME_VIEWER permission to a user or group for the ACME admin.

Expected results

The user gets the ROLE_SONATA_ACME_ADMIN_ACME_VIEWER role and can access the corresponding admin page.

Actual results

The user gets the ROLE_SONATA_ACME_ADMIN_ACME_VIEWER role, but can't access the admin page, because the role is not translated to the sub roles: ROLE_SONATA_ACME_ADMIN_ACME_VIEW, ROLE_SONATA_ACME_ADMIN_ACME_LIST and ROLE_SONATA_ACME_ADMIN_ACME_EXPORT .

Probable soltution.

The securityInformation is only used for ACL permissions inside the AclSecurityHandler class. There is no special handling inside the RoleSecurityHandler class.

Xmblr commented 6 years ago

Faced the same problem. The solution helped me: security.yml

security:
    role_hierarchy:

        ROLE_USER:
          - ROLE_ADMIN_USER_ADDRESS_ALL
          - ROLE_ADMIN_USER_CARGO_ALL
          - ROLE_ADMIN_USER_CITY_ALL
          - ROLE_ADMIN_USER_ORDER_ALL
        ROLE_ADMIN:
          - ROLE_ADMIN
          - ROLE_ADMIN_USERS_ALL
          - ROLE_ADMIN_ADDRESS_ALL
          - ROLE_ADMIN_CARGO_ALL
          - ROLE_ADMIN_CITY_ALL
          - ROLE_ADMIN_ORDER_ALL

config.yml

sonata_admin:
        security:
                 handler: sonata.admin.security.handler.role
stale[bot] commented 4 years ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

VincentLanglet commented 3 years ago

This issue is pretty old, is it still relevant ? Do you have a repository to reproduce the issue ?

I have a config similar to the one proposed by @Xmblr and it works fine for me.

core23 commented 3 years ago

Yes, the issue is still relevant. The solution provide by @Xmblr describes a different problem.

I updated the issue description to provide more information about the actual problem.

VincentLanglet commented 3 years ago

I've never used ACL, I'm kinda interested to debug this in order to learn more but I have a lot of question.

Seems like ACL are not recommended anymore https://github.com/sonata-project/SonataAdminBundle/issues/7303 Does it currently work for Sonata, in another way you described ?

Also can you describe how you do ?

Add a the ACME_VIEWER permission to a user or group for the ACME admin.

This seems not hard to debug: getSecurityInformation is not used a lot in our code. But to debug, I'd like to reproduce the issue with a project or a failing test.

VincentLanglet commented 2 years ago

Also can you describe how you do ?

Add a the ACME_VIEWER permission to a user or group for the ACME admin.

This seems not hard to debug: getSecurityInformation is not used a lot in our code. But to debug, I'd like to reproduce the issue with a project or a failing test.

Can you explain how to reproduce the issue @core23 ? I never used ACL before.

Also, does it means ACL are not working at all so far for SonataAdmin ? If so, it would means nobody is using this and we could deprecate/remove the feature and solve #7303

core23 commented 2 years ago

Also, does it means ACL are not working at all so far for SonataAdmin ? If so, it would means nobody is using this and we could deprecate/remove the feature and solve #7303

This is not related to the ACL itself, but the ACL security handlers works a little different than the normal role handler.

Can you explain how to reproduce the issue @core23 ? I never used ACL before.

I thought the issue description is clear enough, but I can try to explain it a little bit more.

Normally you have the following rules: CREATE, DELETE, EDIT, EXPORT, LIST and VIEW on an admin.

Given the following config, I would expect if you assign the EDITOR role for a specific admin (e.g. AcmeAdmin.php) to a user, the user would be able to receive the EDIT, LIST and CREATE roles for this admin.

sonata_admin:
    security:
        handler:    sonata.admin.security.handler.role

        information:
            VIEWER:    [VIEW, LIST, EXPORT]
            EDITOR:    [EDIT, LIST, CREATE]
            ADMIN:     [OPERATOR, MASTER]

This works if you use the sonata.admin.security.handler.acl securty handler, but not if you use the sonata.admin.security.handler.role handler.

VincentLanglet commented 2 years ago

Looking at the code, we have

$container->setParameter('sonata.admin.configuration.security.information', $config['security']['information']);

then

$definition->addMethodCall('setSecurityInformation', ['%sonata.admin.configuration.security.information%']);

And if you look at all the call of getSecurityInformation, there is

So this seems pretty clear that the feature is ACL-related so I'm not chocked if it's only for sonata.admin.security.handler.acl.

But we might be able to add this to the role security ; currently we're doing 

return $this->isAnyGranted($this->superAdminRoles)
                || $this->isAnyGranted($attributes, $object)
                || $useAll && $this->isAnyGranted([$allRole], $object);

We could add something like 

$baseRole = $this->getBaseRole($admin);

$extraRolesToCheck = [];
foreach ($attributes as $attribute) {
    foreach ($admin->getSecurityInformation() as $role => $permissions) {
        foreach ($permissions as $permission) {
            if (sprintf($this->getBaseRole($admin), $permission) === $attribute) {
                $extraRolesToCheck = sprintf($baseRole, $role);
            }
    }
}

array_unique($extraRolesToCheck);

WDYT @core23 ?

VincentLanglet commented 2 years ago

This works if you use the sonata.admin.security.handler.acl securty handler, but not if you use the sonata.admin.security.handler.role handler.

Looking at the sonata.admin.security.handler.acl isGranted method is 

return $this->isAnyGranted($this->superAdminRoles) ||
                $this->isAnyGranted($attributes, $object);

there is no check to $admin->getSecurityInformation().

Are you sure it works this way for AclHandler ?

Does giving the ROLE_SONATA_ACME_ADMIN_ACME_VIEWER to someone allows him to have ROLE_SONATA_ACME_ADMIN_ACME_VIEW when the AclHandler is used ?

jordisala1991 commented 2 years ago

To me it looks like over complicating a feature that is already flexible enough. If the security information is only used for ACL, imho it should stay the same

VincentLanglet commented 2 years ago

To me it looks like over complicating a feature that is already flexible enough. If the security information is only used for ACL, imho it should stay the same

To me the feature is just doing something different than what is asked here, even for the AclHandler. If you give the ROLE_SONATA_ACME_ADMIN_ACME_VIEWER to someone, it won't work as core would have expect even using the AclSecurityHandler.

So this is more a new feature with a new configuration key which could be something like role_hierarchy. Currently there is only a shortcut ALL => [ .... ]. But this could be a way to introduce more.

But this issue/feature request was pending for 3 years without any activity so I think we can close it.