From 9a5c43250988ca984f75161ff43ff1fa02ee37da Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 2 Jul 2016 20:56:53 +0200 Subject: [PATCH] #655 Encapsulate mail service - Change SendMailSSL to be injected into classes and created regardless of settings - Various minor cleanups (remove accidentally committed test, add more precise logging statement) --- src/main/java/fr/xephi/authme/AuthMe.java | 26 -------- .../authme/ChangePasswordAdminCommand.java | 2 +- .../executable/email/RecoverEmailCommand.java | 9 ++- .../executable/register/RegisterCommand.java | 12 ++-- .../fr/xephi/authme/datasource/MySQL.java | 2 - .../fr/xephi/authme/mail/SendMailSSL.java | 66 +++++++++++++------ .../process/register/AsyncRegister.java | 6 +- .../register/RegisterCommandTest.java | 24 +++++-- .../datasource/SQLiteIntegrationTest.java | 5 -- 9 files changed, 80 insertions(+), 72 deletions(-) diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 3acb8ca80..6959b19e9 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -25,7 +25,6 @@ import fr.xephi.authme.listener.AuthMePlayerListener; import fr.xephi.authme.listener.AuthMePlayerListener16; import fr.xephi.authme.listener.AuthMePlayerListener18; import fr.xephi.authme.listener.AuthMeServerListener; -import fr.xephi.authme.mail.SendMailSSL; import fr.xephi.authme.output.ConsoleFilter; import fr.xephi.authme.output.Log4JFilter; import fr.xephi.authme.output.MessageKey; @@ -79,8 +78,6 @@ import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; -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; /** @@ -114,7 +111,6 @@ public class AuthMe extends JavaPlugin { private BukkitService bukkitService; private AuthMeServiceInitializer initializer; private GeoLiteAPI geoLiteApi; - private SendMailSSL mail; /** * Constructor. @@ -248,9 +244,6 @@ public class AuthMe extends JavaPlugin { // Set console filter setupConsoleFilter(); - // Set up the mail API - setupMailApi(); - // Do a backup on start // TODO: maybe create a backup manager? new PerformBackup(this, newSettings).doBackup(PerformBackup.BackupCause.START); @@ -303,16 +296,6 @@ public class AuthMe extends JavaPlugin { initializer.get(API.class); } - /** - * Set up the mail API, if enabled. - */ - private void setupMailApi() { - // Make sure the mail API is enabled - if (!newSettings.getProperty(MAIL_ACCOUNT).isEmpty() && !newSettings.getProperty(MAIL_PASSWORD).isEmpty()) { - this.mail = new SendMailSSL(this, newSettings); - } - } - /** * Show the settings warnings, for various risky settings. */ @@ -673,15 +656,6 @@ public class AuthMe extends JavaPlugin { return commandHandler.processCommand(sender, commandLabel, args); } - /** - * Get the mailing instance. - * - * @return The send mail instance. - */ - public SendMailSSL getMail() { - return this.mail; - } - // ------------- // Service getters (deprecated) // Use @Inject fields instead diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java index 729a4b4e6..d55e84ac3 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java @@ -75,7 +75,7 @@ public class ChangePasswordAdminCommand implements ExecutableCommand { if (dataSource.updatePassword(auth)) { commandService.send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS); - ConsoleLogger.info(playerNameLowerCase + "'s password changed"); + ConsoleLogger.info(sender.getName() + " changed password of " + playerNameLowerCase); } else { commandService.send(sender, MessageKey.ERROR); } diff --git a/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java b/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java index 636027bbb..8a874af7e 100644 --- a/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java @@ -1,12 +1,12 @@ package fr.xephi.authme.command.executable.email; -import fr.xephi.authme.AuthMe; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.cache.auth.PlayerAuth; import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.command.CommandService; import fr.xephi.authme.command.PlayerCommand; import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.mail.SendMailSSL; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.RandomString; @@ -33,15 +33,14 @@ public class RecoverEmailCommand extends PlayerCommand { private PlayerCache playerCache; @Inject - // TODO #655: Remove injected AuthMe instance once Authme#mail is encapsulated - private AuthMe plugin; + private SendMailSSL sendMailSsl; @Override public void runCommand(Player player, List arguments) { final String playerMail = arguments.get(0); final String playerName = player.getName(); - if (plugin.getMail() == null) { + if (!sendMailSsl.hasAllInformation()) { ConsoleLogger.showError("Mail API is not set"); commandService.send(player, MessageKey.ERROR); return; @@ -76,7 +75,7 @@ public class RecoverEmailCommand extends PlayerCommand { } auth.setPassword(hashNew); dataSource.updatePassword(auth); - plugin.getMail().main(auth, thePass); + sendMailSsl.sendPasswordMail(auth, thePass); commandService.send(player, MessageKey.RECOVERY_EMAIL_SENT_MESSAGE); } else { commandService.send(player, MessageKey.REGISTER_EMAIL_MESSAGE); diff --git a/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java b/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java index ed9f9eef5..cb8c1946c 100644 --- a/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java @@ -3,6 +3,7 @@ package fr.xephi.authme.command.executable.register; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.command.CommandService; import fr.xephi.authme.command.PlayerCommand; +import fr.xephi.authme.mail.SendMailSSL; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.process.Management; import fr.xephi.authme.security.HashAlgorithm; @@ -27,6 +28,9 @@ public class RegisterCommand extends PlayerCommand { @Inject private CommandService commandService; + @Inject + private SendMailSSL sendMailSsl; + @Override public void runCommand(Player player, List arguments) { if (commandService.getProperty(SecuritySettings.PASSWORD_HASH) == HashAlgorithm.TWO_FACTOR) { @@ -63,11 +67,11 @@ public class RegisterCommand extends PlayerCommand { } private void handleEmailRegistration(Player player, List arguments) { - if (commandService.getProperty(EmailSettings.MAIL_ACCOUNT).isEmpty()) { - player.sendMessage("Cannot register: no email address is set for the server. " + if (!sendMailSsl.hasAllInformation()) { + player.sendMessage("Cannot register: not all required settings are set for sending emails. " + "Please contact an administrator"); - ConsoleLogger.showError("Cannot register player '" + player.getName() + "': no email is set " - + "to send emails from. Please add one in your config at " + EmailSettings.MAIL_ACCOUNT.getPath()); + ConsoleLogger.showError("Cannot register player '" + player.getName() + "': no email or password is set " + + "to send emails from. Please adjust your config at " + EmailSettings.MAIL_ACCOUNT.getPath()); return; } diff --git a/src/main/java/fr/xephi/authme/datasource/MySQL.java b/src/main/java/fr/xephi/authme/datasource/MySQL.java index d60f867fc..e1720f959 100644 --- a/src/main/java/fr/xephi/authme/datasource/MySQL.java +++ b/src/main/java/fr/xephi/authme/datasource/MySQL.java @@ -54,12 +54,10 @@ public class MySQL implements DataSource { if (e instanceof IllegalArgumentException) { ConsoleLogger.showError("Invalid database arguments! Please check your configuration!"); ConsoleLogger.showError("If this error persists, please report it to the developer!"); - throw e; } if (e instanceof PoolInitializationException) { ConsoleLogger.showError("Can't initialize database connection! Please check your configuration!"); ConsoleLogger.showError("If this error persists, please report it to the developer!"); - throw new PoolInitializationException(e); } ConsoleLogger.showError("Can't use the Hikari Connection Pool! Please, report this error to the developer!"); throw e; diff --git a/src/main/java/fr/xephi/authme/mail/SendMailSSL.java b/src/main/java/fr/xephi/authme/mail/SendMailSSL.java index 58b64d6b7..949f37774 100644 --- a/src/main/java/fr/xephi/authme/mail/SendMailSSL.java +++ b/src/main/java/fr/xephi/authme/mail/SendMailSSL.java @@ -5,45 +5,72 @@ import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.cache.auth.PlayerAuth; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.properties.EmailSettings; +import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.StringUtils; import org.apache.commons.mail.EmailConstants; import org.apache.commons.mail.EmailException; import org.apache.commons.mail.HtmlEmail; -import org.bukkit.Bukkit; import javax.activation.DataSource; import javax.activation.FileDataSource; import javax.imageio.ImageIO; +import javax.inject.Inject; import javax.mail.Session; import java.io.File; import java.io.IOException; import java.security.Security; import java.util.Properties; +import static fr.xephi.authme.settings.properties.EmailSettings.MAIL_ACCOUNT; +import static fr.xephi.authme.settings.properties.EmailSettings.MAIL_PASSWORD; + /** * @author Xephi59 */ public class SendMailSSL { - private final AuthMe plugin; - private final NewSetting settings; + @Inject + private AuthMe plugin; + @Inject + private NewSetting settings; + @Inject + private BukkitService bukkitService; - public SendMailSSL(AuthMe plugin, NewSetting settings) { - this.plugin = plugin; - this.settings = settings; + SendMailSSL() { } - public void main(final PlayerAuth auth, final String newPass) { - final String mailText = replaceMailTags(settings.getEmailMessage(), plugin, auth, newPass); - Bukkit.getScheduler().runTaskAsynchronously(plugin, new Runnable() { + /** + * Returns whether all necessary settings are set for sending mails. + * + * @return true if the necessary email settings are set, false otherwise + */ + public boolean hasAllInformation() { + return !settings.getProperty(MAIL_ACCOUNT).isEmpty() + && !settings.getProperty(MAIL_PASSWORD).isEmpty(); + } + + /** + * Sends an email to the user with his new password. + * + * @param auth the player auth of the player + * @param newPass the new password + */ + public void sendPasswordMail(final PlayerAuth auth, final String newPass) { + if (!hasAllInformation()) { + ConsoleLogger.showError("Cannot perform email registration: not all email settings are complete"); + return; + } + + final String mailText = replaceMailTags(settings.getEmailMessage(), auth, newPass); + bukkitService.runTaskAsynchronously(new Runnable() { @Override public void run() { Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader()); HtmlEmail email; try { - email = initializeMail(auth, settings); + email = initializeMail(auth.getEmail()); } catch (EmailException e) { ConsoleLogger.logException("Failed to create email with the given settings:", e); return; @@ -54,7 +81,7 @@ public class SendMailSSL { File file = null; if (settings.getProperty(EmailSettings.PASSWORD_AS_IMAGE)) { try { - file = generateImage(auth, plugin, newPass); + file = generateImage(auth.getNickname(), plugin, newPass); content = embedImageIntoEmailContent(file, email, content); } catch (IOException | EmailException e) { ConsoleLogger.logException( @@ -67,13 +94,12 @@ public class SendMailSSL { file.delete(); } } - }); } - private static File generateImage(PlayerAuth auth, AuthMe plugin, String newPass) throws IOException { + private static File generateImage(String name, AuthMe plugin, String newPass) throws IOException { ImageGenerator gen = new ImageGenerator(newPass); - File file = new File(plugin.getDataFolder(), auth.getNickname() + "_new_pass.jpg"); + File file = new File(plugin.getDataFolder(), name + "_new_pass.jpg"); ImageIO.write(gen.generateImage(), "jpg", file); return file; } @@ -85,8 +111,7 @@ public class SendMailSSL { return content.replace("", ""); } - private static HtmlEmail initializeMail(PlayerAuth auth, NewSetting settings) - throws EmailException { + private HtmlEmail initializeMail(String emailAddress) throws EmailException { String senderMail = settings.getProperty(EmailSettings.MAIL_ACCOUNT); String senderName = StringUtils.isEmpty(settings.getProperty(EmailSettings.MAIL_SENDER_NAME)) ? senderMail @@ -98,12 +123,12 @@ public class SendMailSSL { email.setCharset(EmailConstants.UTF_8); email.setSmtpPort(port); email.setHostName(settings.getProperty(EmailSettings.SMTP_HOST)); - email.addTo(auth.getEmail()); + email.addTo(emailAddress); email.setFrom(senderMail, senderName); email.setSubject(settings.getProperty(EmailSettings.RECOVERY_MAIL_SUBJECT)); email.setAuthentication(senderMail, mailPassword); - setPropertiesForPort(email, port, settings); + setPropertiesForPort(email, port); return email; } @@ -124,15 +149,14 @@ public class SendMailSSL { } } - private static String replaceMailTags(String mailText, AuthMe plugin, PlayerAuth auth, String newPass) { + private String replaceMailTags(String mailText, PlayerAuth auth, String newPass) { return mailText .replace("", auth.getNickname()) .replace("", plugin.getServer().getServerName()) .replace("", newPass); } - private static void setPropertiesForPort(HtmlEmail email, int port, NewSetting settings) - throws EmailException { + private void setPropertiesForPort(HtmlEmail email, int port) throws EmailException { switch (port) { case 587: String oAuth2Token = settings.getProperty(EmailSettings.OAUTH2_TOKEN); diff --git a/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java b/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java index 4ddaff5e5..056ae36cd 100644 --- a/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java +++ b/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java @@ -4,6 +4,7 @@ import fr.xephi.authme.AuthMe; import fr.xephi.authme.cache.auth.PlayerAuth; import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.mail.SendMailSSL; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.process.AsynchronousProcess; @@ -55,6 +56,9 @@ public class AsyncRegister implements AsynchronousProcess { @Inject private ValidationService validationService; + @Inject + private SendMailSSL sendMailSsl; + AsyncRegister() { } private boolean preRegisterCheck(Player player, String password) { @@ -137,7 +141,7 @@ public class AsyncRegister implements AsynchronousProcess { } database.updateEmail(auth); database.updateSession(auth); - plugin.getMail().main(auth, password); + sendMailSsl.sendPasswordMail(auth, password); syncProcessManager.processSyncEmailRegister(player); } diff --git a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java index 217b74bef..683302dbf 100644 --- a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java @@ -2,6 +2,7 @@ package fr.xephi.authme.command.executable.register; import fr.xephi.authme.TestHelper; import fr.xephi.authme.command.CommandService; +import fr.xephi.authme.mail.SendMailSSL; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.process.Management; import fr.xephi.authme.security.HashAlgorithm; @@ -49,6 +50,8 @@ public class RegisterCommandTest { @Mock private Management management; + @Mock + private SendMailSSL sendMailSsl; @BeforeClass public static void setup() { @@ -72,7 +75,7 @@ public class RegisterCommandTest { // then verify(sender).sendMessage(argThat(containsString("Player only!"))); - verifyZeroInteractions(management); + verifyZeroInteractions(management, sendMailSsl); } @Test @@ -86,6 +89,7 @@ public class RegisterCommandTest { // then verify(management).performRegister(player, "", "", true); + verifyZeroInteractions(sendMailSsl); } @Test @@ -98,7 +102,7 @@ public class RegisterCommandTest { // then verify(commandService).send(player, MessageKey.USAGE_REGISTER); - verifyZeroInteractions(management); + verifyZeroInteractions(management, sendMailSsl); } @Test @@ -112,7 +116,7 @@ public class RegisterCommandTest { // then verify(commandService).send(player, MessageKey.USAGE_REGISTER); - verifyZeroInteractions(management); + verifyZeroInteractions(management, sendMailSsl); } @Test @@ -127,7 +131,7 @@ public class RegisterCommandTest { // then verify(commandService).send(player, MessageKey.USAGE_REGISTER); - verifyZeroInteractions(management); + verifyZeroInteractions(management, sendMailSsl); } @Test @@ -135,14 +139,15 @@ public class RegisterCommandTest { // given given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(true); given(commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL)).willReturn(false); - given(commandService.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn(""); + given(sendMailSsl.hasAllInformation()).willReturn(false); Player player = mock(Player.class); // when command.executeCommand(player, Collections.singletonList("myMail@example.tld")); // then - verify(player).sendMessage(argThat(containsString("no email address"))); + verify(player).sendMessage(argThat(containsString("not all required settings are set for sending emails"))); + verify(sendMailSsl).hasAllInformation(); verifyZeroInteractions(management); } @@ -155,6 +160,7 @@ public class RegisterCommandTest { given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(true); given(commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL)).willReturn(true); given(commandService.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn("server@example.com"); + given(sendMailSsl.hasAllInformation()).willReturn(true); Player player = mock(Player.class); // when @@ -175,6 +181,7 @@ public class RegisterCommandTest { given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(true); given(commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL)).willReturn(true); given(commandService.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn("server@example.com"); + given(sendMailSsl.hasAllInformation()).willReturn(true); Player player = mock(Player.class); // when @@ -182,6 +189,7 @@ public class RegisterCommandTest { // then verify(commandService).send(player, MessageKey.USAGE_REGISTER); + verify(sendMailSsl).hasAllInformation(); verifyZeroInteractions(management); } @@ -196,6 +204,7 @@ public class RegisterCommandTest { given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(true); given(commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL)).willReturn(true); given(commandService.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn("server@example.com"); + given(sendMailSsl.hasAllInformation()).willReturn(true); Player player = mock(Player.class); // when @@ -203,6 +212,7 @@ public class RegisterCommandTest { // then verify(commandService).validateEmail(playerMail); + verify(sendMailSsl).hasAllInformation(); verify(management).performRegister(eq(player), argThat(stringWithLength(passLength)), eq(playerMail), eq(true)); } @@ -217,7 +227,7 @@ public class RegisterCommandTest { // then verify(commandService).send(player, MessageKey.PASSWORD_MATCH_ERROR); - verifyZeroInteractions(management); + verifyZeroInteractions(management, sendMailSsl); } @Test diff --git a/src/test/java/fr/xephi/authme/datasource/SQLiteIntegrationTest.java b/src/test/java/fr/xephi/authme/datasource/SQLiteIntegrationTest.java index 76d01655b..e672eb49e 100644 --- a/src/test/java/fr/xephi/authme/datasource/SQLiteIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/datasource/SQLiteIntegrationTest.java @@ -120,11 +120,6 @@ public class SQLiteIntegrationTest extends AbstractDataSourceIntegrationTest { assertThat(sqLite.getAllAuths(), hasSize(1)); } - @Test - public void shouldCreate2() throws SQLException { - con.createStatement().execute("SELECT 1 from authme"); - } - @Override protected DataSource getDataSource(String saltColumn) { when(settings.getProperty(DatabaseSettings.MYSQL_COL_SALT)).thenReturn(saltColumn);