[Suggestion] Ability to assign roles to user and/or model entity/id

[konovalov-nk]

Basically, I've already implemented the feature, see this fork.

Here how it works:

/** @test */
public function itCanCheckRolePermissionsForModelId()
{
    $objRole        = new Role();
    $roleAttributes = [
        'name'        => 'Admin',
        'slug'        => str_slug('Admin role', config('laravel-auth.slug-separator')),
        'description' => 'Admin role descriptions.',
    ];
    $role           = $objRole->create($roleAttributes);

    $objPermission        = new Permission();
    $permissionAttributes = [
        'name'        => 'post',
        'slug'        => [
            'create' => true,
            'view'   => true,
            'update' => true,
            'delete' => true,
        ],
        'description' => 'manage post permissions'
    ];
    $permission           = $objPermission->create($permissionAttributes);

    $role->syncPermissions($permission);

    $user           = new User();
    $user->username = 'Role test';
    $user->email    = 'role@test.com';
    $user->password = 'RoleTest';
    $user->save();

    // Assign user role to model entity & id.
    $example_model    = 'example_model';
    $example_model_id = 42;
    $user->assignRole($role, $example_model, $example_model_id);

    $this->assertEquals($user->getRoles(), [ 1 => str_slug('Admin role', config('laravel-auth.slug-separator')) ]);
    $this->assertEquals($user->getPermissions(), [ 'post' => $permissionAttributes['slug'] ]);

    // Permissions given to certain model and id.
    $this->assertTrue($user->able('create.post', $example_model, $example_model_id));
    // User doesn't have permissions on all models and ids.
    $this->assertFalse($user->able('create.post'));
    // User role must have exact model id.
    $this->assertFalse($user->able('create.post'), $example_model, 43);

}

I'm using it like this:

<div class="dashboard-tab-menu">
    <ul>
        @able('view.volunteers.organization', PERMISSION_ORGANIZATION, $organization_id)
            <li>
                <a href="{{ url("volunteers/manage?id={$organization_id}#content-section") }}"
                   class="{{ set_active('volunteers/manage') }}">
                    Volunteers
                </a>
            </li>
        @endable
        @able('view.opportunities', PERMISSION_ORGANIZATION, $organization_id)
            <li>
                <a href="{{ url("opportunities/manage?id={$organization_id}#content-section") }}"
                   class="{{ set_active('opportunities/manage') }}">
                    Opportunities
                </a>
            </li>
        @endable
        @able('view.roles.organization', PERMISSION_ORGANIZATION, $organization_id)
            <li>
                <a href="{{ url("organization/{$organization_id}/roles") }}"
                   class="{{ set_active_route("organizations.invites-roles.get", 'active', ['id' => $organization_id]) }}">
                    Roles & Invites
                </a>
            </li>
        @endable
    </ul>
</div>

This allows me to assign roles to user per entities. For example, users could give other users permissions to edit their posts, etc. Much like groups in a facebook.

Should I clean up my code a little bit and make a pull request?

[kodeine]

@konovalov-nk sure

[konovalov-nk]

@kodeine ok, thanks. I've also fixed some issues as well.

Permission slugs.

• First, it should give a meaningful error when the user uses too many values for 'slug' parameter. It is a simple VARCHAR string and I've encountered a database error when tried to assign too many permission rules to single permission.
• I've increased slug size to 1024 characters (MySQL limit is 3072, so that would work with 3-byte UTF-8 characters). I'm not sure if that is right though or probably some RDBMS doesn't support such long VARCHAR strings. What should we do? Anyway, developers could modify migrations but I suggest adding documentation and best practices for this case.

Roles and permissions relations from actual model instances.

When I've tried to use this package on existing database models like that:

$user_admin = \App\User::first();
$user_admin->assignRole(ROLE_ADMIN);

// $user_admin->getRoles is empty!
$this->assertEquals($user_admin->getRoles(), [
    1 => ROLE_ADMIN
]);
// $user_admin->getPermissions is empty!
$this->assertEquals($user_admin->getPermissions(), $admin_role->getPermissions());

It wouldn't work for me. However, new created user works:

$user_admin = new User();
$user_admin->firstname = 'Role';
$user_admin->lastname = 'test';
$user_admin->newsletter = 0;
$user_admin->email = 'role@test.com';
$user_admin->password = 'RoleTest';
$user_admin->save();

$user_admin->assignRole(ROLE_ADMIN);

// Test passes.
$this->assertEquals($user_admin->getRoles(), [
    1 => ROLE_ADMIN
]);
// Test passes.
$this->assertEquals($user_admin->getPermissions(), $admin_role->getPermissions());

I had to modify core code to fix that behavior (see this commit). Wonder why no one encountered this on Laravel 5.3

[kodeine]

@konovalov-nk
sounds good, please post the PR, also for slug size to 1024 characters we can mention in the wiki as well.

[konovalov-nk]

@kodeine
There is something I'm not sure about, though. I have modified assignRole() and hasRole() methods, but haven't touched the can() method and the code is actually breaking DRY principle because I didn't have enough time to implement it in the right way, so I've added able() method just to make sure I'm not breaking anything. Some features like syncRoles are not working at all with models/ids because I only needed assignRole(), hasRole() and able().

So, I guess it would be better if I spend some time to merge able() method into can() and write additional unit tests to cover new feature. I think I need a week or two to get this done.

Should I make pull request now or it's better to fix everything and send proper pull request later? ;)
I'll make pull requests for slug size and getRoles(), getPermissions() now, though.

[kodeine]

@konovalov-nk please make a proper PR when you have time :) no rush.
You can submit the PR's which you think are good enough.

[konovalov-nk]

@kodeine Sure, thank you for help!

[konovalov-nk]

@kodeine I have one more question.
This project uses semantic versioning.
I've made breaking changes to API. For example:

From:

public function can($permission, $operator = null, $mergePermissions = [])

To:

public function can($permission, $model = '', $reference_id = 0, $operator = null, $mergePermissions = [])

Possible outcomes:

1. Bump version to 2.0?
2. Add separate method able() for backward-compatibility?
3. It's ok because master is still version 0.x and 1.0@dev is not completed yet?

able() version (for backward-compatibility):

public function can($permission, $operator = null, $mergePermissions = [])
{
    $model = '';
    $reference_id = 0;
    return $this->able($permission, $model, $reference_id, $operator, $mergePermissions);
}

[konovalov-nk]

@kodeine I've decided to make 3rd outcome since there are a lot of things changed. See pull request.