diff --git a/src/main/java/client/Client.java b/src/main/java/client/Client.java index 3eb77a3eba..4b69c52bc6 100644 --- a/src/main/java/client/Client.java +++ b/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 { diff --git a/src/main/java/database/JdbiConfig.java b/src/main/java/database/JdbiConfig.java index 06804a9273..2482d9707b 100644 --- a/src/main/java/database/JdbiConfig.java +++ b/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() ); } diff --git a/src/main/java/database/ban/MacBan.java b/src/main/java/database/ban/MacBan.java new file mode 100644 index 0000000000..8d046e1dad --- /dev/null +++ b/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); + } +} diff --git a/src/main/java/database/ban/MacBanRepository.java b/src/main/java/database/ban/MacBanRepository.java new file mode 100644 index 0000000000..0235e6e9e1 --- /dev/null +++ b/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 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; + } + } +} diff --git a/src/main/java/database/ban/MacBanRowMapper.java b/src/main/java/database/ban/MacBanRowMapper.java new file mode 100644 index 0000000000..c36c06757c --- /dev/null +++ b/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 { + + @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(); + } +} diff --git a/src/main/java/net/ChannelDependencies.java b/src/main/java/net/ChannelDependencies.java index 36a9589d65..15d5bef00f 100644 --- a/src/main/java/net/ChannelDependencies.java +++ b/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, - BanService banService + TransitionService transitionService, IpBanManager ipBanManager, MacBanManager macBanManager, + 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); } diff --git a/src/main/java/net/server/Server.java b/src/main/java/net/server/Server.java index 68d3cb52ad..ffc82c6597 100644 --- a/src/main/java/net/server/Server.java +++ b/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(); diff --git a/src/main/java/net/server/handlers/login/CharSelectedHandler.java b/src/main/java/net/server/handlers/login/CharSelectedHandler.java index 969dbd77fb..a7f9a7d3dc 100644 --- a/src/main/java/net/server/handlers/login/CharSelectedHandler.java +++ b/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; } diff --git a/src/main/java/net/server/handlers/login/CharSelectedWithPicHandler.java b/src/main/java/net/server/handlers/login/CharSelectedWithPicHandler.java index 6db9738820..952a577e2b 100644 --- a/src/main/java/net/server/handlers/login/CharSelectedWithPicHandler.java +++ b/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; } diff --git a/src/main/java/net/server/handlers/login/LoginPasswordHandler.java b/src/main/java/net/server/handlers/login/LoginPasswordHandler.java index 63dbfadd46..9181be0174 100644 --- a/src/main/java/net/server/handlers/login/LoginPasswordHandler.java +++ b/src/main/java/net/server/handlers/login/LoginPasswordHandler.java @@ -113,8 +113,7 @@ public final class LoginPasswordHandler implements PacketHandler { return; } - boolean banCheckDisabled = false; - if (!banCheckDisabled && (banService.isBanned(c) || c.hasBannedMac())) { + if (banService.isBanned(c)) { c.sendPacket(PacketCreator.getLoginFailed(3)); return; } diff --git a/src/main/java/net/server/handlers/login/RegisterPicHandler.java b/src/main/java/net/server/handlers/login/RegisterPicHandler.java index 0209369e76..270f587324 100644 --- a/src/main/java/net/server/handlers/login/RegisterPicHandler.java +++ b/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; } diff --git a/src/main/java/net/server/handlers/login/ViewAllCharRegisterPicHandler.java b/src/main/java/net/server/handlers/login/ViewAllCharRegisterPicHandler.java index 72755cdc18..e713eb98e6 100644 --- a/src/main/java/net/server/handlers/login/ViewAllCharRegisterPicHandler.java +++ b/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; } diff --git a/src/main/java/net/server/handlers/login/ViewAllCharSelectedHandler.java b/src/main/java/net/server/handlers/login/ViewAllCharSelectedHandler.java index 1681878960..30c07f8451 100644 --- a/src/main/java/net/server/handlers/login/ViewAllCharSelectedHandler.java +++ b/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; } diff --git a/src/main/java/net/server/handlers/login/ViewAllCharSelectedWithPicHandler.java b/src/main/java/net/server/handlers/login/ViewAllCharSelectedWithPicHandler.java index 3a42eb572e..2ea0391aca 100644 --- a/src/main/java/net/server/handlers/login/ViewAllCharSelectedWithPicHandler.java +++ b/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; } diff --git a/src/main/java/server/ban/MacBanManager.java b/src/main/java/server/ban/MacBanManager.java new file mode 100644 index 0000000000..091666d64e --- /dev/null +++ b/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 bannedMacs = new HashSet<>(); + + public MacBanManager(MacBanRepository macBanRepository) { + this.macBanRepository = macBanRepository; + } + + public synchronized void loadMacBans() { + List 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); + } +} diff --git a/src/main/java/service/BanService.java b/src/main/java/service/BanService.java index 6b7972ea19..24c96bb806 100644 --- a/src/main/java/service/BanService.java +++ b/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 macs = c.getMacs(); + return macs.stream().anyMatch(macBanManager::isBanned); + } } diff --git a/src/main/resources/db/migration/postgresql/V0.10__ban.sql b/src/main/resources/db/migration/postgresql/V0.10__ban.sql index e9ab53e2c7..51a4e9914c 100644 --- a/src/main/resources/db/migration/postgresql/V0.10__ban.sql +++ b/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}; diff --git a/src/test/java/server/ban/MacBanManagerTest.java b/src/test/java/server/ban/MacBanManagerTest.java new file mode 100644 index 0000000000..81f28ef3ba --- /dev/null +++ b/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 macBans = macBanRepository.getAllMacBans(); + assertEquals(1, macBans.size()); + assertEquals(new MacBan(mac, 10733), macBans.getFirst()); + } +}