EssentialsX

EssentialsX

2M Downloads

Permission based item spawn doesn't actually check permissions

devxoul opened this issue ยท 0 comments

commented

Type of bug

Other unexpected behaviour

/ess version output

[22:30:29] [Server thread/INFO]: CONSOLE issued server command: /ess version
[22:30:29] [Server thread/INFO]: Server version: 1.16.5-R0.1-SNAPSHOT 3029-Spigot-79d53c2-e9ad7cb (MC: 1.16.5)
[22:30:29] [Server thread/INFO]: EssentialsX version: 2.19.0-dev+100-d757877
[22:30:29] [Server thread/INFO]: LuckPerms version: 5.3.16
[22:30:29] [Server thread/INFO]: EssentialsXProtect version: 2.19.0-dev+100-d757877
[22:30:29] [Server thread/INFO]: Vault is not installed. Chat and permissions may not work.
[22:30:29] [Server thread/INFO]: You are running a server with limited API functionality. EssentialsX will still work, but certain features may be disabled.
[22:30:29] [Server thread/INFO]: Fetching version information...
[22:30:29] [Craft Scheduler Thread - 20/INFO]: You're running the latest EssentialsX dev build!

Server startup log

Not necessary

EssentialsX config files

Start from auto generated config and set permission-based-item-spawn: true

Error log (if applicable)

Not necessary

Bug description

It seems that User.canSpawnItem() never returns false when permission-based-item-spawn is true and user has only essentials.item permission.

Steps to reproduce

  1. Bootstrap a new server
  2. Set permission-based-item-spawn: true
  3. Add a user permission essentials.item
  4. Log in with the user and execute the command: /item glass 1

Expected behaviour

After step 4, I expected to see 'You don't have such permission' message.

Actual behaviour

A glass is spawned.

It seems that User.canSpawnItem() never returns false when permission-based-item-spawn is true and user has only essentials.item permission.

The documentation says when you set permissino-based-item-spawn to true, item-spawn-blacklist will be ignored.

# Set this to true if you want permission based item spawn rules.
# Note: The blacklist above will be ignored then.

A little change can correct this behavior.

  @Override
  public Boolean canSpawnItem(final Material material) {
      if (ess.getSettings().permissionBasedItemSpawn()) {
          final String name = material.toString().toLowerCase(Locale.ENGLISH).replace("_", "");

          if (isAuthorized("essentials.itemspawn.item-all") || isAuthorized("essentials.itemspawn.item-" + name))
              return true;

          if (VersionUtil.getServerBukkitVersion().isLowerThan(VersionUtil.v1_13_0_R01)) {
              final int id = material.getId();
              if (isAuthorized("essentials.itemspawn.item-" + id)) return true;
          }

+         return false
      }
      return isAuthorized("essentials.itemspawn.exempt") || !ess.getSettings().itemSpawnBlacklist().contains(material);
  }

It seems that the problem started from 3b1cef9. It used to separate both two cases with else(:).

ess.getSettings().permissionBasedItemSpawn() ? (!user.isAuthorized("essentials.itemspawn.item-all") && !user.isAuthorized("essentials.itemspawn.item-" + itemname)) : (!user.isAuthorized("essentials.itemspawn.exempt")