[Core, Mod] Moved core functions to owner cog (#701)

Moved blacklist / whitelist functions to owner cog
Moved [p]modset adminrole / modrole to [p]set command group
Check mod/admin users against blacklist / whitelist:
Users with (server or global) admin or mod role used to be able to skip the whitelist / blacklist checks. This doesn't really make sense from a design pov since these are global checks and the commands are owner only.
[p]set adminrole/modrole: check that the role actually exists before adding it
Added [p]blacklist/whitelist list, to show the members of each list

cogs/mod.py

@@ -94,8 +94,6 @@ class Mod:
 
     def __init__(self, bot):
         self.bot = bot
-        self.whitelist_list = dataIO.load_json("data/mod/whitelist.json")
-        self.blacklist_list = dataIO.load_json("data/mod/blacklist.json")
         self.ignore_list = dataIO.load_json("data/mod/ignorelist.json")
         self.filter = dataIO.load_json("data/mod/filter.json")
         self.past_names = dataIO.load_json("data/mod/past_names.json")
@@ -133,23 +131,17 @@ class Mod:
                    "".format(**_settings))
             await self.bot.say(box(msg))
 
-    @modset.command(name="adminrole", pass_context=True, no_pm=True)
+    @modset.command(name="adminrole", pass_context=True, no_pm=True, hidden=True)
-    async def _modset_adminrole(self, ctx, *, role_name: str):
+    async def _modset_adminrole(self, ctx):
-        """Sets the admin role for this server, case insensitive."""
+        """Use [p]set adminrole instead"""
-        server = ctx.message.server
+        await self.bot.say("This command has been renamed "
-        if server.id not in settings.servers:
+                           "`{}set adminrole`".format(ctx.prefix))
-            await self.bot.say("Remember to set modrole too.")
-        settings.set_server_admin(server, role_name)
-        await self.bot.say("Admin role set to '{}'".format(role_name))
 
-    @modset.command(name="modrole", pass_context=True, no_pm=True)
+    @modset.command(name="modrole", pass_context=True, no_pm=True, hidden=True)
-    async def _modset_modrole(self, ctx, *, role_name: str):
+    async def _modset_modrole(self, ctx):
-        """Sets the mod role for this server, case insensitive."""
+        """Use [p]set modrole instead"""
-        server = ctx.message.server
+        await self.bot.say("This command has been renamed "
-        if server.id not in settings.servers:
+                           "`{}set modrole`".format(ctx.prefix))
-            await self.bot.say("Remember to set adminrole too.")
-        settings.set_server_mod(server, role_name)
-        await self.bot.say("Mod role set to '{}'".format(role_name))
 
     @modset.command(pass_context=True, no_pm=True)
     async def modlog(self, ctx, channel : discord.Channel=None):
@@ -1050,78 +1042,6 @@ class Mod:
         else:
             await self.bot.say("Case #{} updated.".format(case))
 
-    @commands.group(pass_context=True)
-    @checks.is_owner()
-    async def blacklist(self, ctx):
-        """Bans user from using the bot"""
-        if ctx.invoked_subcommand is None:
-            await send_cmd_help(ctx)
-
-    @blacklist.command(name="add")
-    async def _blacklist_add(self, user: discord.Member):
-        """Adds user to bot's blacklist"""
-        if user.id not in self.blacklist_list:
-            self.blacklist_list.append(user.id)
-            dataIO.save_json("data/mod/blacklist.json", self.blacklist_list)
-            await self.bot.say("User has been added to blacklist.")
-        else:
-            await self.bot.say("User is already blacklisted.")
-
-    @blacklist.command(name="remove")
-    async def _blacklist_remove(self, user: discord.Member):
-        """Removes user from bot's blacklist"""
-        if user.id in self.blacklist_list:
-            self.blacklist_list.remove(user.id)
-            dataIO.save_json("data/mod/blacklist.json", self.blacklist_list)
-            await self.bot.say("User has been removed from blacklist.")
-        else:
-            await self.bot.say("User is not in blacklist.")
-
-    @blacklist.command(name="clear")
-    async def _blacklist_clear(self):
-        """Clears the blacklist"""
-        self.blacklist_list = []
-        dataIO.save_json("data/mod/blacklist.json", self.blacklist_list)
-        await self.bot.say("Blacklist is now empty.")
-
-    @commands.group(pass_context=True)
-    @checks.is_owner()
-    async def whitelist(self, ctx):
-        """Users who will be able to use the bot"""
-        if ctx.invoked_subcommand is None:
-            await send_cmd_help(ctx)
-
-    @whitelist.command(name="add")
-    async def _whitelist_add(self, user: discord.Member):
-        """Adds user to bot's whitelist"""
-        if user.id not in self.whitelist_list:
-            if not self.whitelist_list:
-                msg = "\nAll users not in whitelist will be ignored (owner, admins and mods excluded)"
-            else:
-                msg = ""
-            self.whitelist_list.append(user.id)
-            dataIO.save_json("data/mod/whitelist.json", self.whitelist_list)
-            await self.bot.say("User has been added to whitelist." + msg)
-        else:
-            await self.bot.say("User is already whitelisted.")
-
-    @whitelist.command(name="remove")
-    async def _whitelist_remove(self, user: discord.Member):
-        """Removes user from bot's whitelist"""
-        if user.id in self.whitelist_list:
-            self.whitelist_list.remove(user.id)
-            dataIO.save_json("data/mod/whitelist.json", self.whitelist_list)
-            await self.bot.say("User has been removed from whitelist.")
-        else:
-            await self.bot.say("User is not in whitelist.")
-
-    @whitelist.command(name="clear")
-    async def _whitelist_clear(self):
-        """Clears the whitelist"""
-        self.whitelist_list = []
-        dataIO.save_json("data/mod/whitelist.json", self.whitelist_list)
-        await self.bot.say("Whitelist is now empty.")
-
     @commands.group(pass_context=True, no_pm=True)
     @checks.admin_or_permissions(manage_channels=True)
     async def ignore(self, ctx):
@@ -1752,8 +1672,6 @@ def check_files():
     ignore_list = {"SERVERS": [], "CHANNELS": []}
 
     files = {
-        "blacklist.json"      : [],
-        "whitelist.json"      : [],
         "ignorelist.json"     : ignore_list,
         "filter.json"         : {},
         "past_names.json"     : {},

cogs/owner.py

@@ -39,14 +39,13 @@ class OwnerUnloadWithoutReloadError(CogUnloadError):
 
 
 class Owner:
-    """All owner-only commands that relate to debug bot operations.
+    """All owner-only commands that relate to debug bot operations."""
-    """
 
     def __init__(self, bot):
         self.bot = bot
         self.setowner_lock = False
-        self.file_path = "data/red/disabled_commands.json"
+        self.disabled_commands = dataIO.load_json("data/red/disabled_commands.json")
-        self.disabled_commands = dataIO.load_json(self.file_path)
+        self.global_ignores = dataIO.load_json("data/red/global_ignores.json")
         self.session = aiohttp.ClientSession(loop=self.bot.loop)
 
     def __unload(self):
@@ -256,7 +255,7 @@ class Owner:
 
     @commands.group(name="set", pass_context=True)
     async def _set(self, ctx):
-        """Changes Red's global settings."""
+        """Changes Red's core settings"""
         if ctx.invoked_subcommand is None:
             await self.bot.send_cmd_help(ctx)
             return
@@ -506,6 +505,125 @@ class Owner:
             await self.bot.say("Token set. Restart me.")
             log.debug("Token changed.")
 
+    @_set.command(name="adminrole", pass_context=True, no_pm=True)
+    @checks.serverowner()
+    async def _server_adminrole(self, ctx, *, role: discord.Role):
+        """Sets the admin role for this server"""
+        server = ctx.message.server
+        if server.id not in self.bot.settings.servers:
+            await self.bot.say("Remember to set modrole too.")
+        self.bot.settings.set_server_admin(server, role.name)
+        await self.bot.say("Admin role set to '{}'".format(role.name))
+
+    @_set.command(name="modrole", pass_context=True, no_pm=True)
+    @checks.serverowner()
+    async def _server_modrole(self, ctx, *, role: discord.Role):
+        """Sets the mod role for this server"""
+        server = ctx.message.server
+        if server.id not in self.bot.settings.servers:
+            await self.bot.say("Remember to set adminrole too.")
+        self.bot.settings.set_server_mod(server, role.name)
+        await self.bot.say("Mod role set to '{}'".format(role.name))
+
+    @commands.group(pass_context=True)
+    @checks.is_owner()
+    async def blacklist(self, ctx):
+        """Blacklist management commands
+
+        Blacklisted users will be unable to issue commands"""
+        if ctx.invoked_subcommand is None:
+            await self.bot.send_cmd_help(ctx)
+
+    @blacklist.command(name="add")
+    async def _blacklist_add(self, user: discord.Member):
+        """Adds user to Red's global blacklist"""
+        if user.id not in self.global_ignores["blacklist"]:
+            self.global_ignores["blacklist"].append(user.id)
+            self.save_global_ignores()
+            await self.bot.say("User has been blacklisted.")
+        else:
+            await self.bot.say("User is already blacklisted.")
+
+    @blacklist.command(name="remove")
+    async def _blacklist_remove(self, user: discord.Member):
+        """Removes user from Red's global blacklist"""
+        if user.id in self.global_ignores["blacklist"]:
+            self.global_ignores["blacklist"].remove(user.id)
+            self.save_global_ignores()
+            await self.bot.say("User has been removed from the blacklist.")
+        else:
+            await self.bot.say("User is not blacklisted.")
+
+    @blacklist.command(name="list")
+    async def _blacklist_list(self):
+        """Lists users on the blacklist"""
+        blacklist = self._populate_list(self.global_ignores["blacklist"])
+
+        if blacklist:
+            for page in blacklist:
+                await self.bot.say(box(page))
+        else:
+            await self.bot.say("The blacklist is empty.")
+
+    @blacklist.command(name="clear")
+    async def _blacklist_clear(self):
+        """Clears the global blacklist"""
+        self.global_ignores["blacklist"] = []
+        self.save_global_ignores()
+        await self.bot.say("Blacklist is now empty.")
+
+    @commands.group(pass_context=True)
+    @checks.is_owner()
+    async def whitelist(self, ctx):
+        """Whitelist management commands
+
+        If the whitelist is not empty, only whitelisted users will
+        be able to use Red"""
+        if ctx.invoked_subcommand is None:
+            await self.bot.send_cmd_help(ctx)
+
+    @whitelist.command(name="add")
+    async def _whitelist_add(self, user: discord.Member):
+        """Adds user to Red's global whitelist"""
+        if user.id not in self.global_ignores["whitelist"]:
+            if not self.global_ignores["whitelist"]:
+                msg = "\nNon-whitelisted users will be ignored."
+            else:
+                msg = ""
+            self.global_ignores["whitelist"].append(user.id)
+            self.save_global_ignores()
+            await self.bot.say("User has been whitelisted." + msg)
+        else:
+            await self.bot.say("User is already whitelisted.")
+
+    @whitelist.command(name="remove")
+    async def _whitelist_remove(self, user: discord.Member):
+        """Removes user from Red's global whitelist"""
+        if user.id in self.global_ignores["whitelist"]:
+            self.global_ignores["whitelist"].remove(user.id)
+            self.save_global_ignores()
+            await self.bot.say("User has been removed from the whitelist.")
+        else:
+            await self.bot.say("User is not whitelisted.")
+
+    @whitelist.command(name="list")
+    async def _whitelist_list(self):
+        """Lists users on the whitelist"""
+        whitelist = self._populate_list(self.global_ignores["whitelist"])
+
+        if whitelist:
+            for page in whitelist:
+                await self.bot.say(box(page))
+        else:
+            await self.bot.say("The whitelist is empty.")
+
+    @whitelist.command(name="clear")
+    async def _whitelist_clear(self):
+        """Clears the global whitelist"""
+        self.global_ignores["whitelist"] = []
+        self.save_global_ignores()
+        await self.bot.say("Whitelist is now empty.")
+
     @commands.command()
     @checks.is_owner()
     async def shutdown(self, silently : bool=False):
@@ -561,7 +679,7 @@ class Owner:
             comm_obj.enabled = False
             comm_obj.hidden = True
             self.disabled_commands.append(command)
-            dataIO.save_json(self.file_path, self.disabled_commands)
+            self.save_disabled_commands()
             await self.bot.say("Command has been disabled.")
 
     @command_disabler.command()
@@ -569,7 +687,7 @@ class Owner:
         """Enables commands/subcommands"""
         if command in self.disabled_commands:
             self.disabled_commands.remove(command)
-            dataIO.save_json(self.file_path, self.disabled_commands)
+            self.save_disabled_commands()
             await self.bot.say("Command enabled.")
         else:
             await self.bot.say("That command is not disabled.")
@@ -799,6 +917,27 @@ class Owner:
         else:
             await self.bot.say("No exception has occurred yet.")
 
+    def _populate_list(self, _list):
+        """Used for both whitelist / blacklist
+
+        Returns a paginated list"""
+        users = []
+        total = len(_list)
+
+        for user_id in _list:
+            user = discord.utils.get(self.bot.get_all_members(), id=user_id)
+            if user:
+                users.append(str(user))
+
+        if users:
+            not_found = total - len(users)
+            users = ", ".join(users)
+            if not_found:
+                users += "\n\n ... and {} users I could not find".format(not_found)
+            return list(pagify(users, delims=[" ", "\n"]))
+
+        return []
+
     def _load_cog(self, cogname):
         if not self._does_cogfile_exist(cogname):
             raise CogNotFoundError(cogname)
@@ -911,12 +1050,44 @@ class Owner:
 
         return fmt.format(d=days, h=hours, m=minutes, s=seconds)
 
+    def save_global_ignores(self):
+        dataIO.save_json("data/red/global_ignores.json", self.global_ignores)
+
+    def save_disabled_commands(self):
+        dataIO.save_json("data/red/disabled_commands.json", self.disabled_commands)
+
+
+def _import_old_data(data):
+    """Migration from mod.py"""
+    try:
+        data["blacklist"] = dataIO.load_json("data/mod/blacklist.json")
+    except FileNotFoundError:
+        pass
+
+    try:
+        data["whitelist"] = dataIO.load_json("data/mod/whitelist.json")
+    except FileNotFoundError:
+        pass
+
+    return data
+
 
 def check_files():
     if not os.path.isfile("data/red/disabled_commands.json"):
         print("Creating empty disabled_commands.json...")
         dataIO.save_json("data/red/disabled_commands.json", [])
 
+    if not os.path.isfile("data/red/global_ignores.json"):
+        print("Creating empty global_ignores.json...")
+        data = {"blacklist": [], "whitelist": []}
+        try:
+            data = _import_old_data(data)
+        except Exception as e:
+            log.error("Failed to migrate blacklist / whitelist data from "
+                      "mod.py: {}".format(e))
+
+        dataIO.save_json("data/red/global_ignores.json", data)
+
 
 def setup(bot):
     check_files()

red.py

@@ -156,38 +156,39 @@ class Bot(commands.Bot):
         if author == self.user:
             return self.settings.self_bot
 
-        mod = self.get_cog('Mod')
+        mod_cog = self.get_cog('Mod')
+        global_ignores = self.get_cog('Owner').global_ignores
 
-        if mod is not None:
+        if self.settings.owner == author.id:
-            if self.settings.owner == author.id:
+            return True
-                return True
-            if not message.channel.is_private:
-                server = message.server
-                names = (self.settings.get_server_admin(
-                    server), self.settings.get_server_mod(server))
-                results = map(
-                    lambda name: discord.utils.get(author.roles, name=name),
-                    names)
-                for r in results:
-                    if r is not None:
-                        return True
 
-            if author.id in mod.blacklist_list:
+        if author.id in global_ignores["blacklist"]:
+            return False
+
+        if global_ignores["whitelist"]:
+            if author.id not in global_ignores["whitelist"]:
                 return False
 
-            if mod.whitelist_list:
+        if not message.channel.is_private:
-                if author.id not in mod.whitelist_list:
+            server = message.server
-                    return False
+            names = (self.settings.get_server_admin(
+                server), self.settings.get_server_mod(server))
+            results = map(
+                lambda name: discord.utils.get(author.roles, name=name),
+                names)
+            for r in results:
+                if r is not None:
+                    return True
 
+        if mod_cog is not None:
             if not message.channel.is_private:
-                if message.server.id in mod.ignore_list["SERVERS"]:
+                if message.server.id in mod_cog.ignore_list["SERVERS"]:
                     return False
 
-                if message.channel.id in mod.ignore_list["CHANNELS"]:
+                if message.channel.id in mod_cog.ignore_list["CHANNELS"]:
                     return False
-            return True
+
-        else:
+        return True
-            return True
 
     async def pip_install(self, name, *, timeout=None):
         """