You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2021/02/18 22:57:55 UTC

[GitHub] [fineract] ptuomola commented on a change in pull request #1597: Add condition to start TLS (FINERACT-1070)

ptuomola commented on a change in pull request #1597:
URL: https://github.com/apache/fineract/pull/1597#discussion_r578790020



##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/configuration/service/ExternalServicesPropertiesReadPlatformServiceImpl.java
##########
@@ -87,8 +87,6 @@ public SMTPCredentialsData extractData(final ResultSet rs) throws SQLException,
                     password = value;
                 } else if (ExternalServicesConstants.SMTP_HOST.equalsIgnoreCase(name)) {
                     host = value;
-                } else if (ExternalServicesConstants.SMTP_PORT.equalsIgnoreCase(name)) {
-                    port = value;
                 } else if (ExternalServicesConstants.SMTP_USE_TLS.equalsIgnoreCase(name)) {

Review comment:
       Why are we no longer reading the port from the database? 

##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/GmailBackedPlatformEmailService.java
##########
@@ -55,31 +54,42 @@ public void sendToUserAccount(String organisationName, String contactName, Strin
 
     @Override
     public void sendDefinedEmail(EmailDetail emailDetails) {
-        final Email email = new SimpleEmail();
         final SMTPCredentialsData smtpCredentialsData = this.externalServicesReadPlatformService.getSMTPCredentials();
 
         final String authuser = smtpCredentialsData.getUsername();
         final String authpwd = smtpCredentialsData.getPassword();
 
-        // Very Important, Don't use email.setAuthentication()
-        email.setAuthenticator(new DefaultAuthenticator(authuser, authpwd));
-        email.setDebug(false); // true if you want to debug
-        email.setHostName(smtpCredentialsData.getHost());
+        JavaMailSenderImpl mailSender = new JavaMailSenderImpl();
+        mailSender.setHost(smtpCredentialsData.getHost()); // smtp.gmail.com
+        mailSender.setPort(Integer.parseInt(smtpCredentialsData.getPort())); // 587
+
+        // Important: Enable less secure app access for the gmail account used in the following authentication
+
+        mailSender.setUsername(authuser); // use valid gmail address
+        mailSender.setPassword(authpwd); // use password of the above gmail account
+
+        Properties props = mailSender.getJavaMailProperties();
+        props.put("mail.transport.protocol", "smtp");
+        props.put("mail.smtp.auth", "true");
+        props.put("mail.debug", "true");
 
         try {
             if (smtpCredentialsData.isUseTLS()) {
-                // FINERACT-1070: NOT email.setSSLOnConnect(true); email.setSslSmtpPort(smtpCredentialsData.getPort());
-                email.setStartTLSRequired(true);
+                if (smtpCredentialsData.getPort().equals("465")) {

Review comment:
       I don't think hardcoding ports is a good idea...

##########
File path: fineract-provider/dependencies.gradle
##########
@@ -80,7 +80,8 @@ dependencies {
             'org.webjars:webjars-locator-core',
 
             'com.google.cloud.sql:mysql-socket-factory-connector-j-8:1.2.1',
-            'com.squareup.retrofit2:converter-gson'
+            'com.squareup.retrofit2:converter-gson',
+            'com.sun.activation:jakarta.activation:1.2.2'

Review comment:
       In addition to adding the implementation jar here, you need to add an exclusion for the API that is brought in by an implied dependency from jakarta-xml.bind-api. Otherwise you get duplicates for the API classes.

##########
File path: fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/GmailBackedPlatformEmailService.java
##########
@@ -55,31 +54,42 @@ public void sendToUserAccount(String organisationName, String contactName, Strin
 
     @Override
     public void sendDefinedEmail(EmailDetail emailDetails) {
-        final Email email = new SimpleEmail();
         final SMTPCredentialsData smtpCredentialsData = this.externalServicesReadPlatformService.getSMTPCredentials();
 
         final String authuser = smtpCredentialsData.getUsername();
         final String authpwd = smtpCredentialsData.getPassword();
 
-        // Very Important, Don't use email.setAuthentication()
-        email.setAuthenticator(new DefaultAuthenticator(authuser, authpwd));
-        email.setDebug(false); // true if you want to debug
-        email.setHostName(smtpCredentialsData.getHost());
+        JavaMailSenderImpl mailSender = new JavaMailSenderImpl();
+        mailSender.setHost(smtpCredentialsData.getHost()); // smtp.gmail.com

Review comment:
       Rather than creating an instance of JavaMailSenderImpl directly here, wouldn't the right solution be to declare JavaMailSender as a bean and let Spring provide it through dependency injection?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org