spring-projectsGH-8581: Don't overwrite external SshClient

Fixes spring-projects#8581 Do not overwrite configuration of externally provided `SshClient` in the `DefaultSftpSessionFactory` * Replace JUnit `assertDoesNotThrow` by AssertJ `assertThatNoException` in test **Cherry-pick to `6.0.x`**
azaaiman · Mar 28, 2023 · 775e6fd · 775e6fd
1 parent 6d7ee46
commit 775e6fd
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 27 deletions.
diff --git a/...src/main/java/org/springframework/integration/sftp/session/DefaultSftpSessionFactory.java b/...src/main/java/org/springframework/integration/sftp/session/DefaultSftpSessionFactory.java
@@ -61,6 +61,7 @@
  * @author Pat Turner
  * @author Artem Bilan
  * @author Krzysztof Debski
+ * @author Auke Zaaiman
  *
  * @since 2.0
  */
@@ -335,36 +336,45 @@ private void doInitClient() throws IOException {
 		if (this.port <= 0) {
 			this.port = SshConstants.DEFAULT_PORT;
 		}
-		ServerKeyVerifier serverKeyVerifier =
-				this.allowUnknownKeys ? AcceptAllServerKeyVerifier.INSTANCE : RejectAllServerKeyVerifier.INSTANCE;
-		if (this.knownHosts != null) {
-			serverKeyVerifier = new ResourceKnownHostsServerKeyVerifier(this.knownHosts);
-		}
-		this.sshClient.setServerKeyVerifier(serverKeyVerifier);
-
-		this.sshClient.setPasswordIdentityProvider(PasswordIdentityProvider.wrapPasswords(this.password));
-		if (this.privateKey != null) {
-			IoResource<Resource> privateKeyResource =
-					new AbstractIoResource<>(Resource.class, this.privateKey) {
-
-						@Override
-						public InputStream openInputStream() throws IOException {
-							return getResourceValue().getInputStream();
-						}
-					};
-			try {
-				Collection<KeyPair> keys =
-						SecurityUtils.getKeyPairResourceParser()
-									.loadKeyPairs(null, privateKeyResource,
-										FilePasswordProvider.of(this.privateKeyPassphrase));
-				this.sshClient.setKeyIdentityProvider(KeyIdentityProvider.wrapKeyPairs(keys));
+
+		doInitInnerClient();
+
+		this.sshClient.start();
+	}
+
+	private void doInitInnerClient() throws IOException {
+		if (this.isInnerClient) {
+			ServerKeyVerifier serverKeyVerifier =
+					this.allowUnknownKeys ? AcceptAllServerKeyVerifier.INSTANCE : RejectAllServerKeyVerifier.INSTANCE;
+			if (this.knownHosts != null) {
+				serverKeyVerifier = new ResourceKnownHostsServerKeyVerifier(this.knownHosts);
 			}
-			catch (GeneralSecurityException ex) {
-				throw new IOException("Cannot load private key: " + this.privateKey.getFilename(), ex);
+			this.sshClient.setServerKeyVerifier(serverKeyVerifier);
+
+			this.sshClient.setPasswordIdentityProvider(PasswordIdentityProvider.wrapPasswords(this.password));
+			if (this.privateKey != null) {
+				IoResource<Resource> privateKeyResource =
+						new AbstractIoResource<>(Resource.class, this.privateKey) {
+
+							@Override
+							public InputStream openInputStream() throws IOException {
+								return getResourceValue().getInputStream();
+							}
+
+						};
+				try {
+					Collection<KeyPair> keys =
+							SecurityUtils.getKeyPairResourceParser()
+									.loadKeyPairs(null, privateKeyResource,
+											FilePasswordProvider.of(this.privateKeyPassphrase));
+					this.sshClient.setKeyIdentityProvider(KeyIdentityProvider.wrapKeyPairs(keys));
+				}
+				catch (GeneralSecurityException ex) {
+					throw new IOException("Cannot load private key: " + this.privateKey.getFilename(), ex);
+				}
 			}
+			this.sshClient.setUserInteraction(this.userInteraction);
 		}
-		this.sshClient.setUserInteraction(this.userInteraction);
-		this.sshClient.start();
 	}
 
 	@Override

diff --git a/...p/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java b/...p/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java
@@ -23,6 +23,9 @@
 import java.util.Collections;
 import java.util.List;
 
+import org.apache.sshd.client.SshClient;
+import org.apache.sshd.client.auth.password.PasswordIdentityProvider;
+import org.apache.sshd.client.keyverifier.AcceptAllServerKeyVerifier;
 import org.apache.sshd.common.SshException;
 import org.apache.sshd.server.SshServer;
 import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider;
@@ -33,12 +36,14 @@
 import org.springframework.core.task.SimpleAsyncTaskExecutor;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
 import static org.assertj.core.api.Assertions.fail;
 import static org.awaitility.Awaitility.await;
 
 /**
  * @author Gary Russell
  * @author Artem Bilan
+ * @author Auke Zaaiman
  *
  * @since 3.0.2
  */
@@ -126,4 +131,26 @@ public void concurrentGetSessionDoesntCauseFailure() throws IOException {
 		}
 	}
 
+	@Test
+	void externallyProvidedSshClientShouldNotHaveItsConfigurationOverwritten() throws IOException {
+		try (SshServer server = SshServer.setUpDefaultServer()) {
+			server.setPasswordAuthenticator((arg0, arg1, arg2) -> true);
+			server.setPort(0);
+			server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider(new File("hostkey.ser").toPath()));
+			server.setSubsystemFactories(Collections.singletonList(new SftpSubsystemFactory()));
+			server.start();
+
+			SshClient externalClient = SshClient.setUpDefaultClient();
+			externalClient.setServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE);
+			externalClient.setPasswordIdentityProvider(PasswordIdentityProvider.wrapPasswords("pass"));
+
+			DefaultSftpSessionFactory sftpSessionFactory = new DefaultSftpSessionFactory(externalClient, false);
+			sftpSessionFactory.setHost("localhost");
+			sftpSessionFactory.setPort(server.getPort());
+			sftpSessionFactory.setUser("user");
+
+			assertThatNoException().isThrownBy(sftpSessionFactory::getSession);
+		}
+	}
+
 }