eneadm / ladder

Lightweight Permissions for Laravel
MIT License
245 stars 13 forks source link

Some idea questions #3

Closed slakbal closed 1 year ago

slakbal commented 1 year ago

Hi there @eneadm ... loving this simple package.... I have a couple of questions.

  1. I would like to be able to provide a cast for the the "role" property, for example then one can expect i.e. a Enum back. If no cast is provide the behavior stays as it currently is.
protected $casts = [
        'role' => Enums\Role::class, // Enum cast
];
  1. It would be great if another "morphable" column (nullable) can be added for association to another model for example an tenant model i.e. organisation, team or whatever model the morphable trait is added to. If not used the behavior stays as it currently is.

Then something like this would be possible:


//Assigning to Role for a specific organisation
$user->roles()->updateOrCreate(
            ['user_id' => $user->id],
            [
               'tennant_id' => $organisation->id,
               'role' => Role::STAFF,
            ]
        );

then if the tennant is provided it does the lookups:

// Access all of user's roles...
$user->roles($tenant) : Illuminate\Database\Eloquent\Collection

// Determine if the user has the given role... 
$user->hasRole(string $role, $tenant) : bool

// Access all permissions for a given role belonging to the user...
$user->rolePermissions(string $role, $tenant) : ?array

// Determine if the user role has a given permission...
$user->hasRolePermission(string $role, string $permission, $tenant) : bool

// Determine if the user has a given permission...
$user->hasPermission(string $permission, $tenant) : bool
  1. I've implemented the permission checks into my policies. But for every permission check a database query is done against the user_role table, as such on one page load that have 13 granular checks, the query is executed 13 times. Is there a way that the query is executed only once per request life-cycle and then only executed again in the next request?

    public function create(User $user, User $model)
    {
        return $user->hasPermission('user:create');
    }

Query executed 13 times on 1 page.

select exists(select * from `user_role` where `user_role`.`user_id` = 2 and `user_role`.`user_id` is not null and `role` = 'ROOT') as `exists`

Do you think something like this would be possible, and how? Glad to help with the implementation.

Thanks!

eneadm commented 1 year ago

Hello @slakbal,

Those are really good points, actually I thought about these thing while developing the package but for the moment it seemed out of scope and good enough to release as is.

My idea of implementing these enhancements was a bit different though. Since a user can have multiple roles in theory casting a single role is a bit limiting, but here you are correct there has to be some way to append roles and permissions assigned to the model. For the 2nd point I've thought about it and I honestly kind off regret that I didn't chose polymorphic relationships instead, I have to think about this. The 3rd point could be fixed easily actually, it is just a matter of providing more permissions to check against in one go.

Enhancements:

I will create some issues for the points above, thank you for your feedback 🙂

eneadm commented 1 year ago

The permissions() method in HasRoles has been implemented in v1.1.0. I will get back on the other requests soon :-) PR

eneadm commented 1 year ago

With the 2nd of the request regarding the performance completed PR I am going to close the issue. The last request of allowing roles to be assigned to any Model needs further thought and careful implementation in order to have backwards compatibility, in time I will create a PR for it as well. Thank you for your feedback and your help :-)