From 8ed8b3258984dfa5e9962462cc08eb49facd8c73 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Wed, 13 Jan 2016 22:08:40 +0100 Subject: [PATCH 1/8] #437 Add email should not allow to change email - Create separate test for adding email - Check that no email is yet registered for add email --- .../executable/email/AddEmailCommand.java | 15 +++- .../fr/xephi/authme/process/Management.java | 5 +- .../authme/process/email/AsyncAddEmail.java | 62 +++++++++++++++ .../process/email/AsyncChangeEmail.java | 79 ++++++++----------- .../executable/email/AddEmailCommandTest.java | 6 +- 5 files changed, 112 insertions(+), 55 deletions(-) create mode 100644 src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java diff --git a/src/main/java/fr/xephi/authme/command/executable/email/AddEmailCommand.java b/src/main/java/fr/xephi/authme/command/executable/email/AddEmailCommand.java index 96d9aa7a..663372cd 100644 --- a/src/main/java/fr/xephi/authme/command/executable/email/AddEmailCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/email/AddEmailCommand.java @@ -2,6 +2,9 @@ package fr.xephi.authme.command.executable.email; import fr.xephi.authme.command.CommandService; import fr.xephi.authme.command.PlayerCommand; +import fr.xephi.authme.output.MessageKey; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.util.StringUtils; import org.bukkit.entity.Player; import java.util.List; @@ -10,9 +13,15 @@ public class AddEmailCommand extends PlayerCommand { @Override public void runCommand(Player player, List arguments, CommandService commandService) { - String playerMail = arguments.get(0); - String playerMailVerify = arguments.get(1); + String email = arguments.get(0); + String emailConfirmation = arguments.get(1); - commandService.getManagement().performAddEmail(player, playerMail, playerMailVerify); + if (StringUtils.isEmpty(email) || "your@email.com".equals(email) || !Settings.isEmailCorrect(email)) { + commandService.send(player, MessageKey.INVALID_EMAIL); + } else if (email.equals(emailConfirmation)) { + commandService.getManagement().performAddEmail(player, email); + } else { + commandService.send(player, MessageKey.CONFIRM_EMAIL_MESSAGE); + } } } diff --git a/src/main/java/fr/xephi/authme/process/Management.java b/src/main/java/fr/xephi/authme/process/Management.java index 6d262308..10ceb698 100644 --- a/src/main/java/fr/xephi/authme/process/Management.java +++ b/src/main/java/fr/xephi/authme/process/Management.java @@ -1,6 +1,7 @@ package fr.xephi.authme.process; import fr.xephi.authme.AuthMe; +import fr.xephi.authme.process.email.AsyncAddEmail; import fr.xephi.authme.process.email.AsyncChangeEmail; import fr.xephi.authme.process.join.AsynchronousJoin; import fr.xephi.authme.process.login.AsynchronousLogin; @@ -90,11 +91,11 @@ public class Management { }); } - public void performAddEmail(final Player player, final String newEmail, final String newEmailVerify) { + public void performAddEmail(final Player player, final String newEmail) { sched.runTaskAsynchronously(plugin, new Runnable() { @Override public void run() { - new AsyncChangeEmail(player, plugin, null, newEmail, newEmailVerify).process(); + new AsyncAddEmail(plugin, player, newEmail).process(); } }); } diff --git a/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java b/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java new file mode 100644 index 00000000..23e02902 --- /dev/null +++ b/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java @@ -0,0 +1,62 @@ +package fr.xephi.authme.process.email; + +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.output.MessageKey; +import fr.xephi.authme.output.Messages; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.util.StringUtils; +import org.bukkit.entity.Player; + +/** + * Async task to add an email to an account. + */ +public class AsyncAddEmail { + + private AuthMe plugin; + private Player player; + private String email; + private Messages messages; + + public AsyncAddEmail(AuthMe plugin, Player player, String email) { + this.plugin = plugin; + this.messages = plugin.getMessages(); + this.player = player; + this.email = email; + } + + public void process() { + String playerName = player.getName().toLowerCase(); + PlayerCache playerCache = PlayerCache.getInstance(); + + if (playerCache.isAuthenticated(playerName)) { + PlayerAuth auth = PlayerCache.getInstance().getAuth(playerName); + String currentEmail = auth.getEmail(); + + if (currentEmail == null) { + messages.send(player, MessageKey.USAGE_CHANGE_EMAIL); + } else if (StringUtils.isEmpty(email) || "your@email.com".equals(email) || Settings.isEmailCorrect(email)) { + messages.send(player, MessageKey.INVALID_EMAIL); + } else { + auth.setEmail(email); + playerCache.updatePlayer(auth); + messages.send(player, MessageKey.EMAIL_ADDED_SUCCESS); + } + } else { + sendUnloggedMessage(plugin.getDataSource()); + } + } + + private void sendUnloggedMessage(DataSource dataSource) { + if (dataSource.isAuthAvailable(player.getName())) { + messages.send(player, MessageKey.LOGIN_MESSAGE); + } else if (Settings.emailRegistration) { + messages.send(player, MessageKey.REGISTER_EMAIL_MESSAGE); + } else { + messages.send(player, MessageKey.REGISTER_MESSAGE); + } + } + +} diff --git a/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java b/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java index e59aaab8..3e438661 100644 --- a/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java +++ b/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java @@ -3,6 +3,7 @@ package fr.xephi.authme.process.email; 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.output.MessageKey; import fr.xephi.authme.output.Messages; import fr.xephi.authme.settings.Settings; @@ -17,74 +18,56 @@ public class AsyncChangeEmail { private final AuthMe plugin; private final String oldEmail; private final String newEmail; - private final String newEmailVerify; private final Messages m; - public AsyncChangeEmail(Player player, AuthMe plugin, String oldEmail, String newEmail, String newEmailVerify) { + public AsyncChangeEmail(Player player, AuthMe plugin, String oldEmail, String newEmail) { this.m = plugin.getMessages(); this.player = player; this.plugin = plugin; this.oldEmail = oldEmail; this.newEmail = newEmail; - this.newEmailVerify = newEmailVerify; - } - - public AsyncChangeEmail(Player player, AuthMe plugin, String oldEmail, String newEmail) { - this(player, plugin, oldEmail, newEmail, newEmail); } public void process() { String playerName = player.getName().toLowerCase(); if (PlayerCache.getInstance().isAuthenticated(playerName)) { - if (!newEmail.equals(newEmailVerify)) { - m.send(player, MessageKey.CONFIRM_EMAIL_MESSAGE); - return; - } PlayerAuth auth = PlayerCache.getInstance().getAuth(playerName); String currentEmail = auth.getEmail(); - if (oldEmail != null) { - if (StringUtils.isEmpty(currentEmail) || currentEmail.equals("your@email.com")) { - m.send(player, MessageKey.USAGE_ADD_EMAIL); - return; - } - if (!oldEmail.equals(currentEmail)) { - m.send(player, MessageKey.INVALID_OLD_EMAIL); - return; - } - } else { - if (!StringUtils.isEmpty(currentEmail) && !currentEmail.equals("your@email.com")) { - m.send(player, MessageKey.USAGE_CHANGE_EMAIL); - return; - } - } - if (!Settings.isEmailCorrect(newEmail)) { + + if (currentEmail == null) { + m.send(player, MessageKey.USAGE_ADD_EMAIL); + } else if (StringUtils.isEmpty(newEmail) || "your@email.com".equals(newEmail)) { + m.send(player, MessageKey.INVALID_EMAIL); + } else if (!oldEmail.equals(currentEmail)) { + m.send(player, MessageKey.INVALID_OLD_EMAIL); + } else if (Settings.isEmailCorrect(newEmail)) { m.send(player, MessageKey.INVALID_NEW_EMAIL); - return; - } - auth.setEmail(newEmail); - if (!plugin.getDataSource().updateEmail(auth)) { - m.send(player, MessageKey.ERROR); - auth.setEmail(currentEmail); - return; + } else { + saveNewEmail(auth); } + } else { + outputUnloggedMessage(); + } + } + + private void saveNewEmail(PlayerAuth auth) { + auth.setEmail(newEmail); + if (plugin.getDataSource().updateEmail(auth)) { PlayerCache.getInstance().updatePlayer(auth); - if (oldEmail == null) { - m.send(player, MessageKey.EMAIL_ADDED_SUCCESS); - player.sendMessage(auth.getEmail()); - return; - } m.send(player, MessageKey.EMAIL_CHANGED_SUCCESS); } else { - if (plugin.getDataSource().isAuthAvailable(playerName)) { - m.send(player, MessageKey.LOGIN_MESSAGE); - } else { - if (Settings.emailRegistration) { - m.send(player, MessageKey.REGISTER_EMAIL_MESSAGE); - } else { - m.send(player, MessageKey.REGISTER_MESSAGE); - } - } + m.send(player, MessageKey.ERROR); + auth.setEmail(newEmail); } + } + private void outputUnloggedMessage() { + if (plugin.getDataSource().isAuthAvailable(player.getName())) { + m.send(player, MessageKey.LOGIN_MESSAGE); + } else if (Settings.emailRegistration) { + m.send(player, MessageKey.REGISTER_EMAIL_MESSAGE); + } else { + m.send(player, MessageKey.REGISTER_MESSAGE); + } } } diff --git a/src/test/java/fr/xephi/authme/command/executable/email/AddEmailCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/email/AddEmailCommandTest.java index b255a568..931c9c4e 100644 --- a/src/test/java/fr/xephi/authme/command/executable/email/AddEmailCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/email/AddEmailCommandTest.java @@ -2,6 +2,7 @@ package fr.xephi.authme.command.executable.email; import fr.xephi.authme.command.CommandService; import fr.xephi.authme.process.Management; +import fr.xephi.authme.util.WrapperMock; import org.bukkit.command.BlockCommandSender; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; @@ -27,6 +28,7 @@ public class AddEmailCommandTest { @Before public void setUpMocks() { commandService = mock(CommandService.class); + WrapperMock.createInstance(); } @Test @@ -51,10 +53,10 @@ public class AddEmailCommandTest { given(commandService.getManagement()).willReturn(management); // when - command.executeCommand(sender, Arrays.asList("mail@example", "other_example"), commandService); + command.executeCommand(sender, Arrays.asList("mail@example", "mail@example"), commandService); // then - verify(management).performAddEmail(sender, "mail@example", "other_example"); + verify(management).performAddEmail(sender, "mail@example"); } } From 5996d5808140d980248960b2026971339750a02e Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 15 Jan 2016 20:38:12 +0100 Subject: [PATCH 2/8] #437 Create test for AddEmail task; fix bugs --- .../fr/xephi/authme/process/Management.java | 4 +- .../authme/process/email/AsyncAddEmail.java | 28 +-- .../process/email/AsyncChangeEmail.java | 2 +- .../process/email/AsyncAddEmailTest.java | 168 ++++++++++++++++++ 4 files changed, 189 insertions(+), 13 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/process/email/AsyncAddEmailTest.java diff --git a/src/main/java/fr/xephi/authme/process/Management.java b/src/main/java/fr/xephi/authme/process/Management.java index 10ceb698..81b87630 100644 --- a/src/main/java/fr/xephi/authme/process/Management.java +++ b/src/main/java/fr/xephi/authme/process/Management.java @@ -1,6 +1,7 @@ package fr.xephi.authme.process; import fr.xephi.authme.AuthMe; +import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.process.email.AsyncAddEmail; import fr.xephi.authme.process.email.AsyncChangeEmail; import fr.xephi.authme.process.join.AsynchronousJoin; @@ -95,7 +96,8 @@ public class Management { sched.runTaskAsynchronously(plugin, new Runnable() { @Override public void run() { - new AsyncAddEmail(plugin, player, newEmail).process(); + new AsyncAddEmail(plugin, player, newEmail, plugin.getDataSource(), PlayerCache.getInstance()) + .process(); } }); } diff --git a/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java b/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java index 23e02902..92472ff0 100644 --- a/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java +++ b/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java @@ -15,29 +15,30 @@ import org.bukkit.entity.Player; */ public class AsyncAddEmail { - private AuthMe plugin; - private Player player; - private String email; - private Messages messages; + private final Player player; + private final String email; + private final Messages messages; + private final DataSource dataSource; + private final PlayerCache playerCache; - public AsyncAddEmail(AuthMe plugin, Player player, String email) { - this.plugin = plugin; + public AsyncAddEmail(AuthMe plugin, Player player, String email, DataSource dataSource, PlayerCache playerCache) { this.messages = plugin.getMessages(); this.player = player; this.email = email; + this.dataSource = dataSource; + this.playerCache = playerCache; } public void process() { String playerName = player.getName().toLowerCase(); - PlayerCache playerCache = PlayerCache.getInstance(); if (playerCache.isAuthenticated(playerName)) { - PlayerAuth auth = PlayerCache.getInstance().getAuth(playerName); + PlayerAuth auth = playerCache.getAuth(playerName); String currentEmail = auth.getEmail(); - if (currentEmail == null) { + if (currentEmail != null) { messages.send(player, MessageKey.USAGE_CHANGE_EMAIL); - } else if (StringUtils.isEmpty(email) || "your@email.com".equals(email) || Settings.isEmailCorrect(email)) { + } else if (isEmailInvalid(email)) { messages.send(player, MessageKey.INVALID_EMAIL); } else { auth.setEmail(email); @@ -45,10 +46,15 @@ public class AsyncAddEmail { messages.send(player, MessageKey.EMAIL_ADDED_SUCCESS); } } else { - sendUnloggedMessage(plugin.getDataSource()); + sendUnloggedMessage(dataSource); } } + private static boolean isEmailInvalid(String email) { + return StringUtils.isEmpty(email) || "your@email.com".equals(email) + || !Settings.isEmailCorrect(email); + } + private void sendUnloggedMessage(DataSource dataSource) { if (dataSource.isAuthAvailable(player.getName())) { messages.send(player, MessageKey.LOGIN_MESSAGE); diff --git a/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java b/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java index 3e438661..dface22e 100644 --- a/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java +++ b/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java @@ -3,7 +3,6 @@ package fr.xephi.authme.process.email; 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.output.MessageKey; import fr.xephi.authme.output.Messages; import fr.xephi.authme.settings.Settings; @@ -11,6 +10,7 @@ import fr.xephi.authme.util.StringUtils; import org.bukkit.entity.Player; /** + * Async task for changing the email. */ public class AsyncChangeEmail { diff --git a/src/test/java/fr/xephi/authme/process/email/AsyncAddEmailTest.java b/src/test/java/fr/xephi/authme/process/email/AsyncAddEmailTest.java new file mode 100644 index 00000000..6d73b471 --- /dev/null +++ b/src/test/java/fr/xephi/authme/process/email/AsyncAddEmailTest.java @@ -0,0 +1,168 @@ +package fr.xephi.authme.process.email; + +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.output.MessageKey; +import fr.xephi.authme.output.Messages; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.util.WrapperMock; +import org.bukkit.entity.Player; +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.Test; + +import static org.mockito.BDDMockito.given; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Test for {@link AsyncAddEmail}. + */ +public class AsyncAddEmailTest { + + private Messages messages; + private Player player; + private DataSource dataSource; + private PlayerCache playerCache; + + @BeforeClass + public static void setUp() { + WrapperMock.createInstance(); + } + + // Clean up the fields to ensure that no test uses elements of another test + @After + public void removeFieldValues() { + messages = null; + player = null; + dataSource = null; + playerCache = null; + } + + @Test + public void shouldAddEmail() { + // given + AsyncAddEmail process = createProcess("my.mail@example.org"); + given(player.getName()).willReturn("testEr"); + given(playerCache.isAuthenticated("tester")).willReturn(true); + PlayerAuth auth = mock(PlayerAuth.class); + given(auth.getEmail()).willReturn(null); + given(playerCache.getAuth("tester")).willReturn(auth); + + // when + process.process(); + + // then + verify(messages).send(player, MessageKey.EMAIL_ADDED_SUCCESS); + verify(auth).setEmail("my.mail@example.org"); + verify(playerCache).updatePlayer(auth); + } + + @Test + public void shouldNotAddMailIfPlayerAlreadyHasEmail() { + // given + AsyncAddEmail process = createProcess("some.mail@example.org"); + given(player.getName()).willReturn("my_Player"); + given(playerCache.isAuthenticated("my_player")).willReturn(true); + PlayerAuth auth = mock(PlayerAuth.class); + given(auth.getEmail()).willReturn("another@mail.tld"); + given(playerCache.getAuth("my_player")).willReturn(auth); + + // when + process.process(); + + // then + verify(messages).send(player, MessageKey.USAGE_CHANGE_EMAIL); + verify(playerCache, never()).updatePlayer(any(PlayerAuth.class)); + } + + @Test + public void shouldNotAddMailIfItIsInvalid() { + // given + AsyncAddEmail process = createProcess("invalid_mail"); + given(player.getName()).willReturn("my_Player"); + given(playerCache.isAuthenticated("my_player")).willReturn(true); + PlayerAuth auth = mock(PlayerAuth.class); + given(auth.getEmail()).willReturn(null); + given(playerCache.getAuth("my_player")).willReturn(auth); + + // when + process.process(); + + // then + verify(messages).send(player, MessageKey.INVALID_EMAIL); + verify(playerCache, never()).updatePlayer(any(PlayerAuth.class)); + } + + @Test + public void shouldShowLoginMessage() { + // given + AsyncAddEmail process = createProcess("test@mail.com"); + given(player.getName()).willReturn("Username12"); + given(playerCache.isAuthenticated("username12")).willReturn(false); + given(dataSource.isAuthAvailable("Username12")).willReturn(true); + + // when + process.process(); + + // then + verify(messages).send(player, MessageKey.LOGIN_MESSAGE); + verify(playerCache, never()).updatePlayer(any(PlayerAuth.class)); + } + + @Test + public void shouldShowEmailRegisterMessage() { + // given + AsyncAddEmail process = createProcess("test@mail.com"); + given(player.getName()).willReturn("user"); + given(playerCache.isAuthenticated("user")).willReturn(false); + given(dataSource.isAuthAvailable("user")).willReturn(false); + Settings.emailRegistration = true; + + // when + process.process(); + + // then + verify(messages).send(player, MessageKey.REGISTER_EMAIL_MESSAGE); + verify(playerCache, never()).updatePlayer(any(PlayerAuth.class)); + } + + @Test + public void shouldShowRegularRegisterMessage() { + // given + AsyncAddEmail process = createProcess("test@mail.com"); + given(player.getName()).willReturn("user"); + given(playerCache.isAuthenticated("user")).willReturn(false); + given(dataSource.isAuthAvailable("user")).willReturn(false); + Settings.emailRegistration = false; + + // when + process.process(); + + // then + verify(messages).send(player, MessageKey.REGISTER_MESSAGE); + verify(playerCache, never()).updatePlayer(any(PlayerAuth.class)); + } + + /** + * Create an instance of {@link AsyncAddEmail} and save the mcoks to this class' fields. + * + * @param email The email to use + * @return The created process + */ + private AsyncAddEmail createProcess(String email) { + messages = mock(Messages.class); + AuthMe authMe = mock(AuthMe.class); + when(authMe.getMessages()).thenReturn(messages); + player = mock(Player.class); + dataSource = mock(DataSource.class); + playerCache = mock(PlayerCache.class); + return new AsyncAddEmail(authMe, player, email, dataSource, playerCache); + } + +} From 8c05c8df989e9987a7e7df509e7ea2fd29de35ad Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 15 Jan 2016 21:16:06 +0100 Subject: [PATCH 3/8] Add tests for change email process --- .../fr/xephi/authme/process/Management.java | 2 +- .../process/email/AsyncChangeEmail.java | 22 +-- .../process/email/AsyncChangeEmailTest.java | 133 ++++++++++++++++++ 3 files changed, 147 insertions(+), 10 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/process/email/AsyncChangeEmailTest.java diff --git a/src/main/java/fr/xephi/authme/process/Management.java b/src/main/java/fr/xephi/authme/process/Management.java index 81b87630..374b9b91 100644 --- a/src/main/java/fr/xephi/authme/process/Management.java +++ b/src/main/java/fr/xephi/authme/process/Management.java @@ -106,7 +106,7 @@ public class Management { sched.runTaskAsynchronously(plugin, new Runnable() { @Override public void run() { - new AsyncChangeEmail(player, plugin, oldEmail, newEmail).process(); + new AsyncChangeEmail(player, plugin, oldEmail, newEmail, plugin.getDataSource(), PlayerCache.getInstance()).process(); } }); } diff --git a/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java b/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java index dface22e..2d172bb9 100644 --- a/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java +++ b/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java @@ -3,6 +3,7 @@ package fr.xephi.authme.process.email; 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.output.MessageKey; import fr.xephi.authme.output.Messages; import fr.xephi.authme.settings.Settings; @@ -15,23 +16,26 @@ import org.bukkit.entity.Player; public class AsyncChangeEmail { private final Player player; - private final AuthMe plugin; private final String oldEmail; private final String newEmail; private final Messages m; + private final PlayerCache playerCache; + private final DataSource dataSource; - public AsyncChangeEmail(Player player, AuthMe plugin, String oldEmail, String newEmail) { + public AsyncChangeEmail(Player player, AuthMe plugin, String oldEmail, + String newEmail, DataSource dataSource, PlayerCache playerCache) { this.m = plugin.getMessages(); this.player = player; - this.plugin = plugin; this.oldEmail = oldEmail; this.newEmail = newEmail; + this.playerCache = playerCache; + this.dataSource = dataSource; } public void process() { String playerName = player.getName().toLowerCase(); - if (PlayerCache.getInstance().isAuthenticated(playerName)) { - PlayerAuth auth = PlayerCache.getInstance().getAuth(playerName); + if (playerCache.isAuthenticated(playerName)) { + PlayerAuth auth = playerCache.getAuth(playerName); String currentEmail = auth.getEmail(); if (currentEmail == null) { @@ -40,7 +44,7 @@ public class AsyncChangeEmail { m.send(player, MessageKey.INVALID_EMAIL); } else if (!oldEmail.equals(currentEmail)) { m.send(player, MessageKey.INVALID_OLD_EMAIL); - } else if (Settings.isEmailCorrect(newEmail)) { + } else if (!Settings.isEmailCorrect(newEmail)) { m.send(player, MessageKey.INVALID_NEW_EMAIL); } else { saveNewEmail(auth); @@ -52,8 +56,8 @@ public class AsyncChangeEmail { private void saveNewEmail(PlayerAuth auth) { auth.setEmail(newEmail); - if (plugin.getDataSource().updateEmail(auth)) { - PlayerCache.getInstance().updatePlayer(auth); + if (dataSource.updateEmail(auth)) { + playerCache.updatePlayer(auth); m.send(player, MessageKey.EMAIL_CHANGED_SUCCESS); } else { m.send(player, MessageKey.ERROR); @@ -62,7 +66,7 @@ public class AsyncChangeEmail { } private void outputUnloggedMessage() { - if (plugin.getDataSource().isAuthAvailable(player.getName())) { + if (dataSource.isAuthAvailable(player.getName())) { m.send(player, MessageKey.LOGIN_MESSAGE); } else if (Settings.emailRegistration) { m.send(player, MessageKey.REGISTER_EMAIL_MESSAGE); diff --git a/src/test/java/fr/xephi/authme/process/email/AsyncChangeEmailTest.java b/src/test/java/fr/xephi/authme/process/email/AsyncChangeEmailTest.java new file mode 100644 index 00000000..268e2663 --- /dev/null +++ b/src/test/java/fr/xephi/authme/process/email/AsyncChangeEmailTest.java @@ -0,0 +1,133 @@ +package fr.xephi.authme.process.email; + +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.output.MessageKey; +import fr.xephi.authme.output.Messages; +import fr.xephi.authme.util.WrapperMock; +import org.bukkit.entity.Player; +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.Test; + +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Test for {@link AsyncChangeEmail}. + */ +public class AsyncChangeEmailTest { + + private Player player; + private Messages messages; + private PlayerCache playerCache; + private DataSource dataSource; + + @BeforeClass + public static void setUp() { + WrapperMock.createInstance(); + } + + // Prevent the accidental re-use of a field in another test + @After + public void cleanFields() { + player = null; + messages = null; + playerCache = null; + dataSource = null; + } + + @Test + public void shouldAddEmail() { + // given + AsyncChangeEmail process = createProcess("old@mail.tld", "new@mail.tld"); + given(player.getName()).willReturn("Bobby"); + given(playerCache.isAuthenticated("bobby")).willReturn(true); + PlayerAuth auth = authWithMail("old@mail.tld"); + given(playerCache.getAuth("bobby")).willReturn(auth); + given(dataSource.updateEmail(auth)).willReturn(true); + + // when + process.process(); + + // then + verify(dataSource).updateEmail(auth); + verify(playerCache).updatePlayer(auth); + verify(messages).send(player, MessageKey.EMAIL_CHANGED_SUCCESS); + } + + @Test + public void shouldShowAddEmailUsage() { + // given + AsyncChangeEmail process = createProcess("old@mail.tld", "new@mail.tld"); + given(player.getName()).willReturn("Bobby"); + given(playerCache.isAuthenticated("bobby")).willReturn(true); + PlayerAuth auth = authWithMail(null); + given(playerCache.getAuth("bobby")).willReturn(auth); + + // when + process.process(); + + // then + verify(dataSource, never()).updateEmail(auth); + verify(playerCache, never()).updatePlayer(auth); + verify(messages).send(player, MessageKey.USAGE_ADD_EMAIL); + } + + @Test + public void shouldRejectInvalidNewMail() { + // given + AsyncChangeEmail process = createProcess("old@mail.tld", "bogus"); + given(player.getName()).willReturn("Bobby"); + given(playerCache.isAuthenticated("bobby")).willReturn(true); + PlayerAuth auth = authWithMail("old@mail.tld"); + given(playerCache.getAuth("bobby")).willReturn(auth); + + // when + process.process(); + + // then + verify(dataSource, never()).updateEmail(auth); + verify(playerCache, never()).updatePlayer(auth); + verify(messages).send(player, MessageKey.INVALID_NEW_EMAIL); + } + + @Test + public void shouldRejectInvalidOldEmail() { + // given + AsyncChangeEmail process = createProcess("old@mail.tld", "new@mail.tld"); + given(player.getName()).willReturn("Bobby"); + given(playerCache.isAuthenticated("bobby")).willReturn(true); + PlayerAuth auth = authWithMail("other@address.email"); + given(playerCache.getAuth("bobby")).willReturn(auth); + + // when + process.process(); + + // then + verify(dataSource, never()).updateEmail(auth); + verify(playerCache, never()).updatePlayer(auth); + verify(messages).send(player, MessageKey.INVALID_OLD_EMAIL); + } + + private static PlayerAuth authWithMail(String email) { + PlayerAuth auth = mock(PlayerAuth.class); + when(auth.getEmail()).thenReturn(email); + return auth; + } + + private AsyncChangeEmail createProcess(String oldEmail, String newEmail) { + player = mock(Player.class); + messages = mock(Messages.class); + AuthMe authMe = mock(AuthMe.class); + when(authMe.getMessages()).thenReturn(messages); + playerCache = mock(PlayerCache.class); + dataSource = mock(DataSource.class); + return new AsyncChangeEmail(player, authMe, oldEmail, newEmail, dataSource, playerCache); + } +} From ace95f750abd36e72bca6b70f9862f292f3bf082 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 16 Jan 2016 07:57:43 +0100 Subject: [PATCH 4/8] Add more tests for async change email process --- .../process/email/AsyncChangeEmail.java | 11 ++- .../process/email/AsyncChangeEmailTest.java | 86 +++++++++++++++++-- 2 files changed, 87 insertions(+), 10 deletions(-) diff --git a/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java b/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java index 2d172bb9..c69e3795 100644 --- a/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java +++ b/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java @@ -40,12 +40,10 @@ public class AsyncChangeEmail { if (currentEmail == null) { m.send(player, MessageKey.USAGE_ADD_EMAIL); - } else if (StringUtils.isEmpty(newEmail) || "your@email.com".equals(newEmail)) { - m.send(player, MessageKey.INVALID_EMAIL); + } else if (isEmailInvalid(newEmail)) { + m.send(player, MessageKey.INVALID_NEW_EMAIL); } else if (!oldEmail.equals(currentEmail)) { m.send(player, MessageKey.INVALID_OLD_EMAIL); - } else if (!Settings.isEmailCorrect(newEmail)) { - m.send(player, MessageKey.INVALID_NEW_EMAIL); } else { saveNewEmail(auth); } @@ -54,6 +52,11 @@ public class AsyncChangeEmail { } } + private static boolean isEmailInvalid(String email) { + return StringUtils.isEmpty(email) || "your@email.com".equals(email) + || !Settings.isEmailCorrect(email); + } + private void saveNewEmail(PlayerAuth auth) { auth.setEmail(newEmail); if (dataSource.updateEmail(auth)) { diff --git a/src/test/java/fr/xephi/authme/process/email/AsyncChangeEmailTest.java b/src/test/java/fr/xephi/authme/process/email/AsyncChangeEmailTest.java index 268e2663..92cfec67 100644 --- a/src/test/java/fr/xephi/authme/process/email/AsyncChangeEmailTest.java +++ b/src/test/java/fr/xephi/authme/process/email/AsyncChangeEmailTest.java @@ -6,6 +6,7 @@ import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.output.Messages; +import fr.xephi.authme.settings.Settings; import fr.xephi.authme.util.WrapperMock; import org.bukkit.entity.Player; import org.junit.After; @@ -13,6 +14,7 @@ import org.junit.BeforeClass; import org.junit.Test; import static org.mockito.BDDMockito.given; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -61,6 +63,25 @@ public class AsyncChangeEmailTest { verify(messages).send(player, MessageKey.EMAIL_CHANGED_SUCCESS); } + @Test + public void shouldShowErrorIfSaveFails() { + // given + AsyncChangeEmail process = createProcess("old@mail.tld", "new@mail.tld"); + given(player.getName()).willReturn("Bobby"); + given(playerCache.isAuthenticated("bobby")).willReturn(true); + PlayerAuth auth = authWithMail("old@mail.tld"); + given(playerCache.getAuth("bobby")).willReturn(auth); + given(dataSource.updateEmail(auth)).willReturn(false); + + // when + process.process(); + + // then + verify(dataSource).updateEmail(auth); + verify(playerCache, never()).updatePlayer(auth); + verify(messages).send(player, MessageKey.ERROR); + } + @Test public void shouldShowAddEmailUsage() { // given @@ -74,8 +95,8 @@ public class AsyncChangeEmailTest { process.process(); // then - verify(dataSource, never()).updateEmail(auth); - verify(playerCache, never()).updatePlayer(auth); + verify(dataSource, never()).updateEmail(any(PlayerAuth.class)); + verify(playerCache, never()).updatePlayer(any(PlayerAuth.class)); verify(messages).send(player, MessageKey.USAGE_ADD_EMAIL); } @@ -92,8 +113,8 @@ public class AsyncChangeEmailTest { process.process(); // then - verify(dataSource, never()).updateEmail(auth); - verify(playerCache, never()).updatePlayer(auth); + verify(dataSource, never()).updateEmail(any(PlayerAuth.class)); + verify(playerCache, never()).updatePlayer(any(PlayerAuth.class)); verify(messages).send(player, MessageKey.INVALID_NEW_EMAIL); } @@ -110,11 +131,64 @@ public class AsyncChangeEmailTest { process.process(); // then - verify(dataSource, never()).updateEmail(auth); - verify(playerCache, never()).updatePlayer(auth); + verify(dataSource, never()).updateEmail(any(PlayerAuth.class)); + verify(playerCache, never()).updatePlayer(any(PlayerAuth.class)); verify(messages).send(player, MessageKey.INVALID_OLD_EMAIL); } + @Test + public void shouldSendLoginMessage() { + // given + AsyncChangeEmail process = createProcess("old@mail.tld", "new@mail.tld"); + given(player.getName()).willReturn("Bobby"); + given(playerCache.isAuthenticated("bobby")).willReturn(false); + given(dataSource.isAuthAvailable("Bobby")).willReturn(true); + + // when + process.process(); + + // then + verify(dataSource, never()).updateEmail(any(PlayerAuth.class)); + verify(playerCache, never()).updatePlayer(any(PlayerAuth.class)); + verify(messages).send(player, MessageKey.LOGIN_MESSAGE); + } + + @Test + public void shouldShowEmailRegistrationMessage() { + // given + AsyncChangeEmail process = createProcess("old@mail.tld", "new@mail.tld"); + given(player.getName()).willReturn("Bobby"); + given(playerCache.isAuthenticated("bobby")).willReturn(false); + given(dataSource.isAuthAvailable("Bobby")).willReturn(false); + Settings.emailRegistration = true; + + // when + process.process(); + + // then + verify(dataSource, never()).updateEmail(any(PlayerAuth.class)); + verify(playerCache, never()).updatePlayer(any(PlayerAuth.class)); + verify(messages).send(player, MessageKey.REGISTER_EMAIL_MESSAGE); + } + + @Test + public void shouldShowRegistrationMessage() { + // given + AsyncChangeEmail process = createProcess("old@mail.tld", "new@mail.tld"); + given(player.getName()).willReturn("Bobby"); + given(playerCache.isAuthenticated("bobby")).willReturn(false); + given(dataSource.isAuthAvailable("Bobby")).willReturn(false); + Settings.emailRegistration = false; + + // when + process.process(); + + // then + verify(dataSource, never()).updateEmail(any(PlayerAuth.class)); + verify(playerCache, never()).updatePlayer(any(PlayerAuth.class)); + verify(messages).send(player, MessageKey.REGISTER_MESSAGE); + } + private static PlayerAuth authWithMail(String email) { PlayerAuth auth = mock(PlayerAuth.class); when(auth.getEmail()).thenReturn(email); From b0ba89382723fabfcbf0382e8e9e9a602026898c Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 17 Jan 2016 20:41:19 +0100 Subject: [PATCH 5/8] #437 Add/change email should check if email is already used - Untested/incomplete implementation --- .../authme/datasource/CacheDataSource.java | 5 + .../xephi/authme/datasource/DataSource.java | 2 + .../fr/xephi/authme/datasource/FlatFile.java | 174 +----------------- .../fr/xephi/authme/datasource/MySQL.java | 29 ++- .../fr/xephi/authme/datasource/SQLite.java | 19 ++ .../fr/xephi/authme/output/MessageKey.java | 4 +- .../authme/process/email/AsyncAddEmail.java | 5 +- .../process/email/AsyncChangeEmail.java | 2 + src/main/resources/messages/messages_en.yml | 1 + .../process/email/AsyncAddEmailTest.java | 22 +++ .../process/email/AsyncChangeEmailTest.java | 19 ++ 11 files changed, 105 insertions(+), 177 deletions(-) diff --git a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java index e573c6d0..6fe3f226 100644 --- a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java @@ -289,4 +289,9 @@ public class CacheDataSource implements DataSource { public List getLoggedPlayers() { return new ArrayList<>(PlayerCache.getInstance().getCache().values()); } + + @Override + public boolean isEmailStored(String email) { + return source.isEmailStored(email); + } } diff --git a/src/main/java/fr/xephi/authme/datasource/DataSource.java b/src/main/java/fr/xephi/authme/datasource/DataSource.java index 2f466def..b59e1daa 100644 --- a/src/main/java/fr/xephi/authme/datasource/DataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/DataSource.java @@ -218,6 +218,8 @@ public interface DataSource { */ List getLoggedPlayers(); + boolean isEmailStored(String email); + enum DataSourceType { MYSQL, FILE, diff --git a/src/main/java/fr/xephi/authme/datasource/FlatFile.java b/src/main/java/fr/xephi/authme/datasource/FlatFile.java index 4d7b4b38..fd93047c 100644 --- a/src/main/java/fr/xephi/authme/datasource/FlatFile.java +++ b/src/main/java/fr/xephi/authme/datasource/FlatFile.java @@ -52,13 +52,6 @@ public class FlatFile implements DataSource { } } - /** - * Method isAuthAvailable. - * - * @param user String - * - * @return boolean * @see fr.xephi.authme.datasource.DataSource#isAuthAvailable(String) - */ @Override public synchronized boolean isAuthAvailable(String user) { BufferedReader br = null; @@ -97,13 +90,6 @@ public class FlatFile implements DataSource { return null; } - /** - * Method saveAuth. - * - * @param auth PlayerAuth - * - * @return boolean * @see fr.xephi.authme.datasource.DataSource#saveAuth(PlayerAuth) - */ @Override public synchronized boolean saveAuth(PlayerAuth auth) { if (isAuthAvailable(auth.getNickname())) { @@ -127,13 +113,6 @@ public class FlatFile implements DataSource { return true; } - /** - * Method updatePassword. - * - * @param auth PlayerAuth - * - * @return boolean * @see fr.xephi.authme.datasource.DataSource#updatePassword(PlayerAuth) - */ @Override public synchronized boolean updatePassword(PlayerAuth auth) { return updatePassword(auth.getNickname(), auth.getPassword()); @@ -200,13 +179,6 @@ public class FlatFile implements DataSource { return true; } - /** - * Method updateSession. - * - * @param auth PlayerAuth - * - * @return boolean * @see fr.xephi.authme.datasource.DataSource#updateSession(PlayerAuth) - */ @Override public boolean updateSession(PlayerAuth auth) { if (!isAuthAvailable(auth.getNickname())) { @@ -266,13 +238,6 @@ public class FlatFile implements DataSource { return true; } - /** - * Method updateQuitLoc. - * - * @param auth PlayerAuth - * - * @return boolean * @see fr.xephi.authme.datasource.DataSource#updateQuitLoc(PlayerAuth) - */ @Override public boolean updateQuitLoc(PlayerAuth auth) { if (!isAuthAvailable(auth.getNickname())) { @@ -311,13 +276,6 @@ public class FlatFile implements DataSource { return true; } - /** - * Method getIps. - * - * @param ip String - * - * @return int * @see fr.xephi.authme.datasource.DataSource#getIps(String) - */ @Override public int getIps(String ip) { BufferedReader br = null; @@ -348,13 +306,6 @@ public class FlatFile implements DataSource { } } - /** - * Method purgeDatabase. - * - * @param until long - * - * @return int * @see fr.xephi.authme.datasource.DataSource#purgeDatabase(long) - */ @Override public int purgeDatabase(long until) { BufferedReader br = null; @@ -401,13 +352,6 @@ public class FlatFile implements DataSource { return cleared; } - /** - * Method autoPurgeDatabase. - * - * @param until long - * - * @return List of String * @see fr.xephi.authme.datasource.DataSource#autoPurgeDatabase(long) - */ @Override public List autoPurgeDatabase(long until) { BufferedReader br = null; @@ -454,13 +398,6 @@ public class FlatFile implements DataSource { return cleared; } - /** - * Method removeAuth. - * - * @param user String - * - * @return boolean * @see fr.xephi.authme.datasource.DataSource#removeAuth(String) - */ @Override public synchronized boolean removeAuth(String user) { if (!isAuthAvailable(user)) { @@ -505,13 +442,6 @@ public class FlatFile implements DataSource { return true; } - /** - * Method getAuth. - * - * @param user String - * - * @return PlayerAuth * @see fr.xephi.authme.datasource.DataSource#getAuth(String) - */ @Override public synchronized PlayerAuth getAuth(String user) { BufferedReader br = null; @@ -554,31 +484,14 @@ public class FlatFile implements DataSource { return null; } - /** - * Method close. - * - * @see fr.xephi.authme.datasource.DataSource#close() - */ @Override public synchronized void close() { } - /** - * Method reload. - * - * @see fr.xephi.authme.datasource.DataSource#reload() - */ @Override public void reload() { } - /** - * Method updateEmail. - * - * @param auth PlayerAuth - * - * @return boolean * @see fr.xephi.authme.datasource.DataSource#updateEmail(PlayerAuth) - */ @Override public boolean updateEmail(PlayerAuth auth) { if (!isAuthAvailable(auth.getNickname())) { @@ -617,13 +530,6 @@ public class FlatFile implements DataSource { return true; } - /** - * Method getAllAuthsByName. - * - * @param auth PlayerAuth - * - * @return List of String * @see fr.xephi.authme.datasource.DataSource#getAllAuthsByName(PlayerAuth) - */ @Override public List getAllAuthsByName(PlayerAuth auth) { BufferedReader br = null; @@ -654,13 +560,6 @@ public class FlatFile implements DataSource { } } - /** - * Method getAllAuthsByIp. - * - * @param ip String - * - * @return List of String * @see fr.xephi.authme.datasource.DataSource#getAllAuthsByIp(String) - */ @Override public List getAllAuthsByIp(String ip) { BufferedReader br = null; @@ -691,13 +590,6 @@ public class FlatFile implements DataSource { } } - /** - * Method getAllAuthsByEmail. - * - * @param email String - * - * @return List of String * @see fr.xephi.authme.datasource.DataSource#getAllAuthsByEmail(String) - */ @Override public List getAllAuthsByEmail(String email) { BufferedReader br = null; @@ -728,13 +620,6 @@ public class FlatFile implements DataSource { } } - /** - * Method purgeBanned. - * - * @param banned List of String - * - * @see fr.xephi.authme.datasource.DataSource#purgeBanned(List) - */ @Override public void purgeBanned(List banned) { BufferedReader br = null; @@ -776,64 +661,28 @@ public class FlatFile implements DataSource { } } - /** - * Method getType. - * - * @return DataSourceType * @see fr.xephi.authme.datasource.DataSource#getType() - */ @Override public DataSourceType getType() { return DataSourceType.FILE; } - /** - * Method isLogged. - * - * @param user String - * - * @return boolean * @see fr.xephi.authme.datasource.DataSource#isLogged(String) - */ @Override public boolean isLogged(String user) { return PlayerCache.getInstance().isAuthenticated(user); } - /** - * Method setLogged. - * - * @param user String - * - * @see fr.xephi.authme.datasource.DataSource#setLogged(String) - */ @Override public void setLogged(String user) { } - /** - * Method setUnlogged. - * - * @param user String - * - * @see fr.xephi.authme.datasource.DataSource#setUnlogged(String) - */ @Override public void setUnlogged(String user) { } - /** - * Method purgeLogged. - * - * @see fr.xephi.authme.datasource.DataSource#purgeLogged() - */ @Override public void purgeLogged() { } - /** - * Method getAccountsRegistered. - * - * @return int * @see fr.xephi.authme.datasource.DataSource#getAccountsRegistered() - */ @Override public int getAccountsRegistered() { BufferedReader br = null; @@ -857,14 +706,6 @@ public class FlatFile implements DataSource { return result; } - /** - * Method updateName. - * - * @param oldOne String - * @param newOne String - * - * @see fr.xephi.authme.datasource.DataSource#updateName(String, String) - */ @Override public void updateName(String oldOne, String newOne) { PlayerAuth auth = this.getAuth(oldOne); @@ -873,11 +714,6 @@ public class FlatFile implements DataSource { this.removeAuth(oldOne); } - /** - * Method getAllAuths. - * - * @return List of PlayerAuth * @see fr.xephi.authme.datasource.DataSource#getAllAuths() - */ @Override public List getAllAuths() { BufferedReader br = null; @@ -925,13 +761,13 @@ public class FlatFile implements DataSource { return auths; } - /** - * Method getLoggedPlayers. - * - * @return List of PlayerAuth * @see fr.xephi.authme.datasource.DataSource#getLoggedPlayers() - */ @Override public List getLoggedPlayers() { return new ArrayList<>(); } + + @Override + public boolean isEmailStored(String email) { + throw new UnsupportedOperationException("Flat file no longer supported"); + } } diff --git a/src/main/java/fr/xephi/authme/datasource/MySQL.java b/src/main/java/fr/xephi/authme/datasource/MySQL.java index 9e459ad7..21987160 100644 --- a/src/main/java/fr/xephi/authme/datasource/MySQL.java +++ b/src/main/java/fr/xephi/authme/datasource/MySQL.java @@ -971,9 +971,8 @@ public class MySQL implements DataSource { pst.close(); rs.close(); st.close(); - } catch (Exception ex) { - ConsoleLogger.showError(ex.getMessage()); - ConsoleLogger.writeStackTrace(ex); + } catch (SQLException ex) { + logSqlException(ex); } return auths; } @@ -1015,11 +1014,29 @@ public class MySQL implements DataSource { } auths.add(pAuth); } - } catch (Exception ex) { - ConsoleLogger.showError(ex.getMessage()); - ConsoleLogger.writeStackTrace(ex); + } catch (SQLException ex) { + logSqlException(ex); } return auths; } + @Override + public synchronized boolean isEmailStored(String email) { + String sql = "SELECT 1 FROM " + tableName + " WHERE " + columnEmail + " = ?"; + try (Connection con = ds.getConnection()) { + PreparedStatement pst = con.prepareStatement(sql); + pst.setString(1, email); + ResultSet rs = pst.executeQuery(); + return rs.next(); + } catch (SQLException e) { + logSqlException(e); + } + return false; + } + + private static void logSqlException(SQLException e) { + ConsoleLogger.showError("Error executing SQL query: " + StringUtils.formatException(e)); + ConsoleLogger.writeStackTrace(e); + } + } diff --git a/src/main/java/fr/xephi/authme/datasource/SQLite.java b/src/main/java/fr/xephi/authme/datasource/SQLite.java index 9b14b1cc..6ef8a406 100644 --- a/src/main/java/fr/xephi/authme/datasource/SQLite.java +++ b/src/main/java/fr/xephi/authme/datasource/SQLite.java @@ -684,6 +684,25 @@ public class SQLite implements DataSource { return auths; } + @Override + public synchronized boolean isEmailStored(String email) { + try { + PreparedStatement ps = con.prepareStatement( + "SELECT 1 FROM " + tableName + " WHERE LOWER(" + columnEmail + ") = LOWER(?)"); + ps.setString(1, email); + ResultSet rs = ps.executeQuery(); + return rs.next(); + } catch (SQLException e) { + logSqlException(e); + } + return false; + } + + private static void logSqlException(SQLException e) { + ConsoleLogger.showError("Error while executing SQL statement: " + StringUtils.formatException(e)); + ConsoleLogger.writeStackTrace(e); + } + private PlayerAuth buildAuthFromResultSet(ResultSet row) throws SQLException { String salt = !columnSalt.isEmpty() ? row.getString(columnSalt) : null; diff --git a/src/main/java/fr/xephi/authme/output/MessageKey.java b/src/main/java/fr/xephi/authme/output/MessageKey.java index fe006686..0f33dc73 100644 --- a/src/main/java/fr/xephi/authme/output/MessageKey.java +++ b/src/main/java/fr/xephi/authme/output/MessageKey.java @@ -121,7 +121,9 @@ public enum MessageKey { ANTIBOT_AUTO_ENABLED_MESSAGE("antibot_auto_enabled"), - ANTIBOT_AUTO_DISABLED_MESSAGE("antibot_auto_disabled", "%m"); + ANTIBOT_AUTO_DISABLED_MESSAGE("antibot_auto_disabled", "%m"), + + EMAIL_ALREADY_USED_ERROR("email_already_used"); private String key; diff --git a/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java b/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java index 92472ff0..a096d150 100644 --- a/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java +++ b/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java @@ -36,10 +36,13 @@ public class AsyncAddEmail { PlayerAuth auth = playerCache.getAuth(playerName); String currentEmail = auth.getEmail(); - if (currentEmail != null) { + if (currentEmail != null && !"your@mail.com".equals(currentEmail)) { + System.out.println("Email is currentEmail " + currentEmail); // FIXME remove messages.send(player, MessageKey.USAGE_CHANGE_EMAIL); } else if (isEmailInvalid(email)) { messages.send(player, MessageKey.INVALID_EMAIL); + } else if (dataSource.isEmailStored(email)) { + messages.send(player, MessageKey.EMAIL_ALREADY_USED_ERROR); } else { auth.setEmail(email); playerCache.updatePlayer(auth); diff --git a/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java b/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java index c69e3795..a17c262d 100644 --- a/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java +++ b/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java @@ -44,6 +44,8 @@ public class AsyncChangeEmail { m.send(player, MessageKey.INVALID_NEW_EMAIL); } else if (!oldEmail.equals(currentEmail)) { m.send(player, MessageKey.INVALID_OLD_EMAIL); + } else if (dataSource.isEmailStored(newEmail)) { + m.send(player, MessageKey.EMAIL_ALREADY_USED_ERROR); } else { saveNewEmail(auth); } diff --git a/src/main/resources/messages/messages_en.yml b/src/main/resources/messages/messages_en.yml index 2d26cb1f..30859386 100644 --- a/src/main/resources/messages/messages_en.yml +++ b/src/main/resources/messages/messages_en.yml @@ -57,3 +57,4 @@ email_exists: '&cA recovery email was already sent! You can discard it and send country_banned: '&4Your country is banned from this server!' antibot_auto_enabled: '&4[AntiBotService] AntiBot enabled due to the huge number of connections!' antibot_auto_disabled: '&2[AntiBotService] AntiBot disabled disabled after %m minutes!' +email_already_used: '&4The email address is already being used' diff --git a/src/test/java/fr/xephi/authme/process/email/AsyncAddEmailTest.java b/src/test/java/fr/xephi/authme/process/email/AsyncAddEmailTest.java index 6d73b471..97a6a990 100644 --- a/src/test/java/fr/xephi/authme/process/email/AsyncAddEmailTest.java +++ b/src/test/java/fr/xephi/authme/process/email/AsyncAddEmailTest.java @@ -53,6 +53,7 @@ public class AsyncAddEmailTest { PlayerAuth auth = mock(PlayerAuth.class); given(auth.getEmail()).willReturn(null); given(playerCache.getAuth("tester")).willReturn(auth); + given(dataSource.isEmailStored("my.mail@example.org")).willReturn(false); // when process.process(); @@ -72,6 +73,7 @@ public class AsyncAddEmailTest { PlayerAuth auth = mock(PlayerAuth.class); given(auth.getEmail()).willReturn("another@mail.tld"); given(playerCache.getAuth("my_player")).willReturn(auth); + given(dataSource.isEmailStored("some.mail@example.org")).willReturn(false); // when process.process(); @@ -90,6 +92,7 @@ public class AsyncAddEmailTest { PlayerAuth auth = mock(PlayerAuth.class); given(auth.getEmail()).willReturn(null); given(playerCache.getAuth("my_player")).willReturn(auth); + given(dataSource.isEmailStored("invalid_mail")).willReturn(false); // when process.process(); @@ -99,6 +102,25 @@ public class AsyncAddEmailTest { verify(playerCache, never()).updatePlayer(any(PlayerAuth.class)); } + @Test + public void shouldNotAddMailIfAlreadyUsed() { + // given + AsyncAddEmail process = createProcess("player@mail.tld"); + given(player.getName()).willReturn("TestName"); + given(playerCache.isAuthenticated("testname")).willReturn(true); + PlayerAuth auth = mock(PlayerAuth.class); + given(auth.getEmail()).willReturn(null); + given(playerCache.getAuth("testname")).willReturn(auth); + given(dataSource.isEmailStored("player@mail.tld")).willReturn(true); + + // when + process.process(); + + // then + verify(messages).send(player, MessageKey.EMAIL_ALREADY_USED_ERROR); + verify(playerCache, never()).updatePlayer(any(PlayerAuth.class)); + } + @Test public void shouldShowLoginMessage() { // given diff --git a/src/test/java/fr/xephi/authme/process/email/AsyncChangeEmailTest.java b/src/test/java/fr/xephi/authme/process/email/AsyncChangeEmailTest.java index 92cfec67..4faf1483 100644 --- a/src/test/java/fr/xephi/authme/process/email/AsyncChangeEmailTest.java +++ b/src/test/java/fr/xephi/authme/process/email/AsyncChangeEmailTest.java @@ -136,6 +136,25 @@ public class AsyncChangeEmailTest { verify(messages).send(player, MessageKey.INVALID_OLD_EMAIL); } + @Test + public void shouldRejectAlreadyUsedEmail() { + // given + AsyncChangeEmail process = createProcess("old@example.com", "new@example.com"); + given(player.getName()).willReturn("Username"); + given(playerCache.isAuthenticated("username")).willReturn(true); + PlayerAuth auth = authWithMail("old@example.com"); + given(playerCache.getAuth("username")).willReturn(auth); + given(dataSource.isEmailStored("new@example.com")).willReturn(true); + + // when + process.process(); + + // then + verify(dataSource, never()).updateEmail(any(PlayerAuth.class)); + verify(playerCache, never()).updatePlayer(any(PlayerAuth.class)); + verify(messages).send(player, MessageKey.EMAIL_ALREADY_USED_ERROR); + } + @Test public void shouldSendLoginMessage() { // given From 125c45d7153f07f0388175a52103e4ea6288a83a Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 17 Jan 2016 20:46:19 +0100 Subject: [PATCH 6/8] Minor - remove debug line --- src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java b/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java index a096d150..f98817d3 100644 --- a/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java +++ b/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java @@ -37,7 +37,6 @@ public class AsyncAddEmail { String currentEmail = auth.getEmail(); if (currentEmail != null && !"your@mail.com".equals(currentEmail)) { - System.out.println("Email is currentEmail " + currentEmail); // FIXME remove messages.send(player, MessageKey.USAGE_CHANGE_EMAIL); } else if (isEmailInvalid(email)) { messages.send(player, MessageKey.INVALID_EMAIL); From b432223b8823107e116b3352b3e410369f4b845f Mon Sep 17 00:00:00 2001 From: ljacqu Date: Tue, 19 Jan 2016 17:15:49 +0100 Subject: [PATCH 7/8] #437 Avoid LOWER() for SQLite - Implement review comment by DNx5 - avoid use of LOWER() - Close PreparedStatement/ResultSet in call --- .../java/fr/xephi/authme/datasource/SQLite.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/fr/xephi/authme/datasource/SQLite.java b/src/main/java/fr/xephi/authme/datasource/SQLite.java index 6ef8a406..7e17de90 100644 --- a/src/main/java/fr/xephi/authme/datasource/SQLite.java +++ b/src/main/java/fr/xephi/authme/datasource/SQLite.java @@ -170,8 +170,7 @@ public class SQLite implements DataSource { !columnSalt.isEmpty() ? rs.getString(columnSalt) : null); } } catch (SQLException ex) { - ConsoleLogger.showError(ex.getMessage()); - ConsoleLogger.writeStackTrace(ex); + logSqlException(ex); } finally { close(rs); close(pst); @@ -462,7 +461,7 @@ public class SQLite implements DataSource { } return countIp; } catch (SQLException ex) { - ConsoleLogger.showError(ex.getMessage()); + logSqlException(ex); return new ArrayList<>(); } catch (NullPointerException npe) { return new ArrayList<>(); @@ -530,7 +529,7 @@ public class SQLite implements DataSource { pst.executeUpdate(); } } catch (SQLException ex) { - ConsoleLogger.showError(ex.getMessage()); + logSqlException(ex); } finally { close(pst); } @@ -686,14 +685,16 @@ public class SQLite implements DataSource { @Override public synchronized boolean isEmailStored(String email) { - try { - PreparedStatement ps = con.prepareStatement( - "SELECT 1 FROM " + tableName + " WHERE LOWER(" + columnEmail + ") = LOWER(?)"); + String sql = "SELECT 1 FROM " + tableName + " WHERE " + columnEmail + " = ? COLLATE NOCASE;"; + ResultSet rs = null; + try (PreparedStatement ps = con.prepareStatement(sql)) { ps.setString(1, email); - ResultSet rs = ps.executeQuery(); + rs = ps.executeQuery(); return rs.next(); } catch (SQLException e) { logSqlException(e); + } finally { + close(rs); } return false; } From 2cd2b48a1a5b03d6ee0d103160a72ff6699caeb5 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 6 Feb 2016 21:56:08 +0100 Subject: [PATCH 8/8] #437 Email uniqueness in admin command; finalization - Check also in admin command that email is not already used - Misc bug fixing (logic errors, changes lost during large merge) - Use "email" and "setemail" as main labels for /authme subcommands --- .../xephi/authme/command/CommandInitializer.java | 4 ++-- .../command/executable/authme/SetEmailCommand.java | 3 +++ .../java/fr/xephi/authme/process/Management.java | 4 +++- .../xephi/authme/process/email/AsyncAddEmail.java | 14 ++++---------- .../authme/process/email/AsyncChangeEmail.java | 10 ++-------- .../authme/process/email/AsyncAddEmailTest.java | 2 +- 6 files changed, 15 insertions(+), 22 deletions(-) diff --git a/src/main/java/fr/xephi/authme/command/CommandInitializer.java b/src/main/java/fr/xephi/authme/command/CommandInitializer.java index 0a52c700..0a29b109 100644 --- a/src/main/java/fr/xephi/authme/command/CommandInitializer.java +++ b/src/main/java/fr/xephi/authme/command/CommandInitializer.java @@ -133,7 +133,7 @@ public final class CommandInitializer { // Register the getemail command CommandDescription.builder() .parent(AUTHME_BASE) - .labels("getemail", "getmail", "email", "mail") + .labels("email", "mail", "getemail", "getmail") .description("Display player's email") .detailedDescription("Display the email address of the specified player if set.") .withArgument("player", "Player name", true) @@ -144,7 +144,7 @@ public final class CommandInitializer { // Register the setemail command CommandDescription.builder() .parent(AUTHME_BASE) - .labels("chgemail", "chgmail", "setemail", "setmail") + .labels("setemail", "setmail", "chgemail", "chgmail") .description("Change player's email") .detailedDescription("Change the email address of the specified player.") .withArgument("player", "Player name", false) diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/SetEmailCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/SetEmailCommand.java index ca794bb4..d19a2ce6 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/SetEmailCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/SetEmailCommand.java @@ -33,6 +33,9 @@ public class SetEmailCommand implements ExecutableCommand { if (auth == null) { commandService.send(sender, MessageKey.UNKNOWN_USER); return; + } else if (commandService.getDataSource().isEmailStored(playerEmail)) { + commandService.send(sender, MessageKey.EMAIL_ALREADY_USED_ERROR); + return; } // Set the email address diff --git a/src/main/java/fr/xephi/authme/process/Management.java b/src/main/java/fr/xephi/authme/process/Management.java index ec48328f..ace554b7 100644 --- a/src/main/java/fr/xephi/authme/process/Management.java +++ b/src/main/java/fr/xephi/authme/process/Management.java @@ -2,6 +2,7 @@ package fr.xephi.authme.process; import fr.xephi.authme.AuthMe; import fr.xephi.authme.cache.auth.PlayerCache; +import fr.xephi.authme.process.email.AsyncAddEmail; import fr.xephi.authme.process.email.AsyncChangeEmail; import fr.xephi.authme.process.join.AsynchronousJoin; import fr.xephi.authme.process.login.AsynchronousLogin; @@ -99,7 +100,8 @@ public class Management { sched.runTaskAsynchronously(plugin, new Runnable() { @Override public void run() { - new AsyncChangeEmail(player, plugin, null, newEmail, plugin.getDataSource(), PlayerCache.getInstance(), settings).process(); + new AsyncAddEmail(player, plugin, newEmail, plugin.getDataSource(), + PlayerCache.getInstance(), settings).process(); } }); } diff --git a/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java b/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java index 5f0ef8fc..c7673f70 100644 --- a/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java +++ b/src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java @@ -8,7 +8,6 @@ import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.output.Messages; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.Utils; import org.bukkit.entity.Player; @@ -24,7 +23,7 @@ public class AsyncAddEmail { private final PlayerCache playerCache; private final NewSetting settings; - public AsyncAddEmail(AuthMe plugin, Player player, String email, DataSource dataSource, + public AsyncAddEmail(Player player, AuthMe plugin, String email, DataSource dataSource, PlayerCache playerCache, NewSetting settings) { this.messages = plugin.getMessages(); this.player = player; @@ -39,11 +38,11 @@ public class AsyncAddEmail { if (playerCache.isAuthenticated(playerName)) { PlayerAuth auth = playerCache.getAuth(playerName); - String currentEmail = auth.getEmail(); + final String currentEmail = auth.getEmail(); - if (currentEmail != null && !"your@mail.com".equals(currentEmail)) { + if (currentEmail != null && !"your@email.com".equals(currentEmail)) { messages.send(player, MessageKey.USAGE_CHANGE_EMAIL); - } else if (isEmailInvalid(email)) { + } else if (!Utils.isEmailCorrect(email, settings)) { messages.send(player, MessageKey.INVALID_EMAIL); } else if (dataSource.isEmailStored(email)) { messages.send(player, MessageKey.EMAIL_ALREADY_USED_ERROR); @@ -57,11 +56,6 @@ public class AsyncAddEmail { } } - private boolean isEmailInvalid(String email) { - return StringUtils.isEmpty(email) || "your@email.com".equals(email) - || !Utils.isEmailCorrect(email, settings); - } - private void sendUnloggedMessage(DataSource dataSource) { if (dataSource.isAuthAvailable(player.getName())) { messages.send(player, MessageKey.LOGIN_MESSAGE); diff --git a/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java b/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java index 791802fc..f9bbbd07 100644 --- a/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java +++ b/src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java @@ -8,7 +8,6 @@ import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.output.Messages; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.Utils; import org.bukkit.entity.Player; @@ -40,11 +39,11 @@ public class AsyncChangeEmail { String playerName = player.getName().toLowerCase(); if (playerCache.isAuthenticated(playerName)) { PlayerAuth auth = playerCache.getAuth(playerName); - String currentEmail = auth.getEmail(); + final String currentEmail = auth.getEmail(); if (currentEmail == null) { m.send(player, MessageKey.USAGE_ADD_EMAIL); - } else if (isEmailInvalid(newEmail)) { + } else if (newEmail == null || !Utils.isEmailCorrect(newEmail, settings)) { m.send(player, MessageKey.INVALID_NEW_EMAIL); } else if (!oldEmail.equals(currentEmail)) { m.send(player, MessageKey.INVALID_OLD_EMAIL); @@ -58,11 +57,6 @@ public class AsyncChangeEmail { } } - private boolean isEmailInvalid(String email) { - return StringUtils.isEmpty(email) || "your@email.com".equals(email) - || !Utils.isEmailCorrect(email, settings); - } - private void saveNewEmail(PlayerAuth auth) { auth.setEmail(newEmail); if (dataSource.updateEmail(auth)) { diff --git a/src/test/java/fr/xephi/authme/process/email/AsyncAddEmailTest.java b/src/test/java/fr/xephi/authme/process/email/AsyncAddEmailTest.java index 82eb77f1..4aea5e57 100644 --- a/src/test/java/fr/xephi/authme/process/email/AsyncAddEmailTest.java +++ b/src/test/java/fr/xephi/authme/process/email/AsyncAddEmailTest.java @@ -187,7 +187,7 @@ public class AsyncAddEmailTest { dataSource = mock(DataSource.class); playerCache = mock(PlayerCache.class); settings = mock(NewSetting.class); - return new AsyncAddEmail(authMe, player, email, dataSource, playerCache, settings); + return new AsyncAddEmail(player, authMe, email, dataSource, playerCache, settings); } }