Beginnings of a permission layer

[Wunka]

This is a first step in the direction of #1961

Usage:
Users / Groups can be either whitelisted or blacklisted from a permissionPath.
For example a user can be granted the permission: "command" but be blacklisted from "command/spawn"
This would mean the user can run any command except spawn.

Users can be added to groups.
User permissions precede the group permissions, so if a user is blacklisted from the spawn command and joines a group who is whitelisted for the spawn command, the user would still have no permission to use spawn.

Tasks:

• add user black/whitelist
• add groups
• add tests
• save permissions in zon
• send client its complete permission tree to enable: Client-side prediction for (some) chat commands and client-side only commands #2414
• command permission based on arguments (after Better argument parser #1425)
• add commands to perform permission changes

[IntegratedQuantum]

Could you give an example of how this would look?

Also one relevant case would be to allow for localized permissions to e.g. forbid editing in a region (#81)
Would this fit into your system?

[Wunka]

Example for /spawn: (we could also add the prefix command to show that we are talking about command permissions)
server hoster has complete access so: user.addPermision("spawn.*")
other users can only get their spawnpoint: user.addPermision("spawn.get")

you can chain the dots as long as we don't reach the limit with recursions (for checking and adding permissions)
so making something like user.addPermission("blocks.protecotor.[some way to identify the explicit block]") would be possible

[IntegratedQuantum]

Yeah, that seems reasonable (though with how much this is like a file system, I'd prefer if it used slashes).
For the protector block it would probably not address the block directly but rather mentioning a string that can be shared for multiple protector blocks (otherwise it would be a nightmare to add another player to your region).

And of course we should pick a reasonable hierarchy, there should be a subcategory for commands, and inside of commands there should be a subcategory of get commands.

Also one problem I see in your code is that if you enable a parent node then all the subtree information is lost. So if I give you permissions to run all commands, then take this permission away at a later point, you cannot even run /help anymore.
On the other hand it could also be reasonable to be able to disable child nodes from an enabled parent (maybe I want you to be able to run all commands except from world edit commands, or all getter commands, except from getting the seed).

[Wunka]

okay so to adresss a few of your things I think I need somekind of global saved permission tree. So I think I will change the system so that as an example commands register themself like registerPermission("command/get/spawn") that way we have a complete set.

if you enable a parent node then all the subtree information is lost. So if I give you permissions to run all commands, then take this permission away at a later point, you cannot even run /help anymore.

I think I will introduce an optional fn checkPermission(msg) bool to _commands.zig which in default checks just the command name (like it is now) and for help: overwrite it so that it just checks the permission of the command you want to run help on.

On the other hand it could also be reasonable to be able to disable child nodes from an enabled parent (maybe I want you to be able to run all commands except from world edit commands, or all getter commands, except from getting the seed).

This is already supported? There is just not a good way to enable that. But through the global saved permission tree you could do things like: user.addPermission("commands/get/*): user.removePermission("commands/get/seed");

Also in that regard what do you think about * ? should it mabe just be usable at the top so user.addPermission("*") and for commands you don't need it. Just: user.addPermission("commands/get") to get permission to all get commands

[IntegratedQuantum]

okay so to adresss a few of your things I think I need somekind of global saved permission tree.

I don't see how a global permission tree makes this easier, if at all I think it adds more edge cases, since the tree can change between updates (or just during normal gameplay).

[Wunka]

This is already supported? There is just not a good way to enable that. But through the global saved permission tree you could do things like: user.addPermission("commands/get/*): user.removePermission("commands/get/seed");

A lot can probably be done without the global tree but this not for what I see.
(execept I introduce being able to switch between whitelist and blacklist)

[Wunka]

The tree could maybe be also only used to get a picture of the current tree. So "*" instead of giving .all could just loop over all subPermissions in the global tree and add them all to the user.

[IntegratedQuantum]

(execept I introduce being able to switch between whitelist and blacklist)

Yeah, that's what I would propose, just add an enum to each node which has 3 states allowed, forbidden and neutral.
Then during iteration you have to go the entire way from top to bottom of the tree and switch states.

Another approach would be to kill recursion (and I'd always be in favor of that), and have just a single per player white and a single per player black list with the full permission path, then to check permission you'd basically just do:

var iterator = std.mem.findRightMostIteratorWhateverIDontKnow(permissionPath, '/');
while(iterator.next()) |endIndex| {
    if(blacklist.has(permissionPath[0..endIndex])) return false;
    if(whitelist.has(permissionPath[0..endIndex])) return true;
}
return false;

[Wunka]

Another approach would be to kill recursion (and I'd always be in favor of that), and have just a single per player white and a single per player black list with the full permission path, then to check permission you'd basically just do:

Do I understand that right that your findRightMostIteratorWhateverIDontKnow would make out of example "command/get/spawn" -> {"command/get/spawn", "command/get", "command"} ?
that way all whitelist/blacklist would only work on the deepest specialization?
If yes I think I will do that.

[IntegratedQuantum]

Yeah, exactly.
And the list itself could just be a StringHashMapUnmanaged(void)

[Wunka]

Changed to the black/whitelist system. (much better)
While on that I also added groups.

For the whitelist / blacklist system as long as one group has whitelist access you are good to go.
But the userlists have still higher priority so if a user is blacklisted from a command the game doesn't care even if he is part of some root group which has access to everything.

And for commands in general: I don't want to spend as much time into things like seperate permissions for the seperate subcommands as I think this would be much easier with #1425

[IntegratedQuantum]

The other important thing would be sharing with the client. This is relevant for anything that has client-side prediction (e.g. gamemode permissions, protector block, #2414, ...)

For that I'd suggest to precompute the full set of permission from the user+group permission and send that to the client on each change. I think that would be easier and more efficient.

Also another important reminder: Add testing! The permission system should have unit tests for all of its functionality.

[Wunka]

added tests and updated the description of the PR to reflect the changes

[Wunka]

Oh what I also wanted to mention is that I am thinking of adding a kinda root permission. currently there is no way for someone to have access to literally everything (without specifing every top level permission like "command"). So maybe if someone has access to "root" or some other keyword they just skip every check.

Or we start everything with "/" and then you can get access to that

[IntegratedQuantum]

Starting everything with a slash seems to be a reasonable option.