Pre-fetch app owners and fail early on no owners (#4926)

* Pre-fetch app owners and fail early on no owners

* Improve command mention in error message

* Further change the order of startup actions

redbot/__main__.py

@@ -28,7 +28,7 @@ from redbot import _early_init, __version__
 _early_init()
 
 import redbot.logging
-from redbot.core.bot import Red, ExitCodes
+from redbot.core.bot import Red, ExitCodes, _NoOwnerSet
 from redbot.core.cli import interactive_config, confirm, parse_cli_flags
 from redbot.setup import get_data_dir, get_name, save_config
 from redbot.core import data_manager, drivers
@@ -384,7 +384,7 @@ async def run_bot(red: Red, cli_flags: Namespace) -> None:
         await red.http.close()
         sys.exit(0)
     try:
-        await red.start(token, bot=True, cli_flags=cli_flags)
+        await red.start(token, bot=True)
     except discord.LoginFailure:
         log.critical("This token doesn't seem to be valid.")
         db_token = await red._config.token()
@@ -403,6 +403,24 @@ async def run_bot(red: Red, cli_flags: Namespace) -> None:
             style="red",
         )
         sys.exit(1)
+    except _NoOwnerSet:
+        print(
+            "Bot doesn't have any owner set!\n"
+            "This can happen when your bot's application is owned by team"
+            " as team members are NOT owners by default.\n\n"
+            "Remember:\n"
+            "ONLY the person who is hosting Red should be owner."
+            " This has SERIOUS security implications."
+            " The owner can access any data that is present on the host system.\n"
+            "With that out of the way, depending on who you want to be considered as owner,"
+            " you can:\n"
+            "a) pass --team-members-are-owners when launching Red"
+            " - in this case Red will treat all members of the bot application's team as owners\n"
+            f"b) set owner manually with `redbot --edit {cli_flags.instance_name}`\n"
+            "c) pass owner ID(s) when launching Red with --owner"
+            " (and --co-owner if you need more than one) flag\n"
+        )
+        sys.exit(1)
 
     return None
 

redbot/core/bot.py

@@ -75,6 +75,10 @@ def _is_submodule(parent, child):
     return parent == child or child.startswith(parent + ".")
 
 
+class _NoOwnerSet(RuntimeError):
+    """Raised when there is no owner set for the instance that is trying to start."""
+
+
 # Order of inheritance here matters.
 # d.py autoshardedbot should be at the end
 # all of our mixins should happen before,
@@ -217,8 +221,8 @@ class Red(
         self._main_dir = bot_dir
         self._cog_mgr = CogManager()
         self._use_team_features = cli_flags.use_team_features
-        # to prevent multiple calls to app info in `is_owner()`
+        # to prevent multiple calls to app info during startup
-        self._app_owners_fetched = False
+        self._app_info = None
         super().__init__(*args, help_command=None, **kwargs)
         # Do not manually use the help formatter attribute here, see `send_help_for`,
         # for a documented API. The internals of this object are still subject to change.
@@ -1066,15 +1070,15 @@ class Red(
 
     # end Config migrations
 
-    async def pre_flight(self, cli_flags):
+    async def _pre_login(self) -> None:
         """
-        This should only be run once, prior to connecting to discord.
+        This should only be run once, prior to logging in to Discord REST API.
         """
         await self._maybe_update_config()
         self.description = await self._config.description()
 
         init_global_checks(self)
-        init_events(self, cli_flags)
+        init_events(self, self._cli_flags)
 
         if self._owner_id_overwrite is None:
             self._owner_id_overwrite = await self._config.owner()
@@ -1086,9 +1090,13 @@ class Red(
         i18n_regional_format = await self._config.regional_format()
         i18n.set_regional_format(i18n_regional_format)
 
+    async def _pre_connect(self) -> None:
+        """
+        This should only be run once, prior to connecting to Discord gateway.
+        """
         self.add_cog(Core(self))
         self.add_cog(CogManagerUI())
-        if cli_flags.dev:
+        if self._cli_flags.dev:
             self.add_cog(Dev())
 
         await modlog._init(self)
@@ -1119,11 +1127,11 @@ class Red(
                 )
                 python_version_changed = True
         else:
-            if cli_flags.no_cogs is False:
+            if self._cli_flags.no_cogs is False:
                 packages.extend(await self._config.packages())
 
-            if cli_flags.load_cogs:
+            if self._cli_flags.load_cogs:
-                packages.extend(cli_flags.load_cogs)
+                packages.extend(self._cli_flags.load_cogs)
 
         system_changed = False
         machine = platform.machine()
@@ -1189,13 +1197,29 @@ class Red(
         if self.rpc_enabled:
             await self.rpc.initialize(self.rpc_port)
 
+    async def _pre_fetch_owners(self) -> None:
+        app_info = await self.application_info()
+
+        if app_info.team:
+            if self._use_team_features:
+                self.owner_ids.update(m.id for m in app_info.team.members)
+        elif self._owner_id_overwrite is None:
+            self.owner_ids.add(app_info.owner.id)
+
+        self._app_info = app_info
+
+        if not self.owner_ids:
+            raise _NoOwnerSet("Bot doesn't have any owner set!")
+
     async def start(self, *args, **kwargs):
         """
-        Overridden start which ensures cog load and other pre-connection tasks are handled
+        Overridden start which ensures that cog load and other pre-connection tasks are handled.
         """
-        cli_flags = kwargs.pop("cli_flags")
+        await self._pre_login()
-        await self.pre_flight(cli_flags=cli_flags)
+        await self.login(*args)
-        return await super().start(*args, **kwargs)
+        await self._pre_fetch_owners()
+        await self._pre_connect()
+        await self.connect()
 
     async def send_help_for(
         self,
@@ -1279,24 +1303,7 @@ class Red(
         -------
         bool
         """
-        if user.id in self.owner_ids:
+        return user.id in self.owner_ids
-            return True
-
-        ret = False
-        if not self._app_owners_fetched:
-            app = await self.application_info()
-            if app.team:
-                if self._use_team_features:
-                    ids = {m.id for m in app.team.members}
-                    self.owner_ids.update(ids)
-                    ret = user.id in ids
-            elif self._owner_id_overwrite is None:
-                owner_id = app.owner.id
-                self.owner_ids.add(owner_id)
-                ret = user.id == owner_id
-            self._app_owners_fetched = True
-
-        return ret
 
     async def is_admin(self, member: discord.Member) -> bool:
         """Checks if a member is an admin of their guild."""

redbot/core/events.py

@@ -70,19 +70,7 @@ def init_events(bot, cli_flags):
         guilds = len(bot.guilds)
         users = len(set([m for m in bot.get_all_members()]))
 
-        app_info = await bot.application_info()
+        invite_url = discord.utils.oauth_url(bot._app_info.id)
-
-        if app_info.team:
-            if bot._use_team_features:
-                bot.owner_ids.update(m.id for m in app_info.team.members)
-        elif bot._owner_id_overwrite is None:
-            bot.owner_ids.add(app_info.owner.id)
-        bot._app_owners_fetched = True
-
-        try:
-            invite_url = discord.utils.oauth_url(app_info.id)
-        except:
-            invite_url = "Could not fetch invite url"
 
         prefixes = cli_flags.prefix or (await bot._config.prefix())
         lang = await bot._config.locale()
@@ -200,10 +188,6 @@ def init_events(bot, cli_flags):
         if rich_outdated_message:
             rich_console.print(rich_outdated_message)
 
-        if not bot.owner_ids:
-            # we could possibly exit here in future
-            log.warning("Bot doesn't have any owner set!")
-
         bot._color = discord.Colour(await bot._config.color())
         bot._red_ready.set()
         if outdated_red_message: