Sylius / RbacPlugin

Sylius roles and permissions management plugin
32 stars 38 forks source link

HardcodedRouteNameChecker prevents custom_sections to be protected properly #62

Closed arnofo closed 5 years ago

arnofo commented 5 years ago

I have custom resource "article"

app_admin_article:
    resource: |
        alias: app.article
        section: admin
        templates: SyliusAdminBundle:Crud
        redirect: update
        grid: app_admin_article
        vars:
            all:
                header: app.ui.articles_header
                subheader: app.ui.articles_subheader
                templates:
                    form: "admin/form/Article/_form.html.twig"
            index:
                icon: 'newspaper'
    type: sylius.resource

I configured this custom section for rbac

sylius_rbac:
    custom_sections:
        articles:
            - app_admin_article

The entry "Articles" is properly hidden from the main menu but is still accessible via URL.

I have not had the time to look into a proper fix but i noticed the file App\Rbac\Access\Checker\HardcodedRouteNameChecker has a function isAdminRoute which looks for route prefixes to determine if it is an admin route.

public function isAdminRoute(string $routeName): bool
{
        return
            strpos($routeName, 'sylius_admin') !== false ||
            strpos($routeName, 'sylius_rbac_admin') !== false
        ;
}

My custom resource being prefix with app_admin, this will return false.

(This function is called in Sylius\RbacPlugin\Access\Listener\AccessCheckListener::createAccessRequestFromEvent, which i believe is responsible for preventing access by url)


Dirty fix

Create you own file HardcodedRouteNameChecker

<?php

declare(strict_types=1);

namespace App\Rbac\Access\Checker;

use Sylius\RbacPlugin\Access\Checker\RouteNameCheckerInterface;

final class HardcodedRouteNameChecker implements RouteNameCheckerInterface
{
    public function isAdminRoute(string $routeName): bool
    {
        return
            false !== strpos($routeName, 'sylius_admin') ||
            false !== strpos($routeName, 'sylius_rbac_admin') ||
            false !== strpos($routeName, 'app_admin') // my own prefix
        ;
    }
}

override the bundle's class

# in services.yaml
Sylius\RbacPlugin\Access\Checker\RouteNameCheckerInterface:
        class: App\Rbac\Access\Checker\HardcodedRouteNameChecker
diimpp commented 5 years ago

I don't think, that it's dirty fix, but a desired solution for now. I just did the same.

Maybe it should be mentioned in readme to override RouteNameCheckerInterface service.

Zales0123 commented 5 years ago

I agree with @diimpp. Overwriting the default service is totally the way you should customize the behavior of this plugin. The other question is, how can we make it easier and more developer-friendly :) We can start with adding such a tip to the plugin's README.