From c20ef122533159f745286227806fa6eb4b6c9bd0 Mon Sep 17 00:00:00 2001 From: "muhammed.shanid@zemosolabs.com" Date: Mon, 11 Nov 2024 15:02:29 +0530 Subject: [PATCH 1/4] SMTP datasource should work without username and password --- .../java/com/external/plugins/SmtpPlugin.java | 33 ++++++++++++++----- .../com/external/plugins/SmtpPluginTest.java | 8 ++--- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/app/server/appsmith-plugins/smtpPlugin/src/main/java/com/external/plugins/SmtpPlugin.java b/app/server/appsmith-plugins/smtpPlugin/src/main/java/com/external/plugins/SmtpPlugin.java index 69f81748d3fe..68653a7bf738 100644 --- a/app/server/appsmith-plugins/smtpPlugin/src/main/java/com/external/plugins/SmtpPlugin.java +++ b/app/server/appsmith-plugins/smtpPlugin/src/main/java/com/external/plugins/SmtpPlugin.java @@ -212,15 +212,24 @@ public Mono datasourceCreate(DatasourceConfiguration datasourceConfigur prop.put("mail.smtp.port", String.valueOf(port)); prop.put("mail.smtp.ssl.trust", endpoint.getHost()); - String username = authentication.getUsername(); - String password = authentication.getPassword(); + Session session; - Session session = Session.getInstance(prop, new Authenticator() { - @Override - protected PasswordAuthentication getPasswordAuthentication() { - return new PasswordAuthentication(username, password); - } - }); + if (authentication != null && StringUtils.hasText(authentication.getUsername()) + && StringUtils.hasText(authentication.getPassword())) { + + String username = authentication.getUsername(); + String password = authentication.getPassword(); + + session = Session.getInstance(prop, new Authenticator() { + @Override + protected PasswordAuthentication getPasswordAuthentication() { + return new PasswordAuthentication(username, password); + } + }); + } else { + prop.put("mail.smtp.auth", "false"); + session = Session.getInstance(prop); + } return Mono.just(session); } @@ -264,6 +273,10 @@ public Mono testDatasource(Session connection) { log.debug(Thread.currentThread().getName() + ": testDatasource() called for SMTP plugin."); return Mono.fromCallable(() -> { Set invalids = new HashSet<>(); + + boolean isAuthRequired = connection.getProperty("mail.smtp.auth") != null + && connection.getProperty("mail.smtp.auth").equals("true"); + try { Transport transport = connection.getTransport(); if (transport != null) { @@ -273,7 +286,9 @@ public Mono testDatasource(Session connection) { } catch (NoSuchProviderException e) { invalids.add(SMTPErrorMessages.DS_NO_SUCH_PROVIDER_ERROR_MSG); } catch (AuthenticationFailedException e) { - invalids.add(SMTPErrorMessages.DS_AUTHENTICATION_FAILED_ERROR_MSG); + if (isAuthRequired) { + invalids.add(SMTPErrorMessages.DS_AUTHENTICATION_FAILED_ERROR_MSG); + } } catch (MessagingException e) { log.error(e.getMessage()); invalids.add(SMTPErrorMessages.DS_CONNECTION_FAILED_TO_SMTP_SERVER_ERROR_MSG); diff --git a/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java b/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java index 2da564b02d91..ad488caa00a2 100644 --- a/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java +++ b/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java @@ -131,12 +131,10 @@ public void testInvalidPort() { @Test public void testNullAuthentication() { - DatasourceConfiguration invalidDatasourceConfiguration = createDatasourceConfiguration(); - invalidDatasourceConfiguration.setAuthentication(null); + DatasourceConfiguration noAuthDatasourceConfiguration = createDatasourceConfiguration(); + noAuthDatasourceConfiguration.setAuthentication(null); - assertEquals( - Set.of(new AppsmithPluginException(AppsmithPluginError.PLUGIN_AUTHENTICATION_ERROR).getMessage()), - pluginExecutor.validateDatasource(invalidDatasourceConfiguration)); + assertEquals(0, pluginExecutor.validateDatasource(noAuthDatasourceConfiguration).size()); } @Test From a70d0635c14ffd71e33bfb7961c75833cd336597 Mon Sep 17 00:00:00 2001 From: "muhammed.shanid@zemosolabs.com" Date: Tue, 12 Nov 2024 16:18:37 +0530 Subject: [PATCH 2/4] SMTP datasource should work without username and password - added one more testcase - removed unwanted checks --- .../java/com/external/plugins/SmtpPlugin.java | 15 +------ .../com/external/plugins/SmtpPluginTest.java | 44 +++++++++++++++---- 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/app/server/appsmith-plugins/smtpPlugin/src/main/java/com/external/plugins/SmtpPlugin.java b/app/server/appsmith-plugins/smtpPlugin/src/main/java/com/external/plugins/SmtpPlugin.java index 68653a7bf738..d615e58a2004 100644 --- a/app/server/appsmith-plugins/smtpPlugin/src/main/java/com/external/plugins/SmtpPlugin.java +++ b/app/server/appsmith-plugins/smtpPlugin/src/main/java/com/external/plugins/SmtpPlugin.java @@ -257,14 +257,6 @@ public Set validateDatasource(DatasourceConfiguration datasourceConfigur invalids.add(SMTPErrorMessages.DS_MISSING_HOST_ADDRESS_ERROR_MSG); } } - - DBAuth authentication = (DBAuth) datasourceConfiguration.getAuthentication(); - if (authentication == null - || !StringUtils.hasText(authentication.getUsername()) - || !StringUtils.hasText(authentication.getPassword())) { - invalids.add(new AppsmithPluginException(AppsmithPluginError.PLUGIN_AUTHENTICATION_ERROR).getMessage()); - } - return invalids; } @@ -274,9 +266,6 @@ public Mono testDatasource(Session connection) { return Mono.fromCallable(() -> { Set invalids = new HashSet<>(); - boolean isAuthRequired = connection.getProperty("mail.smtp.auth") != null - && connection.getProperty("mail.smtp.auth").equals("true"); - try { Transport transport = connection.getTransport(); if (transport != null) { @@ -286,9 +275,7 @@ public Mono testDatasource(Session connection) { } catch (NoSuchProviderException e) { invalids.add(SMTPErrorMessages.DS_NO_SUCH_PROVIDER_ERROR_MSG); } catch (AuthenticationFailedException e) { - if (isAuthRequired) { - invalids.add(SMTPErrorMessages.DS_AUTHENTICATION_FAILED_ERROR_MSG); - } + invalids.add(SMTPErrorMessages.DS_AUTHENTICATION_FAILED_ERROR_MSG); } catch (MessagingException e) { log.error(e.getMessage()); invalids.add(SMTPErrorMessages.DS_CONNECTION_FAILED_TO_SMTP_SERVER_ERROR_MSG); diff --git a/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java b/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java index ad488caa00a2..e9766988e8d6 100644 --- a/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java +++ b/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java @@ -28,17 +28,10 @@ import reactor.core.publisher.Mono; import reactor.test.StepVerifier; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.stream.Collectors; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; @@ -59,6 +52,11 @@ public class SmtpPluginTest { .withCommand("bin/maildev --base-pathname /maildev --incoming-user " + username + " --incoming-pass " + password + " -s 25"); + @Container + public static final GenericContainer smtpWithoutAuth = new GenericContainer<>(DockerImageName.parse("maildev/maildev")) + .withExposedPorts(1025) + .withCommand("bin/maildev --base-pathname /maildev --smtp-port 1025 --incoming-user '' --incoming-pass ''"); + private final SmtpPlugin.SmtpPluginExecutor pluginExecutor = new SmtpPlugin.SmtpPluginExecutor(); @BeforeAll @@ -137,6 +135,34 @@ public void testNullAuthentication() { assertEquals(0, pluginExecutor.validateDatasource(noAuthDatasourceConfiguration).size()); } + @Test + public void testConnectionWithoutAuth() { + + String smtpHost = "localhost"; + int smtpPort = smtpWithoutAuth.getMappedPort(1025); + + Properties properties = new Properties(); + properties.put("mail.transport.protocol", "smtp"); + properties.put("mail.smtp.host", smtpHost); + properties.put("mail.smtp.port", String.valueOf(smtpPort)); + // No authentication + properties.put("mail.smtp.auth", "false"); + properties.put("mail.smtp.starttls.enable", "false"); + + + try { + Session session = Session.getInstance(properties); + Transport transport = session.getTransport(); + transport.connect(); + assertTrue(true, "Successfully connected to MailDev SMTP server without authentication."); + + transport.close(); + } catch (MessagingException e) { + fail("Failed to connect to MailDev SMTP server: " + e.getMessage()); + } + + } + @Test public void testTestDatasource_withCorrectCredentials_returnsWithoutInvalids() { DatasourceConfiguration dsConfig = createDatasourceConfiguration(); From 16834f3eed0f3db0a980bd2b9d7030ab3d3da192 Mon Sep 17 00:00:00 2001 From: "muhammed.shanid@zemosolabs.com" Date: Wed, 13 Nov 2024 17:43:20 +0530 Subject: [PATCH 3/4] updated test case --- .../com/external/plugins/SmtpPluginTest.java | 54 +++++++------------ 1 file changed, 20 insertions(+), 34 deletions(-) diff --git a/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java b/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java index e9766988e8d6..8e1974c5935b 100644 --- a/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java +++ b/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java @@ -1,6 +1,5 @@ package com.external.plugins; -import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; import com.appsmith.external.helpers.PluginUtils; import com.appsmith.external.models.ActionConfiguration; @@ -53,7 +52,7 @@ public class SmtpPluginTest { + password + " -s 25"); @Container - public static final GenericContainer smtpWithoutAuth = new GenericContainer<>(DockerImageName.parse("maildev/maildev")) + public static final GenericContainer smtpWithoutAuth = new GenericContainer(DockerImageName.parse("maildev/maildev")) .withExposedPorts(1025) .withCommand("bin/maildev --base-pathname /maildev --smtp-port 1025 --incoming-user '' --incoming-pass ''"); @@ -61,8 +60,13 @@ public class SmtpPluginTest { @BeforeAll public static void setup() { - host = smtp.getContainerIpAddress(); - port = Long.valueOf(smtp.getFirstMappedPort()); + //Initialize SMTP connection with default configuration (can be changed per test) + configureSmtpConnection(smtp); //Default + } + + private static void configureSmtpConnection(GenericContainer container) { + host = container.getContainerIpAddress(); + port = Long.valueOf(container.getFirstMappedPort()); } private DatasourceConfiguration createDatasourceConfiguration() { @@ -127,40 +131,22 @@ public void testInvalidPort() { assertEquals(Set.of(), pluginExecutor.validateDatasource(invalidDatasourceConfiguration)); } - @Test - public void testNullAuthentication() { - DatasourceConfiguration noAuthDatasourceConfiguration = createDatasourceConfiguration(); - noAuthDatasourceConfiguration.setAuthentication(null); - - assertEquals(0, pluginExecutor.validateDatasource(noAuthDatasourceConfiguration).size()); - } - @Test public void testConnectionWithoutAuth() { + configureSmtpConnection(smtpWithoutAuth); + DatasourceConfiguration noAuthDatasourceConfiguration = createDatasourceConfiguration(); + noAuthDatasourceConfiguration.setAuthentication(null); // No authentication - String smtpHost = "localhost"; - int smtpPort = smtpWithoutAuth.getMappedPort(1025); - - Properties properties = new Properties(); - properties.put("mail.transport.protocol", "smtp"); - properties.put("mail.smtp.host", smtpHost); - properties.put("mail.smtp.port", String.valueOf(smtpPort)); - // No authentication - properties.put("mail.smtp.auth", "false"); - properties.put("mail.smtp.starttls.enable", "false"); - - - try { - Session session = Session.getInstance(properties); - Transport transport = session.getTransport(); - transport.connect(); - assertTrue(true, "Successfully connected to MailDev SMTP server without authentication."); - - transport.close(); - } catch (MessagingException e) { - fail("Failed to connect to MailDev SMTP server: " + e.getMessage()); - } + Mono testDatasourceMono = pluginExecutor.testDatasource(noAuthDatasourceConfiguration); + StepVerifier.create(testDatasourceMono) + .assertNext(datasourceTestResult -> { + assertNotNull(datasourceTestResult); + assertTrue(datasourceTestResult.isSuccess()); + assertTrue(datasourceTestResult.getInvalids().isEmpty()); + }) + .verifyComplete(); + configureSmtpConnection(smtp); } @Test From 01a58fddb717dcfb34231012be7e108f6e52f8dd Mon Sep 17 00:00:00 2001 From: "muhammed.shanid@zemosolabs.com" Date: Thu, 14 Nov 2024 13:25:48 +0530 Subject: [PATCH 4/4] removed wild card imports from the prod code --- .../java/com/external/plugins/SmtpPluginTest.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java b/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java index 8e1974c5935b..2ecd5af2000b 100644 --- a/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java +++ b/app/server/appsmith-plugins/smtpPlugin/src/test/java/com/external/plugins/SmtpPluginTest.java @@ -27,10 +27,18 @@ import reactor.core.publisher.Mono; import reactor.test.StepVerifier; -import java.util.*; + +import java.util.List; +import java.util.Map; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Set; +import java.util.ArrayList; import java.util.stream.Collectors; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock;