Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement admin only commands and make some commands admin-able #18

Merged
merged 9 commits into from
Jun 5, 2024
+96 −23
Merged

Implement admin only commands and make some commands admin-able #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7f8f417
Read bot admin IDs set in .env file
someyuck May 27, 2024
76d1f99
Add function to check if user is bot admin
someyuck May 27, 2024
41e409a
Add function to check if command author is bot admin
someyuck May 27, 2024
3f8fbb6
Make backend_info admin only
someyuck May 27, 2024
39fcfb5
Add CommandError derivative to ID which check failed
someyuck May 28, 2024
380e75f
Make query and roll commands admin-able
someyuck Jun 4, 2024
82c4599
Update module docstring
someyuck Jun 4, 2024
dfad23a
Apply Linting & Formatting Fixes
someyuck Jun 4, 2024
06e8b2a
Update module docstring
someyuck Jun 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .env.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ MONGO_URI="mongodb://127.0.0.1:100"

# needed by ./bot
DISCORD_TOKEN=""
BOT_ADMINS=""

# needed by ./portal
CAS_LINK="https://cas.my-org.com/login"
Expand Down
117 changes: 94 additions & 23 deletions bot/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

- `get_users_from_discordid()`: Find users from DB given user ID
- `is_verified()`: If a user is present in DB or not
- `is_bot_admin()`: If a user is a bot admin or not
- `is_author_bot_admin()`: If the author of the current message is a bot admin or not
- `get_realname_from_discordid()`: Get a user's real name from their Discord ID.
- `send_link()`: Send link for reattempting authentication.
- `create_roles_if_missing()`: Adds missing roles to a server.
Expand All @@ -15,11 +17,11 @@
- `post_verification()`: Handle role add/delete and nickname set post-verification of given user.
- `verify_user()`: Implements `.verify`.
- `backend_info()`: Logs server details for debug purposes
- `is_academic()`: Checks if server is for academic use.
- `is_academic_or_bot_admin()`: Checks if server is for academic use or if the author is a bot admin.
- `query()`: Returns user details, uses Discord ID to find in DB.
- `query_error()`: Replies eror message if server is not academic.
- `query_error()`: Replies eror message if server is not academic or author is not a bot admin.
- `roll()`: Returns user details, uses roll number to find in DB.
- `roll_error()`: Replies eror message if server is not academic.
- `roll_error()`: Replies eror message if server is not academic or author is not a bot admin.
- `on_ready()`: Logs a message when the bot joins a server.
- `main()`: Reads server config, loads DB and starts bot.

Expand Down Expand Up @@ -57,6 +59,8 @@
SUBPATH = os.getenv("SUBPATH")
BASE_URL = f"{PROTOCOL}://{HOST}{_PORT_AS_SUFFIX}{SUBPATH}"

BOT_ADMINS = [int(id) for id in os.getenv("BOT_ADMINS").split(",") if id != ""]

intent = discord.Intents.default()
intent.message_content = True
bot = commands.Bot(command_prefix=".", intents=intent)
Expand All @@ -73,6 +77,15 @@ class DBEntry(TypedDict):
view: bool


class CheckFailedException(commands.CommandError):
"""Custom exception to help identify which check function failed."""

def __init__(self, check_name: str):
"""Initialise with the name of the check."""
super().__init__()
self.check_name = check_name


Comment on lines +80 to +88
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of storing the "exception type as a string" I would suggest a more OOP-like approach

Suggested change
class CheckFailedException(commands.CommandError):
"""Custom exception to help identify which check function failed."""
def __init__(self, check_name: str):
"""Initialise with the name of the check."""
super().__init__()
self.check_name = check_name
class CheckAdminOrAcademic(commands.CheckFailure):
pass
class CheckAdmin(CheckAdminOrAcademic):
pass
class CheckAcademic(CheckAdminOrAcademic):
pass

This way isinstance(error, CheckAdmin) can detect admin command failure, isinstance(error, CheckAcademic) can detect academic server error, and isinstance(error, CheckAdminOrAcademic) can be used to detect either of above

def get_users_from_discordid(user_id: int):
"""
Finds users from the database, given their ID and returns
Expand All @@ -89,6 +102,20 @@ def is_verified(user_id: int):
return True if get_users_from_discordid(user_id) else False


def is_bot_admin(user_id: int):
"""Checks if the user with the given discord ID is a bot admin or not."""
return user_id in BOT_ADMINS


def is_author_bot_admin(ctx: commands.Context):
"""Wrapper check function for `is_bot_admin`; checks if the user who invoked the command
is a bot admin or not."""
author = ctx.message.author
if not is_bot_admin(author.id):
raise CheckFailedException("is_author_bot_admin")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise CheckFailedException("is_author_bot_admin")
raise CheckAdmin()

return True


def get_realname_from_discordid(user_id: int):
"""Returns the real name of the first user who matches the given ID."""
users = get_users_from_discordid(user_id)
Expand Down Expand Up @@ -221,8 +248,10 @@ async def verify_user(ctx: commands.Context):


@bot.hybrid_command(name="backend_info")
@commands.check(is_author_bot_admin)
async def backend_info(ctx: commands.Context):
"""For debugging server info; sends details of the server."""

uname = platform.uname()
await ctx.reply(
f"Here are the server details:\n"
Expand All @@ -235,27 +264,45 @@ async def backend_info(ctx: commands.Context):
)


def is_academic(ctx: commands.Context):
"""Checks if the server is an academic server."""
@backend_info.error
async def backend_info_error(ctx: commands.Context, error: Exception):
"""If the author of the message is not a bot admin then reply accordingly."""
if (
isinstance(error, CheckFailedException)
and error.check_name == "is_author_bot_admin"
):
Comment on lines +270 to +273
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (
isinstance(error, CheckFailedException)
and error.check_name == "is_author_bot_admin"
):
if isinstance(error, CheckAdmin):

author = ctx.message.author
await ctx.reply(f"{author.mention} is not a bot admin.", ephemeral=True)
else:
await ctx.reply("Some checks failed.", ephemeral=True)


def is_academic_or_bot_admin(ctx: commands.Context):
"""Checks if the server is an academic server or if the author is a bot admin."""
if ctx.guild is None:
return False

try:
return server_configs[ctx.guild.id]["is_academic"]
except KeyError:
return False
author = ctx.message.author
is_admin = is_bot_admin(author.id)
if not server_configs[ctx.guild.id]["is_academic"] and not is_admin:
raise CheckFailedException("is_academic_or_bot_admin")
return True
except KeyError as err:
raise CheckFailedException("server_config") from err
Comment on lines +286 to +292
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to leave this function as it is, as is_academic, and instead define a new function

def is_academic_or_bot_admin(ctx: commands.Context):
    """Checks if the server is an academic server or if the author is a bot admin."""
    return is_author_bot_admin(ctx) or is_academic(ctx)

Of course, you would need to modify is_academic to raise CheckAcademic errors instead of returning False

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realised that @commands.check_any exists. We can make the implementation much simpler with this!



@bot.hybrid_command(name="query")
@commands.check(is_academic)
@commands.check(is_academic_or_bot_admin)
async def query(
ctx: commands.Context,
identifier: discord.User,
):
"""
First checks if the server is an academic one. If so, finds the user who invoked the
command (by Discord ID) in the DB. If present, replies with their name, email and
roll number. Otherwise replies telling the user they are not registed with CAS.
First checks if the server is an academic one or if the author is a bot admin.
If so, finds the user mentioned (by Discord ID) in the command in the DB.
If present, replies with their name, email and roll number. Otherwise
replies telling the author that the mentioned user is not registed with CAS.
"""
if db is None:
await ctx.reply(
Expand All @@ -280,24 +327,37 @@ async def query(
@query.error
async def query_error(ctx: commands.Context, error: Exception):
"""
For the `query` command, if the server is not academic, replies with error message.
For the `query` command, if the server is not academic and if the author is
not a bot admin, replies with an error message.
"""
if isinstance(error, commands.CheckFailure):
await ctx.reply("This server is not for academic purposes.", ephemeral=True)
if (
isinstance(error, CheckFailedException)
and error.check_name == "is_academic_or_bot_admin"
):
Comment on lines +333 to +336
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (
isinstance(error, CheckFailedException)
and error.check_name == "is_academic_or_bot_admin"
):
if isinstance(error, CheckAdminOrAcademic):

author = ctx.message.author
await ctx.reply(
"This server is not for academic purposes "
f"and {author.mention} is not a bot admin.",
ephemeral=True,
)
else:
await ctx.reply("Some checks failed.", ephemeral=True)


@bot.hybrid_command(name="roll")
@commands.check(is_academic)
@commands.check(is_academic_or_bot_admin)
async def roll(
ctx: commands.Context,
identifier: int,
):
"""
First checks if the server is an academic one. If so, finds the user who invoked the
command in the DB. If present, replies with their name, email and
roll number. Otherwise replies telling the user they are not registed with CAS.
First checks if the server is an academic one or if the author is a bot admin.
If so, finds the user mentioned in the command in the DB. If present, replies
with their name, email and roll number. Otherwise replies telling the author
that the mentioned user is not registed with CAS.

Same as the `query` command, except this searches by roll number instead of Discord ID.
Same as the `query` command, except the user is mentioned by roll number
instead of Discord ID.
"""
if db is None:
await ctx.reply(
Expand All @@ -322,10 +382,21 @@ async def roll(
@roll.error
async def roll_error(ctx: commands.Context, error: Exception):
"""
For the `roll` command, if the server is not academic, replies with error message.
For the `roll` command, if the server is not academic and if the author is
not a bot admin, replies with an error message.
"""
if isinstance(error, commands.CheckFailure):
await ctx.reply("This server is not for academic purposes.", ephemeral=True)
author = ctx.message.author
if (
isinstance(error, CheckFailedException)
and error.check_name == "is_academic_or_bot_admin"
):
Comment on lines +389 to +392
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (
isinstance(error, CheckFailedException)
and error.check_name == "is_academic_or_bot_admin"
):
if isinstance(error, CheckAdminOrAcademic):

await ctx.reply(
"This server is not for academic purposes "
f"and {author.mention} is not a bot admin.",
ephemeral=True,
)
else:
await ctx.reply("Some checks failed.", ephemeral=True)


@bot.event
Expand Down
1 change: 1 addition & 0 deletions server_config.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,4 @@ grantroles=Verified
[Discord-CAS Test Server 2024]
serverid=1239556898592788520
grantroles=Verified
is_academic=yes
Loading