Whitelist versus blacklist

[Deuteu]

Currently, admin and user commands are manged by blacklist (admin_commands configuration).

Wouldn't be more suitable to use whitelist ? Fisrt of all, because it's less important to forget an element on a whitelist than an element on a blacklist? And, in my opinion, it would be easier to add intermediary roles (eg: moderator, caster, ...).

What's your opinion about that? Can I make some pull request about that or it's unnecessary you want to stay in blacklist organisation?

[FrNico]

To be more specific, today, by default all commands are accessible to all users. And then we are blacklisting some by declaring them "admin only". I agree that it will make more sense to not have any commands available by default if no config is present, and then whitelist commands for each role. Default config file should obviously include commands like optout, subscribe, mention...

It will also be easier to implement role that way I guess. Cf.: https://github.com/nylonee/hangupsbot/issues/121

[nylonee]

I can see the benefit in this architecture, however because it's a large change, we'd need migration tools for those on the older bots, plus a rewrite of documentation etc.

Migration would be a bit confusing, you'll have to find all plugins, then subtract the ones already on the config, then append the remaining ones to the config.

[Deuteu]

By migration tools, what exactly are you expecting? A script that automaticly does it? (I can't do that) Or a explanation text on how to update your plugin? (I can that.) Yeah I missed the documentation.

All plugins? I've all ready check (and update if needed) all plugins present in this repo. And why change the config? I've allready deleted admin_commands and added user_commands. Indeed every administrator of a bot need to rewrite the config of his bot but I can't do it my self, every owner has his own cofig but at least if he doesn't update his config, his bot would only be more restrictive to user until he does the update.

I argued to be sure of what you want and not because I think it is not necessary, cause I really want this archiecture and I'm willing to do my best to push it there so to make operation needed.

[Riptides]

I believe what @nylonee meant by migration tools is that the new whitelist version of the bot must be able to detect a config.json which is configured for blacklist and reconfigure itself accordingly.

FWIW I accomplish whitelisting by blacklisting everything in the global part of the config, and then for each hangout I copy/paste the list and delete the commands that are allowed in that HO.

[Deuteu]

@Riptides Ok. Thanks for the exponation.

The "all black listing" is a possibility but I don't see how to extend this easily with the project of adding roles.

[nylonee]

Yes sorry, @Riptides is right on what I meant by migration. See how @endofline did it for an earlier syncroom migration.

Without this code, it breaks all backward functionality of the bot, and will leave most botmasters confused. I'll see if I have time to get around to it later, but PR's are welcome of course!

[Riptides]

I see this change (from blacklisting to whitelisting) as being part of the change to having more roles outside of admin and normal user. In the config you define your roles, who is in them, and what commands they have access to. Blacklisting with multiple roles would just be awkward IMHO.

[Deuteu]

@nylonee About the migration tool, should/could it modify the config.json file permanantly or should it just modifiy the config dict only? I'm not used to make such tools, I'm more don't care about people, just evolve with the product. #selfish ^^ So some general todo and donot are welcomed.

[nylonee]

Take a look at what you're proposing as the new config.json

You will want to make sure that when this PR is merged, the migration script will turn any old already-configured config.json into a compatible new config.json.

Since you are actually modifying data, I suggest breaking the migration script into two parts:

• Part 1: Add the new list for "commands_user" and change "commands_admin" to "commands_admin_DEPRECATED". This migration will be pushed at the same time as the new architecture.
• Part 2 (After a few weeks or so): Remove "commands_admin_DEPRECATED"

And here's some quick pseudocode to get things rolling. This would ideally go in handlers.py.

def _migrate_blacklist_to_whitelist_part1(bot):
    if bot.config.exists(["commands_admin"]):
      add the new list "commands_user"
      populate the new list with the right plugins
      rename the old list "commands_admin" -> "commands_admin_DEPRECATED"

def _migrate_blacklist_to_whitelist_part2(bot):
    _migrate_blacklist_to_whitelist_part1(bot) # Run part1 if not already
    if bot.config.exists(["commands_admin_DEPRECATED"]):
      remove "commands_admin_DEPRECATED"

Thoughts from others are welcome too. @endofline let us know what you think of this and the PR (#182).

[endofline]

I'm iffy with the _DEPRECATED renaming convention - it would be better to simply output a reminder to log and stdout that commands_admin will be deprecated after so-and-so time - creating new temporary keys for migration is bad idea generally.

Remain neutral on the blacklist vs whitelist debate though - this requires a framework change that needs to be communicated to the plugin devs properly, otherwise it may break their own custom plugins.

[endofline]

Will attempt to adapt #182 to accommodate both usage patterns (blacklist, whitelist) - more info will be posted on the PR itself

[endofline]

White-list adapted here in 7ec1dac642cb8cf1308b99d5bafd4adbb13d7abf - setting config.json:commands_user will override the default plugin-defined behaviour and only allow commands explicitly listed in the key to be executed by users. #182 will still close on merge of branch framework/whitelist

[Deuteu]

Sorry for delay, was liltte bit busy. I like the endofline's idea to let both structure co-exist and choose one by default if both are present. Will see how to do roles with that now.

[endofline]

Closing this issue since whitelist has been adapted into framework 2.5. Further discussions on roles will take place in #121