Skip to content

Commit

Permalink
Improve permission checking (#1143)
Browse files Browse the repository at this point in the history
* Improve permission checking

Require GuildChannel and Member objects for Guild::user_permissions_in
and GuildChannel and Role objects for Guild::role_permissions_in. This
makes certain classes of errors impossible.

Add variants to ModelError to cover some new cases.

* Add methods to PartialGuild

Guild::user_permissions_in and Guild::role_permissions_in no longer
require any cached information that PartialGuild doesn't also contain,
so PartialGuild can share these methods.
  • Loading branch information
ThatsNoMoon authored Dec 25, 2020
1 parent aed3886 commit dbb3669
Show file tree
Hide file tree
Showing 7 changed files with 210 additions and 165 deletions.
22 changes: 21 additions & 1 deletion src/framework/standard/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,27 @@ pub(crate) async fn has_correct_permissions(
if options.required_permissions().is_empty() {
true
} else if let Some(guild) = message.guild(&cache).await {
let perms = guild.user_permissions_in(message.channel_id, message.author.id);
let channel = match guild.channels.get(&message.channel_id) {
Some(channel) => channel,
None => return false,
};
let member = match guild.members.get(&message.author.id) {
Some(member) => member,
None => return false,
};

let perms = match guild.user_permissions_in(channel, member) {
Ok(perms) => perms,
Err(e) => {
tracing::error!(
"(╯°□°)╯︵ ┻━┻ error getting permissions for user {} in channel {}: {}",
member.user.id,
channel.id,
e
);
return false;
}
};

perms.contains(*options.required_permissions())
} else {
Expand Down
6 changes: 4 additions & 2 deletions src/model/channel/guild_channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,8 @@ impl GuildChannel {
#[inline]
pub async fn permissions_for_user(&self, cache: impl AsRef<Cache>, user_id: impl Into<UserId>) -> Result<Permissions> {
let guild = self.guild(&cache).await.ok_or(Error::Model(ModelError::GuildNotFound))?;
Ok(guild.user_permissions_in(self.id, user_id))
let member = guild.members.get(&user_id.into()).ok_or(Error::Model(ModelError::MemberNotFound))?;
guild.user_permissions_in(self, member)
}

/// Calculates the permissions of a role.
Expand All @@ -573,7 +574,8 @@ impl GuildChannel {
#[inline]
pub async fn permissions_for_role(&self, cache: impl AsRef<Cache>, role_id: impl Into<RoleId>) -> Result<Permissions> {
let guild = self.guild(&cache).await.ok_or(Error::Model(ModelError::GuildNotFound))?;
guild.role_permissions_in(self.id, role_id).ok_or(Error::Model(ModelError::GuildNotFound))
let role = guild.roles.get(&role_id.into()).ok_or(Error::Model(ModelError::RoleNotFound))?;
guild.role_permissions_in(self, role)
}

/// Pins a [`Message`] to the channel.
Expand Down
23 changes: 23 additions & 0 deletions src/model/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,20 @@ pub enum Error {
/// [`RoleId`]: super::id::RoleId
/// [`Cache`]: crate::cache::Cache
RoleNotFound,
/// An indication that a [member][`Member`] could not be found by
/// [Id][`UserId`] in the [`Cache`].
///
/// [`Member`]: super::guild::Member
/// [`UserId`]: super::id::UserId
/// [`Cache`]: crate::cache::Cache
MemberNotFound,
/// An indication that a [channel][`Channel`] could not be found by
/// [Id][`ChannelId`] in the [`Cache`].
///
/// [`Channel`]: super::channel::Channel
/// [`ChannelId`]: super::id::ChannelId
/// [`Cache`]: crate::cache::Cache
ChannelNotFound,
/// Indicates that there are hierarchy problems restricting an action.
///
/// For example, when banning a user, if the other user has a role with an
Expand All @@ -112,6 +126,10 @@ pub enum Error {
///
/// [`Cache`]: crate::cache::Cache
ItemMissing,
/// Indicates that a member, role or channel from the wrong [`Guild`] was provided.
///
/// [`Guild`]: super::guild::Guild
WrongGuild,
/// Indicates that a [`Message`]s content was too long and will not
/// successfully send, as the length is over 2000 codepoints.
///
Expand Down Expand Up @@ -140,11 +158,16 @@ impl Display for Error {
Error::EmbedTooLarge(_) => f.write_str("Embed too large."),
Error::GuildNotFound => f.write_str("Guild not found in the cache."),
Error::RoleNotFound => f.write_str("Role not found in the cache."),
Error::MemberNotFound => f.write_str("Member not found in the cache."),
Error::ChannelNotFound => f.write_str("Channel not found in the cache."),
Error::Hierarchy => f.write_str("Role hierarchy prevents this action."),
Error::InvalidChannelType => f.write_str("The channel cannot perform the action."),
Error::InvalidPermissions(_) => f.write_str("Invalid permissions."),
Error::InvalidUser => f.write_str("The current user cannot perform the action."),
Error::ItemMissing => f.write_str("The required item is missing from the cache."),
Error::WrongGuild => {
f.write_str("Provided member or channel is from the wrong guild.")
}
Error::MessageTooLong(_) => f.write_str("Message too large."),
Error::MessagingBot => f.write_str("Attempted to message another bot user."),
Error::NameTooShort => f.write_str("Name is under the character limit."),
Expand Down
6 changes: 4 additions & 2 deletions src/model/guild/member.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,10 @@ impl Member {
pub async fn default_channel(&self, cache: impl AsRef<Cache>) -> Option<GuildChannel> {
let guild = self.guild_id.to_guild_cached(cache).await?;

for (cid, channel) in &guild.channels {
if guild.user_permissions_in(*cid, self.user.id).read_messages() {
let member = guild.members.get(&self.user.id)?;

for channel in guild.channels.values() {
if guild.user_permissions_in(channel, member).ok()?.read_messages() {
return Some(channel.clone());
}
}
Expand Down
Loading

0 comments on commit dbb3669

Please sign in to comment.