Skip to content

Ban method? Customizable? #2

Open
Open
Ban method? Customizable?#2
Assignees
AitorAstorga
Labels
enhancementNew feature or request

Description

@ScrapMetalGolem
ScrapMetalGolem
opened on Feb 25, 2024

Summary

I don't know the current method the plugin uses to ban the player because we haven't gotten to ban any users with it yet (I assume vanilla ban). But I would like to use banmanager and set a custom reason/message to redirect any legit user to get unbanned.

Basic example

banmanager:ban username reason (like "Suspicious Activity, visit [discord link] to appeal)

Motivation

I expect that if any legit new player gets caught in a false positive, this will give them an opportunity to 1) let me know about it and 2) get unbanned so they can play.

Activity

  1. ScrapMetalGolem
    added
    enhancementNew feature or request
    on Feb 25, 2024

  2. AitorAstorga commented on Feb 26, 2024

    @AitorAstorga
    AitorAstorga
    on Feb 26, 2024
    Collaborator

    Hi, I'm working on it. The plugin currently uses the Bukkit.getBanList(Type.NAME).addBan method. In the Fabric version I do something similar with getUserBanList().add(new BannedPlayerEntry. For your points:

    1. There is a console log that reads "Player was banned for disconnecting within seconds of joining for the first time - suspected bot.", so you will get at least this info when an account is banned.
    2. I will first modify the plugin to allow setting a custom ban text. If this works correctly I will start working for integration with banmanager, I have never used it so I'll have to find about it.

    My idea is to add new commands:

    • /BotBlocker status - Show wether BotBlocker is enabled or disabled.
    • /BotBlocker getTimeLimit - Display the configured time limit for detecting bots.
    • /BotBlocker setBanMessage [message] - Set the ban message.
    • /BotBlocker getBanMessage - Display the configured ban message.

  3. ScrapMetalGolem commented on Feb 26, 2024

    @ScrapMetalGolem
    ScrapMetalGolem
    on Feb 26, 2024
    Author

    That's good news, thanks for maintaining this tool.

  4. AitorAstorga
    self-assigned this
    on Feb 26, 2024

  5. AitorAstorga commented on Feb 28, 2024

    @AitorAstorga
    AitorAstorga
    on Feb 28, 2024
    Collaborator

    Please check BotBlocker-1.4.0-SNAPSHOT. Also check the Changelog.

    I have added a check so if the BanManager plugin is in BotBlocker will use its API to ban instead of the default Bukkit method. I tested it in my server and could see the banned user in the list correctly and unban it afterwards. If it works for you then I will set it as latest release.

  6. ScrapMetalGolem commented on Feb 29, 2024

    @ScrapMetalGolem
    ScrapMetalGolem
    on Feb 29, 2024
    Author

    First attempt with the 1.4.0 version appeared to not work. I followed these steps:

    • removed the UUID:true line of the account I wanted to test with from the players.yml file
    • checked that there are no entries in my banned-players.json file
    • restarted the server with v 1.4.0 plugin
    • checked settings (time, ban message) on my main account
    • joined with alt account and left after a few seconds
    • no information was added to the players.yml file, and BanManager was not triggered
    • there was no associated error message

    However, I decided to try again. I replaced 1.4.0 with 1.3.0 and tried the same method. The account was banned and everything worked as expected for that version. Then I:

    • removed the banned player's record from banned-players.json
    • changed plugin version to 1.4.0
    • restarted the server
    • joined with the suspect account, left quickly
    • the account was banned by BanManager successfully!

    I'm able to correctly unban the account and rejoin the server with it. The only thing I don't fully understand is that the account UUID is not added to the players.yml file anymore, even after playing for a while and leaving the server. The account does seem immune to the plugin now though, so I have to assume that data is being stored elsewhere now (in the banmanager database?)

    I will test this further when I can get some people with untainted accounts to join and test for me, maybe I'll get a different result when an account that never joined before tries it.

  7. AitorAstorga commented on Feb 29, 2024

    @AitorAstorga
    AitorAstorga
    on Feb 29, 2024
    Collaborator

    Wow, that's some good effort, thank you!

    TL;DR: Weird errors are weird. Let's test more. I propose renaming players.yml to whitelist.yml and improving logs.

    BanManager seems to be using its own database, probably in local_bans.mv.db, while the vanilla ban adds an entry into banned-players.json. Not that I really cared too much about that before, as I just use the BanManager API methods and they handle that, but this is definitely an issue for compatibility between Vanilla bans and BanManager bans. That is, as previous BotBlocker used vanilla, those bans will remain in banned-players.json.

    1.3.0 -> 1.4.0 update error

    Your issue when installing the new version confuses me a bit. In the first try you cleaned both players.yml and banned-players.json. Good call. But it didn't work. And then after rolling back to 1.3.0 and getting a ban (so players.yml should be clean) and also cleaning banned-players.json again (so, we should be in the same state as in the beginning), this time it worked! Also strange that there were no errors popping up. Did you see anything in the log file?

    Account not added to players.yml

    The account not being added to players.yml is unexpected too. Even more if it is working regardless. Some weirdness must be going on over here:

    if (timeConnected < timeLimit) {
    String playerName = event.getPlayer().getName();

    // Check if BanManager is installed and enabled
    Plugin banManager = Bukkit.getServer().getPluginManager().getPlugin("BanManager");
    if (banManager != null && banManager.isEnabled()) {
        // Use BanManager to ban the player
        banWithBanManager(event.getPlayer(), banMessage);
    } else {
        // Fallback to Bukkit's default banning system
        Bukkit.getBanList(Type.PROFILE).addBan(playerName, banMessage, null, "BotBlocker");
    }

    getLogger().info("Player '" + playerName + "' was banned for disconnecting within " + timeLimit + " seconds of joining for the first time - suspected bot.");
} else {
    // Add the player to players.yml if it is not banned
    playersCfg.set(playerId.toString(), true);
    savePlayers();
}

    What the code above does is simply check if a user must be banned and ban him (either vanilla or BanManager). And if the user is legit, it is added to the players.yml file. I could understand users not getting added here, but the fact that even with that the immunity is working is even more concerning. My guess would be that it is not working at all, but you mentioned that it correctly bans an alt account, so...

    I will try to do the same from my side, testing with different accounts. Hopefully I have time for that this weekend.

    Regardless, I have some ideas:

    1. I notice that users are added to the players.yml only as a whitelist if they are legit users, but supposed bots are not, they just get banned. Maybe it would make sense to:
      • a) Rename the file to whitelist.yml for clarity. If players.yml is found, rename this file.
      • b) Improve the logging to include messages such as "User x banned." or "User y whitelisted". Also a distinction if using BanManager or vanilla ban.
    2. I should stop using true blankly in the players file. I just did because the method for it is convenient, but I see that it may lead to confusion. Perhaps I should only set it to true but accept that an admin may change it to false without removing the line.
    3. Some config is loaded at the startup. I should either clarify that to avoid problems or make it reload it more often (periodically, after any command issued...).

    And of course, if you've read this far, I accept suggestions :)

  8. ScrapMetalGolem commented on Feb 29, 2024

    @ScrapMetalGolem
    ScrapMetalGolem
    on Feb 29, 2024
    Author

    Further testing provides these clues:

    • If I delete the players.yml file and restart the server, then both whitelisting and banning work as expected, except that I now receive this error from banmanager (whether or not I deleted the file, it happens each time the BotBlocker bans a player now).
    [14:53:12 WARN]: java.lang.NullPointerException: Cannot invoke "me.confuser.banmanager.common.data.PlayerData.getUUID()" because "this.actor" is null
[14:53:12 WARN]:        at BanManagerBukkit.jar//me.confuser.banmanager.common.data.PlayerBanData.equalsBan(PlayerBanData.java:83)
[14:53:12 WARN]:        at BanManagerBukkit.jar//me.confuser.banmanager.common.runnables.BanSync.newBans(BanSync.java:42)
[14:53:12 WARN]:        at BanManagerBukkit.jar//me.confuser.banmanager.common.runnables.BanSync.run(BanSync.java:26)
[14:53:12 WARN]:        at BanManagerBukkit.jar//me.confuser.banmanager.common.runnables.Runner.run(Runner.java:27)
[14:53:12 WARN]:        at org.bukkit.craftbukkit.v1_20_R3.scheduler.CraftTask.run(CraftTask.java:101)
[14:53:12 WARN]:        at org.bukkit.craftbukkit.v1_20_R3.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:57)
[14:53:12 WARN]:        at com.destroystokyo.paper.ServerSchedulerReportingWrapper.run(ServerSchedulerReportingWrapper.java:22)
[14:53:12 WARN]:        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
[14:53:12 WARN]:        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
[14:53:12 WARN]:        at java.base/java.lang.Thread.run(Thread.java:833)
    • If I unban that player, then for the duration of the server uptime, they're now immune to being banned by BotBlocker, and won't be added to the players.yml whitelist file.
    • If I delete the players.yml file and restart the server, then once again all operations are as expected, save the above error message.
    • I did not yet try simply reloading the plugin config to accomplish the same behavior as restarting; it's worth a shot though.

    To clarify a few things:

    • I'm using SQL for BanManager, so the localbans.mv.db file is currently unused. For the record, I don't see anything relevant being stored to those tables. I feel like whatever is causing this behavior is memory-resident.
    • That error I posted above did not happen the first time I was testing. All my console logs are sent to discord and searching brings up only the ones from my current testing session. In between that time I made only a few changes to the server config, stuff that shouldn't be related at all (some configs for discordsrv and a few admin perms in luckperms). This server isn't live for public players yet so there are only a few people using it for development.
    • I honestly don't have a lot of feature suggestions. I don't expect to get a lot of bots to ban with it, since most of those bots are cracked anyway and can't even join us anymore (I previously had the server cracked so we could use alts for testing but I've scrapped that method now). It just seems like a nice extra layer of protection at a lightweight cost to performance.
    • Maybe a /botblocker reload command just to reload the config.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions