Proposal: New plugin hook for op status

[dgw]

The current hardcoded criteria (member of Bucket's control/debug channel or op/protected/owner in the main channel) are proving to be pretty limiting for my instance. But I see no reason to advocate changing those based on my use of the bot.

Letting plugins hook in and grant users op status based on other criteria would be handy, though. I'm not sure at this point if the hook should also support overriding the built-in criteria. It could be two hooks, e.g. extra_op_checks to add criteria and op_check_overrides for criteria that will set $bag{op} to 0 even if the default criteria are met.

I realized that this simple idea could potentially bring a lot of complexity, which is why I made an issue for discussion instead of diving right in with code and a PR. That said, the original idea of adding a single hook for plugins to grant certain non-chanops Bucket op status is the main thing here; anything else that might happen along with it is just extra.

Self-assigned because I will write and test the code once the nature of the hook(s) is decided.

[zigdon]

I think what you are suggesting here is a permission system, that can be independent of the current one (that is enforced by ircd?)

I can imagine that, an API that will allow different plugins to ask 'is user X allowed to Y'. Currently, it's mostly all-or-nothing, but that can be just one implementation of the system.

So plugins can ask 'is the user a superuser' which is the same as they do now, or ask more specific questions like 'is the user allowed to foo', with other plugins providing the definition of what is 'foo'.

In general, I think that would be a nice improvement, and should not be too bad to implement the framework.

• Implement &Can(string) -> bool (which can replace $bag{op}
• &RegisterAcl(string, callback) allowing plugins to add new ACL types

And probably a tri-state setting 'AllowACLAppend' to decide what happens if a plugin tries to register an ACL that is already there (including the default one). If false, it'll fail to load, if 'AND', require all checks to pass, and if 'OR' require at least one check to pass.

Is that over complicated?

[dgw]

It's more than what I was trying to do, but it'd be worth implementing. The main question I have now is would it be worth adding the hook in the short term while the permissions system gets hashed out? (I can just use the hook in my own fork and silently replace it when permissions are done, too.)

I can see this being a good project for expanding my comfort zone with Perl further. I'm pretty clear on what &Can is meant to do; it's the same concept as WordPress's current_user_can() function—or maybe it would be better to be more like user_can() instead, taking a nick as first parameter. Regardless, it'd presumably just return true if the user is an op (maintaining the current system).

Not quite clear on what the string and callback in &RegisterAcl would be, though. I'm guessing, string is the corresponding capability that would be passed to &Can. The callback therefore points to a plugin function that decides whether the passed-in nickname is allowed (meaning callback must take a string)? Or perhaps I've misunderstood your proposed structure.

One thing that occurs to me is that AllowACLAppend should probably be per–ACL type, not global for all ACLs. Now maybe that's over complicated…

[zigdon]

callback would be a function reference, that we can store in a hash.

Let's say we only allow one callback per type (that is, AllowACLAppend is false).
RegisterAcl('foo', &my_callback) -> adds the callback reference to $perms{'foo'}
Can('foo', $user) -> returns &{$perms{'foo'}}($user)

(I think that's perl syntax, I've been writing Go and Python lately)

Does that make sense?

And as far as AllowACLAppend, yeah, it might make sense to be a dict, though having a default value might be a good enough start.

[dgw]

Yep, that makes sense. (I totally get the syntax thing…been writing mostly Python lately myself.)

So for AllowACLAppend = AND or OR, execution of &Can branches to call each function reference in turn, returning false if it gets back a false from a callback (AND mode) or returning true once it finds one callback that returns true (OR mode).

Think I got it. :)

As for the setting itself, maybe for each ACL hash entry, there should be an options key and a callbacks key. So (taking your code example), $perms{foo}{callbacks} contains an array of function refs, and $perms{foo}{options} contains a hash of settings (currently only $perms{foo}{options}{AllowACLAppend}). Or those can be inverted, to separate options from callbacks; that's an implementation detail, probably more important if there will eventually be more per-ACL settings. Not sure if it would be prudent to handle the hash elements differently depending on the append setting, anyway. It would be odd to call $perms{foo} directly if AllowACLAppend eq false, but treat it as an array if one of the other values is set…

[zigdon]

Yeah, I'd implement both cases as arrays, and just check during registration -- if we're not allowing more than one callback, return failure when trying to add to an existing array.

[dgw]

return failure when trying to add to an existing array.

And avoid introducing another #62-type bug!