Voyager: Warning in Roles: the user can become an admin

[Gravitana]

• Laravel Version: 5.5.20
• Voyager Version: 1.0
• PHP Version: 7.1.7
• Database Driver & Version: MySQL 5.7x64

Description:

If the user is not 'admin' and do not have access to 'roles', but he can 'browse_admin', then he can change his role to 'admin' from 'Profile'

Steps To Reproduce:

Dashboard -> navbar -> dropdown -> profile -> Edit my profile -> role

[Victor-emil]

I am facing the exact same issue.

[Ishodnikov]

There is a solution?

[fletch3555]

The solution is to override/customize the user-edit view to add a check around that field checking against the current user.

The issue is complex from Voyager's point of view, since there's no hierarchy with the roles. So we can fairly easily block non-admins (admin role) from changing the role, but if you have admin, editor, publisher, etc, we can't possibly know you want editors to be able to edit, but not publishers. The best solution would be to add a custom permission around it, but even that's flawed (should editors be able to make themselves admin?). The best solution would be a hierarchical role system where you can always edit the role down, but never up. Obviously that's a fairly significant change.

tl;dr this is complex and very much under discussion

[Ishodnikov]

Custom user Controller, function update:

after
// Check permission $this->authorize('edit', $data);

add
if(! Auth::user()->hasPermission('edit_roles')) { unset($request['role_id']); }

[dfk7677]

I think the best solution would be for a different (new) view to be used when editing one's own profile instead of using the generic edit-add.blade.php.

In that view the user either cannot change their own role at all (that could be done only when editing users), or permission logic can be introduced to show or hide the specific Role element. Of course this kind of logic could be introduced in the generic view for editing, but I think such a change would increase the load on a lot of views without reason and it would be more complicated.

[fletch3555]

@dfk7677, the "generic" edit-add view is bread/edit-add.blade.php, but we could easily create a users/edit-add.blade.php to do this. It's still a "generic" view in the sense that it's used for all users and shared for edit and add actions, not just "profile". I'd prefer to avoid creating a separate view just for "profile", as that sets the precedent for a number of other changes.

[dfk7677]

Is it possible to disable profile editing in the backend for now, as it is a security risk for anyone that wants to use Voyager with roles other than admin?

[fletch3555]

Not really. You can just remove the route (or override it)

[dfk7677]

So what is the complete solution? What @Ishodnikov suggested along with a custom users/edit-add.blade.php which doesn't show (or disables) the role dropdown?

[Ishodnikov]

Create custom controller for BREAD users:

<?php

namespace App\Http\Controllers;

use Illuminate\Database\Eloquent\Model;
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Auth;
use TCG\Voyager\Events\BreadDataUpdated;
use TCG\Voyager\Facades\Voyager;
use TCG\Voyager\Http\Controllers\Traits\BreadRelationshipParser;

class AdminUserController extends \TCG\Voyager\Http\Controllers\VoyagerBreadController
{
    use BreadRelationshipParser;

    // POST BR(E)AD
    public function update(Request $request, $id)
    {
        $slug = $this->getSlug($request);

        $dataType = Voyager::model('DataType')->where('slug', '=', $slug)->first();

        // Compatibility with Model binding.
        $id = $id instanceof Model ? $id->{$id->getKeyName()} : $id;

        $data = call_user_func([$dataType->model_name, 'findOrFail'], $id);

        // Check permission
        $this->authorize('edit', $data);

        if (!Auth::user()->hasPermission('edit_roles')) {
            unset($request['role_id']);
        }

        // Validate fields with ajax
        $val = $this->validateBread($request->all(), $dataType->editRows);


        if ($val->fails()) {
            return response()->json(['errors' => $val->messages()]);
        }

        if (!$request->ajax()) {
            $this->insertUpdateData($request, $slug, $dataType->editRows, $data);

            event(new BreadDataUpdated($dataType, $data));

            return back()
                ->with([
                    'message'    => __('voyager.generic.successfully_updated')." {$dataType->display_name_singular}",
                    'alert-type' => 'success',
                ]);
        }
    }
}

At page http://site.com/admin/database/users/bread/edit set Controller Name "AdminUserController"

[emptynick]

This was changed in #2290 (1.x)

[emptynick]

Closing as this will be fixed in 1.1

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. If you have further questions please ask in our Slack group.