PermissionsService Documentation & Stricter Specifications.

[ryantheleach]

It's come to my attention that the PermissionsService documentation is a little lacking in some areas, So I've created this issue to collect comments relating to what needs improving.

https://forums.spongepowered.org/t/luckperms-an-advanced-permissions-system/14274/106

• When you call a #hasPermission method on a Subject (for example), should it include inherited permissions?
• Should it resolve wildcards?
• Should it just include the permissions the Subject explicitly has?
• Whose job is it to apply wildcard rules?
• Is that down to the plugin who implements PermissionService, or the plugins checking for permissions?

[ryantheleach]

Replying to distinguish the list from my opinion.

• When you call a #hasPermission method on a Subject (for example), should it include inherited permissions?
  • Yes. hasPermission is essentially a query that consumers will be using to check to see if an action will take place.
• Should it resolve wildcards?
  • Yes.
• Should it just include the permissions the Subject explicitly has?
  • No, it's a query.
• Whose job is it to apply wildcard rules?
  • The PermissionService

[ST-DDT]

• Should it resolve wildcards?
• Whose job is it to apply wildcard rules?

Wasn't there somewhere the rule that you always inherit the children permissions of a permission?

The user has the following permission:

• plugin.foo

Then he also has

• plugin.foo.bar

Thus plugin.foo and plugin.foo.* are either the same by definition or plugin.foo.* is a strangely named permission that is by no means a wildcard permission.

EDIT: @PermissionDescription#getId() disallows the * character + explains how to setup your/the entire permission structure.

EDIT2: Maybe this should be placed somewhere more prominent like in the PermissionService javadocs.

---

• When you call a #hasPermission method on a Subject (for example), should it include inherited permissions?
• Should it just include the permissions the Subject explicitly has?

IMO the #hasPermission method should consider the following things for the response

• The permissions assigned directly to the subject
• The permissions assigned via inheritance
• The permissions default values (Also see Register permission defaults through CommandCallable/CommandSpec #924)

This is to some extend also stated by the javadocs:

Test whether the subject is permitted to perform an action given as the given permission string.

---

EDIT3:

• Is that down to the plugin who implements PermissionService, or the plugins checking for permissions?

NO its not. The behaviour should be predicable and the same for all plugins.
(otherwise the API is not useable)
The plugin MUST follow the API specs and if it does not its a bug in that plugin.

[Aaron1011]

@zml2008: What are your thoughts on this?

[lucko]

To add onto what's been mentioned here already, I feel stricter guidelines about how defaults should work needs to be included.

In the #hasPermission and equivalent checks, are defaults to be resolved too? There's also some confusion as to whether defaults should be persisted or not.

If they do persist, and you allow server administrators to modify them, then care should be taken to inform plugin developers to not override any changes made by server admins. (I personally feel that they should be transient, but appreciate the benefits of being able to modify or override them)

Additionally, I somewhat agree with @ryantheleach 's thoughts about using PermissionDescriptions, as explained in #924. From what I can tell, there's no use for them at the moment? "the registration does not have any impact on the results of the permission checks of the service." They therefore seem to be somewhat redundant?

Having different defaults for different roles would make sense, given the whole role system is already laid out in PermissionDescription... or, that might be well beyond the scope of the service. I don't know really. I'm just happy to see changes/questions being brought up, as I think a lot does lack clarity.

[ryantheleach]

I think the original goal behind PermissionDescription was to provide templates, + tab-completion for permissions that hadn't yet been filled into a permissions plugin.

[ST-DDT]

PermissionDescription provide a description what the permission is actually good for, so the permission plugin can create some kind of central documentation for server owners. Its pure informational. It also can be used to create some kind of default list so the server owner knows which permission he has to assign. But if he does not assign any permissions the user does not have them.

The problem with default permissions is that they are added secretly to the user, so the admin does not know their use, has issues if they change their definition or are re-set by the plugin. In addition to that he might not know that they exist in the first place. (Blacklist vs whitelist issue)

[lucko]

Ah yes, that does make sense.

I don't think that being added secretly is an issue. It's just something that can be included in plugin's documentation, informing admins that they don't have to manually give the permissions, and to remind them that they might have to override the defaults if they disagree with the plugin author's selection.

I think that therefore is makes more sense that defaults are left as being transient, and are overridden by adding permissions to default groups or directly to subjects through the permission plugin. (in contrast, PEX persists the defaults, and allows server admins to modify them directly.) This needs to be clarified in the JavaDocs. It's causing issues, particularly with Grief Prevention, and will likely cause further issues as plugins override changes made to defaults made by server admins.

GP implements it in a way that doesn't override changes, but other authors may not do the same.
https://github.com/MinecraftPortCentral/GriefPrevention/blob/master/src/me/ryanhamshire/griefprevention/FlatFileDataStore.java#L218 My point is, it needs to be documented.

Anyway, I don't really want to take attention away from the main points in this issue, the defaults thing was just something else I thought I should bring up.

[ST-DDT]

While we are at it we should also fix his:

SpongeAPI/src/main/java/org/spongepowered/api/service/permission/PermissionDescription.java

Line 81 in 6ec0106

* <li>"myplugin.give.type.DIAMOND" - Only grants DIAMOND</li>

What about the providing mod prefix? shouldn't it be minecraft:DIAMONDS but the permission id grammer denies colons for good reason...

Just for reference: #1000 (comment)

[ImMorpheus]

PR (#1387) merged

While we are at it we should also fix his:

Fixed by d453c3e#diff-30abf20e9e6a3cc1d34dd5bf68a981a1