fix: fixed smtp code to add starttls disabled if credentials are not provided (#40005)
## Background [This PR](https://github.com/appsmithorg/appsmith/pull/37319) updated the SMTP datasource in Appsmith to support configurations without requiring a username and password. However, it inadvertently enforced the `mail.smtp.starttls.enable` property even when credentials were not provided. This led to an issue where STARTTLS was unnecessarily enforced, causing problems for users configuring SMTP without authentication, as detailed in [this issue](https://github.com/appsmithorg/appsmith/issues/39965). ## Description This PR resolves the issue by ensuring that the starttls and auth properties are only set when a username and password are provided. This change allows the SMTP datasource to function correctly without authentication, aligning with user expectations and preventing unnecessary STARTTLS enforcement. Fixes #39965 ## Automation /ok-to-test tags="@tag.Datasource" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/14192755822> > Commit: 7334f48a5678490cb1468b4f6460546e7496a3cc > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=14192755822&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Datasource` > Spec: > <hr>Tue, 01 Apr 2025 10:42:30 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved the email configuration experience by ensuring that security settings for authentication and encrypted connections are applied only when valid credentials are provided, resulting in more reliable behavior for both authenticated and non-authenticated email server setups. - Enhanced validation in tests to confirm that session properties reflect the correct settings based on authentication presence. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
ce974faab6
commit
0eb5939cc9
|
|
@ -205,8 +205,6 @@ public class SmtpPlugin extends BasePlugin {
|
|||
|
||||
Properties prop = new Properties();
|
||||
prop.put("mail.transport.protocol", "smtp");
|
||||
prop.put("mail.smtp.auth", true);
|
||||
prop.put("mail.smtp.starttls.enable", "true");
|
||||
prop.put("mail.smtp.host", endpoint.getHost());
|
||||
Long port = (endpoint.getPort() == null || endpoint.getPort() < 0) ? 25 : endpoint.getPort();
|
||||
prop.put("mail.smtp.port", String.valueOf(port));
|
||||
|
|
@ -218,6 +216,10 @@ public class SmtpPlugin extends BasePlugin {
|
|||
&& StringUtils.hasText(authentication.getUsername())
|
||||
&& StringUtils.hasText(authentication.getPassword())) {
|
||||
|
||||
// Set authentication specific properties only when credentials are provided
|
||||
prop.put("mail.smtp.auth", "true");
|
||||
prop.put("mail.smtp.starttls.enable", "true");
|
||||
|
||||
String username = authentication.getUsername();
|
||||
String password = authentication.getPassword();
|
||||
|
||||
|
|
@ -228,6 +230,7 @@ public class SmtpPlugin extends BasePlugin {
|
|||
}
|
||||
});
|
||||
} else {
|
||||
// For non-authenticated SMTP servers
|
||||
prop.put("mail.smtp.auth", "false");
|
||||
session = Session.getInstance(prop);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -37,6 +37,7 @@ 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.assertNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
import static org.mockito.Mockito.anyString;
|
||||
import static org.mockito.Mockito.doNothing;
|
||||
|
|
@ -145,6 +146,15 @@ public class SmtpPluginTest {
|
|||
DatasourceConfiguration noAuthDatasourceConfiguration = createDatasourceConfiguration();
|
||||
noAuthDatasourceConfiguration.setAuthentication(null); // No authentication
|
||||
|
||||
// First create the session and verify its properties
|
||||
Session session =
|
||||
pluginExecutor.datasourceCreate(noAuthDatasourceConfiguration).block();
|
||||
assertNotNull(session);
|
||||
assertEquals("false", session.getProperties().getProperty("mail.smtp.auth"));
|
||||
// Verify STARTTLS is not enabled when there's no authentication
|
||||
assertNull(session.getProperties().getProperty("mail.smtp.starttls.enable"));
|
||||
|
||||
// Then test the datasource connection
|
||||
Mono<DatasourceTestResult> testDatasourceMono = pluginExecutor.testDatasource(noAuthDatasourceConfiguration);
|
||||
|
||||
StepVerifier.create(testDatasourceMono)
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user