From f3a656892fbb31765a7a35e10b57dc0edb2ee407 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 8 Jul 2017 23:21:53 +0200 Subject: [PATCH 1/5] Open 5.4 development iteration --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 2177188d3..b872eb4cd 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ fr.xephi authme - 5.3.3-SNAPSHOT + 5.4-SNAPSHOT AuthMeReloaded The first authentication plugin for the Bukkit API! From cbc794ba209128c1024e406efb0ea751b376741a Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 23 Jul 2017 14:01:35 +0200 Subject: [PATCH 2/5] #1255 Extract forum-specific data source actions into separate extension classes --- .../fr/xephi/authme/datasource/MySQL.java | 387 ++---------------- .../datasource/converter/MySqlToSqlite.java | 9 +- .../mysqlextensions/Ipb4Extension.java | 68 +++ .../mysqlextensions/MySqlExtension.java | 62 +++ .../MySqlExtensionsFactory.java | 33 ++ .../mysqlextensions/NoOpExtension.java | 7 + .../mysqlextensions/PhpBbExtension.java | 93 +++++ .../mysqlextensions/WordpressExtension.java | 107 +++++ .../mysqlextensions/XfBcryptExtension.java | 160 ++++++++ .../initialization/DataSourceProvider.java | 9 +- .../AbstractResourceClosingTest.java | 49 +-- .../datasource/MySqlIntegrationTest.java | 8 +- .../datasource/MySqlResourceClosingTest.java | 12 +- .../datasource/SQLiteResourceClosingTest.java | 5 +- 14 files changed, 589 insertions(+), 420 deletions(-) create mode 100644 src/main/java/fr/xephi/authme/datasource/mysqlextensions/Ipb4Extension.java create mode 100644 src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtension.java create mode 100644 src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtensionsFactory.java create mode 100644 src/main/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtension.java create mode 100644 src/main/java/fr/xephi/authme/datasource/mysqlextensions/PhpBbExtension.java create mode 100644 src/main/java/fr/xephi/authme/datasource/mysqlextensions/WordpressExtension.java create mode 100644 src/main/java/fr/xephi/authme/datasource/mysqlextensions/XfBcryptExtension.java diff --git a/src/main/java/fr/xephi/authme/datasource/MySQL.java b/src/main/java/fr/xephi/authme/datasource/MySQL.java index b86ad7167..22560be0e 100644 --- a/src/main/java/fr/xephi/authme/datasource/MySQL.java +++ b/src/main/java/fr/xephi/authme/datasource/MySQL.java @@ -5,17 +5,15 @@ import com.zaxxer.hikari.HikariDataSource; import com.zaxxer.hikari.pool.HikariPool.PoolInitializationException; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.data.auth.PlayerAuth; -import fr.xephi.authme.security.HashAlgorithm; +import fr.xephi.authme.datasource.mysqlextensions.MySqlExtension; +import fr.xephi.authme.datasource.mysqlextensions.MySqlExtensionsFactory; import fr.xephi.authme.security.crypts.HashedPassword; -import fr.xephi.authme.security.crypts.XfBCrypt; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.DatabaseSettings; import fr.xephi.authme.settings.properties.HooksSettings; -import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.Utils; -import java.sql.Blob; import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.PreparedStatement; @@ -44,19 +42,11 @@ public class MySQL implements DataSource { private int maxLifetime; private List columnOthers; private Columns col; - private HashAlgorithm hashAlgorithm; + private MySqlExtension sqlExtension; private HikariDataSource ds; - private String phpBbPrefix; - private String ipbPrefix; - private String xfPrefix; - private String wordpressPrefix; - private int phpBbGroup; - private int ipbGroup; - private int xfGroup; - - public MySQL(Settings settings) throws ClassNotFoundException, SQLException { - setParameters(settings); + public MySQL(Settings settings, MySqlExtensionsFactory extensionsFactory) throws SQLException { + setParameters(settings, extensionsFactory); // Set the connection arguments (and check if connection is ok) try { @@ -86,9 +76,9 @@ public class MySQL implements DataSource { } @VisibleForTesting - MySQL(Settings settings, HikariDataSource hikariDataSource) { + MySQL(Settings settings, HikariDataSource hikariDataSource, MySqlExtensionsFactory extensionsFactory) { ds = hikariDataSource; - setParameters(settings); + setParameters(settings, extensionsFactory); } /** @@ -96,7 +86,7 @@ public class MySQL implements DataSource { * * @param settings the settings to read properties from */ - private void setParameters(Settings settings) { + private void setParameters(Settings settings, MySqlExtensionsFactory extensionsFactory) { this.host = settings.getProperty(DatabaseSettings.MYSQL_HOST); this.port = settings.getProperty(DatabaseSettings.MYSQL_PORT); this.username = settings.getProperty(DatabaseSettings.MYSQL_USERNAME); @@ -105,14 +95,7 @@ public class MySQL implements DataSource { this.tableName = settings.getProperty(DatabaseSettings.MYSQL_TABLE); this.columnOthers = settings.getProperty(HooksSettings.MYSQL_OTHER_USERNAME_COLS); this.col = new Columns(settings); - this.hashAlgorithm = settings.getProperty(SecuritySettings.PASSWORD_HASH); - this.phpBbPrefix = settings.getProperty(HooksSettings.PHPBB_TABLE_PREFIX); - this.phpBbGroup = settings.getProperty(HooksSettings.PHPBB_ACTIVATED_GROUP_ID); - this.ipbPrefix = settings.getProperty(HooksSettings.IPB_TABLE_PREFIX); - this.ipbGroup = settings.getProperty(HooksSettings.IPB_ACTIVATED_GROUP_ID); - this.xfPrefix = settings.getProperty(HooksSettings.XF_TABLE_PREFIX); - this.xfGroup = settings.getProperty(HooksSettings.XF_ACTIVATED_GROUP_ID); - this.wordpressPrefix = settings.getProperty(HooksSettings.WORDPRESS_TABLE_PREFIX); + sqlExtension = extensionsFactory.buildExtension(col); this.poolSize = settings.getProperty(DatabaseSettings.MYSQL_POOL_SIZE); if (poolSize == -1) { poolSize = Utils.getCoreCount() * 3; @@ -303,28 +286,14 @@ public class MySQL implements DataSource { PlayerAuth auth; try (Connection con = getConnection(); PreparedStatement pst = con.prepareStatement(sql)) { pst.setString(1, user.toLowerCase()); - int id; try (ResultSet rs = pst.executeQuery()) { - if (!rs.next()) { - return null; - } - id = rs.getInt(col.ID); - auth = buildAuthFromResultSet(rs); - } - if (hashAlgorithm == HashAlgorithm.XFBCRYPT) { - try (PreparedStatement pst2 = con.prepareStatement( - "SELECT data FROM " + xfPrefix + "user_authenticate WHERE " + col.ID + "=?;")) { - pst2.setInt(1, id); - try (ResultSet rs = pst2.executeQuery()) { - if (rs.next()) { - Blob blob = rs.getBlob("data"); - byte[] bytes = blob.getBytes(1, (int) blob.length()); - auth.setPassword(new HashedPassword(XfBCrypt.getHashFromBlob(bytes))); - } - } + if (rs.next()) { + int id = rs.getInt(col.ID); + auth = buildAuthFromResultSet(rs); + sqlExtension.extendAuth(auth, id, con); + return auth; } } - return auth; } catch (SQLException ex) { logSqlException(ex); } @@ -364,231 +333,8 @@ public class MySQL implements DataSource { } } } - if (hashAlgorithm == HashAlgorithm.IPB4) { - sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst = con.prepareStatement(sql)) { - pst.setString(1, auth.getNickname()); - try (ResultSet rs = pst.executeQuery()) { - if (rs.next()) { - // Update player group in core_members - sql = "UPDATE " + ipbPrefix + tableName + " SET " + tableName + ".member_group_id=? WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setInt(1, ipbGroup); - pst2.setString(2, auth.getNickname()); - pst2.executeUpdate(); - } - // Get current time without ms - long time = System.currentTimeMillis() / 1000; - // update joined date - sql = "UPDATE " + ipbPrefix + tableName + " SET " + tableName + ".joined=? WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setLong(1, time); - pst2.setString(2, auth.getNickname()); - pst2.executeUpdate(); - } - // Update last_visit - sql = "UPDATE " + ipbPrefix + tableName + " SET " + tableName + ".last_visit=? WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setLong(1, time); - pst2.setString(2, auth.getNickname()); - pst2.executeUpdate(); - } - } - } - } - } else if (hashAlgorithm == HashAlgorithm.PHPBB) { - sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst = con.prepareStatement(sql)) { - pst.setString(1, auth.getNickname()); - try (ResultSet rs = pst.executeQuery()) { - if (rs.next()) { - int id = rs.getInt(col.ID); - // Insert player in phpbb_user_group - sql = "INSERT INTO " + phpBbPrefix - + "user_group (group_id, user_id, group_leader, user_pending) VALUES (?,?,?,?);"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setInt(1, phpBbGroup); - pst2.setInt(2, id); - pst2.setInt(3, 0); - pst2.setInt(4, 0); - pst2.executeUpdate(); - } - // Update username_clean in phpbb_users - sql = "UPDATE " + tableName + " SET " + tableName - + ".username_clean=? WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setString(1, auth.getNickname()); - pst2.setString(2, auth.getNickname()); - pst2.executeUpdate(); - } - // Update player group in phpbb_users - sql = "UPDATE " + tableName + " SET " + tableName - + ".group_id=? WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setInt(1, phpBbGroup); - pst2.setString(2, auth.getNickname()); - pst2.executeUpdate(); - } - // Get current time without ms - long time = System.currentTimeMillis() / 1000; - // Update user_regdate - sql = "UPDATE " + tableName + " SET " + tableName - + ".user_regdate=? WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setLong(1, time); - pst2.setString(2, auth.getNickname()); - pst2.executeUpdate(); - } - // Update user_lastvisit - sql = "UPDATE " + tableName + " SET " + tableName - + ".user_lastvisit=? WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setLong(1, time); - pst2.setString(2, auth.getNickname()); - pst2.executeUpdate(); - } - // Increment num_users - sql = "UPDATE " + phpBbPrefix - + "config SET config_value = config_value + 1 WHERE config_name = 'num_users';"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.executeUpdate(); - } - } - } - } - } else if (hashAlgorithm == HashAlgorithm.WORDPRESS) { - // NOTE: Eclipse says pst should be closed HERE, but it's a bug, we already close it above. -sgdc3 - try (PreparedStatement pst = con.prepareStatement("SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;")) { - pst.setString(1, auth.getNickname()); - try (ResultSet rs = pst.executeQuery()) { - if (rs.next()) { - int id = rs.getInt(col.ID); - sql = "INSERT INTO " + wordpressPrefix + "usermeta (user_id, meta_key, meta_value) VALUES (?,?,?)"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - // First Name - pst2.setInt(1, id); - pst2.setString(2, "first_name"); - pst2.setString(3, ""); - pst2.addBatch(); - // Last Name - pst2.setInt(1, id); - pst2.setString(2, "last_name"); - pst2.setString(3, ""); - pst2.addBatch(); - // Nick Name - pst2.setInt(1, id); - pst2.setString(2, "nickname"); - pst2.setString(3, auth.getNickname()); - pst2.addBatch(); - // Description - pst2.setInt(1, id); - pst2.setString(2, "description"); - pst2.setString(3, ""); - pst2.addBatch(); - // Rich_Editing - pst2.setInt(1, id); - pst2.setString(2, "rich_editing"); - pst2.setString(3, "true"); - pst2.addBatch(); - // Comments_Shortcuts - pst2.setInt(1, id); - pst2.setString(2, "comment_shortcuts"); - pst2.setString(3, "false"); - pst2.addBatch(); - // admin_color - pst2.setInt(1, id); - pst2.setString(2, "admin_color"); - pst2.setString(3, "fresh"); - pst2.addBatch(); - // use_ssl - pst2.setInt(1, id); - pst2.setString(2, "use_ssl"); - pst2.setString(3, "0"); - pst2.addBatch(); - // show_admin_bar_front - pst2.setInt(1, id); - pst2.setString(2, "show_admin_bar_front"); - pst2.setString(3, "true"); - pst2.addBatch(); - // wp_capabilities - pst2.setInt(1, id); - pst2.setString(2, wordpressPrefix + "capabilities"); - pst2.setString(3, "a:1:{s:10:\"subscriber\";b:1;}"); - pst2.addBatch(); - // wp_user_level - pst2.setInt(1, id); - pst2.setString(2, wordpressPrefix + "user_level"); - pst2.setString(3, "0"); - pst2.addBatch(); - // default_password_nag - pst2.setInt(1, id); - pst2.setString(2, "default_password_nag"); - pst2.setString(3, ""); - pst2.addBatch(); - // Execute queries - pst2.executeBatch(); - pst2.clearBatch(); - } - } - } - } - } else if (hashAlgorithm == HashAlgorithm.XFBCRYPT) { - // NOTE: Eclipse says pst should be closed HERE, but it's a bug, we already close it above. -sgdc3 - try (PreparedStatement pst = con.prepareStatement("SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;")) { - pst.setString(1, auth.getNickname()); - try (ResultSet rs = pst.executeQuery()) { - if (rs.next()) { - int id = rs.getInt(col.ID); - // Insert player password, salt in xf_user_authenticate - sql = "INSERT INTO " + xfPrefix + "user_authenticate (user_id, scheme_class, data) VALUES (?,?,?)"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setInt(1, id); - pst2.setString(2, XfBCrypt.SCHEME_CLASS); - String serializedHash = XfBCrypt.serializeHash(auth.getPassword().getHash()); - byte[] bytes = serializedHash.getBytes(); - Blob blob = con.createBlob(); - blob.setBytes(1, bytes); - pst2.setBlob(3, blob); - pst2.executeUpdate(); - } - // Update player group in xf_users - sql = "UPDATE " + tableName + " SET " + tableName + ".user_group_id=? WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setInt(1, xfGroup); - pst2.setString(2, auth.getNickname()); - pst2.executeUpdate(); - } - // Update player permission combination in xf_users - sql = "UPDATE " + tableName + " SET " + tableName + ".permission_combination_id=? WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setInt(1, xfGroup); - pst2.setString(2, auth.getNickname()); - pst2.executeUpdate(); - } - // Insert player privacy combination in xf_user_privacy - sql = "INSERT INTO " + xfPrefix + "user_privacy (user_id, allow_view_profile, allow_post_profile, allow_send_personal_conversation, allow_view_identities, allow_receive_news_feed) VALUES (?,?,?,?,?,?)"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setInt(1, id); - pst2.setString(2, "everyone"); - pst2.setString(3, "members"); - pst2.setString(4, "members"); - pst2.setString(5, "everyone"); - pst2.setString(6, "everyone"); - pst2.executeUpdate(); - } - // Insert player group relation in xf_user_group_relation - sql = "INSERT INTO " + xfPrefix + "user_group_relation (user_id, user_group_id, is_primary) VALUES (?,?,?)"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setInt(1, id); - pst2.setInt(2, xfGroup); - pst2.setString(3, "1"); - pst2.executeUpdate(); - } - } - } - } - } + sqlExtension.saveAuth(auth, con); return true; } catch (SQLException ex) { logSqlException(ex); @@ -624,35 +370,7 @@ public class MySQL implements DataSource { pst.executeUpdate(); } } - if (hashAlgorithm == HashAlgorithm.XFBCRYPT) { - String sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst = con.prepareStatement(sql)) { - pst.setString(1, user); - try (ResultSet rs = pst.executeQuery()) { - if (rs.next()) { - int id = rs.getInt(col.ID); - // Insert password in the correct table - sql = "UPDATE " + xfPrefix + "user_authenticate SET data=? WHERE " + col.ID + "=?;"; - PreparedStatement pst2 = con.prepareStatement(sql); - String serializedHash = XfBCrypt.serializeHash(password.getHash()); - byte[] bytes = serializedHash.getBytes(); - Blob blob = con.createBlob(); - blob.setBytes(1, bytes); - pst2.setBlob(1, blob); - pst2.setInt(2, id); - pst2.executeUpdate(); - pst2.close(); - // ... - sql = "UPDATE " + xfPrefix + "user_authenticate SET scheme_class=? WHERE " + col.ID + "=?;"; - pst2 = con.prepareStatement(sql); - pst2.setString(1, XfBCrypt.SCHEME_CLASS); - pst2.setInt(2, id); - pst2.executeUpdate(); - pst2.close(); - } - } - } - } + sqlExtension.changePassword(user, password, con); return true; } catch (SQLException ex) { logSqlException(ex); @@ -704,22 +422,7 @@ public class MySQL implements DataSource { user = user.toLowerCase(); String sql = "DELETE FROM " + tableName + " WHERE " + col.NAME + "=?;"; try (Connection con = getConnection(); PreparedStatement pst = con.prepareStatement(sql)) { - if (hashAlgorithm == HashAlgorithm.XFBCRYPT) { - sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; - try (PreparedStatement xfSelect = con.prepareStatement(sql)) { - xfSelect.setString(1, user); - try (ResultSet rs = xfSelect.executeQuery()) { - if (rs.next()) { - int id = rs.getInt(col.ID); - sql = "DELETE FROM " + xfPrefix + "user_authenticate WHERE " + col.ID + "=?;"; - try (PreparedStatement xfDelete = con.prepareStatement(sql)) { - xfDelete.setInt(1, id); - xfDelete.executeUpdate(); - } - } - } - } - } + sqlExtension.removeAuth(user, con); pst.setString(1, user.toLowerCase()); pst.executeUpdate(); return true; @@ -922,26 +625,12 @@ public class MySQL implements DataSource { @Override public List getAllAuths() { List auths = new ArrayList<>(); - try (Connection con = getConnection()) { - try (Statement st = con.createStatement()) { - try (ResultSet rs = st.executeQuery("SELECT * FROM " + tableName)) { - while (rs.next()) { - PlayerAuth pAuth = buildAuthFromResultSet(rs); - if (hashAlgorithm == HashAlgorithm.XFBCRYPT) { - try (PreparedStatement pst = con.prepareStatement("SELECT data FROM " + xfPrefix + "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); - } + try (Connection con = getConnection(); Statement st = con.createStatement()) { + try (ResultSet rs = st.executeQuery("SELECT * FROM " + tableName)) { + while (rs.next()) { + PlayerAuth auth = buildAuthFromResultSet(rs); + sqlExtension.extendAuth(auth, rs.getInt(col.ID), con); + auths.add(auth); } } } catch (SQLException ex) { @@ -987,36 +676,6 @@ public class MySQL implements DataSource { .build(); } - /** - * Closes a {@link ResultSet} safely. - * - * @param rs the result set to close - */ - private static void close(ResultSet rs) { - try { - if (rs != null && !rs.isClosed()) { - rs.close(); - } - } catch (SQLException e) { - ConsoleLogger.logException("Could not close ResultSet", e); - } - } - - /** - * Closes a {@link Statement} safely. - * - * @param st the statement set to close - */ - private static void close(Statement st) { - try { - if (st != null && !st.isClosed()) { - st.close(); - } - } catch (SQLException e) { - ConsoleLogger.logException("Could not close Statement", e); - } - } - /** * Checks if the last login column has a type that needs to be migrated. * diff --git a/src/main/java/fr/xephi/authme/datasource/converter/MySqlToSqlite.java b/src/main/java/fr/xephi/authme/datasource/converter/MySqlToSqlite.java index 7b793e8b0..b1a485b97 100644 --- a/src/main/java/fr/xephi/authme/datasource/converter/MySqlToSqlite.java +++ b/src/main/java/fr/xephi/authme/datasource/converter/MySqlToSqlite.java @@ -3,6 +3,7 @@ package fr.xephi.authme.datasource.converter; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.datasource.DataSourceType; import fr.xephi.authme.datasource.MySQL; +import fr.xephi.authme.datasource.mysqlextensions.MySqlExtensionsFactory; import fr.xephi.authme.settings.Settings; import javax.inject.Inject; @@ -14,15 +15,17 @@ import java.sql.SQLException; public class MySqlToSqlite extends AbstractDataSourceConverter { private final Settings settings; + private final MySqlExtensionsFactory mySqlExtensionsFactory; @Inject - MySqlToSqlite(DataSource dataSource, Settings settings) { + MySqlToSqlite(DataSource dataSource, Settings settings, MySqlExtensionsFactory mySqlExtensionsFactory) { super(dataSource, DataSourceType.SQLITE); this.settings = settings; + this.mySqlExtensionsFactory = mySqlExtensionsFactory; } @Override - protected MySQL getSource() throws SQLException, ClassNotFoundException { - return new MySQL(settings); + protected MySQL getSource() throws SQLException { + return new MySQL(settings, mySqlExtensionsFactory); } } diff --git a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/Ipb4Extension.java b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/Ipb4Extension.java new file mode 100644 index 000000000..0407d7d65 --- /dev/null +++ b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/Ipb4Extension.java @@ -0,0 +1,68 @@ +package fr.xephi.authme.datasource.mysqlextensions; + +import fr.xephi.authme.data.auth.PlayerAuth; +import fr.xephi.authme.datasource.Columns; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.DatabaseSettings; +import fr.xephi.authme.settings.properties.HooksSettings; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; + +/** + * Extension for IPB4. + */ +class Ipb4Extension extends MySqlExtension { + + private final Columns col; + private final String tableName; + private final String ipbPrefix; + private final int ipbGroup; + + Ipb4Extension(Settings settings, Columns col) { + this.col = col; + this.tableName = settings.getProperty(DatabaseSettings.MYSQL_TABLE); + this.ipbPrefix = settings.getProperty(HooksSettings.IPB_TABLE_PREFIX); + this.ipbGroup = settings.getProperty(HooksSettings.IPB_ACTIVATED_GROUP_ID); + } + + @Override + public void saveAuth(PlayerAuth auth, Connection con) throws SQLException { + String sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setString(1, auth.getNickname()); + try (ResultSet rs = pst.executeQuery()) { + if (rs.next()) { + // Update player group in core_members + sql = "UPDATE " + ipbPrefix + tableName + + " SET " + tableName + ".member_group_id=? WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst2 = con.prepareStatement(sql)) { + pst2.setInt(1, ipbGroup); + pst2.setString(2, auth.getNickname()); + pst2.executeUpdate(); + } + // Get current time without ms + long time = System.currentTimeMillis() / 1000; + // update joined date + sql = "UPDATE " + ipbPrefix + tableName + + " SET " + tableName + ".joined=? WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst2 = con.prepareStatement(sql)) { + pst2.setLong(1, time); + pst2.setString(2, auth.getNickname()); + pst2.executeUpdate(); + } + // Update last_visit + sql = "UPDATE " + ipbPrefix + tableName + + " SET " + tableName + ".last_visit=? WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst2 = con.prepareStatement(sql)) { + pst2.setLong(1, time); + pst2.setString(2, auth.getNickname()); + pst2.executeUpdate(); + } + } + } + } + } +} diff --git a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtension.java b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtension.java new file mode 100644 index 000000000..8b5bf5756 --- /dev/null +++ b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtension.java @@ -0,0 +1,62 @@ +package fr.xephi.authme.datasource.mysqlextensions; + +import fr.xephi.authme.data.auth.PlayerAuth; +import fr.xephi.authme.security.crypts.HashedPassword; + +import java.sql.Connection; +import java.sql.SQLException; + +/** + * Extension for the MySQL data source for forums. For certain password hashes (e.g. phpBB), we want + * to hook into the forum board and execute some actions specific to the forum software. + */ +public abstract class MySqlExtension { + + /** + * Performs additional actions when a new player is saved. + * + * @param auth the player auth that has been saved + * @param con connection to the sql table + * @throws SQLException . + */ + public void saveAuth(PlayerAuth auth, Connection con) throws SQLException { + // extend for custom behavior + } + + /** + * Writes properties to the given PlayerAuth object that need to be retrieved in a specific manner + * when a PlayerAuth object is read from the table. + * + * @param auth the player auth object to extend + * @param id the database id of the player auth entry + * @param con connection to the sql table + * @throws SQLException . + */ + public void extendAuth(PlayerAuth auth, int id, Connection con) throws SQLException { + // extend for custom behavior + } + + /** + * Performs additional actions when a user's password is changed. + * + * @param user the name of the player (lowercase) + * @param password the new password to set + * @param con connection to the sql table + * @throws SQLException . + */ + public void changePassword(String user, HashedPassword password, Connection con) throws SQLException { + // extend for custom behavior + } + + /** + * Performs additional actions when a player is removed from the database. + * + * @param user the user to remove + * @param con connection to the sql table + * @throws SQLException . + */ + public void removeAuth(String user, Connection con) throws SQLException { + // extend for custom behavior + } + +} diff --git a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtensionsFactory.java b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtensionsFactory.java new file mode 100644 index 000000000..8974e2c1c --- /dev/null +++ b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtensionsFactory.java @@ -0,0 +1,33 @@ +package fr.xephi.authme.datasource.mysqlextensions; + +import fr.xephi.authme.datasource.Columns; +import fr.xephi.authme.security.HashAlgorithm; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.SecuritySettings; + +import javax.inject.Inject; + +/** + * Creates the appropriate {@link MySqlExtension}, depending on the configured password hashing algorithm. + */ +public class MySqlExtensionsFactory { + + @Inject + private Settings settings; + + public MySqlExtension buildExtension(Columns columnsConfig) { + HashAlgorithm hash = settings.getProperty(SecuritySettings.PASSWORD_HASH); + switch (hash) { + case IPB4: + return new Ipb4Extension(settings, columnsConfig); + case PHPBB: + return new PhpBbExtension(settings, columnsConfig); + case WORDPRESS: + return new WordpressExtension(settings, columnsConfig); + case XFBCRYPT: + return new XfBcryptExtension(settings, columnsConfig); + default: + return new NoOpExtension(); + } + } +} diff --git a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtension.java b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtension.java new file mode 100644 index 000000000..8146c6cec --- /dev/null +++ b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtension.java @@ -0,0 +1,7 @@ +package fr.xephi.authme.datasource.mysqlextensions; + +/** + * Extension implementation that does not do anything. + */ +class NoOpExtension extends MySqlExtension { +} diff --git a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/PhpBbExtension.java b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/PhpBbExtension.java new file mode 100644 index 000000000..33a6a88f5 --- /dev/null +++ b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/PhpBbExtension.java @@ -0,0 +1,93 @@ +package fr.xephi.authme.datasource.mysqlextensions; + +import fr.xephi.authme.data.auth.PlayerAuth; +import fr.xephi.authme.datasource.Columns; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.DatabaseSettings; +import fr.xephi.authme.settings.properties.HooksSettings; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; + +/** + * Extensions for phpBB when MySQL is used as data source. + */ +class PhpBbExtension extends MySqlExtension { + + private final Columns col; + private final String tableName; + private final String phpBbPrefix; + private final int phpBbGroup; + + PhpBbExtension(Settings settings, Columns col) { + this.col = col; + this.tableName = settings.getProperty(DatabaseSettings.MYSQL_TABLE); + this.phpBbPrefix = settings.getProperty(HooksSettings.PHPBB_TABLE_PREFIX); + this.phpBbGroup = settings.getProperty(HooksSettings.PHPBB_ACTIVATED_GROUP_ID); + } + + @Override + public void saveAuth(PlayerAuth auth, Connection con) throws SQLException { + String sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setString(1, auth.getNickname()); + try (ResultSet rs = pst.executeQuery()) { + if (rs.next()) { + int id = rs.getInt(col.ID); + updateSpecificsOnSave(id, auth.getNickname(), con); + } + } + } + } + + private void updateSpecificsOnSave(int id, String name, Connection con) throws SQLException { + // Insert player in phpbb_user_group + String sql = "INSERT INTO " + phpBbPrefix + + "user_group (group_id, user_id, group_leader, user_pending) VALUES (?,?,?,?);"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setInt(1, phpBbGroup); + pst.setInt(2, id); + pst.setInt(3, 0); + pst.setInt(4, 0); + pst.executeUpdate(); + } + // Update username_clean in phpbb_users + sql = "UPDATE " + tableName + " SET " + tableName + ".username_clean=? WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setString(1, name); + pst.setString(2, name); + pst.executeUpdate(); + } + // Update player group in phpbb_users + sql = "UPDATE " + tableName + " SET " + tableName + ".group_id=? WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setInt(1, phpBbGroup); + pst.setString(2, name); + pst.executeUpdate(); + } + // Get current time without ms + long time = System.currentTimeMillis() / 1000; + // Update user_regdate + sql = "UPDATE " + tableName + " SET " + tableName + ".user_regdate=? WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setLong(1, time); + pst.setString(2, name); + pst.executeUpdate(); + } + // Update user_lastvisit + sql = "UPDATE " + tableName + " SET " + tableName + ".user_lastvisit=? WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setLong(1, time); + pst.setString(2, name); + pst.executeUpdate(); + } + // Increment num_users + sql = "UPDATE " + phpBbPrefix + + "config SET config_value = config_value + 1 WHERE config_name = 'num_users';"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.executeUpdate(); + } + } +} diff --git a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/WordpressExtension.java b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/WordpressExtension.java new file mode 100644 index 000000000..64e6a13f2 --- /dev/null +++ b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/WordpressExtension.java @@ -0,0 +1,107 @@ +package fr.xephi.authme.datasource.mysqlextensions; + +import fr.xephi.authme.data.auth.PlayerAuth; +import fr.xephi.authme.datasource.Columns; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.DatabaseSettings; +import fr.xephi.authme.settings.properties.HooksSettings; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; + +/** + * MySQL extensions for Wordpress. + */ +class WordpressExtension extends MySqlExtension { + + private final Columns col; + private final String tableName; + private final String wordpressPrefix; + + WordpressExtension(Settings settings, Columns col) { + this.col = col; + this.tableName = settings.getProperty(DatabaseSettings.MYSQL_TABLE); + this.wordpressPrefix = settings.getProperty(HooksSettings.WORDPRESS_TABLE_PREFIX); + } + + @Override + public void saveAuth(PlayerAuth auth, Connection con) throws SQLException { + try (PreparedStatement pst = con.prepareStatement("SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;")) { + pst.setString(1, auth.getNickname()); + try (ResultSet rs = pst.executeQuery()) { + if (rs.next()) { + int id = rs.getInt(col.ID); + String sql = "INSERT INTO " + wordpressPrefix + "usermeta (user_id, meta_key, meta_value) VALUES (?,?,?)"; + try (PreparedStatement pst2 = con.prepareStatement(sql)) { + // First Name + pst2.setInt(1, id); + pst2.setString(2, "first_name"); + pst2.setString(3, ""); + pst2.addBatch(); + // Last Name + pst2.setInt(1, id); + pst2.setString(2, "last_name"); + pst2.setString(3, ""); + pst2.addBatch(); + // Nick Name + pst2.setInt(1, id); + pst2.setString(2, "nickname"); + pst2.setString(3, auth.getNickname()); + pst2.addBatch(); + // Description + pst2.setInt(1, id); + pst2.setString(2, "description"); + pst2.setString(3, ""); + pst2.addBatch(); + // Rich_Editing + pst2.setInt(1, id); + pst2.setString(2, "rich_editing"); + pst2.setString(3, "true"); + pst2.addBatch(); + // Comments_Shortcuts + pst2.setInt(1, id); + pst2.setString(2, "comment_shortcuts"); + pst2.setString(3, "false"); + pst2.addBatch(); + // admin_color + pst2.setInt(1, id); + pst2.setString(2, "admin_color"); + pst2.setString(3, "fresh"); + pst2.addBatch(); + // use_ssl + pst2.setInt(1, id); + pst2.setString(2, "use_ssl"); + pst2.setString(3, "0"); + pst2.addBatch(); + // show_admin_bar_front + pst2.setInt(1, id); + pst2.setString(2, "show_admin_bar_front"); + pst2.setString(3, "true"); + pst2.addBatch(); + // wp_capabilities + pst2.setInt(1, id); + pst2.setString(2, wordpressPrefix + "capabilities"); + pst2.setString(3, "a:1:{s:10:\"subscriber\";b:1;}"); + pst2.addBatch(); + // wp_user_level + pst2.setInt(1, id); + pst2.setString(2, wordpressPrefix + "user_level"); + pst2.setString(3, "0"); + pst2.addBatch(); + // default_password_nag + pst2.setInt(1, id); + pst2.setString(2, "default_password_nag"); + pst2.setString(3, ""); + pst2.addBatch(); + + // Execute queries + pst2.executeBatch(); + pst2.clearBatch(); + } + } + } + } + } +} diff --git a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/XfBcryptExtension.java b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/XfBcryptExtension.java new file mode 100644 index 000000000..de8ffb350 --- /dev/null +++ b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/XfBcryptExtension.java @@ -0,0 +1,160 @@ +package fr.xephi.authme.datasource.mysqlextensions; + +import fr.xephi.authme.data.auth.PlayerAuth; +import fr.xephi.authme.datasource.Columns; +import fr.xephi.authme.security.crypts.HashedPassword; +import fr.xephi.authme.security.crypts.XfBCrypt; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.DatabaseSettings; +import fr.xephi.authme.settings.properties.HooksSettings; + +import java.sql.Blob; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; + +/** + * Extension for XFBCRYPT. + */ +class XfBcryptExtension extends MySqlExtension { + + private final Columns col; + private final String tableName; + private final String xfPrefix; + private final int xfGroup; + + XfBcryptExtension(Settings settings, Columns col) { + this.col = col; + this.tableName = settings.getProperty(DatabaseSettings.MYSQL_TABLE); + this.xfPrefix = settings.getProperty(HooksSettings.XF_TABLE_PREFIX); + this.xfGroup = settings.getProperty(HooksSettings.XF_ACTIVATED_GROUP_ID); + } + + @Override + public void saveAuth(PlayerAuth auth, Connection con) throws SQLException { + String sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setString(1, auth.getNickname()); + try (ResultSet rs = pst.executeQuery()) { + if (rs.next()) { + int id = rs.getInt(col.ID); + updateXenforoTablesOnSave(auth, id, con); + } + } + } + } + + private void updateXenforoTablesOnSave(PlayerAuth auth, int id, Connection con) throws SQLException { + // Insert player password, salt in xf_user_authenticate + String sql = "INSERT INTO " + xfPrefix + "user_authenticate (user_id, scheme_class, data) VALUES (?,?,?)"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setInt(1, id); + pst.setString(2, XfBCrypt.SCHEME_CLASS); + String serializedHash = XfBCrypt.serializeHash(auth.getPassword().getHash()); + byte[] bytes = serializedHash.getBytes(); + Blob blob = con.createBlob(); + blob.setBytes(1, bytes); + pst.setBlob(3, blob); + pst.executeUpdate(); + } + // Update player group in xf_users + sql = "UPDATE " + tableName + " SET " + tableName + ".user_group_id=? WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setInt(1, xfGroup); + pst.setString(2, auth.getNickname()); + pst.executeUpdate(); + } + // Update player permission combination in xf_users + sql = "UPDATE " + tableName + " SET " + tableName + ".permission_combination_id=? WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setInt(1, xfGroup); + pst.setString(2, auth.getNickname()); + pst.executeUpdate(); + } + // Insert player privacy combination in xf_user_privacy + sql = "INSERT INTO " + xfPrefix + "user_privacy (user_id, allow_view_profile, allow_post_profile, " + + "allow_send_personal_conversation, allow_view_identities, allow_receive_news_feed) VALUES (?,?,?,?,?,?)"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setInt(1, id); + pst.setString(2, "everyone"); + pst.setString(3, "members"); + pst.setString(4, "members"); + pst.setString(5, "everyone"); + pst.setString(6, "everyone"); + pst.executeUpdate(); + } + // Insert player group relation in xf_user_group_relation + sql = "INSERT INTO " + xfPrefix + "user_group_relation (user_id, user_group_id, is_primary) VALUES (?,?,?)"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setInt(1, id); + pst.setInt(2, xfGroup); + pst.setString(3, "1"); + pst.executeUpdate(); + } + } + + public void extendAuth(PlayerAuth auth, int id, Connection con) throws SQLException { + try (PreparedStatement pst = con.prepareStatement( + "SELECT data FROM " + xfPrefix + "user_authenticate WHERE " + col.ID + "=?;")) { + pst.setInt(1, id); + try (ResultSet rs = pst.executeQuery()) { + if (rs.next()) { + Blob blob = rs.getBlob("data"); + byte[] bytes = blob.getBytes(1, (int) blob.length()); + auth.setPassword(new HashedPassword(XfBCrypt.getHashFromBlob(bytes))); + } + } + } + } + + @Override + public void changePassword(String user, HashedPassword password, Connection con) throws SQLException { + String sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setString(1, user); + try (ResultSet rs = pst.executeQuery()) { + if (rs.next()) { + int id = rs.getInt(col.ID); + // Insert password in the correct table + // TODO #1255: Close these statements with try-catch + sql = "UPDATE " + xfPrefix + "user_authenticate SET data=? WHERE " + col.ID + "=?;"; + PreparedStatement pst2 = con.prepareStatement(sql); + String serializedHash = XfBCrypt.serializeHash(password.getHash()); + byte[] bytes = serializedHash.getBytes(); + Blob blob = con.createBlob(); + blob.setBytes(1, bytes); + pst2.setBlob(1, blob); + pst2.setInt(2, id); + pst2.executeUpdate(); + pst2.close(); + // ... + sql = "UPDATE " + xfPrefix + "user_authenticate SET scheme_class=? WHERE " + col.ID + "=?;"; + pst2 = con.prepareStatement(sql); + pst2.setString(1, XfBCrypt.SCHEME_CLASS); + pst2.setInt(2, id); + pst2.executeUpdate(); + pst2.close(); + } + } + } + } + + @Override + public void removeAuth(String user, Connection con) throws SQLException { + String sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; + try (PreparedStatement xfSelect = con.prepareStatement(sql)) { + xfSelect.setString(1, user); + try (ResultSet rs = xfSelect.executeQuery()) { + if (rs.next()) { + int id = rs.getInt(col.ID); + sql = "DELETE FROM " + xfPrefix + "user_authenticate WHERE " + col.ID + "=?;"; + try (PreparedStatement xfDelete = con.prepareStatement(sql)) { + xfDelete.setInt(1, id); + xfDelete.executeUpdate(); + } + } + } + } + } +} diff --git a/src/main/java/fr/xephi/authme/initialization/DataSourceProvider.java b/src/main/java/fr/xephi/authme/initialization/DataSourceProvider.java index 594cabf4e..4f5d3d4dd 100644 --- a/src/main/java/fr/xephi/authme/initialization/DataSourceProvider.java +++ b/src/main/java/fr/xephi/authme/initialization/DataSourceProvider.java @@ -9,6 +9,7 @@ import fr.xephi.authme.datasource.FlatFile; import fr.xephi.authme.datasource.MySQL; import fr.xephi.authme.datasource.SQLite; import fr.xephi.authme.datasource.converter.ForceFlatToSqlite; +import fr.xephi.authme.datasource.mysqlextensions.MySqlExtensionsFactory; import fr.xephi.authme.service.BukkitService; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.DatabaseSettings; @@ -36,6 +37,8 @@ public class DataSourceProvider implements Provider { private BukkitService bukkitService; @Inject private PlayerCache playerCache; + @Inject + private MySqlExtensionsFactory mySqlExtensionsFactory; DataSourceProvider() { } @@ -67,7 +70,7 @@ public class DataSourceProvider implements Provider { dataSource = new FlatFile(source); break; case MYSQL: - dataSource = new MySQL(settings); + dataSource = new MySQL(settings, mySqlExtensionsFactory); break; case SQLITE: dataSource = new SQLite(settings); @@ -82,12 +85,12 @@ public class DataSourceProvider implements Provider { dataSource = new CacheDataSource(dataSource, playerCache); } if (DataSourceType.SQLITE.equals(dataSourceType)) { - checkDataSourceSize(dataSource, bukkitService); + checkDataSourceSize(dataSource); } return dataSource; } - private void checkDataSourceSize(final DataSource dataSource, BukkitService bukkitService) { + private void checkDataSourceSize(DataSource dataSource) { bukkitService.runTaskAsynchronously(() -> { int accounts = dataSource.getAccountsRegistered(); if (accounts >= SQLITE_MAX_SIZE) { diff --git a/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java index 1da098e1e..845923b96 100644 --- a/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java +++ b/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java @@ -1,16 +1,13 @@ package fr.xephi.authme.datasource; import ch.jalu.configme.properties.Property; -import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import fr.xephi.authme.TestHelper; import fr.xephi.authme.data.auth.PlayerAuth; -import fr.xephi.authme.security.HashAlgorithm; import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.settings.properties.SecuritySettings; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; @@ -19,7 +16,6 @@ import org.junit.runners.Parameterized; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; -import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.ParameterizedType; @@ -64,13 +60,6 @@ public abstract class AbstractResourceClosingTest { /** Collection of values to use to call methods with the parameters they expect. */ private static final Map, Object> PARAM_VALUES = getDefaultParameters(); - /** - * Custom list of hash algorithms to use to test a method. By default we define {@link HashAlgorithm#XFBCRYPT} as - * algorithms we use as a lot of methods execute additional statements in {@link MySQL}. If other algorithms - * have custom behaviors, they can be supplied in this map so it will be tested as well. - */ - private static final Map CUSTOM_ALGORITHMS = getCustomAlgorithmList(); - /** Mock of a settings instance. */ private static Settings settings; @@ -84,23 +73,21 @@ public abstract class AbstractResourceClosingTest { private List closeables = new ArrayList<>(); /** - * Constructor for the test instance verifying the given method with the given hash algorithm. + * Constructor for the test instance verifying the given method. * * @param method The DataSource method to test * @param name The name of the method - * @param algorithm The hash algorithm to use */ - public AbstractResourceClosingTest(Method method, String name, HashAlgorithm algorithm) { + public AbstractResourceClosingTest(Method method, String name) { // Note ljacqu 20160227: The name parameter is necessary as we pass it from the @Parameters method; // we use the method name in the annotation to name the test sensibly this.method = method; - given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(algorithm); } /** Initialize the settings mock and makes it return the default of any given property by default. */ - @SuppressWarnings({ "unchecked", "rawtypes" }) + @SuppressWarnings("unchecked") @BeforeClass - public static void initializeSettings() throws IOException, ClassNotFoundException { + public static void initializeSettings() { settings = mock(Settings.class); given(settings.getProperty(any(Property.class))).willAnswer(new Answer() { @Override @@ -129,24 +116,16 @@ public abstract class AbstractResourceClosingTest { } /** - * Initialization method -- provides the parameters to run the test with by scanning all DataSource - * methods. By default, we run one test per method with the default hash algorithm, XFBCRYPT. - * If the map of custom algorithms has an entry for the method name, we add an entry for each algorithm - * supplied by the map. + * Initialization method -- provides the parameters to run the test with by scanning all DataSource methods. * * @return Test parameters */ - @Parameterized.Parameters(name = "{1}({2})") + @Parameterized.Parameters(name = "{1}") public static Collection data() { List methods = getDataSourceMethods(); List data = new ArrayList<>(); - // Use XFBCRYPT if nothing else specified as there is a lot of specific behavior to this hash algorithm in MySQL - final HashAlgorithm[] defaultAlgorithm = new HashAlgorithm[]{HashAlgorithm.XFBCRYPT}; for (Method method : methods) { - HashAlgorithm[] algorithms = MoreObjects.firstNonNull(CUSTOM_ALGORITHMS.get(method.getName()), defaultAlgorithm); - for (HashAlgorithm algorithm : algorithms) { - data.add(new Object[]{method, method.getName(), algorithm}); - } + data.add(new Object[]{method, method.getName()}); } return data; } @@ -247,20 +226,6 @@ public abstract class AbstractResourceClosingTest { .build(); } - /** - * Return the custom list of hash algorithms to test a method with to execute code specific to - * one hash algorithm. By default, XFBCRYPT is used. Only MySQL has code specific to algorithms - * but for technical reasons the custom list will be used for all tested classes. - * - * @return List of custom algorithms by method - */ - private static Map getCustomAlgorithmList() { - // We use XFBCRYPT as default encryption method so we don't have to list many of the special cases for it - return ImmutableMap.builder() - .put("saveAuth", new HashAlgorithm[]{HashAlgorithm.PHPBB, HashAlgorithm.WORDPRESS}) - .build(); - } - // --------------------- // Mock initialization // --------------------- diff --git a/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java b/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java index 1ec75312a..62e1ac3ec 100644 --- a/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java @@ -4,6 +4,8 @@ import ch.jalu.configme.properties.Property; import com.zaxxer.hikari.HikariConfig; import com.zaxxer.hikari.HikariDataSource; import fr.xephi.authme.TestHelper; +import fr.xephi.authme.datasource.mysqlextensions.MySqlExtension; +import fr.xephi.authme.datasource.mysqlextensions.MySqlExtensionsFactory; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.DatabaseSettings; import org.junit.After; @@ -30,6 +32,8 @@ public class MySqlIntegrationTest extends AbstractDataSourceIntegrationTest { /** Mock of a settings instance. */ private static Settings settings; + /** Mock of extensions factory. */ + private static MySqlExtensionsFactory extensionsFactory; /** SQL statement to execute before running a test. */ private static String sqlInitialize; /** Connection to the H2 test database. */ @@ -51,6 +55,8 @@ public class MySqlIntegrationTest extends AbstractDataSourceIntegrationTest { return ((Property) invocation.getArguments()[0]).getDefaultValue(); } }); + extensionsFactory = mock(MySqlExtensionsFactory.class); + when(extensionsFactory.buildExtension(any(Columns.class))).thenReturn(mock(MySqlExtension.class)); set(DatabaseSettings.MYSQL_DATABASE, "h2_test"); set(DatabaseSettings.MYSQL_TABLE, "authme"); TestHelper.setRealLogger(); @@ -85,7 +91,7 @@ public class MySqlIntegrationTest extends AbstractDataSourceIntegrationTest { @Override protected DataSource getDataSource(String saltColumn) { when(settings.getProperty(DatabaseSettings.MYSQL_COL_SALT)).thenReturn(saltColumn); - return new MySQL(settings, hikariSource); + return new MySQL(settings, hikariSource, extensionsFactory); } private static void set(Property property, T value) { diff --git a/src/test/java/fr/xephi/authme/datasource/MySqlResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/MySqlResourceClosingTest.java index 7dccb08b0..f41a21641 100644 --- a/src/test/java/fr/xephi/authme/datasource/MySqlResourceClosingTest.java +++ b/src/test/java/fr/xephi/authme/datasource/MySqlResourceClosingTest.java @@ -1,12 +1,14 @@ package fr.xephi.authme.datasource; import com.zaxxer.hikari.HikariDataSource; -import fr.xephi.authme.security.HashAlgorithm; +import fr.xephi.authme.datasource.mysqlextensions.MySqlExtension; +import fr.xephi.authme.datasource.mysqlextensions.MySqlExtensionsFactory; import fr.xephi.authme.settings.Settings; import java.lang.reflect.Method; import java.sql.Connection; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -15,15 +17,17 @@ import static org.mockito.Mockito.mock; */ public class MySqlResourceClosingTest extends AbstractResourceClosingTest { - public MySqlResourceClosingTest(Method method, String name, HashAlgorithm algorithm) { - super(method, name, algorithm); + public MySqlResourceClosingTest(Method method, String name) { + super(method, name); } @Override protected DataSource createDataSource(Settings settings, Connection connection) throws Exception { HikariDataSource hikariDataSource = mock(HikariDataSource.class); given(hikariDataSource.getConnection()).willReturn(connection); - return new MySQL(settings, hikariDataSource); + MySqlExtensionsFactory extensionsFactory = mock(MySqlExtensionsFactory.class); + given(extensionsFactory.buildExtension(any(Columns.class))).willReturn(mock(MySqlExtension.class)); + return new MySQL(settings, hikariDataSource, extensionsFactory); } } diff --git a/src/test/java/fr/xephi/authme/datasource/SQLiteResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/SQLiteResourceClosingTest.java index 9eac94630..69eade8f1 100644 --- a/src/test/java/fr/xephi/authme/datasource/SQLiteResourceClosingTest.java +++ b/src/test/java/fr/xephi/authme/datasource/SQLiteResourceClosingTest.java @@ -1,6 +1,5 @@ package fr.xephi.authme.datasource; -import fr.xephi.authme.security.HashAlgorithm; import fr.xephi.authme.settings.Settings; import java.lang.reflect.Method; @@ -11,8 +10,8 @@ import java.sql.Connection; */ public class SQLiteResourceClosingTest extends AbstractResourceClosingTest { - public SQLiteResourceClosingTest(Method method, String name, HashAlgorithm algorithm) { - super(method, name, algorithm); + public SQLiteResourceClosingTest(Method method, String name) { + super(method, name); } @Override From 8eceaa8cbb66b44bbc10e6bb7c6112ed284bd97a Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 23 Jul 2017 14:54:04 +0200 Subject: [PATCH 3/5] #1255 Simplify MySQL data source extensions - Mostly oving the logic of getting the ID from the DB to the extensions superclass --- .../fr/xephi/authme/datasource/MySQL.java | 5 +- .../mysqlextensions/Ipb4Extension.java | 65 +++----- .../mysqlextensions/MySqlExtension.java | 34 ++++ .../MySqlExtensionsFactory.java | 8 +- .../mysqlextensions/NoOpExtension.java | 7 + .../mysqlextensions/PhpBbExtension.java | 20 +-- .../mysqlextensions/WordpressExtension.java | 153 +++++++++--------- .../mysqlextensions/XfBcryptExtension.java | 84 ++++------ .../xephi/authme/ClassesConsistencyTest.java | 2 + 9 files changed, 190 insertions(+), 188 deletions(-) diff --git a/src/main/java/fr/xephi/authme/datasource/MySQL.java b/src/main/java/fr/xephi/authme/datasource/MySQL.java index 22560be0e..73f758403 100644 --- a/src/main/java/fr/xephi/authme/datasource/MySQL.java +++ b/src/main/java/fr/xephi/authme/datasource/MySQL.java @@ -85,6 +85,7 @@ public class MySQL implements DataSource { * Retrieves various settings. * * @param settings the settings to read properties from + * @param extensionsFactory factory to create the MySQL extension */ private void setParameters(Settings settings, MySqlExtensionsFactory extensionsFactory) { this.host = settings.getProperty(DatabaseSettings.MYSQL_HOST); @@ -435,8 +436,8 @@ public class MySQL implements DataSource { @Override public boolean updateQuitLoc(PlayerAuth auth) { String sql = "UPDATE " + tableName - + " SET " + col.LASTLOC_X + " =?, " + col.LASTLOC_Y + "=?, " + col.LASTLOC_Z + "=?, " + col.LASTLOC_WORLD + "=?, " - + col.LASTLOC_YAW + "=?, " + col.LASTLOC_PITCH + "=?" + + " SET " + col.LASTLOC_X + " =?, " + col.LASTLOC_Y + "=?, " + col.LASTLOC_Z + "=?, " + + col.LASTLOC_WORLD + "=?, " + col.LASTLOC_YAW + "=?, " + col.LASTLOC_PITCH + "=?" + " WHERE " + col.NAME + "=?;"; try (Connection con = getConnection(); PreparedStatement pst = con.prepareStatement(sql)) { pst.setDouble(1, auth.getQuitLocX()); diff --git a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/Ipb4Extension.java b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/Ipb4Extension.java index 0407d7d65..2f39b8f7d 100644 --- a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/Ipb4Extension.java +++ b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/Ipb4Extension.java @@ -3,12 +3,10 @@ package fr.xephi.authme.datasource.mysqlextensions; import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.datasource.Columns; import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.settings.properties.DatabaseSettings; import fr.xephi.authme.settings.properties.HooksSettings; import java.sql.Connection; import java.sql.PreparedStatement; -import java.sql.ResultSet; import java.sql.SQLException; /** @@ -16,53 +14,42 @@ import java.sql.SQLException; */ class Ipb4Extension extends MySqlExtension { - private final Columns col; - private final String tableName; private final String ipbPrefix; private final int ipbGroup; Ipb4Extension(Settings settings, Columns col) { - this.col = col; - this.tableName = settings.getProperty(DatabaseSettings.MYSQL_TABLE); + super(settings, col); this.ipbPrefix = settings.getProperty(HooksSettings.IPB_TABLE_PREFIX); this.ipbGroup = settings.getProperty(HooksSettings.IPB_ACTIVATED_GROUP_ID); } @Override public void saveAuth(PlayerAuth auth, Connection con) throws SQLException { - String sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst = con.prepareStatement(sql)) { - pst.setString(1, auth.getNickname()); - try (ResultSet rs = pst.executeQuery()) { - if (rs.next()) { - // Update player group in core_members - sql = "UPDATE " + ipbPrefix + tableName - + " SET " + tableName + ".member_group_id=? WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setInt(1, ipbGroup); - pst2.setString(2, auth.getNickname()); - pst2.executeUpdate(); - } - // Get current time without ms - long time = System.currentTimeMillis() / 1000; - // update joined date - sql = "UPDATE " + ipbPrefix + tableName - + " SET " + tableName + ".joined=? WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setLong(1, time); - pst2.setString(2, auth.getNickname()); - pst2.executeUpdate(); - } - // Update last_visit - sql = "UPDATE " + ipbPrefix + tableName - + " SET " + tableName + ".last_visit=? WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - pst2.setLong(1, time); - pst2.setString(2, auth.getNickname()); - pst2.executeUpdate(); - } - } - } + // Update player group in core_members + String sql = "UPDATE " + ipbPrefix + tableName + + " SET " + tableName + ".member_group_id=? WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst2 = con.prepareStatement(sql)) { + pst2.setInt(1, ipbGroup); + pst2.setString(2, auth.getNickname()); + pst2.executeUpdate(); + } + // Get current time without ms + long time = System.currentTimeMillis() / 1000; + // update joined date + sql = "UPDATE " + ipbPrefix + tableName + + " SET " + tableName + ".joined=? WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst2 = con.prepareStatement(sql)) { + pst2.setLong(1, time); + pst2.setString(2, auth.getNickname()); + pst2.executeUpdate(); + } + // Update last_visit + sql = "UPDATE " + ipbPrefix + tableName + + " SET " + tableName + ".last_visit=? WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst2 = con.prepareStatement(sql)) { + pst2.setLong(1, time); + pst2.setString(2, auth.getNickname()); + pst2.executeUpdate(); } } } diff --git a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtension.java b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtension.java index 8b5bf5756..07374dfbd 100644 --- a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtension.java +++ b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtension.java @@ -1,10 +1,16 @@ package fr.xephi.authme.datasource.mysqlextensions; import fr.xephi.authme.data.auth.PlayerAuth; +import fr.xephi.authme.datasource.Columns; import fr.xephi.authme.security.crypts.HashedPassword; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.DatabaseSettings; import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; +import java.util.OptionalInt; /** * Extension for the MySQL data source for forums. For certain password hashes (e.g. phpBB), we want @@ -12,6 +18,14 @@ import java.sql.SQLException; */ public abstract class MySqlExtension { + protected final Columns col; + protected final String tableName; + + MySqlExtension(Settings settings, Columns col) { + this.col = col; + this.tableName = settings.getProperty(DatabaseSettings.MYSQL_TABLE); + } + /** * Performs additional actions when a new player is saved. * @@ -59,4 +73,24 @@ public abstract class MySqlExtension { // extend for custom behavior } + /** + * Fetches the database ID of the given name from the database. + * + * @param name the name to get the ID for + * @param con connection to the sql table + * @return id of the playerAuth, or empty OptionalInt if the name is not registered + * @throws SQLException . + */ + protected OptionalInt retrieveIdFromTable(String name, Connection con) throws SQLException { + String sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setString(1, name); + try (ResultSet rs = pst.executeQuery()) { + if (rs.next()) { + return OptionalInt.of(rs.getInt(col.ID)); + } + } + } + return OptionalInt.empty(); + } } diff --git a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtensionsFactory.java b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtensionsFactory.java index 8974e2c1c..dda94d784 100644 --- a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtensionsFactory.java +++ b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/MySqlExtensionsFactory.java @@ -15,6 +15,12 @@ public class MySqlExtensionsFactory { @Inject private Settings settings; + /** + * Creates a new {@link MySqlExtension} object according to the configured hash algorithm. + * + * @param columnsConfig the columns configuration + * @return the extension the MySQL data source should use + */ public MySqlExtension buildExtension(Columns columnsConfig) { HashAlgorithm hash = settings.getProperty(SecuritySettings.PASSWORD_HASH); switch (hash) { @@ -27,7 +33,7 @@ public class MySqlExtensionsFactory { case XFBCRYPT: return new XfBcryptExtension(settings, columnsConfig); default: - return new NoOpExtension(); + return new NoOpExtension(settings, columnsConfig); } } } diff --git a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtension.java b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtension.java index 8146c6cec..6d15f837b 100644 --- a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtension.java +++ b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtension.java @@ -1,7 +1,14 @@ package fr.xephi.authme.datasource.mysqlextensions; +import fr.xephi.authme.datasource.Columns; +import fr.xephi.authme.settings.Settings; + /** * Extension implementation that does not do anything. */ class NoOpExtension extends MySqlExtension { + + NoOpExtension(Settings settings, Columns col) { + super(settings, col); + } } diff --git a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/PhpBbExtension.java b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/PhpBbExtension.java index 33a6a88f5..d78aded15 100644 --- a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/PhpBbExtension.java +++ b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/PhpBbExtension.java @@ -3,42 +3,32 @@ package fr.xephi.authme.datasource.mysqlextensions; import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.datasource.Columns; import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.settings.properties.DatabaseSettings; import fr.xephi.authme.settings.properties.HooksSettings; import java.sql.Connection; import java.sql.PreparedStatement; -import java.sql.ResultSet; import java.sql.SQLException; +import java.util.OptionalInt; /** * Extensions for phpBB when MySQL is used as data source. */ class PhpBbExtension extends MySqlExtension { - private final Columns col; - private final String tableName; private final String phpBbPrefix; private final int phpBbGroup; PhpBbExtension(Settings settings, Columns col) { - this.col = col; - this.tableName = settings.getProperty(DatabaseSettings.MYSQL_TABLE); + super(settings, col); this.phpBbPrefix = settings.getProperty(HooksSettings.PHPBB_TABLE_PREFIX); this.phpBbGroup = settings.getProperty(HooksSettings.PHPBB_ACTIVATED_GROUP_ID); } @Override public void saveAuth(PlayerAuth auth, Connection con) throws SQLException { - String sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst = con.prepareStatement(sql)) { - pst.setString(1, auth.getNickname()); - try (ResultSet rs = pst.executeQuery()) { - if (rs.next()) { - int id = rs.getInt(col.ID); - updateSpecificsOnSave(id, auth.getNickname(), con); - } - } + OptionalInt authId = retrieveIdFromTable(auth.getNickname(), con); + if (authId.isPresent()) { + updateSpecificsOnSave(authId.getAsInt(), auth.getNickname(), con); } } diff --git a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/WordpressExtension.java b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/WordpressExtension.java index 64e6a13f2..22ef97f3c 100644 --- a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/WordpressExtension.java +++ b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/WordpressExtension.java @@ -3,105 +3,100 @@ package fr.xephi.authme.datasource.mysqlextensions; import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.datasource.Columns; import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.settings.properties.DatabaseSettings; import fr.xephi.authme.settings.properties.HooksSettings; import java.sql.Connection; import java.sql.PreparedStatement; -import java.sql.ResultSet; import java.sql.SQLException; +import java.util.OptionalInt; /** * MySQL extensions for Wordpress. */ class WordpressExtension extends MySqlExtension { - private final Columns col; - private final String tableName; private final String wordpressPrefix; WordpressExtension(Settings settings, Columns col) { - this.col = col; - this.tableName = settings.getProperty(DatabaseSettings.MYSQL_TABLE); + super(settings, col); this.wordpressPrefix = settings.getProperty(HooksSettings.WORDPRESS_TABLE_PREFIX); } @Override public void saveAuth(PlayerAuth auth, Connection con) throws SQLException { - try (PreparedStatement pst = con.prepareStatement("SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;")) { - pst.setString(1, auth.getNickname()); - try (ResultSet rs = pst.executeQuery()) { - if (rs.next()) { - int id = rs.getInt(col.ID); - String sql = "INSERT INTO " + wordpressPrefix + "usermeta (user_id, meta_key, meta_value) VALUES (?,?,?)"; - try (PreparedStatement pst2 = con.prepareStatement(sql)) { - // First Name - pst2.setInt(1, id); - pst2.setString(2, "first_name"); - pst2.setString(3, ""); - pst2.addBatch(); - // Last Name - pst2.setInt(1, id); - pst2.setString(2, "last_name"); - pst2.setString(3, ""); - pst2.addBatch(); - // Nick Name - pst2.setInt(1, id); - pst2.setString(2, "nickname"); - pst2.setString(3, auth.getNickname()); - pst2.addBatch(); - // Description - pst2.setInt(1, id); - pst2.setString(2, "description"); - pst2.setString(3, ""); - pst2.addBatch(); - // Rich_Editing - pst2.setInt(1, id); - pst2.setString(2, "rich_editing"); - pst2.setString(3, "true"); - pst2.addBatch(); - // Comments_Shortcuts - pst2.setInt(1, id); - pst2.setString(2, "comment_shortcuts"); - pst2.setString(3, "false"); - pst2.addBatch(); - // admin_color - pst2.setInt(1, id); - pst2.setString(2, "admin_color"); - pst2.setString(3, "fresh"); - pst2.addBatch(); - // use_ssl - pst2.setInt(1, id); - pst2.setString(2, "use_ssl"); - pst2.setString(3, "0"); - pst2.addBatch(); - // show_admin_bar_front - pst2.setInt(1, id); - pst2.setString(2, "show_admin_bar_front"); - pst2.setString(3, "true"); - pst2.addBatch(); - // wp_capabilities - pst2.setInt(1, id); - pst2.setString(2, wordpressPrefix + "capabilities"); - pst2.setString(3, "a:1:{s:10:\"subscriber\";b:1;}"); - pst2.addBatch(); - // wp_user_level - pst2.setInt(1, id); - pst2.setString(2, wordpressPrefix + "user_level"); - pst2.setString(3, "0"); - pst2.addBatch(); - // default_password_nag - pst2.setInt(1, id); - pst2.setString(2, "default_password_nag"); - pst2.setString(3, ""); - pst2.addBatch(); + OptionalInt authId = retrieveIdFromTable(auth.getNickname(), con); + if (authId.isPresent()) { + saveSpecifics(auth, authId.getAsInt(), con); + } + } - // Execute queries - pst2.executeBatch(); - pst2.clearBatch(); - } - } - } + private void saveSpecifics(PlayerAuth auth, int id, Connection con) throws SQLException { + String sql = "INSERT INTO " + wordpressPrefix + "usermeta (user_id, meta_key, meta_value) VALUES (?,?,?)"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + // First Name + pst.setInt(1, id); + pst.setString(2, "first_name"); + pst.setString(3, ""); + pst.addBatch(); + // Last Name + pst.setInt(1, id); + pst.setString(2, "last_name"); + pst.setString(3, ""); + pst.addBatch(); + // Nick Name + pst.setInt(1, id); + pst.setString(2, "nickname"); + pst.setString(3, auth.getNickname()); + pst.addBatch(); + // Description + pst.setInt(1, id); + pst.setString(2, "description"); + pst.setString(3, ""); + pst.addBatch(); + // Rich_Editing + pst.setInt(1, id); + pst.setString(2, "rich_editing"); + pst.setString(3, "true"); + pst.addBatch(); + // Comments_Shortcuts + pst.setInt(1, id); + pst.setString(2, "comment_shortcuts"); + pst.setString(3, "false"); + pst.addBatch(); + // admin_color + pst.setInt(1, id); + pst.setString(2, "admin_color"); + pst.setString(3, "fresh"); + pst.addBatch(); + // use_ssl + pst.setInt(1, id); + pst.setString(2, "use_ssl"); + pst.setString(3, "0"); + pst.addBatch(); + // show_admin_bar_front + pst.setInt(1, id); + pst.setString(2, "show_admin_bar_front"); + pst.setString(3, "true"); + pst.addBatch(); + // wp_capabilities + pst.setInt(1, id); + pst.setString(2, wordpressPrefix + "capabilities"); + pst.setString(3, "a:1:{s:10:\"subscriber\";b:1;}"); + pst.addBatch(); + // wp_user_level + pst.setInt(1, id); + pst.setString(2, wordpressPrefix + "user_level"); + pst.setString(3, "0"); + pst.addBatch(); + // default_password_nag + pst.setInt(1, id); + pst.setString(2, "default_password_nag"); + pst.setString(3, ""); + pst.addBatch(); + + // Execute queries + pst.executeBatch(); + pst.clearBatch(); } } } diff --git a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/XfBcryptExtension.java b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/XfBcryptExtension.java index de8ffb350..46dd41ef9 100644 --- a/src/main/java/fr/xephi/authme/datasource/mysqlextensions/XfBcryptExtension.java +++ b/src/main/java/fr/xephi/authme/datasource/mysqlextensions/XfBcryptExtension.java @@ -5,7 +5,6 @@ import fr.xephi.authme.datasource.Columns; import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.security.crypts.XfBCrypt; import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.settings.properties.DatabaseSettings; import fr.xephi.authme.settings.properties.HooksSettings; import java.sql.Blob; @@ -13,35 +12,27 @@ import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; +import java.util.OptionalInt; /** * Extension for XFBCRYPT. */ class XfBcryptExtension extends MySqlExtension { - private final Columns col; - private final String tableName; private final String xfPrefix; private final int xfGroup; XfBcryptExtension(Settings settings, Columns col) { - this.col = col; - this.tableName = settings.getProperty(DatabaseSettings.MYSQL_TABLE); + super(settings, col); this.xfPrefix = settings.getProperty(HooksSettings.XF_TABLE_PREFIX); this.xfGroup = settings.getProperty(HooksSettings.XF_ACTIVATED_GROUP_ID); } @Override public void saveAuth(PlayerAuth auth, Connection con) throws SQLException { - String sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst = con.prepareStatement(sql)) { - pst.setString(1, auth.getNickname()); - try (ResultSet rs = pst.executeQuery()) { - if (rs.next()) { - int id = rs.getInt(col.ID); - updateXenforoTablesOnSave(auth, id, con); - } - } + OptionalInt authId = retrieveIdFromTable(auth.getNickname(), con); + if (authId.isPresent()) { + updateXenforoTablesOnSave(auth, authId.getAsInt(), con); } } @@ -110,50 +101,39 @@ class XfBcryptExtension extends MySqlExtension { @Override public void changePassword(String user, HashedPassword password, Connection con) throws SQLException { - String sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; - try (PreparedStatement pst = con.prepareStatement(sql)) { - pst.setString(1, user); - try (ResultSet rs = pst.executeQuery()) { - if (rs.next()) { - int id = rs.getInt(col.ID); - // Insert password in the correct table - // TODO #1255: Close these statements with try-catch - sql = "UPDATE " + xfPrefix + "user_authenticate SET data=? WHERE " + col.ID + "=?;"; - PreparedStatement pst2 = con.prepareStatement(sql); - String serializedHash = XfBCrypt.serializeHash(password.getHash()); - byte[] bytes = serializedHash.getBytes(); - Blob blob = con.createBlob(); - blob.setBytes(1, bytes); - pst2.setBlob(1, blob); - pst2.setInt(2, id); - pst2.executeUpdate(); - pst2.close(); - // ... - sql = "UPDATE " + xfPrefix + "user_authenticate SET scheme_class=? WHERE " + col.ID + "=?;"; - pst2 = con.prepareStatement(sql); - pst2.setString(1, XfBCrypt.SCHEME_CLASS); - pst2.setInt(2, id); - pst2.executeUpdate(); - pst2.close(); - } + OptionalInt authId = retrieveIdFromTable(user, con); + if (authId.isPresent()) { + final int id = authId.getAsInt(); + // Insert password in the correct table + String sql = "UPDATE " + xfPrefix + "user_authenticate SET data=? WHERE " + col.ID + "=?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + String serializedHash = XfBCrypt.serializeHash(password.getHash()); + byte[] bytes = serializedHash.getBytes(); + Blob blob = con.createBlob(); + blob.setBytes(1, bytes); + pst.setBlob(1, blob); + pst.setInt(2, id); + pst.executeUpdate(); + } + + // ... + sql = "UPDATE " + xfPrefix + "user_authenticate SET scheme_class=? WHERE " + col.ID + "=?;"; + try (PreparedStatement pst = con.prepareStatement(sql)) { + pst.setString(1, XfBCrypt.SCHEME_CLASS); + pst.setInt(2, id); + pst.executeUpdate(); } } } @Override public void removeAuth(String user, Connection con) throws SQLException { - String sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;"; - try (PreparedStatement xfSelect = con.prepareStatement(sql)) { - xfSelect.setString(1, user); - try (ResultSet rs = xfSelect.executeQuery()) { - if (rs.next()) { - int id = rs.getInt(col.ID); - sql = "DELETE FROM " + xfPrefix + "user_authenticate WHERE " + col.ID + "=?;"; - try (PreparedStatement xfDelete = con.prepareStatement(sql)) { - xfDelete.setInt(1, id); - xfDelete.executeUpdate(); - } - } + OptionalInt authId = retrieveIdFromTable(user, con); + if (authId.isPresent()) { + String sql = "DELETE FROM " + xfPrefix + "user_authenticate WHERE " + col.ID + "=?;"; + try (PreparedStatement xfDelete = con.prepareStatement(sql)) { + xfDelete.setInt(1, authId.getAsInt()); + xfDelete.executeUpdate(); } } } diff --git a/src/test/java/fr/xephi/authme/ClassesConsistencyTest.java b/src/test/java/fr/xephi/authme/ClassesConsistencyTest.java index a8b3a0fa6..651295164 100644 --- a/src/test/java/fr/xephi/authme/ClassesConsistencyTest.java +++ b/src/test/java/fr/xephi/authme/ClassesConsistencyTest.java @@ -5,6 +5,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import fr.xephi.authme.datasource.Columns; +import fr.xephi.authme.datasource.mysqlextensions.MySqlExtension; import fr.xephi.authme.initialization.HasCleanup; import fr.xephi.authme.process.register.executors.RegistrationMethod; import fr.xephi.authme.security.crypts.Whirlpool; @@ -55,6 +56,7 @@ public class ClassesConsistencyTest { /** Classes excluded from the field visibility test. */ private static final Set> CLASSES_EXCLUDED_FROM_VISIBILITY_TEST = ImmutableSet.of( Whirlpool.class, // not our implementation, so we don't touch it + MySqlExtension.class, // has immutable protected fields used by all children Columns.class // uses non-static String constants, which is safe ); From efc06ef2a60f61e0022ea3401e7f54444a25fef3 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 23 Jul 2017 17:29:36 +0200 Subject: [PATCH 4/5] #1255 Create resource-closing tests for the MySQL extensions - Remove test runs with different hash algorithms in the abstract super class - Create resource-closing tests for the new extension classes --- .../AbstractResourceClosingTest.java | 86 +++++-------------- ...tractSqlDataSourceResourceClosingTest.java | 85 ++++++++++++++++++ .../datasource/MySqlResourceClosingTest.java | 2 +- .../datasource/SQLiteResourceClosingTest.java | 2 +- ...ractMySqlExtensionResourceClosingTest.java | 60 +++++++++++++ .../Ipb4ExtensionResourceClosingTest.java | 21 +++++ .../mysqlextensions/NoOpExtensionTest.java | 58 +++++++++++++ .../PhpBbExtensionResourceClosingTest.java | 21 +++++ ...WordpressExtensionResourceClosingTest.java | 21 +++++ .../XfBcryptExtensionResourceClosingTest.java | 21 +++++ 10 files changed, 309 insertions(+), 68 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/datasource/AbstractSqlDataSourceResourceClosingTest.java create mode 100644 src/test/java/fr/xephi/authme/datasource/mysqlextensions/AbstractMySqlExtensionResourceClosingTest.java create mode 100644 src/test/java/fr/xephi/authme/datasource/mysqlextensions/Ipb4ExtensionResourceClosingTest.java create mode 100644 src/test/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtensionTest.java create mode 100644 src/test/java/fr/xephi/authme/datasource/mysqlextensions/PhpBbExtensionResourceClosingTest.java create mode 100644 src/test/java/fr/xephi/authme/datasource/mysqlextensions/WordpressExtensionResourceClosingTest.java create mode 100644 src/test/java/fr/xephi/authme/datasource/mysqlextensions/XfBcryptExtensionResourceClosingTest.java diff --git a/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java index 845923b96..5b2dca588 100644 --- a/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java +++ b/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java @@ -1,14 +1,10 @@ package fr.xephi.authme.datasource; -import ch.jalu.configme.properties.Property; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; import fr.xephi.authme.TestHelper; import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.security.crypts.HashedPassword; -import fr.xephi.authme.settings.Settings; -import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; @@ -33,7 +29,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; @@ -42,36 +37,29 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; /** - * Test class which runs through a datasource implementation and verifies that all + * Test class which runs through objects interacting with a database and verifies that all * instances of {@link AutoCloseable} that are created in the calls are closed again. *

* Instead of an actual connection to a datasource, we pass a mock Connection object * which is set to create additional mocks on demand for Statement and ResultSet objects. * This test ensures that all such objects that are created will be closed again by * keeping a list of mocks ({@link #closeables}) and then verifying that all have been - * closed {@link #verifyHaveMocksBeenClosed()}. + * closed ({@link #verifyHaveMocksBeenClosed()}). */ @RunWith(Parameterized.class) public abstract class AbstractResourceClosingTest { - /** List of DataSource method names not to test. */ - private static final Set IGNORED_METHODS = ImmutableSet.of("reload", "close", "getType"); - /** Collection of values to use to call methods with the parameters they expect. */ private static final Map, Object> PARAM_VALUES = getDefaultParameters(); - /** Mock of a settings instance. */ - private static Settings settings; - - /** The datasource to test. */ - private DataSource dataSource; - /** The DataSource method to test. */ private Method method; /** Keeps track of the closeables which are created during the tested call. */ private List closeables = new ArrayList<>(); + private boolean hasCreatedConnection = false; + /** * Constructor for the test instance verifying the given method. * @@ -84,65 +72,22 @@ public abstract class AbstractResourceClosingTest { this.method = method; } - /** Initialize the settings mock and makes it return the default of any given property by default. */ - @SuppressWarnings("unchecked") @BeforeClass - public static void initializeSettings() { - settings = mock(Settings.class); - given(settings.getProperty(any(Property.class))).willAnswer(new Answer() { - @Override - public Object answer(InvocationOnMock invocation) { - return ((Property) invocation.getArguments()[0]).getDefaultValue(); - } - }); + public static void initializeLogger() { TestHelper.setupLogger(); } - /** Initialize the dataSource implementation to test based on a mock connection. */ - @Before - public void setUpMockConnection() throws Exception { - Connection connection = initConnection(); - dataSource = createDataSource(settings, connection); - } - /** * The actual test -- executes the method given through the constructor and then verifies that all * AutoCloseable mocks it constructed have been closed. */ @Test public void shouldCloseResources() throws IllegalAccessException, InvocationTargetException { - method.invoke(dataSource, buildParamListForMethod(method)); + method.invoke(getObjectUnderTest(), buildParamListForMethod(method)); verifyHaveMocksBeenClosed(); } - /** - * Initialization method -- provides the parameters to run the test with by scanning all DataSource methods. - * - * @return Test parameters - */ - @Parameterized.Parameters(name = "{1}") - public static Collection data() { - List methods = getDataSourceMethods(); - List data = new ArrayList<>(); - for (Method method : methods) { - data.add(new Object[]{method, method.getName()}); - } - return data; - } - - /* Create a DataSource instance with the given mock settings and mock connection. */ - protected abstract DataSource createDataSource(Settings settings, Connection connection) throws Exception; - - /* Get all methods of the DataSource interface, minus the ones in the ignored list. */ - private static List getDataSourceMethods() { - List publicMethods = new ArrayList<>(); - for (Method method : DataSource.class.getDeclaredMethods()) { - if (!IGNORED_METHODS.contains(method.getName())) { - publicMethods.add(method); - } - } - return publicMethods; - } + protected abstract Object getObjectUnderTest(); /** * Verify that all AutoCloseables that have been created during the method execution have been closed. @@ -166,7 +111,7 @@ public abstract class AbstractResourceClosingTest { * @param method The method to create a valid parameter list for * @return Parameter list to invoke the given method with */ - private static Object[] buildParamListForMethod(Method method) { + private Object[] buildParamListForMethod(Method method) { List params = new ArrayList<>(); int index = 0; for (Class paramType : method.getParameterTypes()) { @@ -174,7 +119,7 @@ public abstract class AbstractResourceClosingTest { // but that is a sensible assumption and makes our life much easier later on when juggling with Type Object param = Collection.class.isAssignableFrom(paramType) ? getTypedCollection(method.getGenericParameterTypes()[index]) - : PARAM_VALUES.get(paramType); + : getMethodParameter(paramType); Preconditions.checkNotNull(param, "No param type for " + paramType); params.add(param); ++index; @@ -182,6 +127,15 @@ public abstract class AbstractResourceClosingTest { return params.toArray(); } + private Object getMethodParameter(Class paramType) { + if (paramType.equals(Connection.class)) { + Preconditions.checkArgument(!hasCreatedConnection, "A Connection object was already created in this test run"); + hasCreatedConnection = true; + return initConnection(); + } + return PARAM_VALUES.get(paramType); + } + /** * Return a collection of the required type with some test elements that correspond to the * collection's generic type. @@ -230,11 +184,11 @@ public abstract class AbstractResourceClosingTest { // Mock initialization // --------------------- /** - * Initialize the connection mock which produces additional AutoCloseable mocks and records them. + * Initializes the connection mock which produces additional AutoCloseable mocks and records them. * * @return Connection mock */ - private Connection initConnection() { + protected Connection initConnection() { Connection connection = mock(Connection.class); try { given(connection.prepareStatement(anyString())).willAnswer(preparedStatementAnswer()); diff --git a/src/test/java/fr/xephi/authme/datasource/AbstractSqlDataSourceResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/AbstractSqlDataSourceResourceClosingTest.java new file mode 100644 index 000000000..ea92e4a0c --- /dev/null +++ b/src/test/java/fr/xephi/authme/datasource/AbstractSqlDataSourceResourceClosingTest.java @@ -0,0 +1,85 @@ +package fr.xephi.authme.datasource; + +import ch.jalu.configme.properties.Property; +import com.google.common.collect.ImmutableSet; +import fr.xephi.authme.TestHelper; +import fr.xephi.authme.settings.Settings; +import org.junit.BeforeClass; +import org.junit.runners.Parameterized; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +import java.lang.reflect.Method; +import java.sql.Connection; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Set; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Resource-closing test for SQL data sources. + */ +public abstract class AbstractSqlDataSourceResourceClosingTest extends AbstractResourceClosingTest { + + /** List of DataSource method names not to test. */ + private static final Set IGNORED_METHODS = ImmutableSet.of("reload", "getType"); + + private static Settings settings; + + AbstractSqlDataSourceResourceClosingTest(Method method, String name) { + super(method, name); + } + + @BeforeClass + public static void initializeSettings() { + settings = mock(Settings.class); + given(settings.getProperty(any(Property.class))).willAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocation) { + return ((Property) invocation.getArguments()[0]).getDefaultValue(); + } + }); + TestHelper.setupLogger(); + } + + protected DataSource getObjectUnderTest() { + try { + return createDataSource(settings, initConnection()); + } catch (Exception e) { + throw new IllegalStateException(e); + } + } + + /* Create a DataSource instance with the given mock settings and mock connection. */ + protected abstract DataSource createDataSource(Settings settings, Connection connection) throws Exception; + + /** + * Initialization method -- provides the parameters to run the test with by scanning all DataSource methods. + * + * @return Test parameters + */ + @Parameterized.Parameters(name = "{1}") + public static Collection data() { + List methods = getDataSourceMethods(); + List data = new ArrayList<>(); + for (Method method : methods) { + data.add(new Object[]{method, method.getName()}); + } + return data; + } + + /* Get all methods of the DataSource interface, minus the ones in the ignored list. */ + private static List getDataSourceMethods() { + List publicMethods = new ArrayList<>(); + for (Method method : DataSource.class.getDeclaredMethods()) { + if (!IGNORED_METHODS.contains(method.getName())) { + publicMethods.add(method); + } + } + return publicMethods; + } +} diff --git a/src/test/java/fr/xephi/authme/datasource/MySqlResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/MySqlResourceClosingTest.java index f41a21641..3500560b5 100644 --- a/src/test/java/fr/xephi/authme/datasource/MySqlResourceClosingTest.java +++ b/src/test/java/fr/xephi/authme/datasource/MySqlResourceClosingTest.java @@ -15,7 +15,7 @@ import static org.mockito.Mockito.mock; /** * Resource closing test for {@link MySQL}. */ -public class MySqlResourceClosingTest extends AbstractResourceClosingTest { +public class MySqlResourceClosingTest extends AbstractSqlDataSourceResourceClosingTest { public MySqlResourceClosingTest(Method method, String name) { super(method, name); diff --git a/src/test/java/fr/xephi/authme/datasource/SQLiteResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/SQLiteResourceClosingTest.java index 69eade8f1..c85734cbb 100644 --- a/src/test/java/fr/xephi/authme/datasource/SQLiteResourceClosingTest.java +++ b/src/test/java/fr/xephi/authme/datasource/SQLiteResourceClosingTest.java @@ -8,7 +8,7 @@ import java.sql.Connection; /** * Resource closing test for {@link SQLite}. */ -public class SQLiteResourceClosingTest extends AbstractResourceClosingTest { +public class SQLiteResourceClosingTest extends AbstractSqlDataSourceResourceClosingTest { public SQLiteResourceClosingTest(Method method, String name) { super(method, name); diff --git a/src/test/java/fr/xephi/authme/datasource/mysqlextensions/AbstractMySqlExtensionResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/AbstractMySqlExtensionResourceClosingTest.java new file mode 100644 index 000000000..6425e33b6 --- /dev/null +++ b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/AbstractMySqlExtensionResourceClosingTest.java @@ -0,0 +1,60 @@ +package fr.xephi.authme.datasource.mysqlextensions; + +import ch.jalu.configme.properties.Property; +import fr.xephi.authme.datasource.AbstractResourceClosingTest; +import fr.xephi.authme.datasource.Columns; +import fr.xephi.authme.settings.Settings; +import org.junit.BeforeClass; +import org.junit.runners.Parameterized; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Checks that SQL resources are closed properly in {@link MySqlExtension} implementations. + */ +public abstract class AbstractMySqlExtensionResourceClosingTest extends AbstractResourceClosingTest { + + private static Settings settings; + private static Columns columns; + + public AbstractMySqlExtensionResourceClosingTest(Method method, String name) { + super(method, name); + } + + @BeforeClass + public static void initSettings() { + settings = mock(Settings.class); + given(settings.getProperty(any(Property.class))).willAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocation) { + return ((Property) invocation.getArguments()[0]).getDefaultValue(); + } + }); + columns = new Columns(settings); + } + + @Override + protected MySqlExtension getObjectUnderTest() { + return createExtension(settings, columns); + } + + protected abstract MySqlExtension createExtension(Settings settings, Columns columns); + + @Parameterized.Parameters(name = "{1}") + public static List createParameters() { + return Arrays.stream(MySqlExtension.class.getDeclaredMethods()) + .filter(m -> Modifier.isPublic(m.getModifiers())) + .map(m -> new Object[]{m, m.getName()}) + .collect(Collectors.toList()); + } +} diff --git a/src/test/java/fr/xephi/authme/datasource/mysqlextensions/Ipb4ExtensionResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/Ipb4ExtensionResourceClosingTest.java new file mode 100644 index 000000000..8629a6a7d --- /dev/null +++ b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/Ipb4ExtensionResourceClosingTest.java @@ -0,0 +1,21 @@ +package fr.xephi.authme.datasource.mysqlextensions; + +import fr.xephi.authme.datasource.Columns; +import fr.xephi.authme.settings.Settings; + +import java.lang.reflect.Method; + +/** + * Resource closing test for {@link Ipb4Extension}. + */ +public class Ipb4ExtensionResourceClosingTest extends AbstractMySqlExtensionResourceClosingTest { + + public Ipb4ExtensionResourceClosingTest(Method method, String name) { + super(method, name); + } + + @Override + protected MySqlExtension createExtension(Settings settings, Columns columns) { + return new Ipb4Extension(settings, columns); + } +} diff --git a/src/test/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtensionTest.java b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtensionTest.java new file mode 100644 index 000000000..ce7f4a204 --- /dev/null +++ b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtensionTest.java @@ -0,0 +1,58 @@ +package fr.xephi.authme.datasource.mysqlextensions; + +import ch.jalu.configme.properties.Property; +import fr.xephi.authme.data.auth.PlayerAuth; +import fr.xephi.authme.datasource.Columns; +import fr.xephi.authme.security.crypts.HashedPassword; +import fr.xephi.authme.settings.Settings; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; + +import java.sql.Connection; +import java.sql.SQLException; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyZeroInteractions; + +/** + * Test for {@link NoOpExtension}. + */ +@RunWith(MockitoJUnitRunner.class) +public class NoOpExtensionTest { + + private NoOpExtension extension; + + @Before + @SuppressWarnings("unchecked") + public void createExtension() { + Settings settings = mock(Settings.class); + given(settings.getProperty(any(Property.class))).willAnswer( + invocation -> ((Property) invocation.getArgument(0)).getDefaultValue()); + Columns columns = new Columns(settings); + extension = new NoOpExtension(settings, columns); + } + + @Test + public void shouldNotHaveAnyInteractionsWithConnection() throws SQLException { + // given + Connection connection = mock(Connection.class); + PlayerAuth auth = mock(PlayerAuth.class); + int id = 3; + String name = "Bobby"; + HashedPassword password = new HashedPassword("test", "toast"); + + + // when + extension.extendAuth(auth, id, connection); + extension.changePassword(name, password, connection); + extension.removeAuth(name, connection); + extension.saveAuth(auth, connection); + + // then + verifyZeroInteractions(connection, auth); + } +} diff --git a/src/test/java/fr/xephi/authme/datasource/mysqlextensions/PhpBbExtensionResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/PhpBbExtensionResourceClosingTest.java new file mode 100644 index 000000000..d1f43e1f4 --- /dev/null +++ b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/PhpBbExtensionResourceClosingTest.java @@ -0,0 +1,21 @@ +package fr.xephi.authme.datasource.mysqlextensions; + +import fr.xephi.authme.datasource.Columns; +import fr.xephi.authme.settings.Settings; + +import java.lang.reflect.Method; + +/** + * Resource closing test for {@link PhpBbExtension}. + */ +public class PhpBbExtensionResourceClosingTest extends AbstractMySqlExtensionResourceClosingTest { + + public PhpBbExtensionResourceClosingTest(Method method, String name) { + super(method, name); + } + + @Override + protected MySqlExtension createExtension(Settings settings, Columns columns) { + return new PhpBbExtension(settings, columns); + } +} diff --git a/src/test/java/fr/xephi/authme/datasource/mysqlextensions/WordpressExtensionResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/WordpressExtensionResourceClosingTest.java new file mode 100644 index 000000000..1ee141ffe --- /dev/null +++ b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/WordpressExtensionResourceClosingTest.java @@ -0,0 +1,21 @@ +package fr.xephi.authme.datasource.mysqlextensions; + +import fr.xephi.authme.datasource.Columns; +import fr.xephi.authme.settings.Settings; + +import java.lang.reflect.Method; + +/** + * Resource closing test for {@link WordpressExtension}. + */ +public class WordpressExtensionResourceClosingTest extends AbstractMySqlExtensionResourceClosingTest { + + public WordpressExtensionResourceClosingTest(Method method, String name) { + super(method, name); + } + + @Override + protected MySqlExtension createExtension(Settings settings, Columns columns) { + return new WordpressExtension(settings, columns); + } +} diff --git a/src/test/java/fr/xephi/authme/datasource/mysqlextensions/XfBcryptExtensionResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/XfBcryptExtensionResourceClosingTest.java new file mode 100644 index 000000000..7da4fb198 --- /dev/null +++ b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/XfBcryptExtensionResourceClosingTest.java @@ -0,0 +1,21 @@ +package fr.xephi.authme.datasource.mysqlextensions; + +import fr.xephi.authme.datasource.Columns; +import fr.xephi.authme.settings.Settings; + +import java.lang.reflect.Method; + +/** + * Resource closing test for {@link XfBcryptExtension}. + */ +public class XfBcryptExtensionResourceClosingTest extends AbstractMySqlExtensionResourceClosingTest { + + public XfBcryptExtensionResourceClosingTest(Method method, String name) { + super(method, name); + } + + @Override + protected MySqlExtension createExtension(Settings settings, Columns columns) { + return new XfBcryptExtension(settings, columns); + } +} From 027d0fc7750545618f9d625b89d39b39470ef339 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 23 Jul 2017 18:00:51 +0200 Subject: [PATCH 5/5] Create TestHelper method to make Settings mock return defaults --- src/test/java/fr/xephi/authme/TestHelper.java | 13 +++++++++++++ ...AbstractSqlDataSourceResourceClosingTest.java | 12 +----------- .../authme/datasource/MySqlIntegrationTest.java | 10 +--------- .../authme/datasource/SQLiteIntegrationTest.java | 11 +---------- ...bstractMySqlExtensionResourceClosingTest.java | 13 ++----------- .../mysqlextensions/NoOpExtensionTest.java | 8 ++------ .../EncryptionMethodInfoGatherer.java | 16 ++-------------- 7 files changed, 22 insertions(+), 61 deletions(-) diff --git a/src/test/java/fr/xephi/authme/TestHelper.java b/src/test/java/fr/xephi/authme/TestHelper.java index ee7aaa5e0..5a60bc1e6 100644 --- a/src/test/java/fr/xephi/authme/TestHelper.java +++ b/src/test/java/fr/xephi/authme/TestHelper.java @@ -1,6 +1,8 @@ package fr.xephi.authme; +import ch.jalu.configme.properties.Property; import fr.xephi.authme.service.BukkitService; +import fr.xephi.authme.settings.Settings; import org.bukkit.entity.Player; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; @@ -18,6 +20,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.logging.Logger; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -206,4 +209,14 @@ public final class TestHelper { given(player.getAddress()).willReturn(inetSocketAddress); } + /** + * Configures the Settings mock to return the property's default value for any given property. + * + * @param settings the settings mock + */ + @SuppressWarnings("unchecked") + public static void returnDefaultsForAllProperties(Settings settings) { + given(settings.getProperty(any(Property.class))) + .willAnswer(invocation -> ((Property) invocation.getArgument(0)).getDefaultValue()); + } } diff --git a/src/test/java/fr/xephi/authme/datasource/AbstractSqlDataSourceResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/AbstractSqlDataSourceResourceClosingTest.java index ea92e4a0c..edb6d4f68 100644 --- a/src/test/java/fr/xephi/authme/datasource/AbstractSqlDataSourceResourceClosingTest.java +++ b/src/test/java/fr/xephi/authme/datasource/AbstractSqlDataSourceResourceClosingTest.java @@ -1,13 +1,10 @@ package fr.xephi.authme.datasource; -import ch.jalu.configme.properties.Property; import com.google.common.collect.ImmutableSet; import fr.xephi.authme.TestHelper; import fr.xephi.authme.settings.Settings; import org.junit.BeforeClass; import org.junit.runners.Parameterized; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import java.lang.reflect.Method; import java.sql.Connection; @@ -16,8 +13,6 @@ import java.util.Collection; import java.util.List; import java.util.Set; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; /** @@ -37,12 +32,7 @@ public abstract class AbstractSqlDataSourceResourceClosingTest extends AbstractR @BeforeClass public static void initializeSettings() { settings = mock(Settings.class); - given(settings.getProperty(any(Property.class))).willAnswer(new Answer() { - @Override - public Object answer(InvocationOnMock invocation) { - return ((Property) invocation.getArguments()[0]).getDefaultValue(); - } - }); + TestHelper.returnDefaultsForAllProperties(settings); TestHelper.setupLogger(); } diff --git a/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java b/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java index 62e1ac3ec..9da39a3a6 100644 --- a/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java @@ -11,8 +11,6 @@ import fr.xephi.authme.settings.properties.DatabaseSettings; import org.junit.After; import org.junit.Before; import org.junit.BeforeClass; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import java.io.IOException; import java.nio.file.Files; @@ -42,19 +40,13 @@ public class MySqlIntegrationTest extends AbstractDataSourceIntegrationTest { /** * Set up the settings mock to return specific values for database settings and load {@link #sqlInitialize}. */ - @SuppressWarnings({ "unchecked", "rawtypes" }) @BeforeClass public static void initializeSettings() throws IOException, ClassNotFoundException { // Check that we have an H2 driver Class.forName("org.h2.jdbcx.JdbcDataSource"); settings = mock(Settings.class); - when(settings.getProperty(any(Property.class))).thenAnswer(new Answer() { - @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - return ((Property) invocation.getArguments()[0]).getDefaultValue(); - } - }); + TestHelper.returnDefaultsForAllProperties(settings); extensionsFactory = mock(MySqlExtensionsFactory.class); when(extensionsFactory.buildExtension(any(Columns.class))).thenReturn(mock(MySqlExtension.class)); set(DatabaseSettings.MYSQL_DATABASE, "h2_test"); diff --git a/src/test/java/fr/xephi/authme/datasource/SQLiteIntegrationTest.java b/src/test/java/fr/xephi/authme/datasource/SQLiteIntegrationTest.java index 9ef0c2fa8..6153002a0 100644 --- a/src/test/java/fr/xephi/authme/datasource/SQLiteIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/datasource/SQLiteIntegrationTest.java @@ -9,8 +9,6 @@ import org.junit.After; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import java.io.IOException; import java.nio.file.Files; @@ -22,7 +20,6 @@ import java.sql.Statement; import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.assertThat; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -41,19 +38,13 @@ public class SQLiteIntegrationTest extends AbstractDataSourceIntegrationTest { /** * Set up the settings mock to return specific values for database settings and load {@link #sqlInitialize}. */ - @SuppressWarnings({ "unchecked", "rawtypes" }) @BeforeClass public static void initializeSettings() throws IOException, ClassNotFoundException { // Check that we have an implementation for SQLite Class.forName("org.sqlite.JDBC"); settings = mock(Settings.class); - when(settings.getProperty(any(Property.class))).thenAnswer(new Answer() { - @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - return ((Property) invocation.getArguments()[0]).getDefaultValue(); - } - }); + TestHelper.returnDefaultsForAllProperties(settings); set(DatabaseSettings.MYSQL_DATABASE, "sqlite-test"); set(DatabaseSettings.MYSQL_TABLE, "authme"); TestHelper.setRealLogger(); diff --git a/src/test/java/fr/xephi/authme/datasource/mysqlextensions/AbstractMySqlExtensionResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/AbstractMySqlExtensionResourceClosingTest.java index 6425e33b6..3089e8b99 100644 --- a/src/test/java/fr/xephi/authme/datasource/mysqlextensions/AbstractMySqlExtensionResourceClosingTest.java +++ b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/AbstractMySqlExtensionResourceClosingTest.java @@ -1,13 +1,11 @@ package fr.xephi.authme.datasource.mysqlextensions; -import ch.jalu.configme.properties.Property; +import fr.xephi.authme.TestHelper; import fr.xephi.authme.datasource.AbstractResourceClosingTest; import fr.xephi.authme.datasource.Columns; import fr.xephi.authme.settings.Settings; import org.junit.BeforeClass; import org.junit.runners.Parameterized; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import java.lang.reflect.Method; import java.lang.reflect.Modifier; @@ -15,8 +13,6 @@ import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; /** @@ -34,12 +30,7 @@ public abstract class AbstractMySqlExtensionResourceClosingTest extends Abstract @BeforeClass public static void initSettings() { settings = mock(Settings.class); - given(settings.getProperty(any(Property.class))).willAnswer(new Answer() { - @Override - public Object answer(InvocationOnMock invocation) { - return ((Property) invocation.getArguments()[0]).getDefaultValue(); - } - }); + TestHelper.returnDefaultsForAllProperties(settings); columns = new Columns(settings); } diff --git a/src/test/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtensionTest.java b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtensionTest.java index ce7f4a204..95a5dfa88 100644 --- a/src/test/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtensionTest.java +++ b/src/test/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtensionTest.java @@ -1,6 +1,6 @@ package fr.xephi.authme.datasource.mysqlextensions; -import ch.jalu.configme.properties.Property; +import fr.xephi.authme.TestHelper; import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.datasource.Columns; import fr.xephi.authme.security.crypts.HashedPassword; @@ -13,8 +13,6 @@ import org.mockito.junit.MockitoJUnitRunner; import java.sql.Connection; import java.sql.SQLException; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verifyZeroInteractions; @@ -27,11 +25,9 @@ public class NoOpExtensionTest { private NoOpExtension extension; @Before - @SuppressWarnings("unchecked") public void createExtension() { Settings settings = mock(Settings.class); - given(settings.getProperty(any(Property.class))).willAnswer( - invocation -> ((Property) invocation.getArgument(0)).getDefaultValue()); + TestHelper.returnDefaultsForAllProperties(settings); Columns columns = new Columns(settings); extension = new NoOpExtension(settings, columns); } diff --git a/src/test/java/tools/docs/hashmethods/EncryptionMethodInfoGatherer.java b/src/test/java/tools/docs/hashmethods/EncryptionMethodInfoGatherer.java index 646d8d371..50b4f50d0 100644 --- a/src/test/java/tools/docs/hashmethods/EncryptionMethodInfoGatherer.java +++ b/src/test/java/tools/docs/hashmethods/EncryptionMethodInfoGatherer.java @@ -1,9 +1,9 @@ package tools.docs.hashmethods; -import ch.jalu.configme.properties.Property; import ch.jalu.injector.Injector; import ch.jalu.injector.InjectorBuilder; import com.google.common.collect.ImmutableSet; +import fr.xephi.authme.TestHelper; import fr.xephi.authme.security.HashAlgorithm; import fr.xephi.authme.security.crypts.EncryptionMethod; import fr.xephi.authme.security.crypts.HexSaltedMethod; @@ -11,8 +11,6 @@ import fr.xephi.authme.security.crypts.description.AsciiRestricted; import fr.xephi.authme.security.crypts.description.HasSalt; import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.settings.Settings; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import java.lang.annotation.Annotation; import java.util.HashMap; @@ -20,9 +18,7 @@ import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; /** * Gathers information on {@link EncryptionMethod} implementations based on @@ -140,17 +136,9 @@ public class EncryptionMethodInfoGatherer { return key.cast(map.get(key)); } - @SuppressWarnings("unchecked") private static Injector createInitializer() { Settings settings = mock(Settings.class); - // Return the default value for any property - when(settings.getProperty(any(Property.class))).thenAnswer(new Answer() { - @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - Property property = (Property) invocation.getArguments()[0]; - return property.getDefaultValue(); - } - }); + TestHelper.returnDefaultsForAllProperties(settings); // By passing some bogus "package" to the constructor, the injector will throw if it needs to // instantiate any dependency other than what we provide.