EssentialsX

EssentialsX

2M Downloads

No permissions check for essentials.fly.safelogin

Tsuser1 opened this issue ยท 17 comments

commented

EssentialsX version (/essentials):
2.0.1
Server software (/version):
Spigot-1.12
EssentialsX config (if applicable):
Not applicable

If players have the essentials.fly.safelogin permission they get fly on connect regardless if they have essentials.fly, this violates the "permissions non-inclusive" style of Essentials.

commented

We granted essentials.fly.safelogin and players can just jump off a cliff (mind you they don't have essentials.fly) and relog, then boom! Instant fly.

commented

yeah, that's not good. Is it possible there is another plugin/mod being used that provides "fly" type permissions? I use Residence Reloaded and there are fly flags that can enable flying when flying isn't provided through essentials. Other plugins/mods provide similar options for the same and can cause conflicts like this.

One other question if I may, the essentials versions will show 2.0.1 as the base version, what build are the servers running? That would look like: Essentials reloaded 2.0.1-b528 after typing in the /ess ver on the command console.

commented

Full version: Essentials 2.0.1-b528

I tested this on our development server (no other plugins installed) and confirmed it is an issue. This is an Essentials bug.

commented

As a workaround, ensure that players are only granted essentials.fly.safelogin when they have essentials.fly.

This is most likely unintended behaviour.

commented

if you dont have too many people, you could always try giving it to the people who have fly only rather than per group. I have had to do this while growing. But i am interested how this cn be corrected in a more permanent sense.

commented

Yes and I'm gonna do that for 862 players @FatmanC ;)

(Better solution please.)

commented

I must be missing something in the original post. There are three permissions that I use and they work on all my servers running BungeeCord / Spigot 1.12.1

  1. essentials.fly
  2. essentials.fly.others
  3. essentials.fly.safelogin
    On the worlds I allow flying, only 1 and 3 are provided. Admins / Mods have number 2.
    On the worlds I do NOT allow flying, only the Admins / Mods have all three. All others do not have the permission node. We could have also just negated the permission but not having it means we can grant fly anytime.
commented

@md678685 proposed bugfix #1548

commented

@smmmadden If users run the given command correctly (or any suitable alternative), they will receive a version string 2.0.1-bXYZ. To my knowledge, there isn't an accessible string anywhere in the plugin that reports 2.0.1 as the version. It's up to users to provide the correct information either in the initial post or when prompted.

That said, #1514 aims to simplify the process of collecting version and plugin information from users by providing a single command that outputs and/or dumps info as required. It's not yet finished, hence why it hasn't been merged yet.

@Tsuser1 The fix in your PR works for some use cases but not all. I'll push a fix later.

commented

The Essentials documentation for essentials.fly.safelogin states:

Players with this command (sic) will automatically switch to fly mode if they login whilst floating in the air.

There isn't an actual bug here - EssentialsX is doing exactly what the permission states it should be doing. The issue arises when players have flight enabled for them by an admin but don't have permission to use it - flight is always reenabled when they log back in if they aren't on the ground and have essentials.fly.safelogin, even if this is undesired.

See here for the proposed solution.

Update: see below.

commented

@Tsuser1 After some consideration over both PR #1548 and PR #1555, my suggestion would be to revoke essentials.fly.safelogin from users who shouldn't always be able to fly when they log in. The reason for this is that this permission check has remained unchanged for over 4 years, and it doesn't seem necessary to change it for this use case alone.

You can see the rest of the discussion in PR #1555.

commented

The problem with this @md678685 is that I would have to manually do this for nearly 7,000 players, that's crazy!

commented

If you're using groups, I don't see how this is an issue. If it's absolutely necessary then you could automate the permissions changes with a script, but if you're duplicating lists of permissions per player then you should really be using groups to handle this.

Regardless, the permission's behaviour is intended.

commented

I want this added too, seems pretty BS to me.
~Maelstrom

commented

Also, the fact that this could be a payed add-on makes it even worse. You're basically allowing people to scam their ways into getting an add-on. So please fix the bug. :)
~Maelstrom

commented

@Tsuser1 Relevant question to this issue. Why are you having to do this manually for nearly 7,000 players? Furthermore, with all due respect this is an overlooked area in your setup and not directly an issue of EssentialsX. You're asking us to change the existing behaviour all of our users are relying on so that you do not have to go through the process of updating 7,000 players.

Create groups for grouping players and permissions. This will simplify updating groups of players at a time. If it is an item on your store you may go as far as to create a group for each individual item and maintain it all there. Either way, if something changes in the future with any plugin you're using, you'll still end up updating a group of players.

@Rin-Son-of-Satan if you're hoping to threaten us or scare us into doing something about this invalid issue, don't bother. Go read the license.

commented

Closing as this is expected behaviour as per official Essentials documentation.