Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Establish all permissions as op required #86

Closed
wants to merge 2 commits into from
Closed

Conversation

Oribuin
Copy link

@Oribuin Oribuin commented Dec 24, 2024

hello, after using this plugin on a production environment, we were unaware that players by default had access to import their own image from any source, and assumed it was locked behind permission as it should've been, we were swiftly mistaken.

Players having access to command that establishes a web connection, allowing them to import any image into the server with no way of tracking it is a severe oversight. The type of images that someone could just upload onto a server shouldn't need to be expressed.

I have implemented a new permission, imageframe.player, which can be granted to players to provide them access to all the previous permissions they had access to prior to this change.

I understand this change will result in a large influx of angry "my players have lost access to all their commands >:(" but, I think that is a valuable price to pay for the risk of highly graphic images being uploaded with no prior knowledge.

thank u!

Implements a permission that server owners to use that can grant them all the commands that a player may need to use the plugin.
Remove all player's access to imageframe permissions by default, this is a security risk and access should only be granted selectively
@Oribuin Oribuin changed the title [Permission] Establish all permissions as op required Establish all permissions as op required Dec 24, 2024
@lukemango
Copy link

a much needed change

@LOOHP
Copy link
Owner

LOOHP commented Dec 24, 2024

I think this is a reasonable change. Although perhaps we could just put create, overlay and clone under the new aggregated player permission and keep the others untouched, since those are the only ones that could open network requests.

@Oribuin
Copy link
Author

Oribuin commented Dec 24, 2024

Personally, I don't think any permission should be granted by default (except small examples like an mcmmo permission to actually use a skill)

@LOOHP
Copy link
Owner

LOOHP commented Dec 24, 2024

Usually I leave some default functions on true because I always get questions about something not working or straight up not knowing a feature exists just because they haven't given themselves permission. I guess that's the inverse of this situation. Whether we should have unknowingly enabled functions or unknowingly disabled functions.

@lukemango
Copy link

lukemango commented Dec 24, 2024

In my opinion it is safer to have things disabled unknowingly than enabled so there's no adverse and/or unexpected effects for server operators, resulting in yet more change requests

@LOOHP LOOHP closed this in fe7a551 Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants