Skip to content
This repository has been archived by the owner on Nov 23, 2020. It is now read-only.

[Feature][0.9][Merged] Custom admin middleware #258

Merged
merged 9 commits into from
Mar 22, 2018

Conversation

tabacitu
Copy link
Member

@tabacitu tabacitu commented Mar 6, 2018

Builds upon #67 . I'm going to copy-paste my last comment from there, which explains everything this PR does.

Update: I still agree admin was an uninspired name for this middleware.

But after implementing the guard system in PR #165, I think a more future-proof solution would be to allow the developer to use a custom name, if he so chooses. Or a different middleware, while we're at it. We can fetch the middleware using helpers, and those helpers could get the value from the config. Expanding on how @OwenMelbz did it in PR #87 .

To rephrase, in config/backpack/base.php we'd have:

    // Fully qualified namespace of the User model
    'user_model_fqn' => '\Backpack\Base\app\Models\BackpackUser',

+    // The class for the middleware that checks if the visitor is an admin
+    'middleware_class' => \Backpack\Base\app\Middleware\Admin::class,
+
+    // Alias for that middleware
+    'middleware_key' => 'admin',
+    // Note: It's recommended to use the backpack_middleware() helper everywhere, which pulls this key for you.

This way:

  • the developer could change the admin key for the middleware to anything he wants;
  • the developer could easily use a different middleware, if he so chooses; just create a custom class, then point to that class in the config;
  • the developer could easily use the middleware name, thanks to the helper; so it's backpack_middleware() instead of config('backpack.base.middleware_key'); easiler to remember; easier on the eyes;

Other considerations:

  • keeping the admin name for the middleware will make this backwards-compatible; a non-breaking change; but we could force the name change to checkIfAdmin to make it more Laravely and make sure people upgrade their Backpack correctly; food for thought;
  • the current admin middleware is very basic; it just checks for authentication and makes sure the redirect uses the route_prefix; that's the only reason we didn't use the auth middleware; so I expect anyone who has both users & admins to change this middleware to something else; wether they use PermissionManager or not;
  • we'll still need to instruct people to use this helper in their own Controllers or route files; this can be in the upgrade guide;

Thoughts? @ChrisThompsonTLDR , @OwenMelbz , @ctf0 , @eduardoarandah , @lloy0076 you guys seemed to be interested in this feature. Let me know if you think there's a better solution.

@tabacitu tabacitu mentioned this pull request Mar 8, 2018
7 tasks
@tabacitu
Copy link
Member Author

tabacitu commented Mar 8, 2018

Merged into upgrade branch #259 . We'll leave this PR and branch open, and push fixes here if anything comes up in testing.

@tabacitu tabacitu changed the title [Feature][0.9][Ready] Custom admin middleware [Feature][0.9][Merged] Custom admin middleware Mar 8, 2018
@tabacitu tabacitu merged commit 459d575 into master Mar 22, 2018
@tabacitu tabacitu removed the ready label Mar 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant