pmmp / PocketMine-MP

A server software for Minecraft: Bedrock Edition in PHP
https://pmmp.io
GNU Lesser General Public License v3.0
3.27k stars 1.54k forks source link

A new approach to replace permissions #3935

Closed SOF3 closed 3 years ago

SOF3 commented 3 years ago

This is a draft RFC. Comments are much wanted. Instead of using :-1:, please reply to this issue via a comment to explain what is bad/needs changing.

Abstract

- Ditch permission parent-child mechanism. - Ditch permission default values. - Generalize `op` and `true` into permissions. - Ditch permission plugins. - Users configure permissions using a configuration file ``` pocketmine.everyone -> pocketmine.command.version pocketmine.operator -> pocketmine.command.give, pocketmine.command.gamemode, pocketmine.command.teleport trial-operator -> pocketmine.operator pocketmine.operator -> NOT someplugin.apply-for-op trial-operator -> someplugin.apply-for-op ```

Introduction

We observed the following problems with the existing permissions system:

Changes

Removing op and true

op has been a special-cased flag in PocketMine, leading to confusing semantics and useless classes like ServerOperator. We propose changing these to permission nodes that are registered by default:

For notop and false, they can be complemented with the NOT operator.

Explicit permission setters

This section redefines the model of permission value evaluation. Existing mechanisms like default values and parent-child are not preserved. In the proposed model, all permissions are false by default. Permissions can be set to true either explicitly or implicitly.

Explicit setters are implemented via code. These setters should only be called from the module that manages the permission. They are primarily intended for flag-based permissions. For example, an area protection plugin can explicitly set/unset a permission when the player enters an area.

It is also recommended that PocketMine provides a builtin UI, either through config or command or both, to allow users to define permissions that are stored persistently, preferably using the same storage as the current OfflinePlayer.

Implicit permission setters

Implicit setters are implemented via a sequence of rules in the form

if({permission} is {true/false}), set {permission} to {true/false}.

Different plugins define their own rules rules. and finally the user also writes a set of such rules. An example usage is permission groups, where the plugin declares that formerly default: op permissions have an implicit relation from pocketmine.role.operator. This effectively allows operators to use the resultant permissions. Users can also specify custom rules on custom groups to achieve what group-based permission plugins like PurePerms currently provide.

The full algorithm to compute the permissions of a permissible is as follows:

$result = [];
foreach($knownPermissions as $permission)
    $result[$permission] = false;

foreach($explicitPermissions as $permission)
    $result[$permission] = true;

foreach($implicitPermissions as $rule) {
    if($result[$rule->condition] xor $result->invertCondition)
        $result[$rule->target] = $rule->value;

A suggested syntax for the user-defined instruction sequence may adopt a custom syntax:

FILE := ( RULE )*
RULE := ( "NOT" )? PERMISSION "->" ( "NOT" )? PERMISSION ( "," ( "NOT" )? PERMISSION )*
PERMISSION := /^[A-Za-z0-9\-_\.]+$$/
buchwasa commented 3 years ago

With "ditching permission plugins" I'd like to know how you intend on doing this, a lot of the permission plugins assign the permissions to groups, where PM should definitely not handle the groups part.

jasonw4331 commented 3 years ago

For my own clarification, is the default parameter of permissions being replaced by the assignment of the permission or is it being designated as a flag assigned under another root flag?

Also how does the implicit and explicit permissions be declared apart form each other? Are you calling these permissions implicit meaning undeclared by the user?

SOF3 commented 3 years ago

With "ditching permission plugins" I'd like to know how you intend on doing this, a lot of the permission plugins assign the permissions to groups, where PM should definitely not handle the groups part.

Why shouldn't PocketMine handle the groups part? Permissions are user groups in a nutshell. Permission plugins have been reinventing the wheel by maintaining their own "groups" mechanism instead of using permissions. As I mentioned above,

It is also recommended that PocketMine provides a builtin UI, either through config or command or both, to allow users to define permissions that are stored persistently, preferably using the same storage as the current OfflinePlayer.

This removes the need of permission plugins as their primary functionality is to assign groups to players. Users can directly assign group permissions for a specific player, and the implicit permission graph will handle the process of translating group permissions into "usual" permissions.

I have thought of renaming permission to flag/user flag to make this a more clear concept, but that is not directly related to this RFC.

Let me refer to @CortexPE's list of features in Hierarhcy for reference:

buchwasa commented 3 years ago

I don't mind letting pocketmine maintain a list of permissions, with the idea of

permission.example: default
permission.exampleop: op

but I really don't like the idea of letting pocketmine handle groups. It just seems like additional garbage that a plugin should do instead of the server software. Sure some people may prefer it that way, but then I could say, some people preferred a built in anticheat, which was determined that a plugin should handle.

SOF3 commented 3 years ago

For my own clarification, is the default parameter of permissions being replaced by the assignment of the permission or is it being designated as a flag assigned under another root flag?

All permissions are false by default. To declare default true/op, you should make them inherit pocketmine.role.everyone/pocketmine.role.operator.

Speaking of that, I would like to simplify the naming of permission nodes a bit, but I have not come up with a good solution. This might relate to some proposal I am mentally drafting (and on Discord) related to improved config handling.

Also how does the implicit and explicit permissions be declared apart form each other? Are you calling these permissions implicit meaning undeclared by the user?

Explicit means that it is directly declared by some module, and implicit means it is induced from the instruction sequence. In terms of flow graphs, explicit permissions are the source vertices, and the sequence of instructions provides the flow edges that propagate information from the flow graph to other vertices. Eventually these information propagate to the sink vertices, which are the "usual" permissions read by plugins.

Suppose we have a builtin module pocketmine.role that assigns roles to players (maybe we have a role.yml config file that specifies which player has which permission). An area protection plugin ap marks players standing inside a region called spawn with the permission ap.region.spawn. So they might use API like this:

// PocketMine, when player logins
foreach($player->getOfflineData()->getExplicitPermissions() as $permission)
    $player->asPermissible()->setExplicitPermission($permission, true);

// AP
function onMove(PlayerMoveEvent $event) {
    foreach($this->regions as $region)
        $event->getPlayer()->setExplicitPermission("ap.region.$region", $region->isPlayerInside($event->getPlayer()));
}

Then the user defines the permission to place blocks (assume it is called can.place.blocks, whichever plugin it came from) as follows:

pocketmine.role.everyone -> can.place.blocks
ap.region.spawn -> NOT can.place.blocks
pocketmine.role.builder -> can.place.blocks

The first line may be declared from some module inside PocketMine similar to the default permissions.

The second line may be declared from some module inside ap, probably allowing user to edit it otherwise.

The third line is something defined by the user directly, in permissions.txt or whatever config file as specified in the RFC.

SOF3 commented 3 years ago

I don't mind letting pocketmine maintain a list of permissions, with the idea of

permission.example: default
permission.exampleop: op

but I really don't like the idea of letting pocketmine handle groups. It just seems like additional garbage that a plugin should do instead of the server software. Sure some people may prefer it that way, but then I could say, some people preferred a built in anticheat, which was determined that a plugin should handle.

PocketMine already handles groups. They are what permissions do. For example ops.txt is a special group that we currently hardcode everywhere. What this RFC suggests is just an easier way for the existing concept of groups to get used directly.

SOF3 commented 3 years ago

I don't mind letting pocketmine maintain a list of permissions, with the idea of

permission.example: default
permission.exampleop: op

but I really don't like the idea of letting pocketmine handle groups. It just seems like additional garbage that a plugin should do instead of the server software. Sure some people may prefer it that way, but then I could say, some people preferred a built in anticheat, which was determined that a plugin should handle.

You might comment that anticheat is not desired by everyone, and so it should be removed from the core, and similarly, permission groups are not desired by everyone and should be removed from the core. But wait. Are you sure permissions are desired by everyone at all? People who don't want permission groups won't want permissions anyway. People who want permissions must use permission groups, even if it's as simple as a single group called operator. This RFC just generalizes operator into a group. It does not add any feature.

Or can you think of an example where a permission manager does not use groups?

SOF3 commented 3 years ago

One concern nobody has raised yet: Might there be problems with implicit instruction precedence? Is it possible that someone wants to make sure a certain instruction is executed after another instruction?

This might make sense for plugin dependencies. But the user permissions.txt is run after everything. Are there any precedence requirements other than running instructions in a dependency plugin before running instructions in a dependent plugin?

dktapps commented 3 years ago

I haven't read this whole proposal yet because I was working on something else at the time, so I'm going to need a TL;DR.

I've already made a baseline implementation of permission default removal (without BC breaks to plugin.yml, for now) in https://github.com/pmmp/PocketMine-MP/commit/3a755507c1018f902d2f9f00281d0616357a71a2.

dktapps commented 3 years ago

I want to point out for posterity that several elements of this RFC are already implemented by the current system (albeit very badly). For example, there is no parent/child relationship between permissions despite popular belief. The whole "child permission" thing is poorly named.

As it stands right now, any permission can cause any other permission to be applied. It doesn't have to be related by definition. For example, this means that pocketmine.command.op could, if it wanted to, cause pocketmine.command.give to be assigned when the permissible has pocketmine.command.op. Any permission is able to cause assignment of any other permission, even if not related by definition.

The only parts of this RFC that constitute addition of features are the proposal for a permissions configuration file. No part of the RFC breaks functional backwards compatibility at all (although it may involve API breaks).

ghost commented 3 years ago

no

dktapps commented 3 years ago

I'm going to close this and write a shorter feature proposal that just includes the things we don't already have (i.e. a permission groups config file). The discussion on this issue is misleading to readers.