diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 15a37ab48..3e587d061 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -12,6 +12,7 @@ import fr.xephi.authme.initialization.OnShutdownPlayerSaver; import fr.xephi.authme.initialization.OnStartupTasks; import fr.xephi.authme.initialization.SettingsProvider; import fr.xephi.authme.initialization.TaskCloser; +import fr.xephi.authme.initialization.circulardependency.CircularDependencyInitializer; import fr.xephi.authme.listener.BlockListener; import fr.xephi.authme.listener.EntityListener; import fr.xephi.authme.listener.PlayerListener; @@ -216,6 +217,7 @@ public class AuthMe extends JavaPlugin { // Set all service fields on the AuthMe class instantiateServices(injector); + injector.newInstance(CircularDependencyInitializer.class).initializeCircularDependencies(); // Convert deprecated PLAINTEXT hash entries MigrationService.changePlainTextToSha256(settings, database, new Sha256()); diff --git a/src/main/java/fr/xephi/authme/data/AbstractCaptchaManager.java b/src/main/java/fr/xephi/authme/data/AbstractCaptchaManager.java index 10cfbb86f..ce6766775 100644 --- a/src/main/java/fr/xephi/authme/data/AbstractCaptchaManager.java +++ b/src/main/java/fr/xephi/authme/data/AbstractCaptchaManager.java @@ -72,6 +72,8 @@ public abstract class AbstractCaptchaManager implements SettingsDependent, HasCl captchaCodes.remove(nameLowerCase); processSuccessfulCode(nameLowerCase); return true; + } else { + processUnsuccessfulCode(nameLowerCase); } return false; } @@ -106,6 +108,14 @@ public abstract class AbstractCaptchaManager implements SettingsDependent, HasCl */ protected abstract void processSuccessfulCode(String nameLower); + /** + * Called when a player has failed the captcha code. + * + * @param nameLower the player's name (all lowercase) + */ + protected void processUnsuccessfulCode(String nameLower) { + } + /** * Returns the number of minutes a generated captcha code should live for before it may expire. * diff --git a/src/main/java/fr/xephi/authme/data/RegistrationCaptchaManager.java b/src/main/java/fr/xephi/authme/data/RegistrationCaptchaManager.java index 0bf6e3aea..7022356d1 100644 --- a/src/main/java/fr/xephi/authme/data/RegistrationCaptchaManager.java +++ b/src/main/java/fr/xephi/authme/data/RegistrationCaptchaManager.java @@ -1,8 +1,13 @@ package fr.xephi.authme.data; +import fr.xephi.authme.data.limbo.LimboService; +import fr.xephi.authme.initialization.circulardependency.HasCircularDependency; +import fr.xephi.authme.initialization.circulardependency.InjectAfterInitialization; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.util.expiring.ExpiringSet; +import org.bukkit.Bukkit; +import org.bukkit.entity.Player; import javax.inject.Inject; import java.util.concurrent.TimeUnit; @@ -10,10 +15,12 @@ import java.util.concurrent.TimeUnit; /** * Captcha manager for registration. */ -public class RegistrationCaptchaManager extends AbstractCaptchaManager { +public class RegistrationCaptchaManager extends AbstractCaptchaManager implements HasCircularDependency { private static final int MINUTES_VALID_FOR_REGISTRATION = 30; + private LimboService limboService; + private final ExpiringSet verifiedNamesForRegistration = new ExpiringSet<>(MINUTES_VALID_FOR_REGISTRATION, TimeUnit.MINUTES); private boolean isEnabled; @@ -46,8 +53,19 @@ public class RegistrationCaptchaManager extends AbstractCaptchaManager { verifiedNamesForRegistration.add(nameLower); } + @Override + protected void processUnsuccessfulCode(String nameLower) { + final Player player = Bukkit.getPlayerExact(nameLower); // TODO #930: Pass in player! + limboService.resetMessageTask(player, false); + } + @Override protected int minutesBeforeCodeExpires(Settings settings) { return MINUTES_VALID_FOR_REGISTRATION; } + + @InjectAfterInitialization + public void setLimboService(LimboService limboService) { + this.limboService = limboService; + } } diff --git a/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManager.java b/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManager.java index 4e0c0a336..0ad2b793a 100644 --- a/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManager.java +++ b/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManager.java @@ -1,5 +1,6 @@ package fr.xephi.authme.data.limbo; +import fr.xephi.authme.data.RegistrationCaptchaManager; import fr.xephi.authme.data.auth.PlayerCache; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.message.Messages; @@ -33,6 +34,9 @@ class LimboPlayerTaskManager { @Inject private PlayerCache playerCache; + @Inject + private RegistrationCaptchaManager registrationCaptchaManager; + LimboPlayerTaskManager() { } @@ -41,14 +45,14 @@ class LimboPlayerTaskManager { * * @param player the player * @param limbo the associated limbo player of the player - * @param isRegistered whether the player is registered or not - * (false shows "please register", true shows "please log in") + * @param isRegistered whether the player is registered or not (needed to determine the message in the task) */ void registerMessageTask(Player player, LimboPlayer limbo, boolean isRegistered) { int interval = settings.getProperty(RegistrationSettings.MESSAGE_INTERVAL); - MessageKey key = getMessageKey(isRegistered); + MessageResult messageResult = getMessageKey(player.getName(), isRegistered); if (interval > 0) { - MessageTask messageTask = new MessageTask(player, messages.retrieve(key)); + String[] joinMessage = messages.retrieveSingle(messageResult.messageKey, messageResult.args).split("\n"); + MessageTask messageTask = new MessageTask(player, joinMessage); bukkitService.runTaskTimer(messageTask, 2 * TICKS_PER_SECOND, interval * TICKS_PER_SECOND); limbo.setMessageTask(messageTask); } @@ -84,14 +88,28 @@ class LimboPlayerTaskManager { /** * Returns the appropriate message key according to the registration status and settings. * + * @param name the player's name * @param isRegistered whether or not the username is registered * @return the message key to display to the user */ - private static MessageKey getMessageKey(boolean isRegistered) { + private MessageResult getMessageKey(String name, boolean isRegistered) { if (isRegistered) { - return MessageKey.LOGIN_MESSAGE; + return new MessageResult(MessageKey.LOGIN_MESSAGE); + } else if (registrationCaptchaManager.isCaptchaRequired(name)) { + final String captchaCode = registrationCaptchaManager.getCaptchaCodeOrGenerateNew(name); + return new MessageResult(MessageKey.CAPTCHA_FOR_REGISTRATION_REQUIRED, captchaCode); } else { - return MessageKey.REGISTER_MESSAGE; + return new MessageResult(MessageKey.REGISTER_MESSAGE); + } + } + + private static final class MessageResult { + private final MessageKey messageKey; + private final String[] args; + + MessageResult(MessageKey messageKey, String... args) { + this.messageKey = messageKey; + this.args = args; } } } diff --git a/src/main/java/fr/xephi/authme/initialization/circulardependency/CircularDependencyInitializer.java b/src/main/java/fr/xephi/authme/initialization/circulardependency/CircularDependencyInitializer.java new file mode 100644 index 000000000..beca17925 --- /dev/null +++ b/src/main/java/fr/xephi/authme/initialization/circulardependency/CircularDependencyInitializer.java @@ -0,0 +1,69 @@ +package fr.xephi.authme.initialization.circulardependency; + +import ch.jalu.injector.Injector; +import ch.jalu.injector.utils.ReflectionUtils; + +import javax.inject.Inject; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; + +/** + * Fix for circular dependencies: after initialization this class can be called + * to inject dependencies via {@link InjectAfterInitialization} methods. + */ +public class CircularDependencyInitializer { + + @Inject + private Injector injector; + + CircularDependencyInitializer() { + } + + /** + * Processes all known {@link HasCircularDependency} classes, invoking all methods + * annotated with {@link InjectAfterInitialization}. + */ + public void initializeCircularDependencies() { + for (HasCircularDependency hasCircularDependency : injector.retrieveAllOfType(HasCircularDependency.class)) { + processClass(hasCircularDependency); + } + } + + private void processClass(HasCircularDependency object) { + for (Method method : object.getClass().getDeclaredMethods()) { + if (method.isAnnotationPresent(InjectAfterInitialization.class)) { + Object resolvedObject = resolveParameterForMethodOrThrow(method); + ReflectionUtils.invokeMethod(method, object, resolvedObject); + } + } + } + + /** + * Validates that the given method is a valid {@link InjectAfterInitialization} method + * and resolves the parameter it should be passed (assumes a singleton of the given type is registered + * in the injector). Throws an exception if the parameter type is not a registered singleton. + * + * @param method the method to process + * @return object to pass to the initializer method + */ + private Object resolveParameterForMethodOrThrow(Method method) { + if (method.getParameterCount() != 1) { + throw new IllegalStateException("Method " + method.getDeclaringClass() + "#" + method.getName() + + " should have one parameter only"); + } else if (!Modifier.isPublic(method.getModifiers())) { + throw new IllegalStateException("Method " + method.getDeclaringClass() + "#" + method.getName() + + " should be public"); + } else if (method.getReturnType() != void.class) { + throw new IllegalStateException("Method " + method.getDeclaringClass() + "#" + method.getName() + + " must return void"); + } + + final Class requiredType = method.getParameterTypes()[0]; + final Object resolvedObject = injector.getIfAvailable(requiredType); + if (resolvedObject == null) { + throw new IllegalStateException("Failed to get parameter of type '" + requiredType + + "' for @InjectAfterInitialization method " + method.getDeclaringClass() + "#" + method.getName()); + } + return resolvedObject; + } +} diff --git a/src/main/java/fr/xephi/authme/initialization/circulardependency/HasCircularDependency.java b/src/main/java/fr/xephi/authme/initialization/circulardependency/HasCircularDependency.java new file mode 100644 index 000000000..4805b60ea --- /dev/null +++ b/src/main/java/fr/xephi/authme/initialization/circulardependency/HasCircularDependency.java @@ -0,0 +1,7 @@ +package fr.xephi.authme.initialization.circulardependency; + +/** + * Marker interface for classes with {@link HasCircularDependency} methods. + */ +public interface HasCircularDependency { +} diff --git a/src/main/java/fr/xephi/authme/initialization/circulardependency/InjectAfterInitialization.java b/src/main/java/fr/xephi/authme/initialization/circulardependency/InjectAfterInitialization.java new file mode 100644 index 000000000..cc35e56c0 --- /dev/null +++ b/src/main/java/fr/xephi/authme/initialization/circulardependency/InjectAfterInitialization.java @@ -0,0 +1,21 @@ +package fr.xephi.authme.initialization.circulardependency; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marks a method that should be invoked once injection has taken place. + * This is a fix for circular dependencies. + *

+ * Methods with this annotation must have exactly one parameter whose type is a singleton + * registered in the injector. Classes with such methods must implement the {@link HasCircularDependency} + * marker interface. + * + * @see CircularDependencyInitializer + */ +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.METHOD) +public @interface InjectAfterInitialization { +} diff --git a/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java b/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java index e64d0f7e9..236cc57c4 100644 --- a/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java +++ b/src/test/java/fr/xephi/authme/AuthMeInitializationTest.java @@ -6,8 +6,11 @@ import ch.jalu.injector.InjectorBuilder; import com.google.common.io.Files; import fr.xephi.authme.api.v3.AuthMeApi; import fr.xephi.authme.command.CommandHandler; +import fr.xephi.authme.data.RegistrationCaptchaManager; +import fr.xephi.authme.data.limbo.LimboService; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.initialization.DataFolder; +import fr.xephi.authme.initialization.circulardependency.CircularDependencyInitializer; import fr.xephi.authme.listener.BlockListener; import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.process.Management; @@ -39,6 +42,7 @@ import java.util.logging.Logger; import static fr.xephi.authme.settings.properties.AuthMeSettingsRetriever.buildConfigurationData; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.sameInstance; import static org.junit.Assert.assertThat; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -108,6 +112,7 @@ public class AuthMeInitializationTest { // when authMe.instantiateServices(injector); authMe.registerEventListeners(injector); + injector.newInstance(CircularDependencyInitializer.class).initializeCircularDependencies(); // then // Take a few samples and ensure that they are not null @@ -121,6 +126,17 @@ public class AuthMeInitializationTest { assertThat(injector.getIfAvailable(PermissionsManager.class), not(nullValue())); assertThat(injector.getIfAvailable(ProcessSyncPlayerLogin.class), not(nullValue())); assertThat(injector.getIfAvailable(PurgeService.class), not(nullValue())); + + assertCircularDependencyWasSet(injector); } + private void assertCircularDependencyWasSet(Injector injector) { + RegistrationCaptchaManager registrationCaptchaManager = injector.getIfAvailable(RegistrationCaptchaManager.class); + LimboService limboServiceOnCaptchaManager = ReflectionTestUtils.getFieldValue( + RegistrationCaptchaManager.class, registrationCaptchaManager, "limboService"); + LimboService limboServiceFromInjector = injector.getIfAvailable(LimboService.class); + + assertThat(limboServiceOnCaptchaManager, not(nullValue())); + assertThat(limboServiceOnCaptchaManager, sameInstance(limboServiceFromInjector)); + } } diff --git a/src/test/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManagerTest.java b/src/test/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManagerTest.java index 551d0a7da..e09aadc71 100644 --- a/src/test/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManagerTest.java +++ b/src/test/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManagerTest.java @@ -1,6 +1,7 @@ package fr.xephi.authme.data.limbo; import fr.xephi.authme.TestHelper; +import fr.xephi.authme.data.RegistrationCaptchaManager; import fr.xephi.authme.data.auth.PlayerCache; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.message.Messages; @@ -56,6 +57,9 @@ public class LimboPlayerTaskManagerTest { @Mock private PlayerCache playerCache; + @Mock + private RegistrationCaptchaManager registrationCaptchaManager; + @BeforeClass public static void setupLogger() { TestHelper.setupLogger(); @@ -67,7 +71,7 @@ public class LimboPlayerTaskManagerTest { Player player = mock(Player.class); LimboPlayer limboPlayer = mock(LimboPlayer.class); MessageKey key = MessageKey.REGISTER_MESSAGE; - given(messages.retrieve(key)).willReturn(new String[]{"Please register!"}); + given(messages.retrieveSingle(key)).willReturn("Please register!"); int interval = 12; given(settings.getProperty(RegistrationSettings.MESSAGE_INTERVAL)).willReturn(interval); @@ -76,7 +80,7 @@ public class LimboPlayerTaskManagerTest { // then verify(limboPlayer).setMessageTask(any(MessageTask.class)); - verify(messages).retrieve(key); + verify(messages).retrieveSingle(key); verify(bukkitService).runTaskTimer( any(MessageTask.class), eq(2L * TICKS_PER_SECOND), eq((long) interval * TICKS_PER_SECOND)); } @@ -99,11 +103,14 @@ public class LimboPlayerTaskManagerTest { @Test public void shouldCancelExistingMessageTask() { // given + String name = "rats"; Player player = mock(Player.class); + given(player.getName()).willReturn(name); LimboPlayer limboPlayer = new LimboPlayer(null, true, Collections.singletonList("grp"), false, 0.1f, 0.0f); MessageTask existingMessageTask = mock(MessageTask.class); limboPlayer.setMessageTask(existingMessageTask); given(settings.getProperty(RegistrationSettings.MESSAGE_INTERVAL)).willReturn(8); + given(messages.retrieveSingle(MessageKey.REGISTER_MESSAGE)).willReturn("Please register!"); // when limboPlayerTaskManager.registerMessageTask(player, limboPlayer, false); @@ -111,10 +118,32 @@ public class LimboPlayerTaskManagerTest { // then assertThat(limboPlayer.getMessageTask(), not(nullValue())); assertThat(limboPlayer.getMessageTask(), not(sameInstance(existingMessageTask))); - verify(messages).retrieve(MessageKey.REGISTER_MESSAGE); + verify(registrationCaptchaManager).isCaptchaRequired(name); + verify(messages).retrieveSingle(MessageKey.REGISTER_MESSAGE); verify(existingMessageTask).cancel(); } + @Test + public void shouldInitializeMessageTaskWithCaptchaMessage() { + // given + String name = "race"; + Player player = mock(Player.class); + given(player.getName()).willReturn(name); + LimboPlayer limboPlayer = new LimboPlayer(null, true, Collections.singletonList("grp"), false, 0.1f, 0.0f); + given(settings.getProperty(RegistrationSettings.MESSAGE_INTERVAL)).willReturn(12); + given(registrationCaptchaManager.isCaptchaRequired(name)).willReturn(true); + String captcha = "M032"; + given(registrationCaptchaManager.getCaptchaCodeOrGenerateNew(name)).willReturn(captcha); + given(messages.retrieveSingle(MessageKey.CAPTCHA_FOR_REGISTRATION_REQUIRED, captcha)).willReturn("Need to use captcha"); + + // when + limboPlayerTaskManager.registerMessageTask(player, limboPlayer, false); + + // then + assertThat(limboPlayer.getMessageTask(), not(nullValue())); + verify(messages).retrieveSingle(MessageKey.CAPTCHA_FOR_REGISTRATION_REQUIRED, captcha); + } + @Test public void shouldRegisterTimeoutTask() { // given