vincent15000's avatar

A roles table or an enum to handle roles ?

Hello,

It's the logical continuation of my previous post about roles design.

I will work with non hierarchical roles.

I need 5 roles :

  • superadmin
  • admin
  • manager
  • assistant
  • foreman

The superadmin and admin roles will be stored directly in the users table.

Where do you suggest me to declare the other roles ?

When the superadmin is connected, he can update any user and in the user form he has access to all roles except the superadmin one.

When the manager is connected, he can update any user in his team and in the user form he has access only to the assistant and foreman roles. If I do that with an enum RoleEnum, it's very easy to do.

But what if I save the roles into the database ? The only way I see is to add additional fields in the roles table, like visible_for to specify if the role is visible for the superadmin, the admin, the manager, ... in the user form to fill the select / options field. But the roles table won't be changed anymore during all the application life, so is it really a good idea to use the database to store the roles ? These are really static roles.

Thanks for your advice.

V

0 likes
29 replies
OussamaMater's avatar
Level 37

I would say start as simple as possible. Its always cheaper to add code than to maintain complexity that probably solves nothing.

I don't see why you wouldn't just store the rest of the roles in the users table like you already do with superadmin and admin. Just use a string column instead of an enum, its much easier to add or remove roles, no need for "alters".

You can still cast the role column to an enum, and then define policies/gates to allow or deny actions, like what you are saying.

If the system eventually gets more complex, users with multiple roles, roles with different permissions, maybe even more auth guards so the user model isnt the only one with roles, you will end up using a battle-tested package anyway, like Spatie's laravel-permission. Until then, like you said, "These are really static roles", sp keep it simple, future you will thank you.

1 like
vincent15000's avatar

@OussamaMater

I don't see why you wouldn't just store the rest of the roles in the users table like you already do with superadmin and admin.

For the moment I evaluate if it's better to assign several roles per user or a unique role.

With your suggestion, it's only for a unique role per user, otherwise the role field would be a JSON field and in this situation it's really not recommended.

1 like
OussamaMater's avatar

@vincent15000 Well, that wasn't mentioned. If you know you will assign multiple roles to a user, then yes, it won't work.

My advice still stands: start as simple as possible. When complexity arises, deal with it then.

1 like
vincent15000's avatar

@OussamaMater If I have to keep it simple at the beginning, an enum could be sufficient, I will just have to write the policies accordingly with quite more complex rules.

martinbean's avatar

@vincent15000 Depends if you want adding a role to be a code change and require a deployment, or adding/removing a record in a database.

1 like
vincent15000's avatar

@martinbean The application really won't have any further role in the future, it's a little application with very few functionalities.

Sure, far away in the future, I don't know what the application can become and what the client will want. But for now static roles are probably sufficient. And the client doesn't want to manage the roles.

Some months ago, I have added dynamic roles / permissions to an application, it's not so complicated to do, but it needs more time to write the code, even if I use a package like the Spatie one.

jlrdw's avatar

@vincent15000 For a small app a comma separated field works, then:

    public static function chkRole($role = null)
    {
        $userrole = Auth::user()->role;
        $checkrole = explode(',', $userrole);
        if (in_array($role, $checkrole)) {
            return true;
        }
        return false;
    }

When checking if the role matches the required role in a method.

Of course a query scope or added in the query would be used for a user to see their own data, or admin all.

2 likes
Snapey's avatar

you need to absolutely define if a user can ever have more than one role.

If the answer is yes or possibly, then create a json column on the users table table listing roles

If the answer is a definite no then a string column on the users table is all you need

1 like
Glukinho's avatar

@Snapey

create a json column on the users table table listing roles

Why not pivot table role_user plus BelongsToMany roles() relation?

3 likes
jlrdw's avatar

@Glukinho It's contained in Auth. If Bob had two roles (admin,bookkeeper).

$userrole = Auth::user()->role;

$userrole would be admin,bookkeeper for Bob.

If a method required a role for bookkeeper then:

        if (in_array($role, $checkrole)) {
            return true;
        }

Would return as true. A full query is not necessary.

The required role is passed to the method:

public static function chkRole($role = null)

Here bookkeeper would be passed. However I use bkeep instead.

I just prefer a short comma separated list instead of messing with JSON. But same idea.

I really don't see a need for a roles table and messing around with pivot tables for such a simple thing.

1 like
JussiMannisto's avatar

@jlrdw

I really don't see a need for a roles table and messing around with pivot tables for such a simple thing.

Your app has 10k users. Now you need a view in your admin panel where super admins can search users using different filters, including role. That's not practical if roles are buried in a string column.

I'm not saying you can't use a single column, but I'd go for a pivot table because it's more proper. It takes two migrations, one model, and two relation functions.

I just prefer a short comma separated list instead of messing with JSON.

Laravel has built-in casting for JSON, so you're only dealing with arrays in code. For a comma-separated list, you'll have to write a custom cast to convert between strings and arrays. I'd say that's more messy.

1 like
Glukinho's avatar

@vincent15000 array cast doesn't give you (array) ['user', 'manager', 'admin'] from (string) 'user,manager,admin' AFAIK

It's not hard to make your custom cast for it, but I don't see a reason why use anything custom instead of a relation which is native, obvious and scalable solution.

1 like
vincent15000's avatar

@JussiMannisto @glukinho

You can do it easily like this.

protected $casts = [
	'roles' => 'array',
];
$user->roles = ['admin', 'member'];

So effectivement not from a string, but from an array.

1 like
JussiMannisto's avatar

@vincent15000 What on earth are you on about?

I said that you need a custom cast if you want to store an array as a simple comma-separated list, which was suggested above.

You said you don't need a custom class, because you can just use the array cast. That DOESN'T WORK for a comma-separated list, which is just a string. Both @glukinho and I pointed this out to you. But now it seems you've lost the plot, and you're talking about a different format.

1 like
JussiMannisto's avatar

@jlrdw Yes, but the point was that if you store the roles as a comma-separated list, you need to convert that string back to an array. If you use json or array casting, Eloquent does the conversions automatically, and you only ever deal with arrays.

1 like
jlrdw's avatar

@JussiMannisto There is no automatic, you have:

            public function get($model, $key, $value, $attributes)
            {
                if (! isset($attributes[$key])) {
                    return;
                }

                $data = Json::decode($attributes[$key]);

                return is_array($data) ? new ArrayObject($data, ArrayObject::ARRAY_AS_PROPS) : null;
            }

But explode is just as fast:

     public static function chkRole($role = null)
    {
        $userrole = Auth::user()->role;
        $checkrole = explode(',', $userrole);    ////// here
        if (in_array($role, $checkrole)) {
            return true;
        }
        return false;
    }

One you decode, one you explode, so no big deal.

Edit:

Also it is easy to query a comma separated list, say I want to see who has bookkeeping access:

SELECT 	`id`, 
	`name`, 
	`username`, 
	`role`
	FROM 
	`users`
	WHERE `role` LIKE "%bkeep%";
1 like
Glukinho's avatar

@jlrdw

  1. you have a role 'bkeep' and you add a new role 'bkeep_other' => your where role like... query must be rewritten otherwise it gives wrong result. You have to remember it yourself, but you certainly will not and your app has wrong behaviour, silently. We are talking about roles, so it's direct security threat.

  2. you have 10 000 users => you have performance problems as ...like... query is inefficient and slow.

1 like
JussiMannisto's avatar

@jlrdw This is no good:

WHERE `role` LIKE "%bkeep%";
  1. If you look for "admin", you'll get both admins and super admins.
  2. No indexes can be used, and the entire table must be scanned on every query, as @glukinho alluded to.

You can keep exploding/imploding to convert the data, but it's not as clean as an automated cast.

Edit. Wait, are you suggesting that you have to manually override the getter and setter methods when storing the roles as json or array? You don't need to do any of that. Just add this one line to your model:

protected $casts = ['roles' => 'array'];

Then you can use the property like an array:

foreach($user->roles as $role) {
	dump($role);
}

$user->update([
	'roles' => ['foo', 'bar']
]);
1 like
jlrdw's avatar

@JussiMannisto Couldn't a super admin be called the HMFIC instead?

That's an Old Air Force acronym we used in the 1970's.

But yes a huge app my roles table would be more robust. But @vincent15000 stated a few roles only.

But I have used a comma separated field with users with more than one role with no problems.

Just example

  • Bob is an admin

  • Suzy is admin and does bookkeeping

  • Mary is a bookkeeper only

  • If Bob is logged in, Bob can only do admin stuff and all access to user stuff. But Bob cannot mess with bookkeeping.

  • If Suzy is logged in she can access admin stuff and bookkeeping and accounting stuff.

  • If Mary is logged in she cannot mess with admin stuff, but has access to bookkeeping and accounting stuff.

So I just check at method level if the logged in users role can or cannot access that method / function.

And use query scopes to let a user edit / view their own data or an admin can access all users data.

Each app will be different as to who can do what.

So in pseudocode:

public function makeInvoice()
    {
        if (a required role of bkeep is not true here) {   // bkeep = bookkeeper
            return redirect('somewhere'); // whereever you redirect to if not authorized
        }
        // Rest of method here is accomplished if 
        // the logged in user has the required role of 'bkeep'.
    }

The method is just an example.

No two apps are the same, every app will need authorization or RBAC setup tailored to the specific apps requirements.

1 like

Please or to participate in this conversation.