diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java index d3d975b2..e12f4d38 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java @@ -2,7 +2,6 @@ package fr.xephi.authme.command.executable.authme; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.command.ExecutableCommand; -import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.data.auth.PlayerCache; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.message.MessageKey; @@ -65,16 +64,13 @@ public class ChangePasswordAdminCommand implements ExecutableCommand { * @param sender the sender initiating the password change */ private void changePassword(String nameLowercase, String password, CommandSender sender) { - PlayerAuth auth = getAuth(nameLowercase); - if (auth == null) { + if (!isNameRegistered(nameLowercase)) { commonService.send(sender, MessageKey.UNKNOWN_USER); return; } HashedPassword hashedPassword = passwordSecurity.computeHash(password, nameLowercase); - auth.setPassword(hashedPassword); - - if (dataSource.updatePassword(auth)) { + if (dataSource.updatePassword(nameLowercase, hashedPassword)) { commonService.send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS); ConsoleLogger.info(sender.getName() + " changed password of " + nameLowercase); } else { @@ -82,12 +78,7 @@ public class ChangePasswordAdminCommand implements ExecutableCommand { } } - private PlayerAuth getAuth(String nameLowercase) { - if (playerCache.isAuthenticated(nameLowercase)) { - return playerCache.getAuth(nameLowercase); - } else if (dataSource.isAuthAvailable(nameLowercase)) { - return dataSource.getAuth(nameLowercase); - } - return null; + private boolean isNameRegistered(String nameLowercase) { + return playerCache.isAuthenticated(nameLowercase) || dataSource.isAuthAvailable(nameLowercase); } } diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/debug/DataStatistics.java b/src/main/java/fr/xephi/authme/command/executable/authme/debug/DataStatistics.java index 3bf28e05..00540e8b 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/debug/DataStatistics.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/debug/DataStatistics.java @@ -54,7 +54,6 @@ class DataStatistics implements DebugSection { private void outputDatabaseStats(CommandSender sender) { sender.sendMessage("Total players in DB: " + dataSource.getAccountsRegistered()); - sender.sendMessage("Total marked as logged in in DB: " + dataSource.getLoggedPlayers().size()); if (dataSource instanceof CacheDataSource) { CacheDataSource cacheDataSource = (CacheDataSource) this.dataSource; sender.sendMessage("Cached PlayerAuth objects: " + cacheDataSource.getCachedAuths().size()); diff --git a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java index 2e304a99..a6bf6c09 100644 --- a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java @@ -11,14 +11,15 @@ import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.data.auth.PlayerCache; import fr.xephi.authme.security.crypts.HashedPassword; +import fr.xephi.authme.util.Utils; -import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.Set; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; public class CacheDataSource implements DataSource { @@ -240,7 +241,10 @@ public class CacheDataSource implements DataSource { } @Override - public List getLoggedPlayers() { - return new ArrayList<>(playerCache.getCache().values()); + public List getLoggedPlayersWithEmptyMail() { + return playerCache.getCache().values().stream() + .filter(auth -> Utils.isEmailEmpty(auth.getEmail())) + .map(PlayerAuth::getRealName) + .collect(Collectors.toList()); } } diff --git a/src/main/java/fr/xephi/authme/datasource/DataSource.java b/src/main/java/fr/xephi/authme/datasource/DataSource.java index 080998ae..9d30f933 100644 --- a/src/main/java/fr/xephi/authme/datasource/DataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/DataSource.java @@ -167,11 +167,11 @@ public interface DataSource extends Reloadable { void purgeLogged(); /** - * Return all players which are logged in. + * Return all players which are logged in and whose email is not set. * - * @return All logged in players + * @return logged in players with no email */ - List getLoggedPlayers(); + List getLoggedPlayersWithEmptyMail(); /** * Return the number of registered accounts. diff --git a/src/main/java/fr/xephi/authme/datasource/FlatFile.java b/src/main/java/fr/xephi/authme/datasource/FlatFile.java index 92830064..a59617c4 100644 --- a/src/main/java/fr/xephi/authme/datasource/FlatFile.java +++ b/src/main/java/fr/xephi/authme/datasource/FlatFile.java @@ -423,7 +423,7 @@ public class FlatFile implements DataSource { } @Override - public List getLoggedPlayers() { + public List getLoggedPlayersWithEmptyMail() { throw new UnsupportedOperationException("Flat file no longer supported"); } diff --git a/src/main/java/fr/xephi/authme/datasource/MySQL.java b/src/main/java/fr/xephi/authme/datasource/MySQL.java index 148ea26e..cffb8263 100644 --- a/src/main/java/fr/xephi/authme/datasource/MySQL.java +++ b/src/main/java/fr/xephi/authme/datasource/MySQL.java @@ -927,33 +927,20 @@ public class MySQL implements DataSource { } @Override - public List getLoggedPlayers() { - List auths = new ArrayList<>(); - String sql = "SELECT * FROM " + tableName + " WHERE " + col.IS_LOGGED + "=1;"; + public List getLoggedPlayersWithEmptyMail() { + List players = new ArrayList<>(); + String sql = "SELECT " + col.REAL_NAME + " FROM " + tableName + " WHERE " + col.IS_LOGGED + " = 1" + + " AND (" + col.EMAIL + " = 'your@email.com' OR " + col.EMAIL + " IS NULL);"; try (Connection con = getConnection(); Statement st = con.createStatement(); ResultSet rs = st.executeQuery(sql)) { while (rs.next()) { - PlayerAuth pAuth = buildAuthFromResultSet(rs); - if (hashAlgorithm == HashAlgorithm.XFBCRYPT) { - try (PreparedStatement pst = con.prepareStatement("SELECT data FROM xf_user_authenticate WHERE " + col.ID + "=?;")) { - int id = rs.getInt(col.ID); - pst.setInt(1, id); - ResultSet rs2 = pst.executeQuery(); - if (rs2.next()) { - Blob blob = rs2.getBlob("data"); - byte[] bytes = blob.getBytes(1, (int) blob.length()); - pAuth.setPassword(new HashedPassword(XfBCrypt.getHashFromBlob(bytes))); - } - rs2.close(); - } - } - auths.add(pAuth); + players.add(rs.getString(1)); } } catch (SQLException ex) { logSqlException(ex); } - return auths; + return players; } private PlayerAuth buildAuthFromResultSet(ResultSet row) throws SQLException { diff --git a/src/main/java/fr/xephi/authme/datasource/SQLite.java b/src/main/java/fr/xephi/authme/datasource/SQLite.java index 6212506b..d564224a 100644 --- a/src/main/java/fr/xephi/authme/datasource/SQLite.java +++ b/src/main/java/fr/xephi/authme/datasource/SQLite.java @@ -591,18 +591,18 @@ public class SQLite implements DataSource { } @Override - public List getLoggedPlayers() { - List auths = new ArrayList<>(); - String sql = "SELECT * FROM " + tableName + " WHERE " + col.IS_LOGGED + "=1;"; - try (PreparedStatement pst = con.prepareStatement(sql); ResultSet rs = pst.executeQuery()) { + public List getLoggedPlayersWithEmptyMail() { + List players = new ArrayList<>(); + String sql = "SELECT " + col.REAL_NAME + " FROM " + tableName + " WHERE " + col.IS_LOGGED + " = 1" + + " AND (" + col.EMAIL + " = 'your@email.com' OR " + col.EMAIL + " IS NULL);"; + try (Statement st = con.createStatement(); ResultSet rs = st.executeQuery(sql)) { while (rs.next()) { - PlayerAuth auth = buildAuthFromResultSet(rs); - auths.add(auth); + players.add(rs.getString(1)); } } catch (SQLException ex) { logSqlException(ex); } - return auths; + return players; } private PlayerAuth buildAuthFromResultSet(ResultSet row) throws SQLException { diff --git a/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java b/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java index 32bd7035..20b2abb1 100644 --- a/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java +++ b/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java @@ -3,7 +3,6 @@ package fr.xephi.authme.initialization; import ch.jalu.injector.exceptions.InjectorReflectionException; import fr.xephi.authme.AuthMe; import fr.xephi.authme.ConsoleLogger; -import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.message.Messages; @@ -18,7 +17,6 @@ import fr.xephi.authme.settings.properties.DatabaseSettings; import fr.xephi.authme.settings.properties.EmailSettings; import fr.xephi.authme.settings.properties.PluginSettings; import fr.xephi.authme.settings.properties.SecuritySettings; -import fr.xephi.authme.util.Utils; import org.apache.logging.log4j.LogManager; import org.bstats.Metrics; import org.bukkit.Bukkit; @@ -110,13 +108,10 @@ public class OnStartupTasks { bukkitService.runTaskTimerAsynchronously(new Runnable() { @Override public void run() { - for (PlayerAuth auth : dataSource.getLoggedPlayers()) { - String email = auth.getEmail(); - if (Utils.isEmailEmpty(email)) { - Player player = bukkitService.getPlayerExact(auth.getRealName()); - if (player != null) { - messages.send(player, MessageKey.ADD_EMAIL_MESSAGE); - } + for (String playerWithoutMail : dataSource.getLoggedPlayersWithEmptyMail()) { + Player player = bukkitService.getPlayerExact(playerWithoutMail); + if (player != null) { + messages.send(player, MessageKey.ADD_EMAIL_MESSAGE); } } } diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommandTest.java index a4c80228..9e957a62 100644 --- a/src/test/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommandTest.java @@ -101,14 +101,11 @@ public class ChangePasswordAdminCommandTest { CommandSender sender = mock(CommandSender.class); String player = "my_user12"; String password = "passPass"; - PlayerAuth auth = mock(PlayerAuth.class); - given(playerCache.isAuthenticated(player)).willReturn(true); - given(playerCache.getAuth(player)).willReturn(auth); HashedPassword hashedPassword = mock(HashedPassword.class); given(passwordSecurity.computeHash(password, player)).willReturn(hashedPassword); - given(dataSource.updatePassword(auth)).willReturn(true); + given(dataSource.updatePassword(player, hashedPassword)).willReturn(true); given(validationService.validatePassword(password, player)).willReturn(new ValidationResult()); // when @@ -119,8 +116,7 @@ public class ChangePasswordAdminCommandTest { verify(validationService).validatePassword(password, player); verify(service).send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS); verify(passwordSecurity).computeHash(password, player); - verify(auth).setPassword(hashedPassword); - verify(dataSource).updatePassword(auth); + verify(dataSource).updatePassword(player, hashedPassword); } @Test @@ -129,15 +125,13 @@ public class ChangePasswordAdminCommandTest { CommandSender sender = mock(CommandSender.class); String player = "my_user12"; String password = "passPass"; - PlayerAuth auth = mock(PlayerAuth.class); given(playerCache.isAuthenticated(player)).willReturn(false); given(dataSource.isAuthAvailable(player)).willReturn(true); - given(dataSource.getAuth(player)).willReturn(auth); - given(dataSource.updatePassword(auth)).willReturn(true); given(validationService.validatePassword(password, player)).willReturn(new ValidationResult()); HashedPassword hashedPassword = mock(HashedPassword.class); given(passwordSecurity.computeHash(password, player)).willReturn(hashedPassword); + given(dataSource.updatePassword(player, hashedPassword)).willReturn(true); // when command.executeCommand(sender, Arrays.asList(player, password)); @@ -147,8 +141,7 @@ public class ChangePasswordAdminCommandTest { verify(validationService).validatePassword(password, player); verify(service).send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS); verify(passwordSecurity).computeHash(password, player); - verify(auth).setPassword(hashedPassword); - verify(dataSource).updatePassword(auth); + verify(dataSource).updatePassword(player, hashedPassword); } @Test @@ -157,14 +150,12 @@ public class ChangePasswordAdminCommandTest { CommandSender sender = mock(CommandSender.class); String player = "my_user12"; String password = "passPass"; - PlayerAuth auth = mock(PlayerAuth.class); given(playerCache.isAuthenticated(player)).willReturn(true); - given(playerCache.getAuth(player)).willReturn(auth); given(validationService.validatePassword(password, player)).willReturn(new ValidationResult()); HashedPassword hashedPassword = mock(HashedPassword.class); given(passwordSecurity.computeHash(password, player)).willReturn(hashedPassword); - given(dataSource.updatePassword(auth)).willReturn(false); + given(dataSource.updatePassword(player, hashedPassword)).willReturn(false); // when command.executeCommand(sender, Arrays.asList(player, password)); @@ -174,8 +165,7 @@ public class ChangePasswordAdminCommandTest { verify(validationService).validatePassword(password, player); verify(service).send(sender, MessageKey.ERROR); verify(passwordSecurity).computeHash(password, player); - verify(auth).setPassword(hashedPassword); - verify(dataSource).updatePassword(auth); + verify(dataSource).updatePassword(player, hashedPassword); } } diff --git a/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java b/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java index ddf83bfa..64004fb8 100644 --- a/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java @@ -348,7 +348,8 @@ public abstract class AbstractDataSourceIntegrationTest { public void shouldPerformOperationsOnIsLoggedColumnSuccessfully() { DataSource dataSource = getDataSource(); // on startup no one should be marked as logged - assertThat(dataSource.getLoggedPlayers(), empty()); + assertThat(dataSource.isLogged("user"), equalTo(false)); + assertThat(dataSource.isLogged("bobby"), equalTo(false)); // Mark user as logged dataSource.setLogged("user"); @@ -361,15 +362,16 @@ public abstract class AbstractDataSourceIntegrationTest { // Set bobby logged and unlog user dataSource.setLogged("bobby"); dataSource.setUnlogged("user"); - assertThat(dataSource.getLoggedPlayers(), - contains(hasAuthBasicData("bobby", "Bobby", "your@email.com", "123.45.67.89"))); + + assertThat(dataSource.isLogged("user"), equalTo(false)); + assertThat(dataSource.isLogged("bobby"), equalTo(true)); // Set both as logged (even if Bobby already is logged) dataSource.setLogged("user"); dataSource.setLogged("bobby"); dataSource.purgeLogged(); assertThat(dataSource.isLogged("user"), equalTo(false)); - assertThat(dataSource.getLoggedPlayers(), empty()); + assertThat(dataSource.isLogged("bobby"), equalTo(false)); } @Test @@ -400,4 +402,18 @@ public abstract class AbstractDataSourceIntegrationTest { assertThat(email1.getValue(), equalTo("user@example.org")); assertThat(email2, is(DataSourceResult.unknownPlayer())); } + + @Test + public void shouldGetLoggedPlayersWithoutEmail() { + // given + DataSource dataSource = getDataSource(); + dataSource.setLogged("bobby"); + dataSource.setLogged("user"); + + // when + List loggedPlayersWithEmptyMail = dataSource.getLoggedPlayersWithEmptyMail(); + + // then + assertThat(loggedPlayersWithEmptyMail, contains("Bobby")); + } }