Deprecate permission methods which do not handle overwrites (serenity…

…-rs#3037)

These methods are footguns and shouldn't be trusted, so let's deprecate
and remove them... this also fixes the `author_permissions` method which
got it's foot blown off.

src/cache/mod.rs

@@ -46,7 +46,7 @@ use crate::model::prelude::*;
 mod cache_update;
 mod event;
 mod settings;
-mod wrappers;
+pub(crate) mod wrappers;
 
 #[cfg(feature = "temp_cache")]
 pub(crate) use wrappers::MaybeOwnedArc;

src/cache/wrappers.rs

@@ -12,7 +12,7 @@ use typesize::TypeSize;
 
 #[derive(Debug)]
 /// A wrapper around Option<DashMap<K, V>> to ease disabling specific cache fields.
-pub(crate) struct MaybeMap<K: Eq + Hash, V>(pub(super) Option<DashMap<K, V, BuildHasher>>);
+pub(crate) struct MaybeMap<K: Eq + Hash, V>(pub(crate) Option<DashMap<K, V, BuildHasher>>);
 impl<K: Eq + Hash, V> MaybeMap<K, V> {
     pub fn iter(&self) -> impl Iterator<Item = RefMulti<'_, K, V, BuildHasher>> {
         Option::iter(&self.0).flat_map(DashMap::iter)

src/model/channel/message.rs

@@ -220,6 +220,7 @@ impl Message {
     ///
     /// This may return `None` if:
     /// - The [`Cache`] does not have the current [`Guild`]
+    /// - The [`Guild`] does not have the current channel cached (should never happen).
     /// - This message is not from [`MessageCreateEvent`] and the author's [`Member`] cannot be
     ///   found in [`Guild#structfield.members`].
     #[cfg(feature = "cache")]
@@ -229,10 +230,18 @@ impl Message {
         };
 
         let guild = cache.as_ref().guild(guild_id)?;
+        let channel = if let Some(channel) = guild.channels.get(&self.channel_id) {
+            channel
+        } else if let Some(thread) = guild.threads.iter().find(|th| th.id == self.channel_id) {
+            thread
+        } else {
+            return None;
+        };
+
         if let Some(member) = &self.member {
-            Some(guild.partial_member_permissions(self.author.id, member))
+            Some(guild.partial_member_permissions_in(channel, self.author.id, member))
         } else {
-            Some(guild.member_permissions(guild.members.get(&self.author.id)?))
+            Some(guild.user_permissions_in(channel, guild.members.get(&self.author.id)?))
         }
     }
 
@@ -1443,3 +1452,75 @@ pub struct PollAnswerCount {
     pub count: u64,
     pub me_voted: bool,
 }
+
+// all tests here require cache, move if non-cache test is added
+#[cfg(all(test, feature = "cache"))]
+mod tests {
+    use std::collections::HashMap;
+
+    use dashmap::DashMap;
+
+    use super::{
+        Guild,
+        GuildChannel,
+        Member,
+        Message,
+        PermissionOverwrite,
+        PermissionOverwriteType,
+        Permissions,
+        User,
+        UserId,
+    };
+    use crate::cache::wrappers::MaybeMap;
+    use crate::cache::Cache;
+
+    /// Test that author_permissions checks the permissions in a channel, not just the guild.
+    #[test]
+    fn author_permissions_respects_overwrites() {
+        // Author of the message, with a random ID that won't collide with defaults.
+        let author = User {
+            id: UserId::new(50778944701071),
+            ..Default::default()
+        };
+
+        // Channel with the message, with SEND_MESSAGES on.
+        let channel = GuildChannel {
+            permission_overwrites: vec![PermissionOverwrite {
+                allow: Permissions::SEND_MESSAGES,
+                deny: Permissions::default(),
+                kind: PermissionOverwriteType::Member(author.id),
+            }],
+            ..Default::default()
+        };
+        let channel_id = channel.id;
+
+        // Guild with the author and channel cached, default (empty) permissions.
+        let guild = Guild {
+            channels: HashMap::from([(channel.id, channel)]),
+            members: HashMap::from([(author.id, Member {
+                user: author.clone(),
+                ..Default::default()
+            })]),
+            ..Default::default()
+        };
+
+        // Message, tied to the guild and the channel.
+        let message = Message {
+            author,
+            channel_id,
+            guild_id: Some(guild.id),
+            ..Default::default()
+        };
+
+        // Cache, with the guild setup.
+        let mut cache = Cache::new();
+        cache.guilds = MaybeMap(Some({
+            let guilds = DashMap::default();
+            guilds.insert(guild.id, guild);
+            guilds
+        }));
+
+        // The author should only have the one permission, SEND_MESSAGES.
+        assert_eq!(message.author_permissions(&cache), Some(Permissions::SEND_MESSAGES));
+    }
+}

src/model/guild/member.rs

@@ -435,8 +435,11 @@ impl Member {
     /// And/or returns [`ModelError::ItemMissing`] if the "default channel" of the guild is not
     /// found.
     #[cfg(feature = "cache")]
+    #[deprecated = "Use Guild::member_permissions_in, as this doesn't consider permission overwrites"]
     pub fn permissions(&self, cache: impl AsRef<Cache>) -> Result<Permissions> {
         let guild = cache.as_ref().guild(self.guild_id).ok_or(ModelError::GuildNotFound)?;
+
+        #[allow(deprecated)]
         Ok(guild.member_permissions(self))
     }
 

src/model/guild/mod.rs

@@ -428,6 +428,8 @@ impl Guild {
         required_permissions: Permissions,
     ) -> Result<(), Error> {
         if let Some(member) = self.members.get(&cache.current_user().id) {
+            // This isn't used for any channel-specific permissions, but sucks still.
+            #[allow(deprecated)]
             let bot_permissions = self.member_permissions(member);
             if !bot_permissions.contains(required_permissions) {
                 return Err(Error::Model(ModelError::InvalidPermissions {
@@ -1908,6 +1910,7 @@ impl Guild {
     /// Calculate a [`Member`]'s permissions in the guild.
     #[inline]
     #[must_use]
+    #[deprecated = "Use Guild::member_permissions_in, as this doesn't consider permission overwrites"]
     pub fn member_permissions(&self, member: &Member) -> Permissions {
         Self::user_permissions_in_(
             None,
@@ -1926,6 +1929,7 @@ impl Guild {
     /// Panics if the passed [`UserId`] does not match the [`PartialMember`] id, if user is Some.
     #[inline]
     #[must_use]
+    #[deprecated = "Use Guild::partial_member_permissions_in, as this doesn't consider permission overwrites"]
     pub fn partial_member_permissions(
         &self,
         member_id: UserId,

src/model/guild/partial_guild.rs

@@ -1035,6 +1035,7 @@ impl PartialGuild {
     /// Calculate a [`Member`]'s permissions in the guild.
     #[inline]
     #[must_use]
+    #[deprecated = "Use PartialGuild::member_permissions_in, as this doesn't consider permission overwrites"]
     pub fn member_permissions(&self, member: &Member) -> Permissions {
         Guild::user_permissions_in_(
             None,
@@ -1053,6 +1054,7 @@ impl PartialGuild {
     /// Panics if the passed [`UserId`] does not match the [`PartialMember`] id, if user is Some.
     #[inline]
     #[must_use]
+    #[deprecated = "Use PartialGuild::partial_member_permissions_in, as this doesn't consider permission overwrites"]
     pub fn partial_member_permissions(
         &self,
         member_id: UserId,