This repository was archived by the owner on Nov 23, 2020. It is now read-only.
[Feature][0.9][Merged] Custom admin middleware#258
Merged
Conversation
…Backpack/Base into custom-admin-middleware
…ude multiple classes
7 tasks
Member
Author
|
Merged into upgrade branch #259 . We'll leave this PR and branch open, and push fixes here if anything comes up in testing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds upon #67 . I'm going to copy-paste my last comment from there, which explains everything this PR does.
Update: I still agree
adminwas 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.phpwe'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:
adminkey for the middleware to anything he wants;backpack_middleware()instead ofconfig('backpack.base.middleware_key'); easiler to remember; easier on the eyes;Other considerations:
adminname for the middleware will make this backwards-compatible; a non-breaking change; but we could force the name change tocheckIfAdminto make it more Laravely and make sure people upgrade their Backpack correctly; food for thought;adminmiddleware is very basic; it just checks for authentication and makes sure the redirect uses theroute_prefix; that's the only reason we didn't use theauthmiddleware; so I expect anyone who has both users & admins to change this middleware to something else; wether they use PermissionManager or not;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.