revoltchat / backend

Monorepo for Revolt backend services.
https://developers.revolt.chat/api/
Other
1.14k stars 125 forks source link

feature request: Replace the `permissions` bitfield object with a more robust and extensible object #291

Open QuantumToasted opened 10 months ago

QuantumToasted commented 10 months ago

What do you want to see?

One of my greatest complaints about Discord's permission system is how limited it is as it uses a (64-bit?) bitfield object to represent permissions. On the surface, it's pretty clear why it was chosen initially, as it takes up the least amount of space and still is able to represent a myriad of permission values in a single field.

However, I feel as it has its flaws and limitations:


The "problem":

For example, let's use the existing Permission bitfield as it is today (hopefully I'm not missing any values).

ManageChannels = 1ul << 0,
ManageServer = 1ul << 1,
ManagePermissions = 1ul << 2,
ManageRoles = 1ul << 3,
ManageCustomisation = 1ul << 4,
KickMembers = 1ul << 6,
BanMembers = 1ul << 7,
TimeoutMembers = 1ul << 8,
AssignRoles = 1ul << 9,
ChangeNickname = 1ul << 10,
ManageNicknames = 1ul << 11,
ChangeAvatar = 1ul << 12,
RemoveAvatars = 1ul << 13,
ViewChannel = 1ul << 20,
ReadMessageHistory = 1ul << 21,
SendMessage = 1ul << 22,
ManageMessages = 1ul << 23,
ManageWebhooks = 1ul << 24,
InviteOthers = 1ul << 25,
SendEmbeds = 1ul << 26,
UploadFiles = 1ul << 27,
Masquerade = 1ul << 28,
UseReactions = 1ul << 29,
Connect = 1ul << 30,
Speak = 1ul << 31,
Video = 1ul << 32,
MuteMembers = 1ul << 33,
DeafenMembers = 1ul << 34,
MoveMembers = 1ul << 35

This represents 29 permission values, already almost half of the available bitfield space. Ignoring conflicts with existing/planned values, if we wanted to expand many of these permissions to ones which represented a "single responsibility" (create, edit, delete, etc.) instead of an overreaching "manage" permission...

(Disclaimer: This is a mockup and reflects my opinion alone on what permissions should/could be split)

CreateChannels,
EditChannels,
DeleteChannels,
EditServerDetails,
EditSystemChannels,
EditRolePermissions,
EditChannelPermissions,
CreateRoles,
EditRoles,
DeleteRoles,
CreateCustomizations, // sorry, I'm american
EditCustomizations, // not a feature ATM
DeleteCustomizations,
KickMembers,
BanMembers,
TimeoutMembers,
GrantRoles,
RevokeRoles,
ChangeNickname,
ManageNicknames,
ChangeAvatar,
RemoveAvatars,
ViewChannel,
ReadMessageHistory,
SendMessage,
PinMessages, // not a feature ATM
UnpinMessages,
DeleteMessages,
DeleteReactions,
CreateWebhooks,
EditWebhooks,
DeleteWebhooks,
InviteOthers,
SendEmbeds,
UploadFiles,
Masquerade,
UseReaction,
Connect,
Speak,
Video,
MuteMembers,
DeafenMembers,
MoveMembers

After this: 43 distinct permissions. While still a pretty far reach from the limit(?) of 63, you can see that just something as simple as splitting up a few existing permissions can quickly increase the number of flags used. Discord, if a similar approach were applied to the permission flags there, would likely be dangerously close to the limit.


Proposal

Now, I'm doing a lot of complaining and fearmongering about this and my belief that there's a potential "permission doomsday clock" ticking, but one might ask...So what do you propose is better?

To be honest: I don't have the best answer for this. There are several different ways that this could be accomplished, all of them likely valid, but they all come at the same cost: performance (likely), and the size of the data sent to/from the API. Nothing would come close to a simple integer bitfield. But I'll play devil's advocate and propose something anyway.

Personally, I always found CraftBukkit's permission format to be simple and easily digestible, while also quite extensible. It allows for wildcards (*) to represent "all" values of a particular nested permission.

For example, let's convert my "extended" permissions bitfield into this new system (with a small number of naming changes):

channel.manage.create
channel.manage.edit
channel.manage.delete
channel.message.history
channel.message.send
channel.message.pin
channel.message.unpin
channel.message.delete
channel.message.reactions.add
channel.message.reactions.delete
channel.message.embeds
channel.message.files
channel.message.masquerade
channel.voice.connect
channel.voice.speak
channel.voice.video
server.manage.details
server.manage.systemchannel
server.invites.create
server.invites.delete
permissions.manage.role
permissions.manage.channel
role.manage.create
role.manage.edit
role.manage.delete
role.grant
role.revoke
customization.manage.create
customization.manage.edit
customization.manage.delete
member.manage.kick
member.manage.ban
member.manage.timeout
member.manage.mute
member.manage.deafen
member.manage.move
member.manage.nickname
member.manage.avatar
member.nickname
member.avatar
webhook.manage.create
webhook.manage.edit
webhook.manage.delete

Some benefits of this system:

Some drawbacks of this system:


Conclusion

At the end of the day, this would be an absolutely massive breaking change for Revolt as a platform and would require the cooperative work of dozens of people most likely - frontend, backend, as well as anyone who has written clients, API wrappers, etc, to interface with the API itself. I can't in all honesty say this is the best idea for the platform - but I think it would separate it more from its competitors and maybe give it an edge. Better to do something this drastic early on before the member base gets too high to risk a change this massive?

insertish commented 10 months ago

In terms of feasibility of implementing something like this:

Once the backend stabilises, I do want to release a v2 or rather "v1" of the API. I think this would be an appropriate place to introduce a breaking change like this (both the current API and the new API would both be available for use). Internally, the old API can then just remap a bitfield to the permission strings and vice-versa.