From 7229a8b02b850e38a51dd2d8963938c994cc16bf Mon Sep 17 00:00:00 2001 From: Gabriele C Date: Wed, 18 May 2016 07:49:37 +0200 Subject: [PATCH 01/16] Update exec plugin, the doc task should run manually --- pom.xml | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/pom.xml b/pom.xml index b4bdc34d..c80dcaa1 100644 --- a/pom.xml +++ b/pom.xml @@ -241,7 +241,7 @@ org.codehaus.mojo exec-maven-plugin - 1.4.0 + 1.5.0 test ${project.basedir}/target/test-classes @@ -251,16 +251,6 @@ true - From 92a8a5dd41a77fac334753216555667001c36202 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Wed, 18 May 2016 19:09:25 +0200 Subject: [PATCH 02/16] #704 Remove reloading from hash algorithms - A new instance is created for every hash operation, so reloading will never happen on those classes --- .../java/fr/xephi/authme/security/crypts/BCRYPT.java | 11 +++-------- .../fr/xephi/authme/security/crypts/SALTED2MD5.java | 11 +++-------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/src/main/java/fr/xephi/authme/security/crypts/BCRYPT.java b/src/main/java/fr/xephi/authme/security/crypts/BCRYPT.java index dc1b3676..cda26091 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/BCRYPT.java +++ b/src/main/java/fr/xephi/authme/security/crypts/BCRYPT.java @@ -1,7 +1,6 @@ package fr.xephi.authme.security.crypts; import fr.xephi.authme.ConsoleLogger; -import fr.xephi.authme.initialization.SettingsDependent; import fr.xephi.authme.security.crypts.description.HasSalt; import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.SaltType; @@ -14,13 +13,13 @@ import javax.inject.Inject; @Recommendation(Usage.RECOMMENDED) // provided the salt length is >= 8 @HasSalt(value = SaltType.TEXT) // length depends on the bcryptLog2Rounds setting -public class BCRYPT implements EncryptionMethod, SettingsDependent { +public class BCRYPT implements EncryptionMethod { - private int bCryptLog2Rounds; + private final int bCryptLog2Rounds; @Inject public BCRYPT(NewSetting settings) { - loadSettings(settings); + bCryptLog2Rounds = settings.getProperty(HooksSettings.BCRYPT_LOG2_ROUND); } @Override @@ -54,8 +53,4 @@ public class BCRYPT implements EncryptionMethod, SettingsDependent { return false; } - @Override - public void loadSettings(NewSetting settings) { - bCryptLog2Rounds = settings.getProperty(HooksSettings.BCRYPT_LOG2_ROUND); - } } diff --git a/src/main/java/fr/xephi/authme/security/crypts/SALTED2MD5.java b/src/main/java/fr/xephi/authme/security/crypts/SALTED2MD5.java index a0e92284..a96cc0d5 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/SALTED2MD5.java +++ b/src/main/java/fr/xephi/authme/security/crypts/SALTED2MD5.java @@ -1,6 +1,5 @@ package fr.xephi.authme.security.crypts; -import fr.xephi.authme.initialization.SettingsDependent; import fr.xephi.authme.security.RandomString; import fr.xephi.authme.security.crypts.description.HasSalt; import fr.xephi.authme.security.crypts.description.Recommendation; @@ -15,13 +14,13 @@ import static fr.xephi.authme.security.HashUtils.md5; @Recommendation(Usage.ACCEPTABLE) // presuming that length is something sensible (>= 8) @HasSalt(value = SaltType.TEXT) // length defined by the doubleMd5SaltLength setting -public class SALTED2MD5 extends SeparateSaltMethod implements SettingsDependent { +public class SALTED2MD5 extends SeparateSaltMethod { - private int saltLength; + private final int saltLength; @Inject public SALTED2MD5(NewSetting settings) { - loadSettings(settings); + saltLength = settings.getProperty(SecuritySettings.DOUBLE_MD5_SALT_LENGTH); } @Override @@ -34,9 +33,5 @@ public class SALTED2MD5 extends SeparateSaltMethod implements SettingsDependent return RandomString.generateHex(saltLength); } - @Override - public void loadSettings(NewSetting settings) { - saltLength = settings.getProperty(SecuritySettings.DOUBLE_MD5_SALT_LENGTH); - } } From 14002ee75cc45312ac705b03d8c6131c5d25d847 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Thu, 19 May 2016 19:46:02 +0200 Subject: [PATCH 03/16] #704 Reload settings of ConsoleLogger on /authme reload --- src/main/java/fr/xephi/authme/AuthMe.java | 35 +++++++++---------- .../java/fr/xephi/authme/ConsoleLogger.java | 13 ++++--- .../executable/authme/ReloadCommand.java | 1 + .../executable/authme/ReloadCommandTest.java | 9 +++++ 4 files changed, 35 insertions(+), 23 deletions(-) diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 11001ae7..baeb1bfd 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -44,16 +44,10 @@ import fr.xephi.authme.settings.SettingsMigrationService; import fr.xephi.authme.settings.SpawnLoader; import fr.xephi.authme.settings.properties.DatabaseSettings; import fr.xephi.authme.settings.properties.EmailSettings; - -import static fr.xephi.authme.settings.properties.EmailSettings.MAIL_ACCOUNT; -import static fr.xephi.authme.settings.properties.EmailSettings.MAIL_PASSWORD; -import static fr.xephi.authme.settings.properties.EmailSettings.RECALL_PLAYERS; - import fr.xephi.authme.settings.properties.HooksSettings; import fr.xephi.authme.settings.properties.PluginSettings; import fr.xephi.authme.settings.properties.PurgeSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; -import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.settings.properties.SettingsFieldRetriever; import fr.xephi.authme.settings.propertymap.PropertyMap; import fr.xephi.authme.task.PurgeTask; @@ -64,6 +58,17 @@ import fr.xephi.authme.util.GeoLiteAPI; import fr.xephi.authme.util.MigrationService; import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.Utils; +import org.apache.logging.log4j.LogManager; +import org.bukkit.Bukkit; +import org.bukkit.Location; +import org.bukkit.Server; +import org.bukkit.command.Command; +import org.bukkit.command.CommandSender; +import org.bukkit.entity.Player; +import org.bukkit.plugin.PluginManager; +import org.bukkit.plugin.java.JavaPlugin; +import org.bukkit.scheduler.BukkitScheduler; +import org.bukkit.scheduler.BukkitTask; import java.io.File; import java.sql.SQLException; @@ -76,17 +81,9 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Logger; -import org.apache.logging.log4j.LogManager; -import org.bukkit.Bukkit; -import org.bukkit.Location; -import org.bukkit.Server; -import org.bukkit.command.Command; -import org.bukkit.command.CommandSender; -import org.bukkit.entity.Player; -import org.bukkit.plugin.PluginManager; -import org.bukkit.plugin.java.JavaPlugin; -import org.bukkit.scheduler.BukkitScheduler; -import org.bukkit.scheduler.BukkitTask; +import static fr.xephi.authme.settings.properties.EmailSettings.MAIL_ACCOUNT; +import static fr.xephi.authme.settings.properties.EmailSettings.MAIL_PASSWORD; +import static fr.xephi.authme.settings.properties.EmailSettings.RECALL_PLAYERS; /** * The AuthMe main class. @@ -223,8 +220,8 @@ public class AuthMe extends JavaPlugin { getServer().shutdown(); return; } - ConsoleLogger.setLoggingOptions(newSettings.getProperty(SecuritySettings.USE_LOGGING), - new File(getDataFolder(), "authme.log")); + ConsoleLogger.setLogFile(new File(getDataFolder(), "authme.log")); + ConsoleLogger.setLoggingOptions(newSettings); // Old settings manager if (!loadSettings()) { diff --git a/src/main/java/fr/xephi/authme/ConsoleLogger.java b/src/main/java/fr/xephi/authme/ConsoleLogger.java index acdae309..ebec0291 100644 --- a/src/main/java/fr/xephi/authme/ConsoleLogger.java +++ b/src/main/java/fr/xephi/authme/ConsoleLogger.java @@ -1,6 +1,7 @@ package fr.xephi.authme; import com.google.common.base.Throwables; +import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.util.StringUtils; @@ -21,22 +22,26 @@ public final class ConsoleLogger { private static final String NEW_LINE = System.getProperty("line.separator"); private static final DateFormat DATE_FORMAT = new SimpleDateFormat("[MM-dd HH:mm:ss]"); private static Logger logger; + private static boolean enableDebug = false; private static boolean useLogging = false; private static File logFile; private ConsoleLogger() { - // Service class } public static void setLogger(Logger logger) { ConsoleLogger.logger = logger; } - public static void setLoggingOptions(boolean useLogging, File logFile) { - ConsoleLogger.useLogging = useLogging; + public static void setLogFile(File logFile) { ConsoleLogger.logFile = logFile; } + public static void setLoggingOptions(NewSetting settings) { + ConsoleLogger.useLogging = settings.getProperty(SecuritySettings.USE_LOGGING); + ConsoleLogger.enableDebug = !settings.getProperty(SecuritySettings.REMOVE_SPAM_FROM_CONSOLE); + } + /** * Print an info message. * @@ -50,7 +55,7 @@ public final class ConsoleLogger { } public static void debug(String message) { - if (!AuthMe.getInstance().getSettings().getProperty(SecuritySettings.REMOVE_SPAM_FROM_CONSOLE)) { + if (enableDebug) { logger.fine(message); if (useLogging) { writeLog("Debug: " + message); diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/ReloadCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/ReloadCommand.java index ec709796..48a43f0a 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/ReloadCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/ReloadCommand.java @@ -40,6 +40,7 @@ public class ReloadCommand implements ExecutableCommand { ConsoleLogger.info("Note: cannot change database type during /authme reload"); sender.sendMessage("Note: cannot change database type during /authme reload"); } + ConsoleLogger.setLoggingOptions(settings); initializer.performReloadOnServices(); commandService.send(sender, MessageKey.CONFIG_RELOAD_SUCCESS); } catch (Exception e) { diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/ReloadCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/ReloadCommandTest.java index 5d52bbd8..d9c28b64 100644 --- a/src/test/java/fr/xephi/authme/command/executable/authme/ReloadCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/authme/ReloadCommandTest.java @@ -9,7 +9,9 @@ import fr.xephi.authme.initialization.AuthMeServiceInitializer; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.properties.DatabaseSettings; +import fr.xephi.authme.settings.properties.SecuritySettings; import org.bukkit.command.CommandSender; +import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; @@ -53,6 +55,13 @@ public class ReloadCommandTest { TestHelper.setupLogger(); } + @Before + public void setDefaultSettings() { + // Mock properties retrieved by ConsoleLogger + given(settings.getProperty(SecuritySettings.REMOVE_SPAM_FROM_CONSOLE)).willReturn(false); + given(settings.getProperty(SecuritySettings.USE_LOGGING)).willReturn(false); + } + @Test public void shouldReload() { // given From 383820cd227b5d64238e5ea3e33c5790f48c0c30 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Thu, 19 May 2016 21:44:24 +0200 Subject: [PATCH 04/16] #702 Implement SHA256 php registration - Refactor Bcrypt and Sha256 examples to use common abstract parent class - Implement hashing logic for Sha256 --- .../website_integration/AuthMeController.php | 126 ++++++++++++++++++ samples/website_integration/Bcrypt.php | 20 +++ samples/website_integration/Sha256.php | 48 +++++++ .../bcrypt/integration.php | 107 --------------- .../{bcrypt/form.php => index.php} | 31 +++-- samples/website_integration/sha256/form.php | 52 -------- .../sha256/integration.php | 67 ---------- 7 files changed, 213 insertions(+), 238 deletions(-) create mode 100644 samples/website_integration/AuthMeController.php create mode 100644 samples/website_integration/Bcrypt.php create mode 100644 samples/website_integration/Sha256.php delete mode 100644 samples/website_integration/bcrypt/integration.php rename samples/website_integration/{bcrypt/form.php => index.php} (66%) delete mode 100644 samples/website_integration/sha256/form.php delete mode 100644 samples/website_integration/sha256/integration.php diff --git a/samples/website_integration/AuthMeController.php b/samples/website_integration/AuthMeController.php new file mode 100644 index 00000000..d1e94ddf --- /dev/null +++ b/samples/website_integration/AuthMeController.php @@ -0,0 +1,126 @@ +getHashFromDatabase($username); + if ($hash) { + return $this->isValidPassword($password, $hash); + } + } + return false; + } + + /** + * Returns whether the user exists in the database or not. + * + * @param string $username the username to check + * @return bool true if the user exists; false otherwise + */ + function isUserRegistered($username) { + $mysqli = $this->getAuthmeMySqli(); + if ($mysqli !== null) { + $stmt = $mysqli->prepare('SELECT 1 FROM ' . self::AUTHME_TABLE . ' WHERE username = ?'); + $stmt->bind_param('s', $username); + $stmt->execute(); + return $stmt->fetch(); + } + + // Defensive default to true; we actually don't know + return true; + } + + /** + * Registers a player with the given username. + * + * @param string $username the username to register + * @param string $password the password to associate to the user + * @return bool whether or not the registration was successful + */ + function register($username, $password) { + $mysqli = $this->getAuthmeMySqli(); + if ($mysqli !== null) { + $hash = $this->hash($password); + $stmt = $mysqli->prepare('INSERT INTO ' . self::AUTHME_TABLE . ' (username, realname, password, ip) ' + . 'VALUES (?, ?, ?, ?)'); + $username_low = strtolower($username); + $stmt->bind_param('ssss', $username, $username_low, $hash, $_SERVER['REMOTE_ADDR']); + return $stmt->execute(); + } + return false; + } + + /** + * Hashes the given password. + * + * @param $password string the clear-text password to hash + * @return string the resulting hash + */ + protected abstract function hash($password); + + /** + * Checks whether the given password matches the hash. + * + * @param $password string the clear-text password + * @param $hash string the password hash + * @return boolean true if the password matches, false otherwise + */ + protected abstract function isValidPassword($password, $hash); + + /** + * Returns a connection to the database. + * + * @return mysqli|null the mysqli object or null upon error + */ + private function getAuthmeMySqli() { + // CHANGE YOUR DATABASE DETAILS HERE BELOW: host, user, password, database name + $mysqli = new mysqli('localhost', 'root', '', 'authme'); + if (mysqli_connect_error()) { + printf('Could not connect to AuthMe database. Errno: %d, error: "%s"', + mysqli_connect_errno(), mysqli_connect_error()); + return null; + } + return $mysqli; + } + + /** + * Retrieves the hash associated with the given user from the database. + * + * @param string $username the username whose hash should be retrieved + * @return string|null the hash, or null if unavailable (e.g. username doesn't exist) + */ + private function getHashFromDatabase($username) { + // Add here your database host, username, password and database name + $mysqli = $this->getAuthmeMySqli(); + if ($mysqli !== null) { + $stmt = $mysqli->prepare('SELECT password FROM ' . self::AUTHME_TABLE . ' WHERE username = ?'); + $stmt->bind_param('s', $username); + $stmt->execute(); + $stmt->bind_result($password); + if ($stmt->fetch()) { + return $password; + } + } + return null; + } + +} \ No newline at end of file diff --git a/samples/website_integration/Bcrypt.php b/samples/website_integration/Bcrypt.php new file mode 100644 index 00000000..225baf80 --- /dev/null +++ b/samples/website_integration/Bcrypt.php @@ -0,0 +1,20 @@ +CHARS = self::initRandomChars(); + } + + protected function isValidPassword($password, $hash) { + // $SHA$salt$hash, where hash := sha256(sha256(password) . salt) + $parts = explode('$', $hash); + return count($parts) === 4 && $parts[3] === hash('sha256', hash('sha256', $password) . $parts[2]); + } + + protected function hash($password) { + $salt = $this->generateSalt(); + return '$SHA$' . $salt . '$' . hash('sha256', hash('sha256', $password) . $salt); + } + + /** + * @return string randomly generated salt + */ + private function generateSalt() { + $maxCharIndex = count($this->CHARS) - 1; + $salt = ''; + for ($i = 0; $i < self::SALT_LENGTH; ++$i) { + $salt .= $this->CHARS[mt_rand(0, $maxCharIndex)]; + } + return $salt; + } + + private static function initRandomChars() { + return array_merge(range('0', '9'), range('a', 'f')); + } + +} diff --git a/samples/website_integration/bcrypt/integration.php b/samples/website_integration/bcrypt/integration.php deleted file mode 100644 index 75911838..00000000 --- a/samples/website_integration/bcrypt/integration.php +++ /dev/null @@ -1,107 +0,0 @@ -prepare('SELECT password FROM ' . AUTHME_TABLE . ' WHERE username = ?'); - $stmt->bind_param('s', $username); - $stmt->execute(); - $stmt->bind_result($password); - if ($stmt->fetch()) { - return $password; - } - } - return null; -} - -/** - * Returns whether the user exists in the database or not. - * - * @param string $username the username to check - * @return bool true if the user exists; false otherwise - */ -function authme_has_user($username) { - $mysqli = authme_get_mysqli(); - if ($mysqli !== null) { - $stmt = $mysqli->prepare('SELECT 1 FROM ' . AUTHME_TABLE . ' WHERE username = ?'); - $stmt->bind_param('s', $username); - $stmt->execute(); - return $stmt->fetch(); - } - - // Defensive default to true; we actually don't know - return true; -} - -/** - * Registers a player with the given username. - * - * @param string $username the username to register - * @param string $password the password to associate to the user - * @return bool whether or not the registration was successful - */ -function authme_register($username, $password) { - $mysqli = authme_get_mysqli(); - if ($mysqli !== null) { - $hash = password_hash($password, PASSWORD_BCRYPT); - $stmt = $mysqli->prepare('INSERT INTO ' . AUTHME_TABLE . ' (username, realname, password, ip) ' - . 'VALUES (?, ?, ?, ?)'); - $username_low = strtolower($username); - $stmt->bind_param('ssss', $username, $username_low, $hash, $_SERVER['REMOTE_ADDR']); - return $stmt->execute(); - } - return false; -} - diff --git a/samples/website_integration/bcrypt/form.php b/samples/website_integration/index.php similarity index 66% rename from samples/website_integration/bcrypt/form.php rename to samples/website_integration/index.php index 7801be4d..a5509da8 100644 --- a/samples/website_integration/bcrypt/form.php +++ b/samples/website_integration/index.php @@ -1,6 +1,6 @@ @@ -12,17 +12,24 @@ checkPassword($user, $pass)) { printf('

Hello, %s!

', htmlspecialchars($user)); echo 'Successful login. Nice to have you back!' - . '
Back to form'; + . '
Back to form'; return true; } else { echo '

Error

Invalid username or password.'; @@ -63,15 +70,15 @@ function process_login($user, $pass) { } // Register logic -function process_register($user, $pass) { - if (authme_has_user($user)) { +function process_register($user, $pass, AuthMeController $controller) { + if ($controller->isUserRegistered($user)) { echo '

Error

This user already exists.'; } else { // Note that we don't validate the password or username at all in this demo... - $register_success = authme_register($user, $pass); + $register_success = $controller->register($user, $pass); if ($register_success) { printf('

Welcome, %s!

Thanks for registering', htmlspecialchars($user)); - echo '
Back to form'; + echo '
Back to form'; return true; } else { echo '

Error

Unfortunately, there was an error during the registration.'; diff --git a/samples/website_integration/sha256/form.php b/samples/website_integration/sha256/form.php deleted file mode 100644 index 5ffecf34..00000000 --- a/samples/website_integration/sha256/form.php +++ /dev/null @@ -1,52 +0,0 @@ - - - - - AuthMe Integration Sample - - - -Hello, %s!', htmlspecialchars($user)); - echo 'Successful login. Nice to have you back!' - . '
Back to form'; - $was_successful = true; - } else { - echo '

Error

Invalid username or password.'; - } -} - -if (!$was_successful) { - echo '

Login sample

-This is a demo form for AuthMe website integration. Enter your AuthMe login details -into the following form to test it. -
- - - - -
Name
Pass
-
'; -} - -function get_from_post_or_empty($index_name) { - return trim( - filter_input(INPUT_POST, $index_name, FILTER_UNSAFE_RAW, FILTER_REQUIRE_SCALAR | FILTER_FLAG_STRIP_LOW) - ?: ''); -} -?> - - - diff --git a/samples/website_integration/sha256/integration.php b/samples/website_integration/sha256/integration.php deleted file mode 100644 index e0de0bb1..00000000 --- a/samples/website_integration/sha256/integration.php +++ /dev/null @@ -1,67 +0,0 @@ -prepare("SELECT password FROM $authme_table WHERE username = ?"); - $stmt->bind_param('s', $username); - $stmt->execute(); - $stmt->bind_result($password); - if ($stmt->fetch()) { - return $password; - } - } - return null; -} - -/** - * Checks the given clear-text password against the hash. - * - * @param string $password the clear-text password to check - * @param string $hash the hash to check the password against - * @return bool true iff the password matches the hash, false otherwise - */ -function authme_check_hash($password, $hash) { - // $SHA$salt$hash, where hash := sha256(sha256(password) . salt) - $parts = explode('$', $hash); - return count($parts) === 4 - && $parts[3] === hash('sha256', hash('sha256', $password) . $parts[2]); -} From f0144857894d9a97ae456c1c21f29afff15e0402 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Thu, 19 May 2016 21:50:48 +0200 Subject: [PATCH 05/16] Service injector - implement stricter requirements for PostConstruct methods - Implement similar restrictions as prescribed by the PostConstruct documentation: - Class may have at most one method annotated with PostConstruct - PostConstruct method must return void - Javadoc: replace mentions of injector construction where any injection method was meant --- .../AuthMeServiceInitializer.java | 64 +++++++++++++------ .../authme/permission/PermissionsManager.java | 7 +- .../AuthMeServiceInitializerTest.java | 14 +++- .../samples/InvalidPostConstruct.java | 26 +++++++- .../samples/PostConstructTestClass.java | 13 +--- 5 files changed, 86 insertions(+), 38 deletions(-) diff --git a/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java b/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java index c6d7fd85..23c2f01d 100644 --- a/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java +++ b/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java @@ -144,8 +144,8 @@ public class AuthMeServiceInitializer { } /** - * Instantiates the given class by locating an @Inject constructor and retrieving - * or instantiating its parameters. + * Instantiates the given class by locating its @Inject elements and retrieving + * or instantiating the required instances. * * @param clazz the class to instantiate * @param traversedClasses collection of classes already traversed @@ -164,13 +164,13 @@ public class AuthMeServiceInitializer { validateInjectionHasNoCircularDependencies(injection.getDependencies(), traversedClasses); Object[] dependencies = resolveDependencies(injection, traversedClasses); T object = injection.instantiateWith(dependencies); - executePostConstructMethods(object); + executePostConstructMethod(object); return object; } /** - * Resolves the dependencies for the given constructor, i.e. returns a collection that satisfy - * the constructor's parameter types by retrieving elements or instantiating them where necessary. + * Resolves the dependencies for the given class instantiation, i.e. returns a collection that satisfy + * the class' dependencies by retrieving elements or instantiating them where necessary. * * @param injection the injection parameters * @param traversedClasses collection of traversed classes @@ -247,21 +247,20 @@ public class AuthMeServiceInitializer { + "allowed packages. It must be provided explicitly or the package must be passed to the constructor."); } - private static void executePostConstructMethods(Object object) { - for (Method method : object.getClass().getDeclaredMethods()) { - if (method.isAnnotationPresent(PostConstruct.class)) { - if (method.getParameterTypes().length == 0 && !Modifier.isStatic(method.getModifiers())) { - try { - method.setAccessible(true); - method.invoke(object); - } catch (IllegalAccessException | InvocationTargetException e) { - throw new UnsupportedOperationException(e); - } - } else { - throw new IllegalStateException(String.format("@PostConstruct methods may not be static or have " - + " any parameters. Method '%s' of class '%s' is either static or has parameters", - method.getName(), object.getClass().getSimpleName())); - } + /** + * Executes an object's method annotated with {@link PostConstruct} if present. + * Throws an exception if there are multiple such methods, or if the method is static. + * + * @param object the object to execute the post construct method for + */ + private static void executePostConstructMethod(Object object) { + Method postConstructMethod = getAndValidatePostConstructMethod(object.getClass()); + if (postConstructMethod != null) { + try { + postConstructMethod.setAccessible(true); + postConstructMethod.invoke(object); + } catch (IllegalAccessException | InvocationTargetException e) { + throw new UnsupportedOperationException(e); } } } @@ -272,6 +271,31 @@ public class AuthMeServiceInitializer { } } + /** + * Validate and locate the given class' post construct method. Returns {@code null} if none present. + * + * @param clazz the class to search + * @return post construct method, or null + */ + private static Method getAndValidatePostConstructMethod(Class clazz) { + Method postConstructMethod = null; + for (Method method : clazz.getDeclaredMethods()) { + if (method.isAnnotationPresent(PostConstruct.class)) { + if (postConstructMethod != null) { + throw new IllegalStateException("Multiple methods with @PostConstruct on " + clazz); + } else if (method.getParameterTypes().length > 0 || Modifier.isStatic(method.getModifiers())) { + throw new IllegalStateException("@PostConstruct method may not be static or have any parameters. " + + "Invalid method in " + clazz); + } else if (method.getReturnType() != void.class) { + throw new IllegalStateException("@PostConstruct method must be void. Offending class: " + clazz); + } else { + postConstructMethod = method; + } + } + } + return postConstructMethod; + } + @SafeVarargs private static Injection firstNotNull(Provider>... providers) { for (Provider> provider : providers) { diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 9245a6a9..2c94d6f6 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -96,11 +96,9 @@ public class PermissionsManager implements PermissionsService { /** * Setup and hook into the permissions systems. - * - * @return The detected permissions system. */ @PostConstruct - public PermissionsSystemType setup() { + public void setup() { // Force-unhook from current hooked permissions systems unhook(); @@ -177,7 +175,7 @@ public class PermissionsManager implements PermissionsService { ConsoleLogger.info("Hooked into " + type.getName() + "!"); // Return the used permissions system type - return type; + return; } catch (Exception ex) { // An error occurred, show a warning message @@ -187,7 +185,6 @@ public class PermissionsManager implements PermissionsService { // No recognized permissions system found, show a message and return ConsoleLogger.info("No supported permissions system found! Permissions are disabled!"); - return null; } /** diff --git a/src/test/java/fr/xephi/authme/initialization/AuthMeServiceInitializerTest.java b/src/test/java/fr/xephi/authme/initialization/AuthMeServiceInitializerTest.java index a81571d1..81b2eb66 100644 --- a/src/test/java/fr/xephi/authme/initialization/AuthMeServiceInitializerTest.java +++ b/src/test/java/fr/xephi/authme/initialization/AuthMeServiceInitializerTest.java @@ -166,7 +166,7 @@ public class AuthMeServiceInitializerTest { PostConstructTestClass testClass = initializer.get(PostConstructTestClass.class); // then - assertThat(testClass.werePostConstructsCalled(), equalTo(true)); + assertThat(testClass.wasPostConstructCalled(), equalTo(true)); assertThat(testClass.getBetaManager(), not(nullValue())); } @@ -188,6 +188,18 @@ public class AuthMeServiceInitializerTest { initializer.get(InvalidPostConstruct.ThrowsException.class); } + @Test(expected = RuntimeException.class) + public void shouldThrowForMultiplePostConstructMethods() { + // given / when / then + initializer.get(InvalidPostConstruct.MultiplePostConstructs.class); + } + + @Test(expected = RuntimeException.class) + public void shouldThrowForPostConstructNotReturningVoid() { + // given / when / then + initializer.get(InvalidPostConstruct.NotVoidReturnType.class); + } + @Test(expected = RuntimeException.class) public void shouldThrowForAbstractNonRegisteredDependency() { // given / when / then diff --git a/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java b/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java index 23130e5e..80b6c83e 100644 --- a/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java +++ b/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java @@ -9,10 +9,8 @@ import javax.inject.Inject; public abstract class InvalidPostConstruct { public static final class WithParams { - @SuppressWarnings("unused") @Inject private AlphaService alphaService; - @SuppressWarnings("unused") @Inject private ProvidedClass providedClass; @@ -41,4 +39,28 @@ public abstract class InvalidPostConstruct { throw new IllegalStateException("Exception in post construct"); } } + + public static final class NotVoidReturnType { + @Inject + private ProvidedClass providedClass; + + @PostConstruct + public int returnsInt() { + return 42; + } + } + + public static final class MultiplePostConstructs { + @Inject + private ProvidedClass providedClass; + + @PostConstruct + public void postConstruct1() { + // -- + } + @PostConstruct + public void postConstruct2() { + // -- + } + } } diff --git a/src/test/java/fr/xephi/authme/initialization/samples/PostConstructTestClass.java b/src/test/java/fr/xephi/authme/initialization/samples/PostConstructTestClass.java index 375e58fa..a8b9e64e 100644 --- a/src/test/java/fr/xephi/authme/initialization/samples/PostConstructTestClass.java +++ b/src/test/java/fr/xephi/authme/initialization/samples/PostConstructTestClass.java @@ -17,22 +17,15 @@ public class PostConstructTestClass implements SettingsDependent { @Inject private BetaManager betaManager; private boolean wasPostConstructCalled = false; - private boolean wasSecondPostConstructCalled = false; private boolean wasReloaded = false; @PostConstruct - protected void setFieldToTrue() { + public void postConstructMethod() { wasPostConstructCalled = true; } - @PostConstruct - public int otherPostConstructMethod() { - wasSecondPostConstructCalled = true; - return 42; - } - - public boolean werePostConstructsCalled() { - return wasPostConstructCalled && wasSecondPostConstructCalled; + public boolean wasPostConstructCalled() { + return wasPostConstructCalled; } public BetaManager getBetaManager() { From 9a72fe53b07608a3a80157d7134879cb10a0ef6f Mon Sep 17 00:00:00 2001 From: ljacqu Date: Thu, 19 May 2016 21:55:42 +0200 Subject: [PATCH 06/16] Minor - code householding - Update inaccurate javadoc - Remove unnecessary require call in PHP integration sample --- samples/website_integration/index.php | 1 - .../authme/datasource/AbstractResourceClosingTest.java | 7 ++++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/samples/website_integration/index.php b/samples/website_integration/index.php index a5509da8..0c2a1a1c 100644 --- a/samples/website_integration/index.php +++ b/samples/website_integration/index.php @@ -25,7 +25,6 @@ $pass = get_from_post_or_empty('password'); $was_successful = false; if ($action && $user && $pass) { - require_once('Bcrypt.php'); if ($action === 'Log in') { $was_successful = process_login($user, $pass, $authme_controller); } else if ($action === 'Register') { diff --git a/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java b/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java index b17f5135..1ecdb637 100644 --- a/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java +++ b/src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java @@ -204,10 +204,11 @@ public abstract class AbstractResourceClosingTest { } /** - * Return a list with some test elements that correspond to the given list type's generic type. + * Return a collection of the required type with some test elements that correspond to the + * collection's generic type. * - * @param type The list type to process and build a test list for - * @return Test list with sample elements of the correct type + * @param type The collection type to process and build a test collection for + * @return Test collection with sample elements of the correct type */ private static Collection getTypedCollection(Type type) { if (type instanceof ParameterizedType) { From 4d634086cd20aed9d8e71dcff736bee6e23052ab Mon Sep 17 00:00:00 2001 From: ljacqu Date: Thu, 19 May 2016 22:02:15 +0200 Subject: [PATCH 07/16] #701 Alternate accounts are shown to all players with their own name --- .../authme/process/login/AsynchronousLogin.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java b/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java index da98a05b..fc6bba1a 100644 --- a/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java +++ b/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java @@ -22,7 +22,6 @@ import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.task.MessageTask; -import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.Utils; import org.bukkit.Bukkit; @@ -230,13 +229,14 @@ public class AsynchronousLogin implements Process { ConsoleLogger.info("The user " + player.getName() + " has " + auths.size() + " accounts:"); ConsoleLogger.info(message); - for (Player player : service.getOnlinePlayers()) { - if ((player.getName().equalsIgnoreCase(this.player.getName()) && plugin.getPermissionsManager().hasPermission(player, PlayerPermission.SEE_OWN_ACCOUNTS))) { - player.sendMessage("You own " + auths.size() + " accounts:"); - player.sendMessage(message); - } else if (plugin.getPermissionsManager().hasPermission(player, AdminPermission.SEE_OTHER_ACCOUNTS)) { - player.sendMessage("The user " + player.getName() + " has " + auths.size() + " accounts:"); - player.sendMessage(message); + for (Player onlinePlayer : service.getOnlinePlayers()) { + if (onlinePlayer.getName().equalsIgnoreCase(player.getName()) + && plugin.getPermissionsManager().hasPermission(onlinePlayer, PlayerPermission.SEE_OWN_ACCOUNTS)) { + onlinePlayer.sendMessage("You own " + auths.size() + " accounts:"); + onlinePlayer.sendMessage(message); + } else if (plugin.getPermissionsManager().hasPermission(onlinePlayer, AdminPermission.SEE_OTHER_ACCOUNTS)) { + onlinePlayer.sendMessage("The user " + player.getName() + " has " + auths.size() + " accounts:"); + onlinePlayer.sendMessage(message); } } } From 95b65ae20a855d8d89aa42432553fc6f051b5c46 Mon Sep 17 00:00:00 2001 From: Gabriele C Date: Thu, 19 May 2016 23:06:55 +0200 Subject: [PATCH 08/16] Cleanup --- .../command/executable/authme/PurgeCommand.java | 8 -------- .../xephi/authme/initialization/FieldInjection.java | 1 + .../initialization/samples/InvalidPostConstruct.java | 11 ----------- 3 files changed, 1 insertion(+), 19 deletions(-) diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java index a0811cfe..db91aad6 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java @@ -4,9 +4,7 @@ import fr.xephi.authme.AuthMe; import fr.xephi.authme.command.CommandService; import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.datasource.DataSource; -import fr.xephi.authme.hooks.PluginHooks; import fr.xephi.authme.task.PurgeTask; -import fr.xephi.authme.util.BukkitService; import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; @@ -26,12 +24,6 @@ public class PurgeCommand implements ExecutableCommand { @Inject private DataSource dataSource; - @Inject - private PluginHooks pluginHooks; - - @Inject - private BukkitService bukkitService; - @Inject private AuthMe plugin; diff --git a/src/main/java/fr/xephi/authme/initialization/FieldInjection.java b/src/main/java/fr/xephi/authme/initialization/FieldInjection.java index b8112b87..096c5f52 100644 --- a/src/main/java/fr/xephi/authme/initialization/FieldInjection.java +++ b/src/main/java/fr/xephi/authme/initialization/FieldInjection.java @@ -114,6 +114,7 @@ class FieldInjection implements Injection { return null; } + @SuppressWarnings("unchecked") private static Constructor getDefaultConstructor(Class clazz) { try { Constructor defaultConstructor = clazz.getDeclaredConstructor(); diff --git a/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java b/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java index 80b6c83e..e6d6e3a6 100644 --- a/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java +++ b/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java @@ -9,11 +9,6 @@ import javax.inject.Inject; public abstract class InvalidPostConstruct { public static final class WithParams { - @Inject - private AlphaService alphaService; - @Inject - private ProvidedClass providedClass; - WithParams() { } @PostConstruct @@ -41,9 +36,6 @@ public abstract class InvalidPostConstruct { } public static final class NotVoidReturnType { - @Inject - private ProvidedClass providedClass; - @PostConstruct public int returnsInt() { return 42; @@ -51,9 +43,6 @@ public abstract class InvalidPostConstruct { } public static final class MultiplePostConstructs { - @Inject - private ProvidedClass providedClass; - @PostConstruct public void postConstruct1() { // -- From 92287cb5ddc5952cb42f82886d1c0864ff304324 Mon Sep 17 00:00:00 2001 From: Gabriele C Date: Thu, 19 May 2016 23:18:16 +0200 Subject: [PATCH 09/16] Delay the first "please login/register" message on join --- .../java/fr/xephi/authme/process/join/AsynchronousJoin.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java b/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java index 3aff8941..26336869 100644 --- a/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java +++ b/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java @@ -224,9 +224,9 @@ public class AsynchronousJoin implements Process { : MessageKey.REGISTER_MESSAGE; } if (msgInterval > 0 && LimboCache.getInstance().getLimboPlayer(name) != null) { - BukkitTask msgTask = service.runTask(new MessageTask(service.getBukkitService(), plugin.getMessages(), - name, msg, msgInterval)); - LimboPlayer limboPlayer = LimboCache.getInstance().getLimboPlayer(name); + BukkitTask msgTask = service.runTaskLater((Runnable)new MessageTask(service.getBukkitService(), plugin.getMessages(), + name, msg, msgInterval), 20L); + LimboPlayer limboPlayer = LimboCache.getInstance().getLimboPlayer(name); if (limboPlayer != null) { limboPlayer.setMessageTask(msgTask); } From 6abad1970c71aa80dde864f6e3c2f31ddc479ba5 Mon Sep 17 00:00:00 2001 From: Gabriele C Date: Fri, 20 May 2016 14:58:41 +0200 Subject: [PATCH 10/16] Revert "Cleanup" This reverts commit 95b65ae20a855d8d89aa42432553fc6f051b5c46. --- .../command/executable/authme/PurgeCommand.java | 8 ++++++++ .../xephi/authme/initialization/FieldInjection.java | 1 - .../initialization/samples/InvalidPostConstruct.java | 11 +++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java index db91aad6..a0811cfe 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java @@ -4,7 +4,9 @@ import fr.xephi.authme.AuthMe; import fr.xephi.authme.command.CommandService; import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.hooks.PluginHooks; import fr.xephi.authme.task.PurgeTask; +import fr.xephi.authme.util.BukkitService; import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; @@ -24,6 +26,12 @@ public class PurgeCommand implements ExecutableCommand { @Inject private DataSource dataSource; + @Inject + private PluginHooks pluginHooks; + + @Inject + private BukkitService bukkitService; + @Inject private AuthMe plugin; diff --git a/src/main/java/fr/xephi/authme/initialization/FieldInjection.java b/src/main/java/fr/xephi/authme/initialization/FieldInjection.java index 096c5f52..b8112b87 100644 --- a/src/main/java/fr/xephi/authme/initialization/FieldInjection.java +++ b/src/main/java/fr/xephi/authme/initialization/FieldInjection.java @@ -114,7 +114,6 @@ class FieldInjection implements Injection { return null; } - @SuppressWarnings("unchecked") private static Constructor getDefaultConstructor(Class clazz) { try { Constructor defaultConstructor = clazz.getDeclaredConstructor(); diff --git a/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java b/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java index e6d6e3a6..80b6c83e 100644 --- a/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java +++ b/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java @@ -9,6 +9,11 @@ import javax.inject.Inject; public abstract class InvalidPostConstruct { public static final class WithParams { + @Inject + private AlphaService alphaService; + @Inject + private ProvidedClass providedClass; + WithParams() { } @PostConstruct @@ -36,6 +41,9 @@ public abstract class InvalidPostConstruct { } public static final class NotVoidReturnType { + @Inject + private ProvidedClass providedClass; + @PostConstruct public int returnsInt() { return 42; @@ -43,6 +51,9 @@ public abstract class InvalidPostConstruct { } public static final class MultiplePostConstructs { + @Inject + private ProvidedClass providedClass; + @PostConstruct public void postConstruct1() { // -- From 4303dca46944d4b6dbed75fadf6d01c44205dd12 Mon Sep 17 00:00:00 2001 From: Gabriele C Date: Fri, 20 May 2016 15:45:34 +0200 Subject: [PATCH 11/16] Apply no teleport to the respawn listener, remove Settings usage from the PlayerListener --- .../authme/listener/AuthMePlayerListener.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java index 7dddaa12..a11d2f8b 100644 --- a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java +++ b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java @@ -17,7 +17,7 @@ import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.permission.PlayerStatePermission; import fr.xephi.authme.process.Management; import fr.xephi.authme.settings.NewSetting; -import fr.xephi.authme.settings.Settings; +//import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.SpawnLoader; import fr.xephi.authme.settings.properties.HooksSettings; import fr.xephi.authme.settings.properties.ProtectionSettings; @@ -57,6 +57,7 @@ import org.bukkit.event.player.PlayerShearEntityEvent; import javax.inject.Inject; import java.util.concurrent.ConcurrentHashMap; +import java.util.regex.Pattern; import static fr.xephi.authme.listener.ListenerService.shouldCancelEvent; import static fr.xephi.authme.settings.properties.RestrictionSettings.ALLOWED_MOVEMENT_RADIUS; @@ -354,14 +355,16 @@ public class AuthMePlayerListener implements Listener { return; } - if (name.length() > Settings.getMaxNickLength || name.length() < Settings.getMinNickLength) { + if (name.length() > settings.getProperty(RestrictionSettings.MAX_NICKNAME_LENGTH) || name.length() < settings.getProperty(RestrictionSettings.MIN_NICKNAME_LENGTH)) { event.setKickMessage(m.retrieveSingle(MessageKey.INVALID_NAME_LENGTH)); event.setResult(PlayerLoginEvent.Result.KICK_OTHER); return; } - if (!Settings.nickPattern.matcher(player.getName()).matches() || name.equalsIgnoreCase("Player")) { - event.setKickMessage(m.retrieveSingle(MessageKey.INVALID_NAME_CHARACTERS).replace("REG_EX", Settings.getNickRegex)); + String nickRegEx = settings.getProperty(RestrictionSettings.ALLOWED_NICKNAME_CHARACTERS); + Pattern nickPattern = Pattern.compile(nickRegEx); + if (nickPattern.matcher(player.getName()).matches() || name.equalsIgnoreCase("Player")) { + event.setKickMessage(m.retrieveSingle(MessageKey.INVALID_NAME_CHARACTERS).replace("REG_EX", nickRegEx)); event.setResult(PlayerLoginEvent.Result.KICK_OTHER); return; } @@ -508,11 +511,13 @@ public class AuthMePlayerListener implements Listener { if (!shouldCancelEvent(event)) { return; } - + if (settings.getProperty(RestrictionSettings.NO_TELEPORT)) { + return; + } Player player = event.getPlayer(); String name = player.getName().toLowerCase(); Location spawn = spawnLoader.getSpawnLocation(player); - if (Settings.isSaveQuitLocationEnabled && dataSource.isAuthAvailable(name)) { + if (settings.getProperty(RestrictionSettings.SAVE_QUIT_LOCATION) && dataSource.isAuthAvailable(name)) { PlayerAuth auth = PlayerAuth.builder() .name(name) .realName(player.getName()) From 8dd5420e1a51537dde9507ddd2aafe6b8d193384 Mon Sep 17 00:00:00 2001 From: Gabriele C Date: Fri, 20 May 2016 15:46:46 +0200 Subject: [PATCH 12/16] Whoops --- .../java/fr/xephi/authme/listener/AuthMePlayerListener.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java index a11d2f8b..5d700e4e 100644 --- a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java +++ b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java @@ -17,7 +17,6 @@ import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.permission.PlayerStatePermission; import fr.xephi.authme.process.Management; import fr.xephi.authme.settings.NewSetting; -//import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.SpawnLoader; import fr.xephi.authme.settings.properties.HooksSettings; import fr.xephi.authme.settings.properties.ProtectionSettings; @@ -508,10 +507,10 @@ public class AuthMePlayerListener implements Listener { @EventHandler(ignoreCancelled = true, priority = EventPriority.HIGHEST) public void onPlayerRespawn(PlayerRespawnEvent event) { - if (!shouldCancelEvent(event)) { + if (settings.getProperty(RestrictionSettings.NO_TELEPORT)) { return; } - if (settings.getProperty(RestrictionSettings.NO_TELEPORT)) { + if (!shouldCancelEvent(event)) { return; } Player player = event.getPlayer(); From 2edcb703c11ecd5fd9a31aba15f507322d76a456 Mon Sep 17 00:00:00 2001 From: Gabriele C Date: Fri, 20 May 2016 15:53:39 +0200 Subject: [PATCH 13/16] Wtf was that? D: --- .../authme/listener/AuthMePlayerListener19.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener19.java b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener19.java index f5a20277..f5b073cc 100644 --- a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener19.java +++ b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener19.java @@ -1,6 +1,9 @@ package fr.xephi.authme.listener; +import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.SpawnLoader; +import fr.xephi.authme.settings.properties.RestrictionSettings; + import org.bukkit.event.EventHandler; import org.bukkit.event.EventPriority; import org.bukkit.event.Listener; @@ -16,11 +19,20 @@ public class AuthMePlayerListener19 implements Listener { @Inject private SpawnLoader spawnLoader; - AuthMePlayerListener19() { } + @Inject + private NewSetting settings; + /* WTF was that? We need to check all the settings before moving the player to the spawn! + * + * TODO: fixme please! + * @EventHandler(priority = EventPriority.LOWEST) public void onPlayerSpawn(PlayerSpawnLocationEvent event) { + if(settings.getProperty(RestrictionSettings.NO_TELEPORT)) { + return; + } event.setSpawnLocation(spawnLoader.getSpawnLocation(event.getPlayer())); } + */ } From a355c325c5e243b4b2f05cd3ca16d8bf5ef84af2 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 20 May 2016 17:15:53 +0200 Subject: [PATCH 14/16] #513 Allow to run updateDocs task from command line --- pom.xml | 2 +- src/test/java/tools/docs/UpdateDocsTask.java | 48 +++++++++++++++----- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/pom.xml b/pom.xml index c80dcaa1..dc62638d 100644 --- a/pom.xml +++ b/pom.xml @@ -247,7 +247,7 @@ ${project.basedir}/target/test-classes tools.ToolsRunner - writePermissionsList + updateDocs true diff --git a/src/test/java/tools/docs/UpdateDocsTask.java b/src/test/java/tools/docs/UpdateDocsTask.java index c3fb8562..48624afe 100644 --- a/src/test/java/tools/docs/UpdateDocsTask.java +++ b/src/test/java/tools/docs/UpdateDocsTask.java @@ -4,6 +4,7 @@ import com.google.common.collect.ImmutableSet; import tools.commands.CommandPageCreater; import tools.hashmethods.HashAlgorithmsDescriptionTask; import tools.permissions.PermissionsListWriter; +import tools.utils.AutoToolTask; import tools.utils.ToolTask; import java.util.Scanner; @@ -12,9 +13,9 @@ import java.util.Set; /** * Task that runs all tasks which update files in the docs folder. */ -public class UpdateDocsTask implements ToolTask { +public class UpdateDocsTask implements AutoToolTask { - private final Set> TASKS = ImmutableSet.>of( + private static final Set> TASKS = ImmutableSet.>of( CommandPageCreater.class, HashAlgorithmsDescriptionTask.class, PermissionsListWriter.class); @Override @@ -23,17 +24,25 @@ public class UpdateDocsTask implements ToolTask { } @Override - public void execute(Scanner scanner) { - for (Class taskClass : TASKS) { - try { - ToolTask task = instantiateTask(taskClass); - System.out.println("\nRunning " + task.getTaskName() + "\n-------------------"); + public void execute(final Scanner scanner) { + executeTasks(new TaskRunner() { + @Override + public void execute(ToolTask task) { task.execute(scanner); - } catch (UnsupportedOperationException e) { - System.err.println("Error running task of class '" + taskClass + "'"); - e.printStackTrace(); } - } + }); + } + + @Override + public void executeDefault() { + executeTasks(new TaskRunner() { + @Override + public void execute(ToolTask task) { + if (task instanceof AutoToolTask) { + ((AutoToolTask) task).executeDefault(); + } + } + }); } private static ToolTask instantiateTask(Class clazz) { @@ -43,4 +52,21 @@ public class UpdateDocsTask implements ToolTask { throw new UnsupportedOperationException(e); } } + + private static void executeTasks(TaskRunner runner) { + for (Class taskClass : TASKS) { + try { + ToolTask task = instantiateTask(taskClass); + System.out.println("\nRunning " + task.getTaskName() + "\n-------------------"); + runner.execute(task); + } catch (UnsupportedOperationException e) { + System.err.println("Error running task of class '" + taskClass + "'"); + e.printStackTrace(); + } + } + } + + private interface TaskRunner { + void execute(ToolTask task); + } } From 244e1a2b7d0503c455ca04c92667f0ee2b049671 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 20 May 2016 17:57:14 +0200 Subject: [PATCH 15/16] Injector - don't use instantiation fallback if PostConstruct method is present - Do not instantiate classes with instantiation fallback if they have a PostConstruct method - thanks @sgdc3 for the hint - Change Injector test to check exception messages also --- .../AuthMeServiceInitializer.java | 5 +- .../initialization/InstantiationFallback.java | 13 ++-- .../AuthMeServiceInitializerTest.java | 72 +++++++++++++------ .../initialization/FieldInjectionTest.java | 2 - .../InstantiationFallbackTest.java | 10 +++ .../samples/InstantiationFallbackClasses.java | 11 ++- .../samples/InvalidPostConstruct.java | 3 + 7 files changed, 85 insertions(+), 31 deletions(-) diff --git a/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java b/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java index 23c2f01d..9bc8525c 100644 --- a/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java +++ b/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java @@ -260,7 +260,7 @@ public class AuthMeServiceInitializer { postConstructMethod.setAccessible(true); postConstructMethod.invoke(object); } catch (IllegalAccessException | InvocationTargetException e) { - throw new UnsupportedOperationException(e); + throw new UnsupportedOperationException("Error executing @PostConstruct method", e); } } } @@ -287,7 +287,8 @@ public class AuthMeServiceInitializer { throw new IllegalStateException("@PostConstruct method may not be static or have any parameters. " + "Invalid method in " + clazz); } else if (method.getReturnType() != void.class) { - throw new IllegalStateException("@PostConstruct method must be void. Offending class: " + clazz); + throw new IllegalStateException("@PostConstruct method must have return type void. " + + "Offending class: " + clazz); } else { postConstructMethod = method; } diff --git a/src/main/java/fr/xephi/authme/initialization/InstantiationFallback.java b/src/main/java/fr/xephi/authme/initialization/InstantiationFallback.java index f7017ce1..7a1c0849 100644 --- a/src/main/java/fr/xephi/authme/initialization/InstantiationFallback.java +++ b/src/main/java/fr/xephi/authme/initialization/InstantiationFallback.java @@ -1,5 +1,6 @@ package fr.xephi.authme.initialization; +import javax.annotation.PostConstruct; import javax.inject.Inject; import javax.inject.Provider; import java.lang.reflect.AccessibleObject; @@ -8,7 +9,7 @@ import java.lang.reflect.InvocationTargetException; /** * Fallback instantiation method for classes with an accessible no-args constructor - * and no no {@link Inject} annotations whatsoever. + * and no elements whatsoever annotated with {@link Inject} or {@link PostConstruct}. */ public class InstantiationFallback implements Injection { @@ -54,9 +55,9 @@ public class InstantiationFallback implements Injection { Constructor noArgsConstructor = getNoArgsConstructor(clazz); // Return fallback only if we have no args constructor and no @Inject annotation anywhere if (noArgsConstructor != null - && !isInjectAnnotationPresent(clazz.getDeclaredConstructors()) - && !isInjectAnnotationPresent(clazz.getDeclaredFields()) - && !isInjectAnnotationPresent(clazz.getDeclaredMethods())) { + && !isInjectionAnnotationPresent(clazz.getDeclaredConstructors()) + && !isInjectionAnnotationPresent(clazz.getDeclaredFields()) + && !isInjectionAnnotationPresent(clazz.getDeclaredMethods())) { return new InstantiationFallback<>(noArgsConstructor); } return null; @@ -73,9 +74,9 @@ public class InstantiationFallback implements Injection { } } - private static boolean isInjectAnnotationPresent(A[] accessibles) { + private static boolean isInjectionAnnotationPresent(A[] accessibles) { for (A accessible : accessibles) { - if (accessible.isAnnotationPresent(Inject.class)) { + if (accessible.isAnnotationPresent(Inject.class) || accessible.isAnnotationPresent(PostConstruct.class)) { return true; } } diff --git a/src/test/java/fr/xephi/authme/initialization/AuthMeServiceInitializerTest.java b/src/test/java/fr/xephi/authme/initialization/AuthMeServiceInitializerTest.java index 81b2eb66..d85377f5 100644 --- a/src/test/java/fr/xephi/authme/initialization/AuthMeServiceInitializerTest.java +++ b/src/test/java/fr/xephi/authme/initialization/AuthMeServiceInitializerTest.java @@ -18,8 +18,11 @@ import fr.xephi.authme.initialization.samples.ProvidedClass; import fr.xephi.authme.initialization.samples.Size; import fr.xephi.authme.settings.NewSetting; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; @@ -36,6 +39,11 @@ public class AuthMeServiceInitializerTest { private AuthMeServiceInitializer initializer; + // As we test many cases that throw exceptions, we use JUnit's ExpectedException Rule + // to make sure that we receive the exception we expect + @Rule + public ExpectedException expectedException = ExpectedException.none(); + @Before public void setInitializer() { initializer = new AuthMeServiceInitializer(ALLOWED_PACKAGE); @@ -54,15 +62,17 @@ public class AuthMeServiceInitializerTest { } } - @Test(expected = RuntimeException.class) + @Test public void shouldThrowForInvalidPackage() { // given / when / then + expectRuntimeExceptionWith("outside of the allowed packages"); initializer.get(InvalidClass.class); } - @Test(expected = RuntimeException.class) + @Test public void shouldThrowForUnregisteredPrimitiveType() { // given / when / then + expectRuntimeExceptionWith("Primitive types must be provided"); initializer.get(int.class); } @@ -85,24 +95,27 @@ public class AuthMeServiceInitializerTest { assertThat(object.getGammaService(), equalTo(initializer.get(BetaManager.class).getDependencies()[1])); } - @Test(expected = RuntimeException.class) + @Test public void shouldRecognizeCircularReferences() { // given / when / then + expectRuntimeExceptionWith("Found cyclic dependency"); initializer.get(CircularClasses.Circular3.class); } - @Test(expected = RuntimeException.class) + @Test public void shouldThrowForUnregisteredAnnotation() { // given initializer.provide(Size.class, 4523); // when / then + expectRuntimeExceptionWith("must be registered beforehand"); initializer.get(ClassWithAnnotations.class); } - @Test(expected = RuntimeException.class) - public void shouldThrowForFieldInjectionWithNoDefaultConstructor() { + @Test + public void shouldThrowForFieldInjectionWithoutNoArgsConstructor() { // given / when / then + expectRuntimeExceptionWith("Did not find injection method"); initializer.get(BadFieldInjection.class); } @@ -124,36 +137,41 @@ public class AuthMeServiceInitializerTest { equalTo(result.getBetaManager().getDependencies()[1])); } - @Test(expected = RuntimeException.class) + @Test public void shouldThrowForAnnotationAsKey() { // given / when / then + expectRuntimeExceptionWith("Cannot retrieve annotated elements in this way"); initializer.get(Size.class); } - @Test(expected = RuntimeException.class) + @Test public void shouldThrowForSecondRegistration() { // given / when / then + expectRuntimeExceptionWith("There is already an object present"); initializer.register(ProvidedClass.class, new ProvidedClass("")); } - @Test(expected = RuntimeException.class) + @Test public void shouldThrowForSecondAnnotationRegistration() { // given initializer.provide(Size.class, 12); // when / then + expectRuntimeExceptionWith("already registered"); initializer.provide(Size.class, -8); } - @Test(expected = NullPointerException.class) + @Test public void shouldThrowForNullValueAssociatedToAnnotation() { // given / when / then + expectedException.expect(NullPointerException.class); initializer.provide(Duration.class, null); } - @Test(expected = NullPointerException.class) + @Test public void shouldThrowForRegisterWithNull() { // given / when / then + expectedException.expect(NullPointerException.class); initializer.register(String.class, null); } @@ -170,39 +188,45 @@ public class AuthMeServiceInitializerTest { assertThat(testClass.getBetaManager(), not(nullValue())); } - @Test(expected = RuntimeException.class) + @Test public void shouldThrowForInvalidPostConstructMethod() { // given / when / then + expectRuntimeExceptionWith("@PostConstruct method may not be static or have any parameters"); initializer.get(InvalidPostConstruct.WithParams.class); } - @Test(expected = RuntimeException.class) + @Test public void shouldThrowForStaticPostConstructMethod() { // given / when / then + expectRuntimeExceptionWith("@PostConstruct method may not be static or have any parameters"); initializer.get(InvalidPostConstruct.Static.class); } - @Test(expected = RuntimeException.class) + @Test public void shouldForwardExceptionFromPostConstruct() { // given / when / then + expectRuntimeExceptionWith("Error executing @PostConstruct method"); initializer.get(InvalidPostConstruct.ThrowsException.class); } - @Test(expected = RuntimeException.class) + @Test public void shouldThrowForMultiplePostConstructMethods() { // given / when / then + expectRuntimeExceptionWith("Multiple methods with @PostConstruct"); initializer.get(InvalidPostConstruct.MultiplePostConstructs.class); } - @Test(expected = RuntimeException.class) + @Test public void shouldThrowForPostConstructNotReturningVoid() { // given / when / then + expectRuntimeExceptionWith("@PostConstruct method must have return type void"); initializer.get(InvalidPostConstruct.NotVoidReturnType.class); } - @Test(expected = RuntimeException.class) + @Test public void shouldThrowForAbstractNonRegisteredDependency() { // given / when / then + expectRuntimeExceptionWith("cannot be instantiated"); initializer.get(ClassWithAbstractDependency.class); } @@ -220,12 +244,13 @@ public class AuthMeServiceInitializerTest { assertThat(cwad.getAlphaService(), not(nullValue())); } - @Test(expected = RuntimeException.class) + @Test public void shouldThrowForAlreadyRegisteredClass() { // given initializer.register(BetaManager.class, new BetaManager()); // when / then + expectRuntimeExceptionWith("There is already an object present"); initializer.register(BetaManager.class, new BetaManager()); } @@ -241,9 +266,10 @@ public class AuthMeServiceInitializerTest { assertThat(singletonScoped, not(sameInstance(requestScoped))); } - @Test(expected = RuntimeException.class) + @Test public void shouldThrowForStaticFieldInjection() { // given / when / then + expectRuntimeExceptionWith("is static but annotated with @Inject"); initializer.newInstance(InvalidStaticFieldInjection.class); } @@ -283,10 +309,16 @@ public class AuthMeServiceInitializerTest { assertThat(providedClass.getWasReloaded(), equalTo(true)); } - @Test(expected = RuntimeException.class) + @Test public void shouldThrowForNullSetting() { // given / when / then + expectRuntimeExceptionWith("Settings instance is null"); initializer.performReloadOnServices(); } + private void expectRuntimeExceptionWith(String message) { + expectedException.expect(RuntimeException.class); + expectedException.expectMessage(containsString(message)); + } + } diff --git a/src/test/java/fr/xephi/authme/initialization/FieldInjectionTest.java b/src/test/java/fr/xephi/authme/initialization/FieldInjectionTest.java index b784e4da..39675343 100644 --- a/src/test/java/fr/xephi/authme/initialization/FieldInjectionTest.java +++ b/src/test/java/fr/xephi/authme/initialization/FieldInjectionTest.java @@ -115,11 +115,9 @@ public class FieldInjectionTest { } private static class ThrowingConstructor { - @SuppressWarnings("unused") @Inject private ProvidedClass providedClass; - @SuppressWarnings("unused") public ThrowingConstructor() { throw new UnsupportedOperationException("Exception in constructor"); } diff --git a/src/test/java/fr/xephi/authme/initialization/InstantiationFallbackTest.java b/src/test/java/fr/xephi/authme/initialization/InstantiationFallbackTest.java index c40648f6..2676c404 100644 --- a/src/test/java/fr/xephi/authme/initialization/InstantiationFallbackTest.java +++ b/src/test/java/fr/xephi/authme/initialization/InstantiationFallbackTest.java @@ -65,4 +65,14 @@ public class InstantiationFallbackTest { assertThat(instantiation, nullValue()); } + @Test + public void shouldReturnNullForClassWithPostConstruct() { + // given / when + Injection instantiation = + InstantiationFallback.provide(InstantiationFallbackClasses.ClassWithPostConstruct.class).get(); + + // then + assertThat(instantiation, nullValue()); + } + } diff --git a/src/test/java/fr/xephi/authme/initialization/samples/InstantiationFallbackClasses.java b/src/test/java/fr/xephi/authme/initialization/samples/InstantiationFallbackClasses.java index ae15029b..c7fbd7e2 100644 --- a/src/test/java/fr/xephi/authme/initialization/samples/InstantiationFallbackClasses.java +++ b/src/test/java/fr/xephi/authme/initialization/samples/InstantiationFallbackClasses.java @@ -1,9 +1,10 @@ package fr.xephi.authme.initialization.samples; +import javax.annotation.PostConstruct; import javax.inject.Inject; /** - * Sample class - triggers instantiation fallback. + * Sample class - tests various situations for the instantiation fallback. */ public abstract class InstantiationFallbackClasses { @@ -42,4 +43,12 @@ public abstract class InstantiationFallbackClasses { } } + // Class with @PostConstruct method should never be instantiated by instantiation fallback + public static final class ClassWithPostConstruct { + @PostConstruct + public void postConstructMethod() { + // -- + } + } + } diff --git a/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java b/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java index 80b6c83e..501fad6a 100644 --- a/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java +++ b/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java @@ -34,6 +34,9 @@ public abstract class InvalidPostConstruct { } public static final class ThrowsException { + @Inject + private ProvidedClass providedClass; + @PostConstruct public void throwingPostConstruct() { throw new IllegalStateException("Exception in post construct"); From 5adf81991092e0d76bd99ae6eeaa062513b312d8 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 20 May 2016 18:03:22 +0200 Subject: [PATCH 16/16] Minor - remove unused services from PurgeCommand - Found by sgdc3 in 95b65ae20a855d8d89aa42432553fc6f051b5c46 --- .../authme/command/executable/authme/PurgeCommand.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java index a0811cfe..db91aad6 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/PurgeCommand.java @@ -4,9 +4,7 @@ import fr.xephi.authme.AuthMe; import fr.xephi.authme.command.CommandService; import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.datasource.DataSource; -import fr.xephi.authme.hooks.PluginHooks; import fr.xephi.authme.task.PurgeTask; -import fr.xephi.authme.util.BukkitService; import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; @@ -26,12 +24,6 @@ public class PurgeCommand implements ExecutableCommand { @Inject private DataSource dataSource; - @Inject - private PluginHooks pluginHooks; - - @Inject - private BukkitService bukkitService; - @Inject private AuthMe plugin;