Laravel-Backpack / PermissionManager

Admin interface for managing users, roles, permissions, using Backpack CRUD
http://backpackforlaravel.com
Other
527 stars 168 forks source link

publish route file #267

Closed iMokhles closed 3 years ago

iMokhles commented 3 years ago

publish route file is useful to customize roles, permissions and other stuffs

here's example of how i customized the role has access permission to those routes

Capture d’écran 2021-02-07 à 17 58 39

and of course you need to update kernel routeMiddleware like that to add Spatie's Middlewares Capture d’écran 2021-02-07 à 18 00 02

tabacitu commented 3 years ago

Sure, makes it a little easier to do that, thanks @iMokhles ! Sidenote: personally I would have customized the CheckIfAdmin middleware, a bit more surgical in my opinion and it's already in userland.


One thing we should keep in mind - one of the installation steps for this package right now is publishing everything. And I don't think publishing the routes file is a good idea for ALL cases. Ok, if you wanna do it, go ahead, run the publish command with --tag=routes. But since most people won't ever need to edit the routes file, I think we should change the docs so that only the absolute minimum needed is published by default.

@promatik could you please investigate what that is, and make a PR to change the docs so we merge both this and that together? I believe it's the config, but it might also be the migration (don't know if Laravel now runs the migration from the package automatically) and maybe even the lang files (don't think so though). I believe the change to the docs would be adding --tag="config" or --tag="config,migrations" to the publish command, but again, I'm not really sure what's absolutely needed. Looks like we've been publishing too much anyway, so it's a good opportunity to fix that too.

iMokhles commented 3 years ago

@tabacitu thanks for your response i already tested many cases before using Role's middleware but customizing the CheckIfAdmin middleware won't solve that, let's imagine the following concept

We are building SaaS system and we have 3 Roles ( Super_Admin, Managers and Subscribers ) our saas system has almost 5 CRUDS ( users, roles, permissions, plans, codes ) Subscribers can access Codes CRUD only so as we have created the Plans CRUD manually through the backpack:crud command we can easily denyAccess to it's operations inside the setup function if the current user has a Subscriber Role but what about the other cruds ( users, roles, permissions ) how can we block the access to those CRUDS operations if the current user is Subscriber ? first to come in mind as @promatik suggested copying the code from the route file to backpack/custom.php no because if we group those routes under any middleware the package will continue to load them from the parent location which is under vendor folder, and CheckIfAdmin is a global middleware and we cannot do alot of stuffs there rather than checking the roles but for the route checking permission concept which i showed here https://github.com/Laravel-Backpack/BackupManager/pull/80#issue-403245236 it is hard if we have alot of CRUDS with many permissions so copying the routes makes the package loads ( always ) the routes from the new location and applying the middleware there will be really helpful for everyone

sorry for the long explanation but i hadn't another choice to explain the issue correctly

promatik commented 3 years ago

@tabacitu just pushed #270. So this PR now depends on that one.

tabacitu commented 3 years ago

Oups - forgot this is here. Thanks a lot @iMokhles & @promatik - merged and tagged 🙏