From 920d454d2b64158e50411a0c86f253b31e06249a Mon Sep 17 00:00:00 2001 From: Sumit Kumar Date: Wed, 27 Sep 2023 18:41:46 +0530 Subject: [PATCH] fix: fix MySQL SSH tunnel default port value (#27612) --- .../BasePluginErrorMessages.java | 1 + .../appsmith/external/helpers/SSHUtils.java | 22 ++++++++++++++--- .../external/helpers/SSHUtilsTest.java | 24 ++++++++++++++++++- .../com/external/plugins/MySqlPlugin.java | 4 +++- .../external/utils/MySqlDatasourceUtils.java | 3 ++- 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/exceptions/pluginExceptions/BasePluginErrorMessages.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/exceptions/pluginExceptions/BasePluginErrorMessages.java index 4d0d2518a0a..45a771fbc0f 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/exceptions/pluginExceptions/BasePluginErrorMessages.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/exceptions/pluginExceptions/BasePluginErrorMessages.java @@ -10,6 +10,7 @@ public abstract class BasePluginErrorMessages { public static final String UNKNOWN_CONNECTION_ERROR_MSG = "Unknown connection error. Please reach out to Appsmith " + "customer support to resolve this."; public static final String JDBC_DRIVER_LOADING_ERROR_MSG = "Error loading JDBC Driver class."; + public static final String DS_MISSING_PORT_ERROR_MSG = "Missing database port number for authentication."; public static final String SSH_CONNECTION_FAILED_ERROR_MSG = "Failed to create SSH connection. Please check your datasource" + " configuration."; public static final String DS_MISSING_SSH_USERNAME_ERROR_MSG = "Missing SSH username for authentication."; diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/SSHUtils.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/SSHUtils.java index 501f1acf18e..33d425f1369 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/SSHUtils.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/SSHUtils.java @@ -21,6 +21,7 @@ import static com.appsmith.external.constants.ConnectionMethod.CONNECTION_METHOD_SSH; import static com.appsmith.external.constants.PluginConstants.HostName.LOCALHOST; +import static com.appsmith.external.exceptions.pluginExceptions.BasePluginErrorMessages.DS_MISSING_PORT_ERROR_MSG; import static com.appsmith.external.exceptions.pluginExceptions.BasePluginErrorMessages.SSH_CONNECTION_FAILED_ERROR_MSG; import static java.lang.Math.toIntExact; import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; @@ -118,6 +119,16 @@ public static boolean isSSHEnabled( .getValue()); } + public static int getSSHPortFromConfigOrDefault(DatasourceConfiguration datasourceConfiguration) { + return toIntExact(defaultIfNull(datasourceConfiguration.getSshProxy().getPort(), DEFAULT_SSH_PORT)); + } + + public static int getDBPortFromConfigOrDefault( + DatasourceConfiguration datasourceConfiguration, Long defaultDBPort) { + return toIntExact( + defaultIfNull(datasourceConfiguration.getEndpoints().get(0).getPort(), defaultDBPort)); + } + /** * Create a new SSH connection if configured and return the SSH connection details in the ConnectionContext object. * @param datasourceConfiguration : datasource configuration @@ -127,6 +138,7 @@ public static boolean isSSHEnabled( public static ConnectionContext getConnectionContext( DatasourceConfiguration datasourceConfiguration, int connectionMethodPropertiesIndex, + Long defaultDBPort, Class connectionType) { /* Return empty ConnectionContext if SSH tunnel is not enabled */ @@ -134,13 +146,17 @@ public static ConnectionContext getConnectionContext( return new ConnectionContext(null, null); } + if (datasourceConfiguration.getEndpoints().get(0).getPort() == null && defaultDBPort == null) { + throw new AppsmithPluginException( + AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR, DS_MISSING_PORT_ERROR_MSG); + } + String sshHost = datasourceConfiguration.getSshProxy().getHost(); - int sshPort = toIntExact(datasourceConfiguration.getSshProxy().getPort()); + int sshPort = getSSHPortFromConfigOrDefault(datasourceConfiguration); String sshUsername = datasourceConfiguration.getSshProxy().getUsername(); UploadedFile key = datasourceConfiguration.getSshProxy().getPrivateKey().getKeyFile(); String dbHost = datasourceConfiguration.getEndpoints().get(0).getHost(); - int dbPort = toIntExact( - defaultIfNull(datasourceConfiguration.getEndpoints().get(0).getPort(), DEFAULT_SSH_PORT)); + int dbPort = getDBPortFromConfigOrDefault(datasourceConfiguration, defaultDBPort); SSHTunnelContext sshTunnelContext = null; try { sshTunnelContext = createSSHTunnel(sshHost, sshPort, sshUsername, key, dbHost, dbPort); diff --git a/app/server/appsmith-interfaces/src/test/java/com/appsmith/external/helpers/SSHUtilsTest.java b/app/server/appsmith-interfaces/src/test/java/com/appsmith/external/helpers/SSHUtilsTest.java index 101d56a1440..301c3f3dbc4 100644 --- a/app/server/appsmith-interfaces/src/test/java/com/appsmith/external/helpers/SSHUtilsTest.java +++ b/app/server/appsmith-interfaces/src/test/java/com/appsmith/external/helpers/SSHUtilsTest.java @@ -2,15 +2,21 @@ import com.appsmith.external.models.ConnectionContext; import com.appsmith.external.models.DatasourceConfiguration; +import com.appsmith.external.models.Endpoint; import com.appsmith.external.models.Property; +import com.appsmith.external.models.SSHConnection; import net.schmizz.sshj.SSHClient; import org.junit.jupiter.api.Test; import java.util.ArrayList; +import java.util.List; import static com.appsmith.external.helpers.SSHUtils.getConnectionContext; +import static com.appsmith.external.helpers.SSHUtils.getDBPortFromConfigOrDefault; +import static com.appsmith.external.helpers.SSHUtils.getSSHPortFromConfigOrDefault; import static com.appsmith.external.helpers.SSHUtils.isSSHEnabled; import static com.appsmith.external.helpers.SSHUtils.isSSHTunnelConnected; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; @@ -91,8 +97,24 @@ public void testGetConnectionContext_whenSSHIsDisabled() { properties.add(new Property("Connection method", "Standard")); datasourceConfiguration.setProperties(properties); - ConnectionContext connectionContext = getConnectionContext(datasourceConfiguration, 1, Object.class); + ConnectionContext connectionContext = getConnectionContext(datasourceConfiguration, 1, null, Object.class); assertTrue(connectionContext.getConnection() == null, connectionContext.toString()); assertTrue(connectionContext.getSshTunnelContext() == null, connectionContext.toString()); } + + @Test + public void testDefaultSSHPortValue() { + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setSshProxy(new SSHConnection()); // port number not provided + + assertEquals(getSSHPortFromConfigOrDefault(datasourceConfiguration), 22L); + } + + @Test + public void testDefaultDBPortValue() { + DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setEndpoints(List.of(new Endpoint())); + + assertEquals(getDBPortFromConfigOrDefault(datasourceConfiguration, 1234L), 1234L); + } } diff --git a/app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/plugins/MySqlPlugin.java b/app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/plugins/MySqlPlugin.java index 8c2c21bad02..7cc582bfa39 100644 --- a/app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/plugins/MySqlPlugin.java +++ b/app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/plugins/MySqlPlugin.java @@ -100,6 +100,8 @@ public class MySqlPlugin extends BasePlugin { public static final int CONNECTION_METHOD_INDEX = 1; public static final MySqlErrorUtils mySqlErrorUtils = MySqlErrorUtils.getInstance(); + public static final Long MYSQL_DEFAULT_PORT = 3306L; + /** * Example output for COLUMNS_QUERY: * +------------+-----------+-------------+-------------+-------------+------------+----------------+ @@ -615,7 +617,7 @@ public Mono> datasourceCreate( ConnectionContext connectionContext; try { connectionContext = getConnectionContext( - datasourceConfiguration, CONNECTION_METHOD_INDEX, ConnectionPool.class); + datasourceConfiguration, CONNECTION_METHOD_INDEX, MYSQL_DEFAULT_PORT, ConnectionPool.class); ConnectionPool pool = getNewConnectionPool(datasourceConfiguration, connectionContext); connectionContext.setConnection(pool); return Mono.just(connectionContext); diff --git a/app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/utils/MySqlDatasourceUtils.java b/app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/utils/MySqlDatasourceUtils.java index 2805ecbe85d..ae001730f3a 100644 --- a/app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/utils/MySqlDatasourceUtils.java +++ b/app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/utils/MySqlDatasourceUtils.java @@ -30,6 +30,7 @@ import static com.appsmith.external.exceptions.pluginExceptions.BasePluginErrorMessages.DS_MISSING_SSH_USERNAME_ERROR_MSG; import static com.appsmith.external.helpers.SSHUtils.isSSHEnabled; import static com.external.plugins.MySqlPlugin.CONNECTION_METHOD_INDEX; +import static com.external.plugins.MySqlPlugin.MYSQL_DEFAULT_PORT; import static io.r2dbc.spi.ConnectionFactoryOptions.SSL; import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; import static org.apache.commons.lang3.StringUtils.isBlank; @@ -81,7 +82,7 @@ public static ConnectionFactoryOptions.Builder getBuilder( if (!isSSHEnabled(datasourceConfiguration, CONNECTION_METHOD_INDEX)) { for (Endpoint endpoint : datasourceConfiguration.getEndpoints()) { - hosts.add(endpoint.getHost() + ":" + defaultIfNull(endpoint.getPort(), 3306L)); + hosts.add(endpoint.getHost() + ":" + defaultIfNull(endpoint.getPort(), MYSQL_DEFAULT_PORT)); } } else { hosts.add(LOCALHOST + ":"