ForgeEssentials / ForgeEssentials

Repository for ForgeEssentials 1.x
https://www.curseforge.com/minecraft/mc-mods/forge-essentials-74735
GNU General Public License v3.0
282 stars 144 forks source link

I can not turn off /give #2360

Closed Achim-Sommer closed 5 years ago

Achim-Sommer commented 6 years ago

I want to turn off /give, because JIE Cheating Mode.

So /give is not working. But the player can still turn on Item Cheating mode. And get the Items.

LoganDark commented 6 years ago

did you deop them?

Zethalion commented 5 years ago

Might be an issue on JEI's side. Have you posted on their bug tracker?

LoganDark commented 5 years ago

@Zethalion IIRC, JEI does not use /give if the user is opped and JEI is on the server. It uses JEI on the server to give the player the items.

BSKCorp commented 5 years ago

I like to add to this as I am also having the same problem. I have created a group in FE called Member, gave perm command.give deny. JEI is on client only. Not in server. Players can still use cheat mode to give items to themselves. I even tested Non OPed on the server. Tested with FE disabled (removed) And Cheat mode was denied to give myself the item. Re-activated FE and was able to pull the same item without a problem. No settings changed between FE being active and Non-Active.

ovingiv commented 5 years ago

I would also like to add that I am running to this issue as well. I've done the same steps as BSKCorp and received the same results.

mezz commented 5 years ago

If the player is not OP or creative, JEI checks if they have permission to use the give command. It seems like ForgeEssentials is saying they do have permission, and maybe it blocks them from using it at some other point. Here is the code JEI uses to check: https://github.com/mezz/JustEnoughItems/blob/e5ec552114189aa814940c18624b12c4b8b8e6ab/src/main/java/mezz/jei/util/CommandUtilServer.java#L70

spacebuilder2020 commented 5 years ago

This is probably what is causing the error. In forge essentials, op levels are ignored.

spacebuilder2020 commented 5 years ago

@mezz Also, something to note is the standard implementation of checkPermission simply returns the result of sender.canUseCommand.

To check permission nodes, you need to use net.minecraftforge.server.permission.PermissionAPI.hasPermission

mezz commented 5 years ago

Thanks! I think the JEI code is a bit older than PermissionAPI, I will make sure to update what I have to match the latest practices.

mezz commented 5 years ago

I seems like the PermissionAPI should only apply to modded commands, not vanilla ones like /give.

I tried reproducing this issue with the latest ForgeEssentials and JEI on 1.12.2 but I was not able to. Can someone give me specific steps to reproduce this, assuming I have no knowledge of ForgeEssentials?

Zethalion commented 5 years ago

@mezz Assuming you have a fresh setup without any configuration done to FE. Install FE on server Install JEI on server Install JEI on client Connect to server Ingame: /p group members create /p group members deny command.give /p group members allow fe.perm.debug (makes sure you can use /p debug command) /p user mezz group set members (assuming your name in MC is mezz) /gamemode survival (to be sure) /deop mezz /p debug

Now you should be able to ctrl-click the wrench icon to enable cheat mode and use it to give yourself anything. /p debug will output any permission node accessed.

Next, remove JEI from server, still being de-opped and member of the group members reconnect to the server. Execute ingame: /p debug

Cheat mode can be enabled but this time the JEI won't be able to use /give as shown by /p debug and JEI.

mezz commented 5 years ago

Thanks for the clear instructions.

Here is what I see on the server:

[23:24:49] [Server thread/INFO] [minecraft/DedicatedServer]: Created group members
[23:24:54] [Server thread/INFO] [minecraft/DedicatedServer]: Denied members access to commands.give in zone _SERVER_
[23:25:05] [Server thread/INFO] [minecraft/DedicatedServer]: Allowed members access to fe.perm.debug in zone _SERVER_
[23:25:10] [Server thread/INFO] [minecraft/DedicatedServer]: Set mezz's group(s) to members
[23:25:20] [Server thread/INFO] [minecraft/DedicatedServer]: De-opped mezz

Then on the client when I have JEI on both client and server, and I ctrl-click the wrench icon: 2019-01-09_23 28 23

Zethalion commented 5 years ago

No matter what I do. I can't replicate your screenshot. Used files: forge-1.12.2-14.23.5.2772-universal.jar jei_1.12.2-4.14.3.247.jar forgeessentials-1.12.2-12.3.30-server.jar

image image

BSKCorp commented 5 years ago

I am curious where you got 12.3.30 of Forge Essentials. I am working on reproducing. but only version I can find to download is 12.3.27 on Curse Forge.

BSKCorp commented 5 years ago

Verified - No longer reproducible with - Forge Essentials - 1.12.2-12.2.27 JEI - 1.12.2 - 4.14.3.248 Forge - 14.23.5.2796

I logged in, made sure I was deopped. In the basic rank. No go. https://i.gyazo.com/fbc087456454d186ddc9fe21dea2f6c3.png https://i.gyazo.com/f05f93a329586583fa5d05d955affde1.png

ovingiv commented 5 years ago

If I'm on a local world and try to CTRL+Click the wrench in JEI, it gives me the error "I do not have the permission to use JEI's Cheat Mode" 2019-01-10_13 58 34

If I'm on a dedicated server, I'm able to give myself anything following the steps Zethalion has posted above. 2019-01-10_14 01 16

forge-14.23.5.2806 jei_1.12.2-4.14.3.246 forgeessentials-1.12.2-12.3.30-client forgeessentials-1.12.2-12.3.30-server

Zethalion commented 5 years ago

The official location of releases is on Curse. But when issues arise you must use the latest build from Jenkins: https://github.com/ForgeEssentials/ForgeEssentials#reporting-bugs

mezz commented 5 years ago

I just grabbed the latest version, so I am on a dedicated server with forgeessentials-1.12.2-12.3.30-server jei_1.12.2-4.14.3.248

I deleted the world, configs, and the ForgeEssentials folder before starting the server.

Following the instructions from Zethalion on the client as exactly as I could, I was still unable to reproduce the issue in my dev environment: 2019-01-10_23 38 42

BSKCorp commented 5 years ago

@mezz Do you have JEI installed on the Server side as well? As I didn't and it wouldnt let me take anything. But @ovingiv you have JEI installed on the server side and it is bypassing the perms and allowing the give command. Try without JEI installed Sever side. JEI is really a client side mod anyways and I have seen seen a reason to have it on the Server.

ovingiv commented 5 years ago

I can't check now but I can come back with my findings if that's able to change the outcome. Iirc jei is a dependicy for something I have in my pack server side but I don't remember.

EDIT: Yes removing JEI and NEI from the server side does fix this issue.

mezz commented 5 years ago

I have JEI installed on both the client and the server.

If JEI is not on the server, it is acting like a "hacked" client that is requesting a /give. Security checks here are the responsibility of the server (ForgeEssentials in this case).

ovingiv commented 5 years ago

Unfortunately, removing NEI and JEI from the server now found out breaks parts of NEI that allows OP to save inventories, enable magnet mode and change game mode. With JEI it breaks the ability to move the crafting recipe to the crafting bench. That is what has been found so far.

spacebuilder2020 commented 5 years ago

I did some testing on my own. It appears that if fe.economy.cmdprice.give is set, permissions will prevent jei from spawning items when the user does not have enough money but it will work when it is not set but give is denied.

spacebuilder2020 commented 5 years ago

@mezz It appears that since the economy module listens to the command event and then blocks a command based on a criteria, https://github.com/ForgeEssentials/ForgeEssentials/blob/1.12.2/develop/src/main/java/com/forgeessentials/economy/ModuleEconomy.java#L299 when https://github.com/mezz/JustEnoughItems/blob/e5ec552114189aa814940c18624b12c4b8b8e6ab/src/main/java/mezz/jei/util/CommandUtilServer.java#L80 is called, it blocks the command.

Zethalion commented 5 years ago

For a work-around you will need to leave the economy module enabled. Enter this ingame and you can have JEI both on the server and client: /p group _ALL_ value fe.economy.cmdprice.give 999999999999999999

This is the max amount that is accepted, don't go higher as it will be ignored.

This issue will be kept open as this is not the perfect solution.

Zethalion commented 5 years ago

In addition for ops you might want to add this line to be able to use cheat mode in survival: /p group _OPS_ value fe.economy.cmdprice.give 0

spacebuilder2020 commented 5 years ago

Based on my testing, it appears that forge essentials is working as intended. However, since JEI is not using the permission api but instead using the ICommand.checkPermission function it leads to unexpected results. By default (in most cases) the check permission command only checks to see if the user is op on a (forge) vanilla client. In addition, since this is a overridable method of ICommand, this can be changed by downstream command implementations. This breaks any kind of permission manager as the expected behavior is to be able to block any command even if it is normally usable by any player.

Really, all the checkPermission command does is checks that the user could be able to run a command if conditions are right. In forge essentials, a sender (Sender.canUseCommand) is able to use any command by default but this is then overriden by FE zones.

As a result, since CommandBase.checkPermission is a wrapper for Sender.canUseCommand, it will always return true in forge essentials.

mezz commented 5 years ago

How should I use the permission api to check if the player has permission to use the give command? What are the steps to reproduce this bug?

spacebuilder2020 commented 5 years ago

public static boolean hasPermission(GameProfile profile, String node, @Nullable IContext context); or public static boolean hasPermission(EntityPlayer player, String node);

spacebuilder2020 commented 5 years ago

An example of how forge essentials uses the api is here .

spacebuilder2020 commented 5 years ago

Also, a check that could be useful is a jei specific node before 71 to allow users to disable jei cheat mode without disabling /give or creative mode access. Something like jei.cheats, I guess.