Allow setting ACLs at plugin level

[mchlrhw]

I feel that plugins should represent more than just containers for commands.
I realise that plugins allow you to install/uninstall/enable/disable commands en-masse, but I feel they could be used for more.
For example, when configuring access controls it is convenient to specify the access controls per plugin rather than for each command, especially if the plugin contains many commands.

The following code snippet (taken from a modified version of errBot.py) achieves the desired effect, and also supports per command rules within a plugin:

    def inject_commands_from(self, instance_to_inject):
        classname = instance_to_inject.__class__.__name__
        for name, value in inspect.getmembers(instance_to_inject, inspect.ismethod):
            if getattr(value, '_err_command', False):
                commands = self.re_commands if getattr(value, '_err_re_command') else self.commands
                name = getattr(value, '_err_command_name')

                if name not in self.bot_config.ACCESS_CONTROLS and classname in self.bot_config.ACCESS_CONTROLS:
                    self.bot_config.ACCESS_CONTROLS[name] = self.bot_config.ACCESS_CONTROLS[classname]

You would then be able to configure access like so:

ACCESS_CONTROLS_DEFAULT = {
    'allowusers': BOT_ADMINS,
}

ACCESS_CONTROLS = {
    'help': {},
    'EasterEggs': {},
    'for_my_own_amusement': {'allowusers': BOT_ADMINS},
}

Where EasterEggs would be a plugin that could contain many commands, including the for_my_own_amusement command.

However, doing if name not in self.bot_config.ACCESS_CONTROLS and classname in self.bot_config.ACCESS_CONTROLS feels like a fragile hack, whereas it could be supported natively if the command/plugin relationship was maintained throughout the lifetime of the bot (i.e. if, when a command was injected into the bot, the command name was mapped to a plugin as well as to the command's method/function).

Also, I've noticed that if HIDE_RESTRICTED_COMMANDS = True and a plugin only contains restricted commands, then the plugin name still shows up in !help, even though the output of !help plugin_name is empty. I feel that this goes against the desired effect of hiding restricted commands.

These cases would probably be easier to handle if plugins were tracked along with the commands they contain. I just wanted to see what other people's thoughts on this were before I submitted a pull request.

[gbin]

I haven't give it much thoughts but why not simply allow wildcarding and plugin namespaces ?
Instead of 'command' -> ACL we could do plugin.command -> ACL or plugin.* -> ACL or more elaborate constructs like plugin.*_admin -> ACL

[mchlrhw]

Yeah sure, I've got no problem with that. I just felt that if you wanted to do more complex things at the plugin level (like hiding a whole plugin from the output of !help) then it would make it easier if commands weren't disassociated from their plugin as soon as they are loaded. I tried to bind the plugin name to the command using the @botcmd decorator, but the class isn't accessible at that point because the command is a function and not a bound method. As soon as it becomes a bound method though you lose the ability to use setattr on it.

[zoni]

So I think that

However, doing if name not in self.bot_config.ACCESS_CONTROLS and classname in self.bot_config.ACCESS_CONTROLS feels like a fragile hack, whereas it could be supported natively if the command/plugin relationship was maintained throughout the lifetime of the bot (i.e. if, when a command was injected into the bot, the command name was mapped to a plugin as well as to the command's method/function).

and

Yeah sure, I've got no problem with that. I just felt that if you wanted to do more complex things at the plugin level (like hiding a whole plugin from the output of !help) then it would make it easier if commands weren't disassociated from their plugin as soon as they are loaded.

raise some very good points, but in terms of complexity this might be a lot of work. I do however think that being able to set ACLs at the plugin level instead of individual command level would be very welcome and would like to try and get something like that in for 4.0 rather than later simply because it might require changes to the ACL format, and I'd rather introduce that in a major version upgrade than a minor one.

I'm going to rename the issue title to reflect that, but as I said, the more general point still stands (I just think it might be too much work for 4.0 and may need to wait until further in the future)