QSL Permissions API

[NoahvdAa]

Adds a new library to QSL; permissions!

This is currently just a near-direct port of Lucko's fabric-permissions-api (with his authorization, of course). The test mod includes a very simple global permission implementation that can be toggled with /togglepermission <permission>. The command /permissionrequired requires the permission quilt_permission:required_permission and other permissions can be checked with /checkpermission <permission>.

TODO:

• [x] Allow using Identifiers/Resource Keys as permissions.
• [x] Interface-inject hasPermission into PlayerEntity.
• [x] Allow lookups via UUID.

Discord thread for reference

[NoahvdAa]

methods to add/remove permissions

how should this work? Most permission plugins have a group system, so (un)setting a permission for an individual user could cause semi-unpredictable behaviour (I think)

[Patbox]

This api could always add support for group-representation and have some methods for non-inherited ones

[williambl]

methods to add/remove permissions

how should this work? Most permission plugins have a group system, so (un)setting a permission for an individual user could cause semi-unpredictable behaviour (I think)

I believe most have per-player override mechanisms - though as Patbox says maybe some kind of group support in the API would be good. Don't want it too bloated tho

[NoahvdAa]

This has now been moved into the management library, let's hope I didn't miss anything 😅

[Patbox]

It might be good idea to pass instance of MinecraftServer with event/uuid methods for mods that don't want to depend on global state

[TheGlitch76]

It might be good idea to pass instance of MinecraftServer with event/uuid methods for mods that don't want to depend on global state

That makes sense, @NoahvdAa could you implement that?

[Patbox]

It also could be good to pass ServerCommandSource, so permission checks can be done for non entities (see, command blocks)

[NoahvdAa]

It also could be good to pass ServerCommandSource, so permission checks can be done for non entities (see, command blocks)

That already exists https://github.com/QuiltMC/quilt-standard-libraries/blob/d4834d467c0a5a5efa021d8b338e18f67c0b966e/library/management/permission/src/main/java/org/quiltmc/qsl/permission/api/PermissionCheckEvent.java#L47-L62

[Patbox]

Yes, but not for mods that register handler for that event

[lucko]

@TheGlitch76 asked for my feedback, so here I am, hi! 👋

In general, looks pretty good. However, I have two questions about the changes relative to the Fabric API this was ported from. (naturally 😛)

1. Identifier over String?
   • The rest of the Minecraft ecosystem uses . delimited strings, and server admins are already familiar with this format. e.g. luckperms.user.info
   • The change here would force that permission to become luckperms:user.info on Quilt, and would be a confusing caveat in the usage of plugins/mods that span multiple platforms
   • It is already well accepted that that first component of a permission node should be the mod/plugin id, so is there a benefit of forcing identifiers to be used via the API? Maybe there is.. but I think it is worth discussing!
2. Using a UUID as the source type in the event?
   • Just keeping it as a ServerCommandSource would be much better in my opinion.
   • If the intention was to support lookups for offline players, then I think that needs some more thought. For LuckPerms at least, this usually requires a database lookup. It would be more appropriate if there was a separate event for offline lookups that returned a CompletableFuture<Tristate> instead. Maybe that would be better off as an addition at a later time?

In general, I think the PR would benefit from some additional example implementation/usage to see if the API is actually nice to use. This was the main factor in the initial design: https://github.com/lucko/fabric-permissions-api/blob/master/USAGE.md

Hope that's useful :)

[Patbox]

1. Identifier over String

It's legacy standard. Using identifiers is honestly best way to do it for modern MC. It hould be easy to support by anything really. If you implement it correctly that is (making prefix default to minecraft: would solve issue for legacy, but it's still bad idea for mods to use it)

I can fully agree with 2nd one.

[NoahvdAa]

Thank you for taking the time to review this, and sorry for not replying sooner.

1. Identifier over String?

I originally used strings but I changed them over to identifiers for mainly the following reasons:

• Almost everything in Minecraft uses identifiers now, it wouldn't really make sense for permissions to be the exception, using an arbitrary separator instead.
• It forces authors to properly set a namespace. I've seen a few bukkit plugins (mostly made by beginners) that didn't have properly 'namespaced' their permissions, causing them to potentionally duplicate with other plugins. Requiring people to enter a namespace would mostly solve this problem, since people would essentially have to go out of their way to cause a conflict (since duplicate mod ids aren't allowed).
• Implementations could always have a setting to fall back to legacy permissions, converting the identifier to the legacy format, e.g. quilt_permission:required_permission -> quilt_permission.required_permission.

2. Using a UUID as the source type in the event?

I agree with this one, and the current implementation causes more problems than it solves. I'll remove the UUID-based lookups for now, so we can hopefully get this PR out the door for 1.19.

---

The testmod/example mod provides a basic demo of the API, I'll see if I can expand it or add some extra docs.

[OroArmor]

This targets 1.19 but has 1.18 code

[williambl]

I was going to leave another comment about Numeric permissions but I've convinced myself they're not actually necessary: it's cleaner to use something like essentials.sethome.multiple.[groupname] and then have groupname: 3 in some config file than it is to use essentials.sethome.multiple.3 directly.

With that in mind, I'm now happy with the capabilities the API provides.

[JTK222]

I know I am quite late to the discussion, but as I am starting to port all my mods to a multiloader setup. I required a Permission API and found this with the help of the quilt discord. I took a first glance at it and have some complaints. Just like the Bukkit perms API, this is quite limited in its functionality.

First, it seems to be limited to boolean perms, unless I am mistaken. (E.g. how would you want to make a perm of how many chunks a user can load?)

Second, the entire API seems to be aimed towards command use. And not necessarily towards all possible use cases. It isn't possible to check for offline user perms, which in a modded context isn't something rare. E.g. whether a block breaker has permissions to break a block should be based on the player that placed it. Which might or might not be online.

Right now the system does also not differentiate between players, console, command blocks and similar. Nothing bad on it's own, but it can lead to a bad user experience when modders don't consider this. (E.g. imagine first having to setup your console to have perms to stop the server)

And finally, I cannot see any handling for what happens when there are multiple permission manager mods, it seems as if they will just work chained. But can the user set an order or similar?