BraveNetwork / LinBot

Open source, general-purpose Discord chat bot written in C#
https://bravenetwork.github.io/
MIT License
2 stars 0 forks source link

Global Whitelisted commands for specific servers and users #4

Open 23nd opened 6 years ago

23nd commented 6 years ago

(Navi) another thing that we would want you to work on is adding a whitelist for server modules/commands. The code already exists to blacklist commands/modules globally but we want to be able to whitelist commands for certain servers/users as well.

.globalcommand / .gcmd
Toggles whether a command can be used on any server. Navi/fAEth only
.globalmodule / .gmod
Toggles whether a module can be used on any server. Navi/fAEth only

so basically we have that but we want to be able to say "the default state for this command is off BUT these certain users/servers can use it normally"

Basically we want to be able to add commands to the global blacklist, but make exceptions on a per command basis for certain users (in this case, patreon donators)

23nd commented 6 years ago

@Navi-King For clarification, you want me to do the following?

23nd commented 6 years ago

(Navi) I think it makes sense. Basically you’re mimicking the blacklist structure but using it to whitelist. You’re verifying that blacklisted users can’t use commands in whitelisted servers/channels, but making sure that whitelisted users can still use blacklisted commands?

(Katsu) Well commands aren’t blacklisted. Servers/Channels/Users are blacklisted. Whitelisted servers/channels/users can use globally disabled commands/modules, but whitelisted servers/channels/users are still disabled from using commands when blacklisted blacklist is all-or-nothing (all commands disabled) for specific users/channels/servers(edited) and yes, blacklisted users cannot use commands in whitelisted servers/channels So i won’t be modifying any blacklist stuff, just using it as a structure for whitelist and modifying the global command/module stuff perhaps whitelist is a misnomer

(Navi) Ok so basically the whitelist I want should override global permission but not blacklist

(Katsu) yeah, or actually, it’s also called “blocking”. so unblocking the blocking

(Navi) But yeah basically I want “blocked” commands able to be used by “unblocked” users Obviously the command to modify that should be bot owners only and bot owners should automatically be able to use any blacklisted command anyway. Or at least added to the unblocked list by default

Terminology Notes

23nd commented 6 years ago

TODO LIST

When all items have been checked off, this issue may be closed.

Database, DBModel, and Migration Additions

Note that this probably follows a MVC paradigm, and that the models should be written in code, migrations generated by some sort of dotnet command, and database tables generated at runtime.

Command Additions

Each of these should have [NadekoCommand, Usage, Description, Aliases] attributes, and be added to either GlobalPermissionsCommands.cs or a new WhitelistCommands.cs in the Permissions module.

Service Changes

23nd commented 6 years ago

Status update: pretty much all the commands for adding/removing and creating/deleting have been implemented, along with some info commands to list members and unblocked cmds/mdls for each list.

What's left:

23nd commented 6 years ago

For ResetGlobalPerms, the existing implementation leaves unwanted data in the database. To avoid messing with it, I'll implement a separate ResetGlobalWhitelist and ResetGlobalUnblocked for totally clearing all GlobalWhitelist records and UnblockedCmdOrMdl records (and their relation table records).

23nd commented 6 years ago

Also note that I cannot unblock Custom Reactions (even though they can be added to a whitelist), because NadekoBot does not block them in the CustomReactionsService.TryExecuteEarly behavior (even though they can be added to global permissions via gcmd).

23nd commented 6 years ago

Note 2: Any changes made in the database to either BlockedCmdOrMdl or UnblockedCmdOrMdl may cause the global permissions to be out of sync with the database. A restart will be required to re-sync global permissions, should you find yourself modifying the database directly.

I could reload the GlobalPermissionService via bot command, but that may introduce a moment of vulnerability without any working permission checks.

I also tried to modify the internal hashsets of (Un)BlockedCommands/Modules to sync them with the database, but unfortunately those hashsets are readonly and I don't want to overrule that decision at this time.

Navi-King commented 6 years ago

This is awesome. We do still need to move it to its own module probably though since keeping it in permissions messes with the commands that general users should be able to use. Also it will help future proof it against upstream code changes.