Beautify admin routes

[Pierstoval]

Now we have this kind of pattern for routes:
{prefix}/?entity={entity}&action={action}

It seems that the pattern always rely on these two elements, which can be "defaulted" if needed, with the method getNameOfTheFirstConfiguredEntity and the list action by default.

What about changing the @Route annotation to have a much nicer pattern ?
After some tests I've come with 3 different patterns that can certainly match the whole thing:

* @Route("/",                       name="admin_root",     defaults={"entity": null, "action": "list", "id": 0})
* @Route("/{entity}/{action}",      name="admin_action",   defaults={"action": "list", "id": 0})
* @Route("/{entity}/{action}/{id}", name="admin_element",  defaults={"action": "list"}, requirements={"id": "\d+"})

What do you think ?

[javiereguiluz]

My personal opinion is that URL format doesn't matter for a backend. My guess is that the end users of a backend don't care about URLs because they don't even look at them. They just want the backend to work as expected.

However, even if we eventually add this feature, I think it's too early to do it now. I propose to "play" a bit more with this bundle until we decide what we want to do with URLs.

[Pierstoval]

Was thinking about something though.

If you have a "bigger" backoffice with many people using it, it could be good to use the native Security component to restrict access for some entities and some actions.

For example if you want back-end users only to "view" things but never "edit" them, you can have this configuration in your security.yml:

security:
    role_hierarchy:
        ROLE_MANAGER:       ROLE_USER
        ROLE_MODERATOR:     ROLE_MANAGER
        ROLE_ADMIN:         ROLE_MODERATOR
        ROLE_SUPER_ADMIN:   [ROLE_ADMIN, ROLE_ALLOWED_TO_SWITCH]

    access_control:
        - { path: ^/admin,                roles: ROLE_MANAGER }
        - { path: ^/admin/([^/]+)/edit,   roles: ROLE_MODERATOR }
        - { path: ^/admin/([^/]+)/delete, roles: ROLE_ADMIN }

[javiereguiluz]

For now I'm closing this issue as won't fix for the reasons explained in #93.

@Pierstoval you provided a good use case for this feature ... but I think that in a real backend, this could be better solved using a custom security voter. Moreover, since Symfony 2.6, it's really easy to create them.

[Pierstoval]

May I propose that this feature can be still used by user ?

I've got another idea : simply move the routing from annotations to files, and use a routing.yml and routing_alternate.yml file , which the user can decide to use as he wishes ? By default, the docs propose the "native" routing, but we can add a section specific to this other system that allows the user to change its routing file without changing his back-end, but that allows him to specify access_control paths that are safe for specific roles.

In the controller, there's a simple way to do this : the "path" parameters are null by default, and with a simple $request->query->has('action') for example , we can know easily how the request is handled : with "classic" routes, or with "beautified" routes :)

[javiereguiluz]

@Pierstoval at the end, this is mostly not about "how can we do this" or if there is a simple way to do this. Ultimately, this is about adding some non-trivial feature that in my opinion has a lot of "cons" compared to the few "pros" that it would provide.

[Pierstoval]

Here are the cons you wrote on #93 :

• It (slightly) hurts application performance: routes must be created and each entity defines six different routes.
• It's uncomplete: pagination and sorting would still be passed as query string parameters, so the URLs wouldn't be pretty but "almost pretty". E.g. /admin/customer/list?sortField=foo&page=2&sortDirection=DESC
• It's inflexible. If next month we introduce a new parameter, using the old "catch-all" route, everything will work the same. With the new routes, some params would be pretty and others wouldn't: /admin/customer/list?newParam=newValue
• It's unnecessary because backend users don't care about URLs (they don't even look at them).
• It complicates the code of the templates. Now you always use path('admin', { ... }). The new route collections forces you to use a different route each time.
• It (slightly) complicates the code of the project: a new route loader, 6 controllers (new, edit, list, ...) instead of just one controller (index), etc.
• It complicates overriding the default behavior of the bundle (now you only have to create one controller and that's it).

For the performances : the feature as I may implement it does not create more routes, as there is still one "admin" route, with its optional and "defaulted" parameters. It's mostly related to @manuelj555's PR.
For the second point, "uncomplete", my first goal was not to beautify routes only for design reasons, but for technical reasons (mostly security). Any "beautified" back-end would never put filters , order and pagination in "route params", as it's almost always handled by query string parameters.
For the new parameter, you're totally right, but honestly, if this parameter is so important it can be put in route params, then it's just a third optional parameter, and it the "GET" and the "Route" params have the same name, then it's settled : the router can handle both of them and understand natively whether it's a query parameter or a route parameter, this is what is good with Symfony :)
For the next points, they're mostly dedicated to the associated PR, so it's not of the current implementation proposal concern.

With this arguments, I hope that it's gonna make more pros than cons for this feature. I'd be sad if I had to override the native behavior in my apps or use my fork for this and not the original bundle, this just to handle security issues :)

I can be convinced at last only if you find a true reason to refuse all these points :D

(And crap, I think I deleted my branch, I'll have to redevelop this feature, but no problem if you are convinced ;) )

[Pierstoval]

@javiereguiluz : May I propose back another "style" for this feature, as you can see on my branch?

https://github.com/Pierstoval/EasyAdminBundle/compare/urls

The goal is to only change the routing file to load depending on your "preferred route style". The routing_pretty.yml file is ought to be loaded if you intend to use the Security and access_control components to secure your backoffice only for wanted roles.

The usual behavior is unchanged, except for one more redirection to force "list" to be shown in the url, for the access_control to be checked.

This would allow custom setup in security.yml as I said above and then restrict some areas to specific roles/users.

What do you think ?

[ogizanagi]

Well, it has some good points concerning security issues indeed. But I still have concerns about the points raised by Javier and about the future.
The security points might be considered in another issue. But what do you think about providing a DefaultEasyAdminVoter, where we can easily add our own logic about the pages that can be accessed by a user, and a very light configuration loaded in it :

easy_admin:
    entities:
        Posts:
            roles: ['ROLE_AUTHOR']
            class: AppBundle\Entity\Post
        Pages:
            roles: ['ROLE_AUTHOR']
            class: AppBundle\Entity\Page 
       Categories:
            role: [ROLE_WEBMASTER]
            class: AppBundle\Entity\Category
            list: 
                roles: ['ROLE_MANAGER']

        "Config":
            class: AppBundle\Entity\Config
            role: ['ROLE_ADMIN']

The DefaultEasyAdminVoter will provide a way to handle every case through code, and handle most common ones through config, without requiring any change in the routing pattern.

[Pierstoval]

And what if you are using ACLs ?

Ok, it's bad :p

Your option is good, but then the security comes "out" of the security component. With my PR you can define a security access_control for each entity AND each action, and EasyAdmin is not the base "provider" for security, which is better IMO

[ogizanagi]

It could be handled in the Voter (I had some issues with sf < 2.6 about circular references however... I must check if this is feasible now with the last security improvements and the security.authorization_checker service).

I'm not in favor of adding security logic inside EasyAdminBundle (and your solution was good considering this), but even if we do not provide a simple way through configuration, we could provide an optional helper in the form of an AbstractVoter for the bundle uses.

[Pierstoval]

It'd still be heavy, even though it's more convenient for "adaptability" purposes, but as said, I think the goal of EasyAdmin is to keep away from linking with other "out-of-the-box" components. That's why I implemented both "beautifying admin routes" and "custom actions" in the simplest possible way I could (may there are more solutions, but I don't know them yet ;) ) to stay in the rails of EasyAdmin :)

[Pierstoval]

My urls branch has been rebased on master and updated with docs, if one day it's discussed again