GH-10188: Close ClientSession in DefaultSftpSF on auth fail

Fixes: #10188

 * Prevent a connection leak due to `ClientSession` going out of
   scope in the `DefaultSftpSessionFactory.getSession()` when connecting
   is successful but authentication fails.

Signed-off-by: alastair Mailer <[email protected]>

(cherry picked from commit 851001c)

Author: almailer

spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/DefaultSftpSessionFactory.java

@@ -79,6 +79,7 @@
  * @author Christian Tzolov
  * @author Adama Sorho
  * @author Darryl Smith
+ * @author Alastair Mailer
  *
  * @since 2.0
  */
@@ -351,7 +352,13 @@ private ClientSession initClientSession() throws IOException {
 						.verify(verifyTimeout)
 						.getSession();
 
-		clientSession.auth().verify(verifyTimeout);
+		try {
+			clientSession.auth().verify(verifyTimeout);
+		}
+		catch (IOException ex) {
+			clientSession.close();
+			throw ex;
+		}
 
 		return clientSession;
 	}

spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java

@@ -52,6 +52,7 @@
 import org.springframework.core.task.SimpleAsyncTaskExecutor;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 import static org.assertj.core.api.Assertions.assertThatNoException;
 import static org.assertj.core.api.Assertions.fail;
 import static org.awaitility.Awaitility.await;
@@ -61,17 +62,18 @@
  * @author Artem Bilan
  * @author Auke Zaaiman
  * @author Darryl Smith
+ * @author Alastair Mailer
  *
  * @since 3.0.2
  */
-public class SftpSessionFactoryTests {
+class SftpSessionFactoryTests {
 
 	/*
 	 * Verify the socket is closed if the channel.connect() fails.
 	 * INT-3305
 	 */
 	@Test
-	public void testConnectFailSocketOpen() throws Exception {
+	void testConnectFailSocketOpen() throws Exception {
 		try (SshServer server = SshServer.setUpDefaultServer()) {
 			server.setPasswordAuthenticator((arg0, arg1, arg2) -> true);
 			server.setPort(0);
@@ -118,7 +120,7 @@ public void testConnectFailSocketOpen() throws Exception {
 	}
 
 	@Test
-	public void concurrentGetSessionDoesntCauseFailure() throws Exception {
+	void concurrentGetSessionDoesntCauseFailure() throws Exception {
 		try (SshServer server = SshServer.setUpDefaultServer()) {
 			server.setPasswordAuthenticator((arg0, arg1, arg2) -> true);
 			server.setPort(0);
@@ -335,4 +337,33 @@ protected SftpClient createSftpClient(ClientSession clientSession,
 		}
 	}
 
+	/*
+	 * Verify the socket is closed if authentication fails.
+	 */
+	@Test
+	void noClientSessionLeakOnAuthError() throws Exception {
+		try (SshServer server = SshServer.setUpDefaultServer()) {
+			server.setPasswordAuthenticator((arg0, arg1, arg2) -> false);
+			server.setPort(0);
+			server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider(new File("hostkey.ser").toPath()));
+			server.setSubsystemFactories(Collections.singletonList(new SftpSubsystemFactory()));
+			server.start();
+
+			DefaultSftpSessionFactory sftpSessionFactory = new DefaultSftpSessionFactory();
+			sftpSessionFactory.setHost("localhost");
+			sftpSessionFactory.setPort(server.getPort());
+			sftpSessionFactory.setUser("user");
+			sftpSessionFactory.setAllowUnknownKeys(true);
+
+			assertThatIllegalStateException()
+					.isThrownBy(sftpSessionFactory::getSession)
+					.withCauseInstanceOf(SshException.class)
+					.withStackTraceContaining("No more authentication methods available");
+
+			await().untilAsserted(() -> assertThat(server.getActiveSessions()).hasSize(0));
+
+			sftpSessionFactory.destroy();
+		}
+	}
+
 }