Rename the Backpack Admin middleware to BackpackAdmin

[ChrisThompsonTLDR]

Admin is a common middleware name. Backpack is forcing its middleware, named admin, into the Admin namespace.

[tabacitu]

True. And people might actually want to overwrite this admin middleware to only include users with a certain permission/role.

So I'm now wondering - wouldn't it make sense to publish the middleware in the application folder? Or would it only be confusing (middleware lives inside the application, but all controllers live inside the vendor folder).

What do you think?

[OwenMelbz]

If this is going to be something thats going to be worked on, I'd like to heavily suggest that we make sure its compatible with laravels $guard system.

We had a massive issue that meant the admin and frontend shared user sessions, because they both used Auth::check() so we had to pull out the backpack code and use Auth::guard('backpack')->check() instead, which caused conflicts with the other packages liek settings

[ChrisThompsonTLDR]

Another option might be a user model trait. Similar to what I did here: https://github.com/ChrisThompsonTLDR/impersonate

The traitor package can be used to cleanly publish your trait into the user model of the application: https://github.com/kkszymanowski/traitor

[tabacitu]

How about if we publish the middleware by default? This usually needs to be customized. It's part of my suggestion here: #101.

Also, I've noticed Laravel is going for verbs&sentences in naming middleware: CheckAge in their example, and these for middleware groups:

        \App\Http\Middleware\EncryptCookies::class,
        \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class,
        \Illuminate\Session\Middleware\StartSession::class,
        \Illuminate\View\Middleware\ShareErrorsFromSession::class,
        \App\Http\Middleware\VerifyCsrfToken::class,
        \Illuminate\Routing\Middleware\SubstituteBindings::class,

So maybe we should name the admin middleware something like CheckIfAdmin / CheckAdmin / VerifyIfAdmin / VerifyAdmin? I wouldn't include "Backpack" in the middleware name, as I don't see the middleware as backpack-specific. I think it's just a middleware that yes, is also used in Backpack, but you could just as well use it in the front-end, for some admin-only functionality.

What do you guys think? Any better ideas for a name?

[OwenMelbz]

@tabacitu I thiiiiiiiiinkkk...its silently addressed within -> https://github.com/Laravel-Backpack/Base/pull/87/files#diff-bf16878a4c2c226c376eb91300705856 :D

[ctf0]

a temp solution for any new users until this gets merged

• config

'setup_dashboard_routes' => false,

• routes

Route::group(['prefix' => config('backpack.base.route_prefix')], function () {
    Route::get('dashboard', 'AdminController@dashboard');
    Route::get('/', 'AdminController@redirect');
});

• create a middleware like u usually do

'BackpackAdmin'         => \App\Http\Middleware\BackpackAdmin::class,

• controller

<?php

namespace App\Http\Controllers;

use Backpack\Base\app\Http\Controllers\AdminController as Backpack;

class AdminController extends Backpack
{
    public function __construct()
    {
        $this->middleware('BackpackAdmin');
    }
}

[eduardoarandah]

I think is important for every backpack package:

• Allow to manually register routes, as they commonly need more middleware
• Allow to easily extend Controllers, as they commonly need to check permissions and more methods

Out of the box, all users are capable of doing everything, and is kind of difficult to understand how to insert permission middleware when all routes and controllers are inside vendor folder.

[lloy0076]

@tabacitu - is this still valid if/when all the routes get published? And assuming that happens, will this pull request apply cleanly?

[tabacitu]

@lloy0076 this is a breaking change, we'll need to tell the users to change the "admin" middleware to "backpack"/"backpackadmin" wherever it's used. We'll also need to do that ourselves inside all packages...

[tabacitu]

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;

Thoughts?

[tabacitu]

Just pushed PR #258 with all of the above. Let's move the conversation there please.