How should wildcards work?

[JosephSilber]

I'm currently working on allowing wildcard abilities. Here's how it works:

Bouncer::allow($user)->to('*');

Bouncer::allows('*'); // true
Bouncer::allows('ban-users'); // true

You can also allow all actions on a model:

Bouncer::allow($user)->to('*', $post);

Bouncer::allows('delete', $post); // true
Bouncer::allows('*', $post); // true

Bouncer::allows('ban-users'); // false
Bouncer::allows('*'); // false

You can also allow a specific action on all models:

Bouncer::allow($user)->to('create', '*');

Bouncer::allows('create', User::class); // true
Bouncer::allows('create', Post::class); // true

Bouncer::allows('edit', Post::class); // false
Bouncer::allows('create'); // false

---

So far so good. What I'm not sure about is the following:

Bouncer::allow($user)->to('*');

Bouncer::allows('view-dashboard'); // true

Bouncer::allows('delete', $user); // false

As you can see, a wildcard ability does not allow model abilities. To also allow model abilities you need two wildcards:

Bouncer::allow($user)->to('*', '*');

Bouncer::allows('view-dashboard'); // true
Bouncer::allows('delete', $user); // true

All of this is already implemented. Now onto the question:

Which one of these two makes more sense?

1. A single wildcard only allows simple abilities. Model abilities requires double wildcards.
2. There's no point in ever only allowing a user all simple abilities. Make a single wildcard allow everything.

Option 1 is the way it works now.

Thoughts?

[antonkomarev]

1st option is giving more flexibility, but it's wouldn't be so obvious behaviour. Maybe there should be alias methods which should do the trick and will be more verbose?

[JosephSilber]

Well of course there will be alias methods. This discussion is about the core functionality. The alias methods will probably only go in the Bouncer itself. I'm not sure I want to pollute the trait too much.

[Keoghan]

It think no 1 makes sense, it retains a easy way to give only simple abilities for any project that requires that.

//in my mind
//this
Bouncer::allow($user)->to('*');

//translates to
Bouncer::allow($user)->to('*', null); 
//i.e. allow these permissions which are not constrained to a particular model.

[JosephSilber]

So I guess we'll leave it the way I have it now, which is option 1.

---

Here are the aliases I've been thinking of:

Bouncer::allow($user)->to('*', '*');
Bouncer::allow($user)->all();
$user->allowAll();

Bouncer::allow($user)->to('*', Post::class);
Bouncer::allow($user)->allOn(Post::class);
$user->allowAllOn(Post::class);

Bouncer::allow($user)->to('create', '*');
Bouncer::allow($user)->onAllTo('create');
$user->allowOnAllTo(Post::class);

Bouncer::allow($user)->to('*');
Bouncer::allow($user)->allSimple();
$user->allowAllSimple();

I'm not really excited about all of these.

I'm open to suggestions.

[Gummibeer]

In my opinion there is no usecase for this - it's something like $user->is('admin'); and the wildcards can/will produce some unwanted behaviours on growing systems. If there is a need to combine abilities into groups and allow these there is the way of custom gates.

But I will never use total wildcard abilities.

[Keoghan]

Hmmm.

I don't think I'd use it directly very often on a user, but perhaps on a role - such as 'admin'. Which would avoid the need for me to have a separate check for is('admin') in my policies, and allow the access to be controlled more simply through the DB. In reality, both cases should probably be done more explicitly, but are handy when working through a prototype for example.

If we allow wildcard abilities, then I guess denies becomes a little more necessary.

In terms of naming, I ran along the lines of Bouncer::allow($user)->toDoAnything(); but it's pretty tricky to come up with something readable/sensible for the action wildcard on all :)

[ognjenm]

Hi, is there option to revoke all permissions for user.
I have UI were adding and removing permissions with checkboxes. It would be easier to revoke all permissions and then to add new permissions from array.
Great package.

[Gummibeer]

@ognjenm have you tried?

$user->abilities()->sync([]);
$user->roles()->sync([]);

This should work, if it does you also can do:

$user->abilities()->sync($abilityIDsArray);
$user->roles()->sync($roleIDsArray);

But I don't know if it will conflict with the Bouncer Cache.

[ognjenm]

Thanks @Gummibeer I can clear cache after this command.

[taiyeoguns]

Hi @JosephSilber,

Will there be a way to retract a role from all users in one go using wildcards?

So something like:

Bouncer::retract('admin'); \\retract admin role from all users

instead of only having per user:

Bouncer::retract('admin')->from($user);

[JosephSilber]

@taiyeoguns you can do it like this:

Bouncer::retract('admin')->from(User::pluck('id')->all());

If you want it to be more efficient, then only select users with that role:

Bouncer::retract('admin')->from(User::whereIs('admin')->pluck('id')->all());

---

I think this is not a common enough action to warrant a dedicated way to do it in the package. Maybe in the future we can add more helpers for such things, but for now this'll suffice.

[taiyeoguns]

Thanks, I'll try this.

[coolynx]

Wildcards without exceptions messes up things a bit.

I would have a role Admin with all possible abilities, except for some super-admin abilities that are only meant for rare cases. To get those abilities I add/remove it manually.

This would be the case to give everything to the role or to a user.

$user->allow('*');

Then make this user super admin or take away those abilities.

$user->allow('super-admin');

It will create 2 rules. super-admin rule is useless because wildcard overrides it. But you can not remove it from wildcard as well.

Bouncer::denies('super-admin');

Can remove only this tag as a record in database if it was added. But can not remove this ability from wildcard or mark it as forbidden ability.

Maybe this is the case with forbidden field in permissions table (asked in #43 )? If so, then how to get it working?

[JosephSilber]

@coolynx As I've just explained here, denying abilities may possibly come in a future release.

[autaut03]

Second variant is more logical and it'll be better. There's no point of keeping two levels of ROOT access - it's ROOT and it would be stupid, if there was another ROOT with another permissions.

[OnceWas]

I am building a system where we only allow one role per user. Abilities will be inherited by the role. If a user's role is changed, i.e. from admin to reviewer, I want to make sure that the user is only a reviewer, and no longer an admin, after the change. Something like

Bouncer::retract('*')->from($user);

would be helpful, such that when the user is then updated with Bouncer::assign($request->new_user_role)->to($user); the user is only associated with the new role.

[JosephSilber]

@OnceWas you can currently accomplish this very easily:

$user->roles->each(function ($role) {
    $user->retract($role);
});

---

Update: we now have a sync method:

Bouncer::sync($user)->roles([$request->new_user_role]);

[JosephSilber]

Here's my current thinking with regards to the method aliasing:

Bouncer::allow($user)->everything();
// Equivalent to
Bouncer::allow($user)->to('*', '*');

Bouncer::allow($user)->toAdministrate(Post::class);
// Equivalent to
Bouncer::allow($user)->to('*', Post::class);

Bouncer::allow($user)->toAlways('view');
// Equivalent to
Bouncer::allow($user)->to('view', '*');

// Only allowing simple abilities will not get an alias and will not even be documented:
Bouncer::allow($user)->to('*');

Thoughts?

[coolynx]

We are thinking about an alias for the wildcard, right? 😉 So, lets stick to it and focus only on wildcard.

If I translate a * (wildcard) to a human language, then for me it means everything, all or any (symbol). So, the simple way to call an alias would be to allow , Everything, All or Any on something specific you define or to everything.

If we take into consideration that people are lazy (like me) and like to write less code, read less documentation, remember fewer specific variations of possible namings, then for me there is only one alias left - all. 😏 Because Everything is too long to write and Any is confusing.

Bouncer::allow($user)->toAll();
// Equivalent to
Bouncer::allow($user)->to('*', '*');

Bouncer::allow($user)->toAll(Post::class);
// Equivalent to
Bouncer::allow($user)->to('*', Post::class);

Bouncer::allow($user)->toAll('view');
// Equivalent to
Bouncer::allow($user)->to('view', '*');

To complicate things and give more power and freedom to users, then there is a possibility on going Laravel way. This is just a quick thought.

Bouncer::allow($user)->toAll();
// Equivalent to
Bouncer::allow($user)->to('*', '*');

Bouncer::allow($user)->toAll(Post::class);
// Equivalent to
Bouncer::allow($user)->to('*', Post::class);

Bouncer::allow($user)->toView();
// or splitting into 3 keywords - to view all
// where view - name of the ability
// and all - wildcard
Bouncer::allow($user)->toViewAll();
// Equivalent to
Bouncer::allow($user)->to('view', '*');

Bouncer::allow($user)->toView(Post::class);
// Equivalent to
Bouncer::allow($user)->to('view', Post::class);

Bouncer::allow($user)->toAdminister(Post::class);
// Equivalent to
Bouncer::allow($user)->to('administer', Post::class);

[JosephSilber]

@coolynx thanks for your comments. Always appreciated.

Let me first address the second half of your proposal, and state quite simply that I do not want magic methods. So let's put that to rest.

With regards to the first half of your proposal I see two problems:

1. It doesn't read as fluently. We're trying to construct readable phrases as much as possible, and "to all" doesn't really speak to me as much as "to administrate posts" and "to always view".
2. From a technical POV, we cannot allow both ->toAll(Post::class) and ->toAll('view'). They're both simple strings, and there's no way to tell whether what's being passed in is a class name or an ability name.

[Keoghan]

Hi,

I like these:

Bouncer::allow($user)->everything();
// Equivalent to
Bouncer::allow($user)->to('*', '*');

Bouncer::allow($user)->toAdminister(Post::class);
// Equivalent to
Bouncer::allow($user)->to('*', Post::class);

Bouncer::allow($user)->toAlways('view');
// Equivalent to
Bouncer::allow($user)->to('view', '*');

// Only allowing simple abilities will not get an alias and will not even be documented:
Bouncer::allow($user)->to('*');

Perhaps

Bouncer::allow($user)->everything();
// would be more consistent as
Bouncer::allow($user)->toAdministerEverything(); 

Would you consider applying the same to roles?

Part of me feels like being able to say the following would be nice, but a nightmare/impossible to implement alongside the simple abilities

Bouncer::allow($user)->to('view')->everything();
// instead of 
Bouncer::allow($user)->toAlways('view');

[JosephSilber]

@Keoghan toAdministrateEverything definitely reads better, but no one wants to type that...

For the other one: I truly wish we could do something like this:

Bouncer::allow($user)->to('view')->everything();

But I don't think it's possible...

[Arcesilas]

Hi,

One possibility would be to have objects for abilities. But yeah, it's a little heavy.
Think of the ability to Read (in CRUD oriented semantics) a model:

use Silber\Bouncer\Abilities\Read;
Bouncer::allow($user)->to(Read)->everything();

Makes it possible. Of course, since method to() can take an array of abilities, it has to me a little more complex than it is currently (iterate over each ability passed as argument to check whether it's a string or an Ability instance): give abilities which are strings, and chain (returning $this) to let another method to handle the other abilities (which would be Ability instances).

Not sure it's relevant. Don't know if anyone may like it, it's just a way to make it possible.

It's not necessary to create hundreds of Ability classes: Create, Read, Update and Delete may be enough. Users could be free to create their owns if they like.

[JosephSilber]

I don't think the complexity is worth it.

[Keoghan]

For giggles I put together a rough piece of code that would allow a method to 'lookahead' to see if it's being chained.

Then ->to('view') could either return an object to chain onto, or perform an action.

https://github.com/Keoghan/lookahead

[JosephSilber]

@Keoghan cool proof of concept!

Not that I'd ever ship anything like that 😆

[Arcesilas]

I'm wondering if wildcards as they are currently planned allow developper/admin to grant abilities on a user's own items.
For instance, I'd like to allow a user to do whatever he wants with his own posts: view, edit, delete, etc. Of course, it's possible to allow on a model, ie on an instance:

Bouncer::allow($user)->to('*', $post);

But this means many database entries: one for each pair user/post. If we consider we have to do it for each post for each user, it may grow fast.

Could something like:

Bouncer::allow('users')->to('*', App\Post::class, 'user_id');

be possible?
The third argument, optional, is the column of the table to scope the permission: compare $user->id with $post->user_id. If not set, it means User can do anything with any App\Post

If current user belongs to users group, then he can view/edit/delete/create an App\Post if he owns it ($post->user_id == $user->id).

[JosephSilber]

@Arcesilas The scope of a database-bound ACL system (or at least Bouncer's scope) is to record static permissions. Custom logic like that belongs in the application code.

Bouncer allows you to define regular callbacks on the Gate itself:

Gate::define('edit', function ($user, $post) {
    if ( ! $post instanceof Post) {
        return null;
    }

    return $user->id == $post->user_id;
});

If you want to allow all actions on a post, use a policy:

class PostPolicy
{
    public function before($user, $ability, $arguments)
    {
        return $arguments[0]->user_id == $user->id;
    }
}

[Arcesilas]

That's right! Thanks for your answer, which reminds me that Policies are still useful! :)

[JosephSilber]

Just pushed up wildcard aliases with the following syntax:

Bouncer::allow($user)->everything();
Bouncer::allow($user)->toManage($post);
Bouncer::allow($user)->toManage(Post::class);
Bouncer::allow($user)->toAlways('view');

[JosephSilber]

@Arcesilas with the new alpha 3 release, you don't need that policy anymore. You can replace it with this simple call:

Bouncer::allow($user)->toOwn(App\Post::class);

If you only want to allow a single ability on owned models, you can pass that as a second argument:

Bouncer::allow($user)->toOwn(App\Post::class, 'view');

You can also pass an array of abilities you want to allow on owned models:

Bouncer::allow($user)->toOwn(App\Post::class, ['view', 'update']);

[Arcesilas]

Hey ! That is really great ! Thanks a lot for your work !

[JosephSilber]

@Keoghan I actually got this to work:

Bouncer::allow($user)->to('view')->everything();

[Keoghan]

@JosephSilber awesome stuff. You’ve put a whole lot of work into bouncer over the last year!