Skip to content

fix(dialog): keep the pre-join dialog open when the timeout is 0 #3164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Xephi merged 1 commit into from
Oct 11, 2026
Merged

fix(dialog): keep the pre-join dialog open when the timeout is 0 #3164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions authme-paper-common/src/main/java/fr/xephi/authme/listener/PaperDialogFlowListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> response) {
if (response != null) {
response.complete(null);
}
}

private void handleBlockingLoginDialog(PlayerConfigurationConnection connection, long sessionId, String playerName) {
CompletableFuture<String> 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);
Expand Down Expand Up @@ -284,9 +293,11 @@ private void processPreJoinRecoverySubmit(long sessionId, DialogResponseView dia
private void handleBlockingRegisterDialog(PlayerConfigurationConnection connection, long sessionId,
String playerName, Dialog dialog) {
CompletableFuture<String> 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);
Expand Down
12 changes: 9 additions & 3 deletions authme-paper-common/src/main/java/fr/xephi/authme/listener/PaperLoginValidationListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down
108 changes: 107 additions & 1 deletion authme-paper-common/src/test/java/fr/xephi/authme/listener/PaperDialogFlowListenerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand All @@ -42,13 +44,15 @@
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;
import static org.mockito.ArgumentMatchers.any;
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;
Expand Down Expand Up @@ -503,10 +507,11 @@ public void shouldRetireSessionOfClosedConnectionOnly() throws Exception {
given(gone.isConnected()).willReturn(false);
PlayerConfigurationConnection live = mockConnection("Bobby");
given(live.isConnected()).willReturn(true);
CompletableFuture<String> goneFuture = new CompletableFuture<>();
CompletableFuture<String> 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(
Expand All @@ -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<Integer> 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) {
Expand Down
14 changes: 14 additions & 0 deletions ...paper-common/src/test/java/fr/xephi/authme/listener/PaperLoginValidationListenerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
Loading