appsmithorg / appsmith

Platform to build admin panels, internal tools, and dashboards. Integrates with 25+ databases and any API.
https://www.appsmith.com
Apache License 2.0
34.6k stars 3.73k forks source link

[Bug]: SMTP datasource should work without username and password #37271

Closed sneha122 closed 21 hours ago

sneha122 commented 1 week ago

Is there an existing issue for this?

Description

When configuring SMTP datasource without username and password, always throws error Invalid authentication credentials. Please check datasource configuration.

Ideally we should be able to configure SMTP datasource without username and password too. More specifically we need to remove the manual validations added on SMTP code base as it is possible to configure smtp service without password as well

Screenshot 2024-11-07 at 11 37 57 AM

Steps To Reproduce

  1. Use any email service for configuring SMTP datasource without setting up username and password
  2. Create SMTP datasource with host port, keeping username and password blank
  3. Test the configuration

Public Sample App

No response

Environment

Production

Severity

Medium (Frustrating UX)

Issue video log

No response

Version

v1.47

shanid544 commented 1 week ago

@sneha122 Shall I pick this?

sneha122 commented 1 week ago

@shanid544 Sure you can pick this up, could you please let me know what test case are you planning to add here?

shanid544 commented 1 week ago

@sneha122 Sure, I will do.

shanid544 commented 1 week ago

@sneha122 I used - docker run -d -p 1025:1025 -p 8025:8025 mailhog/mailhog, able to reproduce the issue & fixed the issue, now we can configure SMTP with & without credentials. will test one more time & attach screenshots

sneha122 commented 1 week ago

Thanks @shanid544 that's super helpful, can you please add relevant test cases along with the fix if possible?

shanid544 commented 1 week ago

@sneha122 , Will update the existing test case

shanid544 commented 1 week ago

@sneha122 case 1: Suppose user adding username alone/password alone as the part of conf, test should pass, or custom error saying "Username and password must both be provided"?

sneha122 commented 1 week ago

@shanid544 Is it even possible to configure SMTP datasource with just the username or just the password? I don't think it is. But if it is then we need the test to pass in such cases

sneha122 commented 4 days ago

Hi @shanid544 Were you able to figure out the test cases for this issue? If yes are you planning to raise a PR for this anytime soon?

shanid544 commented 4 days ago

@sneha122 will raise soon

shanid544 commented 4 days ago

Could you please review this

Solution Details

To fix this, the following changes were implemented: Updated Validation Method (validateDatasource) Before: Enforced mandatory username and password validation. After: Removed the strict validation check for authentication fields, allowing for configurations without credentials.

if (authentication == null || !StringUtils.hasText(authentication.getUsername()) || !StringUtils.hasText(authentication.getPassword())) {
    invalids.add(new AppsmithPluginException(AppsmithPluginError.PLUGIN_AUTHENTICATION_ERROR).getMessage());
}

Modified the SMTP Session Creation (datasourceCreate) Before: Always initialized the SMTP session with authentication, assuming credentials were required. After: Updated the session creation to support both authenticated and unauthenticated configurations.


Properties prop = new Properties();
prop.put("mail.smtp.auth", "false"); // Default to no authentication

if (authentication != null && StringUtils.hasText(authentication.getUsername()) && StringUtils.hasText(authentication.getPassword())) {
    prop.put("mail.smtp.auth", "true");
    session = Session.getInstance(prop, new Authenticator() {
        @Override
        protected PasswordAuthentication getPasswordAuthentication() {
            return new PasswordAuthentication(username, password);
        }
    });
} else {
    session = Session.getInstance(prop);
}

Enhanced Testing for Authentication Scenarios (testDatasource) Before: Errors were logged if authentication failed, even for servers where authentication wasn’t required. After: Introduced a flag to detect if authentication was required based on the session configuration, and adjusted error handling accordingly.

boolean isAuthRequired = "true".equals(connection.getProperty("mail.smtp.auth"));
if (isAuthRequired && transport != null) {
    try {
        transport.connect();
    } catch (AuthenticationFailedException e) {
        invalids.add(SMTPErrorMessages.DS_AUTHENTICATION_FAILED_ERROR_MSG);
    }
}

Testing and Verification Unit Tests Without Authentication: Updated testNullAuthentication test case to ensure no errors are returned when authentication is absent.

@Test
public void testNullAuthentication() {
    DatasourceConfiguration invalidDatasourceConfiguration = createDatasourceConfiguration();
    invalidDatasourceConfiguration.setAuthentication(null);
    assertEquals(0, pluginExecutor.validateDatasource(invalidDatasourceConfiguration).size());
}

manual test: Manual Testing

  1. SMTP Server Testing with MailHog

Environment: Configured SMTP datasource using MailHog, set up via Docker. Steps: Configured the SMTP datasource in Appsmith without authentication (no username or password). Screenshot from 2024-11-11 14-44-11 Created a query to send an email. Screenshot from 2024-11-11 14-44-56 Result: Email was successfully sent without requiring authentication credentials. Screenshot from 2024-11-11 14-45-32

  1. Gmail SMTP Server Testing (Without Authentication)

Environment: Configured SMTP datasource using Gmail's SMTP server without providing authentication credentials. Steps: Set up the Gmail SMTP server in Appsmith without entering a username or password. Screenshot from 2024-11-11 14-46-49 Created a query to send an email. Result: Appsmith was able to connect to the Gmail SMTP server, and the email was successfully sent without authentication credentials. Screenshot from 2024-11-11 14-48-27 Expected Result: Error should be thrown due to missing authentication credentials. Gmail requires authentication for sending emails, so the system should reject connections without credentials. Go back and edit the datasource - add username and password, Will be able to send the mail.

  1. Gmail SMTP Server Testing (With Authentication)

Environment: Configured SMTP datasource using Gmail's SMTP server with valid username and password. Steps: Entered Gmail SMTP server details along with valid authentication credentials in Appsmith. Created a query to send an email. Result: Email was successfully sent using the authenticated configuration.

shanid544 commented 4 days ago

@sneha122 Could you please review this PR

sneha122 commented 4 days ago

Thank you @shanid544 for the PR, I will review this