diff --git a/authme-paper-common/src/main/java/fr/xephi/authme/listener/PaperDialogFlowListener.java b/authme-paper-common/src/main/java/fr/xephi/authme/listener/PaperDialogFlowListener.java index e39fda0ea4..9344ffc0df 100644 --- a/authme-paper-common/src/main/java/fr/xephi/authme/listener/PaperDialogFlowListener.java +++ b/authme-paper-common/src/main/java/fr/xephi/authme/listener/PaperDialogFlowListener.java @@ -206,17 +206,26 @@ public void onPlayerConnectionClose(PlayerConnectionCloseEvent event) { private void retireSession(PlayerConfigurationConnection connection) { Long sessionId = connectionSessions.remove(connection); if (sessionId != null) { - pendingLoginResponses.remove(sessionId); - pendingRegisterResponses.remove(sessionId); + releaseResponse(pendingLoginResponses.remove(sessionId)); + releaseResponse(pendingRegisterResponses.remove(sessionId)); preJoinDialogService.retireSession(sessionId); } } + // Without a timeout nothing else wakes the configuration thread waiting on a connection that is gone + private static void releaseResponse(CompletableFuture response) { + if (response != null) { + response.complete(null); + } + } + private void handleBlockingLoginDialog(PlayerConfigurationConnection connection, long sessionId, String playerName) { CompletableFuture loginResponse = new CompletableFuture<>(); - long timeoutSeconds = Math.max(commonService.getProperty(RestrictionSettings.LOGIN_TIMEOUT), 1); - loginResponse.completeOnTimeout( - messages.retrieveSingle(playerName, MessageKey.LOGIN_TIMEOUT_ERROR), timeoutSeconds, TimeUnit.SECONDS); + int timeoutSeconds = commonService.getProperty(RestrictionSettings.LOGIN_TIMEOUT); + if (timeoutSeconds > 0) { + loginResponse.completeOnTimeout( + messages.retrieveSingle(playerName, MessageKey.LOGIN_TIMEOUT_ERROR), timeoutSeconds, TimeUnit.SECONDS); + } String normalizedName = playerName.toLowerCase(Locale.ROOT); pendingLoginResponses.put(sessionId, loginResponse); preJoinDialogService.registerPreJoinFuture(sessionId, loginResponse); @@ -284,9 +293,11 @@ private void processPreJoinRecoverySubmit(long sessionId, DialogResponseView dia private void handleBlockingRegisterDialog(PlayerConfigurationConnection connection, long sessionId, String playerName, Dialog dialog) { CompletableFuture registerResponse = new CompletableFuture<>(); - long timeoutSeconds = Math.max(commonService.getProperty(RestrictionSettings.REGISTER_TIMEOUT), 1); - registerResponse.completeOnTimeout( - messages.retrieveSingle(playerName, MessageKey.LOGIN_TIMEOUT_ERROR), timeoutSeconds, TimeUnit.SECONDS); + int timeoutSeconds = commonService.getProperty(RestrictionSettings.REGISTER_TIMEOUT); + if (timeoutSeconds > 0) { + registerResponse.completeOnTimeout( + messages.retrieveSingle(playerName, MessageKey.LOGIN_TIMEOUT_ERROR), timeoutSeconds, TimeUnit.SECONDS); + } pendingRegisterResponses.put(sessionId, registerResponse); connection.getAudience().showDialog(dialog); diff --git a/authme-paper-common/src/main/java/fr/xephi/authme/listener/PaperLoginValidationListener.java b/authme-paper-common/src/main/java/fr/xephi/authme/listener/PaperLoginValidationListener.java index df71ac5f62..6a70b722cb 100644 --- a/authme-paper-common/src/main/java/fr/xephi/authme/listener/PaperLoginValidationListener.java +++ b/authme-paper-common/src/main/java/fr/xephi/authme/listener/PaperLoginValidationListener.java @@ -31,6 +31,9 @@ public class PaperLoginValidationListener implements Listener { /** Safety net for a connection that never reports itself as gone; not the release mechanism. */ private static final long CLAIM_GRACE_MILLIS = TimeUnit.SECONDS.toMillis(60); + /** Used when a timeout is disabled: the pre-join dialog then stays open for as long as the connection does. */ + private static final long CLAIM_WITHOUT_TIMEOUT_MILLIS = TimeUnit.DAYS.toMillis(1); + @Inject private OnJoinVerifier onJoinVerifier; @@ -142,9 +145,12 @@ private void denyConnection(PlayerConnectionValidateLoginEvent event, String pla } private long getClaimTtlMillis() { - int timeoutSeconds = Math.max(settings.getProperty(RestrictionSettings.LOGIN_TIMEOUT), - settings.getProperty(RestrictionSettings.REGISTER_TIMEOUT)); - return TimeUnit.SECONDS.toMillis(Math.max(timeoutSeconds, 0)) + CLAIM_GRACE_MILLIS; + int loginTimeout = settings.getProperty(RestrictionSettings.LOGIN_TIMEOUT); + int registerTimeout = settings.getProperty(RestrictionSettings.REGISTER_TIMEOUT); + if (loginTimeout <= 0 || registerTimeout <= 0) { + return CLAIM_WITHOUT_TIMEOUT_MILLIS; + } + return TimeUnit.SECONDS.toMillis(Math.max(loginTimeout, registerTimeout)) + CLAIM_GRACE_MILLIS; } private static String getPlayerName(PlayerLoginConnection connection) { diff --git a/authme-paper-common/src/test/java/fr/xephi/authme/listener/PaperDialogFlowListenerTest.java b/authme-paper-common/src/test/java/fr/xephi/authme/listener/PaperDialogFlowListenerTest.java index 313ee35c25..c6c756d001 100644 --- a/authme-paper-common/src/test/java/fr/xephi/authme/listener/PaperDialogFlowListenerTest.java +++ b/authme-paper-common/src/test/java/fr/xephi/authme/listener/PaperDialogFlowListenerTest.java @@ -1,5 +1,6 @@ package fr.xephi.authme.listener; +import ch.jalu.configme.properties.Property; import com.destroystokyo.paper.event.player.PlayerConnectionCloseEvent; import com.destroystokyo.paper.profile.PlayerProfile; import fr.xephi.authme.data.ProxySessionManager; @@ -24,6 +25,7 @@ import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import io.papermc.paper.connection.PlayerConfigurationConnection; +import io.papermc.paper.dialog.Dialog; import io.papermc.paper.dialog.DialogResponseView; import io.papermc.paper.event.connection.configuration.AsyncPlayerConnectionConfigureEvent; import io.papermc.paper.event.player.PlayerCustomClickEvent; @@ -42,6 +44,7 @@ import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; @@ -49,6 +52,7 @@ import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.willAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.never; @@ -503,10 +507,11 @@ public void shouldRetireSessionOfClosedConnectionOnly() throws Exception { given(gone.isConnected()).willReturn(false); PlayerConfigurationConnection live = mockConnection("Bobby"); given(live.isConnected()).willReturn(true); + CompletableFuture goneFuture = new CompletableFuture<>(); CompletableFuture liveFuture = new CompletableFuture<>(); connectionSessions(listener).put(gone, SESSION_ID); connectionSessions(listener).put(live, SESSION_ID + 1); - pendingResponses(listener, "pendingLoginResponses").put(SESSION_ID, new CompletableFuture<>()); + pendingResponses(listener, "pendingLoginResponses").put(SESSION_ID, goneFuture); pendingResponses(listener, "pendingLoginResponses").put(SESSION_ID + 1, liveFuture); listener.onPlayerConnectionClose(new PlayerConnectionCloseEvent( @@ -517,6 +522,107 @@ public void shouldRetireSessionOfClosedConnectionOnly() throws Exception { assertThat(connectionSessions(listener).containsKey(gone), is(false)); assertThat(connectionSessions(listener).get(live), is(SESSION_ID + 1)); assertThat(pendingResponses(listener, "pendingLoginResponses").get(SESSION_ID + 1), is(liveFuture)); + assertThat("thread waiting on the closed connection must be released", goneFuture.isDone(), is(true)); + assertThat(liveFuture.isDone(), is(false)); + } + + @Test + public void shouldKeepPreJoinLoginDialogOpenWhenLoginTimeoutIsDisabled() throws Exception { + PaperDialogFlowListener listener = new PaperDialogFlowListener(); + PreJoinDialogService preJoinDialogService = mock(PreJoinDialogService.class); + Audience audience = mock(Audience.class); + PlayerConfigurationConnection connection = newBlockingDialogSetup(listener, preJoinDialogService, audience); + setField(listener, "proxySessionManager", mock(ProxySessionManager.class)); + setField(listener, "dialogWindowService", mock(DialogWindowService.class)); + givenTimeout(listener, RestrictionSettings.LOGIN_TIMEOUT, 0); + givenDialogIsAnsweredLate(listener, audience, "pendingLoginResponses"); + + try (var helperStatic = mockStatic(PaperDialogHelper.class)) { + helperStatic.when(() -> PaperDialogHelper.createPreJoinLoginDialog(any())).thenReturn(null); + + invokeHandleBlockingLoginDialog(listener, connection); + } + + verify(preJoinDialogService, never()).storePendingKickMessage(anyLong(), anyString()); + } + + @Test + public void shouldKeepPreJoinRegisterDialogOpenWhenRegisterTimeoutIsDisabled() throws Exception { + PaperDialogFlowListener listener = new PaperDialogFlowListener(); + PreJoinDialogService preJoinDialogService = mock(PreJoinDialogService.class); + Audience audience = mock(Audience.class); + PlayerConfigurationConnection connection = newBlockingDialogSetup(listener, preJoinDialogService, audience); + givenTimeout(listener, RestrictionSettings.REGISTER_TIMEOUT, 0); + givenDialogIsAnsweredLate(listener, audience, "pendingRegisterResponses"); + + invokeHandleBlockingRegisterDialog(listener, connection); + + verify(preJoinDialogService, never()).storePendingKickMessage(anyLong(), anyString()); + } + + @Test + public void shouldKickWhenPreJoinLoginDialogTimesOut() throws Exception { + PaperDialogFlowListener listener = new PaperDialogFlowListener(); + PreJoinDialogService preJoinDialogService = mock(PreJoinDialogService.class); + PlayerConfigurationConnection connection = + newBlockingDialogSetup(listener, preJoinDialogService, mock(Audience.class)); + setField(listener, "proxySessionManager", mock(ProxySessionManager.class)); + setField(listener, "dialogWindowService", mock(DialogWindowService.class)); + givenTimeout(listener, RestrictionSettings.LOGIN_TIMEOUT, 1); + + try (var helperStatic = mockStatic(PaperDialogHelper.class)) { + helperStatic.when(() -> PaperDialogHelper.createPreJoinLoginDialog(any())).thenReturn(null); + + invokeHandleBlockingLoginDialog(listener, connection); + } + + verify(preJoinDialogService).storePendingKickMessage(SESSION_ID, "Timed out!"); + } + + private static PlayerConfigurationConnection newBlockingDialogSetup(PaperDialogFlowListener listener, + PreJoinDialogService preJoinDialogService, + Audience audience) throws Exception { + Messages messages = mock(Messages.class); + setField(listener, "messages", messages); + setField(listener, "preJoinDialogService", preJoinDialogService); + given(messages.retrieveSingle("Bobby", MessageKey.LOGIN_TIMEOUT_ERROR)).willReturn("Timed out!"); + + PlayerConfigurationConnection connection = mockConnection("Bobby"); + given(connection.getAudience()).willReturn(audience); + return connection; + } + + private static void givenTimeout(PaperDialogFlowListener listener, Property property, + int seconds) throws Exception { + CommonService commonService = mock(CommonService.class); + setField(listener, "commonService", commonService); + given(commonService.getProperty(property)).willReturn(seconds); + } + + // The player answers after 1.2 seconds; a disabled timeout used to end the wait after one second + private static void givenDialogIsAnsweredLate(PaperDialogFlowListener listener, Audience audience, + String responseField) { + willAnswer(invocation -> { + pendingResponses(listener, responseField).get(SESSION_ID) + .completeOnTimeout(null, 1200, TimeUnit.MILLISECONDS); + return null; + }).given(audience).showDialog(any()); + } + + private static void invokeHandleBlockingLoginDialog(PaperDialogFlowListener listener, + PlayerConfigurationConnection connection) throws Exception { + Method method = PaperDialogFlowListener.class.getDeclaredMethod("handleBlockingLoginDialog", + PlayerConfigurationConnection.class, long.class, String.class); + method.setAccessible(true); + method.invoke(listener, connection, SESSION_ID, "Bobby"); + } + + private static void invokeHandleBlockingRegisterDialog(PaperDialogFlowListener listener, + PlayerConfigurationConnection connection) throws Exception { + Method method = PaperDialogFlowListener.class.getDeclaredMethod("handleBlockingRegisterDialog", + PlayerConfigurationConnection.class, long.class, String.class, Dialog.class); + method.setAccessible(true); + method.invoke(listener, connection, SESSION_ID, "Bobby", null); } private static PlayerConfigurationConnection mockConnection(String playerName) { diff --git a/authme-paper-common/src/test/java/fr/xephi/authme/listener/PaperLoginValidationListenerTest.java b/authme-paper-common/src/test/java/fr/xephi/authme/listener/PaperLoginValidationListenerTest.java index 7cbb047d0e..05e90f290f 100644 --- a/authme-paper-common/src/test/java/fr/xephi/authme/listener/PaperLoginValidationListenerTest.java +++ b/authme-paper-common/src/test/java/fr/xephi/authme/listener/PaperLoginValidationListenerTest.java @@ -25,6 +25,7 @@ import java.net.InetAddress; import java.util.UUID; +import java.util.concurrent.TimeUnit; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; @@ -74,6 +75,19 @@ public void shouldClaimNameDuringLoginPhase() { verify(pendingConnectionRegistry).tryClaim(eq("Bobby"), eq(connection), anyLong()); } + @Test + public void shouldClaimNameForLongerThanTheGracePeriodWhenTimeoutIsDisabled() { + given(settings.getProperty(RestrictionSettings.LOGIN_TIMEOUT)).willReturn(0); + given(settings.getProperty(RestrictionSettings.REGISTER_TIMEOUT)).willReturn(30); + PlayerLoginConnection connection = newLoginConnection("Bobby"); + given(pendingConnectionRegistry.tryClaim(eq("Bobby"), eq(connection), anyLong())).willReturn(true); + PlayerConnectionValidateLoginEvent event = new PlayerConnectionValidateLoginEvent(connection, null); + + listener.onPlayerConnectionValidateLogin(event); + + verify(pendingConnectionRegistry).tryClaim("Bobby", connection, TimeUnit.DAYS.toMillis(1)); + } + @Test public void shouldKickDuplicateSessionOnValidationEvent() throws FailedVerificationException { PlayerLoginConnection connection = newLoginConnection("Bobby");