Merge pull request #152 from Skillz4Killz/fix-perms

rework permissions checking

src/utils/permissions.ts

@@ -2,6 +2,7 @@ import { cacheHandlers } from "../controllers/cache.ts";
 import { botID } from "../module/client.ts";
 import type { Guild } from "../structures/guild.ts";
 import type { Role } from "../structures/role.ts";
+import type { RawOverwrite } from "../types/guild.ts";
 import type { Permission } from "../types/permission.ts";
 import { Permissions } from "../types/permission.ts";
 
@@ -104,87 +105,77 @@ export async function hasChannelPermissions(
   const member = guild.members.get(memberID);
   if (!member) return false;
 
-  const memberOverwrite = channel.permission_overwrites?.find((o) =>
+  let memberOverwrite: RawOverwrite | undefined;
-    o.id === memberID
+  let everyoneOverwrite: RawOverwrite | undefined;
-  );
+  let rolesOverwrites: RawOverwrite[] = [];
 
-  const rolesOverwrites = channel.permission_overwrites?.filter((o) =>
+  for (const overwrite of channel.permission_overwrites || []) {
-    member.roles.includes(o.id)
+    // If the overwrite on this channel is specific to this member
-  );
+    if (overwrite.id === memberID) memberOverwrite = overwrite;
-
+    // If it is the everyone role overwrite
-  const everyoneOverwrite = channel.permission_overwrites?.find((o) =>
+    if (overwrite.id === guild.id) everyoneOverwrite = overwrite;
-    o.id === guild.id
+    // If it is one of the roles the member has
-  );
+    if (member.roles.includes(overwrite.id)) rolesOverwrites.push(overwrite);
+  }
 
   const allowedPermissions = new Set<Permissions>();
 
+  // Member perms override everything so we must check them first
   if (memberOverwrite) {
-    // One of the necessary permissions is denied
+    for (const perm of permissions) {
-    if (
+      // One of the necessary permissions is denied. Since this is main permission we can cancel if its denied.
-      permissions.some((perm) => BigInt(memberOverwrite.deny) & BigInt(perm))
+      if (BigInt(memberOverwrite.deny) & BigInt(perm)) return false;
-    ) {
-      return false;
-    }
-    permissions.forEach((perm) => {
       // Already allowed perm
-      if (allowedPermissions.has(perm)) return;
+      if (allowedPermissions.has(perm)) continue;
       // This perm is allowed so we save it
       if (BigInt(memberOverwrite.allow) & BigInt(perm)) {
         allowedPermissions.add(perm);
       }
-    });
+    }
   }
 
   // Check the necessary permissions for roles
-  if (rolesOverwrites?.length) {
+  for (const perm of permissions) {
-    if (
+    // If this is already allowed, skip
-      rolesOverwrites.some((overwrite) =>
+    if (allowedPermissions.has(perm)) continue;
-        permissions.some((perm) =>
-          (BigInt(overwrite.deny) & BigInt(perm)) &&
-          // If another role allows these perms then they are not denied
-          !rolesOverwrites.some((o) => BigInt(o.allow) & BigInt(perm)) &&
-          // Make sure the memberOverwrite does not allow this perm
-          !(memberOverwrite && BigInt(memberOverwrite.allow) & BigInt(perm))
-        )
-      )
-    ) {
-      return false;
-    }
 
-    permissions.forEach((perm) => {
+    for (const overwrite of rolesOverwrites) {
-      // Already allowed perm
-      if (allowedPermissions.has(perm)) return;
-      rolesOverwrites.forEach((overwrite) => {
       // This perm is allowed so we save it
       if (BigInt(overwrite.allow) & BigInt(perm)) {
         allowedPermissions.add(perm);
-        }
+        break;
-      });
-    });
       }
 
-  // Check the necessary permissions for everyone
+      // If this role denies it we need to save and check if another role allows it, allows > deny
-  if (
+      if (BigInt(overwrite.deny) & BigInt(perm)) {
-    everyoneOverwrite
+        // This role denies his perm, but before denying we need to check all other roles if any allow as allow > deny
-  ) {
+        const isAllowed = rolesOverwrites.some((o) =>
-    if (
+          BigInt(o.allow) & BigInt(perm)
-      permissions.some((perm) =>
+        );
-        BigInt(everyoneOverwrite.deny) & BigInt(perm) &&
+        if (isAllowed) continue;
-        !allowedPermissions.has(perm)
+        // This permission is in fact denied. Since Roles overrule everything below here we can cancel ou here
-      )
-    ) {
         return false;
       }
-    // If all permissions are granted
-    if (
-      permissions.every((perm) =>
-        BigInt(everyoneOverwrite.allow) & BigInt(perm)
-      )
-    ) {
-      return true;
     }
   }
 
+  if (everyoneOverwrite) {
+    for (const perm of permissions) {
+      // Already allowed perm
+      if (allowedPermissions.has(perm)) continue;
+      // One of the necessary permissions is denied. Since everyone overwrite overrides role perms we can cancel here
+      if (BigInt(everyoneOverwrite.deny) & BigInt(perm)) return false;
+      // This perm is allowed so we save it
+      if (BigInt(everyoneOverwrite.allow) & BigInt(perm)) {
+        allowedPermissions.add(perm);
+      }
+    }
+  }
+
+  // Is there any remaining permission to check role perms or can we determine that permissions are allowed
+  if (permissions.every((perm) => allowedPermissions.has(perm))) return true;
+
+  // Some permission was not explicitly allowed so we default to checking role perms directly
   return botHasPermission(guild.id, permissions);
 }