Redo mac bans - reduce amount of db queries on login

Works just like ip and hwid bans in that they are loaded on startup

src/main/java/client/Client.java

@@ -315,42 +315,6 @@ public class Client extends ChannelInboundHandlerAdapter {
         return inServerTransition;
     }
 
-    // TODO: load macbans on server start and query it on demand. This query should not be run on every login!
-    @Deprecated
-    public boolean hasBannedMac() {
-        if (macs.isEmpty()) {
-            return false;
-        }
-        boolean ret = false;
-        int i;
-        StringBuilder sql = new StringBuilder("SELECT COUNT(*) FROM macbans WHERE mac IN (");
-        for (i = 0; i < macs.size(); i++) {
-            sql.append("?");
-            if (i != macs.size() - 1) {
-                sql.append(", ");
-            }
-        }
-        sql.append(")");
-
-        try (Connection con = DatabaseConnection.getConnection();
-             PreparedStatement ps = con.prepareStatement(sql.toString())) {
-            i = 0;
-            for (String mac : macs) {
-                ps.setString(++i, mac);
-            }
-            try (ResultSet rs = ps.executeQuery()) {
-                rs.next();
-                if (rs.getInt(1) > 0) {
-                    ret = true;
-                }
-            }
-        } catch (Exception e) {
-            e.printStackTrace();
-        }
-
-        return ret;
-    }
-
     // TODO: Recode to close statements...
     // Only used from ban command.
     private void loadMacsIfNescessary() throws SQLException {

src/main/java/database/JdbiConfig.java

@@ -3,6 +3,7 @@ package database;
 import database.account.AccountRowMapper;
 import database.ban.HwidBanRowMapper;
 import database.ban.IpBanRowMapper;
+import database.ban.MacBanRowMapper;
 import database.drop.GlobalMonsterDropRowMapper;
 import database.drop.MonsterDropRowMapper;
 import database.maker.MakerIngredientRowMapper;
@@ -40,6 +41,7 @@ public final class JdbiConfig {
                 new ShopItemRowMapper(),
                 new MonsterCardRowMapper(),
                 new IpBanRowMapper(),
+                new MacBanRowMapper(),
                 new HwidBanRowMapper()
         );
     }

src/main/java/database/ban/MacBan.java

@@ -0,0 +1,12 @@
+package database.ban;
+
+import lombok.Builder;
+
+import java.util.Objects;
+
+@Builder
+public record MacBan(String mac, Integer accountId) {
+    public MacBan {
+        Objects.requireNonNull(mac);
+    }
+}

src/main/java/database/ban/MacBanRepository.java

@@ -0,0 +1,45 @@
+package database.ban;
+
+import database.PgDatabaseConnection;
+import lombok.extern.slf4j.Slf4j;
+import org.jdbi.v3.core.Handle;
+
+import java.util.List;
+
+/**
+ * @author Ponk
+ */
+@Slf4j
+public class MacBanRepository {
+    private final PgDatabaseConnection connection;
+
+    public MacBanRepository(PgDatabaseConnection connection) {
+        this.connection = connection;
+    }
+
+    public List<MacBan> getAllMacBans() {
+        String sql = """
+                SELECT mac, account_id
+                FROM mac_ban""";
+        try (Handle handle = connection.getHandle()) {
+            return handle.createQuery(sql)
+                    .mapTo(MacBan.class)
+                    .list();
+        }
+    }
+
+    public boolean saveMacBan(int accountId, String mac) {
+        String sql = """
+                INSERT INTO mac_ban (account_id, mac)
+                VALUES (:accountId, :mac)""";
+        try (Handle handle = connection.getHandle()) {
+            return handle.createUpdate(sql)
+                    .bind("accountId", accountId)
+                    .bind("mac", mac)
+                    .execute() > 0;
+        } catch (Exception e) {
+            log.error("Failed to save mac ban. The mac is already banned? accountId: {}, mac: {}", accountId, mac, e);
+            return false;
+        }
+    }
+}

src/main/java/database/ban/MacBanRowMapper.java

@@ -0,0 +1,18 @@
+package database.ban;
+
+import org.jdbi.v3.core.mapper.RowMapper;
+import org.jdbi.v3.core.statement.StatementContext;
+
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+public class MacBanRowMapper implements RowMapper<MacBan> {
+
+    @Override
+    public MacBan map(ResultSet rs, StatementContext ctx) throws SQLException {
+        return MacBan.builder()
+                .mac(rs.getString("mac"))
+                .accountId(rs.getObject("account_id", Integer.class))
+                .build();
+    }
+}

src/main/java/net/ChannelDependencies.java

@@ -10,6 +10,7 @@ import database.drop.DropProvider;
 import lombok.Builder;
 import server.ban.HwidBanManager;
 import server.ban.IpBanManager;
+import server.ban.MacBanManager;
 import server.shop.ShopFactory;
 import service.AccountService;
 import service.BanService;
@@ -27,8 +28,8 @@ public record ChannelDependencies(
         CharacterCreator characterCreator, CharacterLoader characterLoader, CharacterSaver characterSaver,
         NoteService noteService, FredrickProcessor fredrickProcessor, MakerProcessor makerProcessor,
         DropProvider dropProvider, CommandsExecutor commandsExecutor, ShopFactory shopFactory,
-        TransitionService transitionService, IpBanManager ipBanManager, HwidBanManager hwidBanManager,
+        TransitionService transitionService, IpBanManager ipBanManager, MacBanManager macBanManager,
-        BanService banService
+        HwidBanManager hwidBanManager,BanService banService
 ) {
 
     public ChannelDependencies {
@@ -44,6 +45,7 @@ public record ChannelDependencies(
         Objects.requireNonNull(shopFactory);
         Objects.requireNonNull(transitionService);
         Objects.requireNonNull(ipBanManager);
+        Objects.requireNonNull(macBanManager);
         Objects.requireNonNull(hwidBanManager);
         Objects.requireNonNull(banService);
     }

src/main/java/net/server/Server.java

@@ -47,6 +47,7 @@ import database.PgDatabaseConnection;
 import database.account.AccountRepository;
 import database.ban.HwidBanRepository;
 import database.ban.IpBanRepository;
+import database.ban.MacBanRepository;
 import database.character.CharacterLoader;
 import database.character.CharacterRepository;
 import database.character.CharacterSaver;
@@ -89,6 +90,7 @@ import server.ThreadManager;
 import server.TimerManager;
 import server.ban.HwidBanManager;
 import server.ban.IpBanManager;
+import server.ban.MacBanManager;
 import server.expeditions.ExpeditionBossLog;
 import server.life.PlayerNPC;
 import server.quest.Quest;
@@ -721,6 +723,7 @@ public class Server {
         futures.add(initExecutor.submit(Quest::loadAllQuests));
         futures.add(initExecutor.submit(SkillbookInformationProvider::loadAllSkillbookInformation));
         futures.add(initExecutor.submit(channelDependencies.ipBanManager()::loadIpBans));
+        futures.add(initExecutor.submit(channelDependencies.macBanManager()::loadMacBans));
         futures.add(initExecutor.submit(channelDependencies.hwidBanManager()::loadHwidBans));
         initExecutor.shutdown();
 
@@ -836,8 +839,10 @@ public class Server {
         DropProvider dropProvider = new DropProvider(new DropRepository(connection));
         ShopFactory shopFactory = new ShopFactory(new ShopDao(connection));
         IpBanManager ipBanManager = new IpBanManager(new IpBanRepository(connection));
+        MacBanManager macBanManager = new MacBanManager(new MacBanRepository(connection));
         HwidBanManager hwidBanManager = new HwidBanManager(new HwidBanRepository(connection));
-        BanService banService = new BanService(accountService, transitionService, ipBanManager, hwidBanManager);
+        BanService banService = new BanService(accountService, transitionService, ipBanManager, macBanManager,
+                hwidBanManager);
         ChannelDependencies channelDependencies = ChannelDependencies.builder()
                 .accountService(accountService)
                 .characterCreator(new CharacterCreator(connection, characterRepository))
@@ -852,6 +857,7 @@ public class Server {
                 .shopFactory(shopFactory)
                 .transitionService(transitionService)
                 .ipBanManager(ipBanManager)
+                .macBanManager(macBanManager)
                 .hwidBanManager(hwidBanManager)
                 .banService(banService)
                 .build();

src/main/java/net/server/handlers/login/CharSelectedHandler.java

@@ -85,7 +85,7 @@ public final class CharSelectedHandler extends AbstractPacketHandler {
             return;
         }
 
-        if (banService.isBanned(c) || c.hasBannedMac()) {
+        if (banService.isBanned(c)) {
             SessionCoordinator.getInstance().closeSession(c, true);
             return;
         }

src/main/java/net/server/handlers/login/CharSelectedWithPicHandler.java

@@ -59,7 +59,7 @@ public class CharSelectedWithPicHandler extends AbstractPacketHandler {
         c.updateHwid(hwid);
         c.setHwid(hwid);
 
-        if (banService.isBanned(c) || c.hasBannedMac()) {
+        if (banService.isBanned(c)) {
             SessionCoordinator.getInstance().closeSession(c, true);
             return;
         }

src/main/java/net/server/handlers/login/LoginPasswordHandler.java

@@ -113,8 +113,7 @@ public final class LoginPasswordHandler implements PacketHandler {
             return;
         }
 
-        boolean banCheckDisabled = false;
+        if (banService.isBanned(c)) {
-        if (!banCheckDisabled && (banService.isBanned(c) || c.hasBannedMac())) {
             c.sendPacket(PacketCreator.getLoginFailed(3));
             return;
         }

src/main/java/net/server/handlers/login/RegisterPicHandler.java

@@ -58,7 +58,7 @@ public final class RegisterPicHandler extends AbstractPacketHandler {
             return;
         }
 
-        if (banService.isBanned(c) || c.hasBannedMac()) {
+        if (banService.isBanned(c)) {
             SessionCoordinator.getInstance().closeSession(c, true);
             return;
         }

src/main/java/net/server/handlers/login/ViewAllCharRegisterPicHandler.java

@@ -55,7 +55,7 @@ public final class ViewAllCharRegisterPicHandler extends AbstractPacketHandler {
         c.updateHwid(hwid);
         c.setHwid(hwid);
 
-        if (banService.isBanned(c) || c.hasBannedMac()) {
+        if (banService.isBanned(c)) {
             SessionCoordinator.getInstance().closeSession(c, true);
             return;
         }

src/main/java/net/server/handlers/login/ViewAllCharSelectedHandler.java

@@ -81,7 +81,7 @@ public final class ViewAllCharSelectedHandler extends AbstractPacketHandler {
         c.updateHwid(hwid);
         c.setHwid(hwid);
 
-        if (banService.isBanned(c) || c.hasBannedMac()) {
+        if (banService.isBanned(c)) {
             SessionCoordinator.getInstance().closeSession(c, true);
             return;
         }

src/main/java/net/server/handlers/login/ViewAllCharSelectedWithPicHandler.java

@@ -62,7 +62,7 @@ public class ViewAllCharSelectedWithPicHandler extends AbstractPacketHandler {
         c.updateHwid(hwid);
         c.setHwid(hwid);
 
-        if (banService.isBanned(c) || c.hasBannedMac()) {
+        if (banService.isBanned(c)) {
             SessionCoordinator.getInstance().closeSession(c, true);
             return;
         }

src/main/java/server/ban/MacBanManager.java

@@ -0,0 +1,44 @@
+package server.ban;
+
+import database.ban.MacBan;
+import database.ban.MacBanRepository;
+import lombok.extern.slf4j.Slf4j;
+import net.jcip.annotations.ThreadSafe;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * @author Ponk
+ */
+@ThreadSafe
+@Slf4j
+public class MacBanManager {
+    private final MacBanRepository macBanRepository;
+    private final Set<String> bannedMacs = new HashSet<>();
+
+    public MacBanManager(MacBanRepository macBanRepository) {
+        this.macBanRepository = macBanRepository;
+    }
+
+    public synchronized void loadMacBans() {
+        List<MacBan> macBans = macBanRepository.getAllMacBans();
+        log.debug("Loaded {} mac bans", macBans.size());
+        bannedMacs.addAll(macBans.stream().map(MacBan::mac).toList());
+    }
+
+    public synchronized boolean isBanned(String mac) {
+        return bannedMacs.contains(mac);
+    }
+
+    public synchronized void banMac(String mac, int accountId) {
+        if (mac == null) {
+            throw new IllegalArgumentException("mac cannot be null");
+        }
+        // TODO: validate mac format. Or create "Mac" model class.
+
+        bannedMacs.add(mac);
+        macBanRepository.saveMacBan(accountId, mac);
+    }
+}

src/main/java/service/BanService.java

@@ -11,11 +11,13 @@ import net.server.coordinator.session.Hwid;
 import server.TimerManager;
 import server.ban.HwidBanManager;
 import server.ban.IpBanManager;
+import server.ban.MacBanManager;
 import tools.PacketCreator;
 
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 @Slf4j
@@ -23,13 +25,15 @@ public class BanService {
     private final AccountService accountService;
     private final TransitionService transitionService;
     private final IpBanManager ipBanManager;
+    private final MacBanManager macBanManager;
     private final HwidBanManager hwidBanManager;
 
     public BanService(AccountService accountService, TransitionService transitionService, IpBanManager ipBanManager,
-                      HwidBanManager hwidBanManager) {
+                      MacBanManager macBanManager, HwidBanManager hwidBanManager) {
         this.accountService = accountService;
         this.transitionService = transitionService;
         this.ipBanManager = ipBanManager;
+        this.macBanManager = macBanManager;
         this.hwidBanManager = hwidBanManager;
     }
 
@@ -121,7 +125,7 @@ public class BanService {
     }
 
     public boolean isBanned(Client c) {
-        return isIpBanned(c) || isHwidBanned(c);
+        return isIpBanned(c) || isHwidBanned(c) || isMacBanned(c);
     }
 
     private boolean isIpBanned(Client c) {
@@ -133,4 +137,9 @@ public class BanService {
         Hwid hwid = c.getHwid();
         return hwid != null && hwidBanManager.isBanned(hwid);
     }
+
+    private boolean isMacBanned(Client c) {
+        Set<String> macs = c.getMacs();
+        return macs.stream().anyMatch(macBanManager::isBanned);
+    }
 }

src/main/resources/db/migration/postgresql/V0.10__ban.sql

@@ -15,3 +15,12 @@ CREATE TABLE hwid_ban
     PRIMARY KEY (hwid)
 );
 GRANT SELECT ON TABLE hwid_ban TO ${server-username};
+
+CREATE TABLE mac_ban
+(
+    mac varchar(30) NOT NULL,
+account_id integer,
+    created_at timestamp DEFAULT now() NOT NULL,
+    PRIMARY KEY (mac)
+);
+GRANT SELECT, INSERT ON TABLE mac_ban TO ${server-username};

src/test/java/server/ban/MacBanManagerTest.java

@@ -0,0 +1,58 @@
+package server.ban;
+
+import database.DatabaseTest;
+import database.ban.MacBan;
+import database.ban.MacBanRepository;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import testutil.AnyValues;
+
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class MacBanManagerTest extends DatabaseTest {
+    private MacBanRepository macBanRepository;
+    private MacBanManager macBanManager;
+
+    @BeforeEach
+    void setUp() {
+        this.macBanRepository = new MacBanRepository(connection);
+        this.macBanManager = new MacBanManager(macBanRepository);
+    }
+
+    @AfterEach
+    void deleteMacBans() {
+        clearTable("mac_ban");
+    }
+
+    @Test
+    void loadMacBans_shouldLoadFromRepository() {
+        String mac = "4A-16-A2-9C-B0-6D";
+        assertFalse(macBanManager.isBanned(mac));
+
+        macBanManager.loadMacBans();
+        assertFalse(macBanManager.isBanned(mac));
+
+        macBanRepository.saveMacBan(AnyValues.integer(), mac);
+        macBanManager.loadMacBans();
+
+        assertTrue(macBanManager.isBanned(mac));
+    }
+
+    @Test
+    void banIp_shouldSaveInRepository() {
+        String mac = "1F-45-B0-FB-2E-DF";
+        assertFalse(macBanManager.isBanned(mac));
+
+        macBanManager.banMac(mac, 10733);
+
+        assertTrue(macBanManager.isBanned(mac));
+        List<MacBan> macBans = macBanRepository.getAllMacBans();
+        assertEquals(1, macBans.size());
+        assertEquals(new MacBan(mac, 10733), macBans.getFirst());
+    }
+}